All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mmc: sdhci: Fix HISPD bit handling
@ 2020-06-09 14:01 ` Jagan Teki
  0 siblings, 0 replies; 10+ messages in thread
From: Jagan Teki @ 2020-06-09 14:01 UTC (permalink / raw)
  To: Peng Fan, Jaehoon Chung, Kever Yang
  Cc: sunil, u-boot, linux-rockchip, linux-amarula, Jagan Teki

SDHCI HISPD bits need to be configured based on desired mmc
timings mode and some HISPD quirks.

So, handle the HISPD bit based on the mmc computed selected
mode(timing parameter) rather than fixed mmc card clock
frequency.

Linux handle the HISPD similar like this in below commit,

commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling")

This eventually fixed the mmc write issue observed in
rk3399 sdhci controller.

Bug log for refernece,
=> gpt write mmc 0 $partitions
Writing GPT: mmc write failed
** Can't write to device 0 **
** Can't write to device 0 **
error!

Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v2:
- collect Jaehoon R-b

 drivers/mmc/sdhci.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 92cc8434af..280b8c88eb 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc)
 			ctrl &= ~SDHCI_CTRL_4BITBUS;
 	}
 
-	if (mmc->clock > 26000000)
-		ctrl |= SDHCI_CTRL_HISPD;
-	else
-		ctrl &= ~SDHCI_CTRL_HISPD;
-
-	if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
-	    (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
-		ctrl &= ~SDHCI_CTRL_HISPD;
+	if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
+	    !(host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE)) {
+		if (mmc->selected_mode == MMC_HS ||
+		    mmc->selected_mode == SD_HS ||
+		    mmc->selected_mode == MMC_DDR_52 ||
+		    mmc->selected_mode == MMC_HS_200 ||
+		    mmc->selected_mode == MMC_HS_400 ||
+		    mmc->selected_mode == UHS_SDR25 ||
+		    mmc->selected_mode == UHS_SDR50 ||
+		    mmc->selected_mode == UHS_SDR104 ||
+		    mmc->selected_mode == UHS_DDR50)
+			ctrl |= SDHCI_CTRL_HISPD;
+		else
+			ctrl &= ~SDHCI_CTRL_HISPD;
+	}
 
 	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
 
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v2] mmc: sdhci: Fix HISPD bit handling
@ 2020-06-09 14:01 ` Jagan Teki
  0 siblings, 0 replies; 10+ messages in thread
From: Jagan Teki @ 2020-06-09 14:01 UTC (permalink / raw)
  To: u-boot

SDHCI HISPD bits need to be configured based on desired mmc
timings mode and some HISPD quirks.

So, handle the HISPD bit based on the mmc computed selected
mode(timing parameter) rather than fixed mmc card clock
frequency.

Linux handle the HISPD similar like this in below commit,

commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling")

This eventually fixed the mmc write issue observed in
rk3399 sdhci controller.

Bug log for refernece,
=> gpt write mmc 0 $partitions
Writing GPT: mmc write failed
** Can't write to device 0 **
** Can't write to device 0 **
error!

Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v2:
- collect Jaehoon R-b

 drivers/mmc/sdhci.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 92cc8434af..280b8c88eb 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc)
 			ctrl &= ~SDHCI_CTRL_4BITBUS;
 	}
 
