On 24/11/16 15:34, Ulf Hansson wrote: > On 24 November 2016 at 13:41, Ziji Hu <huziji@xxxxxxxxxxx> wrote: >> On 2016/11/24 18:43, Ulf Hansson wrote: >>> On 31 October 2016 at 12:09, Gregory CLEMENT >>> <gregory.clement@xxxxxxxxxxxxxxxxxx> wrote: >>>> From: Ziji Hu <huziji@xxxxxxxxxxx> >>>> +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. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html