[PATCH 6/6] block: Remove "select BLK" from non-block drivers

[Tom Rini]

On Thu, Jan 16, 2025 at 10:21:36AM +0100, Quentin Schulz wrote:
> Hi Tom,
> 
> On 1/15/25 9:20 PM, Tom Rini wrote:
> > On Wed, Jan 15, 2025 at 06:49:45PM +0100, Quentin Schulz wrote:
> > > Hi Tom,
> > > 
> > > On 1/14/25 5:59 PM, Tom Rini wrote:
> > > > On Tue, Jan 14, 2025 at 02:53:48PM +0100, Quentin Schulz wrote:
> > > > > Hi Tom,
> > > > > 
> > > > > On 12/20/24 11:22 PM, Tom Rini wrote:
> > > > > > Now that block drivers are all selecting the BLK symbol, there's no need
> > > > > > for other options to be select'ing BLK so that other required
> > > > > > functionality can be enabled. Remove these places.
> > > > > > 
> > > > > 
> > > > > We have multiple commands depending on the BLK symbol.
> > > > 
> > > > Yes.
> > > > 
> > > > > BOOTSTD also depends on it, but I assume we should be able to network boot
> > > > > without HW block drivers?
> > > > 
> > > > Correct. That's part of the motivation for this series (which I wasn't
> > > > clear enough about on its own). Without something like this series if we
> > > > remove the BLK dependency from BOOTSTD then some other platforms fail to
> > > > build or grow a bunch in size (as BOOTSTD is default y and now it's
> > > > enabled on those platforms).
> > > > 
> > > > > CMD_UFETCH wouldn't be usable without those drivers as well.
> > > > > 
> > > > > Should we do something about that by making them not depend on BLK e.g. use
> > > > > CONFIG_IS_ENABLED in the right places? Not sure if all devicess based on
> > > > > those archs have at least one HW block driver enabled. I guess checking if
> > > > > all .config before and after that change are identical would help us figure
> > > > > out if this could introduce a regression?
> > > > 
> > > > There's a few options, depending on what the command is. For CMD_UFETCH
> > > > it's likely that a small restructure would be needed to not try and
> > > 
> > > My point is that I believe this patch is too hastily removing the select BLK
> > > because some symbols have "depends on BLK" and by removing the select, we
> > > make those symbols unselectable. This can cascade if other symbols depend on
> > > those now unselectable symbols.
> > > 
> > > Also, removing the select BLK from architecture/target symbols can introduce
> > > regressions if BLK really is required?
> > > 
> > > I think a reasonable (albeit cumbersome) solution is to migrate all those
> > > selects to the impacted defconfigs so that effectively no change is made for
> > > existing devices. If maintainers want to remove BLK, they could then do it
> > > later.
> > > 
> > > This also makes sure that BOOTSTD and CMD_UFETCH (and others) are still
> > > enabled, since BLK would still be enabled, just from a different location.
> > > Then another patch series could remove BOOTSTD dependency on BLK by adapting
> > > the code, same for CMD_UFETCH (and others).
> > > 
> > > Does this make sense? Am I missing something?
> > 
> > What you're missing, I believe, is that this patch exists because prior
> > to the first patch in the series and also going back to when BLK was
> > optional, if something higher level like ARCH_ROCKCHIP didn't select BLK
> > then it wouldn't be able to prompt about its MMC driver. This is similar
> > to all of the high level places that "select DM" as that used to be
> > meaningful but now it's not. As part of testing the series I did my
> > world build before/after and the only change is that as noted in the
> > cover letter, espresso7420 now has MMC again. Does this help? Thanks.
> > 
> 
> Please add this to the patch commit log as well. The cover letter content
> doesn't make it to the git history :)
> 
> But with the world build you've done, I'm now confident what I was worried
> about will not happen, therefore:
> 
> Reviewed-by: Quentin Schulz <quentin.schulz at cherry.de>

Thanks, I'll reword this slightly to include the above when applying.

-- 
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/20250116/a4e30fc8/attachment.sig>