All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] mmc: sdhci: Fix HISPD bit handling
@ 2020-06-18 14:03 Jagan Teki
  2020-06-22  1:49 ` Peng Fan
  0 siblings, 1 reply; 8+ messages in thread
From: Jagan Teki @ 2020-06-18 14:03 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 but no
SDHCI_QUIRK_BROKEN_HISPD_MODE,

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>
Tested-by: Suniel Mahesh <sunil@amarulasolutions.com> # roc-rk3399-pc
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v4:
- update commit message
- simplify the logic.

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

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 92cc8434af..6cb702111b 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -567,6 +567,7 @@ static int sdhci_set_ios(struct mmc *mmc)
 #endif
 	u32 ctrl;
 	struct sdhci_host *host = mmc->priv;
+	bool no_hispd_bit = false;
 
 	if (host->ops && host->ops->set_control_reg)
 		host->ops->set_control_reg(host);
@@ -594,14 +595,24 @@ 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;
+		no_hispd_bit = true;
+
+	if (!no_hispd_bit) {
+		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] 8+ messages in thread
* [PATCH v4] mmc: sdhci: Fix HISPD bit handling
  2020-06-18 14:03 [PATCH v4] mmc: sdhci: Fix HISPD bit handling Jagan Teki
@ 2020-06-22  1:49 ` Peng Fan
  2020-06-22  9:24   ` Jaehoon Chung
  0 siblings, 1 reply; 8+ messages in thread
From: Peng Fan @ 2020-06-22  1:49 UTC (permalink / raw)
  To: u-boot

Jaehoon,

> Subject: [PATCH v4] mmc: sdhci: Fix HISPD bit handling

Are you fine with this v4?

Thanks,
Peng.

> 
> 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 but no
> SDHCI_QUIRK_BROKEN_HISPD_MODE,
> 
> 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>
> Tested-by: Suniel Mahesh <sunil@amarulasolutions.com> # roc-rk3399-pc
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v4:
> - update commit message
> - simplify the logic.
> 
>  drivers/mmc/sdhci.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index
> 92cc8434af..6cb702111b 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -567,6 +567,7 @@ static int sdhci_set_ios(struct mmc *mmc)  #endif
>  	u32 ctrl;
>  	struct sdhci_host *host = mmc->priv;
> +	bool no_hispd_bit = false;
> 
>  	if (host->ops && host->ops->set_control_reg)
>  		host->ops->set_control_reg(host);
> @@ -594,14 +595,24 @@ 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;
> +		no_hispd_bit = true;
> +
> +	if (!no_hispd_bit) {
> +		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	[flat|nested] 8+ messages in thread
* [PATCH v4] mmc: sdhci: Fix HISPD bit handling
  2020-06-22  1:49 ` Peng Fan
@ 2020-06-22  9:24   ` Jaehoon Chung
  2020-06-22  9:26     ` Jagan Teki
  0 siblings, 1 reply; 8+ messages in thread
From: Jaehoon Chung @ 2020-06-22  9:24 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On 6/22/20 10:49 AM, Peng Fan wrote:
> Jaehoon,
> 
>> Subject: [PATCH v4] mmc: sdhci: Fix HISPD bit handling
> 
> Are you fine with this v4?

Thanks for sharing it. i didn't see patch v4.

> 
> Thanks,
> Peng.
> 
>>
>> 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 but no
>> SDHCI_QUIRK_BROKEN_HISPD_MODE,
>>
>> 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>
>> Tested-by: Suniel Mahesh <sunil@amarulasolutions.com> # roc-rk3399-pc
>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>> ---
>> Changes for v4:
>> - update commit message
>> - simplify the logic.
>>
>>  drivers/mmc/sdhci.c | 23 +++++++++++++++++------
>>  1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index
>> 92cc8434af..6cb702111b 100644
>> --- a/drivers/mmc/sdhci.c
>> +++ b/drivers/mmc/sdhci.c
>> @@ -567,6 +567,7 @@ static int sdhci_set_ios(struct mmc *mmc)  #endif
>>  	u32 ctrl;
>>  	struct sdhci_host *host = mmc->priv;
>> +	bool no_hispd_bit = false;
>>
>>  	if (host->ops && host->ops->set_control_reg)
>>  		host->ops->set_control_reg(host);
>> @@ -594,14 +595,24 @@ 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;
>> +		no_hispd_bit = true;

