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

[Marcin Wojtas <mw@semihalf.com>]

pt., 12 paź 2018 o 17:55 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
>
> 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.

Do you mean to update more fields in HC2 than UHS_MODE_SEL?