Re: [PATCH v2 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 12 October 2018 at 07:06, Marcin Wojtas <mw@semihalf.com> wrote:
> pt., 12 paź 2018 o 03:41 Wu, Hao A <hao.a.wu@intel.com> napisał(a):
>>
>> > -----Original Message-----
>> > From: Marcin Wojtas [mailto:mw@semihalf.com]
>> > Sent: Thursday, October 11, 2018 11:43 PM
>> > To: Wu, Hao A
>> > Cc: Ni, Ruiyu; Ard Biesheuvel; Tian, Feng; Tomasz Michalec; Dong, Eric; edk2-
>> > devel-01; Gao, Liming; nadavh@marvell.com; Kinney, Michael D; Zeng, Star
>> > Subject: Re: [edk2] [PATCH v2 2/4] MdeModulePkg/SdMmcPciHcDxe: Add
>> > UhsSignaling to SdMmcOverride protocol
>> >
>> > wt., 9 paź 2018 o 13:51 Marcin Wojtas <mw@semihalf.com> napisał(a):
>> > >
>> > > wt., 9 paź 2018 o 13:45 Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > napisał(a):
>> > > >
>> > > > On 9 October 2018 at 13:32, Marcin Wojtas <mw@semihalf.com> wrote:
>> > > > > wt., 9 paź 2018 o 13:28 Wu, Hao A <hao.a.wu@intel.com> napisał(a):
>> > > > >>
>> > > > >> > -----Original Message-----
>> > > > >> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
>> > Behalf Of Ard
>> > > > >> > Biesheuvel
>> > > > >> > Sent: Monday, October 08, 2018 11:10 PM
>> > > > >> > To: Marcin Wojtas; Ni, Ruiyu; Wu, Hao A
>> > > > >> > Cc: Tian, Feng; Tomasz Michalec; Dong, Eric; edk2-devel-01; Gao,
>> > Liming;
>> > > > >> > Nadav Haklai; Kinney, Michael D; Zeng, Star
>> > > > >> > Subject: Re: [edk2] [PATCH v2 2/4] MdeModulePkg/SdMmcPciHcDxe:
>> > Add
>> > > > >> > UhsSignaling to SdMmcOverride protocol
>> > > > >> >
>> > > > ...
>> > > > >> >
>> > > > >> > I suppose this is defined by the eMMC spec.
>> > > > >> >
>> > > > >> > Ruiyu, Hao, could you clarify? Are the host control 2 register values
>> > > > >> > for HS200/HS400 defined by the eMMC spec?
>> > > > >>
>> > > > >> Hi Ard and Marcin,
>> > > > >>
>> > > > >> As far as I know, the EMMC Electrical Standard Spec 5.1 (latest) does
>> > not
>> > > > >> mention on how to set the "UHS Mode Select" field of the Host
>> > Control 2
>> > > > >> Register when switching to HS200/HS400. (Actually, the EMMC spec
>> > does not
>> > > > >> mention Host Control 2 Register at all)
>> > > > >>
>> > > > >> When it comes to setting the bus mode for EMMC devices, the current
>> > > > >> implementation of the SdMmcPciHcDxe driver does a mapping when
>> > setting the
>> > > > >> Host Control 2 Register:
>> > > > >>
>> > > > >> EMMC High Speed SDR - Freq: 0-52 MHz, Data Rate: Single
>> > > > >> matches
>> > > > >> SD   SDR25          - Freq: 0-50 MHz, Data Rate: Single
>> > > > >>
>> > > > >> EMMC High Speed DDR - Freq: 0-52 MHz, Data Rate: Dual
>> > > > >> matches
>> > > > >> SD   DDR50          - Freq: 0-50 MHz, Data Rate: Dual
>> > > > >>
>> > > > >> EMMC HS200          - Freq: 0-200 MHz, Data Rate: Single
>> > > > >> matches
>> > > > >> SD   SDR104         - Freq: 0-208 MHz, Data Rate: Single
>> > > > >>
>> > > > >> EMMC HS400          - Freq: 0-200 MHz, Data Rate: Dual
>> > > > >> matches
>> > > > >> SD   None
>> > > > >>
>> > > > >> And there is no obvious counterpart for the EMMC HS400 mode in the
>> > SD
>> > > > >> spec. The driver currently sets the "UHS Mode Select" field to a
>> > reserved
>> > > > >> value 0x5.
>> > > > >>
>> > > > >
>> > > > > Thank you Hao, above is on par with what the default UhsSignaling
>> > > > > routine does in this patch. IMO especially in case the EMMC standard
>> > > > > is not unequivocal regarding UHS_MODE_SEL, I'd encourage to accept
>> > > > > some way of updating HostControl2 register, depending on the
>> > > > > implementation. What is your opinion Ard?
>> > > > >
>> > > >
>> > > > I would like to know where the current values in SdMmcPciHcDxe come
>> > > > from if they are not defined in any spec.
>> > > >
>> > > > How do we know which ones are the correct ones?
>> > >
>> > > Hao, can you justify used values?
>> > >
>> >
>> > Hi Hao,
>> >
>> > Can you please take a look at the UHS_MODE_SEL values source for eMMC?
>>
>> Hi Marcin,
>>
>> Sorry for the delayed response.
>>
>> For the current implementation of the SdMmcPciHcDxe driver, the selecting
>> of "UHS Mode Select" field value of the Host Control 2 Register is based
>> on a Max Clock Frequency & Data Rate (Single or Dual) matching
>> relationship between the:
>>
>> A. Table 3-6 of the SD Specifications Part 1 Physical Layer Simplified
>> Specification Version 4.10
>>
>> and
>>
>> B. Table 4 of the EMMC Electrical Standard Spec 5.1
>>
>> The matching details was included in my previous reply. The only missing
>> part is there seems no matching for the EMMC HS400 mode in the SD
>> specifications. For this case, we are currently using the same approach
>> with the Linux implementation, that is to set the "UHS Mode Select" to a
>> value of 0x5 (not standard).
>>
>
> Hao,
>
> Thanks a lot for the clarification.
>
> Ard,
>
> Knowing the numbers details, what is your view of the UhsSignaling handling?
>

I think it makes sense to be able to override the SD->MMC mapping for
HC2 attributes. But it seems to me that this mapping is rather ad-hoc
and so it should apply to all configuration that is inferred:
UhsSignalling does not quite cover it.

So I think the approach is correct, but we need a better name.