No. ctrl variable is set to bit[2] of HOST_CONTROL register.
But Some samsung SoCs are using its bit[2] as other purpose.
So it needs to revert the value with "ctrl &= ~SDHCI_CTRL_HISPD".
Because it's possible to set to 1.

SDHCI_QUIRK_NO_HISPD_BIT means that it's used other purpose.

Best Regards,
Jaehoon Chung

>> +
>> +	if (!no_hispd_bit) {
>> +		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	[flat|nested] 8+ messages in thread
* [PATCH v4] mmc: sdhci: Fix HISPD bit handling
  2020-06-22  9:24   ` Jaehoon Chung
@ 2020-06-22  9:26     ` Jagan Teki
  2020-06-22  9:44       ` Jaehoon Chung
  0 siblings, 1 reply; 8+ messages in thread
From: Jagan Teki @ 2020-06-22  9:26 UTC (permalink / raw)
  To: u-boot

On Mon, Jun 22, 2020 at 2:54 PM Jaehoon Chung <jh80.chung@samsung.com> wrote:
>
> Hi Peng,
>
> On 6/22/20 10:49 AM, Peng Fan wrote:
> > Jaehoon,
> >
> >> Subject: [PATCH v4] mmc: sdhci: Fix HISPD bit handling
> >
> > Are you fine with this v4?
>
> Thanks for sharing it. i didn't see patch v4.
>
> >
> > Thanks,
> > Peng.
> >
> >>
> >> 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 but no
> >> SDHCI_QUIRK_BROKEN_HISPD_MODE,
> >>
> >> 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>
> >> Tested-by: Suniel Mahesh <sunil@amarulasolutions.com> # roc-rk3399-pc
> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >> ---
> >> Changes for v4:
> >> - update commit message
> >> - simplify the logic.
> >>
> >>  drivers/mmc/sdhci.c | 23 +++++++++++++++++------
> >>  1 file changed, 17 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index
> >> 92cc8434af..6cb702111b 100644
> >> --- a/drivers/mmc/sdhci.c
> >> +++ b/drivers/mmc/sdhci.c
> >> @@ -567,6 +567,7 @@ static int sdhci_set_ios(struct mmc *mmc)  #endif
> >>      u32 ctrl;
> >>      struct sdhci_host *host = mmc->priv;
> >> +    bool no_hispd_bit = false;
> >>
> >>      if (host->ops && host->ops->set_control_reg)
> >>              host->ops->set_control_reg(host);
> >> @@ -594,14 +595,24 @@ 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;
> >> +            no_hispd_bit = true;
>
> No. ctrl variable is set to bit[2] of HOST_CONTROL register.
> But Some samsung SoCs are using its bit[2] as other purpose.
> So it needs to revert the value with "ctrl &= ~SDHCI_CTRL_HISPD".
> Because it's possible to set to 1.
>
> SDHCI_QUIRK_NO_HISPD_BIT means that it's used other purpose.

So, what are you suggesting? could you please elaborate?

Jagan.

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH v4] mmc: sdhci: Fix HISPD bit handling
  2020-06-22  9:26     ` Jagan Teki
@ 2020-06-22  9:44       ` Jaehoon Chung
  2020-06-24  1:36         ` Peng Fan
  0 siblings, 1 reply; 8+ messages in thread
From: Jaehoon Chung @ 2020-06-22  9:44 UTC (permalink / raw)
  To: u-boot

On 6/22/20 6:26 PM, Jagan Teki wrote:
> On Mon, Jun 22, 2020 at 2:54 PM Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>
>> Hi Peng,
>>
>> On 6/22/20 10:49 AM, Peng Fan wrote:
>>> Jaehoon,
>>>
>>>> Subject: [PATCH v4] mmc: sdhci: Fix HISPD bit handling
>>>
>>> Are you fine with this v4?
>>
>> Thanks for sharing it. i didn't see patch v4.
>>
>>>
>>> Thanks,
>>> Peng.
>>>
>>>>
>>>> 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 but no
>>>> SDHCI_QUIRK_BROKEN_HISPD_MODE,
>>>>
>>>> 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>
>>>> Tested-by: Suniel Mahesh <sunil@amarulasolutions.com> # roc-rk3399-pc
>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>>> ---
>>>> Changes for v4:
>>>> - update commit message
>>>> - simplify the logic.
>>>>
>>>>  drivers/mmc/sdhci.c | 23 +++++++++++++++++------
>>>>  1 file changed, 17 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index
>>>> 92cc8434af..6cb702111b 100644
>>>> --- a/drivers/mmc/sdhci.c
>>>> +++ b/drivers/mmc/sdhci.c
>>>> @@ -567,6 +567,7 @@ static int sdhci_set_ios(struct mmc *mmc)  #endif
>>>>      u32 ctrl;
>>>>      struct sdhci_host *host = mmc->priv;
>>>> +    bool no_hispd_bit = false;
>>>>
>>>>      if (host->ops && host->ops->set_control_reg)
>>>>              host->ops->set_control_reg(host);
>>>> @@ -594,14 +595,24 @@ 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;
>>>> +            no_hispd_bit = true;
>>
>> No. ctrl variable is set to bit[2] of HOST_CONTROL register.
>> But Some samsung SoCs are using its bit[2] as other purpose.
>> So it needs to revert the value with "ctrl &= ~SDHCI_CTRL_HISPD".
>> Because it's possible to set to 1.
>>
>> SDHCI_QUIRK_NO_HISPD_BIT means that it's used other purpose.
> 
> So, what are you suggesting? could you please elaborate?


if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) || 
    (host->quirks & SDHCI_QUIRK_NO_BROKEN_HISPD_MODE)) {
	ctrl &= ~SDHCI_CTRL_HISPD;
	no_hispd_bit = true;
}

