| Message ID | 20220126221725.710167-9-bhupesh.sharma@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add ethernet support for Qualcomm SA8155p-ADP board | expand |
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 >
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 --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, };
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(-)