diff mbox series

mmc: sdhci: Write to HOST_CONTROL2 register for HS400 speed mode

Message ID 20210405144428.12159-1-a-govindraju@ti.com
State New
Delegated to: Peng Fan
Headers show
Series mmc: sdhci: Write to HOST_CONTROL2 register for HS400 speed mode | expand

Commit Message

Aswath Govindraju April 5, 2021, 2:44 p.m. UTC
From: Faiz Abbas <faiz_abbas@ti.com>

Enable HS400 speed mode by writing to HOST_CONTROL2 register.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
---
 drivers/mmc/sdhci.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jaehoon Chung April 5, 2021, 10:18 p.m. UTC | #1
Hi,

On 4/5/21 11:44 PM, Aswath Govindraju wrote:
> From: Faiz Abbas <faiz_abbas@ti.com>
> 
> Enable HS400 speed mode by writing to HOST_CONTROL2 register.

I didn't find HS400 bit at HOST_CONTROL2 register. (I have checked SD Specific v4.20)
If I missed something, let me know, plz.

In include/sdhci.h, it mentioned to "Non-standard".

Best Regards,
Jaehoon Chung

> 
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
> ---
>  drivers/mmc/sdhci.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index d9ab6a0a839e..eea4701d8af5 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -507,6 +507,9 @@ void sdhci_set_uhs_timing(struct sdhci_host *host)
>  	case MMC_HS_200:
>  		reg |= SDHCI_CTRL_UHS_SDR104;
>  		break;
> +	case MMC_HS_400:
> +		reg |= SDHCI_CTRL_HS400;
> +		break;
>  	default:
>  		reg |= SDHCI_CTRL_UHS_SDR12;
>  	}
>
Aswath Govindraju April 6, 2021, 5:20 a.m. UTC | #2
Hi Jaehoon,

On 06/04/21 3:48 am, Jaehoon Chung wrote:
> Hi,
> 
> On 4/5/21 11:44 PM, Aswath Govindraju wrote:
>> From: Faiz Abbas <faiz_abbas@ti.com>
>>
>> Enable HS400 speed mode by writing to HOST_CONTROL2 register.
> 
> I didn't find HS400 bit at HOST_CONTROL2 register. (I have checked SD Specific v4.20)
> If I missed something, let me know, plz.
> 
> In include/sdhci.h, it mentioned to "Non-standard".
> 

Thank you for pointing this out.

Yes, this is not a part of SD specification and in the specification
this field is marked as reserved. I have added this, as this bit mask
has been defined in include/sdhci.h. Sorry about this. I'll post a
respin taking this into a consideration.

Thanks,
Aswath

> Best Regards,
> Jaehoon Chung
> 
>>
>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>> ---
>>  drivers/mmc/sdhci.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index d9ab6a0a839e..eea4701d8af5 100644
>> --- a/drivers/mmc/sdhci.c
>> +++ b/drivers/mmc/sdhci.c
>> @@ -507,6 +507,9 @@ void sdhci_set_uhs_timing(struct sdhci_host *host)
>>  	case MMC_HS_200:
>>  		reg |= SDHCI_CTRL_UHS_SDR104;
>>  		break;
>> +	case MMC_HS_400:
>> +		reg |= SDHCI_CTRL_HS400;
>> +		break;
>>  	default:
>>  		reg |= SDHCI_CTRL_UHS_SDR12;
>>  	}
>>
>
Aswath Govindraju April 6, 2021, 8:26 a.m. UTC | #3
Hi Jaehoon,

On 06/04/21 10:50 am, Aswath Govindraju wrote:
> Hi Jaehoon,
> 
> On 06/04/21 3:48 am, Jaehoon Chung wrote:
>> Hi,
>>
>> On 4/5/21 11:44 PM, Aswath Govindraju wrote:
>>> From: Faiz Abbas <faiz_abbas@ti.com>
>>>
>>> Enable HS400 speed mode by writing to HOST_CONTROL2 register.
>>
>> I didn't find HS400 bit at HOST_CONTROL2 register. (I have checked SD Specific v4.20)
>> If I missed something, let me know, plz.
>>
>> In include/sdhci.h, it mentioned to "Non-standard".
>>
> 
> Thank you for pointing this out.
> 
> Yes, this is not a part of SD specification and in the specification
> this field is marked as reserved. I have added this, as this bit mask
> has been defined in include/sdhci.h. Sorry about this. I'll post a
> respin taking this into a consideration.
> 

