[U-Boot] [PATCH] mmc: sdhci: consider SDHCI_QUIRK_NO_HISPD_BIT in host_caps

[Jaehoon Chung]

Dear Hannes,

On 03/02/2018 04:16 PM, Hannes Schmelzer wrote:
>>
>> On 03/01/2018 11:36 PM, Hannes Schmelzer wrote:
>>> Some IP-core implementations of the SDHCI have different troubles on 
> the
>>> silicon where they are placed.
>>>
>>> On ZYNQ platform for example Xilinx doesn't accept the hold timing of 
> an
>>> eMMC chip which operates in High-Speed mode. Due to this fact the
>>> "SDHCI_QUIRK_NO_HISPD_BIT" quirk was born.
>>>
>>> This commit reflects this fact within the host-controller 
> capabilities,
>>> so that the layer above (mmc-driver) can setup the card correctly.
>>>
>>> Otherwise the MMC card will be switched into high-speed mode and 
> causes
>>> possible timing violation on the host-controller side.
>>>
>>> Signed-off-by: Hannes Schmelzer <oe5hpm at oevsv.at>
>>
>> SDHCI_QUIRK_NO_HISPD_BIT had added because of Samsung SoC.
>> (I had added this quirk, so i know exactly what purpose.)
> 
> Also on other platforms this bit is used, have look to the zynq_sdhci.c
> https://github.com/u-boot/u-boot/blob/master/drivers/mmc/zynq_sdhci.c
> 
> 56 #ifdef CONFIG_ZYNQ_HISPD_BROKEN 
> 57      host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT; 
> 58 #endif 
> 
>> Samsung SoC didn't use the SDHCI_CTRL_HISPD bit at HOST_CNOTROL 
> register. 
>> Instead, its bit is used to other purpose.
> 
> Please explain me more about this other purpose,
> maybe i'm missunterstand here something.

SD Host Controller has the HOST Controller register (Offset 0x28).
BIT[2] is "High speed enable".
- BIT[2] : 0 - Normal Speed mode
- BIT[2] : 1 - High Speed mode.
This bit is "RW" attribute.

And Samsung SoC also has the HOST Controller register (offset 0x28).
BIT[2] is "Output Edge Inversion". 
- BIT[2] : 0 - Rising edge output.
- BIT[2] : 1 - Falling edge output.

"High speed enable" bit is standard. but Samsung SoC is using it for other purpose.
So I had added the quirks for SDHCI_QUIRK_NO_HISPD_BIT.
It means that there is no 'high speed bit'.

If your patch is applied, Samsung SoC can work the unexpected behavior.

If your ip also doesn't use this bit, then no problem. 
But your patch tried to use the other purpose.

> 
> Maybe i've to introduce some new quirk, which tells the ip-core and the 
> other layers> above not using HIGH_SPEED. But i think still this is the right one ... 

Well, i don't want to add the new quirk, if there is a solution not to add the quriks.
If really needs, you need to add the SDHCI_QUIRK_BROKEN_HISPD_MODE.

#define SDHCI_QUIRK_BROKEN_HISPD_MODE (1 << 5)

and change from SDHCI_QUIRK_NO_HSIPD_BIT to SDHCI_QUIRK_BROKEN_HISPD_MODE in zynq_sdhci.c.

I will update the descriptions about quirks. Because it can be confused what purpose is used.

Best Regards,
Jaehoon Chung

> please have a
> look to:
> 
> https://github.com/u-boot/u-boot/blob/master/drivers/mmc/mmc.c
> 
> static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps)
> {
>         ....
> }
> 
> Here is the matching between host and card capabilities done:
> 1850    /* Restrict card's capabilities by what the host can do */ 
> 1851    card_caps &= mmc->host_caps; 
> 
> Today only the host side behaves correctly if "CONFIG_ZYNQ_HISPD_BROKEN" 
> is set.
> But the card still operates in HS-Mode and thats wrong.
> 
>> Best Regards,
>> Jaehoon Chung
> cheers,
> Hannes
> 
> 
> 
> 
>