On 24/11/16 15:34, Ulf Hansson wrote:

On 24 November 2016 at 13:41, Ziji Hu [off-list ref] wrote:

quoted

On 2016/11/24 18:43, Ulf Hansson wrote:

quoted

On 31 October 2016 at 12:09, Gregory CLEMENT
[off-list ref] wrote:

quoted

From: Ziji Hu <huziji@marvell.com>
+static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
+                                            struct mmc_ios *ios)
+{
+       struct sdhci_host *host = mmc_priv(mmc);
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
+
+       /*
+        * Before SD/SDIO set signal voltage, SD bus clock should be
+        * disabled. However, sdhci_set_clock will also disable the Internal
+        * clock in mmc_set_signal_voltage().

If that's the case then that is wrong in the generic sdhci code.
What's the reason why it can't be fixed there instead of having this
workaround?

    In my very own opinion, SD Spec doesn't specify whether SDCLK should be
    enabled or not during power setting.
    Enabling SDCLK might be a special condition only required by our SDHC.
    I try to avoid breaking other vendors' SDHC functionality
    if their SDHCs require SDCLK disabled.
    Thus I prefer to keep it inside our SDHC driver.

I let Adrian comment on this.

For sure we should avoid breaking other sdhci variant, but on the
other hand *if* the generic code is wrong we should fix it!

Yes, this looks like something that could perhaps be fixed in sdhci.  I will
look into it.