| Message ID | 1418826607-11817-1-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Geert, Thank you for the patch. On Wednesday 17 December 2014 15:30:07 Geert Uytterhoeven wrote: > While DIV6 clocks require the divisor field to be non-zero when stopping > the clock, some clocks (e.g. ZB on sh73a0) fail to be re-enabled later > if the divisor field is changed when stopping the clock. > The reason for this is unknown. > > To fix this, do not touch the divisor field if it's already non-zero. > > On kzm9g, the smsc911x Ethernet controller is connected to the sh73a0 > Bus State Controller, which is clocked by the ZB clock. Without this > fix, if the ZB clock is disabled during system suspend, and re-enabled > during resume, the kernel locks up when the smsc911x driver tries to > access the Ethernet registers. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> By the way, have you noticed the issue on any platform other than sh73a0 ? There might be a chance to get feedback on the hardware on a new platform, but not on such an old one. > --- > Mike, I can queue up this patch in a branch, together with other clk-related > patches for shmobile for 3.20, and ask you to pull later. > Is that OK for you? > > Thanks! > > v2: > - Add a comment to the code > - Add a note about our (lack of) understanding of the hardware. > --- > drivers/clk/shmobile/clk-div6.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/clk/shmobile/clk-div6.c > b/drivers/clk/shmobile/clk-div6.c index 639241e31e03ec24..efbaf6c81b7530b8 > 100644 > --- a/drivers/clk/shmobile/clk-div6.c > +++ b/drivers/clk/shmobile/clk-div6.c > @@ -54,12 +54,19 @@ static int cpg_div6_clock_enable(struct clk_hw *hw) > static void cpg_div6_clock_disable(struct clk_hw *hw) > { > struct div6_clock *clock = to_div6_clock(hw); > + u32 val; > > - /* DIV6 clocks require the divisor field to be non-zero when stopping > - * the clock. > + val = clk_readl(clock->reg); > + val |= CPG_DIV6_CKSTP; > + /* > + * DIV6 clocks require the divisor field to be non-zero when stopping > + * the clock. However, some clocks (e.g. ZB on sh73a0) fail to be > + * re-enabled later if the divisor field is changed when stopping the > + * clock > */ > - clk_writel(clk_readl(clock->reg) | CPG_DIV6_CKSTP | CPG_DIV6_DIV_MASK, > - clock->reg); > + if (!(val & CPG_DIV6_DIV_MASK)) > + val |= CPG_DIV6_DIV_MASK; > + clk_writel(val, clock->reg); > } > > static int cpg_div6_clock_is_enabled(struct clk_hw *hw)
Hi Laurent, On Wed, Dec 17, 2014 at 3:40 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Wednesday 17 December 2014 15:30:07 Geert Uytterhoeven wrote: >> While DIV6 clocks require the divisor field to be non-zero when stopping >> the clock, some clocks (e.g. ZB on sh73a0) fail to be re-enabled later >> if the divisor field is changed when stopping the clock. >> The reason for this is unknown. >> >> To fix this, do not touch the divisor field if it's already non-zero. >> >> On kzm9g, the smsc911x Ethernet controller is connected to the sh73a0 >> Bus State Controller, which is clocked by the ZB clock. Without this >> fix, if the ZB clock is disabled during system suspend, and re-enabled >> during resume, the kernel locks up when the smsc911x driver tries to >> access the Ethernet registers. >> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks! > By the way, have you noticed the issue on any platform other than sh73a0 ? > There might be a chance to get feedback on the hardware on a new platform, but > not on such an old one. No, it doesn't seem to happen with e.g. the SD and MMC clocks on r8a7791, which are also div6. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/clk/shmobile/clk-div6.c b/drivers/clk/shmobile/clk-div6.c index 639241e31e03ec24..efbaf6c81b7530b8 100644 --- a/drivers/clk/shmobile/clk-div6.c +++ b/drivers/clk/shmobile/clk-div6.c @@ -54,12 +54,19 @@ static int cpg_div6_clock_enable(struct clk_hw *hw) static void cpg_div6_clock_disable(struct clk_hw *hw) { struct div6_clock *clock = to_div6_clock(hw); + u32 val; - /* DIV6 clocks require the divisor field to be non-zero when stopping - * the clock. + val = clk_readl(clock->reg); + val |= CPG_DIV6_CKSTP; + /* + * DIV6 clocks require the divisor field to be non-zero when stopping + * the clock. However, some clocks (e.g. ZB on sh73a0) fail to be + * re-enabled later if the divisor field is changed when stopping the + * clock */ - clk_writel(clk_readl(clock->reg) | CPG_DIV6_CKSTP | CPG_DIV6_DIV_MASK, - clock->reg); + if (!(val & CPG_DIV6_DIV_MASK)) + val |= CPG_DIV6_DIV_MASK; + clk_writel(val, clock->reg); } static int cpg_div6_clock_is_enabled(struct clk_hw *hw)
While DIV6 clocks require the divisor field to be non-zero when stopping the clock, some clocks (e.g. ZB on sh73a0) fail to be re-enabled later if the divisor field is changed when stopping the clock. The reason for this is unknown. To fix this, do not touch the divisor field if it's already non-zero. On kzm9g, the smsc911x Ethernet controller is connected to the sh73a0 Bus State Controller, which is clocked by the ZB clock. Without this fix, if the ZB clock is disabled during system suspend, and re-enabled during resume, the kernel locks up when the smsc911x driver tries to access the Ethernet registers. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Mike, I can queue up this patch in a branch, together with other clk-related patches for shmobile for 3.20, and ask you to pull later. Is that OK for you? Thanks! v2: - Add a comment to the code - Add a note about our (lack of) understanding of the hardware. --- drivers/clk/shmobile/clk-div6.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)