-	if (mmc->clock > 26000000)
-		ctrl |= SDHCI_CTRL_HISPD;
-	else
-		ctrl &= ~SDHCI_CTRL_HISPD;
-
-	if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
-	    (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
-		ctrl &= ~SDHCI_CTRL_HISPD;
+	if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
+	    !(host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE)) {
+		if (mmc->selected_mode == MMC_HS ||
+		    mmc->selected_mode == SD_HS ||
+		    mmc->selected_mode == MMC_DDR_52 ||
+		    mmc->selected_mode == MMC_HS_200 ||
+		    mmc->selected_mode == MMC_HS_400 ||
+		    mmc->selected_mode == UHS_SDR25 ||
+		    mmc->selected_mode == UHS_SDR50 ||
+		    mmc->selected_mode == UHS_SDR104 ||
+		    mmc->selected_mode == UHS_DDR50)
+			ctrl |= SDHCI_CTRL_HISPD;
+		else
+			ctrl &= ~SDHCI_CTRL_HISPD;
+	}
 
 	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
 
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH v2] mmc: sdhci: Fix HISPD bit handling
  2020-06-09 14:01 ` Jagan Teki
@ 2020-06-09 14:38     ` Robin Murphy
  -1 siblings, 0 replies; 10+ messages in thread
From: Robin Murphy @ 2020-06-09 14:38 UTC (permalink / raw)
  To: Jagan Teki, Peng Fan, Jaehoon Chung, Kever Yang
  Cc: u-boot-0aAXYlwwYIKGBzrmiIFOJg,
	sunil-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/,
	linux-amarula-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 2020-06-09 15:01, Jagan Teki wrote:
> SDHCI HISPD bits need to be configured based on desired mmc
> timings mode and some HISPD quirks.
> 
> So, handle the HISPD bit based on the mmc computed selected
> mode(timing parameter) rather than fixed mmc card clock
> frequency.
> 
> Linux handle the HISPD similar like this in below commit,
> 
> commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling")
> 
> This eventually fixed the mmc write issue observed in
> rk3399 sdhci controller.
> 
> Bug log for refernece,
> => gpt write mmc 0 $partitions
> Writing GPT: mmc write failed
> ** Can't write to device 0 **
> ** Can't write to device 0 **
> error!
> 
> Cc: Kever Yang <kever.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Cc: Peng Fan <peng.fan-3arQi8VN3Tc@public.gmane.org>
> Reviewed-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> ---
> Changes for v2:
> - collect Jaehoon R-b
> 
>   drivers/mmc/sdhci.c | 23 +++++++++++++++--------
>   1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 92cc8434af..280b8c88eb 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc)
>   			ctrl &= ~SDHCI_CTRL_4BITBUS;
>   	}
>   
> -	if (mmc->clock > 26000000)
> -		ctrl |= SDHCI_CTRL_HISPD;
> -	else
> -		ctrl &= ~SDHCI_CTRL_HISPD;
> -
> -	if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
> -	    (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
> -		ctrl &= ~SDHCI_CTRL_HISPD;
> +	if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||

Should that be "&&" rather than "||"? Otherwise this will always 
evaluate to true unless *both* quirks are set, which isn't equivalent to 
the check being removed above.

Robin.

> +	    !(host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE)) {
> +		if (mmc->selected_mode == MMC_HS ||
> +		    mmc->selected_mode == SD_HS ||
> +		    mmc->selected_mode == MMC_DDR_52 ||
> +		    mmc->selected_mode == MMC_HS_200 ||
> +		    mmc->selected_mode == MMC_HS_400 ||
> +		    mmc->selected_mode == UHS_SDR25 ||
> +		    mmc->selected_mode == UHS_SDR50 ||
> +		    mmc->selected_mode == UHS_SDR104 ||
> +		    mmc->selected_mode == UHS_DDR50)
> +			ctrl |= SDHCI_CTRL_HISPD;
> +		else
> +			ctrl &= ~SDHCI_CTRL_HISPD;
> +	}
>   
>   	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>   
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH v2] mmc: sdhci: Fix HISPD bit handling
@ 2020-06-09 14:38     ` Robin Murphy
  0 siblings, 0 replies; 10+ messages in thread
From: Robin Murphy @ 2020-06-09 14:38 UTC (permalink / raw)
  To: u-boot

On 2020-06-09 15:01, Jagan Teki wrote:
> SDHCI HISPD bits need to be configured based on desired mmc
> timings mode and some HISPD quirks.
> 
> So, handle the HISPD bit based on the mmc computed selected
> mode(timing parameter) rather than fixed mmc card clock
> frequency.
> 
> Linux handle the HISPD similar like this in below commit,
> 
> commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling")
> 
> This eventually fixed the mmc write issue observed in
> rk3399 sdhci controller.
> 
> Bug log for refernece,
> => gpt write mmc 0 $partitions
> Writing GPT: mmc write failed
> ** Can't write to device 0 **
> ** Can't write to device 0 **
> error!
> 
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v2:
> - collect Jaehoon R-b
> 
>   drivers/mmc/sdhci.c | 23 +++++++++++++++--------
>   1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 92cc8434af..280b8c88eb 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc)
>   			ctrl &= ~SDHCI_CTRL_4BITBUS;
>   	}
>   
> -	if (mmc->clock > 26000000)
> -		ctrl |= SDHCI_CTRL_HISPD;
> -	else
> -		ctrl &= ~SDHCI_CTRL_HISPD;
> -
> -	if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
> -	    (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
> -		ctrl &= ~SDHCI_CTRL_HISPD;
> +	if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||

Should that be "&&" rather than "||"? Otherwise this will always 
evaluate to true unless *both* quirks are set, which isn't equivalent to 
the check being removed above.

Robin.

> +	    !(host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE)) {
> +		if (mmc->selected_mode == MMC_HS ||
> +		    mmc->selected_mode == SD_HS ||
> +		    mmc->selected_mode == MMC_DDR_52 ||
> +		    mmc->selected_mode == MMC_HS_200 ||
> +		    mmc->selected_mode == MMC_HS_400 ||
> +		    mmc->selected_mode == UHS_SDR25 ||
> +		    mmc->selected_mode == UHS_SDR50 ||
> +		    mmc->selected_mode == UHS_SDR104 ||
> +		    mmc->selected_mode == UHS_DDR50)
> +			ctrl |= SDHCI_CTRL_HISPD;
> +		else
> +			ctrl &= ~SDHCI_CTRL_HISPD;
> +	}
>   
>   	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>   
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2] mmc: sdhci: Fix HISPD bit handling
  2020-06-09 14:38     ` Robin Murphy
