[PATCH v4 3/4] mmc: dw_mmc-rockchip: Skip all phases bigger than 270 degrees

Detlev Casanova posted 4 patches 1 year, 5 months ago
There is a newer version of this series
[PATCH v4 3/4] mmc: dw_mmc-rockchip: Skip all phases bigger than 270 degrees
Posted by Detlev Casanova 1 year, 5 months ago
From: Shawn Lin <shawn.lin@rock-chips.com>

Per design recommendation, it'd better not try to use any phase
which is bigger than 270. Let's officially follow this.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
(cherry picked from commit 2a53aab5cfa43065b2e979959d727332a8a03c03)
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 drivers/mmc/host/dw_mmc-rockchip.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index 2748f9bf2691..1458cb5fd5c7 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -310,6 +310,9 @@ static int dw_mci_rk3288_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
 
 	/* Try each phase and extract good ranges */
 	for (i = 0; i < priv->num_phases; ) {
+		/* Cannot guarantee any phases larger than 270 would work well */
+		if (TUNING_ITERATION_TO_PHASE(i, priv->num_phases) > 270)
+			break;
 		rockchip_mmc_set_phase(host, true,
 				       TUNING_ITERATION_TO_PHASE(
 						i,
-- 
2.46.0
Re: [PATCH v4 3/4] mmc: dw_mmc-rockchip: Skip all phases bigger than 270 degrees
Posted by Dragan Simic 1 year, 5 months ago
Hello Detlev,

On 2024-08-22 23:15, Detlev Casanova wrote:
> From: Shawn Lin <shawn.lin@rock-chips.com>
> 
> Per design recommendation, it'd better not try to use any phase
> which is bigger than 270. Let's officially follow this.

Would it be possible to provide a reference to the actual design
specification?  This change affects all users of the dw_mmc-rockchip
driver, so in case any regressions are found later, having as much
detail as possible can only be beneficial.

> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> (cherry picked from commit 2a53aab5cfa43065b2e979959d727332a8a03c03)
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  drivers/mmc/host/dw_mmc-rockchip.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c
> b/drivers/mmc/host/dw_mmc-rockchip.c
> index 2748f9bf2691..1458cb5fd5c7 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -310,6 +310,9 @@ static int dw_mci_rk3288_execute_tuning(struct
> dw_mci_slot *slot, u32 opcode)
> 
>  	/* Try each phase and extract good ranges */
>  	for (i = 0; i < priv->num_phases; ) {
> +		/* Cannot guarantee any phases larger than 270 would work well */
> +		if (TUNING_ITERATION_TO_PHASE(i, priv->num_phases) > 270)
> +			break;
>  		rockchip_mmc_set_phase(host, true,
>  				       TUNING_ITERATION_TO_PHASE(
>  						i,
Re: [PATCH v4 3/4] mmc: dw_mmc-rockchip: Skip all phases bigger than 270 degrees
Posted by Detlev Casanova 1 year, 5 months ago
Hi Dragan,

On Friday, 23 August 2024 01:45:07 EDT Dragan Simic wrote:
> Hello Detlev,
> 
> On 2024-08-22 23:15, Detlev Casanova wrote:
> > From: Shawn Lin <shawn.lin@rock-chips.com>
> > 
> > Per design recommendation, it'd better not try to use any phase
> > which is bigger than 270. Let's officially follow this.
> 
> Would it be possible to provide a reference to the actual design
> specification?  This change affects all users of the dw_mmc-rockchip
> driver, so in case any regressions are found later, having as much
> detail as possible can only be beneficial.

I don't have the reference and only trusting rockchip on this. This could be 
specific to rockchip hardware.
Anyway, the drivers works well on my side on my rk3576 armsom sige5 without 
this patch, so I'm willing to drop it completely.

> > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > (cherry picked from commit 2a53aab5cfa43065b2e979959d727332a8a03c03)
> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > ---
> > 
> >  drivers/mmc/host/dw_mmc-rockchip.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/mmc/host/dw_mmc-rockchip.c
> > b/drivers/mmc/host/dw_mmc-rockchip.c
> > index 2748f9bf2691..1458cb5fd5c7 100644
> > --- a/drivers/mmc/host/dw_mmc-rockchip.c
> > +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> > @@ -310,6 +310,9 @@ static int dw_mci_rk3288_execute_tuning(struct
> > dw_mci_slot *slot, u32 opcode)
> > 
> >  	/* Try each phase and extract good ranges */
> >  	for (i = 0; i < priv->num_phases; ) {
> > 
> > +		/* Cannot guarantee any phases larger than 270 would 
work well */
> > +		if (TUNING_ITERATION_TO_PHASE(i, priv->num_phases) > 
270)
> > +			break;
> > 
> >  		rockchip_mmc_set_phase(host, true,
> >  		
> >  				       TUNING_ITERATION_TO_PHASE(
> >  						
> >  						i,
Re: [PATCH v4 3/4] mmc: dw_mmc-rockchip: Skip all phases bigger than 270 degrees
Posted by Dragan Simic 1 year, 5 months ago
Hello Detlev,

On 2024-08-23 15:59, Detlev Casanova wrote:
> On Friday, 23 August 2024 01:45:07 EDT Dragan Simic wrote:
>> Hello Detlev,
>> 
>> On 2024-08-22 23:15, Detlev Casanova wrote:
>> > From: Shawn Lin <shawn.lin@rock-chips.com>
>> >
>> > Per design recommendation, it'd better not try to use any phase
>> > which is bigger than 270. Let's officially follow this.
>> 
>> Would it be possible to provide a reference to the actual design
>> specification?  This change affects all users of the dw_mmc-rockchip
>> driver, so in case any regressions are found later, having as much
>> detail as possible can only be beneficial.
> 
> I don't have the reference and only trusting rockchip on this. This 
> could be
> specific to rockchip hardware.
> Anyway, the drivers works well on my side on my rk3576 armsom sige5 
> without
> this patch, so I'm willing to drop it completely.

I think it would be better if you'd drop it in this series, and submit
it later separately, as a follow-up patch, to reduce the chances for any
possible regressions.  Maybe we'll also have more background information
available by then, who knows.

>> > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> > (cherry picked from commit 2a53aab5cfa43065b2e979959d727332a8a03c03)
>> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
>> > ---
>> >
>> >  drivers/mmc/host/dw_mmc-rockchip.c | 3 +++
>> >  1 file changed, 3 insertions(+)
>> >
>> > diff --git a/drivers/mmc/host/dw_mmc-rockchip.c
>> > b/drivers/mmc/host/dw_mmc-rockchip.c
>> > index 2748f9bf2691..1458cb5fd5c7 100644
>> > --- a/drivers/mmc/host/dw_mmc-rockchip.c
>> > +++ b/drivers/mmc/host/dw_mmc-rockchip.c
>> > @@ -310,6 +310,9 @@ static int dw_mci_rk3288_execute_tuning(struct
>> > dw_mci_slot *slot, u32 opcode)
>> >
>> >  	/* Try each phase and extract good ranges */
>> >  	for (i = 0; i < priv->num_phases; ) {
>> >
>> > +		/* Cannot guarantee any phases larger than 270 would
> work well */
>> > +		if (TUNING_ITERATION_TO_PHASE(i, priv->num_phases) >
> 270)
>> > +			break;
>> >
>> >  		rockchip_mmc_set_phase(host, true,
>> >
>> >  				       TUNING_ITERATION_TO_PHASE(
>> >
>> >  						i,