Re: [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality

From: Ziji Hu
Date: Fri Nov 25 2016 - 08:57:29 EST

Hi Ulf,

On 2016/11/25 21:06, Ulf Hansson wrote:
> [...]
>>>>>>> +
>>>>>>> + /*
>>>>>>> + * Xenon Specific property:
>>>>>>> + * emmc: explicitly indicate whether this slot is for eMMC
>>>>>>> + * slotno: the index of slot. Refer to SDHC_SYS_CFG_INFO register
>>>>>>> + * tun-count: the interval between re-tuning
>>>>>>> + * PHY type: "sdhc phy", "emmc phy 5.0" or "emmc phy 5.1"
>>>>>>> + */
>>>>>>> + if (of_property_read_bool(np, "marvell,xenon-emmc"))
>>>>>>> + priv->emmc_slot = true;
>>>>>> So, you need this because of the eMMC voltage switch behaviour, right?
>>>>>> Then I would rather like to describe this a generic DT bindings for
>>>>>> the eMMC voltage level support. There have acutally been some earlier
>>>>>> discussions for this, but we haven't yet made some changes.
>>>>>> I think what is missing is a mmc-ddr-3_3v DT binding, which when set,
>>>>>> allows the host driver to accept I/O voltage switches to 3.3V. If not
>>>>>> supported the ->start_signal_voltage_switch() ops may return -EINVAL.
>>>>>> This would inform the mmc core to move on to the next supported
>>>>>> voltage level. There might be some minor additional changes to the mmc
>>>>>> card initialization sequence, but those should be simple.
>>>>>> I can help out to look into this, unless you want to do it yourself of course!?
>>>>> Yes. One of the reasons is to provide eMMC specific voltage setting.
>>>>> But in my very own opinion, it should be irrelevant to voltage level.
>>>>> The eMMC voltage setting on our SDHC is different from SD/SDIO voltage switch.
>>>>> It will become more complex with different SOC implementation details.
>>>> Got it. Although I think we can cope with that fine just by using the
>>>> different SD/eMMC speed modes settings defined in DT (or from the
>>>> SDHCI caps register)
>>> In my very opinion, I'm not sure if there is any corner case that driver cannot
>>> determine the eMMC card type from DT and SDHC caps.
>>>>> Unfortunately, MMC driver cannot determine the card type yet when eMMC voltage
>>>>> setting should be executed.
>>>>> Thus an flag is required here to tell driver to execute eMMC voltage setting.
>>>>> Besides, additional eMMC specific settings might be implemented in future, besides
>>>>> voltage setting. Most of them should be completed before MMC driver recognizes the
>>>>> card type. Thus I have to keep this flag to indicate current SDHC is for eMMC.
>>>> I doubt you will need a generic "eMMC" flag, but let's see when we go forward.
>>>> Currently it's clear you don't need such a flag, so I will submit a
>>>> change adding a DT binding for "mmc-ddr-3_3v" then we can take it from
>>>> there, to see if it suits your needs.
>> Another reason for a special "xenon-emmc" property is that our host IP usually can
>> support both eMMC and SD. Whether a host is used as eMMC or SD depends on the
>> final implementation of the actual product.
>> Thus our host driver needs to know whether current SDHC is fixed as eMMC or SD.
>> So far, It can only get the information from DT.
> As a matter of fact for mounted non-removable cards, such as eMMC, we
> already have the option to describe some of their characteristics in
> DT. Perhaps that's what you need?
> Please have a look at:
> Documentation/devicetree/bindings/mmc/mmc-card.txt
I will try this mmc-card sub-node.

Thank you very much.

Best regards,
Hu Ziji

>> After out host driver get the card type information from DT, it can prepare eMMC
>> specific voltage, set eMMC specific mmc->caps/caps2 flags and do other
>> vendor specific init, before card init procedure.
>> Otherwise, our host driver has to wait until card type is determined in mmc_rescan().
>> A generic "eMMC" flag is unnecessary. I just require a private property,
>> which is only used in our host driver and DT.
>> Thank you.
>> Best regards,
>> Hu Ziji
>>> Actually, our eMMC is usually fixed as 1.8V.
>>> The pair "no-sd" + "no-sdio" can provide the similar information.
>>> But I'm not sure if it is proper to use those two property in such a way.
>>> Thank you.
>>> Best regards
>>> Hu Ziji
>>>> [...]
>>>> Kind regards
>>>> Uffe
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at
> Kind regards
> Uffe