| Message ID | 20220531071657.104121-1-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
| Series | clk: renesas: rzg2l: Fix reset status function | expand |
Hi Biju, On Tue, May 31, 2022 at 9:17 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > As per RZ/G2L HW(Rev.1.10) manual, reset monitor register value 0 means > reset signal is not applied (deassert state) and 1 means reset signal > is applied (assert state). > > reset_control_status() expects a positive value if the reset line is > asserted. But rzg2l_cpg_status function returns zero for asserted > state. > > This patch fixes the issue by adding double inverted logic, so that > reset_control_status returns a positive value if the reset line is > asserted. > > Fixes: ef3c613ccd68 ("clk: renesas: Add CPG core wrapper for RZ/G2L SoC") > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> This bug has been present since v5.14, and went unnoticed so far. Is it OK for you to queue this in renesas-clk-for-v5.20, or do you see a reason to fast-track this fix to v5.19? Thanks! 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
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH] clk: renesas: rzg2l: Fix reset status function > > Hi Biju, > > On Tue, May 31, 2022 at 9:17 AM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > As per RZ/G2L HW(Rev.1.10) manual, reset monitor register value 0 > > means reset signal is not applied (deassert state) and 1 means reset > > signal is applied (assert state). > > > > reset_control_status() expects a positive value if the reset line is > > asserted. But rzg2l_cpg_status function returns zero for asserted > > state. > > > > This patch fixes the issue by adding double inverted logic, so that > > reset_control_status returns a positive value if the reset line is > > asserted. > > > > Fixes: ef3c613ccd68 ("clk: renesas: Add CPG core wrapper for RZ/G2L > > SoC") > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > This bug has been present since v5.14, and went unnoticed so far. > Is it OK for you to queue this in renesas-clk-for-v5.20, or do you see a > reason to fast-track this fix to v5.19? renesas-clk-for-v5.20 should be fine. This issue found while adding reset support for VSPD. There is 1 user prior to this[1] [1] https://elixir.bootlin.com/linux/latest/source/drivers/mmc/host/renesas_sdhi_core.c#L576 Cheers, Biju
Hi Biju, On Tue, May 31, 2022 at 9:58 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Subject: Re: [PATCH] clk: renesas: rzg2l: Fix reset status function > > On Tue, May 31, 2022 at 9:17 AM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > As per RZ/G2L HW(Rev.1.10) manual, reset monitor register value 0 > > > means reset signal is not applied (deassert state) and 1 means reset > > > signal is applied (assert state). > > > > > > reset_control_status() expects a positive value if the reset line is > > > asserted. But rzg2l_cpg_status function returns zero for asserted > > > state. > > > > > > This patch fixes the issue by adding double inverted logic, so that > > > reset_control_status returns a positive value if the reset line is > > > asserted. > > > > > > Fixes: ef3c613ccd68 ("clk: renesas: Add CPG core wrapper for RZ/G2L > > > SoC") > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > This bug has been present since v5.14, and went unnoticed so far. > > Is it OK for you to queue this in renesas-clk-for-v5.20, or do you see a > > reason to fast-track this fix to v5.19? > > renesas-clk-for-v5.20 should be fine. This issue found while adding reset support OK. > for VSPD. There is 1 user prior to this[1] > > [1] https://elixir.bootlin.com/linux/latest/source/drivers/mmc/host/renesas_sdhi_core.c#L576 Oh, and that one works by accident, as it ignores the return value of read_poll_timeout()... 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
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH] clk: renesas: rzg2l: Fix reset status function > > Hi Biju, > > On Tue, May 31, 2022 at 9:58 AM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > > Subject: Re: [PATCH] clk: renesas: rzg2l: Fix reset status function > > > On Tue, May 31, 2022 at 9:17 AM Biju Das > > > <biju.das.jz@bp.renesas.com> > > > wrote: > > > > As per RZ/G2L HW(Rev.1.10) manual, reset monitor register value 0 > > > > means reset signal is not applied (deassert state) and 1 means > > > > reset signal is applied (assert state). > > > > > > > > reset_control_status() expects a positive value if the reset line > > > > is asserted. But rzg2l_cpg_status function returns zero for > > > > asserted state. > > > > > > > > This patch fixes the issue by adding double inverted logic, so > > > > that reset_control_status returns a positive value if the reset > > > > line is asserted. > > > > > > > > Fixes: ef3c613ccd68 ("clk: renesas: Add CPG core wrapper for > > > > RZ/G2L > > > > SoC") > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > > This bug has been present since v5.14, and went unnoticed so far. > > > Is it OK for you to queue this in renesas-clk-for-v5.20, or do you > > > see a reason to fast-track this fix to v5.19? > > > > renesas-clk-for-v5.20 should be fine. This issue found while adding > > reset support > > OK. > > > for VSPD. There is 1 user prior to this[1] > > > Oh, and that one works by accident, as it ignores the return value of > read_poll_timeout()... Yes, I agree. Cheers, Biju
diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c index e2999ab2b53c..3ff6ecd61756 100644 --- a/drivers/clk/renesas/rzg2l-cpg.c +++ b/drivers/clk/renesas/rzg2l-cpg.c @@ -1180,7 +1180,7 @@ static int rzg2l_cpg_status(struct reset_controller_dev *rcdev, s8 monbit = info->resets[id].monbit; if (info->has_clk_mon_regs) { - return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask); + return !!(readl(priv->base + CLK_MRST_R(reg)) & bitmask); } else if (monbit >= 0) { u32 monbitmask = BIT(monbit);
As per RZ/G2L HW(Rev.1.10) manual, reset monitor register value 0 means reset signal is not applied (deassert state) and 1 means reset signal is applied (assert state). reset_control_status() expects a positive value if the reset line is asserted. But rzg2l_cpg_status function returns zero for asserted state. This patch fixes the issue by adding double inverted logic, so that reset_control_status returns a positive value if the reset line is asserted. Fixes: ef3c613ccd68 ("clk: renesas: Add CPG core wrapper for RZ/G2L SoC") Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- drivers/clk/renesas/rzg2l-cpg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)