@ 2020-06-10  3:45       ` Jaehoon Chung
  -1 siblings, 0 replies; 10+ messages in thread
From: Jaehoon Chung @ 2020-06-10  3:45 UTC (permalink / raw)
  To: Robin Murphy, Jagan Teki, Peng Fan, Kever Yang
  Cc: u-boot, linux-rockchip, linux-amarula, sunil

On 6/9/20 11:38 PM, Robin Murphy wrote:
> On 2020-06-09 15:01, Jagan Teki wrote:
>> SDHCI HISPD bits need to be configured based on desired mmc
>> timings mode and some HISPD quirks.
>>
>> So, handle the HISPD bit based on the mmc computed selected
>> mode(timing parameter) rather than fixed mmc card clock
>> frequency.
>>
>> Linux handle the HISPD similar like this in below commit,
>>
>> commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling")
>>
>> This eventually fixed the mmc write issue observed in
>> rk3399 sdhci controller.
>>
>> Bug log for refernece,
>> => gpt write mmc 0 $partitions
>> Writing GPT: mmc write failed
>> ** Can't write to device 0 **
>> ** Can't write to device 0 **
>> error!
>>
>> Cc: Kever Yang <kever.yang@rock-chips.com>
>> Cc: Peng Fan <peng.fan@nxp.com>
>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>> ---
>> Changes for v2:
>> - collect Jaehoon R-b
>>
>>   drivers/mmc/sdhci.c | 23 +++++++++++++++--------
>>   1 file changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index 92cc8434af..280b8c88eb 100644
>> --- a/drivers/mmc/sdhci.c
>> +++ b/drivers/mmc/sdhci.c
>> @@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc)
>>               ctrl &= ~SDHCI_CTRL_4BITBUS;
>>       }
>>   -    if (mmc->clock > 26000000)
>> -        ctrl |= SDHCI_CTRL_HISPD;
>> -    else
>> -        ctrl &= ~SDHCI_CTRL_HISPD;
>> -
>> -    if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
>> -        (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
>> -        ctrl &= ~SDHCI_CTRL_HISPD;
>> +    if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
> 
> Should that be "&&" rather than "||"? Otherwise this will always evaluate to true unless *both* quirks are set, which isn't equivalent to the check being removed above.


You're right.

> 
> Robin.
> 
>> +        !(host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE)) {
>> +        if (mmc->selected_mode == MMC_HS ||
>> +            mmc->selected_mode == SD_HS ||
>> +            mmc->selected_mode == MMC_DDR_52 ||
>> +            mmc->selected_mode == MMC_HS_200 ||
>> +            mmc->selected_mode == MMC_HS_400 ||
>> +            mmc->selected_mode == UHS_SDR25 ||
>> +            mmc->selected_mode == UHS_SDR50 ||
>> +            mmc->selected_mode == UHS_SDR104 ||
>> +            mmc->selected_mode == UHS_DDR50)
>> +            ctrl |= SDHCI_CTRL_HISPD;
>> +        else
>> +            ctrl &= ~SDHCI_CTRL_HISPD;
>> +    }
>>         sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>>  
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH v2] mmc: sdhci: Fix HISPD bit handling
@ 2020-06-10  3:45       ` Jaehoon Chung
  0 siblings, 0 replies; 10+ messages in thread
