All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mmc: sdhci-xenon: add set_power callback
@ 2017-08-21 18:02 Zhoujie Wu
  2017-08-28  9:57 ` Adrian Hunter
  0 siblings, 1 reply; 9+ messages in thread
From: Zhoujie Wu @ 2017-08-21 18:02 UTC (permalink / raw)
  To: ulf.hansson, adrian.hunter, linux-mmc
  Cc: zmxu, jszhang, nadavh, xigu, dingwei, kostap, hannah, hongd,
	dougj, ygao, liuw, gregory.clement, thomas.petazzoni, Zhoujie Wu

Xenon sdh controller requests proper SD bus voltage select
bits programmed even with vmmc power supply. Any reserved
value(100b-000b) programmed in this field will lead to controller
ignore SD bus power bit and keep its value at zero.
Add set_power callback to handle this.

Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
---
Updated according to Jisheng Zhang's comment.
 drivers/mmc/host/sdhci-xenon.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
index edd4d915..a4be2fd 100644
--- a/drivers/mmc/host/sdhci-xenon.c
+++ b/drivers/mmc/host/sdhci-xenon.c
@@ -210,8 +210,27 @@ static void xenon_set_uhs_signaling(struct sdhci_host *host,
 	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
 }
 
+static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
+		unsigned short vdd)
+{
+	struct mmc_host *mmc = host->mmc;
+	u8 pwr = host->pwr;
+
+	sdhci_set_power_noreg(host, mode, vdd);
+
+	if (host->pwr == pwr)
+		return;
+
+	if (host->pwr == 0)
+		vdd = 0;
+
+	if (!IS_ERR(mmc->supply.vmmc))
+		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+}
+
 static const struct sdhci_ops sdhci_xenon_ops = {
 	.set_clock		= sdhci_set_clock,
+	.set_power		= xenon_set_power,
 	.set_bus_width		= sdhci_set_bus_width,
 	.reset			= xenon_reset,
 	.set_uhs_signaling	= xenon_set_uhs_signaling,
-- 
1.9.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mmc: sdhci-xenon: add set_power callback
  2017-08-21 18:02 [PATCH v2] mmc: sdhci-xenon: add set_power callback Zhoujie Wu
@ 2017-08-28  9:57 ` Adrian Hunter
  2017-08-28 15:54   ` Gregory CLEMENT
  0 siblings, 1 reply; 9+ messages in thread
From: Adrian Hunter @ 2017-08-28  9:57 UTC (permalink / raw)
  To: Zhoujie Wu, ulf.hansson, linux-mmc
  Cc: zmxu, jszhang, nadavh, xigu, dingwei, kostap, hannah, hongd,
	dougj, ygao, liuw, gregory.clement, thomas.petazzoni

On 21/08/17 21:02, Zhoujie Wu wrote:
> Xenon sdh controller requests proper SD bus voltage select
> bits programmed even with vmmc power supply. Any reserved
> value(100b-000b) programmed in this field will lead to controller
> ignore SD bus power bit and keep its value at zero.
> Add set_power callback to handle this.
> 
> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> Updated according to Jisheng Zhang's comment.
>  drivers/mmc/host/sdhci-xenon.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
> index edd4d915..a4be2fd 100644
> --- a/drivers/mmc/host/sdhci-xenon.c
> +++ b/drivers/mmc/host/sdhci-xenon.c
> @@ -210,8 +210,27 @@ static void xenon_set_uhs_signaling(struct sdhci_host *host,
>  	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>  }
>  
> +static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
> +		unsigned short vdd)
> +{
> +	struct mmc_host *mmc = host->mmc;
> +	u8 pwr = host->pwr;
> +
> +	sdhci_set_power_noreg(host, mode, vdd);
> +
> +	if (host->pwr == pwr)
> +		return;
> +
> +	if (host->pwr == 0)
> +		vdd = 0;
> +
> +	if (!IS_ERR(mmc->supply.vmmc))
> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> +}
> +
>  static const struct sdhci_ops sdhci_xenon_ops = {
>  	.set_clock		= sdhci_set_clock,
> +	.set_power		= xenon_set_power,
>  	.set_bus_width		= sdhci_set_bus_width,
>  	.reset			= xenon_reset,
>  	.set_uhs_signaling	= xenon_set_uhs_signaling,
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mmc: sdhci-xenon: add set_power callback
  2017-08-28  9:57 ` Adrian Hunter
