[U-Boot] [PATCH v4 01/18] mmc: show hardware partition sizes in mmcinfo output

[Diego Santa Cruz]

> -----Original Message-----
> From: Pantelis Antoniou [mailto:panto at antoniou-consulting.com]
> Sent: Friday, January 23, 2015 9:35 AM
> To: Diego Santa Cruz
> Cc: Stephen Warren; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH v4 01/18] mmc: show hardware partition sizes in
> mmcinfo output
> 
> Hi Diego,
> 
> > On Jan 23, 2015, at 10:30 , Diego Santa Cruz <Diego.SantaCruz at spinetix.com>
> wrote:
> >
> >> -----Original Message-----
> >> From: Stephen Warren [mailto:swarren at wwwdotorg.org]
> >> Sent: Thursday, January 22, 2015 8:59 PM
> >> To: Pantelis Antoniou
> >> Cc: Diego Santa Cruz; u-boot at lists.denx.de
> >> Subject: Re: [U-Boot] [PATCH v4 01/18] mmc: show hardware partition sizes
> in
> >> mmcinfo output
> >>
> >> On 01/22/2015 12:45 PM, Pantelis Antoniou wrote:
> >>> Hi Stephen,
> >>>
> >>>> On Jan 22, 2015, at 20:42 , Stephen Warren <swarren at wwwdotorg.org> wrote:
> >>>>
> >>>> On 12/23/2014 02:50 AM, Diego Santa Cruz wrote:
> >>>>> There is currently no command that will provide an overview of the
> >> hardware
> >>>>> partitions present on an eMMC device, one has to switch to every
> partition
> >>>>> via "mmc dev" and run mmcinfo for each to get the partition's capacity.
> >>>>> This commit adds a few lines of output to mmcinfo with the sizes of the
> >>>>> present partitions, like this:
> >>>>>
> >>>>> Device: OMAP SD/MMC
> >>>>> Manufacturer ID: fe
> >>>>> OEM: 14e
> >>>>> Name: MMC16
> >>>>> Tran Speed: 52000000
> >>>>> Rd Block Len: 512
> >>>>> MMC version 4.41
> >>>>> High Capacity: Yes
> >>>>> Capacity: 13.8 GiB
> >>>>> Bus Width: 4-bit
> >>>>> User Capacity: 13.8 GiB
> >>>>> Boot Capacity: 16 MiB
> >>>>> RPMB Capacity: 128 KiB
> >>>>> GP1 Capacity: 64 MiB
> >>>>> GP2 Capacity: 64 MiB
> >>>>
> >>>> I have an MMC device which has at least boot HW partitions, yet with the
> >> very latest code in u-boot.git, I don't see the additional lines mentioned
> >> above. My HW partitions are still working fine, since I can select a boot
> >> partition and mmcinfo shows the correct "Capacity" for it:
> >>>>
> >>>> Any ideas why?
> >>>>
> >>>> Tegra124 (Jetson TK1) # mmc dev 0
> >>>> switch to partitions #0, OK
> >>>> mmc0(part 0) is current device
> >>>> Tegra124 (Jetson TK1) # mmcinfo
> >>>> Device: Tegra SD/MMC
> >>>> Manufacturer ID: 45
> >>>> OEM: 100
> >>>> Name: SEM16
> >>>> Tran Speed: 52000000
> >>>> Rd Block Len: 512
> >>>> MMC version 4.5
> >>>> High Capacity: Yes
> >>>> Capacity: 14.7 GiB <<<< Sounds right for a 16GB device with partitions
> >>>> Bus Width: 8-bit
> >>>> Erase Group Size: 512 KiB
> >>>> <<<< No HW partition information is printed here
> >>>>
> >>>> Tegra124 (Jetson TK1) # mmc dev 0 1 <<<< select "boot0" HW partition
> >>>> switch to partitions #1, OK
> >>>> mmc0(part 1) is current device
> >>>> Tegra124 (Jetson TK1) # mmcinfo
> >>>> Device: Tegra SD/MMC
> >>>> Manufacturer ID: 45
> >>>> OEM: 100
> >>>> Name: SEM16
> >>>> Tran Speed: 52000000
> >>>> Rd Block Len: 512
> >>>> MMC version 4.5
> >>>> High Capacity: Yes
> >>>> Capacity: 4 MiB <<<< "boot0" partition size correctly reported
> >>>> Bus Width: 8-bit
> >>>> Erase Group Size: 512 KiB
> >>>
> >>> That is really weird; are you sure you got the latest version of u-boot
> >>> containing those patches?
> >>>
> >>>>       if (!IS_SD(mmc) && mmc->version >= MMC_VERSION_4_41) {
> >>
> >> Ah, my device is MMC 4.5, and the version numbers aren't monotonic:
> >>
> >> #define MMC_VERSION_4_41	(MMC_VERSION_MMC | 0x429)
> >> #define MMC_VERSION_4_5		(MMC_VERSION_MMC | 0x405)
> >>
> >> Should that be 0x450, or do we need some more complex version comparison
> >> logic?
> >>
> >> FWIW, if I hack the test you quoted to always pass, then the data that's
> >> printed looks plausible. At the very least, the boot capacity agrees
> >> with Linux.
> >
> > Thanks for spotting this, looking at all the defines in mmc.h they are
> >
> > #define MMC_VERSION_UNKNOWN	(MMC_VERSION_MMC)
> > #define MMC_VERSION_1_2		(MMC_VERSION_MMC | 0x102)
> > #define MMC_VERSION_1_4		(MMC_VERSION_MMC | 0x104)
> > #define MMC_VERSION_2_2		(MMC_VERSION_MMC | 0x202)
> > #define MMC_VERSION_3		(MMC_VERSION_MMC | 0x300)
> > #define MMC_VERSION_4		(MMC_VERSION_MMC | 0x400)
> > #define MMC_VERSION_4_1		(MMC_VERSION_MMC | 0x401)
> > #define MMC_VERSION_4_2		(MMC_VERSION_MMC | 0x402)
> > #define MMC_VERSION_4_3		(MMC_VERSION_MMC | 0x403)
> > #define MMC_VERSION_4_41		(MMC_VERSION_MMC | 0x429)
> > #define MMC_VERSION_4_5		(MMC_VERSION_MMC | 0x405)
> > #define MMC_VERSION_5_0		(MMC_VERSION_MMC | 0x500)
> >
> > I do not get it why MMC_VERSION_4_41 is 0x429, it should be 0x404 to follow
> the sequence.
> >
> > Wouldn't it be sane to change it to be
> >
> > #define MMC_VERSION_4_41		(MMC_VERSION_MMC | 0x404)
> >
> > I checked mmc_startup() and these defines are not matching bitfields in CSD
> nor EXT_CSD, so I think it should be safe to change them.
> >
> 
> Changing them is one thing; we have to change the version printout too.
> 

Of course, dumb me..., forget my idea. So changing the others to 0x410, 0x420, ... 0x450, etc., as you propose, would keep version comparisons as they are and the version printout would be easier to handle.

Thanks for volunteering to fix it.

Regards,

Diego

-- 
Diego Santa Cruz, PhD
Technology Architect
spinetix.com