From: Jaehoon Chung @ 2020-06-10  3:45 UTC (permalink / raw)
  To: u-boot

On 6/9/20 11:38 PM, Robin Murphy wrote:
> On 2020-06-09 15:01, Jagan Teki wrote:
>> SDHCI HISPD bits need to be configured based on desired mmc
>> timings mode and some HISPD quirks.
>>
>> So, handle the HISPD bit based on the mmc computed selected
>> mode(timing parameter) rather than fixed mmc card clock
>> frequency.
>>
>> Linux handle the HISPD similar like this in below commit,
>>
>> commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling")
>>
>> This eventually fixed the mmc write issue observed in
>> rk3399 sdhci controller.
>>
>> Bug log for refernece,
>> => gpt write mmc 0 $partitions
>> Writing GPT: mmc write failed
>> ** Can't write to device 0 **
>> ** Can't write to device 0 **
>> error!
>>
>> Cc: Kever Yang <kever.yang@rock-chips.com>
>> Cc: Peng Fan <peng.fan@nxp.com>
>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>> ---
>> Changes for v2:
>> - collect Jaehoon R-b
>>
>> ? drivers/mmc/sdhci.c | 23 +++++++++++++++--------
>> ? 1 file changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index 92cc8434af..280b8c88eb 100644
>> --- a/drivers/mmc/sdhci.c
>> +++ b/drivers/mmc/sdhci.c
>> @@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc)
>> ????????????? ctrl &= ~SDHCI_CTRL_4BITBUS;
>> ????? }
>> ? -??? if (mmc->clock > 26000000)
>> -??????? ctrl |= SDHCI_CTRL_HISPD;
>> -??? else
>> -??????? ctrl &= ~SDHCI_CTRL_HISPD;
>> -
>> -??? if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
>> -??????? (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
>> -??????? ctrl &= ~SDHCI_CTRL_HISPD;
>> +??? if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
> 
> Should that be "&&" rather than "||"? Otherwise this will always evaluate to true unless *both* quirks are set, which isn't equivalent to the check being removed above.


You're right.

> 
> Robin.
> 
>> +??????? !(host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE)) {
>> +??????? if (mmc->selected_mode == MMC_HS ||
>> +??????????? mmc->selected_mode == SD_HS ||
>> +??????????? mmc->selected_mode == MMC_DDR_52 ||
>> +??????????? mmc->selected_mode == MMC_HS_200 ||
>> +??????????? mmc->selected_mode == MMC_HS_400 ||
>> +??????????? mmc->selected_mode == UHS_SDR25 ||
>> +??????????? mmc->selected_mode == UHS_SDR50 ||
>> +??????????? mmc->selected_mode == UHS_SDR104 ||
>> +??????????? mmc->selected_mode == UHS_DDR50)
>> +??????????? ctrl |= SDHCI_CTRL_HISPD;
>> +??????? else
>> +??????????? ctrl &= ~SDHCI_CTRL_HISPD;
>> +??? }
>> ? ????? sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>> ?
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2] mmc: sdhci: Fix HISPD bit handling
  2020-06-10  3:45       ` Jaehoon Chung
@ 2020-06-10 10:37           ` Marc Zyngier
  -1 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2020-06-10 10:37 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: Peng Fan, u-boot-0aAXYlwwYIKGBzrmiIFOJg,
	linux-amarula-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/,
	Kever Yang, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Jagan Teki, sunil-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/,
	Robin Murphy