Just adding "ctrl &= ~SDHCI_CTRL_HISPD;" into its condition.
Then it's helpful to me.

Best Regards,
Jaehoon Chung

> 
> Jagan.
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH v4] mmc: sdhci: Fix HISPD bit handling
  2020-06-22  9:44       ` Jaehoon Chung
@ 2020-06-24  1:36         ` Peng Fan
  2020-06-24  6:26           ` Jaehoon Chung
  2020-06-24  8:37           ` Jagan Teki
  0 siblings, 2 replies; 8+ messages in thread
From: Peng Fan @ 2020-06-24  1:36 UTC (permalink / raw)
  To: u-boot

All:

> Subject: Re: [PATCH v4] mmc: sdhci: Fix HISPD bit handling
> 
> On 6/22/20 6:26 PM, Jagan Teki wrote:
> > On Mon, Jun 22, 2020 at 2:54 PM Jaehoon Chung
> <jh80.chung@samsung.com> wrote:
> >>
> >> Hi Peng,
> >>
> >> On 6/22/20 10:49 AM, Peng Fan wrote:
> >>> Jaehoon,
> >>>
> >>>> Subject: [PATCH v4] mmc: sdhci: Fix HISPD bit handling
> >>>
> >>> Are you fine with this v4?
> >>
> >> Thanks for sharing it. i didn't see patch v4.
> >>
> >>>
> >>> Thanks,
> >>> Peng.
> >>>
> >>>>
> >>>> 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 but no
> >>>> SDHCI_QUIRK_BROKEN_HISPD_MODE,
> >>>>
> >>>> 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>
> >>>> Tested-by: Suniel Mahesh <sunil@amarulasolutions.com> #
> >>>> roc-rk3399-pc
> >>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >>>> ---
> >>>> Changes for v4:
> >>>> - update commit message
> >>>> - simplify the logic.
> >>>>
> >>>>  drivers/mmc/sdhci.c | 23 +++++++++++++++++------
> >>>>  1 file changed, 17 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index
> >>>> 92cc8434af..6cb702111b 100644
> >>>> --- a/drivers/mmc/sdhci.c
> >>>> +++ b/drivers/mmc/sdhci.c
> >>>> @@ -567,6 +567,7 @@ static int sdhci_set_ios(struct mmc *mmc)
> #endif
> >>>>      u32 ctrl;
> >>>>      struct sdhci_host *host = mmc->priv;
> >>>> +    bool no_hispd_bit = false;
> >>>>
> >>>>      if (host->ops && host->ops->set_control_reg)
> >>>>              host->ops->set_control_reg(host); @@ -594,14
> +595,24
> >>>> @@ 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;
> >>>> +            no_hispd_bit = true;
> >>
> >> No. ctrl variable is set to bit[2] of HOST_CONTROL register.
> >> But Some samsung SoCs are using its bit[2] as other purpose.
> >> So it needs to revert the value with "ctrl &= ~SDHCI_CTRL_HISPD".
> >> Because it's possible to set to 1.
> >>
> >> SDHCI_QUIRK_NO_HISPD_BIT means that it's used other purpose.
> >
> > So, what are you suggesting? could you please elaborate?
> 
> 
> if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
>     (host->quirks & SDHCI_QUIRK_NO_BROKEN_HISPD_MODE)) {
> 	ctrl &= ~SDHCI_CTRL_HISPD;
> 	no_hispd_bit = true;
> }
> 
> Just adding "ctrl &= ~SDHCI_CTRL_HISPD;" into its condition.

