diff mbox series

[8/8] clk: qcom: gcc-sm8150: Use PWRSTS_ON (only) as a workaround for emac gdsc

Message ID 20220126221725.710167-9-bhupesh.sharma@linaro.org (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series Add ethernet support for Qualcomm SA8155p-ADP board | expand

Checks

Context Check Description
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 17 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Bhupesh Sharma Jan. 26, 2022, 10:17 p.m. UTC
EMAC GDSC currently has issues (seen on SA8155p-ADP) when its
turn'ed ON, once its already in OFF state. So, use PWRSTS_ON
state (only) as a workaround for now.

Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 drivers/clk/qcom/gcc-sm8150.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Bjorn Andersson Feb. 1, 2022, 12:05 a.m. UTC | #1
On Wed 26 Jan 16:17 CST 2022, Bhupesh Sharma wrote:

> EMAC GDSC currently has issues (seen on SA8155p-ADP) when its
> turn'ed ON, once its already in OFF state. So, use PWRSTS_ON
> state (only) as a workaround for now.
> 
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  drivers/clk/qcom/gcc-sm8150.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c
> index 2e71afed81fd..fd7e931d3c09 100644
> --- a/drivers/clk/qcom/gcc-sm8150.c
> +++ b/drivers/clk/qcom/gcc-sm8150.c
> @@ -3449,12 +3449,16 @@ static struct clk_branch gcc_video_xo_clk = {
>  	},
>  };
>  
> +/* To Do: EMAC GDSC currently has issues when its turn'ed ON, once
> + * its already in OFF state. So use PWRSTS_ON state (only) as a
> + * workaround for now.

So you're not able to turn on the GDSC after turning it off?

> + */
>  static struct gdsc emac_gdsc = {
>  	.gdscr = 0x6004,
>  	.pd = {
>  		.name = "emac_gdsc",
>  	},
> -	.pwrsts = PWRSTS_OFF_ON,
> +	.pwrsts = PWRSTS_ON,

Doesn't this tell the gdsc driver that the only state supported is "on"
and hence prohibit you from turning it on in the first place?

>  	.flags = POLL_CFG_GDSCR,

You could add ALWAYS_ON to .flags, but we need a better description of
the actual problem that you're working around.

Regards,
Bjorn

>  };
>  
> -- 
> 2.34.1
>
Bhupesh Sharma March 1, 2022, 7:44 p.m. UTC | #2
Hi Bjorn,

On Tue, 1 Feb 2022 at 05:35, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
>
> On Wed 26 Jan 16:17 CST 2022, Bhupesh Sharma wrote:
>
> > EMAC GDSC currently has issues (seen on SA8155p-ADP) when its
> > turn'ed ON, once its already in OFF state. So, use PWRSTS_ON
> > state (only) as a workaround for now.
> >
> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Cc: Stephen Boyd <sboyd@kernel.org>
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> > ---
> >  drivers/clk/qcom/gcc-sm8150.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c
> > index 2e71afed81fd..fd7e931d3c09 100644
> > --- a/drivers/clk/qcom/gcc-sm8150.c
> > +++ b/drivers/clk/qcom/gcc-sm8150.c
> > @@ -3449,12 +3449,16 @@ static struct clk_branch gcc_video_xo_clk = {
> >       },
> >  };
> >
> > +/* To Do: EMAC GDSC currently has issues when its turn'ed ON, once
> > + * its already in OFF state. So use PWRSTS_ON state (only) as a
> > + * workaround for now.
>
> So you're not able to turn on the GDSC after turning it off?

Indeed. On the SM8150 platform (SA8155p ADP board), what I am
observing is that the
ethernet interface CLKs (RGMII clock etc) cannot be turned on once the
EMAC GDSC is moved
from an OFF to ON state. This is because the EMAC GDSC cannot be
properly turned ON once it is
in the OFF state.

So, basically if we leave the EMAC GDSC on from boot (which is default
bootloader setting), the eth interface
can always come up fine and it can also be used for traffic tx/rx.

> > + */
> >  static struct gdsc emac_gdsc = {
> >       .gdscr = 0x6004,
> >       .pd = {
> >               .name = "emac_gdsc",
> >       },
> > -     .pwrsts = PWRSTS_OFF_ON,
> > +     .pwrsts = PWRSTS_ON,
>
> Doesn't this tell the gdsc driver that the only state supported is "on"
> and hence prohibit you from turning it on in the first place?

That's correct indeed.  Without this hack in place, the EMAC GDSC is not able to
switch from an OFF to ON state, so when the 'eth' interface is turned
up it fails (as RGMII CLK is unavailable):

qcom-ethqos 20000.ethernet eth0: PHY [stmmac-0:07] driver [Micrel
KSZ9031 Gigabit PHY] (irq=150)
<..snip..>
qcom-ethqos 20000.ethernet: Failed to reset the dma
qcom-ethqos 20000.ethernet eth0: stmmac_hw_setup: DMA engine
initialization failed
qcom-ethqos 20000.ethernet eth0: stmmac_open: Hw setup failed

> >       .flags = POLL_CFG_GDSCR,
>
> You could add ALWAYS_ON to .flags, but we need a better description of
> the actual problem that you're working around.

I agree. Let me add the above 'stmmac dma reset' issue while
describing the workaround in the next version of the patch.

Regards,
Bhupesh

> >  };
> >
> > --
> > 2.34.1
> >
diff mbox series

Patch

diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c
index 2e71afed81fd..fd7e931d3c09 100644
--- a/drivers/clk/qcom/gcc-sm8150.c
+++ b/drivers/clk/qcom/gcc-sm8150.c
@@ -3449,12 +3449,16 @@  static struct clk_branch gcc_video_xo_clk = {
 	},
 };
 
+/* To Do: EMAC GDSC currently has issues when its turn'ed ON, once
+ * its already in OFF state. So use PWRSTS_ON state (only) as a
+ * workaround for now.
+ */
 static struct gdsc emac_gdsc = {
 	.gdscr = 0x6004,
 	.pd = {
 		.name = "emac_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_ON,
 	.flags = POLL_CFG_GDSCR,
 };