* [PATCH 00/14] renesas: rzg3s: Add support for Ethernet
@ 2023-11-20 7:00 Claudiu
2023-11-20 7:00 ` [PATCH 01/14] clk: renesas: rzg2l-cpg: Reuse code in rzg2l_cpg_reset() Claudiu
` (14 more replies)
0 siblings, 15 replies; 53+ messages in thread
From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw)
To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas,
magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd,
m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein,
eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj,
biju.das.jz
Cc: linux-renesas-soc, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea,
Claudiu Beznea
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Hi,
Series adds Ethernet support for Renesas RZ/G3S Ethernet.
Along with it preparatory cleanups and fixes were included.
Patches 1-4 are clock specific.
Patches 5-8 are pinctrl specific.
Patches 9-13 are device tree specific.
Patch 14 updates multi_v7_defconfig with RAVB flag.
Thank you,
Claudiu Beznea
Claudiu Beznea (14):
clk: renesas: rzg2l-cpg: Reuse code in rzg2l_cpg_reset()
clk: renesas: rzg2l-cpg: Check reset monitor registers
clk: renesas: rzg2l-cpg: Add support for MSTOP
clk: renesas: r9a08g045-cpg: Add clock and reset support for ETH0 and
ETH1
pinctrl: renesas: rzg2l: Move arg in the main function block
pinctrl: renesas: rzg2l: Add pin configuration support for pinmux
groups
pinctrl: renesas: rzg2l: Add support to select power source for
Ethernet pins
pinctrl: renesas: rzg2l: add output enable support
dt-bindings: net: renesas,etheravb: Document RZ/G3S support
arm64: renesas: r9a08g045: Add Ethernet nodes
arm64: renesas: rzg3s-smarc-som: Invert the logic for SW_SD2_EN macro
arm64: dts: renesas: Improve documentation for SW_SD0_DEV_SEL
arm64: dts: renesas: rzg3s-smarc-som: Enable Ethernet interfaces
arm: multi_v7_defconfig: Enable CONFIG_RAVB
.../bindings/net/renesas,etheravb.yaml | 1 +
arch/arm/configs/multi_v7_defconfig | 1 +
arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 32 ++++
.../boot/dts/renesas/rzg3s-smarc-som.dtsi | 153 +++++++++++++++-
drivers/clk/renesas/r9a07g043-cpg.c | 116 ++++++------
drivers/clk/renesas/r9a07g044-cpg.c | 158 ++++++++---------
drivers/clk/renesas/r9a08g045-cpg.c | 64 +++++--
drivers/clk/renesas/r9a09g011-cpg.c | 116 ++++++------
drivers/clk/renesas/rzg2l-cpg.c | 166 +++++++++++++++---
drivers/clk/renesas/rzg2l-cpg.h | 21 ++-
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 166 ++++++++++++++++--
11 files changed, 736 insertions(+), 258 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 53+ messages in thread* [PATCH 01/14] clk: renesas: rzg2l-cpg: Reuse code in rzg2l_cpg_reset() 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-11-23 15:48 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 02/14] clk: renesas: rzg2l-cpg: Check reset monitor registers Claudiu ` (13 subsequent siblings) 14 siblings, 1 reply; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Code in rzg2l_cpg_reset() is equivalent with the combined code of rzg2l_cpg_assert() and rzg2l_cpg_deassert(). There is no need to have different versions thus re-use rzg2l_cpg_assert() and rzg2l_cpg_deassert(). Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- drivers/clk/renesas/rzg2l-cpg.c | 38 +++++++++++++-------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c index 764bd72cf059..3189c3167ba8 100644 --- a/drivers/clk/renesas/rzg2l-cpg.c +++ b/drivers/clk/renesas/rzg2l-cpg.c @@ -1410,29 +1410,6 @@ rzg2l_cpg_register_mod_clk(const struct rzg2l_mod_clk *mod, #define rcdev_to_priv(x) container_of(x, struct rzg2l_cpg_priv, rcdev) -static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev, - unsigned long id) -{ - 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 dis = BIT(info->resets[id].bit); - u32 we = dis << 16; - - dev_dbg(rcdev->dev, "reset id:%ld offset:0x%x\n", id, CLK_RST_R(reg)); - - /* Reset module */ - writel(we, priv->base + CLK_RST_R(reg)); - - /* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */ - udelay(35); - - /* Release module from reset state */ - writel(we | dis, priv->base + CLK_RST_R(reg)); - - return 0; -} - static int rzg2l_cpg_assert(struct reset_controller_dev *rcdev, unsigned long id) { @@ -1463,6 +1440,21 @@ static int rzg2l_cpg_deassert(struct reset_controller_dev *rcdev, return 0; } +static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev, + unsigned long id) +{ + int ret; + + ret = rzg2l_cpg_assert(rcdev, id); + 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); +} + static int rzg2l_cpg_status(struct reset_controller_dev *rcdev, unsigned long id) { -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 01/14] clk: renesas: rzg2l-cpg: Reuse code in rzg2l_cpg_reset() 2023-11-20 7:00 ` [PATCH 01/14] clk: renesas: rzg2l-cpg: Reuse code in rzg2l_cpg_reset() Claudiu @ 2023-11-23 15:48 ` Geert Uytterhoeven 0 siblings, 0 replies; 53+ messages in thread From: Geert Uytterhoeven @ 2023-11-23 15:48 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Code in rzg2l_cpg_reset() is equivalent with the combined code of > rzg2l_cpg_assert() and rzg2l_cpg_deassert(). There is no need to have > different versions thus re-use rzg2l_cpg_assert() and rzg2l_cpg_deassert(). > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> i.e. will queue in renesas-clk-for-v6.8. 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 02/14] clk: renesas: rzg2l-cpg: Check reset monitor registers 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu 2023-11-20 7:00 ` [PATCH 01/14] clk: renesas: rzg2l-cpg: Reuse code in rzg2l_cpg_reset() Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-11-23 15:53 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 03/14] clk: renesas: rzg2l-cpg: Add support for MSTOP Claudiu ` (12 subsequent siblings) 14 siblings, 1 reply; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> 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 <claudiu.beznea.uj@bp.renesas.com> --- 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 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 02/14] clk: renesas: rzg2l-cpg: Check reset monitor registers 2023-11-20 7:00 ` [PATCH 02/14] clk: renesas: rzg2l-cpg: Check reset monitor registers Claudiu @ 2023-11-23 15:53 ` Geert Uytterhoeven 2023-11-23 17:19 ` claudiu beznea 0 siblings, 1 reply; 53+ messages in thread From: Geert Uytterhoeven @ 2023-11-23 15:53 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > 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 <claudiu.beznea.uj@bp.renesas.com> 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 -- 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 02/14] clk: renesas: rzg2l-cpg: Check reset monitor registers 2023-11-23 15:53 ` Geert Uytterhoeven @ 2023-11-23 17:19 ` claudiu beznea 0 siblings, 0 replies; 53+ messages in thread From: claudiu beznea @ 2023-11-23 17:19 UTC (permalink / raw) To: Geert Uytterhoeven Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea On 23.11.2023 17:53, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> 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 <claudiu.beznea.uj@bp.renesas.com> > > 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 > ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 03/14] clk: renesas: rzg2l-cpg: Add support for MSTOP 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu 2023-11-20 7:00 ` [PATCH 01/14] clk: renesas: rzg2l-cpg: Reuse code in rzg2l_cpg_reset() Claudiu 2023-11-20 7:00 ` [PATCH 02/14] clk: renesas: rzg2l-cpg: Check reset monitor registers Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-11-23 16:35 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 04/14] clk: renesas: r9a08g045-cpg: Add clock and reset support for ETH0 and ETH1 Claudiu ` (11 subsequent siblings) 14 siblings, 1 reply; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> RZ/{G2L, V2L, G3S} based CPG versions have support for saving extra power when clocks are disabled by activating module standby. This is done though MSTOP specific registers that are part of CPG. Each individual module have one or more bits associated in one MSTOP register (see table "Registers for Module Standby Mode" from HW manuals). Hardware manual associates modules' clocks to one or more MSTOP bits. There are 3 mappings available (identified by researching RZ/G2L, RZ/G3S, RZ/V2L HW manuals): case 1: N clocks mapped to N MSTOP bits (with N={0, ..., X}) case 2: N clocks mapped to 1 MSTOP bit (with N={0, ..., X}) case 3: N clocks mapped to M MSTOP bits (with N={0, ..., X}, M={0, ..., Y}) Case 3 has been currently identified on RZ/V2L for VCPL4 module. To cover all 3 cases the individual platform drivers will provide to clock driver MSTOP register offset and associated bits in this register as a bitmask and the clock driver will apply this bitmask to proper MSTOP register. As most of the modules have more than one clock and these clocks are mapped to 1 MSTOP bitmap that need to be applied to MSTOP registers, to avoid switching the module to/out of standby when the module has enabled/disabled clocks a counter has been associated to each module (though struct mstop::count) which is incremented/decremented every time a module's clock is enabled/disabled and the settings to MSTOP register are applied only when the counter reaches zero (counter zero means either 1st clock of the module is going to be enabled or all clocks of the module are going to be disabled). The MSTOP functionality has been instantiated at the moment for RZ/G3S. Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- drivers/clk/renesas/r9a07g043-cpg.c | 116 ++++++++++---------- drivers/clk/renesas/r9a07g044-cpg.c | 158 ++++++++++++++-------------- drivers/clk/renesas/r9a08g045-cpg.c | 50 ++++++--- drivers/clk/renesas/r9a09g011-cpg.c | 116 ++++++++++---------- drivers/clk/renesas/rzg2l-cpg.c | 104 ++++++++++++++++++ drivers/clk/renesas/rzg2l-cpg.h | 21 +++- 6 files changed, 348 insertions(+), 217 deletions(-) diff --git a/drivers/clk/renesas/r9a07g043-cpg.c b/drivers/clk/renesas/r9a07g043-cpg.c index b70bb378ab46..7a40bc445054 100644 --- a/drivers/clk/renesas/r9a07g043-cpg.c +++ b/drivers/clk/renesas/r9a07g043-cpg.c @@ -147,124 +147,124 @@ static const struct cpg_core_clk r9a07g043_core_clks[] __initconst = { static struct rzg2l_mod_clk r9a07g043_mod_clks[] = { #ifdef CONFIG_ARM64 DEF_MOD("gic", R9A07G043_GIC600_GICCLK, R9A07G043_CLK_P1, - 0x514, 0), + 0x514, 0, 0), DEF_MOD("ia55_pclk", R9A07G043_IA55_PCLK, R9A07G043_CLK_P2, - 0x518, 0), + 0x518, 0, 0), DEF_MOD("ia55_clk", R9A07G043_IA55_CLK, R9A07G043_CLK_P1, - 0x518, 1), + 0x518, 1, 0), #endif #ifdef CONFIG_RISCV DEF_MOD("iax45_pclk", R9A07G043_IAX45_PCLK, R9A07G043_CLK_P2, - 0x518, 0), + 0x518, 0, 0), DEF_MOD("iax45_clk", R9A07G043_IAX45_CLK, R9A07G043_CLK_P1, - 0x518, 1), + 0x518, 1, 0), #endif DEF_MOD("dmac_aclk", R9A07G043_DMAC_ACLK, R9A07G043_CLK_P1, - 0x52c, 0), + 0x52c, 0, 0), DEF_MOD("dmac_pclk", R9A07G043_DMAC_PCLK, CLK_P1_DIV2, - 0x52c, 1), + 0x52c, 1, 0), DEF_MOD("ostm0_pclk", R9A07G043_OSTM0_PCLK, R9A07G043_CLK_P0, - 0x534, 0), + 0x534, 0, 0), DEF_MOD("ostm1_pclk", R9A07G043_OSTM1_PCLK, R9A07G043_CLK_P0, - 0x534, 1), + 0x534, 1, 0), DEF_MOD("ostm2_pclk", R9A07G043_OSTM2_PCLK, R9A07G043_CLK_P0, - 0x534, 2), + 0x534, 2, 0), DEF_MOD("mtu_x_mck", R9A07G043_MTU_X_MCK_MTU3, R9A07G043_CLK_P0, - 0x538, 0), + 0x538, 0, 0), DEF_MOD("wdt0_pclk", R9A07G043_WDT0_PCLK, R9A07G043_CLK_P0, - 0x548, 0), + 0x548, 0, 0), DEF_MOD("wdt0_clk", R9A07G043_WDT0_CLK, R9A07G043_OSCCLK, - 0x548, 1), + 0x548, 1, 0), DEF_MOD("spi_clk2", R9A07G043_SPI_CLK2, R9A07G043_CLK_SPI1, - 0x550, 0), + 0x550, 0, 0), DEF_MOD("spi_clk", R9A07G043_SPI_CLK, R9A07G043_CLK_SPI0, - 0x550, 1), + 0x550, 1, 0), DEF_MOD("sdhi0_imclk", R9A07G043_SDHI0_IMCLK, CLK_SD0_DIV4, - 0x554, 0), + 0x554, 0, 0), DEF_MOD("sdhi0_imclk2", R9A07G043_SDHI0_IMCLK2, CLK_SD0_DIV4, - 0x554, 1), + 0x554, 1, 0), DEF_MOD("sdhi0_clk_hs", R9A07G043_SDHI0_CLK_HS, R9A07G043_CLK_SD0, - 0x554, 2), + 0x554, 2, 0), DEF_MOD("sdhi0_aclk", R9A07G043_SDHI0_ACLK, R9A07G043_CLK_P1, - 0x554, 3), + 0x554, 3, 0), DEF_MOD("sdhi1_imclk", R9A07G043_SDHI1_IMCLK, CLK_SD1_DIV4, - 0x554, 4), + 0x554, 4, 0), DEF_MOD("sdhi1_imclk2", R9A07G043_SDHI1_IMCLK2, CLK_SD1_DIV4, - 0x554, 5), + 0x554, 5, 0), DEF_MOD("sdhi1_clk_hs", R9A07G043_SDHI1_CLK_HS, R9A07G043_CLK_SD1, - 0x554, 6), + 0x554, 6, 0), DEF_MOD("sdhi1_aclk", R9A07G043_SDHI1_ACLK, R9A07G043_CLK_P1, - 0x554, 7), + 0x554, 7, 0), DEF_MOD("ssi0_pclk", R9A07G043_SSI0_PCLK2, R9A07G043_CLK_P0, - 0x570, 0), + 0x570, 0, 0), DEF_MOD("ssi0_sfr", R9A07G043_SSI0_PCLK_SFR, R9A07G043_CLK_P0, - 0x570, 1), + 0x570, 1, 0), DEF_MOD("ssi1_pclk", R9A07G043_SSI1_PCLK2, R9A07G043_CLK_P0, - 0x570, 2), + 0x570, 2, 0), DEF_MOD("ssi1_sfr", R9A07G043_SSI1_PCLK_SFR, R9A07G043_CLK_P0, - 0x570, 3), + 0x570, 3, 0), DEF_MOD("ssi2_pclk", R9A07G043_SSI2_PCLK2, R9A07G043_CLK_P0, - 0x570, 4), + 0x570, 4, 0), DEF_MOD("ssi2_sfr", R9A07G043_SSI2_PCLK_SFR, R9A07G043_CLK_P0, - 0x570, 5), + 0x570, 5, 0), DEF_MOD("ssi3_pclk", R9A07G043_SSI3_PCLK2, R9A07G043_CLK_P0, - 0x570, 6), + 0x570, 6, 0), DEF_MOD("ssi3_sfr", R9A07G043_SSI3_PCLK_SFR, R9A07G043_CLK_P0, - 0x570, 7), + 0x570, 7, 0), DEF_MOD("usb0_host", R9A07G043_USB_U2H0_HCLK, R9A07G043_CLK_P1, - 0x578, 0), + 0x578, 0, 0), DEF_MOD("usb1_host", R9A07G043_USB_U2H1_HCLK, R9A07G043_CLK_P1, - 0x578, 1), + 0x578, 1, 0), DEF_MOD("usb0_func", R9A07G043_USB_U2P_EXR_CPUCLK, R9A07G043_CLK_P1, - 0x578, 2), + 0x578, 2, 0), DEF_MOD("usb_pclk", R9A07G043_USB_PCLK, R9A07G043_CLK_P1, - 0x578, 3), + 0x578, 3, 0), DEF_COUPLED("eth0_axi", R9A07G043_ETH0_CLK_AXI, R9A07G043_CLK_M0, - 0x57c, 0), + 0x57c, 0, 0), DEF_COUPLED("eth0_chi", R9A07G043_ETH0_CLK_CHI, R9A07G043_CLK_ZT, - 0x57c, 0), + 0x57c, 0, 0), DEF_COUPLED("eth1_axi", R9A07G043_ETH1_CLK_AXI, R9A07G043_CLK_M0, - 0x57c, 1), + 0x57c, 1, 0), DEF_COUPLED("eth1_chi", R9A07G043_ETH1_CLK_CHI, R9A07G043_CLK_ZT, - 0x57c, 1), + 0x57c, 1, 0), DEF_MOD("i2c0", R9A07G043_I2C0_PCLK, R9A07G043_CLK_P0, - 0x580, 0), + 0x580, 0, 0), DEF_MOD("i2c1", R9A07G043_I2C1_PCLK, R9A07G043_CLK_P0, - 0x580, 1), + 0x580, 1, 0), DEF_MOD("i2c2", R9A07G043_I2C2_PCLK, R9A07G043_CLK_P0, - 0x580, 2), + 0x580, 2, 0), DEF_MOD("i2c3", R9A07G043_I2C3_PCLK, R9A07G043_CLK_P0, - 0x580, 3), + 0x580, 3, 0), DEF_MOD("scif0", R9A07G043_SCIF0_CLK_PCK, R9A07G043_CLK_P0, - 0x584, 0), + 0x584, 0, 0), DEF_MOD("scif1", R9A07G043_SCIF1_CLK_PCK, R9A07G043_CLK_P0, - 0x584, 1), + 0x584, 1, 0), DEF_MOD("scif2", R9A07G043_SCIF2_CLK_PCK, R9A07G043_CLK_P0, - 0x584, 2), + 0x584, 2, 0), DEF_MOD("scif3", R9A07G043_SCIF3_CLK_PCK, R9A07G043_CLK_P0, - 0x584, 3), + 0x584, 3, 0), DEF_MOD("scif4", R9A07G043_SCIF4_CLK_PCK, R9A07G043_CLK_P0, - 0x584, 4), + 0x584, 4, 0), DEF_MOD("sci0", R9A07G043_SCI0_CLKP, R9A07G043_CLK_P0, - 0x588, 0), + 0x588, 0, 0), DEF_MOD("sci1", R9A07G043_SCI1_CLKP, R9A07G043_CLK_P0, - 0x588, 1), + 0x588, 1, 0), DEF_MOD("rspi0", R9A07G043_RSPI0_CLKB, R9A07G043_CLK_P0, - 0x590, 0), + 0x590, 0, 0), DEF_MOD("rspi1", R9A07G043_RSPI1_CLKB, R9A07G043_CLK_P0, - 0x590, 1), + 0x590, 1, 0), DEF_MOD("rspi2", R9A07G043_RSPI2_CLKB, R9A07G043_CLK_P0, - 0x590, 2), + 0x590, 2, 0), DEF_MOD("canfd", R9A07G043_CANFD_PCLK, R9A07G043_CLK_P0, - 0x594, 0), + 0x594, 0, 0), DEF_MOD("gpio", R9A07G043_GPIO_HCLK, R9A07G043_OSCCLK, - 0x598, 0), + 0x598, 0, 0), DEF_MOD("adc_adclk", R9A07G043_ADC_ADCLK, R9A07G043_CLK_TSU, - 0x5a8, 0), + 0x5a8, 0, 0), DEF_MOD("adc_pclk", R9A07G043_ADC_PCLK, R9A07G043_CLK_P0, - 0x5a8, 1), + 0x5a8, 1, 0), DEF_MOD("tsu_pclk", R9A07G043_TSU_PCLK, R9A07G043_CLK_TSU, - 0x5ac, 0), + 0x5ac, 0, 0), }; static struct rzg2l_reset r9a07g043_resets[] = { diff --git a/drivers/clk/renesas/r9a07g044-cpg.c b/drivers/clk/renesas/r9a07g044-cpg.c index 1047278c9079..ed0c17341803 100644 --- a/drivers/clk/renesas/r9a07g044-cpg.c +++ b/drivers/clk/renesas/r9a07g044-cpg.c @@ -204,163 +204,163 @@ static const struct { } mod_clks = { .common = { DEF_MOD("gic", R9A07G044_GIC600_GICCLK, R9A07G044_CLK_P1, - 0x514, 0), + 0x514, 0, 0), DEF_MOD("ia55_pclk", R9A07G044_IA55_PCLK, R9A07G044_CLK_P2, - 0x518, 0), + 0x518, 0, 0), DEF_MOD("ia55_clk", R9A07G044_IA55_CLK, R9A07G044_CLK_P1, - 0x518, 1), + 0x518, 1, 0), DEF_MOD("dmac_aclk", R9A07G044_DMAC_ACLK, R9A07G044_CLK_P1, - 0x52c, 0), + 0x52c, 0, 0), DEF_MOD("dmac_pclk", R9A07G044_DMAC_PCLK, CLK_P1_DIV2, - 0x52c, 1), + 0x52c, 1, 0), DEF_MOD("ostm0_pclk", R9A07G044_OSTM0_PCLK, R9A07G044_CLK_P0, - 0x534, 0), + 0x534, 0, 0), DEF_MOD("ostm1_pclk", R9A07G044_OSTM1_PCLK, R9A07G044_CLK_P0, - 0x534, 1), + 0x534, 1, 0), DEF_MOD("ostm2_pclk", R9A07G044_OSTM2_PCLK, R9A07G044_CLK_P0, - 0x534, 2), + 0x534, 2, 0), DEF_MOD("mtu_x_mck", R9A07G044_MTU_X_MCK_MTU3, R9A07G044_CLK_P0, - 0x538, 0), + 0x538, 0, 0), DEF_MOD("gpt_pclk", R9A07G044_GPT_PCLK, R9A07G044_CLK_P0, - 0x540, 0), + 0x540, 0, 0), DEF_MOD("poeg_a_clkp", R9A07G044_POEG_A_CLKP, R9A07G044_CLK_P0, - 0x544, 0), + 0x544, 0, 0), DEF_MOD("poeg_b_clkp", R9A07G044_POEG_B_CLKP, R9A07G044_CLK_P0, - 0x544, 1), + 0x544, 1, 0), DEF_MOD("poeg_c_clkp", R9A07G044_POEG_C_CLKP, R9A07G044_CLK_P0, - 0x544, 2), + 0x544, 2, 0), DEF_MOD("poeg_d_clkp", R9A07G044_POEG_D_CLKP, R9A07G044_CLK_P0, - 0x544, 3), + 0x544, 3, 0), DEF_MOD("wdt0_pclk", R9A07G044_WDT0_PCLK, R9A07G044_CLK_P0, - 0x548, 0), + 0x548, 0, 0), DEF_MOD("wdt0_clk", R9A07G044_WDT0_CLK, R9A07G044_OSCCLK, - 0x548, 1), + 0x548, 1, 0), DEF_MOD("wdt1_pclk", R9A07G044_WDT1_PCLK, R9A07G044_CLK_P0, - 0x548, 2), + 0x548, 2, 0), DEF_MOD("wdt1_clk", R9A07G044_WDT1_CLK, R9A07G044_OSCCLK, - 0x548, 3), + 0x548, 3, 0), DEF_MOD("spi_clk2", R9A07G044_SPI_CLK2, R9A07G044_CLK_SPI1, - 0x550, 0), + 0x550, 0, 0), DEF_MOD("spi_clk", R9A07G044_SPI_CLK, R9A07G044_CLK_SPI0, - 0x550, 1), + 0x550, 1, 0), DEF_MOD("sdhi0_imclk", R9A07G044_SDHI0_IMCLK, CLK_SD0_DIV4, - 0x554, 0), + 0x554, 0, 0), DEF_MOD("sdhi0_imclk2", R9A07G044_SDHI0_IMCLK2, CLK_SD0_DIV4, - 0x554, 1), + 0x554, 1, 0), DEF_MOD("sdhi0_clk_hs", R9A07G044_SDHI0_CLK_HS, R9A07G044_CLK_SD0, - 0x554, 2), + 0x554, 2, 0), DEF_MOD("sdhi0_aclk", R9A07G044_SDHI0_ACLK, R9A07G044_CLK_P1, - 0x554, 3), + 0x554, 3, 0), DEF_MOD("sdhi1_imclk", R9A07G044_SDHI1_IMCLK, CLK_SD1_DIV4, - 0x554, 4), + 0x554, 4, 0), DEF_MOD("sdhi1_imclk2", R9A07G044_SDHI1_IMCLK2, CLK_SD1_DIV4, - 0x554, 5), + 0x554, 5, 0), DEF_MOD("sdhi1_clk_hs", R9A07G044_SDHI1_CLK_HS, R9A07G044_CLK_SD1, - 0x554, 6), + 0x554, 6, 0), DEF_MOD("sdhi1_aclk", R9A07G044_SDHI1_ACLK, R9A07G044_CLK_P1, - 0x554, 7), + 0x554, 7, 0), DEF_MOD("gpu_clk", R9A07G044_GPU_CLK, R9A07G044_CLK_G, - 0x558, 0), + 0x558, 0, 0), DEF_MOD("gpu_axi_clk", R9A07G044_GPU_AXI_CLK, R9A07G044_CLK_P1, - 0x558, 1), + 0x558, 1, 0), DEF_MOD("gpu_ace_clk", R9A07G044_GPU_ACE_CLK, R9A07G044_CLK_P1, - 0x558, 2), + 0x558, 2, 0), DEF_MOD("cru_sysclk", R9A07G044_CRU_SYSCLK, CLK_M2_DIV2, - 0x564, 0), + 0x564, 0, 0), DEF_MOD("cru_vclk", R9A07G044_CRU_VCLK, R9A07G044_CLK_M2, - 0x564, 1), + 0x564, 1, 0), DEF_MOD("cru_pclk", R9A07G044_CRU_PCLK, R9A07G044_CLK_ZT, - 0x564, 2), + 0x564, 2, 0), DEF_MOD("cru_aclk", R9A07G044_CRU_ACLK, R9A07G044_CLK_M0, - 0x564, 3), + 0x564, 3, 0), DEF_MOD("dsi_pll_clk", R9A07G044_MIPI_DSI_PLLCLK, R9A07G044_CLK_M1, - 0x568, 0), + 0x568, 0, 0), DEF_MOD("dsi_sys_clk", R9A07G044_MIPI_DSI_SYSCLK, CLK_M2_DIV2, - 0x568, 1), + 0x568, 1, 0), DEF_MOD("dsi_aclk", R9A07G044_MIPI_DSI_ACLK, R9A07G044_CLK_P1, - 0x568, 2), + 0x568, 2, 0), DEF_MOD("dsi_pclk", R9A07G044_MIPI_DSI_PCLK, R9A07G044_CLK_P2, - 0x568, 3), + 0x568, 3, 0), DEF_MOD("dsi_vclk", R9A07G044_MIPI_DSI_VCLK, R9A07G044_CLK_M3, - 0x568, 4), + 0x568, 4, 0), DEF_MOD("dsi_lpclk", R9A07G044_MIPI_DSI_LPCLK, R9A07G044_CLK_M4, - 0x568, 5), + 0x568, 5, 0), DEF_COUPLED("lcdc_a", R9A07G044_LCDC_CLK_A, R9A07G044_CLK_M0, - 0x56c, 0), + 0x56c, 0, 0), DEF_COUPLED("lcdc_p", R9A07G044_LCDC_CLK_P, R9A07G044_CLK_ZT, - 0x56c, 0), + 0x56c, 0, 0), DEF_MOD("lcdc_clk_d", R9A07G044_LCDC_CLK_D, R9A07G044_CLK_M3, - 0x56c, 1), + 0x56c, 1, 0), DEF_MOD("ssi0_pclk", R9A07G044_SSI0_PCLK2, R9A07G044_CLK_P0, - 0x570, 0), + 0x570, 0, 0), DEF_MOD("ssi0_sfr", R9A07G044_SSI0_PCLK_SFR, R9A07G044_CLK_P0, - 0x570, 1), + 0x570, 1, 0), DEF_MOD("ssi1_pclk", R9A07G044_SSI1_PCLK2, R9A07G044_CLK_P0, - 0x570, 2), + 0x570, 2, 0), DEF_MOD("ssi1_sfr", R9A07G044_SSI1_PCLK_SFR, R9A07G044_CLK_P0, - 0x570, 3), + 0x570, 3, 0), DEF_MOD("ssi2_pclk", R9A07G044_SSI2_PCLK2, R9A07G044_CLK_P0, - 0x570, 4), + 0x570, 4, 0), DEF_MOD("ssi2_sfr", R9A07G044_SSI2_PCLK_SFR, R9A07G044_CLK_P0, - 0x570, 5), + 0x570, 5, 0), DEF_MOD("ssi3_pclk", R9A07G044_SSI3_PCLK2, R9A07G044_CLK_P0, - 0x570, 6), + 0x570, 6, 0), DEF_MOD("ssi3_sfr", R9A07G044_SSI3_PCLK_SFR, R9A07G044_CLK_P0, - 0x570, 7), + 0x570, 7, 0), DEF_MOD("usb0_host", R9A07G044_USB_U2H0_HCLK, R9A07G044_CLK_P1, - 0x578, 0), + 0x578, 0, 0), DEF_MOD("usb1_host", R9A07G044_USB_U2H1_HCLK, R9A07G044_CLK_P1, - 0x578, 1), + 0x578, 1, 0), DEF_MOD("usb0_func", R9A07G044_USB_U2P_EXR_CPUCLK, R9A07G044_CLK_P1, - 0x578, 2), + 0x578, 2, 0), DEF_MOD("usb_pclk", R9A07G044_USB_PCLK, R9A07G044_CLK_P1, - 0x578, 3), + 0x578, 3, 0), DEF_COUPLED("eth0_axi", R9A07G044_ETH0_CLK_AXI, R9A07G044_CLK_M0, - 0x57c, 0), + 0x57c, 0, 0), DEF_COUPLED("eth0_chi", R9A07G044_ETH0_CLK_CHI, R9A07G044_CLK_ZT, - 0x57c, 0), + 0x57c, 0, 0), DEF_COUPLED("eth1_axi", R9A07G044_ETH1_CLK_AXI, R9A07G044_CLK_M0, - 0x57c, 1), + 0x57c, 1, 0), DEF_COUPLED("eth1_chi", R9A07G044_ETH1_CLK_CHI, R9A07G044_CLK_ZT, - 0x57c, 1), + 0x57c, 1, 0), DEF_MOD("i2c0", R9A07G044_I2C0_PCLK, R9A07G044_CLK_P0, - 0x580, 0), + 0x580, 0, 0), DEF_MOD("i2c1", R9A07G044_I2C1_PCLK, R9A07G044_CLK_P0, - 0x580, 1), + 0x580, 1, 0), DEF_MOD("i2c2", R9A07G044_I2C2_PCLK, R9A07G044_CLK_P0, - 0x580, 2), + 0x580, 2, 0), DEF_MOD("i2c3", R9A07G044_I2C3_PCLK, R9A07G044_CLK_P0, - 0x580, 3), + 0x580, 3, 0), DEF_MOD("scif0", R9A07G044_SCIF0_CLK_PCK, R9A07G044_CLK_P0, - 0x584, 0), + 0x584, 0, 0), DEF_MOD("scif1", R9A07G044_SCIF1_CLK_PCK, R9A07G044_CLK_P0, - 0x584, 1), + 0x584, 1, 0), DEF_MOD("scif2", R9A07G044_SCIF2_CLK_PCK, R9A07G044_CLK_P0, - 0x584, 2), + 0x584, 2, 0), DEF_MOD("scif3", R9A07G044_SCIF3_CLK_PCK, R9A07G044_CLK_P0, - 0x584, 3), + 0x584, 3, 0), DEF_MOD("scif4", R9A07G044_SCIF4_CLK_PCK, R9A07G044_CLK_P0, - 0x584, 4), + 0x584, 4, 0), DEF_MOD("sci0", R9A07G044_SCI0_CLKP, R9A07G044_CLK_P0, - 0x588, 0), + 0x588, 0, 0), DEF_MOD("sci1", R9A07G044_SCI1_CLKP, R9A07G044_CLK_P0, - 0x588, 1), + 0x588, 1, 0), DEF_MOD("rspi0", R9A07G044_RSPI0_CLKB, R9A07G044_CLK_P0, - 0x590, 0), + 0x590, 0, 0), DEF_MOD("rspi1", R9A07G044_RSPI1_CLKB, R9A07G044_CLK_P0, - 0x590, 1), + 0x590, 1, 0), DEF_MOD("rspi2", R9A07G044_RSPI2_CLKB, R9A07G044_CLK_P0, - 0x590, 2), + 0x590, 2, 0), DEF_MOD("canfd", R9A07G044_CANFD_PCLK, R9A07G044_CLK_P0, - 0x594, 0), + 0x594, 0, 0), DEF_MOD("gpio", R9A07G044_GPIO_HCLK, R9A07G044_OSCCLK, - 0x598, 0), + 0x598, 0, 0), DEF_MOD("adc_adclk", R9A07G044_ADC_ADCLK, R9A07G044_CLK_TSU, - 0x5a8, 0), + 0x5a8, 0, 0), DEF_MOD("adc_pclk", R9A07G044_ADC_PCLK, R9A07G044_CLK_P0, - 0x5a8, 1), + 0x5a8, 1, 0), DEF_MOD("tsu_pclk", R9A07G044_TSU_PCLK, R9A07G044_CLK_TSU, - 0x5ac, 0), + 0x5ac, 0, 0), }, #ifdef CONFIG_CLK_R9A07G054 .drp = { diff --git a/drivers/clk/renesas/r9a08g045-cpg.c b/drivers/clk/renesas/r9a08g045-cpg.c index 4394cb241d99..6ff40763a00a 100644 --- a/drivers/clk/renesas/r9a08g045-cpg.c +++ b/drivers/clk/renesas/r9a08g045-cpg.c @@ -187,23 +187,39 @@ static const struct cpg_core_clk r9a08g045_core_clks[] __initconst = { }; static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = { - DEF_MOD("gic_gicclk", R9A08G045_GIC600_GICCLK, R9A08G045_CLK_P1, 0x514, 0), - DEF_MOD("ia55_clk", R9A08G045_IA55_CLK, R9A08G045_CLK_P1, 0x518, 1), - DEF_MOD("dmac_aclk", R9A08G045_DMAC_ACLK, R9A08G045_CLK_P3, 0x52c, 0), - DEF_MOD("sdhi0_imclk", R9A08G045_SDHI0_IMCLK, CLK_SD0_DIV4, 0x554, 0), - DEF_MOD("sdhi0_imclk2", R9A08G045_SDHI0_IMCLK2, CLK_SD0_DIV4, 0x554, 1), - DEF_MOD("sdhi0_clk_hs", R9A08G045_SDHI0_CLK_HS, R9A08G045_CLK_SD0, 0x554, 2), - DEF_MOD("sdhi0_aclk", R9A08G045_SDHI0_ACLK, R9A08G045_CLK_P1, 0x554, 3), - DEF_MOD("sdhi1_imclk", R9A08G045_SDHI1_IMCLK, CLK_SD1_DIV4, 0x554, 4), - DEF_MOD("sdhi1_imclk2", R9A08G045_SDHI1_IMCLK2, CLK_SD1_DIV4, 0x554, 5), - DEF_MOD("sdhi1_clk_hs", R9A08G045_SDHI1_CLK_HS, R9A08G045_CLK_SD1, 0x554, 6), - DEF_MOD("sdhi1_aclk", R9A08G045_SDHI1_ACLK, R9A08G045_CLK_P1, 0x554, 7), - DEF_MOD("sdhi2_imclk", R9A08G045_SDHI2_IMCLK, CLK_SD2_DIV4, 0x554, 8), - DEF_MOD("sdhi2_imclk2", R9A08G045_SDHI2_IMCLK2, CLK_SD2_DIV4, 0x554, 9), - DEF_MOD("sdhi2_clk_hs", R9A08G045_SDHI2_CLK_HS, R9A08G045_CLK_SD2, 0x554, 10), - DEF_MOD("sdhi2_aclk", R9A08G045_SDHI2_ACLK, R9A08G045_CLK_P1, 0x554, 11), - DEF_MOD("scif0_clk_pck", R9A08G045_SCIF0_CLK_PCK, R9A08G045_CLK_P0, 0x584, 0), - DEF_MOD("gpio_hclk", R9A08G045_GPIO_HCLK, R9A08G045_OSCCLK, 0x598, 0), + DEF_MOD("gic_gicclk", R9A08G045_GIC600_GICCLK, R9A08G045_CLK_P1, 0x514, 0, + MSTOP(ACPU, BIT(3))), + DEF_MOD("ia55_clk", R9A08G045_IA55_CLK, R9A08G045_CLK_P1, 0x518, 1, + MSTOP(PERI_CPU, BIT(13))), + DEF_MOD("dmac_aclk", R9A08G045_DMAC_ACLK, R9A08G045_CLK_P3, 0x52c, 0, + MSTOP(REG1, BIT(2))), + DEF_MOD("sdhi0_imclk", R9A08G045_SDHI0_IMCLK, CLK_SD0_DIV4, 0x554, 0, + MSTOP(PERI_COM, BIT(0))), + DEF_MOD("sdhi0_imclk2", R9A08G045_SDHI0_IMCLK2, CLK_SD0_DIV4, 0x554, 1, + MSTOP(PERI_COM, BIT(0))), + DEF_MOD("sdhi0_clk_hs", R9A08G045_SDHI0_CLK_HS, R9A08G045_CLK_SD0, 0x554, 2, + MSTOP(PERI_COM, BIT(0))), + DEF_MOD("sdhi0_aclk", R9A08G045_SDHI0_ACLK, R9A08G045_CLK_P1, 0x554, 3, + MSTOP(PERI_COM, BIT(0))), + DEF_MOD("sdhi1_imclk", R9A08G045_SDHI1_IMCLK, CLK_SD1_DIV4, 0x554, 4, + MSTOP(PERI_COM, BIT(1))), + DEF_MOD("sdhi1_imclk2", R9A08G045_SDHI1_IMCLK2, CLK_SD1_DIV4, 0x554, 5, + MSTOP(PERI_COM, BIT(1))), + DEF_MOD("sdhi1_clk_hs", R9A08G045_SDHI1_CLK_HS, R9A08G045_CLK_SD1, 0x554, 6, + MSTOP(PERI_COM, BIT(1))), + DEF_MOD("sdhi1_aclk", R9A08G045_SDHI1_ACLK, R9A08G045_CLK_P1, 0x554, 7, + MSTOP(PERI_COM, BIT(1))), + DEF_MOD("sdhi2_imclk", R9A08G045_SDHI2_IMCLK, CLK_SD2_DIV4, 0x554, 8, + MSTOP(PERI_COM, BIT(11))), + DEF_MOD("sdhi2_imclk2", R9A08G045_SDHI2_IMCLK2, CLK_SD2_DIV4, 0x554, 9, + MSTOP(PERI_COM, BIT(11))), + DEF_MOD("sdhi2_clk_hs", R9A08G045_SDHI2_CLK_HS, R9A08G045_CLK_SD2, 0x554, 10, + MSTOP(PERI_COM, BIT(11))), + DEF_MOD("sdhi2_aclk", R9A08G045_SDHI2_ACLK, R9A08G045_CLK_P1, 0x554, 11, + MSTOP(PERI_COM, BIT(11))), + DEF_MOD("scif0_clk_pck", R9A08G045_SCIF0_CLK_PCK, R9A08G045_CLK_P0, 0x584, 0, + MSTOP(MCPU2, BIT(1))), + DEF_MOD("gpio_hclk", R9A08G045_GPIO_HCLK, R9A08G045_OSCCLK, 0x598, 0, 0), }; static const struct rzg2l_reset r9a08g045_resets[] = { diff --git a/drivers/clk/renesas/r9a09g011-cpg.c b/drivers/clk/renesas/r9a09g011-cpg.c index dda9f29dff33..eae408f8604a 100644 --- a/drivers/clk/renesas/r9a09g011-cpg.c +++ b/drivers/clk/renesas/r9a09g011-cpg.c @@ -152,64 +152,64 @@ static const struct cpg_core_clk r9a09g011_core_clks[] __initconst = { }; static const struct rzg2l_mod_clk r9a09g011_mod_clks[] __initconst = { - DEF_MOD("pfc", R9A09G011_PFC_PCLK, CLK_MAIN, 0x400, 2), - DEF_MOD("gic", R9A09G011_GIC_CLK, CLK_SEL_B_D2, 0x400, 5), - DEF_MOD("sdi0_aclk", R9A09G011_SDI0_ACLK, CLK_SEL_D, 0x408, 0), - DEF_MOD("sdi0_imclk", R9A09G011_SDI0_IMCLK, CLK_SEL_SDI, 0x408, 1), - DEF_MOD("sdi0_imclk2", R9A09G011_SDI0_IMCLK2, CLK_SEL_SDI, 0x408, 2), - DEF_MOD("sdi0_clk_hs", R9A09G011_SDI0_CLK_HS, CLK_PLL2_800, 0x408, 3), - DEF_MOD("sdi1_aclk", R9A09G011_SDI1_ACLK, CLK_SEL_D, 0x408, 4), - DEF_MOD("sdi1_imclk", R9A09G011_SDI1_IMCLK, CLK_SEL_SDI, 0x408, 5), - DEF_MOD("sdi1_imclk2", R9A09G011_SDI1_IMCLK2, CLK_SEL_SDI, 0x408, 6), - DEF_MOD("sdi1_clk_hs", R9A09G011_SDI1_CLK_HS, CLK_PLL2_800, 0x408, 7), - DEF_MOD("emm_aclk", R9A09G011_EMM_ACLK, CLK_SEL_D, 0x408, 8), - DEF_MOD("emm_imclk", R9A09G011_EMM_IMCLK, CLK_SEL_SDI, 0x408, 9), - DEF_MOD("emm_imclk2", R9A09G011_EMM_IMCLK2, CLK_SEL_SDI, 0x408, 10), - DEF_MOD("emm_clk_hs", R9A09G011_EMM_CLK_HS, CLK_PLL2_800, 0x408, 11), - DEF_COUPLED("eth_axi", R9A09G011_ETH0_CLK_AXI, CLK_PLL2_200, 0x40c, 8), - DEF_COUPLED("eth_chi", R9A09G011_ETH0_CLK_CHI, CLK_PLL2_100, 0x40c, 8), - DEF_MOD("eth_clk_gptp", R9A09G011_ETH0_GPTP_EXT, CLK_PLL2_100, 0x40c, 9), - DEF_MOD("usb_aclk_h", R9A09G011_USB_ACLK_H, CLK_SEL_D, 0x40c, 4), - DEF_MOD("usb_aclk_p", R9A09G011_USB_ACLK_P, CLK_SEL_D, 0x40c, 5), - DEF_MOD("usb_pclk", R9A09G011_USB_PCLK, CLK_SEL_E, 0x40c, 6), - DEF_MOD("syc_cnt_clk", R9A09G011_SYC_CNT_CLK, CLK_MAIN_24, 0x41c, 12), - DEF_MOD("iic_pclk0", R9A09G011_IIC_PCLK0, CLK_SEL_E, 0x420, 12), - DEF_MOD("cperi_grpb", R9A09G011_CPERI_GRPB_PCLK, CLK_SEL_E, 0x424, 0), - DEF_MOD("tim_clk_8", R9A09G011_TIM8_CLK, CLK_MAIN_2, 0x424, 4), - DEF_MOD("tim_clk_9", R9A09G011_TIM9_CLK, CLK_MAIN_2, 0x424, 5), - DEF_MOD("tim_clk_10", R9A09G011_TIM10_CLK, CLK_MAIN_2, 0x424, 6), - DEF_MOD("tim_clk_11", R9A09G011_TIM11_CLK, CLK_MAIN_2, 0x424, 7), - DEF_MOD("tim_clk_12", R9A09G011_TIM12_CLK, CLK_MAIN_2, 0x424, 8), - DEF_MOD("tim_clk_13", R9A09G011_TIM13_CLK, CLK_MAIN_2, 0x424, 9), - DEF_MOD("tim_clk_14", R9A09G011_TIM14_CLK, CLK_MAIN_2, 0x424, 10), - DEF_MOD("tim_clk_15", R9A09G011_TIM15_CLK, CLK_MAIN_2, 0x424, 11), - DEF_MOD("iic_pclk1", R9A09G011_IIC_PCLK1, CLK_SEL_E, 0x424, 12), - DEF_MOD("cperi_grpc", R9A09G011_CPERI_GRPC_PCLK, CLK_SEL_E, 0x428, 0), - DEF_MOD("tim_clk_16", R9A09G011_TIM16_CLK, CLK_MAIN_2, 0x428, 4), - DEF_MOD("tim_clk_17", R9A09G011_TIM17_CLK, CLK_MAIN_2, 0x428, 5), - DEF_MOD("tim_clk_18", R9A09G011_TIM18_CLK, CLK_MAIN_2, 0x428, 6), - DEF_MOD("tim_clk_19", R9A09G011_TIM19_CLK, CLK_MAIN_2, 0x428, 7), - DEF_MOD("tim_clk_20", R9A09G011_TIM20_CLK, CLK_MAIN_2, 0x428, 8), - DEF_MOD("tim_clk_21", R9A09G011_TIM21_CLK, CLK_MAIN_2, 0x428, 9), - DEF_MOD("tim_clk_22", R9A09G011_TIM22_CLK, CLK_MAIN_2, 0x428, 10), - DEF_MOD("tim_clk_23", R9A09G011_TIM23_CLK, CLK_MAIN_2, 0x428, 11), - DEF_MOD("wdt0_pclk", R9A09G011_WDT0_PCLK, CLK_SEL_E, 0x428, 12), - DEF_MOD("wdt0_clk", R9A09G011_WDT0_CLK, CLK_MAIN, 0x428, 13), - DEF_MOD("cperi_grpf", R9A09G011_CPERI_GRPF_PCLK, CLK_SEL_E, 0x434, 0), - DEF_MOD("pwm8_clk", R9A09G011_PWM8_CLK, CLK_MAIN, 0x434, 4), - DEF_MOD("pwm9_clk", R9A09G011_PWM9_CLK, CLK_MAIN, 0x434, 5), - DEF_MOD("pwm10_clk", R9A09G011_PWM10_CLK, CLK_MAIN, 0x434, 6), - DEF_MOD("pwm11_clk", R9A09G011_PWM11_CLK, CLK_MAIN, 0x434, 7), - DEF_MOD("pwm12_clk", R9A09G011_PWM12_CLK, CLK_MAIN, 0x434, 8), - DEF_MOD("pwm13_clk", R9A09G011_PWM13_CLK, CLK_MAIN, 0x434, 9), - DEF_MOD("pwm14_clk", R9A09G011_PWM14_CLK, CLK_MAIN, 0x434, 10), - DEF_MOD("cperi_grpg", R9A09G011_CPERI_GRPG_PCLK, CLK_SEL_E, 0x438, 0), - DEF_MOD("cperi_grph", R9A09G011_CPERI_GRPH_PCLK, CLK_SEL_E, 0x438, 1), - DEF_MOD("urt_pclk", R9A09G011_URT_PCLK, CLK_SEL_E, 0x438, 4), - DEF_MOD("urt0_clk", R9A09G011_URT0_CLK, CLK_SEL_W0, 0x438, 5), - DEF_MOD("csi0_clk", R9A09G011_CSI0_CLK, CLK_SEL_CSI0, 0x438, 8), - DEF_MOD("csi4_clk", R9A09G011_CSI4_CLK, CLK_SEL_CSI4, 0x438, 12), - DEF_MOD("ca53", R9A09G011_CA53_CLK, CLK_DIV_A, 0x448, 0), + DEF_MOD("pfc", R9A09G011_PFC_PCLK, CLK_MAIN, 0x400, 2, 0), + DEF_MOD("gic", R9A09G011_GIC_CLK, CLK_SEL_B_D2, 0x400, 5, 0), + DEF_MOD("sdi0_aclk", R9A09G011_SDI0_ACLK, CLK_SEL_D, 0x408, 0, 0), + DEF_MOD("sdi0_imclk", R9A09G011_SDI0_IMCLK, CLK_SEL_SDI, 0x408, 1, 0), + DEF_MOD("sdi0_imclk2", R9A09G011_SDI0_IMCLK2, CLK_SEL_SDI, 0x408, 2, 0), + DEF_MOD("sdi0_clk_hs", R9A09G011_SDI0_CLK_HS, CLK_PLL2_800, 0x408, 3, 0), + DEF_MOD("sdi1_aclk", R9A09G011_SDI1_ACLK, CLK_SEL_D, 0x408, 4, 0), + DEF_MOD("sdi1_imclk", R9A09G011_SDI1_IMCLK, CLK_SEL_SDI, 0x408, 5, 0), + DEF_MOD("sdi1_imclk2", R9A09G011_SDI1_IMCLK2, CLK_SEL_SDI, 0x408, 6, 0), + DEF_MOD("sdi1_clk_hs", R9A09G011_SDI1_CLK_HS, CLK_PLL2_800, 0x408, 7, 0), + DEF_MOD("emm_aclk", R9A09G011_EMM_ACLK, CLK_SEL_D, 0x408, 8, 0), + DEF_MOD("emm_imclk", R9A09G011_EMM_IMCLK, CLK_SEL_SDI, 0x408, 9, 0), + DEF_MOD("emm_imclk2", R9A09G011_EMM_IMCLK2, CLK_SEL_SDI, 0x408, 10, 0), + DEF_MOD("emm_clk_hs", R9A09G011_EMM_CLK_HS, CLK_PLL2_800, 0x408, 11, 0), + DEF_COUPLED("eth_axi", R9A09G011_ETH0_CLK_AXI, CLK_PLL2_200, 0x40c, 8, 0), + DEF_COUPLED("eth_chi", R9A09G011_ETH0_CLK_CHI, CLK_PLL2_100, 0x40c, 8, 0), + DEF_MOD("eth_clk_gptp", R9A09G011_ETH0_GPTP_EXT, CLK_PLL2_100, 0x40c, 9, 0), + DEF_MOD("usb_aclk_h", R9A09G011_USB_ACLK_H, CLK_SEL_D, 0x40c, 4, 0), + DEF_MOD("usb_aclk_p", R9A09G011_USB_ACLK_P, CLK_SEL_D, 0x40c, 5, 0), + DEF_MOD("usb_pclk", R9A09G011_USB_PCLK, CLK_SEL_E, 0x40c, 6, 0), + DEF_MOD("syc_cnt_clk", R9A09G011_SYC_CNT_CLK, CLK_MAIN_24, 0x41c, 12, 0), + DEF_MOD("iic_pclk0", R9A09G011_IIC_PCLK0, CLK_SEL_E, 0x420, 12, 0), + DEF_MOD("cperi_grpb", R9A09G011_CPERI_GRPB_PCLK, CLK_SEL_E, 0x424, 0, 0), + DEF_MOD("tim_clk_8", R9A09G011_TIM8_CLK, CLK_MAIN_2, 0x424, 4, 0), + DEF_MOD("tim_clk_9", R9A09G011_TIM9_CLK, CLK_MAIN_2, 0x424, 5, 0), + DEF_MOD("tim_clk_10", R9A09G011_TIM10_CLK, CLK_MAIN_2, 0x424, 6, 0), + DEF_MOD("tim_clk_11", R9A09G011_TIM11_CLK, CLK_MAIN_2, 0x424, 7, 0), + DEF_MOD("tim_clk_12", R9A09G011_TIM12_CLK, CLK_MAIN_2, 0x424, 8, 0), + DEF_MOD("tim_clk_13", R9A09G011_TIM13_CLK, CLK_MAIN_2, 0x424, 9, 0), + DEF_MOD("tim_clk_14", R9A09G011_TIM14_CLK, CLK_MAIN_2, 0x424, 10, 0), + DEF_MOD("tim_clk_15", R9A09G011_TIM15_CLK, CLK_MAIN_2, 0x424, 11, 0), + DEF_MOD("iic_pclk1", R9A09G011_IIC_PCLK1, CLK_SEL_E, 0x424, 12, 0), + DEF_MOD("cperi_grpc", R9A09G011_CPERI_GRPC_PCLK, CLK_SEL_E, 0x428, 0, 0), + DEF_MOD("tim_clk_16", R9A09G011_TIM16_CLK, CLK_MAIN_2, 0x428, 4, 0), + DEF_MOD("tim_clk_17", R9A09G011_TIM17_CLK, CLK_MAIN_2, 0x428, 5, 0), + DEF_MOD("tim_clk_18", R9A09G011_TIM18_CLK, CLK_MAIN_2, 0x428, 6, 0), + DEF_MOD("tim_clk_19", R9A09G011_TIM19_CLK, CLK_MAIN_2, 0x428, 7, 0), + DEF_MOD("tim_clk_20", R9A09G011_TIM20_CLK, CLK_MAIN_2, 0x428, 8, 0), + DEF_MOD("tim_clk_21", R9A09G011_TIM21_CLK, CLK_MAIN_2, 0x428, 9, 0), + DEF_MOD("tim_clk_22", R9A09G011_TIM22_CLK, CLK_MAIN_2, 0x428, 10, 0), + DEF_MOD("tim_clk_23", R9A09G011_TIM23_CLK, CLK_MAIN_2, 0x428, 11, 0), + DEF_MOD("wdt0_pclk", R9A09G011_WDT0_PCLK, CLK_SEL_E, 0x428, 12, 0), + DEF_MOD("wdt0_clk", R9A09G011_WDT0_CLK, CLK_MAIN, 0x428, 13, 0), + DEF_MOD("cperi_grpf", R9A09G011_CPERI_GRPF_PCLK, CLK_SEL_E, 0x434, 0, 0), + DEF_MOD("pwm8_clk", R9A09G011_PWM8_CLK, CLK_MAIN, 0x434, 4, 0), + DEF_MOD("pwm9_clk", R9A09G011_PWM9_CLK, CLK_MAIN, 0x434, 5, 0), + DEF_MOD("pwm10_clk", R9A09G011_PWM10_CLK, CLK_MAIN, 0x434, 6, 0), + DEF_MOD("pwm11_clk", R9A09G011_PWM11_CLK, CLK_MAIN, 0x434, 7, 0), + DEF_MOD("pwm12_clk", R9A09G011_PWM12_CLK, CLK_MAIN, 0x434, 8, 0), + DEF_MOD("pwm13_clk", R9A09G011_PWM13_CLK, CLK_MAIN, 0x434, 9, 0), + DEF_MOD("pwm14_clk", R9A09G011_PWM14_CLK, CLK_MAIN, 0x434, 10, 0), + DEF_MOD("cperi_grpg", R9A09G011_CPERI_GRPG_PCLK, CLK_SEL_E, 0x438, 0, 0), + DEF_MOD("cperi_grph", R9A09G011_CPERI_GRPH_PCLK, CLK_SEL_E, 0x438, 1, 0), + DEF_MOD("urt_pclk", R9A09G011_URT_PCLK, CLK_SEL_E, 0x438, 4, 0), + DEF_MOD("urt0_clk", R9A09G011_URT0_CLK, CLK_SEL_W0, 0x438, 5, 0), + DEF_MOD("csi0_clk", R9A09G011_CSI0_CLK, CLK_SEL_CSI0, 0x438, 8, 0), + DEF_MOD("csi4_clk", R9A09G011_CSI4_CLK, CLK_SEL_CSI4, 0x438, 12, 0), + DEF_MOD("ca53", R9A09G011_CA53_CLK, CLK_DIV_A, 0x448, 0, 0), }; static const struct rzg2l_reset r9a09g011_resets[] = { diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c index 2922dc884e35..6fb815862e44 100644 --- a/drivers/clk/renesas/rzg2l-cpg.c +++ b/drivers/clk/renesas/rzg2l-cpg.c @@ -1177,6 +1177,17 @@ rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core, core->name, PTR_ERR(clk)); } +/** + * struct mstop - MSTOP specific data structure + * @count: reference counter for MSTOP settings (when zero the settings + * are applied to register) + * @conf: MSTOP configuration (register offset, setup bits) + */ +struct mstop { + u32 count; + u32 conf; +}; + /** * struct mstp_clock - MSTP gating clock * @@ -1186,6 +1197,7 @@ rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core, * @enabled: soft state of the clock, if it is coupled with another clock * @priv: CPG/MSTP private data * @sibling: pointer to the other coupled clock + * @mstop: MSTOP configuration */ struct mstp_clock { struct clk_hw hw; @@ -1194,10 +1206,46 @@ struct mstp_clock { bool enabled; struct rzg2l_cpg_priv *priv; struct mstp_clock *sibling; + struct mstop *mstop; }; #define to_mod_clock(_hw) container_of(_hw, struct mstp_clock, hw) +/* Need to be called with a lock held to avoid concurent access to mstop->count. */ +static void rzg2l_mod_clock_module_set_standby(struct mstp_clock *clock, + bool standby) +{ + struct rzg2l_cpg_priv *priv = clock->priv; + struct mstop *mstop = clock->mstop; + bool update = false; + u32 value; + + if (!mstop) + return; + + value = MSTOP_MASK(mstop->conf) << 16; + + if (standby) { + value |= MSTOP_MASK(mstop->conf); + /* Avoid overflow. */ + if (mstop->count > 0) + mstop->count--; + + if (!mstop->count) + update = true; + } else { + if (!mstop->count) + update = true; + + /* Avoid overflow. */ + if (mstop->count + 1 != 0) + mstop->count++; + } + + if (update) + writel(value, priv->base + MSTOP_OFF(mstop->conf)); +} + static int rzg2l_mod_clock_endisable(struct clk_hw *hw, bool enable) { struct mstp_clock *clock = to_mod_clock(hw); @@ -1205,6 +1253,7 @@ static int rzg2l_mod_clock_endisable(struct clk_hw *hw, bool enable) unsigned int reg = clock->off; struct device *dev = priv->dev; u32 bitmask = BIT(clock->bit); + unsigned long flags; u32 value; int error; @@ -1220,7 +1269,10 @@ static int rzg2l_mod_clock_endisable(struct clk_hw *hw, bool enable) if (enable) value |= bitmask; + spin_lock_irqsave(&priv->rmw_lock, flags); + rzg2l_mod_clock_module_set_standby(clock, !enable); writel(value, priv->base + CLK_ON_R(reg)); + spin_unlock_irqrestore(&priv->rmw_lock, flags); if (!enable) return 0; @@ -1328,6 +1380,27 @@ static struct mstp_clock return NULL; } +static struct mstop *rzg2l_mod_clock_get_mstop(struct rzg2l_cpg_priv *priv, u32 conf) +{ + for (unsigned int i = 0; i < priv->num_mod_clks; i++) { + struct mstp_clock *clk; + struct clk_hw *hw; + + if (priv->clks[priv->num_core_clks + i] == ERR_PTR(-ENOENT)) + continue; + + hw = __clk_get_hw(priv->clks[priv->num_core_clks + i]); + clk = to_mod_clock(hw); + if (!clk->mstop) + continue; + + if (clk->mstop->conf == conf) + return clk->mstop; + } + + return NULL; +} + static void __init rzg2l_cpg_register_mod_clk(const struct rzg2l_mod_clk *mod, const struct rzg2l_cpg_info *info, @@ -1401,6 +1474,37 @@ rzg2l_cpg_register_mod_clk(const struct rzg2l_mod_clk *mod, } } + if (mod->mstop_conf) { + struct mstop *mstop = rzg2l_mod_clock_get_mstop(priv, mod->mstop_conf); + + if (mstop) { + clock->mstop = mstop; + } else { + mstop = devm_kzalloc(dev, sizeof(*mstop), GFP_KERNEL); + if (!mstop) { + clk_unregister(clk); + goto fail; + } + + mstop->conf = mod->mstop_conf; + clock->mstop = mstop; + } + + if (rzg2l_mod_clock_is_enabled(&clock->hw)) { + if (clock->sibling) + clock->mstop->count = 1; + else + clock->mstop->count++; + } + + /* + * Out of reset all modules are enabled. Set module to standby + * in case associated clocks are disabled at probe. + */ + if (!clock->mstop->count) + rzg2l_mod_clock_module_set_standby(clock, true); + } + return; fail: diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h index 6e38c8fc888c..10ee8aa4a5da 100644 --- a/drivers/clk/renesas/rzg2l-cpg.h +++ b/drivers/clk/renesas/rzg2l-cpg.h @@ -27,6 +27,11 @@ #define CPG_PL6_ETH_SSEL (0x418) #define CPG_PL5_SDIV (0x420) #define CPG_RST_MON (0x680) +#define CPG_ACPU_MSTOP (0xB60) +#define CPG_MCPU2_MSTOP (0xB68) +#define CPG_PERI_COM_MSTOP (0xB6C) +#define CPG_PERI_CPU_MSTOP (0xB70) +#define CPG_REG1_MSTOP (0xB80) #define CPG_OTHERFUNC1_REG (0xBE8) #define CPG_SIPLL5_STBY_RESETB BIT(0) @@ -68,6 +73,10 @@ #define SEL_PLL6_2 SEL_PLL_PACK(CPG_PL6_ETH_SSEL, 0, 1) #define SEL_GPU2 SEL_PLL_PACK(CPG_PL6_SSEL, 12, 1) +#define MSTOP(name, bitmask) ((CPG_##name##_MSTOP) << 16 | (bitmask)) +#define MSTOP_OFF(conf) ((conf) >> 16) +#define MSTOP_MASK(conf) ((conf) & GENMASK(15, 0)) + #define EXTAL_FREQ_IN_MEGA_HZ (24) /** @@ -191,26 +200,28 @@ struct rzg2l_mod_clk { const char *name; unsigned int id; unsigned int parent; + u32 mstop_conf; u16 off; u8 bit; bool is_coupled; }; -#define DEF_MOD_BASE(_name, _id, _parent, _off, _bit, _is_coupled) \ +#define DEF_MOD_BASE(_name, _id, _parent, _off, _bit, _mstop_conf, _is_coupled) \ { \ .name = _name, \ .id = MOD_CLK_BASE + (_id), \ .parent = (_parent), \ + .mstop_conf = (_mstop_conf), \ .off = (_off), \ .bit = (_bit), \ .is_coupled = (_is_coupled), \ } -#define DEF_MOD(_name, _id, _parent, _off, _bit) \ - DEF_MOD_BASE(_name, _id, _parent, _off, _bit, false) +#define DEF_MOD(_name, _id, _parent, _off, _bit, _mstop_conf) \ + DEF_MOD_BASE(_name, _id, _parent, _off, _bit, _mstop_conf, false) -#define DEF_COUPLED(_name, _id, _parent, _off, _bit) \ - DEF_MOD_BASE(_name, _id, _parent, _off, _bit, true) +#define DEF_COUPLED(_name, _id, _parent, _off, _bit, _mstop_conf) \ + DEF_MOD_BASE(_name, _id, _parent, _off, _bit, _mstop_conf, true) /** * struct rzg2l_reset - Reset definitions -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 03/14] clk: renesas: rzg2l-cpg: Add support for MSTOP 2023-11-20 7:00 ` [PATCH 03/14] clk: renesas: rzg2l-cpg: Add support for MSTOP Claudiu @ 2023-11-23 16:35 ` Geert Uytterhoeven 2023-11-24 9:08 ` Geert Uytterhoeven 2023-11-24 9:24 ` claudiu beznea 0 siblings, 2 replies; 53+ messages in thread From: Geert Uytterhoeven @ 2023-11-23 16:35 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > RZ/{G2L, V2L, G3S} based CPG versions have support for saving extra > power when clocks are disabled by activating module standby. This is done > though MSTOP specific registers that are part of CPG. Each individual > module have one or more bits associated in one MSTOP register (see table > "Registers for Module Standby Mode" from HW manuals). Hardware manual > associates modules' clocks to one or more MSTOP bits. There are 3 mappings > available (identified by researching RZ/G2L, RZ/G3S, RZ/V2L HW manuals): > > case 1: N clocks mapped to N MSTOP bits (with N={0, ..., X}) > case 2: N clocks mapped to 1 MSTOP bit (with N={0, ..., X}) > case 3: N clocks mapped to M MSTOP bits (with N={0, ..., X}, M={0, ..., Y}) > > Case 3 has been currently identified on RZ/V2L for VCPL4 module. > > To cover all 3 cases the individual platform drivers will provide to > clock driver MSTOP register offset and associated bits in this register > as a bitmask and the clock driver will apply this bitmask to proper > MSTOP register. > > As most of the modules have more than one clock and these clocks are > mapped to 1 MSTOP bitmap that need to be applied to MSTOP registers, > to avoid switching the module to/out of standby when the module has > enabled/disabled clocks a counter has been associated to each module > (though struct mstop::count) which is incremented/decremented every > time a module's clock is enabled/disabled and the settings to MSTOP > register are applied only when the counter reaches zero (counter zero > means either 1st clock of the module is going to be enabled or all clocks > of the module are going to be disabled). Thanks for your patch! > The MSTOP functionality has been instantiated at the moment for RZ/G3S. Do you plan to add support for the other SoCs, too? > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- a/drivers/clk/renesas/r9a08g045-cpg.c > +++ b/drivers/clk/renesas/r9a08g045-cpg.c > @@ -187,23 +187,39 @@ static const struct cpg_core_clk r9a08g045_core_clks[] __initconst = { > }; > > static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = { > - DEF_MOD("gic_gicclk", R9A08G045_GIC600_GICCLK, R9A08G045_CLK_P1, 0x514, 0), > - DEF_MOD("ia55_clk", R9A08G045_IA55_CLK, R9A08G045_CLK_P1, 0x518, 1), > - DEF_MOD("dmac_aclk", R9A08G045_DMAC_ACLK, R9A08G045_CLK_P3, 0x52c, 0), > - DEF_MOD("sdhi0_imclk", R9A08G045_SDHI0_IMCLK, CLK_SD0_DIV4, 0x554, 0), > - DEF_MOD("sdhi0_imclk2", R9A08G045_SDHI0_IMCLK2, CLK_SD0_DIV4, 0x554, 1), > - DEF_MOD("sdhi0_clk_hs", R9A08G045_SDHI0_CLK_HS, R9A08G045_CLK_SD0, 0x554, 2), > - DEF_MOD("sdhi0_aclk", R9A08G045_SDHI0_ACLK, R9A08G045_CLK_P1, 0x554, 3), > - DEF_MOD("sdhi1_imclk", R9A08G045_SDHI1_IMCLK, CLK_SD1_DIV4, 0x554, 4), > - DEF_MOD("sdhi1_imclk2", R9A08G045_SDHI1_IMCLK2, CLK_SD1_DIV4, 0x554, 5), > - DEF_MOD("sdhi1_clk_hs", R9A08G045_SDHI1_CLK_HS, R9A08G045_CLK_SD1, 0x554, 6), > - DEF_MOD("sdhi1_aclk", R9A08G045_SDHI1_ACLK, R9A08G045_CLK_P1, 0x554, 7), > - DEF_MOD("sdhi2_imclk", R9A08G045_SDHI2_IMCLK, CLK_SD2_DIV4, 0x554, 8), > - DEF_MOD("sdhi2_imclk2", R9A08G045_SDHI2_IMCLK2, CLK_SD2_DIV4, 0x554, 9), > - DEF_MOD("sdhi2_clk_hs", R9A08G045_SDHI2_CLK_HS, R9A08G045_CLK_SD2, 0x554, 10), > - DEF_MOD("sdhi2_aclk", R9A08G045_SDHI2_ACLK, R9A08G045_CLK_P1, 0x554, 11), > - DEF_MOD("scif0_clk_pck", R9A08G045_SCIF0_CLK_PCK, R9A08G045_CLK_P0, 0x584, 0), > - DEF_MOD("gpio_hclk", R9A08G045_GPIO_HCLK, R9A08G045_OSCCLK, 0x598, 0), > + DEF_MOD("gic_gicclk", R9A08G045_GIC600_GICCLK, R9A08G045_CLK_P1, 0x514, 0, > + MSTOP(ACPU, BIT(3))), According to Rev. 1.00 of the Hardware User's Manual, bit 3 of the CPG_BUS_ACPU_MSTOP register is reserved? Also, gic_gicclk is a critical module clock, so I guess this module must never be put into standby? > --- a/drivers/clk/renesas/rzg2l-cpg.c > +++ b/drivers/clk/renesas/rzg2l-cpg.c > @@ -1177,6 +1177,17 @@ rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core, > core->name, PTR_ERR(clk)); > } > > +/** > + * struct mstop - MSTOP specific data structure > + * @count: reference counter for MSTOP settings (when zero the settings > + * are applied to register) > + * @conf: MSTOP configuration (register offset, setup bits) > + */ > +struct mstop { > + u32 count; > + u32 conf; > +}; > + > /** > * struct mstp_clock - MSTP gating clock > * > @@ -1186,6 +1197,7 @@ rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core, > * @enabled: soft state of the clock, if it is coupled with another clock > * @priv: CPG/MSTP private data > * @sibling: pointer to the other coupled clock > + * @mstop: MSTOP configuration > */ > struct mstp_clock { > struct clk_hw hw; > @@ -1194,10 +1206,46 @@ struct mstp_clock { > bool enabled; > struct rzg2l_cpg_priv *priv; > struct mstp_clock *sibling; > + struct mstop *mstop; > }; > > #define to_mod_clock(_hw) container_of(_hw, struct mstp_clock, hw) > > +/* Need to be called with a lock held to avoid concurent access to mstop->count. */ concurrent > +static void rzg2l_mod_clock_module_set_standby(struct mstp_clock *clock, > + bool standby) > +{ > + struct rzg2l_cpg_priv *priv = clock->priv; > + struct mstop *mstop = clock->mstop; > + bool update = false; > + u32 value; > + > + if (!mstop) > + return; > + > + value = MSTOP_MASK(mstop->conf) << 16; > + > + if (standby) { > + value |= MSTOP_MASK(mstop->conf); > + /* Avoid overflow. */ > + if (mstop->count > 0) > + mstop->count--; Should we add a WARN() here, or is it sufficient to rely on the WARN() in drivers/clk/clk.c:clk_core_disable()? > + > + if (!mstop->count) > + update = true; > + } else { > + if (!mstop->count) > + update = true; > + > + /* Avoid overflow. */ > + if (mstop->count + 1 != 0) > + mstop->count++; Trying to avoid an overflow won't help much here. The counter will be wrong afterwards anyway, and when decrementing again later, the module will be put in standby too soon... > + } > + > + if (update) > + writel(value, priv->base + MSTOP_OFF(mstop->conf)); > +} > + > static int rzg2l_mod_clock_endisable(struct clk_hw *hw, bool enable) > { > struct mstp_clock *clock = to_mod_clock(hw); > @@ -1401,6 +1474,37 @@ rzg2l_cpg_register_mod_clk(const struct rzg2l_mod_clk *mod, > } > } > > + if (mod->mstop_conf) { > + struct mstop *mstop = rzg2l_mod_clock_get_mstop(priv, mod->mstop_conf); > + > + if (mstop) { > + clock->mstop = mstop; Please move the common assignment after the if/else block... > + } else { ... so this can just become "if (!mstop) {". > + mstop = devm_kzalloc(dev, sizeof(*mstop), GFP_KERNEL); > + if (!mstop) { > + clk_unregister(clk); > + goto fail; Please use "goto unregister", and call clk_unregister() after the new unregister label. > + } > + > + mstop->conf = mod->mstop_conf; > + clock->mstop = mstop; > + } > + > + if (rzg2l_mod_clock_is_enabled(&clock->hw)) { > + if (clock->sibling) > + clock->mstop->count = 1; > + else > + clock->mstop->count++; > + } > + > + /* > + * Out of reset all modules are enabled. Set module to standby > + * in case associated clocks are disabled at probe. Is that always true? What about kexec and crashdump kernels? > + */ > + if (!clock->mstop->count) > + rzg2l_mod_clock_module_set_standby(clock, true); > + } > + > return; > > fail: > diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h > index 6e38c8fc888c..10ee8aa4a5da 100644 > --- a/drivers/clk/renesas/rzg2l-cpg.h > +++ b/drivers/clk/renesas/rzg2l-cpg.h > @@ -68,6 +73,10 @@ > #define SEL_PLL6_2 SEL_PLL_PACK(CPG_PL6_ETH_SSEL, 0, 1) > #define SEL_GPU2 SEL_PLL_PACK(CPG_PL6_SSEL, 12, 1) > > +#define MSTOP(name, bitmask) ((CPG_##name##_MSTOP) << 16 | (bitmask)) I believe the bitmask is always a single bit. So perhaps let MSTOP() take the bit number instead of the bitmaskl? You can still store BIT(bit) inside the macro. > +#define MSTOP_OFF(conf) ((conf) >> 16) > +#define MSTOP_MASK(conf) ((conf) & GENMASK(15, 0)) > + > #define EXTAL_FREQ_IN_MEGA_HZ (24) > > /** 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 03/14] clk: renesas: rzg2l-cpg: Add support for MSTOP 2023-11-23 16:35 ` Geert Uytterhoeven @ 2023-11-24 9:08 ` Geert Uytterhoeven 2023-11-27 7:37 ` claudiu beznea 2023-11-24 9:24 ` claudiu beznea 1 sibling, 1 reply; 53+ messages in thread From: Geert Uytterhoeven @ 2023-11-24 9:08 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Thu, Nov 23, 2023 at 5:35 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > > > RZ/{G2L, V2L, G3S} based CPG versions have support for saving extra > > power when clocks are disabled by activating module standby. This is done > > though MSTOP specific registers that are part of CPG. Each individual > > module have one or more bits associated in one MSTOP register (see table > > "Registers for Module Standby Mode" from HW manuals). Hardware manual > > associates modules' clocks to one or more MSTOP bits. There are 3 mappings > > available (identified by researching RZ/G2L, RZ/G3S, RZ/V2L HW manuals): > > > > case 1: N clocks mapped to N MSTOP bits (with N={0, ..., X}) > > case 2: N clocks mapped to 1 MSTOP bit (with N={0, ..., X}) > > case 3: N clocks mapped to M MSTOP bits (with N={0, ..., X}, M={0, ..., Y}) > > > > Case 3 has been currently identified on RZ/V2L for VCPL4 module. > > > > To cover all 3 cases the individual platform drivers will provide to > > clock driver MSTOP register offset and associated bits in this register > > as a bitmask and the clock driver will apply this bitmask to proper > > MSTOP register. > > > > As most of the modules have more than one clock and these clocks are > > mapped to 1 MSTOP bitmap that need to be applied to MSTOP registers, > > to avoid switching the module to/out of standby when the module has > > enabled/disabled clocks a counter has been associated to each module > > (though struct mstop::count) which is incremented/decremented every > > time a module's clock is enabled/disabled and the settings to MSTOP > > register are applied only when the counter reaches zero (counter zero > > means either 1st clock of the module is going to be enabled or all clocks > > of the module are going to be disabled). > > Thanks for your patch! > > > The MSTOP functionality has been instantiated at the moment for RZ/G3S. > > > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > --- a/drivers/clk/renesas/rzg2l-cpg.c > > +++ b/drivers/clk/renesas/rzg2l-cpg.c > > @@ -1177,6 +1177,17 @@ rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core, > > core->name, PTR_ERR(clk)); > > } > > > > +/** > > + * struct mstop - MSTOP specific data structure > > + * @count: reference counter for MSTOP settings (when zero the settings > > + * are applied to register) > > + * @conf: MSTOP configuration (register offset, setup bits) > > + */ > > +struct mstop { > > + u32 count; > > + u32 conf; > > +}; > > + > > /** > > * struct mstp_clock - MSTP gating clock > > * > > @@ -1186,6 +1197,7 @@ rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core, > > * @enabled: soft state of the clock, if it is coupled with another clock > > * @priv: CPG/MSTP private data > > * @sibling: pointer to the other coupled clock > > + * @mstop: MSTOP configuration > > */ > > struct mstp_clock { > > struct clk_hw hw; > > @@ -1194,10 +1206,46 @@ struct mstp_clock { > > bool enabled; > > struct rzg2l_cpg_priv *priv; > > struct mstp_clock *sibling; > > + struct mstop *mstop; > > }; > > > > #define to_mod_clock(_hw) container_of(_hw, struct mstp_clock, hw) > > > > +/* Need to be called with a lock held to avoid concurent access to mstop->count. */ > > concurrent > > > +static void rzg2l_mod_clock_module_set_standby(struct mstp_clock *clock, > > + bool standby) > > +{ > > + struct rzg2l_cpg_priv *priv = clock->priv; > > + struct mstop *mstop = clock->mstop; > > + bool update = false; > > + u32 value; > > + > > + if (!mstop) > > + return; > > + > > + value = MSTOP_MASK(mstop->conf) << 16; > > + > > + if (standby) { > > + value |= MSTOP_MASK(mstop->conf); > > + /* Avoid overflow. */ > > + if (mstop->count > 0) > > + mstop->count--; > > Should we add a WARN() here, or is it sufficient to rely on the WARN() > in drivers/clk/clk.c:clk_core_disable()? > > > + > > + if (!mstop->count) > > + update = true; > > + } else { > > + if (!mstop->count) > > + update = true; > > + > > + /* Avoid overflow. */ > > + if (mstop->count + 1 != 0) > > + mstop->count++; > > Trying to avoid an overflow won't help much here. The counter > will be wrong afterwards anyway, and when decrementing again later, the > module will be put in standby too soon... > > > + } > > + > > + if (update) > > + writel(value, priv->base + MSTOP_OFF(mstop->conf)); > > +} After giving this some more thought, it feels odd to derive the standby state of a module from the state of its module clocks, while the latter are already controlled through Runtime PM and a Clock Domain. A first alternative solution could be to drop the GENPD_FLAG_PM_CLK flag from the RZ/G2L CPG clock domain, and provide your own gpd_dev_ops.start() and .stop() callbacks that take care of both module standby and clocks (through pm_clk_{resume,suspend}(). (See https://elixir.bootlin.com/linux/v6.7-rc2/source/drivers/base/power/domain.c#L2093 for the GENPD_FLAG_PM_CLK case). That still leaves you with a need to associate an MSTOP register and bitmask with a device through its module clocks. A second alternative solution could be to increase #power-domain-cells from zero to one, and register individual PM Domains for each module, and control module standby from the generic_pm_domain.power_{on,off}() callbacks. Devices would specify the module using the power-domains = <&cpg <id> > property in DT, with <id> one of the to-be-added list of modules in include/dt-bindings/clock/r9a08g045-cpg.h. The RZ/G2L CPG driver can handle the mapping from <id> to MSTOP register and bitmask. This solution requires updates to DT, but you can keep compatibility with old DTBs by only registering the new PM Domains when #power-domain-cells is one. The extra power saving would only be applicable with new DTBs, though. Thoughts? > > --- a/drivers/clk/renesas/rzg2l-cpg.h > > +++ b/drivers/clk/renesas/rzg2l-cpg.h > > > @@ -68,6 +73,10 @@ > > #define SEL_PLL6_2 SEL_PLL_PACK(CPG_PL6_ETH_SSEL, 0, 1) > > #define SEL_GPU2 SEL_PLL_PACK(CPG_PL6_SSEL, 12, 1) > > > > +#define MSTOP(name, bitmask) ((CPG_##name##_MSTOP) << 16 | (bitmask)) > > I believe the bitmask is always a single bit. > So perhaps let MSTOP() take the bit number instead of the bitmaskl? > You can still store BIT(bit) inside the macro. I was wrong, the N->N or N->M cases need a bitmask. > > +#define MSTOP_OFF(conf) ((conf) >> 16) > > +#define MSTOP_MASK(conf) ((conf) & GENMASK(15, 0)) > > + > > #define EXTAL_FREQ_IN_MEGA_HZ (24) 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 03/14] clk: renesas: rzg2l-cpg: Add support for MSTOP 2023-11-24 9:08 ` Geert Uytterhoeven @ 2023-11-27 7:37 ` claudiu beznea 2023-12-01 15:36 ` Geert Uytterhoeven 0 siblings, 1 reply; 53+ messages in thread From: claudiu beznea @ 2023-11-27 7:37 UTC (permalink / raw) To: Geert Uytterhoeven Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi, Geert, On 24.11.2023 11:08, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Thu, Nov 23, 2023 at 5:35 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: >>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >>> >>> RZ/{G2L, V2L, G3S} based CPG versions have support for saving extra >>> power when clocks are disabled by activating module standby. This is done >>> though MSTOP specific registers that are part of CPG. Each individual >>> module have one or more bits associated in one MSTOP register (see table >>> "Registers for Module Standby Mode" from HW manuals). Hardware manual >>> associates modules' clocks to one or more MSTOP bits. There are 3 mappings >>> available (identified by researching RZ/G2L, RZ/G3S, RZ/V2L HW manuals): >>> >>> case 1: N clocks mapped to N MSTOP bits (with N={0, ..., X}) >>> case 2: N clocks mapped to 1 MSTOP bit (with N={0, ..., X}) >>> case 3: N clocks mapped to M MSTOP bits (with N={0, ..., X}, M={0, ..., Y}) >>> >>> Case 3 has been currently identified on RZ/V2L for VCPL4 module. >>> >>> To cover all 3 cases the individual platform drivers will provide to >>> clock driver MSTOP register offset and associated bits in this register >>> as a bitmask and the clock driver will apply this bitmask to proper >>> MSTOP register. >>> >>> As most of the modules have more than one clock and these clocks are >>> mapped to 1 MSTOP bitmap that need to be applied to MSTOP registers, >>> to avoid switching the module to/out of standby when the module has >>> enabled/disabled clocks a counter has been associated to each module >>> (though struct mstop::count) which is incremented/decremented every >>> time a module's clock is enabled/disabled and the settings to MSTOP >>> register are applied only when the counter reaches zero (counter zero >>> means either 1st clock of the module is going to be enabled or all clocks >>> of the module are going to be disabled). >> >> Thanks for your patch! >> >>> The MSTOP functionality has been instantiated at the moment for RZ/G3S. >>> >>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >>> --- a/drivers/clk/renesas/rzg2l-cpg.c >>> +++ b/drivers/clk/renesas/rzg2l-cpg.c >>> @@ -1177,6 +1177,17 @@ rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core, >>> core->name, PTR_ERR(clk)); >>> } >>> >>> +/** >>> + * struct mstop - MSTOP specific data structure >>> + * @count: reference counter for MSTOP settings (when zero the settings >>> + * are applied to register) >>> + * @conf: MSTOP configuration (register offset, setup bits) >>> + */ >>> +struct mstop { >>> + u32 count; >>> + u32 conf; >>> +}; >>> + >>> /** >>> * struct mstp_clock - MSTP gating clock >>> * >>> @@ -1186,6 +1197,7 @@ rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core, >>> * @enabled: soft state of the clock, if it is coupled with another clock >>> * @priv: CPG/MSTP private data >>> * @sibling: pointer to the other coupled clock >>> + * @mstop: MSTOP configuration >>> */ >>> struct mstp_clock { >>> struct clk_hw hw; >>> @@ -1194,10 +1206,46 @@ struct mstp_clock { >>> bool enabled; >>> struct rzg2l_cpg_priv *priv; >>> struct mstp_clock *sibling; >>> + struct mstop *mstop; >>> }; >>> >>> #define to_mod_clock(_hw) container_of(_hw, struct mstp_clock, hw) >>> >>> +/* Need to be called with a lock held to avoid concurent access to mstop->count. */ >> >> concurrent >> >>> +static void rzg2l_mod_clock_module_set_standby(struct mstp_clock *clock, >>> + bool standby) >>> +{ >>> + struct rzg2l_cpg_priv *priv = clock->priv; >>> + struct mstop *mstop = clock->mstop; >>> + bool update = false; >>> + u32 value; >>> + >>> + if (!mstop) >>> + return; >>> + >>> + value = MSTOP_MASK(mstop->conf) << 16; >>> + >>> + if (standby) { >>> + value |= MSTOP_MASK(mstop->conf); >>> + /* Avoid overflow. */ >>> + if (mstop->count > 0) >>> + mstop->count--; >> >> Should we add a WARN() here, or is it sufficient to rely on the WARN() >> in drivers/clk/clk.c:clk_core_disable()? >> >>> + >>> + if (!mstop->count) >>> + update = true; >>> + } else { >>> + if (!mstop->count) >>> + update = true; >>> + >>> + /* Avoid overflow. */ >>> + if (mstop->count + 1 != 0) >>> + mstop->count++; >> >> Trying to avoid an overflow won't help much here. The counter >> will be wrong afterwards anyway, and when decrementing again later, the >> module will be put in standby too soon... >> >>> + } >>> + >>> + if (update) >>> + writel(value, priv->base + MSTOP_OFF(mstop->conf)); >>> +} > > After giving this some more thought, it feels odd to derive the standby > state of a module from the state of its module clocks, while the latter > are already controlled through Runtime PM and a Clock Domain. Thanks for sharing this. > > A first alternative solution could be to drop the GENPD_FLAG_PM_CLK > flag from the RZ/G2L CPG clock domain, and provide your own > gpd_dev_ops.start() and .stop() callbacks that take care of both > module standby and clocks (through pm_clk_{resume,suspend}(). > (See https://elixir.bootlin.com/linux/v6.7-rc2/source/drivers/base/power/domain.c#L2093 > for the GENPD_FLAG_PM_CLK case). > That still leaves you with a need to associate an MSTOP register and > bitmask with a device through its module clocks. > > A second alternative solution could be to increase #power-domain-cells > from zero to one, and register individual PM Domains for each module, > and control module standby from the generic_pm_domain.power_{on,off}() > callbacks. Devices would specify the module using the power-domains = > <&cpg <id> > property in DT, with <id> one of the to-be-added list of > modules in include/dt-bindings/clock/r9a08g045-cpg.h. The RZ/G2L CPG > driver can handle the mapping from <id> to MSTOP register and bitmask. > This solution requires updates to DT, but you can keep compatibility > with old DTBs by only registering the new PM Domains when > #power-domain-cells is one. > The extra power saving would only be applicable with new DTBs, though. I prefer this alternative even though it cannot be applied for old DTBs, it looks to me that is more modular. What do you think? The only thing is that MSTOP is not really a power off/on switch (if it would be implemented with generic_pm_domain.power_{on, off}) but is more like a clock disable/enable functionality (it should not be an issue though, just saying)... According to manual (I'm referring to Figure 41.4 Block Connection Overview for Module Standby Mode of HW manula of RZ/G3S), it disables/enables the module's bus clock. Thank you, Claudiu Beznea > > Thoughts? > >>> --- a/drivers/clk/renesas/rzg2l-cpg.h >>> +++ b/drivers/clk/renesas/rzg2l-cpg.h >> >>> @@ -68,6 +73,10 @@ >>> #define SEL_PLL6_2 SEL_PLL_PACK(CPG_PL6_ETH_SSEL, 0, 1) >>> #define SEL_GPU2 SEL_PLL_PACK(CPG_PL6_SSEL, 12, 1) >>> >>> +#define MSTOP(name, bitmask) ((CPG_##name##_MSTOP) << 16 | (bitmask)) >> >> I believe the bitmask is always a single bit. >> So perhaps let MSTOP() take the bit number instead of the bitmaskl? >> You can still store BIT(bit) inside the macro. > > I was wrong, the N->N or N->M cases need a bitmask. > >>> +#define MSTOP_OFF(conf) ((conf) >> 16) >>> +#define MSTOP_MASK(conf) ((conf) & GENMASK(15, 0)) >>> + >>> #define EXTAL_FREQ_IN_MEGA_HZ (24) > > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 03/14] clk: renesas: rzg2l-cpg: Add support for MSTOP 2023-11-27 7:37 ` claudiu beznea @ 2023-12-01 15:36 ` Geert Uytterhoeven 0 siblings, 0 replies; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-01 15:36 UTC (permalink / raw) To: claudiu beznea Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Mon, Nov 27, 2023 at 8:37 AM claudiu beznea <claudiu.beznea@tuxon.dev> wrote: > On 24.11.2023 11:08, Geert Uytterhoeven wrote: > > On Thu, Nov 23, 2023 at 5:35 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > >> On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > >>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >>> > >>> RZ/{G2L, V2L, G3S} based CPG versions have support for saving extra > >>> power when clocks are disabled by activating module standby. This is done > >>> though MSTOP specific registers that are part of CPG. Each individual > >>> module have one or more bits associated in one MSTOP register (see table > >>> "Registers for Module Standby Mode" from HW manuals). Hardware manual > >>> associates modules' clocks to one or more MSTOP bits. There are 3 mappings > >>> available (identified by researching RZ/G2L, RZ/G3S, RZ/V2L HW manuals): > >>> > >>> case 1: N clocks mapped to N MSTOP bits (with N={0, ..., X}) > >>> case 2: N clocks mapped to 1 MSTOP bit (with N={0, ..., X}) > >>> case 3: N clocks mapped to M MSTOP bits (with N={0, ..., X}, M={0, ..., Y}) > >>> > >>> Case 3 has been currently identified on RZ/V2L for VCPL4 module. > >>> > >>> To cover all 3 cases the individual platform drivers will provide to > >>> clock driver MSTOP register offset and associated bits in this register > >>> as a bitmask and the clock driver will apply this bitmask to proper > >>> MSTOP register. > >>> > >>> As most of the modules have more than one clock and these clocks are > >>> mapped to 1 MSTOP bitmap that need to be applied to MSTOP registers, > >>> to avoid switching the module to/out of standby when the module has > >>> enabled/disabled clocks a counter has been associated to each module > >>> (though struct mstop::count) which is incremented/decremented every > >>> time a module's clock is enabled/disabled and the settings to MSTOP > >>> register are applied only when the counter reaches zero (counter zero > >>> means either 1st clock of the module is going to be enabled or all clocks > >>> of the module are going to be disabled). > > After giving this some more thought, it feels odd to derive the standby > > state of a module from the state of its module clocks, while the latter > > are already controlled through Runtime PM and a Clock Domain. > > > > A first alternative solution could be to drop the GENPD_FLAG_PM_CLK > > flag from the RZ/G2L CPG clock domain, and provide your own > > gpd_dev_ops.start() and .stop() callbacks that take care of both > > module standby and clocks (through pm_clk_{resume,suspend}(). > > (See https://elixir.bootlin.com/linux/v6.7-rc2/source/drivers/base/power/domain.c#L2093 > > for the GENPD_FLAG_PM_CLK case). > > That still leaves you with a need to associate an MSTOP register and > > bitmask with a device through its module clocks. > > > > A second alternative solution could be to increase #power-domain-cells > > from zero to one, and register individual PM Domains for each module, > > and control module standby from the generic_pm_domain.power_{on,off}() > > callbacks. Devices would specify the module using the power-domains = > > <&cpg <id> > property in DT, with <id> one of the to-be-added list of > > modules in include/dt-bindings/clock/r9a08g045-cpg.h. The RZ/G2L CPG > > driver can handle the mapping from <id> to MSTOP register and bitmask. > > This solution requires updates to DT, but you can keep compatibility > > with old DTBs by only registering the new PM Domains when > > #power-domain-cells is one. > > The extra power saving would only be applicable with new DTBs, though. > > I prefer this alternative even though it cannot be applied for old DTBs, it > looks to me that is more modular. What do you think? I prefer the second alternative, too. > The only thing is that MSTOP is not really a power off/on switch (if it > would be implemented with generic_pm_domain.power_{on, off}) but is more That's fine: Linux' PM Domains are fairly generic and abstract, and not limited to pure power domains/areas. > like a clock disable/enable functionality (it should not be an issue > though, just saying)... According to manual (I'm referring to Figure 41.4 > Block Connection Overview for Module Standby Mode of HW manula of RZ/G3S), > it disables/enables the module's bus clock. Thanks for the pointer! That picture nicely shows the internal behavior. For comparison, on SH/R-Mobile and R-Car SoCs there is a similar internal structure, but it is less visible to the programmer: there are no individual controls for each clock or reset that is fed into a module. These are all hidden behind a single Module Stop resp. Reset control bit. In Linux, we modeled the module stop bit as a gate clock, controlled by Runtime PM through the Clock Domain's .start()/.stop() callbacks. Note that you also have to take into account Figure 41.2 ("Modules in Power Domain"). When adding support for power transitions later, you can register a PM Domain representing PD_ISOVCC, and use that as the parent PM Domain for the individual PM Domains for modules belonging to PD_ISOVCC. All of that can be handled in the driver, and would not need any changes to DT. 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 03/14] clk: renesas: rzg2l-cpg: Add support for MSTOP 2023-11-23 16:35 ` Geert Uytterhoeven 2023-11-24 9:08 ` Geert Uytterhoeven @ 2023-11-24 9:24 ` claudiu beznea 1 sibling, 0 replies; 53+ messages in thread From: claudiu beznea @ 2023-11-24 9:24 UTC (permalink / raw) To: Geert Uytterhoeven Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi, Geert, On 23.11.2023 18:35, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> RZ/{G2L, V2L, G3S} based CPG versions have support for saving extra >> power when clocks are disabled by activating module standby. This is done >> though MSTOP specific registers that are part of CPG. Each individual >> module have one or more bits associated in one MSTOP register (see table >> "Registers for Module Standby Mode" from HW manuals). Hardware manual >> associates modules' clocks to one or more MSTOP bits. There are 3 mappings >> available (identified by researching RZ/G2L, RZ/G3S, RZ/V2L HW manuals): >> >> case 1: N clocks mapped to N MSTOP bits (with N={0, ..., X}) >> case 2: N clocks mapped to 1 MSTOP bit (with N={0, ..., X}) >> case 3: N clocks mapped to M MSTOP bits (with N={0, ..., X}, M={0, ..., Y}) >> >> Case 3 has been currently identified on RZ/V2L for VCPL4 module. >> >> To cover all 3 cases the individual platform drivers will provide to >> clock driver MSTOP register offset and associated bits in this register >> as a bitmask and the clock driver will apply this bitmask to proper >> MSTOP register. >> >> As most of the modules have more than one clock and these clocks are >> mapped to 1 MSTOP bitmap that need to be applied to MSTOP registers, >> to avoid switching the module to/out of standby when the module has >> enabled/disabled clocks a counter has been associated to each module >> (though struct mstop::count) which is incremented/decremented every >> time a module's clock is enabled/disabled and the settings to MSTOP >> register are applied only when the counter reaches zero (counter zero >> means either 1st clock of the module is going to be enabled or all clocks >> of the module are going to be disabled). > > Thanks for your patch! > >> The MSTOP functionality has been instantiated at the moment for RZ/G3S. > > Do you plan to add support for the other SoCs, too? Yes. > >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >> --- a/drivers/clk/renesas/r9a08g045-cpg.c >> +++ b/drivers/clk/renesas/r9a08g045-cpg.c >> @@ -187,23 +187,39 @@ static const struct cpg_core_clk r9a08g045_core_clks[] __initconst = { >> }; >> >> static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = { >> - DEF_MOD("gic_gicclk", R9A08G045_GIC600_GICCLK, R9A08G045_CLK_P1, 0x514, 0), >> - DEF_MOD("ia55_clk", R9A08G045_IA55_CLK, R9A08G045_CLK_P1, 0x518, 1), >> - DEF_MOD("dmac_aclk", R9A08G045_DMAC_ACLK, R9A08G045_CLK_P3, 0x52c, 0), >> - DEF_MOD("sdhi0_imclk", R9A08G045_SDHI0_IMCLK, CLK_SD0_DIV4, 0x554, 0), >> - DEF_MOD("sdhi0_imclk2", R9A08G045_SDHI0_IMCLK2, CLK_SD0_DIV4, 0x554, 1), >> - DEF_MOD("sdhi0_clk_hs", R9A08G045_SDHI0_CLK_HS, R9A08G045_CLK_SD0, 0x554, 2), >> - DEF_MOD("sdhi0_aclk", R9A08G045_SDHI0_ACLK, R9A08G045_CLK_P1, 0x554, 3), >> - DEF_MOD("sdhi1_imclk", R9A08G045_SDHI1_IMCLK, CLK_SD1_DIV4, 0x554, 4), >> - DEF_MOD("sdhi1_imclk2", R9A08G045_SDHI1_IMCLK2, CLK_SD1_DIV4, 0x554, 5), >> - DEF_MOD("sdhi1_clk_hs", R9A08G045_SDHI1_CLK_HS, R9A08G045_CLK_SD1, 0x554, 6), >> - DEF_MOD("sdhi1_aclk", R9A08G045_SDHI1_ACLK, R9A08G045_CLK_P1, 0x554, 7), >> - DEF_MOD("sdhi2_imclk", R9A08G045_SDHI2_IMCLK, CLK_SD2_DIV4, 0x554, 8), >> - DEF_MOD("sdhi2_imclk2", R9A08G045_SDHI2_IMCLK2, CLK_SD2_DIV4, 0x554, 9), >> - DEF_MOD("sdhi2_clk_hs", R9A08G045_SDHI2_CLK_HS, R9A08G045_CLK_SD2, 0x554, 10), >> - DEF_MOD("sdhi2_aclk", R9A08G045_SDHI2_ACLK, R9A08G045_CLK_P1, 0x554, 11), >> - DEF_MOD("scif0_clk_pck", R9A08G045_SCIF0_CLK_PCK, R9A08G045_CLK_P0, 0x584, 0), >> - DEF_MOD("gpio_hclk", R9A08G045_GPIO_HCLK, R9A08G045_OSCCLK, 0x598, 0), >> + DEF_MOD("gic_gicclk", R9A08G045_GIC600_GICCLK, R9A08G045_CLK_P1, 0x514, 0, >> + MSTOP(ACPU, BIT(3))), > > According to Rev. 1.00 of the Hardware User's Manual, bit 3 of the > CPG_BUS_ACPU_MSTOP register is reserved? Hm... you're right. I've followed table 44.4 Registers for Module Standby Mode to populate MSTOPs in r9a08g045_mod_clks[]. That table indicates bit 3 for GIC. > > Also, gic_gicclk is a critical module clock, so I guess this module > must never be put into standby? Good point. I'll remove the MSTOPs for critical clocks. > >> --- a/drivers/clk/renesas/rzg2l-cpg.c >> +++ b/drivers/clk/renesas/rzg2l-cpg.c >> @@ -1177,6 +1177,17 @@ rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core, >> core->name, PTR_ERR(clk)); >> } >> >> +/** >> + * struct mstop - MSTOP specific data structure >> + * @count: reference counter for MSTOP settings (when zero the settings >> + * are applied to register) >> + * @conf: MSTOP configuration (register offset, setup bits) >> + */ >> +struct mstop { >> + u32 count; >> + u32 conf; >> +}; >> + >> /** >> * struct mstp_clock - MSTP gating clock >> * >> @@ -1186,6 +1197,7 @@ rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core, >> * @enabled: soft state of the clock, if it is coupled with another clock >> * @priv: CPG/MSTP private data >> * @sibling: pointer to the other coupled clock >> + * @mstop: MSTOP configuration >> */ >> struct mstp_clock { >> struct clk_hw hw; >> @@ -1194,10 +1206,46 @@ struct mstp_clock { >> bool enabled; >> struct rzg2l_cpg_priv *priv; >> struct mstp_clock *sibling; >> + struct mstop *mstop; >> }; >> >> #define to_mod_clock(_hw) container_of(_hw, struct mstp_clock, hw) >> >> +/* Need to be called with a lock held to avoid concurent access to mstop->count. */ > > concurrent > >> +static void rzg2l_mod_clock_module_set_standby(struct mstp_clock *clock, >> + bool standby) >> +{ >> + struct rzg2l_cpg_priv *priv = clock->priv; >> + struct mstop *mstop = clock->mstop; >> + bool update = false; >> + u32 value; >> + >> + if (!mstop) >> + return; >> + >> + value = MSTOP_MASK(mstop->conf) << 16; >> + >> + if (standby) { >> + value |= MSTOP_MASK(mstop->conf); >> + /* Avoid overflow. */ >> + if (mstop->count > 0) >> + mstop->count--; > > Should we add a WARN() here, or is it sufficient to rely on the WARN() > in drivers/clk/clk.c:clk_core_disable()? I think it would be good to have it as mstop->count could be incremented/decremented by more than one clock and could overflow faster than struct clk_core::enable_count > >> + >> + if (!mstop->count) >> + update = true; >> + } else { >> + if (!mstop->count) >> + update = true; >> + >> + /* Avoid overflow. */ >> + if (mstop->count + 1 != 0) >> + mstop->count++; > > Trying to avoid an overflow won't help much here. The counter > will be wrong afterwards anyway, and when decrementing again later, the > module will be put in standby too soon... That's true. Would you prefer to have a WARN() for this too? > >> + } >> + >> + if (update) >> + writel(value, priv->base + MSTOP_OFF(mstop->conf)); >> +} >> + >> static int rzg2l_mod_clock_endisable(struct clk_hw *hw, bool enable) >> { >> struct mstp_clock *clock = to_mod_clock(hw); > >> @@ -1401,6 +1474,37 @@ rzg2l_cpg_register_mod_clk(const struct rzg2l_mod_clk *mod, >> } >> } >> >> + if (mod->mstop_conf) { >> + struct mstop *mstop = rzg2l_mod_clock_get_mstop(priv, mod->mstop_conf); >> + >> + if (mstop) { >> + clock->mstop = mstop; > > Please move the common assignment after the if/else block... > >> + } else { > > ... so this can just become "if (!mstop) {". Ok, I'll review it. > >> + mstop = devm_kzalloc(dev, sizeof(*mstop), GFP_KERNEL); >> + if (!mstop) { >> + clk_unregister(clk); >> + goto fail; > > Please use "goto unregister", and call clk_unregister() after the new > unregister label. I kept it like this as I considered otherwise the error path might become unnecessary complicated. > >> + } >> + >> + mstop->conf = mod->mstop_conf; >> + clock->mstop = mstop; >> + } >> + >> + if (rzg2l_mod_clock_is_enabled(&clock->hw)) { >> + if (clock->sibling) >> + clock->mstop->count = 1; >> + else >> + clock->mstop->count++; >> + } >> + >> + /* >> + * Out of reset all modules are enabled. Set module to standby >> + * in case associated clocks are disabled at probe. > > Is that always true? > What about kexec and crashdump kernels? I was referring to the hardware reset. In case we reach this point with clocks already enabled by a previous kernel the state of the clocks in hardware should be enabled and the mstop->count should be updated accordingly by the above if block. Let me know if I'm missing something. > >> + */ >> + if (!clock->mstop->count) >> + rzg2l_mod_clock_module_set_standby(clock, true); >> + } >> + >> return; >> >> fail: >> diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h >> index 6e38c8fc888c..10ee8aa4a5da 100644 >> --- a/drivers/clk/renesas/rzg2l-cpg.h >> +++ b/drivers/clk/renesas/rzg2l-cpg.h > >> @@ -68,6 +73,10 @@ >> #define SEL_PLL6_2 SEL_PLL_PACK(CPG_PL6_ETH_SSEL, 0, 1) >> #define SEL_GPU2 SEL_PLL_PACK(CPG_PL6_SSEL, 12, 1) >> >> +#define MSTOP(name, bitmask) ((CPG_##name##_MSTOP) << 16 | (bitmask)) > > I believe the bitmask is always a single bit. > So perhaps let MSTOP() take the bit number instead of the bitmaskl? > You can still store BIT(bit) inside the macro. It is not always the case. That is why I've added the bitmask. The identified scenarios are highlighted in commit description: case 1: N clocks mapped to N MSTOP bits (with N={0, ..., X}) case 2: N clocks mapped to 1 MSTOP bit (with N={0, ..., X}) case 3: N clocks mapped to M MSTOP bits (with N={0, ..., X}, M={0, ..., Y}) Thank you for your review, Claudiu Beznea > >> +#define MSTOP_OFF(conf) ((conf) >> 16) >> +#define MSTOP_MASK(conf) ((conf) & GENMASK(15, 0)) >> + >> #define EXTAL_FREQ_IN_MEGA_HZ (24) >> >> /** > > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 04/14] clk: renesas: r9a08g045-cpg: Add clock and reset support for ETH0 and ETH1 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu ` (2 preceding siblings ...) 2023-11-20 7:00 ` [PATCH 03/14] clk: renesas: rzg2l-cpg: Add support for MSTOP Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-12-01 15:59 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 05/14] pinctrl: renesas: rzg2l: Move arg in the main function block Claudiu ` (10 subsequent siblings) 14 siblings, 1 reply; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> RZ/G3S has 2 Gigabit Ethernet interfaces available. Add clock and reset support for both of them. Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- drivers/clk/renesas/r9a08g045-cpg.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/clk/renesas/r9a08g045-cpg.c b/drivers/clk/renesas/r9a08g045-cpg.c index 6ff40763a00a..e78dbc74df27 100644 --- a/drivers/clk/renesas/r9a08g045-cpg.c +++ b/drivers/clk/renesas/r9a08g045-cpg.c @@ -181,9 +181,11 @@ static const struct cpg_core_clk r9a08g045_core_clks[] __initconst = { DEF_G3S_DIV("P3", R9A08G045_CLK_P3, CLK_PLL3_DIV2_4, DIVPL3C, G3S_DIVPL3C_STS, dtable_1_32, 0, 0, 0, NULL), DEF_FIXED("P3_DIV2", CLK_P3_DIV2, R9A08G045_CLK_P3, 1, 2), + DEF_FIXED("ZT", R9A08G045_CLK_ZT, CLK_PLL3_DIV2_8, 1, 1), DEF_FIXED("S0", R9A08G045_CLK_S0, CLK_SEL_PLL4, 1, 2), DEF_FIXED("OSC", R9A08G045_OSCCLK, CLK_EXTAL, 1, 1), DEF_FIXED("OSC2", R9A08G045_OSCCLK2, CLK_EXTAL, 1, 3), + DEF_FIXED("HP", R9A08G045_CLK_HP, CLK_PLL6, 1, 2), }; static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = { @@ -217,6 +219,16 @@ static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = { MSTOP(PERI_COM, BIT(11))), DEF_MOD("sdhi2_aclk", R9A08G045_SDHI2_ACLK, R9A08G045_CLK_P1, 0x554, 11, MSTOP(PERI_COM, BIT(11))), + DEF_COUPLED("eth0_axi", R9A08G045_ETH0_CLK_AXI, R9A08G045_CLK_M0, 0x57c, 0, + MSTOP(PERI_COM, BIT(2))), + DEF_COUPLED("eth0_chi", R9A08G045_ETH0_CLK_CHI, R9A08G045_CLK_ZT, 0x57c, 0, + MSTOP(PERI_COM, BIT(2))), + DEF_MOD("eth0_refclk", R9A08G045_ETH0_REFCLK, R9A08G045_CLK_HP, 0x57c, 8, 0), + DEF_COUPLED("eth1_axi", R9A08G045_ETH1_CLK_AXI, R9A08G045_CLK_M0, 0x57c, 1, + MSTOP(PERI_COM, BIT(3))), + DEF_COUPLED("eth1_chi", R9A08G045_ETH1_CLK_CHI, R9A08G045_CLK_ZT, 0x57c, 1, + MSTOP(PERI_COM, BIT(3))), + DEF_MOD("eth1_refclk", R9A08G045_ETH1_REFCLK, R9A08G045_CLK_HP, 0x57c, 9, 0), DEF_MOD("scif0_clk_pck", R9A08G045_SCIF0_CLK_PCK, R9A08G045_CLK_P0, 0x584, 0, MSTOP(MCPU2, BIT(1))), DEF_MOD("gpio_hclk", R9A08G045_GPIO_HCLK, R9A08G045_OSCCLK, 0x598, 0, 0), @@ -228,6 +240,8 @@ static const struct rzg2l_reset r9a08g045_resets[] = { DEF_RST(R9A08G045_SDHI0_IXRST, 0x854, 0), DEF_RST(R9A08G045_SDHI1_IXRST, 0x854, 1), DEF_RST(R9A08G045_SDHI2_IXRST, 0x854, 2), + DEF_RST(R9A08G045_ETH0_RST_HW_N, 0x87c, 0), + DEF_RST(R9A08G045_ETH1_RST_HW_N, 0x87c, 1), DEF_RST(R9A08G045_SCIF0_RST_SYSTEM_N, 0x884, 0), DEF_RST(R9A08G045_GPIO_RSTN, 0x898, 0), DEF_RST(R9A08G045_GPIO_PORT_RESETN, 0x898, 1), -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 04/14] clk: renesas: r9a08g045-cpg: Add clock and reset support for ETH0 and ETH1 2023-11-20 7:00 ` [PATCH 04/14] clk: renesas: r9a08g045-cpg: Add clock and reset support for ETH0 and ETH1 Claudiu @ 2023-12-01 15:59 ` Geert Uytterhoeven 2023-12-04 7:34 ` claudiu beznea 0 siblings, 1 reply; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-01 15:59 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > RZ/G3S has 2 Gigabit Ethernet interfaces available. Add clock and reset > support for both of them. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks for your patch! > --- a/drivers/clk/renesas/r9a08g045-cpg.c > +++ b/drivers/clk/renesas/r9a08g045-cpg.c > @@ -217,6 +219,16 @@ static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = { > MSTOP(PERI_COM, BIT(11))), > DEF_MOD("sdhi2_aclk", R9A08G045_SDHI2_ACLK, R9A08G045_CLK_P1, 0x554, 11, > MSTOP(PERI_COM, BIT(11))), > + DEF_COUPLED("eth0_axi", R9A08G045_ETH0_CLK_AXI, R9A08G045_CLK_M0, 0x57c, 0, > + MSTOP(PERI_COM, BIT(2))), > + DEF_COUPLED("eth0_chi", R9A08G045_ETH0_CLK_CHI, R9A08G045_CLK_ZT, 0x57c, 0, > + MSTOP(PERI_COM, BIT(2))), > + DEF_MOD("eth0_refclk", R9A08G045_ETH0_REFCLK, R9A08G045_CLK_HP, 0x57c, 8, 0), > + DEF_COUPLED("eth1_axi", R9A08G045_ETH1_CLK_AXI, R9A08G045_CLK_M0, 0x57c, 1, > + MSTOP(PERI_COM, BIT(3))), > + DEF_COUPLED("eth1_chi", R9A08G045_ETH1_CLK_CHI, R9A08G045_CLK_ZT, 0x57c, 1, > + MSTOP(PERI_COM, BIT(3))), > + DEF_MOD("eth1_refclk", R9A08G045_ETH1_REFCLK, R9A08G045_CLK_HP, 0x57c, 9, 0), > DEF_MOD("scif0_clk_pck", R9A08G045_SCIF0_CLK_PCK, R9A08G045_CLK_P0, 0x584, 0, > MSTOP(MCPU2, BIT(1))), > DEF_MOD("gpio_hclk", R9A08G045_GPIO_HCLK, R9A08G045_OSCCLK, 0x598, 0, 0), LGTM, pending the MSTOP() part. Is the MSTOP() handling needed to function? IIUIC, all modules are enabled out of reset. If it is not needed, I can take this patch and remove the MSTOP() part. 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 04/14] clk: renesas: r9a08g045-cpg: Add clock and reset support for ETH0 and ETH1 2023-12-01 15:59 ` Geert Uytterhoeven @ 2023-12-04 7:34 ` claudiu beznea 0 siblings, 0 replies; 53+ messages in thread From: claudiu beznea @ 2023-12-04 7:34 UTC (permalink / raw) To: Geert Uytterhoeven Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi, Geert, On 01.12.2023 17:59, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> RZ/G3S has 2 Gigabit Ethernet interfaces available. Add clock and reset >> support for both of them. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Thanks for your patch! > >> --- a/drivers/clk/renesas/r9a08g045-cpg.c >> +++ b/drivers/clk/renesas/r9a08g045-cpg.c >> @@ -217,6 +219,16 @@ static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = { >> MSTOP(PERI_COM, BIT(11))), >> DEF_MOD("sdhi2_aclk", R9A08G045_SDHI2_ACLK, R9A08G045_CLK_P1, 0x554, 11, >> MSTOP(PERI_COM, BIT(11))), >> + DEF_COUPLED("eth0_axi", R9A08G045_ETH0_CLK_AXI, R9A08G045_CLK_M0, 0x57c, 0, >> + MSTOP(PERI_COM, BIT(2))), >> + DEF_COUPLED("eth0_chi", R9A08G045_ETH0_CLK_CHI, R9A08G045_CLK_ZT, 0x57c, 0, >> + MSTOP(PERI_COM, BIT(2))), >> + DEF_MOD("eth0_refclk", R9A08G045_ETH0_REFCLK, R9A08G045_CLK_HP, 0x57c, 8, 0), >> + DEF_COUPLED("eth1_axi", R9A08G045_ETH1_CLK_AXI, R9A08G045_CLK_M0, 0x57c, 1, >> + MSTOP(PERI_COM, BIT(3))), >> + DEF_COUPLED("eth1_chi", R9A08G045_ETH1_CLK_CHI, R9A08G045_CLK_ZT, 0x57c, 1, >> + MSTOP(PERI_COM, BIT(3))), >> + DEF_MOD("eth1_refclk", R9A08G045_ETH1_REFCLK, R9A08G045_CLK_HP, 0x57c, 9, 0), >> DEF_MOD("scif0_clk_pck", R9A08G045_SCIF0_CLK_PCK, R9A08G045_CLK_P0, 0x584, 0, >> MSTOP(MCPU2, BIT(1))), >> DEF_MOD("gpio_hclk", R9A08G045_GPIO_HCLK, R9A08G045_OSCCLK, 0x598, 0, 0), > > LGTM, pending the MSTOP() part. > > Is the MSTOP() handling needed to function? IIUIC, all modules are > enabled > out of reset. MSTOP is not needed for Ethernet to work. Indeed, all modules are enabled out of reset. > If it is not needed, I can take this patch and remove the MSTOP() part. It's OK for me. Thank you for handling this. Anyway, let me know if you encounter issues with it so I can resend it. Thank you, Claudiu Beznea > > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 05/14] pinctrl: renesas: rzg2l: Move arg in the main function block 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu ` (3 preceding siblings ...) 2023-11-20 7:00 ` [PATCH 04/14] clk: renesas: r9a08g045-cpg: Add clock and reset support for ETH0 and ETH1 Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-12-01 16:15 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 06/14] pinctrl: renesas: rzg2l: Add pin configuration support for pinmux groups Claudiu ` (9 subsequent siblings) 14 siblings, 1 reply; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Move arg in the main block of the function as this is used by 3 out of 4 case blocks of switch-case. In this way some lines of code are removed. Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- drivers/pinctrl/renesas/pinctrl-rzg2l.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index 9de350ad7e7d..21ee628363fa 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -842,7 +842,7 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev, struct rzg2l_pinctrl_pin_settings settings = pctrl->settings[_pin]; unsigned int *pin_data = pin->drv_data; enum pin_config_param param; - unsigned int i; + unsigned int i, arg; u32 cfg, off; int ret; u8 bit; @@ -865,8 +865,7 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev, param = pinconf_to_config_param(_configs[i]); switch (param) { case PIN_CONFIG_INPUT_ENABLE: { - unsigned int arg = - pinconf_to_config_argument(_configs[i]); + arg = pinconf_to_config_argument(_configs[i]); if (!(cfg & PIN_CFG_IEN)) return -EINVAL; @@ -880,9 +879,10 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev, break; case PIN_CONFIG_DRIVE_STRENGTH: { - unsigned int arg = pinconf_to_config_argument(_configs[i]); unsigned int index; + arg = pinconf_to_config_argument(_configs[i]); + if (!(cfg & PIN_CFG_IOLH_A) || hwcfg->drive_strength_ua) return -EINVAL; @@ -907,9 +907,10 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev, break; case PIN_CONFIG_OUTPUT_IMPEDANCE_OHMS: { - unsigned int arg = pinconf_to_config_argument(_configs[i]); unsigned int index; + arg = pinconf_to_config_argument(_configs[i]); + if (!(cfg & PIN_CFG_IOLH_B) || !hwcfg->iolh_groupb_oi[0]) return -EINVAL; -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 05/14] pinctrl: renesas: rzg2l: Move arg in the main function block 2023-11-20 7:00 ` [PATCH 05/14] pinctrl: renesas: rzg2l: Move arg in the main function block Claudiu @ 2023-12-01 16:15 ` Geert Uytterhoeven 2023-12-04 7:37 ` claudiu beznea 0 siblings, 1 reply; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-01 16:15 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Move arg in the main block of the function as this is used by 3 out of 4 > case blocks of switch-case. In this way some lines of code are removed. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks for your patch! > drivers/pinctrl/renesas/pinctrl-rzg2l.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) Unfortunately your claim is not really backed by the diffstat. What about moving index, too? 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 05/14] pinctrl: renesas: rzg2l: Move arg in the main function block 2023-12-01 16:15 ` Geert Uytterhoeven @ 2023-12-04 7:37 ` claudiu beznea 0 siblings, 0 replies; 53+ messages in thread From: claudiu beznea @ 2023-12-04 7:37 UTC (permalink / raw) To: Geert Uytterhoeven Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea On 01.12.2023 18:15, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> Move arg in the main block of the function as this is used by 3 out of 4 >> case blocks of switch-case. In this way some lines of code are removed. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Thanks for your patch! > >> drivers/pinctrl/renesas/pinctrl-rzg2l.c | 11 ++++++----- >> 1 file changed, 6 insertions(+), 5 deletions(-) > > Unfortunately your claim is not really backed by the diffstat. > What about moving index, too? Sure, I can move it, too. > > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 06/14] pinctrl: renesas: rzg2l: Add pin configuration support for pinmux groups 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu ` (4 preceding siblings ...) 2023-11-20 7:00 ` [PATCH 05/14] pinctrl: renesas: rzg2l: Move arg in the main function block Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-12-01 16:51 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 07/14] pinctrl: renesas: rzg2l: Add support to select power source for Ethernet pins Claudiu ` (8 subsequent siblings) 14 siblings, 1 reply; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> On RZ/G3S different Ethernet pins needs to be configured with different settings (e.g. power-source need to be set, RGMII TXC, TX_CTL pins need output-enable). Commit adjust driver to allow specifying pin configuration for pinmux groups. With this DT settings like the following are taken into account by driver: eth0_pins: eth0 { tx_ctl { pinmux = <RZG2L_PORT_PINMUX(1, 1, 1)>; /* ET0_TX_CTL */ power-source = <1800>; output-enable; drive-strength-microamp = <5200>; }; }; Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- drivers/pinctrl/renesas/pinctrl-rzg2l.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index 21ee628363fa..819698dacef0 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -376,8 +376,11 @@ static int rzg2l_dt_subnode_to_map(struct pinctrl_dev *pctldev, goto done; } - if (num_pinmux) + if (num_pinmux) { nmaps += 1; + if (num_configs) + nmaps += 1; + } if (num_pins) nmaps += num_pins; @@ -462,6 +465,16 @@ static int rzg2l_dt_subnode_to_map(struct pinctrl_dev *pctldev, maps[idx].data.mux.function = name; idx++; + if (num_configs) { + ret = rzg2l_map_add_config(&maps[idx], name, + PIN_MAP_TYPE_CONFIGS_GROUP, + configs, num_configs); + if (ret < 0) + goto remove_group; + + idx++; + }; + dev_dbg(pctrl->dev, "Parsed %pOF with %d pins\n", np, num_pinmux); ret = 0; goto done; -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 06/14] pinctrl: renesas: rzg2l: Add pin configuration support for pinmux groups 2023-11-20 7:00 ` [PATCH 06/14] pinctrl: renesas: rzg2l: Add pin configuration support for pinmux groups Claudiu @ 2023-12-01 16:51 ` Geert Uytterhoeven 0 siblings, 0 replies; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-01 16:51 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > On RZ/G3S different Ethernet pins needs to be configured with different > settings (e.g. power-source need to be set, RGMII TXC, TX_CTL pins need > output-enable). Commit adjust driver to allow specifying pin configuration > for pinmux groups. With this DT settings like the following are taken > into account by driver: > > eth0_pins: eth0 { > tx_ctl { > pinmux = <RZG2L_PORT_PINMUX(1, 1, 1)>; /* ET0_TX_CTL */ > power-source = <1800>; > output-enable; > drive-strength-microamp = <5200>; > }; > }; > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks for your patch! > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > @@ -376,8 +376,11 @@ static int rzg2l_dt_subnode_to_map(struct pinctrl_dev *pctldev, > goto done; > } > > - if (num_pinmux) > + if (num_pinmux) { > nmaps += 1; > + if (num_configs) > + nmaps += 1; I think this would be more readable, and better follow the style of the surrounding statements, if this new check would not be nested under the num_pinmux check. > + } > > if (num_pins) > nmaps += num_pins; Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 07/14] pinctrl: renesas: rzg2l: Add support to select power source for Ethernet pins 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu ` (5 preceding siblings ...) 2023-11-20 7:00 ` [PATCH 06/14] pinctrl: renesas: rzg2l: Add pin configuration support for pinmux groups Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-12-01 17:11 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 08/14] pinctrl: renesas: rzg2l: Add output enable support Claudiu ` (7 subsequent siblings) 14 siblings, 1 reply; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> For Ethernet pins GPIO controller available on RZ/G3S (but also on RZ/G2L) allows setting the power source. Based on the interface b/w Ethernet controller and Ethernet PHY and board design specific power source need to be selected. The GPIO controller allow 1.8V, 2.5V and 3.3V power source selection for Ethernet pins and this could be selected though ETHX_POC registers (X={0, 1}). Commit adjust the driver to support this and does proper instantiation for RZ/G3S and RZ/G2L SoC. On RZ/G2L only get operation has been tested at the moment. Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- drivers/pinctrl/renesas/pinctrl-rzg2l.c | 34 +++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index 819698dacef0..1401bb215686 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -107,8 +107,10 @@ #define IEN(off) (0x1800 + (off) * 8) #define ISEL(off) (0x2C00 + (off) * 8) #define SD_CH(off, ch) ((off) + (ch) * 4) +#define ETH_POC(off, ch) ((off) + (ch) * 4) #define QSPI (0x3008) +#define PVDD_2500 2 /* I/O domain voltage 2.5V */ #define PVDD_1800 1 /* I/O domain voltage <= 1.8V */ #define PVDD_3300 0 /* I/O domain voltage >= 3.3V */ @@ -135,10 +137,12 @@ * struct rzg2l_register_offsets - specific register offsets * @pwpr: PWPR register offset * @sd_ch: SD_CH register offset + * @eth_poc: ETH_POC register offset */ struct rzg2l_register_offsets { u16 pwpr; u16 sd_ch; + u16 eth_poc; }; /** @@ -604,6 +608,10 @@ static int rzg2l_caps_to_pwr_reg(const struct rzg2l_register_offsets *regs, u32 return SD_CH(regs->sd_ch, 0); if (caps & PIN_CFG_IO_VMC_SD1) return SD_CH(regs->sd_ch, 1); + if (caps & PIN_CFG_IO_VMC_ETH0) + return ETH_POC(regs->eth_poc, 0); + if (caps & PIN_CFG_IO_VMC_ETH1) + return ETH_POC(regs->eth_poc, 1); if (caps & PIN_CFG_IO_VMC_QSPI) return QSPI; @@ -615,6 +623,7 @@ static int rzg2l_get_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; const struct rzg2l_register_offsets *regs = &hwcfg->regs; int pwr_reg; + u32 val; if (caps & PIN_CFG_SOFT_PS) return pctrl->settings[pin].power_source; @@ -623,7 +632,16 @@ static int rzg2l_get_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps if (pwr_reg < 0) return pwr_reg; - return (readl(pctrl->base + pwr_reg) & PVDD_MASK) ? 1800 : 3300; + val = readl(pctrl->base + pwr_reg); + if (val == PVDD_1800) + return 1800; + if (val == PVDD_2500) + return 2500; + if (val == PVDD_3300) + return 3300; + + /* Should not happen. */ + return -EINVAL; } static int rzg2l_set_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps, u32 ps) @@ -631,17 +649,27 @@ static int rzg2l_set_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; const struct rzg2l_register_offsets *regs = &hwcfg->regs; int pwr_reg; + u32 val; if (caps & PIN_CFG_SOFT_PS) { pctrl->settings[pin].power_source = ps; return 0; } + if (ps == 1800) + val = PVDD_1800; + else if (ps == 2500) + val = PVDD_2500; + else if (ps == 3300) + val = PVDD_3300; + else + return -EINVAL; + pwr_reg = rzg2l_caps_to_pwr_reg(regs, caps); if (pwr_reg < 0) return pwr_reg; - writel((ps == 1800) ? PVDD_1800 : PVDD_3300, pctrl->base + pwr_reg); + writel(val, pctrl->base + pwr_reg); pctrl->settings[pin].power_source = ps; return 0; @@ -1891,6 +1919,7 @@ static const struct rzg2l_hwcfg rzg2l_hwcfg = { .regs = { .pwpr = 0x3014, .sd_ch = 0x3000, + .eth_poc = 0x300c, }, .iolh_groupa_ua = { /* 3v3 power source */ @@ -1903,6 +1932,7 @@ static const struct rzg2l_hwcfg rzg3s_hwcfg = { .regs = { .pwpr = 0x3000, .sd_ch = 0x3004, + .eth_poc = 0x3010, }, .iolh_groupa_ua = { /* 1v8 power source */ -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 07/14] pinctrl: renesas: rzg2l: Add support to select power source for Ethernet pins 2023-11-20 7:00 ` [PATCH 07/14] pinctrl: renesas: rzg2l: Add support to select power source for Ethernet pins Claudiu @ 2023-12-01 17:11 ` Geert Uytterhoeven 0 siblings, 0 replies; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-01 17:11 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > For Ethernet pins GPIO controller available on RZ/G3S (but also on RZ/G2L) > allows setting the power source. Based on the interface b/w Ethernet > controller and Ethernet PHY and board design specific power source need to > be selected. The GPIO controller allow 1.8V, 2.5V and 3.3V power source > selection for Ethernet pins and this could be selected though ETHX_POC > registers (X={0, 1}). > > Commit adjust the driver to support this and does proper instantiation > for RZ/G3S and RZ/G2L SoC. On RZ/G2L only get operation has been tested > at the moment. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks for your patch! > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > @@ -623,7 +632,16 @@ static int rzg2l_get_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps > if (pwr_reg < 0) > return pwr_reg; > > - return (readl(pctrl->base + pwr_reg) & PVDD_MASK) ? 1800 : 3300; This removes the last user of PVDD_MASK. I guess it can be removed, as all unused register bits are documented to read as zeroes. > + val = readl(pctrl->base + pwr_reg); While these registers are documented to support access sizes of 8/16/32 bits on RZ/G3S, RZ/G2L is limited to 8 bits, so this should have used readb() from the beginning. > + if (val == PVDD_1800) > + return 1800; > + if (val == PVDD_2500) > + return 2500; > + if (val == PVDD_3300) > + return 3300; > + > + /* Should not happen. */ > + return -EINVAL; Please use a switch() statement. > } > > static int rzg2l_set_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps, u32 ps) > @@ -631,17 +649,27 @@ static int rzg2l_set_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps > const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; > const struct rzg2l_register_offsets *regs = &hwcfg->regs; > int pwr_reg; > + u32 val; > > if (caps & PIN_CFG_SOFT_PS) { > pctrl->settings[pin].power_source = ps; > return 0; > } > > + if (ps == 1800) > + val = PVDD_1800; > + else if (ps == 2500) > + val = PVDD_2500; > + else if (ps == 3300) > + val = PVDD_3300; > + else > + return -EINVAL; Please use a switch() statement. > + > pwr_reg = rzg2l_caps_to_pwr_reg(regs, caps); > if (pwr_reg < 0) > return pwr_reg; > > - writel((ps == 1800) ? PVDD_1800 : PVDD_3300, pctrl->base + pwr_reg); > + writel(val, pctrl->base + pwr_reg); writeb() for RZ/G2L. > pctrl->settings[pin].power_source = ps; > > return 0; 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 08/14] pinctrl: renesas: rzg2l: Add output enable support 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu ` (6 preceding siblings ...) 2023-11-20 7:00 ` [PATCH 07/14] pinctrl: renesas: rzg2l: Add support to select power source for Ethernet pins Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-12-01 17:25 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 09/14] dt-bindings: net: renesas,etheravb: Document RZ/G3S support Claudiu ` (6 subsequent siblings) 14 siblings, 1 reply; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Some of the Ethernet pins on RZ/G3S (but also valid for RZ/G2L) need to have direction of the IO buffer set as output for Ethernet to work properly. On RZ/G3S these pins are P1_0/P7_0, P1_1/P7_1 with could have the following Ethernet functions: TXC/TX_CLK or TX_CTL/TX_EN. To be able to configure this the output enable has been implemented. This is implemented with 2 per-platform read/write functions to be able to simply validate the pins supporting this on a platform basis. Moreover, on RZ/G2L the register though which these settings could be done is 8 bits long whereas on RZ/G3S this is a 32 bit register. The Ethernet pins supporting OEN are different. These differences could be handled in platform specific OEN read/write functions. Add support for OEN and enable it for RZ/G3S. Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- drivers/pinctrl/renesas/pinctrl-rzg2l.c | 106 +++++++++++++++++++++++- 1 file changed, 104 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index 1401bb215686..e942204e08a2 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -57,6 +57,7 @@ #define PIN_CFG_FILCLKSEL BIT(12) #define PIN_CFG_IOLH_C BIT(13) #define PIN_CFG_SOFT_PS BIT(14) +#define PIN_CFG_OEN BIT(15) #define RZG2L_MPXED_COMMON_PIN_FUNCS(group) \ (PIN_CFG_IOLH_##group | \ @@ -109,6 +110,7 @@ #define SD_CH(off, ch) ((off) + (ch) * 4) #define ETH_POC(off, ch) ((off) + (ch) * 4) #define QSPI (0x3008) +#define ETH_MODE (0x3018) #define PVDD_2500 2 /* I/O domain voltage 2.5V */ #define PVDD_1800 1 /* I/O domain voltage <= 1.8V */ @@ -195,6 +197,7 @@ struct rzg2l_pinctrl_data { unsigned int n_port_pins; unsigned int n_dedicated_pins; const struct rzg2l_hwcfg *hwcfg; + const struct rzg2l_cfg_ops *ops; }; /** @@ -228,6 +231,16 @@ struct rzg2l_pinctrl { struct rzg2l_pinctrl_pin_settings *settings; }; +/* + * struct rzg2l_cfg_ops - platform specific configuration ops + * @read_oen: read OEN function + * @write_oen: write OEN function + */ +struct rzg2l_cfg_ops { + u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin); + int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen); +}; + static const u16 available_ps[] = { 1800, 2500, 3300 }; static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl, @@ -776,6 +789,67 @@ static bool rzg2l_ds_is_supported(struct rzg2l_pinctrl *pctrl, u32 caps, return false; } +static bool rzg3s_oen_is_supported(u32 caps, u8 pin) +{ + if (!(caps & PIN_CFG_OEN)) + return false; + + /* + * Only pins 0 and 1 of P1 and P7 are supporting this thus add a simple + * check for this here. + */ + if (pin > 1) + return false; + + return true; +} + +static u8 rzg3s_pin_to_oen_bit(u32 offset, u8 pin) +{ + if (pin) + pin *= 2; + + if (offset / RZG2L_PINS_PER_PORT == 7) + pin += 1; + + return pin; +} + +static u32 rzg3s_read_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin) +{ + u8 bit; + + if (!rzg3s_oen_is_supported(caps, pin)) + return 0; + + bit = rzg3s_pin_to_oen_bit(offset, pin); + + return !(readl(pctrl->base + ETH_MODE) & BIT(bit)); +} + +static int rzg3s_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen) +{ + unsigned long flags; + u32 val; + u8 bit; + + if (!rzg3s_oen_is_supported(caps, pin)) + return -EINVAL; + + bit = rzg3s_pin_to_oen_bit(offset, pin); + + spin_lock_irqsave(&pctrl->lock, flags); + val = readl(pctrl->base + ETH_MODE); + if (oen) + val &= ~BIT(bit); + else + val |= BIT(bit); + writel(val, pctrl->base + ETH_MODE); + spin_unlock_irqrestore(&pctrl->lock, flags); + + return 0; +} + static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, unsigned int _pin, unsigned long *config) @@ -813,6 +887,16 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, return -EINVAL; break; + case PIN_CONFIG_OUTPUT_ENABLE: + if (!(pctrl->data->ops && pctrl->data->ops->read_oen)) + return -EINVAL; + + arg = pctrl->data->ops->read_oen(pctrl, cfg, _pin, bit); + if (!arg) + return -EINVAL; + + break; + case PIN_CONFIG_POWER_SOURCE: ret = rzg2l_get_power_source(pctrl, _pin, cfg); if (ret < 0) @@ -915,6 +999,16 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev, break; } + case PIN_CONFIG_OUTPUT_ENABLE: + if (!(pctrl->data->ops && pctrl->data->ops->write_oen)) + return -EINVAL; + + arg = pinconf_to_config_argument(_configs[i]); + ret = pctrl->data->ops->write_oen(pctrl, cfg, _pin, bit, !!arg); + if (ret) + return ret; + break; + case PIN_CONFIG_POWER_SOURCE: settings.power_source = pinconf_to_config_argument(_configs[i]); break; @@ -1365,7 +1459,8 @@ static const u32 r9a07g043_gpio_configs[] = { static const u32 r9a08g045_gpio_configs[] = { RZG2L_GPIO_PORT_PACK(4, 0x20, RZG3S_MPXED_PIN_FUNCS(A)), /* P0 */ RZG2L_GPIO_PORT_PACK(5, 0x30, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_C | - PIN_CFG_IO_VMC_ETH0)), /* P1 */ + PIN_CFG_IO_VMC_ETH0)) | + PIN_CFG_IEN | PIN_CFG_OEN, /* P1 */ RZG2L_GPIO_PORT_PACK(4, 0x31, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_C | PIN_CFG_IO_VMC_ETH0)), /* P2 */ RZG2L_GPIO_PORT_PACK(4, 0x32, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_C | @@ -1375,7 +1470,8 @@ static const u32 r9a08g045_gpio_configs[] = { RZG2L_GPIO_PORT_PACK(5, 0x21, RZG3S_MPXED_PIN_FUNCS(A)), /* P5 */ RZG2L_GPIO_PORT_PACK(5, 0x22, RZG3S_MPXED_PIN_FUNCS(A)), /* P6 */ RZG2L_GPIO_PORT_PACK(5, 0x34, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_C | - PIN_CFG_IO_VMC_ETH1)), /* P7 */ + PIN_CFG_IO_VMC_ETH1)) | + PIN_CFG_IEN | PIN_CFG_OEN, /* P7 */ RZG2L_GPIO_PORT_PACK(5, 0x35, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_C | PIN_CFG_IO_VMC_ETH1)), /* P8 */ RZG2L_GPIO_PORT_PACK(4, 0x36, RZG2L_MPXED_ETH_PIN_FUNCS(PIN_CFG_IOLH_C | @@ -1958,6 +2054,11 @@ static const struct rzg2l_hwcfg rzg3s_hwcfg = { .func_base = 1, }; +static const struct rzg2l_cfg_ops rzg3s_ops = { + .read_oen = rzg3s_read_oen, + .write_oen = rzg3s_write_oen, +}; + static struct rzg2l_pinctrl_data r9a07g043_data = { .port_pins = rzg2l_gpio_names, .port_pin_configs = r9a07g043_gpio_configs, @@ -1987,6 +2088,7 @@ static struct rzg2l_pinctrl_data r9a08g045_data = { .n_port_pins = ARRAY_SIZE(r9a08g045_gpio_configs) * RZG2L_PINS_PER_PORT, .n_dedicated_pins = ARRAY_SIZE(rzg3s_dedicated_pins), .hwcfg = &rzg3s_hwcfg, + .ops = &rzg3s_ops, }; static const struct of_device_id rzg2l_pinctrl_of_table[] = { -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 08/14] pinctrl: renesas: rzg2l: Add output enable support 2023-11-20 7:00 ` [PATCH 08/14] pinctrl: renesas: rzg2l: Add output enable support Claudiu @ 2023-12-01 17:25 ` Geert Uytterhoeven 0 siblings, 0 replies; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-01 17:25 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, Thanks for your patch! On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Some of the Ethernet pins on RZ/G3S (but also valid for RZ/G2L) need to > have direction of the IO buffer set as output for Ethernet to work > properly. On RZ/G3S these pins are P1_0/P7_0, P1_1/P7_1 with could have > the following Ethernet functions: TXC/TX_CLK or TX_CTL/TX_EN. To be able > to configure this the output enable has been implemented. This is > implemented with 2 per-platform read/write functions to be able to simply > validate the pins supporting this on a platform basis. Moreover, on RZ/G2L > the register though which these settings could be done is 8 bits long > whereas on RZ/G3S this is a 32 bit register. The Ethernet pins supporting > OEN are different. These differences could be handled in platform specific > OEN read/write functions. These registers are documented to support access sizes of 8/16/32 bits on RZ/G3S. Hence you don't need to differentiate, but can just use 8-bit accesses on all platforms. 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 09/14] dt-bindings: net: renesas,etheravb: Document RZ/G3S support 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu ` (7 preceding siblings ...) 2023-11-20 7:00 ` [PATCH 08/14] pinctrl: renesas: rzg2l: Add output enable support Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-11-20 15:39 ` Conor Dooley ` (2 more replies) 2023-11-20 7:00 ` [PATCH 10/14] arm64: renesas: r9a08g045: Add Ethernet nodes Claudiu ` (5 subsequent siblings) 14 siblings, 3 replies; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Document Ethernet RZ/G3S support. Ethernet IP is similar to the one available on RZ/G2L devices. Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- Documentation/devicetree/bindings/net/renesas,etheravb.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/net/renesas,etheravb.yaml b/Documentation/devicetree/bindings/net/renesas,etheravb.yaml index 5d074f27d462..38b71e687513 100644 --- a/Documentation/devicetree/bindings/net/renesas,etheravb.yaml +++ b/Documentation/devicetree/bindings/net/renesas,etheravb.yaml @@ -58,6 +58,7 @@ properties: - renesas,r9a07g043-gbeth # RZ/G2UL - renesas,r9a07g044-gbeth # RZ/G2{L,LC} - renesas,r9a07g054-gbeth # RZ/V2L + - renesas,r9a08g045-gbeth # RZ/G3S - const: renesas,rzg2l-gbeth # RZ/{G2L,G2UL,V2L} family reg: true -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 09/14] dt-bindings: net: renesas,etheravb: Document RZ/G3S support 2023-11-20 7:00 ` [PATCH 09/14] dt-bindings: net: renesas,etheravb: Document RZ/G3S support Claudiu @ 2023-11-20 15:39 ` Conor Dooley 2023-11-20 18:39 ` Sergey Shtylyov 2023-11-21 16:29 ` Geert Uytterhoeven 2 siblings, 0 replies; 53+ messages in thread From: Conor Dooley @ 2023-11-20 15:39 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea [-- Attachment #1: Type: text/plain, Size: 370 bytes --] On Mon, Nov 20, 2023 at 09:00:19AM +0200, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Document Ethernet RZ/G3S support. Ethernet IP is similar to the one > available on RZ/G2L devices. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Acked-by: Conor Dooley <conor.dooley@microchip.com> Cheers, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 09/14] dt-bindings: net: renesas,etheravb: Document RZ/G3S support 2023-11-20 7:00 ` [PATCH 09/14] dt-bindings: net: renesas,etheravb: Document RZ/G3S support Claudiu 2023-11-20 15:39 ` Conor Dooley @ 2023-11-20 18:39 ` Sergey Shtylyov 2023-11-21 16:29 ` Geert Uytterhoeven 2 siblings, 0 replies; 53+ messages in thread From: Sergey Shtylyov @ 2023-11-20 18:39 UTC (permalink / raw) To: Claudiu, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea On 11/20/23 10:00 AM, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Document Ethernet RZ/G3S support. Ethernet IP is similar to the one > available on RZ/G2L devices. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] MBR, Sergey ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 09/14] dt-bindings: net: renesas,etheravb: Document RZ/G3S support 2023-11-20 7:00 ` [PATCH 09/14] dt-bindings: net: renesas,etheravb: Document RZ/G3S support Claudiu 2023-11-20 15:39 ` Conor Dooley 2023-11-20 18:39 ` Sergey Shtylyov @ 2023-11-21 16:29 ` Geert Uytterhoeven 2 siblings, 0 replies; 53+ messages in thread From: Geert Uytterhoeven @ 2023-11-21 16:29 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Document Ethernet RZ/G3S support. Ethernet IP is similar to the one > available on RZ/G2L devices. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 10/14] arm64: renesas: r9a08g045: Add Ethernet nodes 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu ` (8 preceding siblings ...) 2023-11-20 7:00 ` [PATCH 09/14] dt-bindings: net: renesas,etheravb: Document RZ/G3S support Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-12-01 17:35 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 11/14] arm64: renesas: rzg3s-smarc-som: Invert the logic for SW_SD2_EN macro Claudiu ` (4 subsequent siblings) 14 siblings, 1 reply; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Add Ethernet nodes available on RZ/G3S (R9A08G045). Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi index 6c7b29b69d0e..1caa0587fdd4 100644 --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi @@ -149,6 +149,38 @@ sdhi2: mmc@11c20000 { status = "disabled"; }; + eth0: ethernet@11c30000 { + compatible = "renesas,r9a08g045-gbeth", "renesas,rzg2l-gbeth"; + reg = <0 0x11c30000 0 0x10000>; + interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "mux", "fil", "arp_ns"; + clocks = <&cpg CPG_MOD R9A08G045_ETH0_CLK_AXI>, + <&cpg CPG_MOD R9A08G045_ETH0_CLK_CHI>, + <&cpg CPG_MOD R9A08G045_ETH0_REFCLK>; + clock-names = "axi", "chi", "refclk"; + resets = <&cpg R9A08G045_ETH0_RST_HW_N>; + power-domains = <&cpg>; + status = "disabled"; + }; + + eth1: ethernet@11c40000 { + compatible = "renesas,r9a08g045-gbeth", "renesas,rzg2l-gbeth"; + reg = <0 0x11c40000 0 0x10000>; + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "mux", "fil", "arp_ns"; + clocks = <&cpg CPG_MOD R9A08G045_ETH1_CLK_AXI>, + <&cpg CPG_MOD R9A08G045_ETH1_CLK_CHI>, + <&cpg CPG_MOD R9A08G045_ETH1_REFCLK>; + clock-names = "axi", "chi", "refclk"; + resets = <&cpg R9A08G045_ETH1_RST_HW_N>; + power-domains = <&cpg>; + status = "disabled"; + }; + gic: interrupt-controller@12400000 { compatible = "arm,gic-v3"; #interrupt-cells = <3>; -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 10/14] arm64: renesas: r9a08g045: Add Ethernet nodes 2023-11-20 7:00 ` [PATCH 10/14] arm64: renesas: r9a08g045: Add Ethernet nodes Claudiu @ 2023-12-01 17:35 ` Geert Uytterhoeven 2023-12-04 7:41 ` claudiu beznea 0 siblings, 1 reply; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-01 17:35 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Add Ethernet nodes available on RZ/G3S (R9A08G045). > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks for your patch! > --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi > +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi > @@ -149,6 +149,38 @@ sdhi2: mmc@11c20000 { > status = "disabled"; > }; > > + eth0: ethernet@11c30000 { > + compatible = "renesas,r9a08g045-gbeth", "renesas,rzg2l-gbeth"; > + reg = <0 0x11c30000 0 0x10000>; > + interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "mux", "fil", "arp_ns"; > + clocks = <&cpg CPG_MOD R9A08G045_ETH0_CLK_AXI>, > + <&cpg CPG_MOD R9A08G045_ETH0_CLK_CHI>, > + <&cpg CPG_MOD R9A08G045_ETH0_REFCLK>; > + clock-names = "axi", "chi", "refclk"; > + resets = <&cpg R9A08G045_ETH0_RST_HW_N>; > + power-domains = <&cpg>; Perhaps add a default phy mode, like on other SoCs? phy-mode = "rgmii"'; Also missing: #address-cells = <1>; #size-cells = <0>; > + status = "disabled"; > + }; Same comments for eth1. 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 10/14] arm64: renesas: r9a08g045: Add Ethernet nodes 2023-12-01 17:35 ` Geert Uytterhoeven @ 2023-12-04 7:41 ` claudiu beznea 2023-12-04 8:02 ` Geert Uytterhoeven 0 siblings, 1 reply; 53+ messages in thread From: claudiu beznea @ 2023-12-04 7:41 UTC (permalink / raw) To: Geert Uytterhoeven Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi, Geert, On 01.12.2023 19:35, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> Add Ethernet nodes available on RZ/G3S (R9A08G045). >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Thanks for your patch! > >> --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi >> +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi >> @@ -149,6 +149,38 @@ sdhi2: mmc@11c20000 { >> status = "disabled"; >> }; >> >> + eth0: ethernet@11c30000 { >> + compatible = "renesas,r9a08g045-gbeth", "renesas,rzg2l-gbeth"; >> + reg = <0 0x11c30000 0 0x10000>; >> + interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>; >> + interrupt-names = "mux", "fil", "arp_ns"; >> + clocks = <&cpg CPG_MOD R9A08G045_ETH0_CLK_AXI>, >> + <&cpg CPG_MOD R9A08G045_ETH0_CLK_CHI>, >> + <&cpg CPG_MOD R9A08G045_ETH0_REFCLK>; >> + clock-names = "axi", "chi", "refclk"; >> + resets = <&cpg R9A08G045_ETH0_RST_HW_N>; >> + power-domains = <&cpg>; > > Perhaps add a default phy mode, like on other SoCs? > > phy-mode = "rgmii"'; I skipped this (even it was available on the other SoCs) as I consider the phy-mode is board specific. > > Also missing: > > #address-cells = <1>; > #size-cells = <0>; Same for these. > >> + status = "disabled"; >> + }; > > Same comments for eth1. > > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 10/14] arm64: renesas: r9a08g045: Add Ethernet nodes 2023-12-04 7:41 ` claudiu beznea @ 2023-12-04 8:02 ` Geert Uytterhoeven 2023-12-04 8:38 ` claudiu beznea 0 siblings, 1 reply; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-04 8:02 UTC (permalink / raw) To: claudiu beznea Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Mon, Dec 4, 2023 at 8:41 AM claudiu beznea <claudiu.beznea@tuxon.dev> wrote: > On 01.12.2023 19:35, Geert Uytterhoeven wrote: > > On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >> > >> Add Ethernet nodes available on RZ/G3S (R9A08G045). > >> > >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > > > Thanks for your patch! > > > >> --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi > >> +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi > >> @@ -149,6 +149,38 @@ sdhi2: mmc@11c20000 { > >> status = "disabled"; > >> }; > >> > >> + eth0: ethernet@11c30000 { > >> + compatible = "renesas,r9a08g045-gbeth", "renesas,rzg2l-gbeth"; > >> + reg = <0 0x11c30000 0 0x10000>; > >> + interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>, > >> + <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>, > >> + <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>; > >> + interrupt-names = "mux", "fil", "arp_ns"; > >> + clocks = <&cpg CPG_MOD R9A08G045_ETH0_CLK_AXI>, > >> + <&cpg CPG_MOD R9A08G045_ETH0_CLK_CHI>, > >> + <&cpg CPG_MOD R9A08G045_ETH0_REFCLK>; > >> + clock-names = "axi", "chi", "refclk"; > >> + resets = <&cpg R9A08G045_ETH0_RST_HW_N>; > >> + power-domains = <&cpg>; > > > > Perhaps add a default phy mode, like on other SoCs? > > > > phy-mode = "rgmii"'; > > I skipped this (even it was available on the other SoCs) as I consider the > phy-mode is board specific. IC. Still, it's good to have some consistency across boards. > > Also missing: > > > > #address-cells = <1>; > > #size-cells = <0>; > > Same for these. These are required, and always have the same values, so it makes more sense to have them in the SoC .dtsi file, once. 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 10/14] arm64: renesas: r9a08g045: Add Ethernet nodes 2023-12-04 8:02 ` Geert Uytterhoeven @ 2023-12-04 8:38 ` claudiu beznea 2023-12-04 9:00 ` Geert Uytterhoeven 0 siblings, 1 reply; 53+ messages in thread From: claudiu beznea @ 2023-12-04 8:38 UTC (permalink / raw) To: Geert Uytterhoeven Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea On 04.12.2023 10:02, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Mon, Dec 4, 2023 at 8:41 AM claudiu beznea <claudiu.beznea@tuxon.dev> wrote: >> On 01.12.2023 19:35, Geert Uytterhoeven wrote: >>> On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: >>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >>>> >>>> Add Ethernet nodes available on RZ/G3S (R9A08G045). >>>> >>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >>> >>> Thanks for your patch! >>> >>>> --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi >>>> +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi >>>> @@ -149,6 +149,38 @@ sdhi2: mmc@11c20000 { >>>> status = "disabled"; >>>> }; >>>> >>>> + eth0: ethernet@11c30000 { >>>> + compatible = "renesas,r9a08g045-gbeth", "renesas,rzg2l-gbeth"; >>>> + reg = <0 0x11c30000 0 0x10000>; >>>> + interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>, >>>> + <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>; >>>> + interrupt-names = "mux", "fil", "arp_ns"; >>>> + clocks = <&cpg CPG_MOD R9A08G045_ETH0_CLK_AXI>, >>>> + <&cpg CPG_MOD R9A08G045_ETH0_CLK_CHI>, >>>> + <&cpg CPG_MOD R9A08G045_ETH0_REFCLK>; >>>> + clock-names = "axi", "chi", "refclk"; >>>> + resets = <&cpg R9A08G045_ETH0_RST_HW_N>; >>>> + power-domains = <&cpg>; >>> >>> Perhaps add a default phy mode, like on other SoCs? >>> >>> phy-mode = "rgmii"'; >> >> I skipped this (even it was available on the other SoCs) as I consider the >> phy-mode is board specific. > > IC. Still, it's good to have some consistency across boards. > >>> Also missing: >>> >>> #address-cells = <1>; >>> #size-cells = <0>; >> >> Same for these. > > These are required, and always have the same values, so it makes more > sense to have them in the SoC .dtsi file, once. I remember I had a compilation warning with an Ethernet controller configured with fixed-link having #address-cells, #size-cells. With fixed-link these were not needed. Anyway... I'll keep all in dtsi if you prefer it this way. Thank you, Claudiu Beznea > > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 10/14] arm64: renesas: r9a08g045: Add Ethernet nodes 2023-12-04 8:38 ` claudiu beznea @ 2023-12-04 9:00 ` Geert Uytterhoeven 0 siblings, 0 replies; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-04 9:00 UTC (permalink / raw) To: claudiu beznea Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Mon, Dec 4, 2023 at 9:38 AM claudiu beznea <claudiu.beznea@tuxon.dev> wrote: > On 04.12.2023 10:02, Geert Uytterhoeven wrote: > > On Mon, Dec 4, 2023 at 8:41 AM claudiu beznea <claudiu.beznea@tuxon.dev> wrote: > >> On 01.12.2023 19:35, Geert Uytterhoeven wrote: > >>> On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > >>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >>>> > >>>> Add Ethernet nodes available on RZ/G3S (R9A08G045). > >>>> > >>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >>> > >>> Thanks for your patch! > >>> > >>>> --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi > >>>> +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi > >>>> @@ -149,6 +149,38 @@ sdhi2: mmc@11c20000 { > >>>> status = "disabled"; > >>>> }; > >>>> > >>>> + eth0: ethernet@11c30000 { > >>>> + compatible = "renesas,r9a08g045-gbeth", "renesas,rzg2l-gbeth"; > >>>> + reg = <0 0x11c30000 0 0x10000>; > >>>> + interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>, > >>>> + <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>, > >>>> + <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>; > >>>> + interrupt-names = "mux", "fil", "arp_ns"; > >>>> + clocks = <&cpg CPG_MOD R9A08G045_ETH0_CLK_AXI>, > >>>> + <&cpg CPG_MOD R9A08G045_ETH0_CLK_CHI>, > >>>> + <&cpg CPG_MOD R9A08G045_ETH0_REFCLK>; > >>>> + clock-names = "axi", "chi", "refclk"; > >>>> + resets = <&cpg R9A08G045_ETH0_RST_HW_N>; > >>>> + power-domains = <&cpg>; > >>> > >>> Perhaps add a default phy mode, like on other SoCs? > >>> > >>> phy-mode = "rgmii"'; > >> > >> I skipped this (even it was available on the other SoCs) as I consider the > >> phy-mode is board specific. > > > > IC. Still, it's good to have some consistency across boards. > > > >>> Also missing: > >>> > >>> #address-cells = <1>; > >>> #size-cells = <0>; > >> > >> Same for these. > > > > These are required, and always have the same values, so it makes more > > sense to have them in the SoC .dtsi file, once. > > I remember I had a compilation warning with an Ethernet controller > configured with fixed-link having #address-cells, #size-cells. With > fixed-link these were not needed. I think EtherAVB always use MDIO for management, so fixed-link is not applicable. > Anyway... I'll keep all in dtsi if you prefer it this way. Yes please, 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 11/14] arm64: renesas: rzg3s-smarc-som: Invert the logic for SW_SD2_EN macro 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu ` (9 preceding siblings ...) 2023-11-20 7:00 ` [PATCH 10/14] arm64: renesas: r9a08g045: Add Ethernet nodes Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-12-06 10:33 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 12/14] arm64: dts: renesas: Improve documentation for SW_SD0_DEV_SEL Claudiu ` (3 subsequent siblings) 14 siblings, 1 reply; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> The intention of SW_SD2_EN macro was to reflect the state of SW_CONFIG3 switch available on RZ/G3S Smarc Module. According to documentation SD2 is enabled when switch is in OFF state. For this, changed the logic of marco to map value 0 to switch's OFF state and value 1 to switch's ON state. Along with this update the description for each state for better understanding. The value of SW_SD2_EN macro was not changed in file because, according to documentation, the default state for this switch is ON. Fixes: adb4f0c5699c ("arm64: dts: renesas: Add initial support for RZ/G3S SMARC SoM") Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi index 01a4a9da7afc..275b14acd2ee 100644 --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi @@ -14,8 +14,8 @@ * 0 - SD0 is connected to eMMC * 1 - SD0 is connected to uSD0 card * @SW_SD2_EN: - * 0 - SCIF1, SSI0, IRQ0, IRQ1 connected to SoC - * 1 - SD2 is connected to SoC + * 0 - (switch OFF) SD2 is connected to SoC + * 1 - (switch ON) SCIF1, SSI0, IRQ0, IRQ1 connected to SoC */ #define SW_SD0_DEV_SEL 1 #define SW_SD2_EN 1 @@ -25,7 +25,7 @@ / { aliases { mmc0 = &sdhi0; -#if SW_SD2_EN +#if !SW_SD2_EN mmc2 = &sdhi2; #endif }; @@ -116,7 +116,7 @@ &sdhi0 { }; #endif -#if SW_SD2_EN +#if !SW_SD2_EN &sdhi2 { pinctrl-0 = <&sdhi2_pins>; pinctrl-names = "default"; -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 11/14] arm64: renesas: rzg3s-smarc-som: Invert the logic for SW_SD2_EN macro 2023-11-20 7:00 ` [PATCH 11/14] arm64: renesas: rzg3s-smarc-som: Invert the logic for SW_SD2_EN macro Claudiu @ 2023-12-06 10:33 ` Geert Uytterhoeven 2023-12-06 10:56 ` Geert Uytterhoeven 0 siblings, 1 reply; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-06 10:33 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Mon, Nov 20, 2023 at 8:03 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > The intention of SW_SD2_EN macro was to reflect the state of SW_CONFIG3 > switch available on RZ/G3S Smarc Module. According to documentation SD2 > is enabled when switch is in OFF state. For this, changed the logic of > marco to map value 0 to switch's OFF state and value 1 to switch's ON > state. Along with this update the description for each state for better > understanding. > > The value of SW_SD2_EN macro was not changed in file because, according to > documentation, the default state for this switch is ON. > > Fixes: adb4f0c5699c ("arm64: dts: renesas: Add initial support for RZ/G3S SMARC SoM") > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks for your patch! > --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi > +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi > @@ -14,8 +14,8 @@ > * 0 - SD0 is connected to eMMC > * 1 - SD0 is connected to uSD0 card > * @SW_SD2_EN: > - * 0 - SCIF1, SSI0, IRQ0, IRQ1 connected to SoC > - * 1 - SD2 is connected to SoC > + * 0 - (switch OFF) SD2 is connected to SoC > + * 1 - (switch ON) SCIF1, SSI0, IRQ0, IRQ1 connected to SoC I think this is still confusing: SW_SD2_EN refers to an active-low signal (SW_SD2_EN#) in the schematics. Before, SW_SD2_EN used assertion-logic (1 is enabled), and didn't match the physical signal level. After your patch, SW_SD2_EN matches the active-low physical level, but this is not reflected in the name... > */ > #define SW_SD0_DEV_SEL 1 > #define SW_SD2_EN 1 > @@ -25,7 +25,7 @@ / { > > aliases { > mmc0 = &sdhi0; > -#if SW_SD2_EN > +#if !SW_SD2_EN ... so this condition looks really weird. > mmc2 = &sdhi2; > #endif > }; > @@ -116,7 +116,7 @@ &sdhi0 { > }; > #endif > > -#if SW_SD2_EN > +#if !SW_SD2_EN > &sdhi2 { > pinctrl-0 = <&sdhi2_pins>; > pinctrl-names = "default"; So I think SW_SD2_EN should be renamed to SW_SD2_EN_N. Cfr. SW_ET0_EN_N on RZ/G2UL: arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * DIP-Switch SW1 setting arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * 1 : High; 0: Low arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * SW1-2 : SW_SD0_DEV_SEL (0: uSD; 1: eMMC) arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * SW1-3 : SW_ET0_EN_N (0: ETHER0; 1: CAN0, CAN1, SSI1, RSPI1) arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * Please change below macros according to SW1 setting on the SoM 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 11/14] arm64: renesas: rzg3s-smarc-som: Invert the logic for SW_SD2_EN macro 2023-12-06 10:33 ` Geert Uytterhoeven @ 2023-12-06 10:56 ` Geert Uytterhoeven 2023-12-06 11:11 ` claudiu beznea 0 siblings, 1 reply; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-06 10:56 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea On Wed, Dec 6, 2023 at 11:33 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Mon, Nov 20, 2023 at 8:03 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > > > The intention of SW_SD2_EN macro was to reflect the state of SW_CONFIG3 > > switch available on RZ/G3S Smarc Module. According to documentation SD2 > > is enabled when switch is in OFF state. For this, changed the logic of > > marco to map value 0 to switch's OFF state and value 1 to switch's ON > > state. Along with this update the description for each state for better > > understanding. > > > > The value of SW_SD2_EN macro was not changed in file because, according to > > documentation, the default state for this switch is ON. > > > > Fixes: adb4f0c5699c ("arm64: dts: renesas: Add initial support for RZ/G3S SMARC SoM") > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Thanks for your patch! > > > --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi > > +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi > > @@ -14,8 +14,8 @@ > > * 0 - SD0 is connected to eMMC > > * 1 - SD0 is connected to uSD0 card > > * @SW_SD2_EN: > > - * 0 - SCIF1, SSI0, IRQ0, IRQ1 connected to SoC > > - * 1 - SD2 is connected to SoC > > + * 0 - (switch OFF) SD2 is connected to SoC > > + * 1 - (switch ON) SCIF1, SSI0, IRQ0, IRQ1 connected to SoC > > I think this is still confusing: SW_SD2_EN refers to an active-low signal > (SW_SD2_EN#) in the schematics. OMG, while the signal is called "SW_SD2_EN#" in the schematics, it is _not_ active-low! SW_D2_EN# drives a STG3692 quad SPDT switch, and SD2 is enabled if SW_D2_EN# is high... The RZ/G3S SMARC Module User Manual says: Signal SW_SD2_EN ON: SD2 is disabled. Signal SW_SD2_EN OFF: SD2 is enabled. So whatever we do, something will look odd :-( > Before, SW_SD2_EN used assertion-logic (1 is enabled), and didn't > match the physical signal level. > After your patch, SW_SD2_EN matches the active-low physical level, but > this is not reflected in the name... > > > */ > > #define SW_SD0_DEV_SEL 1 > > #define SW_SD2_EN 1 > > @@ -25,7 +25,7 @@ / { > > > > aliases { > > mmc0 = &sdhi0; > > -#if SW_SD2_EN > > +#if !SW_SD2_EN > > ... so this condition looks really weird. Still, I think the original looks nicer here. So I suggest to keep the original logic, but clarify the position of the switch. Does that make sense? > > > mmc2 = &sdhi2; > > #endif > > }; > > @@ -116,7 +116,7 @@ &sdhi0 { > > }; > > #endif > > > > -#if SW_SD2_EN > > +#if !SW_SD2_EN > > &sdhi2 { > > pinctrl-0 = <&sdhi2_pins>; > > pinctrl-names = "default"; > > So I think SW_SD2_EN should be renamed to SW_SD2_EN_N. > > Cfr. SW_ET0_EN_N on RZ/G2UL: > > arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * DIP-Switch SW1 setting > arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * 1 : High; 0: Low > arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * SW1-2 : > SW_SD0_DEV_SEL (0: uSD; 1: eMMC) > arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * SW1-3 : > SW_ET0_EN_N (0: ETHER0; 1: CAN0, CAN1, SSI1, RSPI1) > arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * Please change > below macros according to SW1 setting on the SoM 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 11/14] arm64: renesas: rzg3s-smarc-som: Invert the logic for SW_SD2_EN macro 2023-12-06 10:56 ` Geert Uytterhoeven @ 2023-12-06 11:11 ` claudiu beznea 2023-12-06 11:27 ` Geert Uytterhoeven 0 siblings, 1 reply; 53+ messages in thread From: claudiu beznea @ 2023-12-06 11:11 UTC (permalink / raw) To: Geert Uytterhoeven Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi, Geert, On 06.12.2023 12:56, Geert Uytterhoeven wrote: > On Wed, Dec 6, 2023 at 11:33 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> On Mon, Nov 20, 2023 at 8:03 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: >>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >>> >>> The intention of SW_SD2_EN macro was to reflect the state of SW_CONFIG3 >>> switch available on RZ/G3S Smarc Module. According to documentation SD2 >>> is enabled when switch is in OFF state. For this, changed the logic of >>> marco to map value 0 to switch's OFF state and value 1 to switch's ON >>> state. Along with this update the description for each state for better >>> understanding. >>> >>> The value of SW_SD2_EN macro was not changed in file because, according to >>> documentation, the default state for this switch is ON. >>> >>> Fixes: adb4f0c5699c ("arm64: dts: renesas: Add initial support for RZ/G3S SMARC SoM") >>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> Thanks for your patch! >> >>> --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi >>> +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi >>> @@ -14,8 +14,8 @@ >>> * 0 - SD0 is connected to eMMC >>> * 1 - SD0 is connected to uSD0 card >>> * @SW_SD2_EN: >>> - * 0 - SCIF1, SSI0, IRQ0, IRQ1 connected to SoC >>> - * 1 - SD2 is connected to SoC >>> + * 0 - (switch OFF) SD2 is connected to SoC >>> + * 1 - (switch ON) SCIF1, SSI0, IRQ0, IRQ1 connected to SoC >> >> I think this is still confusing: SW_SD2_EN refers to an active-low signal >> (SW_SD2_EN#) in the schematics. > > OMG, while the signal is called "SW_SD2_EN#" in the schematics, it is > _not_ active-low! > SW_D2_EN# drives a STG3692 quad SPDT switch, and SD2 is enabled > if SW_D2_EN# is high... > > The RZ/G3S SMARC Module User Manual says: > > Signal SW_SD2_EN ON: SD2 is disabled. > Signal SW_SD2_EN OFF: SD2 is enabled. I followed the description in this manual, chapter 2.1.1 SW_CONFIG. The idea was that these macros to correspond to individual switches, to match that table (describing switches position) with this code as the user in the end sets those switches described in table at 2.1.1 w/o necessary going deep into schematic (at least in the beginning when trying different functionalities). Do you think it would be better if we will have these macros named SWCONFIGX, X in {1, 2, 3, 4, 5, 6} ? > > So whatever we do, something will look odd :-( > >> Before, SW_SD2_EN used assertion-logic (1 is enabled), and didn't >> match the physical signal level. >> After your patch, SW_SD2_EN matches the active-low physical level, but >> this is not reflected in the name... >> >>> */ >>> #define SW_SD0_DEV_SEL 1 >>> #define SW_SD2_EN 1 >>> @@ -25,7 +25,7 @@ / { >>> >>> aliases { >>> mmc0 = &sdhi0; >>> -#if SW_SD2_EN >>> +#if !SW_SD2_EN >> >> ... so this condition looks really weird. > > Still, I think the original looks nicer here. > > So I suggest to keep the original logic, but clarify the position of > the switch. > Does that make sense? It will still be odd, AFAICT, as this way as we will map 0 to ON and 1 to OFF... A bit counterintuitive. > > >> >>> mmc2 = &sdhi2; >>> #endif >>> }; >>> @@ -116,7 +116,7 @@ &sdhi0 { >>> }; >>> #endif >>> >>> -#if SW_SD2_EN >>> +#if !SW_SD2_EN >>> &sdhi2 { >>> pinctrl-0 = <&sdhi2_pins>; >>> pinctrl-names = "default"; >> >> So I think SW_SD2_EN should be renamed to SW_SD2_EN_N. >> >> Cfr. SW_ET0_EN_N on RZ/G2UL: >> >> arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * DIP-Switch SW1 setting >> arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * 1 : High; 0: Low >> arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * SW1-2 : >> SW_SD0_DEV_SEL (0: uSD; 1: eMMC) >> arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * SW1-3 : >> SW_ET0_EN_N (0: ETHER0; 1: CAN0, CAN1, SSI1, RSPI1) >> arch/arm64/boot/dts/renesas/r9a07g043u11-smarc.dts- * Please change >> below macros according to SW1 setting on the SoM > > 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 11/14] arm64: renesas: rzg3s-smarc-som: Invert the logic for SW_SD2_EN macro 2023-12-06 11:11 ` claudiu beznea @ 2023-12-06 11:27 ` Geert Uytterhoeven 2023-12-06 11:31 ` claudiu beznea 0 siblings, 1 reply; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-06 11:27 UTC (permalink / raw) To: claudiu beznea Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Wed, Dec 6, 2023 at 12:12 PM claudiu beznea <claudiu.beznea@tuxon.dev> wrote: > On 06.12.2023 12:56, Geert Uytterhoeven wrote: > > On Wed, Dec 6, 2023 at 11:33 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > >> On Mon, Nov 20, 2023 at 8:03 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > >>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >>> > >>> The intention of SW_SD2_EN macro was to reflect the state of SW_CONFIG3 > >>> switch available on RZ/G3S Smarc Module. According to documentation SD2 > >>> is enabled when switch is in OFF state. For this, changed the logic of > >>> marco to map value 0 to switch's OFF state and value 1 to switch's ON > >>> state. Along with this update the description for each state for better > >>> understanding. > >>> > >>> The value of SW_SD2_EN macro was not changed in file because, according to > >>> documentation, the default state for this switch is ON. > >>> > >>> Fixes: adb4f0c5699c ("arm64: dts: renesas: Add initial support for RZ/G3S SMARC SoM") > >>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >> > >> Thanks for your patch! > >> > >>> --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi > >>> +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi > >>> @@ -14,8 +14,8 @@ > >>> * 0 - SD0 is connected to eMMC > >>> * 1 - SD0 is connected to uSD0 card > >>> * @SW_SD2_EN: > >>> - * 0 - SCIF1, SSI0, IRQ0, IRQ1 connected to SoC > >>> - * 1 - SD2 is connected to SoC > >>> + * 0 - (switch OFF) SD2 is connected to SoC > >>> + * 1 - (switch ON) SCIF1, SSI0, IRQ0, IRQ1 connected to SoC > >> > >> I think this is still confusing: SW_SD2_EN refers to an active-low signal > >> (SW_SD2_EN#) in the schematics. > > > > OMG, while the signal is called "SW_SD2_EN#" in the schematics, it is > > _not_ active-low! > > SW_D2_EN# drives a STG3692 quad SPDT switch, and SD2 is enabled > > if SW_D2_EN# is high... > > > > The RZ/G3S SMARC Module User Manual says: > > > > Signal SW_SD2_EN ON: SD2 is disabled. > > Signal SW_SD2_EN OFF: SD2 is enabled. > > I followed the description in this manual, chapter 2.1.1 SW_CONFIG. The > idea was that these macros to correspond to individual switches, to match > that table (describing switches position) with this code as the user in the > end sets those switches described in table at 2.1.1 w/o necessary going > deep into schematic (at least in the beginning when trying different > functionalities). > > Do you think it would be better if we will have these macros named > SWCONFIGX, X in {1, 2, 3, 4, 5, 6} ? Perhaps. A disadvantage would be that SW_CONFIG%u doesn't give any indication about its purpose... > > So whatever we do, something will look odd :-( > > > >> Before, SW_SD2_EN used assertion-logic (1 is enabled), and didn't > >> match the physical signal level. > >> After your patch, SW_SD2_EN matches the active-low physical level, but > >> this is not reflected in the name... > >> > >>> */ > >>> #define SW_SD0_DEV_SEL 1 > >>> #define SW_SD2_EN 1 > >>> @@ -25,7 +25,7 @@ / { > >>> > >>> aliases { > >>> mmc0 = &sdhi0; > >>> -#if SW_SD2_EN > >>> +#if !SW_SD2_EN > >> > >> ... so this condition looks really weird. > > > > Still, I think the original looks nicer here. > > > > So I suggest to keep the original logic, but clarify the position of > > the switch. > > Does that make sense? > > It will still be odd, AFAICT, as this way as we will map 0 to ON and 1 to > OFF... A bit counterintuitive. Most switches on board pull signals LOW when the switch is ON... 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 11/14] arm64: renesas: rzg3s-smarc-som: Invert the logic for SW_SD2_EN macro 2023-12-06 11:27 ` Geert Uytterhoeven @ 2023-12-06 11:31 ` claudiu beznea 0 siblings, 0 replies; 53+ messages in thread From: claudiu beznea @ 2023-12-06 11:31 UTC (permalink / raw) To: Geert Uytterhoeven Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea On 06.12.2023 13:27, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Wed, Dec 6, 2023 at 12:12 PM claudiu beznea <claudiu.beznea@tuxon.dev> wrote: >> On 06.12.2023 12:56, Geert Uytterhoeven wrote: >>> On Wed, Dec 6, 2023 at 11:33 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: >>>> On Mon, Nov 20, 2023 at 8:03 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: >>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >>>>> >>>>> The intention of SW_SD2_EN macro was to reflect the state of SW_CONFIG3 >>>>> switch available on RZ/G3S Smarc Module. According to documentation SD2 >>>>> is enabled when switch is in OFF state. For this, changed the logic of >>>>> marco to map value 0 to switch's OFF state and value 1 to switch's ON >>>>> state. Along with this update the description for each state for better >>>>> understanding. >>>>> >>>>> The value of SW_SD2_EN macro was not changed in file because, according to >>>>> documentation, the default state for this switch is ON. >>>>> >>>>> Fixes: adb4f0c5699c ("arm64: dts: renesas: Add initial support for RZ/G3S SMARC SoM") >>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >>>> >>>> Thanks for your patch! >>>> >>>>> --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi >>>>> +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi >>>>> @@ -14,8 +14,8 @@ >>>>> * 0 - SD0 is connected to eMMC >>>>> * 1 - SD0 is connected to uSD0 card >>>>> * @SW_SD2_EN: >>>>> - * 0 - SCIF1, SSI0, IRQ0, IRQ1 connected to SoC >>>>> - * 1 - SD2 is connected to SoC >>>>> + * 0 - (switch OFF) SD2 is connected to SoC >>>>> + * 1 - (switch ON) SCIF1, SSI0, IRQ0, IRQ1 connected to SoC >>>> >>>> I think this is still confusing: SW_SD2_EN refers to an active-low signal >>>> (SW_SD2_EN#) in the schematics. >>> >>> OMG, while the signal is called "SW_SD2_EN#" in the schematics, it is >>> _not_ active-low! >>> SW_D2_EN# drives a STG3692 quad SPDT switch, and SD2 is enabled >>> if SW_D2_EN# is high... >>> >>> The RZ/G3S SMARC Module User Manual says: >>> >>> Signal SW_SD2_EN ON: SD2 is disabled. >>> Signal SW_SD2_EN OFF: SD2 is enabled. >> >> I followed the description in this manual, chapter 2.1.1 SW_CONFIG. The >> idea was that these macros to correspond to individual switches, to match >> that table (describing switches position) with this code as the user in the >> end sets those switches described in table at 2.1.1 w/o necessary going >> deep into schematic (at least in the beginning when trying different >> functionalities). >> >> Do you think it would be better if we will have these macros named >> SWCONFIGX, X in {1, 2, 3, 4, 5, 6} ? > > Perhaps. A disadvantage would be that SW_CONFIG%u doesn't > give any indication about its purpose... That's the reason I chose initially to have the signal names instead of SWCONFIGX. Now seeing that signal names could be confusing I tend to go with SWCONFIGx instead. > >>> So whatever we do, something will look odd :-( >>> >>>> Before, SW_SD2_EN used assertion-logic (1 is enabled), and didn't >>>> match the physical signal level. >>>> After your patch, SW_SD2_EN matches the active-low physical level, but >>>> this is not reflected in the name... >>>> >>>>> */ >>>>> #define SW_SD0_DEV_SEL 1 >>>>> #define SW_SD2_EN 1 >>>>> @@ -25,7 +25,7 @@ / { >>>>> >>>>> aliases { >>>>> mmc0 = &sdhi0; >>>>> -#if SW_SD2_EN >>>>> +#if !SW_SD2_EN >>>> >>>> ... so this condition looks really weird. >>> >>> Still, I think the original looks nicer here. >>> >>> So I suggest to keep the original logic, but clarify the position of >>> the switch. >>> Does that make sense? >> >> It will still be odd, AFAICT, as this way as we will map 0 to ON and 1 to >> OFF... A bit counterintuitive. > > Most switches on board pull signals LOW when the switch is ON... > > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 12/14] arm64: dts: renesas: Improve documentation for SW_SD0_DEV_SEL 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu ` (10 preceding siblings ...) 2023-11-20 7:00 ` [PATCH 11/14] arm64: renesas: rzg3s-smarc-som: Invert the logic for SW_SD2_EN macro Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-11-20 8:41 ` Sergey Shtylyov 2023-12-06 11:03 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 13/14] arm64: dts: renesas: rzg3s-smarc-som: Enable Ethernet interfaces Claudiu ` (2 subsequent siblings) 14 siblings, 2 replies; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Add switch OFF/OFF description to values of SW_SD0_DEV_SEL for better understanding. Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi index 275b14acd2ee..e090a4837468 100644 --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi @@ -11,8 +11,8 @@ /* * Signals of SW_CONFIG switches: * @SW_SD0_DEV_SEL: - * 0 - SD0 is connected to eMMC - * 1 - SD0 is connected to uSD0 card + * 0 - (switch OFF) SD0 is connected to eMMC + * 1 - (switch ON) SD0 is connected to uSD0 card * @SW_SD2_EN: * 0 - (switch OFF) SD2 is connected to SoC * 1 - (switch ON) SCIF1, SSI0, IRQ0, IRQ1 connected to SoC -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 12/14] arm64: dts: renesas: Improve documentation for SW_SD0_DEV_SEL 2023-11-20 7:00 ` [PATCH 12/14] arm64: dts: renesas: Improve documentation for SW_SD0_DEV_SEL Claudiu @ 2023-11-20 8:41 ` Sergey Shtylyov 2023-12-06 11:03 ` Geert Uytterhoeven 1 sibling, 0 replies; 53+ messages in thread From: Sergey Shtylyov @ 2023-11-20 8:41 UTC (permalink / raw) To: Claudiu, s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea On 11/20/23 10:00 AM, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Add switch OFF/OFF description to values of SW_SD0_DEV_SEL for OFF/ON probably? > better understanding. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> [...] MBR, Sergey ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 12/14] arm64: dts: renesas: Improve documentation for SW_SD0_DEV_SEL 2023-11-20 7:00 ` [PATCH 12/14] arm64: dts: renesas: Improve documentation for SW_SD0_DEV_SEL Claudiu 2023-11-20 8:41 ` Sergey Shtylyov @ 2023-12-06 11:03 ` Geert Uytterhoeven 1 sibling, 0 replies; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-06 11:03 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, On Mon, Nov 20, 2023 at 8:03 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Add switch OFF/OFF description to values of SW_SD0_DEV_SEL for > better understanding. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks for your patch! > --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi > +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi > @@ -11,8 +11,8 @@ > /* > * Signals of SW_CONFIG switches: > * @SW_SD0_DEV_SEL: > - * 0 - SD0 is connected to eMMC > - * 1 - SD0 is connected to uSD0 card > + * 0 - (switch OFF) SD0 is connected to eMMC > + * 1 - (switch ON) SD0 is connected to uSD0 card > * @SW_SD2_EN: > * 0 - (switch OFF) SD2 is connected to SoC > * 1 - (switch ON) SCIF1, SSI0, IRQ0, IRQ1 connected to SoC I guess this makes sense Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 13/14] arm64: dts: renesas: rzg3s-smarc-som: Enable Ethernet interfaces 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu ` (11 preceding siblings ...) 2023-11-20 7:00 ` [PATCH 12/14] arm64: dts: renesas: Improve documentation for SW_SD0_DEV_SEL Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-12-06 11:22 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 14/14] arm: multi_v7_defconfig: Enable CONFIG_RAVB Claudiu 2023-11-23 15:01 ` [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Linus Walleij 14 siblings, 1 reply; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> RZ/G3S Smarc Module has Ethernet PHYs (KSZ9131) connected to each Ethernet IP. For this add proper DT bindings to enable the Ethernet communication though these PHYs. The interface b/w PHYs and MACs is RGMII. The skew settings were set to zero as based on phy-mode (rgmii-id) the KSZ9131 driver enables internal DLL which adds 2ns delay b/w clocks (TX/RX) and data signals. Different pin settings were applied to TXC, TX_CTL compared with the rest of the RGMII pins to comply with requirements for these pins imposed by HW manual of RZ/G3S (see chapters "Ether Ch0 Voltage Mode Control Register (ETH0_POC)", "Ether Ch1 Voltage Mode Control Register (ETH1_POC)", for power source selection, "Ether MII/RGMII Mode Control Register (ETH_MODE)" for output-enable and "Input Enable Control Register (IEN_m)" for input-enable configurations). Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- Hi, Geert, Please note that at the moment (on top of linux-next) the PHYs are probed in poll mode as the interrupt controller support is not included yet. It is work in progress (see [1]). Thank you, Claudiu Beznea [1] https://lore.kernel.org/all/20231115142749.853106-1-claudiu.beznea.uj@bp.renesas.com/ .../boot/dts/renesas/rzg3s-smarc-som.dtsi | 145 +++++++++++++++++- 1 file changed, 144 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi index e090a4837468..571cade41647 100644 --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi @@ -25,7 +25,10 @@ / { aliases { mmc0 = &sdhi0; -#if !SW_SD2_EN +#if SW_SD2_EN + eth0 = ð0; + eth1 = ð1; +#else mmc2 = &sdhi2; #endif }; @@ -81,6 +84,64 @@ vcc_sdhi2: regulator2 { }; }; +#if SW_SD2_EN +ð0 { + pinctrl-0 = <ð0_pins>; + pinctrl-names = "default"; + phy-handle = <&phy0>; + phy-mode = "rgmii-id"; + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + phy0: ethernet-phy@7 { + reg = <7>; + interrupt-parent = <&pinctrl>; + interrupts = <RZG2L_GPIO(12, 0) IRQ_TYPE_EDGE_FALLING>; + rxc-skew-psec = <0>; + txc-skew-psec = <0>; + rxdv-skew-psec = <0>; + txen-skew-psec = <0>; + rxd0-skew-psec = <0>; + rxd1-skew-psec = <0>; + rxd2-skew-psec = <0>; + rxd3-skew-psec = <0>; + txd0-skew-psec = <0>; + txd1-skew-psec = <0>; + txd2-skew-psec = <0>; + txd3-skew-psec = <0>; + }; +}; + +ð1 { + pinctrl-0 = <ð1_pins>; + pinctrl-names = "default"; + phy-handle = <&phy1>; + phy-mode = "rgmii-id"; + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + phy1: ethernet-phy@7 { + reg = <7>; + interrupt-parent = <&pinctrl>; + interrupts = <RZG2L_GPIO(12, 1) IRQ_TYPE_EDGE_FALLING>; + rxc-skew-psec = <0>; + txc-skew-psec = <0>; + rxdv-skew-psec = <0>; + txen-skew-psec = <0>; + rxd0-skew-psec = <0>; + rxd1-skew-psec = <0>; + rxd2-skew-psec = <0>; + rxd3-skew-psec = <0>; + txd0-skew-psec = <0>; + txd1-skew-psec = <0>; + txd2-skew-psec = <0>; + txd3-skew-psec = <0>; + }; +}; +#endif + &extal_clk { clock-frequency = <24000000>; }; @@ -128,6 +189,88 @@ &sdhi2 { #endif &pinctrl { + eth0-phy-irq-hog { + gpio-hog; + gpios = <RZG2L_GPIO(12, 0) GPIO_ACTIVE_LOW>; + input; + line-name = "eth0-phy-irq"; + }; + + eth0_pins: eth0 { + txc { + pinmux = <RZG2L_PORT_PINMUX(1, 0, 1)>; /* ET0_TXC */ + power-source = <1800>; + output-enable; + input-enable; + drive-strength-microamp = <5200>; + }; + + tx_ctl { + pinmux = <RZG2L_PORT_PINMUX(1, 1, 1)>; /* ET0_TX_CTL */ + power-source = <1800>; + output-enable; + drive-strength-microamp = <5200>; + }; + + mux { + pinmux = <RZG2L_PORT_PINMUX(1, 2, 1)>, /* ET0_TXD0 */ + <RZG2L_PORT_PINMUX(1, 3, 1)>, /* ET0_TXD1 */ + <RZG2L_PORT_PINMUX(1, 4, 1)>, /* ET0_TXD2 */ + <RZG2L_PORT_PINMUX(2, 0, 1)>, /* ET0_TXD3 */ + <RZG2L_PORT_PINMUX(3, 0, 1)>, /* ET0_RXC */ + <RZG2L_PORT_PINMUX(3, 1, 1)>, /* ET0_RX_CTL */ + <RZG2L_PORT_PINMUX(3, 2, 1)>, /* ET0_RXD0 */ + <RZG2L_PORT_PINMUX(3, 3, 1)>, /* ET0_RXD1 */ + <RZG2L_PORT_PINMUX(4, 0, 1)>, /* ET0_RXD2 */ + <RZG2L_PORT_PINMUX(4, 1, 1)>, /* ET0_RXD3 */ + <RZG2L_PORT_PINMUX(4, 3, 1)>, /* ET0_MDC */ + <RZG2L_PORT_PINMUX(4, 4, 1)>, /* ET0_MDIO */ + <RZG2L_PORT_PINMUX(4, 5, 1)>; /* ET0_LINKSTA */ + power-source = <1800>; + }; + }; + + eth1-phy-irq-hog { + gpio-hog; + gpios = <RZG2L_GPIO(12, 1) GPIO_ACTIVE_LOW>; + input; + line-name = "eth1-phy-irq"; + }; + + eth1_pins: eth1 { + txc { + pinmux = <RZG2L_PORT_PINMUX(7, 0, 1)>; /* ET1_TXC */ + power-source = <1800>; + output-enable; + input-enable; + drive-strength-microamp = <5200>; + }; + + tx_ctl { + pinmux = <RZG2L_PORT_PINMUX(7, 1, 1)>; /* ET1_TX_CTL */ + power-source = <1800>; + output-enable; + drive-strength-microamp = <5200>; + }; + + mux { + pinmux = <RZG2L_PORT_PINMUX(7, 2, 1)>, /* ET1_TXD0 */ + <RZG2L_PORT_PINMUX(7, 3, 1)>, /* ET1_TXD1 */ + <RZG2L_PORT_PINMUX(7, 4, 1)>, /* ET1_TXD2 */ + <RZG2L_PORT_PINMUX(8, 0, 1)>, /* ET1_TXD3 */ + <RZG2L_PORT_PINMUX(8, 4, 1)>, /* ET1_RXC */ + <RZG2L_PORT_PINMUX(9, 0, 1)>, /* ET1_RX_CTL */ + <RZG2L_PORT_PINMUX(9, 1, 1)>, /* ET1_RXD0 */ + <RZG2L_PORT_PINMUX(9, 2, 1)>, /* ET1_RXD1 */ + <RZG2L_PORT_PINMUX(9, 3, 1)>, /* ET1_RXD2 */ + <RZG2L_PORT_PINMUX(10, 0, 1)>, /* ET1_RXD3 */ + <RZG2L_PORT_PINMUX(10, 2, 1)>, /* ET1_MDC */ + <RZG2L_PORT_PINMUX(10, 3, 1)>, /* ET1_MDIO */ + <RZG2L_PORT_PINMUX(10, 4, 1)>; /* ET1_LINKSTA */ + power-source = <1800>; + }; + }; + sdhi0_pins: sd0 { data { pins = "SD0_DATA0", "SD0_DATA1", "SD0_DATA2", "SD0_DATA3"; -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 13/14] arm64: dts: renesas: rzg3s-smarc-som: Enable Ethernet interfaces 2023-11-20 7:00 ` [PATCH 13/14] arm64: dts: renesas: rzg3s-smarc-som: Enable Ethernet interfaces Claudiu @ 2023-12-06 11:22 ` Geert Uytterhoeven 2023-12-06 11:48 ` claudiu beznea 0 siblings, 1 reply; 53+ messages in thread From: Geert Uytterhoeven @ 2023-12-06 11:22 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi Claudiu, Thanks for your patch! On Mon, Nov 20, 2023 at 8:03 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > RZ/G3S Smarc Module has Ethernet PHYs (KSZ9131) connected to each Ethernet > IP. For this add proper DT bindings to enable the Ethernet communication > though these PHYs. > > The interface b/w PHYs and MACs is RGMII. The skew settings were set to > zero as based on phy-mode (rgmii-id) the KSZ9131 driver enables internal > DLL which adds 2ns delay b/w clocks (TX/RX) and data signals. So shouldn't you just use phy-mode "rgmii" instead? > Different pin settings were applied to TXC, TX_CTL compared with the rest > of the RGMII pins to comply with requirements for these pins imposed by > HW manual of RZ/G3S (see chapters "Ether Ch0 Voltage Mode Control > Register (ETH0_POC)", "Ether Ch1 Voltage Mode Control Register (ETH1_POC)", > for power source selection, "Ether MII/RGMII Mode Control Register > (ETH_MODE)" for output-enable and "Input Enable Control Register (IEN_m)" > for input-enable configurations). > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi > +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi > @@ -25,7 +25,10 @@ / { > > aliases { > mmc0 = &sdhi0; > -#if !SW_SD2_EN > +#if SW_SD2_EN Cfr. my comment on [PATCH 11/14], this looks odd... > + eth0 = ð0; > + eth1 = ð1; > +#else > mmc2 = &sdhi2; > #endif > }; > @@ -81,6 +84,64 @@ vcc_sdhi2: regulator2 { > }; > }; > > +#if SW_SD2_EN Likewise. > +ð0 { > + pinctrl-0 = <ð0_pins>; > + pinctrl-names = "default"; > + phy-handle = <&phy0>; > + phy-mode = "rgmii-id"; > + #address-cells = <1>; > + #size-cells = <0>; #{address,size}-cells should be in the SoC-specific .dtsi. Same for eth1. > + status = "okay"; The rest LGTM. 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 13/14] arm64: dts: renesas: rzg3s-smarc-som: Enable Ethernet interfaces 2023-12-06 11:22 ` Geert Uytterhoeven @ 2023-12-06 11:48 ` claudiu beznea 0 siblings, 0 replies; 53+ messages in thread From: claudiu beznea @ 2023-12-06 11:48 UTC (permalink / raw) To: Geert Uytterhoeven Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea Hi, Geert, On 06.12.2023 13:22, Geert Uytterhoeven wrote: > Hi Claudiu, > > Thanks for your patch! > > On Mon, Nov 20, 2023 at 8:03 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> RZ/G3S Smarc Module has Ethernet PHYs (KSZ9131) connected to each Ethernet >> IP. For this add proper DT bindings to enable the Ethernet communication >> though these PHYs. >> >> The interface b/w PHYs and MACs is RGMII. The skew settings were set to >> zero as based on phy-mode (rgmii-id) the KSZ9131 driver enables internal >> DLL which adds 2ns delay b/w clocks (TX/RX) and data signals. > > So shouldn't you just use phy-mode "rgmii" instead? I chose it like this for simpler configuration of the skew settings. The PHY supports fixed 2ns delays which is enough for RGMII. And this is configured based on phy-mode="rgmii-id". As this delay depends also on soldering length I consider it better this way. The other variant would have been using phy-mode="rgmii" + skew settings. Also, same phy-mode is used by rzg2ul-smarc-som.dtsi which is using the same PHY. >> Different pin settings were applied to TXC, TX_CTL compared with the rest >> of the RGMII pins to comply with requirements for these pins imposed by >> HW manual of RZ/G3S (see chapters "Ether Ch0 Voltage Mode Control >> Register (ETH0_POC)", "Ether Ch1 Voltage Mode Control Register (ETH1_POC)", >> for power source selection, "Ether MII/RGMII Mode Control Register >> (ETH_MODE)" for output-enable and "Input Enable Control Register (IEN_m)" >> for input-enable configurations). >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >> --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi >> +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi >> @@ -25,7 +25,10 @@ / { >> >> aliases { >> mmc0 = &sdhi0; >> -#if !SW_SD2_EN >> +#if SW_SD2_EN > > Cfr. my comment on [PATCH 11/14], this looks odd... > >> + eth0 = ð0; >> + eth1 = ð1; >> +#else >> mmc2 = &sdhi2; >> #endif >> }; >> @@ -81,6 +84,64 @@ vcc_sdhi2: regulator2 { >> }; >> }; >> >> +#if SW_SD2_EN > > Likewise. > >> +ð0 { >> + pinctrl-0 = <ð0_pins>; >> + pinctrl-names = "default"; >> + phy-handle = <&phy0>; >> + phy-mode = "rgmii-id"; >> + #address-cells = <1>; >> + #size-cells = <0>; > > #{address,size}-cells should be in the SoC-specific .dtsi. > Same for eth1. > >> + status = "okay"; > > The rest LGTM. > > 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 > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 14/14] arm: multi_v7_defconfig: Enable CONFIG_RAVB 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu ` (12 preceding siblings ...) 2023-11-20 7:00 ` [PATCH 13/14] arm64: dts: renesas: rzg3s-smarc-som: Enable Ethernet interfaces Claudiu @ 2023-11-20 7:00 ` Claudiu 2023-11-20 8:44 ` Arnd Bergmann 2023-11-23 15:01 ` [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Linus Walleij 14 siblings, 1 reply; 53+ messages in thread From: Claudiu @ 2023-11-20 7:00 UTC (permalink / raw) To: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, linus.walleij, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz Cc: linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, claudiu.beznea, Claudiu Beznea From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> ravb driver is used by RZ/G1H. Enable it in multi_v7_defconfig. Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- arch/arm/configs/multi_v7_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index 10fd74bf85f9..9a04564566a7 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -272,6 +272,7 @@ CONFIG_KS8851=y CONFIG_LAN966X_SWITCH=m CONFIG_R8169=y CONFIG_SH_ETH=y +CONFIG_RAVB=y CONFIG_SMSC911X=y CONFIG_SNI_AVE=y CONFIG_STMMAC_ETH=y -- 2.39.2 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 14/14] arm: multi_v7_defconfig: Enable CONFIG_RAVB 2023-11-20 7:00 ` [PATCH 14/14] arm: multi_v7_defconfig: Enable CONFIG_RAVB Claudiu @ 2023-11-20 8:44 ` Arnd Bergmann 2023-11-20 8:56 ` claudiu beznea 2023-11-20 8:58 ` Geert Uytterhoeven 0 siblings, 2 replies; 53+ messages in thread From: Arnd Bergmann @ 2023-11-20 8:44 UTC (permalink / raw) To: Claudiu Beznea, Sergey Shtylyov, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, krzysztof.kozlowski+dt, Conor Dooley, Russell King, Geert Uytterhoeven, Magnus Damm, Michael Turquette, Stephen Boyd, Linus Walleij, Philipp Zabel, Marek Szyprowski, Alexandre Torgue, Andrew Davis, Mark Brown, Alexander Stein, eugen.hristev, sergei.shtylyov, Lad, Prabhakar, Biju Das Cc: Linux-Renesas, Netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, open list:GPIO SUBSYSTEM, Claudiu Beznea On Mon, Nov 20, 2023, at 08:00, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > ravb driver is used by RZ/G1H. Enable it in multi_v7_defconfig. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> We have a mix of =y and =m for ethernet drivers, and usually only have drivers built-in when they are frequently tested with NFS root booting. Do you need this as well, or could it be =m instead? Arnd ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 14/14] arm: multi_v7_defconfig: Enable CONFIG_RAVB 2023-11-20 8:44 ` Arnd Bergmann @ 2023-11-20 8:56 ` claudiu beznea 2023-11-20 8:58 ` Geert Uytterhoeven 1 sibling, 0 replies; 53+ messages in thread From: claudiu beznea @ 2023-11-20 8:56 UTC (permalink / raw) To: Arnd Bergmann, Sergey Shtylyov, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, krzysztof.kozlowski+dt, Conor Dooley, Russell King, Geert Uytterhoeven, Magnus Damm, Michael Turquette, Stephen Boyd, Linus Walleij, Philipp Zabel, Marek Szyprowski, Alexandre Torgue, Andrew Davis, Mark Brown, Alexander Stein, eugen.hristev, sergei.shtylyov, Lad, Prabhakar, Biju Das Cc: Linux-Renesas, Netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, open list:GPIO SUBSYSTEM, Claudiu Beznea On 20.11.2023 10:44, Arnd Bergmann wrote: > On Mon, Nov 20, 2023, at 08:00, Claudiu wrote: >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> ravb driver is used by RZ/G1H. Enable it in multi_v7_defconfig. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > We have a mix of =y and =m for ethernet drivers, and usually > only have drivers built-in when they are frequently tested > with NFS root booting. > > Do you need this as well, or could it be =m instead? I would prefer to have it =y as internal testing infrastructure is using NFS. Thank you, Claudiu Beznea > > Arnd ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 14/14] arm: multi_v7_defconfig: Enable CONFIG_RAVB 2023-11-20 8:44 ` Arnd Bergmann 2023-11-20 8:56 ` claudiu beznea @ 2023-11-20 8:58 ` Geert Uytterhoeven 2023-11-20 9:05 ` claudiu beznea 2023-11-27 10:01 ` Geert Uytterhoeven 1 sibling, 2 replies; 53+ messages in thread From: Geert Uytterhoeven @ 2023-11-20 8:58 UTC (permalink / raw) To: Arnd Bergmann Cc: Claudiu Beznea, Sergey Shtylyov, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, krzysztof.kozlowski+dt, Conor Dooley, Russell King, Magnus Damm, Michael Turquette, Stephen Boyd, Linus Walleij, Philipp Zabel, Marek Szyprowski, Alexandre Torgue, Andrew Davis, Mark Brown, Alexander Stein, eugen.hristev, sergei.shtylyov, Lad, Prabhakar, Biju Das, Linux-Renesas, Netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, open list:GPIO SUBSYSTEM, Claudiu Beznea On Mon, Nov 20, 2023 at 9:44 AM Arnd Bergmann <arnd@arndb.de> wrote: > On Mon, Nov 20, 2023, at 08:00, Claudiu wrote: > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > > > ravb driver is used by RZ/G1H. Enable it in multi_v7_defconfig. Used by: - iWave-RZ/G1M/G1N Qseven carrier board, - iWave-RZ/G1H Qseven board, - iWave-RZG1E SODIMM carrier board, - iWave-RZ/G1C single board computer. So I'd write "used by various iWave RZ/G1 development boards". > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > We have a mix of =y and =m for ethernet drivers, and usually > only have drivers built-in when they are frequently tested > with NFS root booting. > > Do you need this as well, or could it be =m instead? As the default chosen/bootargs for the iWave-RZ/G1M/G1N Qseven carrier board contains root=/dev/nfs, builtin is appropriate. The iWave-RZ/G1H Qseven board defaults to root=/dev/mmcblk0p1. 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 14/14] arm: multi_v7_defconfig: Enable CONFIG_RAVB 2023-11-20 8:58 ` Geert Uytterhoeven @ 2023-11-20 9:05 ` claudiu beznea 2023-11-27 10:01 ` Geert Uytterhoeven 1 sibling, 0 replies; 53+ messages in thread From: claudiu beznea @ 2023-11-20 9:05 UTC (permalink / raw) To: Geert Uytterhoeven, Arnd Bergmann Cc: Sergey Shtylyov, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, krzysztof.kozlowski+dt, Conor Dooley, Russell King, Magnus Damm, Michael Turquette, Stephen Boyd, Linus Walleij, Philipp Zabel, Marek Szyprowski, Alexandre Torgue, Andrew Davis, Mark Brown, Alexander Stein, eugen.hristev, sergei.shtylyov, Lad, Prabhakar, Biju Das, Linux-Renesas, Netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, open list:GPIO SUBSYSTEM, Claudiu Beznea On 20.11.2023 10:58, Geert Uytterhoeven wrote: > On Mon, Nov 20, 2023 at 9:44 AM Arnd Bergmann <arnd@arndb.de> wrote: >> On Mon, Nov 20, 2023, at 08:00, Claudiu wrote: >>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >>> >>> ravb driver is used by RZ/G1H. Enable it in multi_v7_defconfig. > > Used by: > - iWave-RZ/G1M/G1N Qseven carrier board, > - iWave-RZ/G1H Qseven board, > - iWave-RZG1E SODIMM carrier board, > - iWave-RZ/G1C single board computer. > > So I'd write "used by various iWave RZ/G1 development boards". OK, I'll update it in v2. I noticed it is needed while checking various bits on a RZ/G1H based board so I considered that if there is at least one user for it it is enough to have it enabled. Thank you, Claudiu Beznea > >>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> We have a mix of =y and =m for ethernet drivers, and usually >> only have drivers built-in when they are frequently tested >> with NFS root booting. >> >> Do you need this as well, or could it be =m instead? > > As the default chosen/bootargs for the iWave-RZ/G1M/G1N Qseven carrier > board contains root=/dev/nfs, builtin is appropriate. > The iWave-RZ/G1H Qseven board defaults to root=/dev/mmcblk0p1. > > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 14/14] arm: multi_v7_defconfig: Enable CONFIG_RAVB 2023-11-20 8:58 ` Geert Uytterhoeven 2023-11-20 9:05 ` claudiu beznea @ 2023-11-27 10:01 ` Geert Uytterhoeven 1 sibling, 0 replies; 53+ messages in thread From: Geert Uytterhoeven @ 2023-11-27 10:01 UTC (permalink / raw) To: Arnd Bergmann Cc: Claudiu Beznea, Sergey Shtylyov, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, krzysztof.kozlowski+dt, Conor Dooley, Russell King, Magnus Damm, Michael Turquette, Stephen Boyd, Linus Walleij, Philipp Zabel, Marek Szyprowski, Alexandre Torgue, Andrew Davis, Mark Brown, Alexander Stein, eugen.hristev, sergei.shtylyov, Lad, Prabhakar, Biju Das, Linux-Renesas, Netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, open list:GPIO SUBSYSTEM, Claudiu Beznea On Mon, Nov 20, 2023 at 9:58 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Mon, Nov 20, 2023 at 9:44 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Mon, Nov 20, 2023, at 08:00, Claudiu wrote: > > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > > > > > ravb driver is used by RZ/G1H. Enable it in multi_v7_defconfig. > > Used by: > - iWave-RZ/G1M/G1N Qseven carrier board, > - iWave-RZ/G1H Qseven board, > - iWave-RZG1E SODIMM carrier board, > - iWave-RZ/G1C single board computer. > > So I'd write "used by various iWave RZ/G1 development boards". Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> i.e. will queue in renesas-devel for v6.8, with the above updated. 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 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 00/14] renesas: rzg3s: Add support for Ethernet 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu ` (13 preceding siblings ...) 2023-11-20 7:00 ` [PATCH 14/14] arm: multi_v7_defconfig: Enable CONFIG_RAVB Claudiu @ 2023-11-23 15:01 ` Linus Walleij 14 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2023-11-23 15:01 UTC (permalink / raw) To: Claudiu Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux, geert+renesas, magnus.damm, mturquette, sboyd, p.zabel, arnd, m.szyprowski, alexandre.torgue, afd, broonie, alexander.stein, eugen.hristev, sergei.shtylyov, prabhakar.mahadev-lad.rj, biju.das.jz, linux-renesas-soc, netdev, devicetree, linux-kernel, linux-arm-kernel, linux-clk, linux-gpio, Claudiu Beznea On Mon, Nov 20, 2023 at 8:00 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > Patches 5-8 are pinctrl specific. I expect Geert to pick these once he's happy with them and merge them into his tree for pull request to my pinctrl tree. If you want some other merging approach then inform us! Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
end of thread, other threads:[~2023-12-06 11:48 UTC | newest] Thread overview: 53+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-20 7:00 [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Claudiu 2023-11-20 7:00 ` [PATCH 01/14] clk: renesas: rzg2l-cpg: Reuse code in rzg2l_cpg_reset() Claudiu 2023-11-23 15:48 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 02/14] clk: renesas: rzg2l-cpg: Check reset monitor registers Claudiu 2023-11-23 15:53 ` Geert Uytterhoeven 2023-11-23 17:19 ` claudiu beznea 2023-11-20 7:00 ` [PATCH 03/14] clk: renesas: rzg2l-cpg: Add support for MSTOP Claudiu 2023-11-23 16:35 ` Geert Uytterhoeven 2023-11-24 9:08 ` Geert Uytterhoeven 2023-11-27 7:37 ` claudiu beznea 2023-12-01 15:36 ` Geert Uytterhoeven 2023-11-24 9:24 ` claudiu beznea 2023-11-20 7:00 ` [PATCH 04/14] clk: renesas: r9a08g045-cpg: Add clock and reset support for ETH0 and ETH1 Claudiu 2023-12-01 15:59 ` Geert Uytterhoeven 2023-12-04 7:34 ` claudiu beznea 2023-11-20 7:00 ` [PATCH 05/14] pinctrl: renesas: rzg2l: Move arg in the main function block Claudiu 2023-12-01 16:15 ` Geert Uytterhoeven 2023-12-04 7:37 ` claudiu beznea 2023-11-20 7:00 ` [PATCH 06/14] pinctrl: renesas: rzg2l: Add pin configuration support for pinmux groups Claudiu 2023-12-01 16:51 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 07/14] pinctrl: renesas: rzg2l: Add support to select power source for Ethernet pins Claudiu 2023-12-01 17:11 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 08/14] pinctrl: renesas: rzg2l: Add output enable support Claudiu 2023-12-01 17:25 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 09/14] dt-bindings: net: renesas,etheravb: Document RZ/G3S support Claudiu 2023-11-20 15:39 ` Conor Dooley 2023-11-20 18:39 ` Sergey Shtylyov 2023-11-21 16:29 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 10/14] arm64: renesas: r9a08g045: Add Ethernet nodes Claudiu 2023-12-01 17:35 ` Geert Uytterhoeven 2023-12-04 7:41 ` claudiu beznea 2023-12-04 8:02 ` Geert Uytterhoeven 2023-12-04 8:38 ` claudiu beznea 2023-12-04 9:00 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 11/14] arm64: renesas: rzg3s-smarc-som: Invert the logic for SW_SD2_EN macro Claudiu 2023-12-06 10:33 ` Geert Uytterhoeven 2023-12-06 10:56 ` Geert Uytterhoeven 2023-12-06 11:11 ` claudiu beznea 2023-12-06 11:27 ` Geert Uytterhoeven 2023-12-06 11:31 ` claudiu beznea 2023-11-20 7:00 ` [PATCH 12/14] arm64: dts: renesas: Improve documentation for SW_SD0_DEV_SEL Claudiu 2023-11-20 8:41 ` Sergey Shtylyov 2023-12-06 11:03 ` Geert Uytterhoeven 2023-11-20 7:00 ` [PATCH 13/14] arm64: dts: renesas: rzg3s-smarc-som: Enable Ethernet interfaces Claudiu 2023-12-06 11:22 ` Geert Uytterhoeven 2023-12-06 11:48 ` claudiu beznea 2023-11-20 7:00 ` [PATCH 14/14] arm: multi_v7_defconfig: Enable CONFIG_RAVB Claudiu 2023-11-20 8:44 ` Arnd Bergmann 2023-11-20 8:56 ` claudiu beznea 2023-11-20 8:58 ` Geert Uytterhoeven 2023-11-20 9:05 ` claudiu beznea 2023-11-27 10:01 ` Geert Uytterhoeven 2023-11-23 15:01 ` [PATCH 00/14] renesas: rzg3s: Add support for Ethernet Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).