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
State New
Headers show
Series Add ethernet support for Qualcomm SA8155p-ADP board | expand

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

Bhupesh Sharma March 1, 2022, 7:44 p.m. UTC | #1
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,
 };