On the other hand, the same has been accepted in kernel i.e. setting
SDHCI_CTRL_HS400 bit in HOST_CONTROL2 register, drivers/mmc/host/sdhci.c
in sdhci_set_uhs_signaling(). I think although this bit has not been
mentioned in the specification, it seems it is used by many controllers
for HS400 mode. So, as this is not specific to a controller can setting
this, be allowed in the common sdhci_set_uhs_timing() function ?

Thanks,
Aswath

> Thanks,
> Aswath
> 
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>>> ---
>>>  drivers/mmc/sdhci.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>>> index d9ab6a0a839e..eea4701d8af5 100644
>>> --- a/drivers/mmc/sdhci.c
>>> +++ b/drivers/mmc/sdhci.c
>>> @@ -507,6 +507,9 @@ void sdhci_set_uhs_timing(struct sdhci_host *host)
>>>  	case MMC_HS_200:
>>>  		reg |= SDHCI_CTRL_UHS_SDR104;
>>>  		break;
>>> +	case MMC_HS_400:
>>> +		reg |= SDHCI_CTRL_HS400;
>>> +		break;
>>>  	default:
>>>  		reg |= SDHCI_CTRL_UHS_SDR12;
>>>  	}
>>>
>>
>
Jaehoon Chung April 6, 2021, 9:14 a.m. UTC | #4
Hi Aswath,

On 4/6/21 5:26 PM, Aswath Govindraju wrote:
> Hi Jaehoon,
> 
> On 06/04/21 10:50 am, Aswath Govindraju wrote:
>> Hi Jaehoon,
>>
>> On 06/04/21 3:48 am, Jaehoon Chung wrote:
>>> Hi,
>>>
>>> On 4/5/21 11:44 PM, Aswath Govindraju wrote:
>>>> From: Faiz Abbas <faiz_abbas@ti.com>
>>>>
>>>> Enable HS400 speed mode by writing to HOST_CONTROL2 register.
>>>
>>> I didn't find HS400 bit at HOST_CONTROL2 register. (I have checked SD Specific v4.20)
>>> If I missed something, let me know, plz.
>>>
>>> In include/sdhci.h, it mentioned to "Non-standard".
>>>
>>
>> Thank you for pointing this out.
>>
>> Yes, this is not a part of SD specification and in the specification
>> this field is marked as reserved. I have added this, as this bit mask
>> has been defined in include/sdhci.h. Sorry about this. I'll post a
>> respin taking this into a consideration.
>>
> 
> On the other hand, the same has been accepted in kernel i.e. setting
> SDHCI_CTRL_HS400 bit in HOST_CONTROL2 register, drivers/mmc/host/sdhci.c
> in sdhci_set_uhs_signaling(). I think although this bit has not been
> mentioned in the specification, it seems it is used by many controllers
> for HS400 mode. So, as this is not specific to a controller can setting
> this, be allowed in the common sdhci_set_uhs_timing() function ?

Frankly, i think that it can be used. Just I wondered that i missed something from Specification. :)
Thanks for sharing information. I didn't check on kernel side.


Best Regards,
Jaehoon Chung

