[PATCH RFT v4 3/3] fastboot: integrate block flashing back-end

[Tom Rini]

On Fri, Jun 06, 2025 at 11:23:54AM +0200, Neil Armstrong wrote:
> On 06/06/2025 09:22, Mattijs Korpershoek wrote:
> > On jeu., juin 05, 2025 at 19:48, Neil Armstrong <neil.armstrong at linaro.org> wrote:
> > 
> > > On 05/06/2025 16:21, Tom Rini wrote:
> > > > On Thu, Jun 05, 2025 at 10:16:54AM +0200, Neil Armstrong wrote:
> > > > > On 22/05/2025 16:39, Tom Rini wrote:
> > > > > > On Thu, May 22, 2025 at 02:37:07PM +0200, Neil Armstrong wrote:
> > > > > > 
> > > > > > > From: Dmitrii Merkurev <dimorinny at google.com>
> > > > > > > 
> > > > > > > 1. Get partition info/size
> > > > > > > 2. Erase partition
> > > > > > > 3. Flash partition
> > > > > > > 4. BCB
> > > > > > > 
> > > > > > > Signed-off-by: Dmitrii Merkurev <dimorinny at google.com>
> > > > > > > Reviewed-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
> > > > > > > Tested-by: Mattijs Korpershoek <mkorpershoek at kernel.org>
> > > > > > > Signed-off-by: Neil Armstrong <neil.armstrong at linaro.org>
> > > > > > > ---
> > > > > > >     drivers/fastboot/Kconfig      | 29 +++++++++++++++++++++++++++++
> > > > > > >     drivers/fastboot/Makefile     |  1 +
> > > > > > >     drivers/fastboot/fb_command.c |  8 ++++++++
> > > > > > >     drivers/fastboot/fb_common.c  | 22 ++++++++++++++++++----
> > > > > > >     drivers/fastboot/fb_getvar.c  |  8 +++++++-
> > > > > > >     5 files changed, 63 insertions(+), 5 deletions(-)
> > > > > > 
> > > > > > I know this was posted before I replied with more feedback moments ago.
> > > > > > 
> > > > > > [snip]
> > > > > > > @@ -193,6 +197,31 @@ config FASTBOOT_MMC_USER_NAME
> > > > > > >     	  defined here.
> > > > > > >     	  The default target name for erasing EMMC_USER is "mmc0".
> > > > > > > +config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
> > > > > > > +	string "Define FASTBOOT block interface name"
> > > > > > > +	depends on FASTBOOT_FLASH_BLOCK
> > > > > > > +	default ""
> > > > > > > +	help
> > > > > > > +	  The fastboot "flash" and "erase" commands support operations
> > > > > > > +	  on any Block device, this should specify the block device name
> > > > > > > +	  like ide, scsi, usb, sata, nvme, virtio, blkmap, mtd...
> > > > > > > +	  The mmc block device type can be used but most of the features
> > > > > > > +	  available in the FASTBOOT_MMC will be missing.
> > > > > > > +	  Consider using FASTBOOT_MMC on a MMC block device until all
> > > > > > > +	  features are migrated.
> > > > > > 
> > > > > > A default like "" in order to un-stick configs that are now here and
> > > > > > enabling the option is wrong. If we're enabling new functionality for
> > > > > > platforms, it needs to be configured correctly. This leads to building
> > > > > > code on platforms that won't be used on the platform so we've likely
> > > > > > added run-time bloat for no benefit.
> > > > > 
> > > > > I agree but what's the solution ? I'll prefer having no default as it was initially.
> > > > 
> > > > No defaults is correct here, yes. It's just that the primary
> > > > dependencies need to be fixed so that platforms don't get stuck on the
> > > > prompt on features they won't actually use either.
> > > > 
> > > > Seeing what boards get stuck, and then having an idea on what
> > > > dependencies trip them up is tricky. What I usually do in this
> > > > situation, to see what platform is stuck on the prompt is:
> > > > - In one terminal, fire off tools/qconfig -sC. Then wait for it to
> > > >     seemingly be stuck with just one or two platforms left to finish
> > > >     syncing.
> > > > - In another terminal, 'ps uxwwww | grep make' to see what the build
> > > >     directory of one of those stuck platforms is, then manually save off
> > > >     the .config file, begin investigation.
> > > > 
> > > > That should provide what platform is asking this question and not having
> > > > a reasonable answer. Then it's a matter of seeing if it makes sense for
> > > > this platform to be here and so just needs to be migrated to this
> > > > functionality or if it's here because of some dependency problem, for
> > > > example what I was talking about in the previous part of this series.
> > > > 
> > > 
> > > Ok I can't reproduce the crash with the last version, somehow v4 fixed it,
> > > and the changes I did still work:
> > > https://source.denx.de/u-boot/custodians/u-boot-ufs/-/pipelines/26523
> > 
> > Hmm, maybe v4 "fixed" it because we have:
> > 
> > default 0 for FASTBOOT_FLASH_BLOCK_DEVICE_ID
> > 
> > If you drop that, do you still not reproduce? (note that we don't care
> > as much since we agreed upon using "default 0" for device id, but it's
> > odd that the build issue is no longer there.
> 
> Yes confirmed I removed the "default 0" and it failed again.

So the question is where does it fail, and are those platforms which
want this feature (and a default of 0 is correct) or is this covering up
an error (platform doesn't want / use this, but now builds it anyways
and it's not configured correctly or usefully for the platform) ?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250606/eb5a646b/attachment.sig>