I pushed this patch with the upper code added to my CI
with a update in commit log:
https://travis-ci.org/github/MrVan/u-boot/builds/701486010

Please let me know if any objections.

Thanks,
Peng.

> Then it's helpful to me.
> 
> Best Regards,
> Jaehoon Chung
> 
> >
> > Jagan.
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH v4] mmc: sdhci: Fix HISPD bit handling
  2020-06-24  1:36         ` Peng Fan
@ 2020-06-24  6:26           ` Jaehoon Chung
  2020-06-24  8:37           ` Jagan Teki
  1 sibling, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2020-06-24  6:26 UTC (permalink / raw)
  To: u-boot

On 6/24/20 10:36 AM, Peng Fan wrote:
> All:
> 
>> Subject: Re: [PATCH v4] mmc: sdhci: Fix HISPD bit handling
>>
>> On 6/22/20 6:26 PM, Jagan Teki wrote:
>>> On Mon, Jun 22, 2020 at 2:54 PM Jaehoon Chung
>> <jh80.chung@samsung.com> wrote:
>>>>
>>>> Hi Peng,
>>>>
>>>> On 6/22/20 10:49 AM, Peng Fan wrote:
>>>>> Jaehoon,
>>>>>
>>>>>> Subject: [PATCH v4] mmc: sdhci: Fix HISPD bit handling
>>>>>
>>>>> Are you fine with this v4?
>>>>
>>>> Thanks for sharing it. i didn't see patch v4.
>>>>
>>>>>
>>>>> Thanks,
>>>>> Peng.
>>>>>
>>>>>>
>>>>>> 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 but no
>>>>>> SDHCI_QUIRK_BROKEN_HISPD_MODE,
>>>>>>
>>>>>> 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>
>>>>>> Tested-by: Suniel Mahesh <sunil@amarulasolutions.com> #
>>>>>> roc-rk3399-pc
>>>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>>>>> ---
>>>>>> Changes for v4:
>>>>>> - update commit message
>>>>>> - simplify the logic.
>>>>>>
>>>>>>  drivers/mmc/sdhci.c | 23 +++++++++++++++++------
>>>>>>  1 file changed, 17 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index
>>>>>> 92cc8434af..6cb702111b 100644
>>>>>> --- a/drivers/mmc/sdhci.c
>>>>>> +++ b/drivers/mmc/sdhci.c
>>>>>> @@ -567,6 +567,7 @@ static int sdhci_set_ios(struct mmc *mmc)
>> #endif
>>>>>>      u32 ctrl;
>>>>>>      struct sdhci_host *host = mmc->priv;
>>>>>> +    bool no_hispd_bit = false;
>>>>>>
>>>>>>      if (host->ops && host->ops->set_control_reg)
>>>>>>              host->ops->set_control_reg(host); @@ -594,14
>> +595,24
>>>>>> @@ 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;
>>>>>> +            no_hispd_bit = true;
>>>>
>>>> No. ctrl variable is set to bit[2] of HOST_CONTROL register.
>>>> But Some samsung SoCs are using its bit[2] as other purpose.
>>>> So it needs to revert the value with "ctrl &= ~SDHCI_CTRL_HISPD".
>>>> Because it's possible to set to 1.
>>>>
>>>> SDHCI_QUIRK_NO_HISPD_BIT means that it's used other purpose.
>>>
>>> So, what are you suggesting? could you please elaborate?
>>
>>
>> if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
>>     (host->quirks & SDHCI_QUIRK_NO_BROKEN_HISPD_MODE)) {
>> 	ctrl &= ~SDHCI_CTRL_HISPD;
>> 	no_hispd_bit = true;
>> }
>>
>> Just adding "ctrl &= ~SDHCI_CTRL_HISPD;" into its condition.
> 
> I pushed this patch with the upper code added to my CI
> with a update in commit log:
> https://protect2.fireeye.com/url?k=a0ad72d4-fd674763-a0acf99b-0cc47a3003e8-b7ff57d36725a01c&q=1&u=https%3A%2F%2Ftravis-ci.org%2Fgithub%2FMrVan%2Fu-boot%2Fbuilds%2F701486010
> 
> Please let me know if any objections.

Thanks!

> 
> Thanks,
> Peng.
> 
>> Then it's helpful to me.
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> Jagan.
>>>
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH v4] mmc: sdhci: Fix HISPD bit handling
  2020-06-24  1:36         ` Peng Fan
  2020-06-24  6:26           ` Jaehoon Chung
@ 2020-06-24  8:37           ` Jagan Teki
  1 sibling, 0 replies; 8+ messages in thread
