| Message ID | ebc41558-2a0a-bf6d-05df-077e3c769344@cogentembedded.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | clk: renesas: r8a77980-cpg-mssr: fix RPC-IF module clock's parent | expand |
Hi Sergei, Thanks for your patch! On Thu, Mar 7, 2019 at 8:53 PM Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > Testing has shown that the RPC-IF module clock's parent is the RPCD2 clock, > not the RPC one -- the RPC-IF register reads stall otherwise... Perhaps... Or something else is wrong with how the RPC driver uses the clock hierarchy. According to the docs, RPC clocks RPC-PHY, and RPCD2 clocks RPC-LINK. Currently nothing references RPCD2, so it is disabled automatically. If you make RPC -> RPCD2 -> RPC-IF, enabling RPC-IF indeed enables both RPC and RPCD2. Perhaps the RPC device node does need a reference to RPCD2? Is this also the case on R-Car V3M, where RPCD2 is not controlled by the CPG, but by the DIVREG register in the RPC-IF module itself? See also section 62.4.7 (Frequency change), which does not have a subsection for V3H, but it may be impacted (changing RPCD2 causes an additional read of RPCCKR, satisfying the read-after-write requirement documented there). > Fixes: 94e3935b5756 ("clk: renesas: r8a77980: Add RPC clocks") > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > --- renesas-drivers.orig/drivers/clk/renesas/r8a77980-cpg-mssr.c > +++ renesas-drivers/drivers/clk/renesas/r8a77980-cpg-mssr.c > @@ -171,7 +171,7 @@ static const struct mssr_mod_clk r8a7798 > DEF_MOD("gpio1", 911, R8A77980_CLK_CP), > DEF_MOD("gpio0", 912, R8A77980_CLK_CP), > DEF_MOD("can-fd", 914, R8A77980_CLK_S3D2), > - DEF_MOD("rpc-if", 917, R8A77980_CLK_RPC), > + DEF_MOD("rpc-if", 917, R8A77980_CLK_RPCD2), > DEF_MOD("i2c4", 927, R8A77980_CLK_S0D6), > DEF_MOD("i2c3", 928, R8A77980_CLK_S0D6), > DEF_MOD("i2c2", 929, R8A77980_CLK_S3D2), Gr{oetje,eeting}s, Geert
On 03/11/2019 12:30 PM, Geert Uytterhoeven wrote: >> Testing has shown that the RPC-IF module clock's parent is the RPCD2 clock, >> not the RPC one -- the RPC-IF register reads stall otherwise... > > Perhaps... Or something else is wrong with how the RPC driver uses the > clock hierarchy. Yes, explicitly enabling RPCD2 did help as well. > According to the docs, RPC clocks RPC-PHY, and RPCD2 clocks RPC-LINK. I've also read the manual. Note it's not that reading a PHY register hangs the kernel but reading CMNCR... > Currently nothing references RPCD2, so it is disabled automatically. Only on V3H. > If you make RPC -> RPCD2 -> RPC-IF, enabling RPC-IF indeed enables > both RPC and RPCD2. Sure, it does. :-) > Perhaps the RPC device node does need a reference to RPCD2? Looks like that wouldn't hurt either... > Is this also the case on R-Car V3M, where RPCD2 is not controlled by the > CPG, but by the DIVREG register in the RPC-IF module itself? No hangs were seen on V3M, despite the RPCD2 clock remained disabled. The RPC clocks on both V3H and V3M suffer from a lack of clear documentation... > See also section 62.4.7 (Frequency change), which does not have a > subsection for V3H, but it may be impacted (changing RPCD2 causes > an additional read of RPCCKR, satisfying the read-after-write requirement > documented there). Hmm, haven't seen this item before; it looks like we can't use the standard divider component. BTW, this section in the 1.50 manual tells to use the soft reset on V3MOK. OK, I'll investigate this... >> Fixes: 94e3935b5756 ("clk: renesas: r8a77980: Add RPC clocks") >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> [...] MBR, Sergei
On 03/11/2019 08:14 PM, Sergei Shtylyov wrote: [...] > No hangs were seen on V3M, despite the RPCD2 clock remained disabled. > The RPC clocks on both V3H and V3M suffer from a lack of clear documentation... > >> See also section 62.4.7 (Frequency change), which does not have a >> subsection for V3H, but it may be impacted (changing RPCD2 causes >> an additional read of RPCCKR, satisfying the read-after-write requirement >> documented there). > > Hmm, haven't seen this item before; it looks like we can't use the standard > divider component. BTW, this section in the 1.50 manual tells to use the soft > reset on V3MOK. OK, I'll investigate this... Now I have: the frequency change didn't seem to happen, and without RPCD2 enabled the CMNCR read still hangs, even when I added readl() call into clk_writel(). So I think my patch was correct. >>> Fixes: 94e3935b5756 ("clk: renesas: r8a77980: Add RPC clocks") >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > [...] MBR, Sergei
Index: renesas-drivers/drivers/clk/renesas/r8a77980-cpg-mssr.c =================================================================== --- renesas-drivers.orig/drivers/clk/renesas/r8a77980-cpg-mssr.c +++ renesas-drivers/drivers/clk/renesas/r8a77980-cpg-mssr.c @@ -171,7 +171,7 @@ static const struct mssr_mod_clk r8a7798 DEF_MOD("gpio1", 911, R8A77980_CLK_CP), DEF_MOD("gpio0", 912, R8A77980_CLK_CP), DEF_MOD("can-fd", 914, R8A77980_CLK_S3D2), - DEF_MOD("rpc-if", 917, R8A77980_CLK_RPC), + DEF_MOD("rpc-if", 917, R8A77980_CLK_RPCD2), DEF_MOD("i2c4", 927, R8A77980_CLK_S0D6), DEF_MOD("i2c3", 928, R8A77980_CLK_S0D6), DEF_MOD("i2c2", 929, R8A77980_CLK_S3D2),
Testing has shown that the RPC-IF module clock's parent is the RPCD2 clock, not the RPC one -- the RPC-IF register reads stall otherwise... Fixes: 94e3935b5756 ("clk: renesas: r8a77980: Add RPC clocks") Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- This patch is against the 'clk-renesas' branch of Geert Uytterhoeven's 'renesas-drivers.git' repo. drivers/clk/renesas/r8a77980-cpg-mssr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)