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.sch...@cherry.de>
Thanks, I'll reword this slightly to include the above when applying. -- Tom
signature.asc
Description: PGP signature