From: Jagan Teki @ 2020-06-24  8:37 UTC (permalink / raw)
  To: u-boot

On Wed, Jun 24, 2020 at 7:06 AM Peng Fan <peng.fan@nxp.com> wrote:
>
> All:
>
> > Subject: Re: [PATCH v4] mmc: sdhci: Fix HISPD bit handling
> >
> > On 6/22/20 6:26 PM, Jagan Teki wrote:
> > > On Mon, Jun 22, 2020 at 2:54 PM Jaehoon Chung
> > <jh80.chung@samsung.com> wrote:
> > >>
> > >> Hi Peng,
> > >>
> > >> On 6/22/20 10:49 AM, Peng Fan wrote:
> > >>> Jaehoon,
> > >>>
> > >>>> Subject: [PATCH v4] mmc: sdhci: Fix HISPD bit handling
> > >>>
> > >>> Are you fine with this v4?
> > >>
> > >> Thanks for sharing it. i didn't see patch v4.
> > >>
> > >>>
> > >>> Thanks,
> > >>> Peng.
> > >>>
> > >>>>
> > >>>> 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 but no
> > >>>> SDHCI_QUIRK_BROKEN_HISPD_MODE,
> > >>>>
> > >>>> 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>
> > >>>> Tested-by: Suniel Mahesh <sunil@amarulasolutions.com> #
> > >>>> roc-rk3399-pc
> > >>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > >>>> ---
> > >>>> Changes for v4:
> > >>>> - update commit message
> > >>>> - simplify the logic.
> > >>>>
> > >>>>  drivers/mmc/sdhci.c | 23 +++++++++++++++++------
> > >>>>  1 file changed, 17 insertions(+), 6 deletions(-)
> > >>>>
> > >>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index
> > >>>> 92cc8434af..6cb702111b 100644
> > >>>> --- a/drivers/mmc/sdhci.c
> > >>>> +++ b/drivers/mmc/sdhci.c
> > >>>> @@ -567,6 +567,7 @@ static int sdhci_set_ios(struct mmc *mmc)
> > #endif
> > >>>>      u32 ctrl;
> > >>>>      struct sdhci_host *host = mmc->priv;
> > >>>> +    bool no_hispd_bit = false;
> > >>>>
> > >>>>      if (host->ops && host->ops->set_control_reg)
> > >>>>              host->ops->set_control_reg(host); @@ -594,14
> > +595,24
> > >>>> @@ 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;
> > >>>> +            no_hispd_bit = true;
> > >>
> > >> No. ctrl variable is set to bit[2] of HOST_CONTROL register.
> > >> But Some samsung SoCs are using its bit[2] as other purpose.
> > >> So it needs to revert the value with "ctrl &= ~SDHCI_CTRL_HISPD".
> > >> Because it's possible to set to 1.
> > >>
> > >> SDHCI_QUIRK_NO_HISPD_BIT means that it's used other purpose.
> > >
> > > So, what are you suggesting? could you please elaborate?
> >
> >
> > if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) ||
> >     (host->quirks & SDHCI_QUIRK_NO_BROKEN_HISPD_MODE)) {
> >       ctrl &= ~SDHCI_CTRL_HISPD;
> >       no_hispd_bit = true;
> > }
> >
> > Just adding "ctrl &= ~SDHCI_CTRL_HISPD;" into its condition.
>
> I pushed this patch with the upper code added to my CI
> with a update in commit log:
> https://travis-ci.org/github/MrVan/u-boot/builds/701486010
>
> Please let me know if any objections.

Thanks for taking care of this.

Jagan.

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

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-18 14:03 [PATCH v4] mmc: sdhci: Fix HISPD bit handling Jagan Teki
2020-06-22  1:49 ` Peng Fan
2020-06-22  9:24   ` Jaehoon Chung
2020-06-22  9:26     ` Jagan Teki
2020-06-22  9:44       ` Jaehoon Chung
2020-06-24  1:36         ` Peng Fan
2020-06-24  6:26           ` Jaehoon Chung
2020-06-24  8:37           ` 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.