On Wed, Mar 26, 2025 at 10:22:34AM +0100, Quentin Schulz wrote: > Hi Marek, > > On 3/26/25 2:37 AM, Marek Vasut wrote: > > On 3/26/25 1:15 AM, Tom Rini wrote: > > > On Wed, Mar 26, 2025 at 12:02:08AM +0100, Marek Vasut wrote: > > > > On 3/24/25 1:39 PM, Quentin Schulz wrote: > > > > > Hi Marek, > > > > > > > > Hi, > > > > > > > > > On 3/22/25 10:55 PM, Marek Vasut wrote: > > > > > > Rename the variable and add ENV_ prefix, so that all configuration > > > > > > options which are related to environment would have an CONFIG_ENV_ > > > > > > prefix. No functional change. > > > > > > > > > > > > Use ENV_SDMMC_DEVICE_INDEX to clarify this is the SD/MMC device > > > > > > index, a number, as enumerated by U-Boot. Update the help text > > > > > > accordingly. > > > > > > > > > > > > > > > > I disagree with this part of the rename (MMC->SDMMC), our drivers all > > > > > are using mmc (env/mmc.*, drivers/mmc/*, cmd/mmc.c, ...) for > > > > > both SD and > > > > > MMC. We also have ENV_IS_IN_MMC which is for both. I don't think it's > > > > > worth the confusion. > > > > > > > > My main problem is, that it is confusing to call everything > > > > _mmc_ even if > > > > most of it also applies to SD . There is no way to discern parts > > > > which are > > > > MMC specific from parts which are common to SD and MMC now. > > > > > > Do we have support for anything using the MMC part of the standard that > > > is not storage? I certainly agree it's confusingly named atm, but I > > > think changing some parts but not other parts will make it worse, not > > > better. > > That's not what I am concerned about. What I am concerned about is that > > we are talking about partitions, and only eMMC supports HW partitions, > > but both SD and MMC support SW partitions . So we should refer to > > CONFIG_..._SDMMC_..._SW_PARTITION here when talking about SW partitions > > , but CONFIG_..._MMC_..._HW_PARTITION when talking about eMMC HW > > partitions. Currently we are missing the two and it yields a horrid > > confusion. That is in fact what prompted this series. > > But the controllers handling SD cards or eMMC are both called mmc in device > tree and in U-Boot (see mmc command). So for me, MMC covers both. As a > matter of fact, the mmc command also handles the boot partition that is > specific to eMMC. > > Quite interestingly, the MMC (MultiMediaCard) Wikipedia page starts with > > "Not to be confused with SD card or eMMC" and then the first sentence in the > history section is "The latest version of the eMMC standard", c.f. > https://en.wikipedia.org/wiki/MultiMediaCard > > So essentially, I'm wondering: > - if boot partitions are specific to eMMC and not simpy MMC (and are not > part of SD card). I know eMMC has a separate standard than MMC. > - if there are other things than eMMC (and SD cards) that are supported by > the MMC subsystem (outside of the embedded world). It seems like this is > taking the MMC = eMMC shortcut that I'm not sure is correct? > > To add to the confusion, Rockchip often has MMC controllers named SDMMC (and > their use varies depending on the SoC, sometimes for SD, sometimes for eMMC, > sometimes for SDIO).
This I think is part of the problem (and re Rockchip, my recollection is that at least some TI parts and manuals have similar naming issues). It's a terribly confusing area to start without and outside of U-Boot itself. My suggestions in general would be: - Don't rename files / commands (and this series was not doing so either). - Whenever possible take things out of CONFIG_SYS / CFG_SYS and instead rename them at least partially for CONFIG_ENV_... or CONFIG_MMC_... or CONFIG_SPL_... - We only have two "SDMMC" symbols so lets not add more - Use "MMC_" as the namespace and help text (and where useful, doc/ documentation) to further explain to humans what is being implemented. -- Tom
signature.asc
Description: PGP signature