On Wed, 10 Jun 2020 12:45:33 +0900
Jaehoon Chung <jh80.chung@samsung.com> wrote:

> On 6/9/20 11:38 PM, Robin Murphy wrote:
> > On 2020-06-09 15:01, Jagan Teki wrote:  
> >> SDHCI HISPD bits need to be configured based on desired mmc
> >> timings mode and some HISPD quirks.
> >>
> >> So, handle the HISPD bit based on the mmc computed selected
> >> mode(timing parameter) rather than fixed mmc card clock
> >> frequency.
> >>
> >> Linux handle the HISPD similar like this in below commit,
> >>
> >> commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling")
> >>
> >> This eventually fixed the mmc write issue observed in
> >> rk3399 sdhci controller.
> >>
> >> Bug log for refernece,  
> >> => gpt write mmc 0 $partitions  
> >> Writing GPT: mmc write failed
> >> ** Can't write to device 0 **
> >> ** Can't write to device 0 **
> >> error!
> >>
> >> Cc: Kever Yang <kever.yang@rock-chips.com>
> >> Cc: Peng Fan <peng.fan@nxp.com>
> >> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >> ---
> >> Changes for v2:
> >> - collect Jaehoon R-b
> >>
> >>   drivers/mmc/sdhci.c | 23 +++++++++++++++--------
> >>   1 file changed, 15 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> >> index 92cc8434af..280b8c88eb 100644
> >> --- a/drivers/mmc/sdhci.c
> >> +++ b/drivers/mmc/sdhci.c
> >> @@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc)
> >>               ctrl &= ~SDHCI_CTRL_4BITBUS;
> >>       }
> >>   -    if (mmc->clock > 26000000)
> >> -        ctrl |= SDHCI_CTRL_HISPD;
> >> -    else
> >> -        ctrl &= ~SDHCI_CTRL_HISPD;
> >> -
> >> -    if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
> >> -        (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
> >> -        ctrl &= ~SDHCI_CTRL_HISPD;
> >> +    if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||  
> > 
> > Should that be "&&" rather than "||"? Otherwise this will always
> > evaluate to true unless *both* quirks are set, which isn't
> > equivalent to the check being removed above.  
> 
> 
> You're right.

It'd be great if you could respin this patch quickly and get it merged,
as it just helped me getting my NanoPC-T4 up and running.

FWIW:

Tested-by: Marc Zyngier <maz@kernel.org>

	M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH v2] mmc: sdhci: Fix HISPD bit handling
