| Message ID | 20240410122657.2051132-1-claudiu.beznea.uj@bp.renesas.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | clk: renesas: rzg2l: Add support for power domains | expand |
Hi Claudiu, CC pmdomain, watchdog On Wed, Apr 10, 2024 at 2:27 PM Claudiu <claudiu.beznea@tuxon.dev> wrote: > Series adds support for power domains on rzg2l driver. > > RZ/G2L kind of devices support a functionality called MSTOP (module > stop/standby). According to hardware manual the module could be switch > to standby after its clocks are disabled. The reverse order of operation > should be done when enabling a module (get the module out of standby, > enable its clocks etc). > > In [1] the MSTOP settings were implemented by adding code in driver > to attach the MSTOP state to the IP clocks. But it has been proposed > to implement it as power domain. The result is this series. > > Along with MSTOP functionality there is also module power down > functionality (which is currently available only on RZ/G3S). This has > been also implemented through power domains. > > The DT bindings were updated with power domain IDs (plain integers > that matches the DT with driver data structures). The current DT > bindings were updated with module IDs for the modules listed in tables > with name "Registers for Module Standby Mode" (see HW manual) exception > being RZ/G3S where, due to the power down functionality, the DDR, > TZCDDR, OTFDE_DDR were also added, to avoid system being blocked due > to the following lines of code from patch 6/9. > > + /* Prepare for power down the BUSes in power down mode. */ > + if (info->pm_domain_pwrdn_mstop) > + writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP); > > Domain IDs were added to all SoC specific bindings. > > Thank you, > Claudiu Beznea > > Changes in v3: > - collected tags > - dinamically detect if a SCIF is serial console and populate > pd->suspend_check > - dropped patch 09/10 from v2 Thanks for the update! I have provided my R-b for all patches, and the usual path for these patches would be for me to queue patches 1-8 in renesas-clk for v6.10, and to queue 9 in renesas-devel. However: 1. I had missed before the pmdomain people weren't CCed before, they still might have some comments, 2. Patch 9 has a hard dependency on the rest of the series, so it has to wait one more cycle, 3. Adding the watchdog domain has a dependency on [1]. 2 and 2 may be resolved using an immutable branch. Are my assumptions correct? Thanks! [1] "[PATCH RESEND v8 09/10] watchdog: rzg2l_wdt: Power on the PM domain in rzg2l_wdt_restart()" https://lore.kernel.org/all/20240410134044.2138310-10-claudiu.beznea.uj@bp.renesas.com > Changes in v2: > - addressed review comments > - dropped: > - dt-bindings: clock: r9a09g011-cpg: Add always-on power domain IDs > - clk: renesas: r9a07g043: Add initial support for power domains > - clk: renesas: r9a07g044: Add initial support for power domains > - clk: renesas: r9a09g011: Add initial support for power domains > - clk: renesas: r9a09g011: Add initial support for power domains > - arm64: dts: renesas: r9a07g043: Update #power-domain-cells = <1> > - arm64: dts: renesas: r9a07g044: Update #power-domain-cells = <1> > - arm64: dts: renesas: r9a07g054: Update #power-domain-cells = <1> > - arm64: dts: renesas: r9a09g011: Update #power-domain-cells = <1> > as suggested in the review process > - dropped "arm64: dts: renesas: rzg3s-smarc-som: Guard the ethernet IRQ > GPIOs with proper flags" patch as it was integrated > - added suspend to RAM support > - collected tag > > [1] https://lore.kernel.org/all/20231120070024.4079344-4-claudiu.beznea.uj@bp.renesas.com/ > > > Claudiu Beznea (9): > dt-bindings: clock: r9a07g043-cpg: Add power domain IDs > dt-bindings: clock: r9a07g044-cpg: Add power domain IDs > dt-bindings: clock: r9a07g054-cpg: Add power domain IDs > dt-bindings: clock: r9a08g045-cpg: Add power domain IDs > dt-bindings: clock: renesas,rzg2l-cpg: Update #power-domain-cells = > <1> for RZ/G3S > clk: renesas: rzg2l: Extend power domain support > clk: renesas: r9a08g045: Add support for power domains > clk: renesas: rzg2l-cpg: Add suspend/resume support for power domains > arm64: dts: renesas: r9a08g045: Update #power-domain-cells = <1> > > .../bindings/clock/renesas,rzg2l-cpg.yaml | 18 +- > arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 20 +- > drivers/clk/renesas/r9a08g045-cpg.c | 61 ++++ > drivers/clk/renesas/rzg2l-cpg.c | 269 +++++++++++++++++- > drivers/clk/renesas/rzg2l-cpg.h | 77 +++++ > include/dt-bindings/clock/r9a07g043-cpg.h | 52 ++++ > include/dt-bindings/clock/r9a07g044-cpg.h | 58 ++++ > include/dt-bindings/clock/r9a07g054-cpg.h | 58 ++++ > include/dt-bindings/clock/r9a08g045-cpg.h | 70 +++++ > 9 files changed, 659 insertions(+), 24 deletions(-) Gr{oetje,eeting}s, Geert
Hi, Geert, On 11.04.2024 18:30, Geert Uytterhoeven wrote: > Hi Claudiu, > > CC pmdomain, watchdog > > On Wed, Apr 10, 2024 at 2:27 PM Claudiu <claudiu.beznea@tuxon.dev> wrote: >> Series adds support for power domains on rzg2l driver. >> >> RZ/G2L kind of devices support a functionality called MSTOP (module >> stop/standby). According to hardware manual the module could be switch >> to standby after its clocks are disabled. The reverse order of operation >> should be done when enabling a module (get the module out of standby, >> enable its clocks etc). >> >> In [1] the MSTOP settings were implemented by adding code in driver >> to attach the MSTOP state to the IP clocks. But it has been proposed >> to implement it as power domain. The result is this series. >> >> Along with MSTOP functionality there is also module power down >> functionality (which is currently available only on RZ/G3S). This has >> been also implemented through power domains. >> >> The DT bindings were updated with power domain IDs (plain integers >> that matches the DT with driver data structures). The current DT >> bindings were updated with module IDs for the modules listed in tables >> with name "Registers for Module Standby Mode" (see HW manual) exception >> being RZ/G3S where, due to the power down functionality, the DDR, >> TZCDDR, OTFDE_DDR were also added, to avoid system being blocked due >> to the following lines of code from patch 6/9. >> >> + /* Prepare for power down the BUSes in power down mode. */ >> + if (info->pm_domain_pwrdn_mstop) >> + writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP); >> >> Domain IDs were added to all SoC specific bindings. >> >> Thank you, >> Claudiu Beznea >> >> Changes in v3: >> - collected tags >> - dinamically detect if a SCIF is serial console and populate >> pd->suspend_check >> - dropped patch 09/10 from v2 > > Thanks for the update! > > I have provided my R-b for all patches, and the usual path for these > patches would be for me to queue patches 1-8 in renesas-clk for v6.10, > and to queue 9 in renesas-devel. > > However: > 1. I had missed before the pmdomain people weren't CCed before, > they still might have some comments, My bad here, I missed it too. > 2. Patch 9 has a hard dependency on the rest of the series, so > it has to wait one more cycle, I think 5/9 should also wait to avoid binding validation failures. > 3. Adding the watchdog domain has a dependency on [1]. Adding the code for it in patch 7/9 w/o passing it as reference to watchdog node (as in patch 9/9) is harmless. The previous behavior will be in place. At the moment the watchdog domain initialization code is not in patch 7/9 and the patch 9/9 has reference to watchdog domain to pass the DT binding validation. The probe will fail though, as I wasn't sure what should be better to drop: device probe or reset functionality. I mentioned it in patch for suggestions. > > 2 and 2 may be resolved using an immutable branch. 2 and 3? Immutable branch should be good, AFAICT. If that would be the strategy I can send an update to also add the initialization data for watchdog domain in 7/9. Or I can send an update afterwards. Please let me know how would you prefer. Thank you, Claudiu Beznea > Are my assumptions correct? > > Thanks! > > [1] "[PATCH RESEND v8 09/10] watchdog: rzg2l_wdt: Power on the PM > domain in rzg2l_wdt_restart()" > https://lore.kernel.org/all/20240410134044.2138310-10-claudiu.beznea.uj@bp.renesas.com > >> Changes in v2: >> - addressed review comments >> - dropped: >> - dt-bindings: clock: r9a09g011-cpg: Add always-on power domain IDs >> - clk: renesas: r9a07g043: Add initial support for power domains >> - clk: renesas: r9a07g044: Add initial support for power domains >> - clk: renesas: r9a09g011: Add initial support for power domains >> - clk: renesas: r9a09g011: Add initial support for power domains >> - arm64: dts: renesas: r9a07g043: Update #power-domain-cells = <1> >> - arm64: dts: renesas: r9a07g044: Update #power-domain-cells = <1> >> - arm64: dts: renesas: r9a07g054: Update #power-domain-cells = <1> >> - arm64: dts: renesas: r9a09g011: Update #power-domain-cells = <1> >> as suggested in the review process >> - dropped "arm64: dts: renesas: rzg3s-smarc-som: Guard the ethernet IRQ >> GPIOs with proper flags" patch as it was integrated >> - added suspend to RAM support >> - collected tag >> >> [1] https://lore.kernel.org/all/20231120070024.4079344-4-claudiu.beznea.uj@bp.renesas.com/ >> >> >> Claudiu Beznea (9): >> dt-bindings: clock: r9a07g043-cpg: Add power domain IDs >> dt-bindings: clock: r9a07g044-cpg: Add power domain IDs >> dt-bindings: clock: r9a07g054-cpg: Add power domain IDs >> dt-bindings: clock: r9a08g045-cpg: Add power domain IDs >> dt-bindings: clock: renesas,rzg2l-cpg: Update #power-domain-cells = >> <1> for RZ/G3S >> clk: renesas: rzg2l: Extend power domain support >> clk: renesas: r9a08g045: Add support for power domains >> clk: renesas: rzg2l-cpg: Add suspend/resume support for power domains >> arm64: dts: renesas: r9a08g045: Update #power-domain-cells = <1> >> >> .../bindings/clock/renesas,rzg2l-cpg.yaml | 18 +- >> arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 20 +- >> drivers/clk/renesas/r9a08g045-cpg.c | 61 ++++ >> drivers/clk/renesas/rzg2l-cpg.c | 269 +++++++++++++++++- >> drivers/clk/renesas/rzg2l-cpg.h | 77 +++++ >> include/dt-bindings/clock/r9a07g043-cpg.h | 52 ++++ >> include/dt-bindings/clock/r9a07g044-cpg.h | 58 ++++ >> include/dt-bindings/clock/r9a07g054-cpg.h | 58 ++++ >> include/dt-bindings/clock/r9a08g045-cpg.h | 70 +++++ >> 9 files changed, 659 insertions(+), 24 deletions(-) > > Gr{oetje,eeting}s, > > Geert >
On Wed, 10 Apr 2024 at 14:27, Claudiu <claudiu.beznea@tuxon.dev> wrote: > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Hi, > > Series adds support for power domains on rzg2l driver. > > RZ/G2L kind of devices support a functionality called MSTOP (module > stop/standby). According to hardware manual the module could be switch > to standby after its clocks are disabled. The reverse order of operation > should be done when enabling a module (get the module out of standby, > enable its clocks etc). > > In [1] the MSTOP settings were implemented by adding code in driver > to attach the MSTOP state to the IP clocks. But it has been proposed > to implement it as power domain. The result is this series. > > Along with MSTOP functionality there is also module power down > functionality (which is currently available only on RZ/G3S). This has > been also implemented through power domains. > > The DT bindings were updated with power domain IDs (plain integers > that matches the DT with driver data structures). The current DT > bindings were updated with module IDs for the modules listed in tables > with name "Registers for Module Standby Mode" (see HW manual) exception > being RZ/G3S where, due to the power down functionality, the DDR, > TZCDDR, OTFDE_DDR were also added, to avoid system being blocked due > to the following lines of code from patch 6/9. > > + /* Prepare for power down the BUSes in power down mode. */ > + if (info->pm_domain_pwrdn_mstop) > + writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP); > > Domain IDs were added to all SoC specific bindings. > > Thank you, > Claudiu Beznea > > Changes in v3: > - collected tags > - dinamically detect if a SCIF is serial console and populate > pd->suspend_check > - dropped patch 09/10 from v2 > > Changes in v2: > - addressed review comments > - dropped: > - dt-bindings: clock: r9a09g011-cpg: Add always-on power domain IDs > - clk: renesas: r9a07g043: Add initial support for power domains > - clk: renesas: r9a07g044: Add initial support for power domains > - clk: renesas: r9a09g011: Add initial support for power domains > - clk: renesas: r9a09g011: Add initial support for power domains > - arm64: dts: renesas: r9a07g043: Update #power-domain-cells = <1> > - arm64: dts: renesas: r9a07g044: Update #power-domain-cells = <1> > - arm64: dts: renesas: r9a07g054: Update #power-domain-cells = <1> > - arm64: dts: renesas: r9a09g011: Update #power-domain-cells = <1> > as suggested in the review process > - dropped "arm64: dts: renesas: rzg3s-smarc-som: Guard the ethernet IRQ > GPIOs with proper flags" patch as it was integrated > - added suspend to RAM support > - collected tag > > [1] https://lore.kernel.org/all/20231120070024.4079344-4-claudiu.beznea.uj@bp.renesas.com/ > > > Claudiu Beznea (9): > dt-bindings: clock: r9a07g043-cpg: Add power domain IDs > dt-bindings: clock: r9a07g044-cpg: Add power domain IDs > dt-bindings: clock: r9a07g054-cpg: Add power domain IDs > dt-bindings: clock: r9a08g045-cpg: Add power domain IDs > dt-bindings: clock: renesas,rzg2l-cpg: Update #power-domain-cells = > <1> for RZ/G3S > clk: renesas: rzg2l: Extend power domain support > clk: renesas: r9a08g045: Add support for power domains > clk: renesas: rzg2l-cpg: Add suspend/resume support for power domains In particular patches like the above I would appreciate to be cced on to help review, but I understand that it's easy to miss in cases like this. That said, maybe we should start separating and moving the power-domain parts out from the clk directory into the pmdomain directory instead, that should improve these situations!? > arm64: dts: renesas: r9a08g045: Update #power-domain-cells = <1> > > .../bindings/clock/renesas,rzg2l-cpg.yaml | 18 +- > arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 20 +- > drivers/clk/renesas/r9a08g045-cpg.c | 61 ++++ > drivers/clk/renesas/rzg2l-cpg.c | 269 +++++++++++++++++- > drivers/clk/renesas/rzg2l-cpg.h | 77 +++++ > include/dt-bindings/clock/r9a07g043-cpg.h | 52 ++++ > include/dt-bindings/clock/r9a07g044-cpg.h | 58 ++++ > include/dt-bindings/clock/r9a07g054-cpg.h | 58 ++++ > include/dt-bindings/clock/r9a08g045-cpg.h | 70 +++++ > 9 files changed, 659 insertions(+), 24 deletions(-) > Kind regards Uffe
Hi Ulf, On Fri, Apr 12, 2024 at 1:31 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > That said, maybe we should start separating and moving the > power-domain parts out from the clk directory into the pmdomain > directory instead, that should improve these situations!? The clk and pmdomain functions are tied rather closely together on Renesas SoCs, that's why the clock drivers are also pmdomain providers. Gr{oetje,eeting}s, Geert
On Mon, 15 Apr 2024 at 09:28, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Ulf, > > On Fri, Apr 12, 2024 at 1:31 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > That said, maybe we should start separating and moving the > > power-domain parts out from the clk directory into the pmdomain > > directory instead, that should improve these situations!? > > The clk and pmdomain functions are tied rather closely together on > Renesas SoCs, that's why the clock drivers are also pmdomain providers. > I understand, it's your call to make! Anyway, I just wanted to help with reviews and to make sure genpd providers get implemented in a nice and proper way. Kind regards Uffe
Hi, Ulf, On 16.04.2024 14:14, Ulf Hansson wrote: > On Mon, 15 Apr 2024 at 09:28, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> >> Hi Ulf, >> >> On Fri, Apr 12, 2024 at 1:31 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> That said, maybe we should start separating and moving the >>> power-domain parts out from the clk directory into the pmdomain >>> directory instead, that should improve these situations!? >> >> The clk and pmdomain functions are tied rather closely together on >> Renesas SoCs, that's why the clock drivers are also pmdomain providers. >> > > I understand, it's your call to make! > > Anyway, I just wanted to help with reviews and to make sure genpd > providers get implemented in a nice and proper way. I'll keep in mind to also add you and PM domain list for future patches, if any. Would you prefer to re-send this series and cc you and pm domain list? Thank you, Claudiu Beznea > > Kind regards > Uffe
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Hi, Series adds support for power domains on rzg2l driver. RZ/G2L kind of devices support a functionality called MSTOP (module stop/standby). According to hardware manual the module could be switch to standby after its clocks are disabled. The reverse order of operation should be done when enabling a module (get the module out of standby, enable its clocks etc). In [1] the MSTOP settings were implemented by adding code in driver to attach the MSTOP state to the IP clocks. But it has been proposed to implement it as power domain. The result is this series. Along with MSTOP functionality there is also module power down functionality (which is currently available only on RZ/G3S). This has been also implemented through power domains. The DT bindings were updated with power domain IDs (plain integers that matches the DT with driver data structures). The current DT bindings were updated with module IDs for the modules listed in tables with name "Registers for Module Standby Mode" (see HW manual) exception being RZ/G3S where, due to the power down functionality, the DDR, TZCDDR, OTFDE_DDR were also added, to avoid system being blocked due to the following lines of code from patch 6/9. + /* Prepare for power down the BUSes in power down mode. */ + if (info->pm_domain_pwrdn_mstop) + writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP); Domain IDs were added to all SoC specific bindings. Thank you, Claudiu Beznea Changes in v3: - collected tags - dinamically detect if a SCIF is serial console and populate pd->suspend_check - dropped patch 09/10 from v2 Changes in v2: - addressed review comments - dropped: - dt-bindings: clock: r9a09g011-cpg: Add always-on power domain IDs - clk: renesas: r9a07g043: Add initial support for power domains - clk: renesas: r9a07g044: Add initial support for power domains - clk: renesas: r9a09g011: Add initial support for power domains - clk: renesas: r9a09g011: Add initial support for power domains - arm64: dts: renesas: r9a07g043: Update #power-domain-cells = <1> - arm64: dts: renesas: r9a07g044: Update #power-domain-cells = <1> - arm64: dts: renesas: r9a07g054: Update #power-domain-cells = <1> - arm64: dts: renesas: r9a09g011: Update #power-domain-cells = <1> as suggested in the review process - dropped "arm64: dts: renesas: rzg3s-smarc-som: Guard the ethernet IRQ GPIOs with proper flags" patch as it was integrated - added suspend to RAM support - collected tag [1] https://lore.kernel.org/all/20231120070024.4079344-4-claudiu.beznea.uj@bp.renesas.com/ Claudiu Beznea (9): dt-bindings: clock: r9a07g043-cpg: Add power domain IDs dt-bindings: clock: r9a07g044-cpg: Add power domain IDs dt-bindings: clock: r9a07g054-cpg: Add power domain IDs dt-bindings: clock: r9a08g045-cpg: Add power domain IDs dt-bindings: clock: renesas,rzg2l-cpg: Update #power-domain-cells = <1> for RZ/G3S clk: renesas: rzg2l: Extend power domain support clk: renesas: r9a08g045: Add support for power domains clk: renesas: rzg2l-cpg: Add suspend/resume support for power domains arm64: dts: renesas: r9a08g045: Update #power-domain-cells = <1> .../bindings/clock/renesas,rzg2l-cpg.yaml | 18 +- arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 20 +- drivers/clk/renesas/r9a08g045-cpg.c | 61 ++++ drivers/clk/renesas/rzg2l-cpg.c | 269 +++++++++++++++++- drivers/clk/renesas/rzg2l-cpg.h | 77 +++++ include/dt-bindings/clock/r9a07g043-cpg.h | 52 ++++ include/dt-bindings/clock/r9a07g044-cpg.h | 58 ++++ include/dt-bindings/clock/r9a07g054-cpg.h | 58 ++++ include/dt-bindings/clock/r9a08g045-cpg.h | 70 +++++ 9 files changed, 659 insertions(+), 24 deletions(-)