@ 2017-08-28 15:54   ` Gregory CLEMENT
  2017-08-28 18:45     ` Zhoujie Wu
  0 siblings, 1 reply; 9+ messages in thread
From: Gregory CLEMENT @ 2017-08-28 15:54 UTC (permalink / raw)
  To: Zhoujie Wu, ulf.hansson, Adrian Hunter
  Cc: linux-mmc, zmxu, jszhang, nadavh, xigu, dingwei, kostap, hannah,
	hongd, dougj, ygao, liuw, thomas.petazzoni

Hi,
 
 On lun., août 28 2017, Adrian Hunter <adrian.hunter@intel.com> wrote:

> On 21/08/17 21:02, Zhoujie Wu wrote:
>> Xenon sdh controller requests proper SD bus voltage select
>> bits programmed even with vmmc power supply. Any reserved
>> value(100b-000b) programmed in this field will lead to controller
>> ignore SD bus power bit and keep its value at zero.
>> Add set_power callback to handle this.
>> 
>> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

I tested this patch on the Armada 3720 DB board. And thanks to this patch
and by adding a vmmc regulator in the device tree I did not have anymore
the issue UHS card not detected on warm reset.

Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Actually for me this should be a fix. Indeed if I try to use a vmmc
regulator without this patch then the SD card does not work at all I
only have the following messages:

"mmc1: Timeout waiting for hardware cmd interrupt."

Thanks,

Gregory