> 
> Thanks,
> Aswath
> 
>> Thanks,
>> Aswath
>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>>
>>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>>>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>>>> ---
>>>>  drivers/mmc/sdhci.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>>>> index d9ab6a0a839e..eea4701d8af5 100644
>>>> --- a/drivers/mmc/sdhci.c
>>>> +++ b/drivers/mmc/sdhci.c
>>>> @@ -507,6 +507,9 @@ void sdhci_set_uhs_timing(struct sdhci_host *host)
>>>>  	case MMC_HS_200:
>>>>  		reg |= SDHCI_CTRL_UHS_SDR104;
>>>>  		break;
>>>> +	case MMC_HS_400:
>>>> +		reg |= SDHCI_CTRL_HS400;
>>>> +		break;
>>>>  	default:
>>>>  		reg |= SDHCI_CTRL_UHS_SDR12;
>>>>  	}
>>>>
>>>
>>
> 
>
Jaehoon Chung April 6, 2021, 10:22 p.m. UTC | #5
On 4/5/21 11:44 PM, Aswath Govindraju wrote:
> From: Faiz Abbas <faiz_abbas@ti.com>
> 
> Enable HS400 speed mode by writing to HOST_CONTROL2 register.
> 
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  drivers/mmc/sdhci.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index d9ab6a0a839e..eea4701d8af5 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -507,6 +507,9 @@ void sdhci_set_uhs_timing(struct sdhci_host *host)
>  	case MMC_HS_200:
>  		reg |= SDHCI_CTRL_UHS_SDR104;
>  		break;
> +	case MMC_HS_400:
> +		reg |= SDHCI_CTRL_HS400;
> +		break;
>  	default:
>  		reg |= SDHCI_CTRL_UHS_SDR12;
>  	}
>
Aswath Govindraju May 10, 2021, 1:48 p.m. UTC | #6
Hi Peng,

On 07/04/21 3:52 am, Jaehoon Chung wrote:
> On 4/5/21 11:44 PM, Aswath Govindraju wrote:
>> From: Faiz Abbas <faiz_abbas@ti.com>
>>
>> Enable HS400 speed mode by writing to HOST_CONTROL2 register.
>>
>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
> 
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> 

Can you please pick this patch if there are no comments.

Thanks,
Aswath

> Best Regards,
> Jaehoon Chung
> 
>> ---
>>  drivers/mmc/sdhci.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index d9ab6a0a839e..eea4701d8af5 100644
>> --- a/drivers/mmc/sdhci.c
>> +++ b/drivers/mmc/sdhci.c
>> @@ -507,6 +507,9 @@ void sdhci_set_uhs_timing(struct sdhci_host *host)
>>  	case MMC_HS_200:
>>  		reg |= SDHCI_CTRL_UHS_SDR104;
>>  		break;
>> +	case MMC_HS_400:
>> +		reg |= SDHCI_CTRL_HS400;
>> +		break;
>>  	default:
>>  		reg |= SDHCI_CTRL_UHS_SDR12;
>>  	}
>>
>
Aswath Govindraju June 9, 2021, 3:26 p.m. UTC | #7
Hi Peng,

On 10/05/21 7:18 pm, Aswath Govindraju wrote:
> Hi Peng,
> 
> On 07/04/21 3:52 am, Jaehoon Chung wrote:
>> On 4/5/21 11:44 PM, Aswath Govindraju wrote:
>>> From: Faiz Abbas <faiz_abbas@ti.com>
>>>
>>> Enable HS400 speed mode by writing to HOST_CONTROL2 register.
>>>
>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>>
>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>>
> 
> Can you please pick this patch if there are no comments.
> 

May I know if this okay to be merged ?

Thanks,
Aswath

> Thanks,
> Aswath
> 
>> Best Regards,
>> Jaehoon Chung
>>
>>> ---
>>>  drivers/mmc/sdhci.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>>> index d9ab6a0a839e..eea4701d8af5 100644
>>> --- a/drivers/mmc/sdhci.c
>>> +++ b/drivers/mmc/sdhci.c
>>> @@ -507,6 +507,9 @@ void sdhci_set_uhs_timing(struct sdhci_host *host)
>>>  	case MMC_HS_200:
>>>  		reg |= SDHCI_CTRL_UHS_SDR104;
>>>  		break;
>>> +	case MMC_HS_400:
>>> +		reg |= SDHCI_CTRL_HS400;
>>> +		break;
>>>  	default:
>>>  		reg |= SDHCI_CTRL_UHS_SDR12;
>>>  	}
>>>
>>
>
diff mbox series

Patch

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index d9ab6a0a839e..eea4701d8af5 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -507,6 +507,9 @@  void sdhci_set_uhs_timing(struct sdhci_host *host)
 	case MMC_HS_200:
 		reg |= SDHCI_CTRL_UHS_SDR104;
 		break;
+	case MMC_HS_400:
+		reg |= SDHCI_CTRL_HS400;
+		break;
 	default:
 		reg |= SDHCI_CTRL_UHS_SDR12;
 	}