From: Claudiu Beznea <[email protected]>
Hardware manual of both RZ/G2L and RZ/G3S specifies that reset monitor
registers need to be interrogated when the reset signals are toggled
(chapters "Procedures for Supplying and Stopping Reset Signals" and
"Procedure for Activating Modules"). Without this there is a chance that
different modules (e.g. Ethernet) to not be ready after reset signal is
toggled leading to failures (on probe or resume from deep sleep states).
Fixes: ef3c613ccd68 ("clk: renesas: Add CPG core wrapper for RZ/G2L SoC")
Signed-off-by: Claudiu Beznea <[email protected]>
---
Hi, Geert,
In case you apply this patch and patch 1/13 as is, please add a Depend-on
tag on this patch to point to patch 1/13 for proper backporting.
Thank you,
Claudiu Beznea
drivers/clk/renesas/rzg2l-cpg.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index 3189c3167ba8..2922dc884e35 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -1416,12 +1416,23 @@ static int rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
const struct rzg2l_cpg_info *info = priv->info;
unsigned int reg = info->resets[id].off;
- u32 value = BIT(info->resets[id].bit) << 16;
+ u32 dis = BIT(info->resets[id].bit);
+ u32 value = dis << 16;
+ int ret = 0;
dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id, CLK_RST_R(reg));
writel(value, priv->base + CLK_RST_R(reg));
- return 0;
+
+ if (info->has_clk_mon_regs) {
+ ret = readl_poll_timeout_atomic(priv->base + CLK_MRST_R(reg), value,
+ value & dis, 10, 200);
+ } else {
+ /* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
+ udelay(35);
+ }
+
+ return ret;
}
static int rzg2l_cpg_deassert(struct reset_controller_dev *rcdev,
@@ -1432,12 +1443,22 @@ static int rzg2l_cpg_deassert(struct reset_controller_dev *rcdev,
unsigned int reg = info->resets[id].off;
u32 dis = BIT(info->resets[id].bit);
u32 value = (dis << 16) | dis;
+ int ret = 0;
dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id,
CLK_RST_R(reg));
writel(value, priv->base + CLK_RST_R(reg));
- return 0;
+
+ if (info->has_clk_mon_regs) {
+ ret = readl_poll_timeout_atomic(priv->base + CLK_MRST_R(reg), value,
+ !(value & dis), 10, 200);
+ } else {
+ /* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
+ udelay(35);
+ }
+
+ return ret;
}
static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev,
@@ -1449,9 +1470,6 @@ static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev,
if (ret)
return ret;
- /* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
- udelay(35);
-
return rzg2l_cpg_deassert(rcdev, id);
}
--
2.39.2
Hi Claudiu,
On Mon, Nov 20, 2023 at 8:01 AM Claudiu <[email protected]> wrote:
> From: Claudiu Beznea <[email protected]>
>
> Hardware manual of both RZ/G2L and RZ/G3S specifies that reset monitor
> registers need to be interrogated when the reset signals are toggled
> (chapters "Procedures for Supplying and Stopping Reset Signals" and
> "Procedure for Activating Modules"). Without this there is a chance that
> different modules (e.g. Ethernet) to not be ready after reset signal is
> toggled leading to failures (on probe or resume from deep sleep states).
>
> Fixes: ef3c613ccd68 ("clk: renesas: Add CPG core wrapper for RZ/G2L SoC")
> Signed-off-by: Claudiu Beznea <[email protected]>
Thanks for your patch!
> In case you apply this patch and patch 1/13 as is, please add a Depend-on
> tag on this patch to point to patch 1/13 for proper backporting.
There is no such Depend-on tag? Anyway, this patch won't apply if 1/13
is not backported...
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -1416,12 +1416,23 @@ static int rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
> struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> const struct rzg2l_cpg_info *info = priv->info;
> unsigned int reg = info->resets[id].off;
> - u32 value = BIT(info->resets[id].bit) << 16;
> + u32 dis = BIT(info->resets[id].bit);
> + u32 value = dis << 16;
> + int ret = 0;
>
> dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id, CLK_RST_R(reg));
>
> writel(value, priv->base + CLK_RST_R(reg));
> - return 0;
> +
> + if (info->has_clk_mon_regs) {
> + ret = readl_poll_timeout_atomic(priv->base + CLK_MRST_R(reg), value,
> + value & dis, 10, 200);
> + } else {
> + /* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
> + udelay(35);
> + }
I think this should also take into account CPG_RST_MON on RZ/V2M,
cfr. rzg2l_cpg_status().
> +
> + return ret;
> }
>
> static int rzg2l_cpg_deassert(struct reset_controller_dev *rcdev,
> @@ -1432,12 +1443,22 @@ static int rzg2l_cpg_deassert(struct reset_controller_dev *rcdev,
> unsigned int reg = info->resets[id].off;
> u32 dis = BIT(info->resets[id].bit);
> u32 value = (dis << 16) | dis;
> + int ret = 0;
>
> dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id,
> CLK_RST_R(reg));
>
> writel(value, priv->base + CLK_RST_R(reg));
> - return 0;
> +
> + if (info->has_clk_mon_regs) {
> + ret = readl_poll_timeout_atomic(priv->base + CLK_MRST_R(reg), value,
> + !(value & dis), 10, 200);
> + } else {
> + /* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
> + udelay(35);
> + }
Likewise.
> +
> + return ret;
> }
>
> static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev,
The rest LGTM.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On 23.11.2023 17:53, Geert Uytterhoeven wrote:
> Hi Claudiu,
>
> On Mon, Nov 20, 2023 at 8:01 AM Claudiu <[email protected]> wrote:
>> From: Claudiu Beznea <[email protected]>
>>
>> Hardware manual of both RZ/G2L and RZ/G3S specifies that reset monitor
>> registers need to be interrogated when the reset signals are toggled
>> (chapters "Procedures for Supplying and Stopping Reset Signals" and
>> "Procedure for Activating Modules"). Without this there is a chance that
>> different modules (e.g. Ethernet) to not be ready after reset signal is
>> toggled leading to failures (on probe or resume from deep sleep states).
>>
>> Fixes: ef3c613ccd68 ("clk: renesas: Add CPG core wrapper for RZ/G2L SoC")
>> Signed-off-by: Claudiu Beznea <[email protected]>
>
> Thanks for your patch!
>
>> In case you apply this patch and patch 1/13 as is, please add a Depend-on
>> tag on this patch to point to patch 1/13 for proper backporting.
>
> There is no such Depend-on tag? Anyway, this patch won't apply if 1/13
typo again... it should have been "Depends-on" which is true, it is not
documented anywhere, but I saw it is used in some commits. Maybe I should
stop using it...
> is not backported...
>
>> --- a/drivers/clk/renesas/rzg2l-cpg.c
>> +++ b/drivers/clk/renesas/rzg2l-cpg.c
>> @@ -1416,12 +1416,23 @@ static int rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
>> struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
>> const struct rzg2l_cpg_info *info = priv->info;
>> unsigned int reg = info->resets[id].off;
>> - u32 value = BIT(info->resets[id].bit) << 16;
>> + u32 dis = BIT(info->resets[id].bit);
>> + u32 value = dis << 16;
>> + int ret = 0;
>>
>> dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id, CLK_RST_R(reg));
>>
>> writel(value, priv->base + CLK_RST_R(reg));
>> - return 0;
>> +
>> + if (info->has_clk_mon_regs) {
>> + ret = readl_poll_timeout_atomic(priv->base + CLK_MRST_R(reg), value,
>> + value & dis, 10, 200);
>> + } else {
>> + /* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
>> + udelay(35);
>> + }
>
> I think this should also take into account CPG_RST_MON on RZ/V2M,
> cfr. rzg2l_cpg_status().
Hm... ok, I'll have a look though it will be a bit difficult to test it ATM.
>
>> +
>> + return ret;
>> }
>>
>> static int rzg2l_cpg_deassert(struct reset_controller_dev *rcdev,
>> @@ -1432,12 +1443,22 @@ static int rzg2l_cpg_deassert(struct reset_controller_dev *rcdev,
>> unsigned int reg = info->resets[id].off;
>> u32 dis = BIT(info->resets[id].bit);
>> u32 value = (dis << 16) | dis;
>> + int ret = 0;
>>
>> dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id,
>> CLK_RST_R(reg));
>>
>> writel(value, priv->base + CLK_RST_R(reg));
>> - return 0;
>> +
>> + if (info->has_clk_mon_regs) {
>> + ret = readl_poll_timeout_atomic(priv->base + CLK_MRST_R(reg), value,
>> + !(value & dis), 10, 200);
>> + } else {
>> + /* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
>> + udelay(35);
>> + }
>
> Likewise.
>
>> +
>> + return ret;
>> }
>>
>> static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev,
>
> The rest LGTM.
>
> Gr{oetje,eeting}s,
>
> Geert
>