>
>> ---
>> Updated according to Jisheng Zhang's comment.
>>  drivers/mmc/host/sdhci-xenon.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>> 
>> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
>> index edd4d915..a4be2fd 100644
>> --- a/drivers/mmc/host/sdhci-xenon.c
>> +++ b/drivers/mmc/host/sdhci-xenon.c
>> @@ -210,8 +210,27 @@ static void xenon_set_uhs_signaling(struct sdhci_host *host,
>>  	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>  }
>>  
>> +static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
>> +		unsigned short vdd)
>> +{
>> +	struct mmc_host *mmc = host->mmc;
>> +	u8 pwr = host->pwr;
>> +
>> +	sdhci_set_power_noreg(host, mode, vdd);
>> +
>> +	if (host->pwr == pwr)
>> +		return;
>> +
>> +	if (host->pwr == 0)
>> +		vdd = 0;
>> +
>> +	if (!IS_ERR(mmc->supply.vmmc))
>> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>> +}
>> +
>>  static const struct sdhci_ops sdhci_xenon_ops = {
>>  	.set_clock		= sdhci_set_clock,
>> +	.set_power		= xenon_set_power,
>>  	.set_bus_width		= sdhci_set_bus_width,
>>  	.reset			= xenon_reset,
>>  	.set_uhs_signaling	= xenon_set_uhs_signaling,
>> 
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mmc: sdhci-xenon: add set_power callback
  2017-08-28 15:54   ` Gregory CLEMENT
@ 2017-08-28 18:45     ` Zhoujie Wu
  2017-08-29  3:29       ` Shawn Lin
  2017-08-29 10:18       ` Gregory CLEMENT
  0 siblings, 2 replies; 9+ messages in thread
From: Zhoujie Wu @ 2017-08-28 18:45 UTC (permalink / raw)
  To: Gregory CLEMENT, ulf.hansson, Adrian Hunter
  Cc: linux-mmc, zmxu, jszhang, nadavh, xigu, dingwei, kostap, hannah,
	hongd, dougj, ygao, liuw, thomas.petazzoni

Hi Gregory,

On 08/28/2017 08:54 AM, Gregory CLEMENT wrote:
> Hi,
>   
>   On lun., août 28 2017, Adrian Hunter <adrian.hunter@intel.com> wrote:
>
>> On 21/08/17 21:02, Zhoujie Wu wrote:
>>> Xenon sdh controller requests proper SD bus voltage select
>>> bits programmed even with vmmc power supply. Any reserved
>>> value(100b-000b) programmed in this field will lead to controller
>>> ignore SD bus power bit and keep its value at zero.
>>> Add set_power callback to handle this.
>>>
>>> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> I tested this patch on the Armada 3720 DB board. And thanks to this patch
> and by adding a vmmc regulator in the device tree I did not have anymore
> the issue UHS card not detected on warm reset.
>
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> Actually for me this should be a fix. Indeed if I try to use a vmmc
> regulator without this patch then the SD card does not work at all I
> only have the following messages:
>
> "mmc1: Timeout waiting for hardware cmd interrupt."
This patch with sd vmmc power supply(must be claimed as AON, or card 
detection can't work)enabled can solve the issue for warm reset. This 
patch is must for xenon controller once vmmc power supply enabled, since 
the HW will ignore any reserved value for sd bus voltage select field 
and ignore the pwr_en in this case.
But recently we met another issue after resume, it is similar as warm 
reset. When resume back, SD card(UHS) is re-initialized and it is 
recognized as HS again. It needs power-cycle before initialization to 
solve the resume issue. But vmmc is claimed as AON, it won't be shut 
down in standby, so vmmc and this patch won't solve resume issue. I have 
no better idea to solve both warm reset and resume issue, I added a 
dedicated pwrseq for a3700 sd card, it will do power cycle during pre 
and post pwrseq, but keep power when power off the card.
Do you have any other better suggestion?
>
> Thanks,
>
> Gregory
>
>
>>> ---
>>> Updated according to Jisheng Zhang's comment.
>>>   drivers/mmc/host/sdhci-xenon.c | 19 +++++++++++++++++++
>>>   1 file changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
>>> index edd4d915..a4be2fd 100644
>>> --- a/drivers/mmc/host/sdhci-xenon.c
>>> +++ b/drivers/mmc/host/sdhci-xenon.c
>>> @@ -210,8 +210,27 @@ static void xenon_set_uhs_signaling(struct sdhci_host *host,
>>>   	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>>   }
>>>   
>>> +static void xenon_set_power(struct sdhci_host *host, unsigned char mode,
>>> +		unsigned short vdd)
>>> +{
>>> +	struct mmc_host *mmc = host->mmc;
>>> +	u8 pwr = host->pwr;
>>> +
>>> +	sdhci_set_power_noreg(host, mode, vdd);
>>> +
>>> +	if (host->pwr == pwr)
>>> +		return;
>>> +
>>> +	if (host->pwr == 0)
>>> +		vdd = 0;
>>> +
>>> +	if (!IS_ERR(mmc->supply.vmmc))
>>> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>>> +}
>>> +
>>>   static const struct sdhci_ops sdhci_xenon_ops = {
>>>   	.set_clock		= sdhci_set_clock,
>>> +	.set_power		= xenon_set_power,
>>>   	.set_bus_width		= sdhci_set_bus_width,
>>>   	.reset			= xenon_reset,
>>>   	.set_uhs_signaling	= xenon_set_uhs_signaling,
>>>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mmc: sdhci-xenon: add set_power callback
  2017-08-28 18:45     ` Zhoujie Wu
@ 2017-08-29  3:29       ` Shawn Lin
  2017-08-29  3:56         ` Shawn Lin
  2017-08-29 10:18       ` Gregory CLEMENT
  1 sibling, 1 reply; 9+ messages in thread
From: Shawn Lin @ 2017-08-29  3:29 UTC (permalink / raw)
  To: Zhoujie Wu, Gregory CLEMENT, ulf.hansson, Adrian Hunter
  Cc: shawn.lin, linux-mmc, zmxu, jszhang, nadavh, xigu, dingwei,
	kostap, hannah, hongd, dougj, ygao, liuw, thomas.petazzoni

On 2017/8/29 2:45, Zhoujie Wu wrote:
> Hi Gregory,
> 
> On 08/28/2017 08:54 AM, Gregory CLEMENT wrote:
>> Hi,
>>   On lun., août 28 2017, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>>> On 21/08/17 21:02, Zhoujie Wu wrote:
>>>> Xenon sdh controller requests proper SD bus voltage select
>>>> bits programmed even with vmmc power supply. Any reserved
>>>> value(100b-000b) programmed in this field will lead to controller
>>>> ignore SD bus power bit and keep its value at zero.
>>>> Add set_power callback to handle this.
>>>>
>>>> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
>>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>> I tested this patch on the Armada 3720 DB board. And thanks to this patch
>> and by adding a vmmc regulator in the device tree I did not have anymore
>> the issue UHS card not detected on warm reset.
>>
>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>
>> Actually for me this should be a fix. Indeed if I try to use a vmmc
>> regulator without this patch then the SD card does not work at all I
>> only have the following messages:
>>
>> "mmc1: Timeout waiting for hardware cmd interrupt."
> This patch with sd vmmc power supply(must be claimed as AON, or card 
> detection can't work)enabled can solve the issue for warm reset. This 
> patch is must for xenon controller once vmmc power supply enabled, since 
> the HW will ignore any reserved value for sd bus voltage select field 
> and ignore the pwr_en in this case.
> But recently we met another issue after resume, it is similar as warm 
> reset. When resume back, SD card(UHS) is re-initialized and it is 
> recognized as HS again. It needs power-cycle before initialization to 
> solve the resume issue. But vmmc is claimed as AON, it won't be shut 

This is a expected behaviour from the spec's POV. So you need a high
level solution to solve it for all hosts. I just quickly cook a patch[1]
for test, maybe it doesn't work at all but I guess it helps.:)

[1]: https://patchwork.kernel.org/patch/9926715/

> down in standby, so vmmc and this patch won't solve resume issue. I have 
> no better idea to solve both warm reset and resume issue, I added a 
> dedicated pwrseq for a3700 sd card, it will do power cycle during pre 
> and post pwrseq, but keep power when power off the card.
> Do you have any other better suggestion?
>>
>> Thanks,
>>
>> Gregory
>>
>>
>>>> ---
>>>> Updated according to Jisheng Zhang's comment.
>>>>   drivers/mmc/host/sdhci-xenon.c | 19 +++++++++++++++++++
>>>>   1 file changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci-xenon.c 
>>>> b/drivers/mmc/host/sdhci-xenon.c
>>>> index edd4d915..a4be2fd 100644
>>>> --- a/drivers/mmc/host/sdhci-xenon.c
>>>> +++ b/drivers/mmc/host/sdhci-xenon.c
>>>> @@ -210,8 +210,27 @@ static void xenon_set_uhs_signaling(struct 
>>>> sdhci_host *host,
>>>>       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>>>   }
>>>> +static void xenon_set_power(struct sdhci_host *host, unsigned char 
>>>> mode,
>>>> +        unsigned short vdd)
>>>> +{
>>>> +    struct mmc_host *mmc = host->mmc;
>>>> +    u8 pwr = host->pwr;
>>>> +
>>>> +    sdhci_set_power_noreg(host, mode, vdd);
>>>> +
>>>> +    if (host->pwr == pwr)
>>>> +        return;
>>>> +
>>>> +    if (host->pwr == 0)
>>>> +        vdd = 0;
>>>> +
>>>> +    if (!IS_ERR(mmc->supply.vmmc))
>>>> +        mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>>>> +}
>>>> +
>>>>   static const struct sdhci_ops sdhci_xenon_ops = {
>>>>       .set_clock        = sdhci_set_clock,
>>>> +    .set_power        = xenon_set_power,
>>>>       .set_bus_width        = sdhci_set_bus_width,
>>>>       .reset            = xenon_reset,
>>>>       .set_uhs_signaling    = xenon_set_uhs_signaling,
>>>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mmc: sdhci-xenon: add set_power callback
  2017-08-29  3:29       ` Shawn Lin
@ 2017-08-29  3:56         ` Shawn Lin
  0 siblings, 0 replies; 9+ messages in thread
From: Shawn Lin @ 2017-08-29  3:56 UTC (permalink / raw)
  To: Zhoujie Wu, Adrian Hunter
  Cc: Gregory CLEMENT, ulf.hansson, shawn.lin, linux-mmc, zmxu,
	jszhang, nadavh, xigu, dingwei, kostap, hannah, hongd, dougj,
	ygao, liuw, thomas.petazzoni


On 2017/8/29 11:29, Shawn Lin wrote:
> On 2017/8/29 2:45, Zhoujie Wu wrote:
>> Hi Gregory,
>>
>> On 08/28/2017 08:54 AM, Gregory CLEMENT wrote:
>>> Hi,
>>>   On lun., août 28 2017, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>
>>>> On 21/08/17 21:02, Zhoujie Wu wrote:
>>>>> Xenon sdh controller requests proper SD bus voltage select
>>>>> bits programmed even with vmmc power supply. Any reserved
>>>>> value(100b-000b) programmed in this field will lead to controller
>>>>> ignore SD bus power bit and keep its value at zero.
>>>>> Add set_power callback to handle this.
>>>>>
>>>>> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
>>>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>>> I tested this patch on the Armada 3720 DB board. And thanks to this 
>>> patch
>>> and by adding a vmmc regulator in the device tree I did not have anymore
>>> the issue UHS card not detected on warm reset.
>>>
>>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>
>>> Actually for me this should be a fix. Indeed if I try to use a vmmc
>>> regulator without this patch then the SD card does not work at all I
>>> only have the following messages:
>>>
>>> "mmc1: Timeout waiting for hardware cmd interrupt."
>> This patch with sd vmmc power supply(must be claimed as AON, or card 
>> detection can't work)enabled can solve the issue for warm reset. This 
>> patch is must for xenon controller once vmmc power supply enabled, 
>> since the HW will ignore any reserved value for sd bus voltage select 
>> field and ignore the pwr_en in this case.
>> But recently we met another issue after resume, it is similar as warm 
>> reset. When resume back, SD card(UHS) is re-initialized and it is 
>> recognized as HS again. It needs power-cycle before initialization to 
>> solve the resume issue. But vmmc is claimed as AON, it won't be shut 
> 
> This is a expected behaviour from the spec's POV. So you need a high
> level solution to solve it for all hosts. I just quickly cook a patch[1]
> for test, maybe it doesn't work at all but I guess it helps.:)
> 
> [1]: https://patchwork.kernel.org/patch/9926715/

Hmm.. sorry. update v2 for setting voltage to 1v8 for that case.

https://patchwork.kernel.org/patch/9926723/

> 
>> down in standby, so vmmc and this patch won't solve resume issue. I 
>> have no better idea to solve both warm reset and resume issue, I added 
>> a dedicated pwrseq for a3700 sd card, it will do power cycle during 
>> pre and post pwrseq, but keep power when power off the card.
>> Do you have any other better suggestion?
>>>
>>> Thanks,
>>>
>>> Gregory
>>>
>>>
>>>>> ---
>>>>> Updated according to Jisheng Zhang's comment.
>>>>>   drivers/mmc/host/sdhci-xenon.c | 19 +++++++++++++++++++
>>>>>   1 file changed, 19 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mmc/host/sdhci-xenon.c 
>>>>> b/drivers/mmc/host/sdhci-xenon.c
>>>>> index edd4d915..a4be2fd 100644
>>>>> --- a/drivers/mmc/host/sdhci-xenon.c
>>>>> +++ b/drivers/mmc/host/sdhci-xenon.c
>>>>> @@ -210,8 +210,27 @@ static void xenon_set_uhs_signaling(struct 
>>>>> sdhci_host *host,
>>>>>       sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>>>>   }
>>>>> +static void xenon_set_power(struct sdhci_host *host, unsigned char 
>>>>> mode,
>>>>> +        unsigned short vdd)
>>>>> +{
>>>>> +    struct mmc_host *mmc = host->mmc;
>>>>> +    u8 pwr = host->pwr;
>>>>> +
>>>>> +    sdhci_set_power_noreg(host, mode, vdd);
>>>>> +
>>>>> +    if (host->pwr == pwr)
>>>>> +        return;
>>>>> +
>>>>> +    if (host->pwr == 0)
>>>>> +        vdd = 0;
>>>>> +
>>>>> +    if (!IS_ERR(mmc->supply.vmmc))
>>>>> +        mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>>>>> +}
>>>>> +
>>>>>   static const struct sdhci_ops sdhci_xenon_ops = {
>>>>>       .set_clock        = sdhci_set_clock,
>>>>> +    .set_power        = xenon_set_power,
>>>>>       .set_bus_width        = sdhci_set_bus_width,
>>>>>       .reset            = xenon_reset,
>>>>>       .set_uhs_signaling    = xenon_set_uhs_signaling,
>>>>>
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
> 
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mmc: sdhci-xenon: add set_power callback
  2017-08-28 18:45     ` Zhoujie Wu
  2017-08-29  3:29       ` Shawn Lin
@ 2017-08-29 10:18       ` Gregory CLEMENT
  2017-08-29 10:25         ` Adrian Hunter
  2017-08-30 13:13         ` Ulf Hansson
  1 sibling, 2 replies; 9+ messages in thread
From: Gregory CLEMENT @ 2017-08-29 10:18 UTC (permalink / raw)
  To: ulf.hansson, Adrian Hunter, Zhoujie Wu
  Cc: linux-mmc, zmxu, jszhang, nadavh, xigu, dingwei, kostap, hannah,
	hongd, dougj, ygao, liuw, thomas.petazzoni

Hi Zhoujie,
 
 On lun., août 28 2017, Zhoujie Wu <zjwu@marvell.com> wrote:

> Hi Gregory,
>
> On 08/28/2017 08:54 AM, Gregory CLEMENT wrote:
>> Hi,
>>     On lun., août 28 2017, Adrian Hunter <adrian.hunter@intel.com>
>> wrote:
>>
>>> On 21/08/17 21:02, Zhoujie Wu wrote:
>>>> Xenon sdh controller requests proper SD bus voltage select
>>>> bits programmed even with vmmc power supply. Any reserved
>>>> value(100b-000b) programmed in this field will lead to controller
>>>> ignore SD bus power bit and keep its value at zero.
>>>> Add set_power callback to handle this.
>>>>
>>>> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
>>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>> I tested this patch on the Armada 3720 DB board. And thanks to this patch
>> and by adding a vmmc regulator in the device tree I did not have anymore
>> the issue UHS card not detected on warm reset.
>>
>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>
>> Actually for me this should be a fix. Indeed if I try to use a vmmc
>> regulator without this patch then the SD card does not work at all I
>> only have the following messages:
>>
>> "mmc1: Timeout waiting for hardware cmd interrupt."
> This patch with sd vmmc power supply(must be claimed as AON, or card
> detection can't work)enabled can solve the issue for warm reset. This
> patch is must for xenon controller once vmmc power supply enabled,
> since the HW will ignore any reserved value for sd bus voltage select
> field and ignore the pwr_en in this case.

That's why I would like that this patch was tagged as a fix.

Adrian, Ulf, do you think it could be applied on the v4.13-rc or at
least applied with the following tags ?

Fixes: 3a3748dba881(" mmc: sdhci-xenon: Add Marvell Xenon SDHC core
functionality")
Cc: <stable@vger.kernel.org>

Thanks,

Gregory

> But recently we met another issue after resume, it is similar as warm
> reset. When resume back, SD card(UHS) is re-initialized and it is
> recognized as HS again. It needs power-cycle before initialization to
> solve the resume issue. But vmmc is claimed as AON, it won't be shut
> down in standby, so vmmc and this patch won't solve resume issue. I
> have no better idea to solve both warm reset and resume issue, I added
> a dedicated pwrseq for a3700 sd card, it will do power cycle during
> pre and post pwrseq, but keep power when power off the card.
> Do you have any other better suggestion?
>>
>> Thanks,
>>
>> Gregory
>>
>>
>>>> ---
>>>> Updated according to Jisheng Zhang's comment.
>>>>   drivers/mmc/host/sdhci-xenon.c | 19 +++++++++++++++++++
>>>>   1 file changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
>>>> index edd4d915..a4be2fd 100644
>>>> --- a/drivers/mmc/host/sdhci-xenon.c
>>>> +++ b/drivers/mmc/host/sdhci-xenon.c
>>>> @@ -210,8 +210,27 @@ static void xenon_set_uhs_signaling(struct sdhci_host *host,
>>>>   	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>>>   }
>>>>   +static void xenon_set_power(struct sdhci_host *host, unsigned
>>>> char mode,
>>>> +		unsigned short vdd)
>>>> +{
>>>> +	struct mmc_host *mmc = host->mmc;
>>>> +	u8 pwr = host->pwr;
>>>> +
>>>> +	sdhci_set_power_noreg(host, mode, vdd);
>>>> +
>>>> +	if (host->pwr == pwr)
>>>> +		return;
>>>> +
>>>> +	if (host->pwr == 0)
>>>> +		vdd = 0;
>>>> +
>>>> +	if (!IS_ERR(mmc->supply.vmmc))
>>>> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>>>> +}
>>>> +
>>>>   static const struct sdhci_ops sdhci_xenon_ops = {
>>>>   	.set_clock		= sdhci_set_clock,
>>>> +	.set_power		= xenon_set_power,
>>>>   	.set_bus_width		= sdhci_set_bus_width,
>>>>   	.reset			= xenon_reset,
>>>>   	.set_uhs_signaling	= xenon_set_uhs_signaling,
>>>>
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mmc: sdhci-xenon: add set_power callback
  2017-08-29 10:18       ` Gregory CLEMENT
@ 2017-08-29 10:25         ` Adrian Hunter
  2017-08-30 13:13         ` Ulf Hansson
  1 sibling, 0 replies; 9+ messages in thread
From: Adrian Hunter @ 2017-08-29 10:25 UTC (permalink / raw)
  To: Gregory CLEMENT, ulf.hansson, Zhoujie Wu
  Cc: linux-mmc, zmxu, jszhang, nadavh, xigu, dingwei, kostap, hannah,
	hongd, dougj, ygao, liuw, thomas.petazzoni

On 29/08/17 13:18, Gregory CLEMENT wrote:
> Hi Zhoujie,
>  
>  On lun., août 28 2017, Zhoujie Wu <zjwu@marvell.com> wrote:
> 
>> Hi Gregory,
>>
>> On 08/28/2017 08:54 AM, Gregory CLEMENT wrote:
>>> Hi,
>>>     On lun., août 28 2017, Adrian Hunter <adrian.hunter@intel.com>
>>> wrote:
>>>
>>>> On 21/08/17 21:02, Zhoujie Wu wrote:
>>>>> Xenon sdh controller requests proper SD bus voltage select
>>>>> bits programmed even with vmmc power supply. Any reserved
>>>>> value(100b-000b) programmed in this field will lead to controller
>>>>> ignore SD bus power bit and keep its value at zero.
>>>>> Add set_power callback to handle this.
>>>>>
>>>>> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
>>>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>>> I tested this patch on the Armada 3720 DB board. And thanks to this patch
>>> and by adding a vmmc regulator in the device tree I did not have anymore
>>> the issue UHS card not detected on warm reset.
>>>
>>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>
>>> Actually for me this should be a fix. Indeed if I try to use a vmmc
>>> regulator without this patch then the SD card does not work at all I
>>> only have the following messages:
>>>
>>> "mmc1: Timeout waiting for hardware cmd interrupt."
>> This patch with sd vmmc power supply(must be claimed as AON, or card
>> detection can't work)enabled can solve the issue for warm reset. This
>> patch is must for xenon controller once vmmc power supply enabled,
>> since the HW will ignore any reserved value for sd bus voltage select
>> field and ignore the pwr_en in this case.
> 
> That's why I would like that this patch was tagged as a fix.
> 
> Adrian, Ulf, do you think it could be applied on the v4.13-rc or at
> least applied with the following tags ?
> 
> Fixes: 3a3748dba881(" mmc: sdhci-xenon: Add Marvell Xenon SDHC core
> functionality")
> Cc: <stable@vger.kernel.org>

I have no objections.

> 
> Thanks,
> 
> Gregory
> 
>> But recently we met another issue after resume, it is similar as warm
>> reset. When resume back, SD card(UHS) is re-initialized and it is
>> recognized as HS again. It needs power-cycle before initialization to
>> solve the resume issue. But vmmc is claimed as AON, it won't be shut
>> down in standby, so vmmc and this patch won't solve resume issue. I
>> have no better idea to solve both warm reset and resume issue, I added
>> a dedicated pwrseq for a3700 sd card, it will do power cycle during
>> pre and post pwrseq, but keep power when power off the card.
>> Do you have any other better suggestion?
>>>
>>> Thanks,
>>>
>>> Gregory
>>>
>>>
>>>>> ---
>>>>> Updated according to Jisheng Zhang's comment.
>>>>>   drivers/mmc/host/sdhci-xenon.c | 19 +++++++++++++++++++
>>>>>   1 file changed, 19 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
>>>>> index edd4d915..a4be2fd 100644
>>>>> --- a/drivers/mmc/host/sdhci-xenon.c
>>>>> +++ b/drivers/mmc/host/sdhci-xenon.c
>>>>> @@ -210,8 +210,27 @@ static void xenon_set_uhs_signaling(struct sdhci_host *host,
>>>>>   	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>>>>   }
>>>>>   +static void xenon_set_power(struct sdhci_host *host, unsigned
>>>>> char mode,
>>>>> +		unsigned short vdd)
>>>>> +{
>>>>> +	struct mmc_host *mmc = host->mmc;
>>>>> +	u8 pwr = host->pwr;
>>>>> +
>>>>> +	sdhci_set_power_noreg(host, mode, vdd);
>>>>> +
>>>>> +	if (host->pwr == pwr)
>>>>> +		return;
>>>>> +
>>>>> +	if (host->pwr == 0)
>>>>> +		vdd = 0;
>>>>> +
>>>>> +	if (!IS_ERR(mmc->supply.vmmc))
>>>>> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>>>>> +}
>>>>> +
>>>>>   static const struct sdhci_ops sdhci_xenon_ops = {
>>>>>   	.set_clock		= sdhci_set_clock,
>>>>> +	.set_power		= xenon_set_power,
>>>>>   	.set_bus_width		= sdhci_set_bus_width,
>>>>>   	.reset			= xenon_reset,
>>>>>   	.set_uhs_signaling	= xenon_set_uhs_signaling,
>>>>>
>>
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mmc: sdhci-xenon: add set_power callback
  2017-08-29 10:18       ` Gregory CLEMENT
  2017-08-29 10:25         ` Adrian Hunter
@ 2017-08-30 13:13         ` Ulf Hansson
  1 sibling, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2017-08-30 13:13 UTC (permalink / raw)
  To: Gregory CLEMENT, Zhoujie Wu
  Cc: Adrian Hunter, linux-mmc, Jimmy Xu, Jisheng Zhang, Nadav Haklai,
	Victor Gu, Wilson Ding, Kostya Porotchkin, Hanna Hawa, hongd,
	Doug Jones, Ryan Gao, Wei(SOCP) Liu, Thomas Petazzoni

On 29 August 2017 at 12:18, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> Hi Zhoujie,
>
>  On lun., août 28 2017, Zhoujie Wu <zjwu@marvell.com> wrote:
>
>> Hi Gregory,
>>
>> On 08/28/2017 08:54 AM, Gregory CLEMENT wrote:
>>> Hi,
>>>     On lun., août 28 2017, Adrian Hunter <adrian.hunter@intel.com>
>>> wrote:
>>>
>>>> On 21/08/17 21:02, Zhoujie Wu wrote:
>>>>> Xenon sdh controller requests proper SD bus voltage select
>>>>> bits programmed even with vmmc power supply. Any reserved
>>>>> value(100b-000b) programmed in this field will lead to controller
>>>>> ignore SD bus power bit and keep its value at zero.
>>>>> Add set_power callback to handle this.
>>>>>
>>>>> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
>>>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>>> I tested this patch on the Armada 3720 DB board. And thanks to this patch
>>> and by adding a vmmc regulator in the device tree I did not have anymore
>>> the issue UHS card not detected on warm reset.
>>>
>>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>
>>> Actually for me this should be a fix. Indeed if I try to use a vmmc
>>> regulator without this patch then the SD card does not work at all I
>>> only have the following messages:
>>>
>>> "mmc1: Timeout waiting for hardware cmd interrupt."
>> This patch with sd vmmc power supply(must be claimed as AON, or card
>> detection can't work)enabled can solve the issue for warm reset. This
>> patch is must for xenon controller once vmmc power supply enabled,
>> since the HW will ignore any reserved value for sd bus voltage select
>> field and ignore the pwr_en in this case.
>
> That's why I would like that this patch was tagged as a fix.
>
> Adrian, Ulf, do you think it could be applied on the v4.13-rc or at
> least applied with the following tags ?
>
> Fixes: 3a3748dba881(" mmc: sdhci-xenon: Add Marvell Xenon SDHC core
> functionality")
> Cc: <stable@vger.kernel.org>
>
> Thanks,
>
> Gregory

Thanks, applied for fixes and by adding the above tags!

[...]

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-08-30 13:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-21 18:02 [PATCH v2] mmc: sdhci-xenon: add set_power callback Zhoujie Wu
2017-08-28  9:57 ` Adrian Hunter
2017-08-28 15:54   ` Gregory CLEMENT
2017-08-28 18:45     ` Zhoujie Wu
2017-08-29  3:29       ` Shawn Lin
2017-08-29  3:56         ` Shawn Lin
2017-08-29 10:18       ` Gregory CLEMENT
2017-08-29 10:25         ` Adrian Hunter
2017-08-30 13:13         ` Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.