| Message ID | 20250522-topic-fastboot-blk-v4-3-af7f7f30564d@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fastboot: add support for generic block flashing | expand |
On Thu, May 22, 2025 at 02:37:07PM +0200, Neil Armstrong wrote: > From: Dmitrii Merkurev <dimorinny@google.com> > > 1. Get partition info/size > 2. Erase partition > 3. Flash partition > 4. BCB > > Signed-off-by: Dmitrii Merkurev <dimorinny@google.com> > Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> > Tested-by: Mattijs Korpershoek <mkorpershoek@kernel.org> > Signed-off-by: Neil Armstrong <neil.armstrong@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. > +config FASTBOOT_FLASH_BLOCK_DEVICE_ID > + int "Define FASTBOOT block device identifier" > + depends on FASTBOOT_FLASH_BLOCK > + default 0 > + help > + The fastboot "flash" and "erase" commands support operations > + on any Block device, this should specify the block device > + identifier on the system, as a number. > + The device identifier should be 0 for first device on the > + interface type specified in FLASH_BLOCK_INTERFACE_NAME config, > + 1 the second, etc... This help should be one paragraph and note something along the lines of: - Device identifiers are numbered starting from 0. - The most common case is to use the first controller. And then yes, "default 0" is fine here because it is a reasonable default when configuring the system to use the functionality.
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@google.com> >> >> 1. Get partition info/size >> 2. Erase partition >> 3. Flash partition >> 4. BCB >> >> Signed-off-by: Dmitrii Merkurev <dimorinny@google.com> >> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> >> Tested-by: Mattijs Korpershoek <mkorpershoek@kernel.org> >> Signed-off-by: Neil Armstrong <neil.armstrong@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. > >> +config FASTBOOT_FLASH_BLOCK_DEVICE_ID >> + int "Define FASTBOOT block device identifier" >> + depends on FASTBOOT_FLASH_BLOCK >> + default 0 >> + help >> + The fastboot "flash" and "erase" commands support operations >> + on any Block device, this should specify the block device >> + identifier on the system, as a number. >> + The device identifier should be 0 for first device on the >> + interface type specified in FLASH_BLOCK_INTERFACE_NAME config, >> + 1 the second, etc... > > This help should be one paragraph and note something along the lines of: > - Device identifiers are numbered starting from 0. > - The most common case is to use the first controller. > > And then yes, "default 0" is fine here because it is a reasonable > default when configuring the system to use the functionality. Right, I'll update with that. Thanks, Neil >
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@google.com> > > > > > > 1. Get partition info/size > > > 2. Erase partition > > > 3. Flash partition > > > 4. BCB > > > > > > Signed-off-by: Dmitrii Merkurev <dimorinny@google.com> > > > Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> > > > Tested-by: Mattijs Korpershoek <mkorpershoek@kernel.org> > > > Signed-off-by: Neil Armstrong <neil.armstrong@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.
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@google.com> >>>> >>>> 1. Get partition info/size >>>> 2. Erase partition >>>> 3. Flash partition >>>> 4. BCB >>>> >>>> Signed-off-by: Dmitrii Merkurev <dimorinny@google.com> >>>> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> >>>> Tested-by: Mattijs Korpershoek <mkorpershoek@kernel.org> >>>> Signed-off-by: Neil Armstrong <neil.armstrong@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 Neil
On jeu., juin 05, 2025 at 19:48, Neil Armstrong <neil.armstrong@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@google.com> >>>>> >>>>> 1. Get partition info/size >>>>> 2. Erase partition >>>>> 3. Flash partition >>>>> 4. BCB >>>>> >>>>> Signed-off-by: Dmitrii Merkurev <dimorinny@google.com> >>>>> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> >>>>> Tested-by: Mattijs Korpershoek <mkorpershoek@kernel.org> >>>>> Signed-off-by: Neil Armstrong <neil.armstrong@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. > > Neil
On 06/06/2025 09:22, Mattijs Korpershoek wrote: > On jeu., juin 05, 2025 at 19:48, Neil Armstrong <neil.armstrong@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@google.com> >>>>>> >>>>>> 1. Get partition info/size >>>>>> 2. Erase partition >>>>>> 3. Flash partition >>>>>> 4. BCB >>>>>> >>>>>> Signed-off-by: Dmitrii Merkurev <dimorinny@google.com> >>>>>> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> >>>>>> Tested-by: Mattijs Korpershoek <mkorpershoek@kernel.org> >>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@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. Neil > >> >> Neil >
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@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@google.com> > > > > > > > > > > > > > > 1. Get partition info/size > > > > > > > 2. Erase partition > > > > > > > 3. Flash partition > > > > > > > 4. BCB > > > > > > > > > > > > > > Signed-off-by: Dmitrii Merkurev <dimorinny@google.com> > > > > > > > Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> > > > > > > > Tested-by: Mattijs Korpershoek <mkorpershoek@kernel.org> > > > > > > > Signed-off-by: Neil Armstrong <neil.armstrong@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) ?
diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig index 33825ee408fbd9aff26cd390a140421c7c98ecc3..2ea5b7f73c1338ad435e87cb1ecfbcc7728f2244 100644 --- a/drivers/fastboot/Kconfig +++ b/drivers/fastboot/Kconfig @@ -119,6 +119,10 @@ config FASTBOOT_FLASH_NAND bool "FASTBOOT on NAND" depends on MTD_RAW_NAND && CMD_MTDPARTS +config FASTBOOT_FLASH_BLOCK + bool "FASTBOOT on block device" + depends on BLK + endchoice config FASTBOOT_FLASH_MMC_DEV @@ -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. + +config FASTBOOT_FLASH_BLOCK_DEVICE_ID + int "Define FASTBOOT block device identifier" + depends on FASTBOOT_FLASH_BLOCK + default 0 + help + The fastboot "flash" and "erase" commands support operations + on any Block device, this should specify the block device + identifier on the system, as a number. + The device identifier should be 0 for first device on the + interface type specified in FLASH_BLOCK_INTERFACE_NAME config, + 1 the second, etc... + config FASTBOOT_GPT_NAME string "Target name for updating GPT" depends on FASTBOOT_FLASH_MMC && EFI_PARTITION diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile index c2214c968ab357371f5d3d27ecc9c1a3e9404e89..91e98763e8eab84ccd9b8e5354ff1419f61ef372 100644 --- a/drivers/fastboot/Makefile +++ b/drivers/fastboot/Makefile @@ -3,6 +3,7 @@ obj-y += fb_common.o obj-y += fb_getvar.o obj-y += fb_command.o +obj-$(CONFIG_FASTBOOT_FLASH_BLOCK) += fb_block.o # MMC reuses block implementation obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_block.o fb_mmc.o obj-$(CONFIG_FASTBOOT_FLASH_NAND) += fb_nand.o diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c index 2cdbac50ac4a0ce501753e95c1918ffa5d11158d..e6aee13e01618ee6567bf00527d3df327ae06f1c 100644 --- a/drivers/fastboot/fb_command.c +++ b/drivers/fastboot/fb_command.c @@ -8,6 +8,7 @@ #include <env.h> #include <fastboot.h> #include <fastboot-internal.h> +#include <fb_block.h> #include <fb_mmc.h> #include <fb_nand.h> #include <part.h> @@ -337,6 +338,10 @@ void fastboot_data_complete(char *response) */ static void __maybe_unused flash(char *cmd_parameter, char *response) { + if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK)) + fastboot_block_flash_write(cmd_parameter, fastboot_buf_addr, + image_size, response); + if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC)) fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr, image_size, response); @@ -357,6 +362,9 @@ static void __maybe_unused flash(char *cmd_parameter, char *response) */ static void __maybe_unused erase(char *cmd_parameter, char *response) { + if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK)) + fastboot_block_erase(cmd_parameter, response); + if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC)) fastboot_mmc_erase(cmd_parameter, response); diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c index 68f92c4b887c8442cc212b8613fb70c7251cdcdf..dac5528f80908bf5b1224284c9ecd492394e4f0e 100644 --- a/drivers/fastboot/fb_common.c +++ b/drivers/fastboot/fb_common.c @@ -97,16 +97,24 @@ int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason) [FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot", [FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery" }; - const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC, - CONFIG_FASTBOOT_FLASH_MMC_DEV, -1); - if (!IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC)) + int device = config_opt_enabled(CONFIG_FASTBOOT_FLASH_BLOCK, + CONFIG_FASTBOOT_FLASH_BLOCK_DEVICE_ID, -1); + if (device == -1) { + device = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC, + CONFIG_FASTBOOT_FLASH_MMC_DEV, -1); + } + const char *bcb_iface = config_opt_enabled(CONFIG_FASTBOOT_FLASH_BLOCK, + CONFIG_FASTBOOT_FLASH_BLOCK_INTERFACE_NAME, + "mmc"); + + if (device == -1) return -EINVAL; if (reason >= FASTBOOT_REBOOT_REASONS_COUNT) return -EINVAL; - ret = bcb_find_partition_and_load("mmc", mmc_dev, "misc"); + ret = bcb_find_partition_and_load(bcb_iface, device, "misc"); if (ret) goto out; @@ -226,8 +234,14 @@ void fastboot_set_progress_callback(void (*progress)(const char *msg)) */ void fastboot_init(void *buf_addr, u32 buf_size) { +#if IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK) + if (!strcmp(CONFIG_FASTBOOT_FLASH_BLOCK_INTERFACE_NAME, "mmc")) + printf("Warning: the fastboot block backend features are limited, consider using the MMC backend\n"); +#endif + fastboot_buf_addr = buf_addr ? buf_addr : (void *)CONFIG_FASTBOOT_BUF_ADDR; fastboot_buf_size = buf_size ? buf_size : CONFIG_FASTBOOT_BUF_SIZE; fastboot_set_progress_callback(NULL); + } diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index 9c2ce65a4e5bce0da6b18aa1b2818f7db556c528..f083b21c797dc7e55315f2cba017a4372483fa92 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -7,6 +7,7 @@ #include <fastboot.h> #include <fastboot-internal.h> #include <fb_mmc.h> +#include <fb_block.h> #include <fb_nand.h> #include <fs.h> #include <part.h> @@ -114,7 +115,12 @@ static int getvar_get_part_info(const char *part_name, char *response, struct disk_partition disk_part; struct part_info *part_info; - if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC)) { + if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK)) { + r = fastboot_block_get_part_info(part_name, &dev_desc, &disk_part, + response); + if (r >= 0 && size) + *size = disk_part.size * disk_part.blksz; + } else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC)) { r = fastboot_mmc_get_part_info(part_name, &dev_desc, &disk_part, response); if (r >= 0 && size)