@ 2020-06-10 10:37           ` Marc Zyngier
  0 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2020-06-10 10:37 UTC (permalink / raw)
  To: u-boot

On Wed, 10 Jun 2020 12:45:33 +0900
Jaehoon Chung <jh80.chung@samsung.com> wrote:

> On 6/9/20 11:38 PM, Robin Murphy wrote:
> > On 2020-06-09 15:01, Jagan Teki wrote:  
> >> SDHCI HISPD bits need to be configured based on desired mmc
> >> timings mode and some HISPD quirks.
> >>
> >> So, handle the HISPD bit based on the mmc computed selected
> >> mode(timing parameter) rather than fixed mmc card clock
> >> frequency.
> >>
> >> Linux handle the HISPD similar like this in below commit,
> >>
> >> commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling")
> >>
> >> This eventually fixed the mmc write issue observed in
> >> rk3399 sdhci controller.
> >>
> >> Bug log for refernece,  
> >> => gpt write mmc 0 $partitions  
> >> Writing GPT: mmc write failed
> >> ** Can't write to device 0 **
> >> ** Can't write to device 0 **
> >> error!
> >>
> >> Cc: Kever Yang <kever.yang@rock-chips.com>
> >> Cc: Peng Fan <peng.fan@nxp.com>
> >> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >> ---
> >> Changes for v2:
> >> - collect Jaehoon R-b
> >>
> >> ? drivers/mmc/sdhci.c | 23 +++++++++++++++--------
> >> ? 1 file changed, 15 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> >> index 92cc8434af..280b8c88eb 100644
> >> --- a/drivers/mmc/sdhci.c
> >> +++ b/drivers/mmc/sdhci.c
> >> @@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc)
> >> ????????????? ctrl &= ~SDHCI_CTRL_4BITBUS;
> >> ????? }
> >> ? -??? if (mmc->clock > 26000000)
> >> -??????? ctrl |= SDHCI_CTRL_HISPD;
> >> -??? else
> >> -??????? ctrl &= ~SDHCI_CTRL_HISPD;
> >> -
> >> -??? if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
> >> -??????? (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
> >> -??????? ctrl &= ~SDHCI_CTRL_HISPD;
> >> +??? if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||  
> > 
> > Should that be "&&" rather than "||"? Otherwise this will always
> > evaluate to true unless *both* quirks are set, which isn't
> > equivalent to the check being removed above.  
> 
> 
> You're right.

It'd be great if you could respin this patch quickly and get it merged,
as it just helped me getting my NanoPC-T4 up and running.

FWIW:

Tested-by: Marc Zyngier <maz@kernel.org>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v2] mmc: sdhci: Fix HISPD bit handling
  2020-06-09 14:38     ` Robin Murphy
@ 2020-06-10 11:44       ` Jagan Teki
  -1 siblings, 0 replies; 10+ messages in thread
From: Jagan Teki @ 2020-06-10 11:44 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Peng Fan, Jaehoon Chung, Kever Yang, U-Boot-Denx,
	open list:ARM/Rockchip SoC...,
	linux-amarula, Suniel Mahesh

On Tue, Jun 9, 2020 at 8:08 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2020-06-09 15:01, Jagan Teki wrote:
> > SDHCI HISPD bits need to be configured based on desired mmc
> > timings mode and some HISPD quirks.
> >
> > So, handle the HISPD bit based on the mmc computed selected
> > mode(timing parameter) rather than fixed mmc card clock
> > frequency.
> >
> > Linux handle the HISPD similar like this in below commit,
> >
> > commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling")
> >
> > This eventually fixed the mmc write issue observed in
> > rk3399 sdhci controller.
> >
> > Bug log for refernece,
> > => gpt write mmc 0 $partitions
> > Writing GPT: mmc write failed
> > ** Can't write to device 0 **
> > ** Can't write to device 0 **
> > error!
> >
> > Cc: Kever Yang <kever.yang@rock-chips.com>
> > Cc: Peng Fan <peng.fan@nxp.com>
> > Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > Changes for v2:
> > - collect Jaehoon R-b
> >
> >   drivers/mmc/sdhci.c | 23 +++++++++++++++--------
> >   1 file changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> > index 92cc8434af..280b8c88eb 100644
> > --- a/drivers/mmc/sdhci.c
> > +++ b/drivers/mmc/sdhci.c
> > @@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc)
> >                       ctrl &= ~SDHCI_CTRL_4BITBUS;
> >       }
> >
> > -     if (mmc->clock > 26000000)
> > -             ctrl |= SDHCI_CTRL_HISPD;
> > -     else
> > -             ctrl &= ~SDHCI_CTRL_HISPD;
> > -
> > -     if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
> > -         (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
> > -             ctrl &= ~SDHCI_CTRL_HISPD;
> > +     if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
>
> Should that be "&&" rather than "||"? Otherwise this will always
> evaluate to true unless *both* quirks are set, which isn't equivalent to
> the check being removed above.

Correct, thanks for the catch. I have updated ib v3.

Jagan.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH v2] mmc: sdhci: Fix HISPD bit handling
@ 2020-06-10 11:44       ` Jagan Teki
  0 siblings, 0 replies; 10+ messages in thread
