[PATCH 4/8] env: Rename SYS_MMC_ENV_DEV to ENV_SDMMC_DEVICE_INDEX

[Tom Rini]

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
-------------- 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/20250327/8da7b89b/attachment.sig>