From: Jagan Teki @ 2020-06-10 11:44 UTC (permalink / raw)
  To: u-boot

On Tue, Jun 9, 2020 at 8:08 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2020-06-09 15:01, Jagan Teki wrote:
> > SDHCI HISPD bits need to be configured based on desired mmc
> > timings mode and some HISPD quirks.
> >
> > So, handle the HISPD bit based on the mmc computed selected
> > mode(timing parameter) rather than fixed mmc card clock
> > frequency.
> >
> > Linux handle the HISPD similar like this in below commit,
> >
> > commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling")
> >
> > This eventually fixed the mmc write issue observed in
> > rk3399 sdhci controller.
> >
> > Bug log for refernece,
> > => gpt write mmc 0 $partitions
> > Writing GPT: mmc write failed
> > ** Can't write to device 0 **
> > ** Can't write to device 0 **
> > error!
> >
> > Cc: Kever Yang <kever.yang@rock-chips.com>
> > Cc: Peng Fan <peng.fan@nxp.com>
> > Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > Changes for v2:
> > - collect Jaehoon R-b
> >
> >   drivers/mmc/sdhci.c | 23 +++++++++++++++--------
> >   1 file changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> > index 92cc8434af..280b8c88eb 100644
> > --- a/drivers/mmc/sdhci.c
> > +++ b/drivers/mmc/sdhci.c
> > @@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc)
> >                       ctrl &= ~SDHCI_CTRL_4BITBUS;
> >       }
> >
> > -     if (mmc->clock > 26000000)
> > -             ctrl |= SDHCI_CTRL_HISPD;
> > -     else
> > -             ctrl &= ~SDHCI_CTRL_HISPD;
> > -
> > -     if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
> > -         (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE))
> > -             ctrl &= ~SDHCI_CTRL_HISPD;
> > +     if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
>
> Should that be "&&" rather than "||"? Otherwise this will always
> evaluate to true unless *both* quirks are set, which isn't equivalent to
> the check being removed above.

Correct, thanks for the catch. I have updated ib v3.

Jagan.

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-06-10 11:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-09 14:01 [PATCH v2] mmc: sdhci: Fix HISPD bit handling Jagan Teki
2020-06-09 14:01 ` Jagan Teki
     [not found] ` <20200609140135.131887-1-jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
2020-06-09 14:38   ` Robin Murphy
2020-06-09 14:38     ` Robin Murphy
2020-06-10  3:45     ` Jaehoon Chung
2020-06-10  3:45       ` Jaehoon Chung
     [not found]       ` <b46fe992-89d4-4a5f-fdb3-e2ec53de2b11-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2020-06-10 10:37         ` Marc Zyngier
2020-06-10 10:37           ` Marc Zyngier
2020-06-10 11:44     ` Jagan Teki
2020-06-10 11:44       ` Jagan Teki

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.