[PATCH v3 0/2] clk: renesas: r9a06g032: Add clock domain support

[PATCH v3 0/2] clk: renesas: r9a06g032: Add clock domain support

[Gareth Williams]

There are several clocks on the r9ag032 which are currently not enabled
in their drivers that can be delegated to clock domain system for power
management. Therefore add support for clock domain functionality to the
r9a06g032 clock driver after updating the relevant dt-bindings file.

Gareth Williams (2):
  dt-bindings: clock: renesas,r9a06g032-sysctrl: Document power Domains
  clk: renesas: r9a06g032: Add clock domain support

 .../bindings/clock/renesas,r9a06g032-sysctrl.txt   |   3 +
 drivers/clk/renesas/r9a06g032-clocks.c             | 229 ++++++++++++++-------
 2 files changed, 163 insertions(+), 69 deletions(-)

-- 
2.7.4

[PATCH v3 1/2] dt-bindings: clock: renesas,r9a06g032-sysctrl: Document power Domains

[Gareth Williams]

The driver is gaining power domain support, so add the new property
to the DT binding and update the examples.

Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
---
 Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt b/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt
index d60b997..6c706cd 100644
--- a/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt
+++ b/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt
@@ -13,6 +13,7 @@ Required Properties:
 	- external (optional) RGMII_REFCLK
   - clock-names: Must be:
         clock-names = "mclk", "rtc", "jtag", "rgmii_ref_ext";
+  - #power-domain-cells : Must be 0
 
 Examples
 --------
@@ -27,6 +28,7 @@ Examples
 		clocks = <&ext_mclk>, <&ext_rtc_clk>,
 				<&ext_jtag_clk>, <&ext_rgmii_ref>;
 		clock-names = "mclk", "rtc", "jtag", "rgmii_ref_ext";
+		#power-domain-cells = <0>;
 	};
 
   - Other nodes can use the clocks provided by SYSCTRL as in:
@@ -40,4 +42,5 @@ Examples
 		reg-io-width = <4>;
 		clocks = <&sysctrl R9A06G032_CLK_UART0>;
 		clock-names = "baudclk";
+		power-domains = <&sysctrl>;
 	};
-- 
2.7.4

Re: [PATCH v3 1/2] dt-bindings: clock: renesas,r9a06g032-sysctrl: Document power Domains

[Geert Uytterhoeven]

Hi Gareth,

On Fri, May 24, 2019 at 5:32 PM Gareth Williams
<gareth.williams.jx@renesas.com> wrote:
> The driver is gaining power domain support, so add the new property
> to the DT binding and update the examples.
>
> Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt
> +++ b/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt
@@ -40,4 +42,5 @@ Examples
>                 reg-io-width = <4>;
>                 clocks = <&sysctrl R9A06G032_CLK_UART0>;
>                 clock-names = "baudclk";
> +               power-domains = <&sysctrl>;

This is an interesting example: according to the driver,
R9A06G032_CLK_UART0, is not clock used for power management?

Oh, the real uart0 node in arch/arm/boot/dts/r9a06g032.dtsi uses

    clocks = <&sysctrl R9A06G032_CLK_UART0>, <&sysctrl R9A06G032_HCLK_UART0>;
    clock-names = "baudclk", "apb_pclk";

That does make sense...

With the above fixed:
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

RE: [PATCH v3 1/2] dt-bindings: clock: renesas,r9a06g032-sysctrl: Document power Domains

[Phil Edworthy]

Hi Geert,

On 28 May 2019 08:29 Geert Uytterhoeven wrote:
> On Fri, May 24, 2019 at 5:32 PM Gareth Williams wrote:
> > The driver is gaining power domain support, so add the new property to
> > the DT binding and update the examples.
> >
> > Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> 
> Thanks for your patch!
> 
> > ---
> > a/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.tx
> > t
> > +++ b/Documentation/devicetree/bindings/clock/renesas,r9a06g032-
> sysctr
> > +++ l.txt
> @@ -40,4 +42,5 @@ Examples
> >                 reg-io-width = <4>;
> >                 clocks = <&sysctrl R9A06G032_CLK_UART0>;
> >                 clock-names = "baudclk";
> > +               power-domains = <&sysctrl>;
> 
> This is an interesting example: according to the driver,
> R9A06G032_CLK_UART0, is not clock used for power management?
> 
> Oh, the real uart0 node in arch/arm/boot/dts/r9a06g032.dtsi uses
> 
>     clocks = <&sysctrl R9A06G032_CLK_UART0>, <&sysctrl
> R9A06G032_HCLK_UART0>;
>     clock-names = "baudclk", "apb_pclk";
> 
> That does make sense...
Note that the Synopsys DW uart driver already gets the "apb_pclk" clock, so
we don’t actually need to use clock domains to enable this clock.

This is also true for many of the peripheral drivers used on rzn1 (Synopsys
gpio controller, i2c controller, gmac, dmac, Arasan sdio controller). The
commit to add this clock to the i2c controller driver is my fault, as I was
following the pattern of the others.

Of the few drivers that don't already get the hclk/pclk used to access the
peripherals is the Synopsys spi controller (though that currently doesn’t
support runtime PM) and the USB Host controller.

BR
Phil
 
> With the above fixed:
> 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

Re: [PATCH v3 1/2] dt-bindings: clock: renesas,r9a06g032-sysctrl: Document power Domains

[Geert Uytterhoeven]

Hi Phil,

On Mon, Jun 3, 2019 at 10:29 AM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> On 28 May 2019 08:29 Geert Uytterhoeven wrote:
> > On Fri, May 24, 2019 at 5:32 PM Gareth Williams wrote:
> > > The driver is gaining power domain support, so add the new property to
> > > the DT binding and update the examples.
> > >
> > > Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>

> > > ---
> > > a/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt
> > > +++ b/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt
> > @@ -40,4 +42,5 @@ Examples
> > >                 reg-io-width = <4>;
> > >                 clocks = <&sysctrl R9A06G032_CLK_UART0>;
> > >                 clock-names = "baudclk";
> > > +               power-domains = <&sysctrl>;
> >
> > This is an interesting example: according to the driver,
> > R9A06G032_CLK_UART0, is not clock used for power management?
> >
> > Oh, the real uart0 node in arch/arm/boot/dts/r9a06g032.dtsi uses
> >
> >     clocks = <&sysctrl R9A06G032_CLK_UART0>, <&sysctrl
> > R9A06G032_HCLK_UART0>;
> >     clock-names = "baudclk", "apb_pclk";
> >
> > That does make sense...
> Note that the Synopsys DW uart driver already gets the "apb_pclk" clock, so
> we don’t actually need to use clock domains to enable this clock.

That is not necessarily a problem:
  1) DT describes hardware, not software policy,
  2) It doesn't hurt to enable a clock twice.

There are still some R-Car drivers that manage clocks themselves, but
we're slowly migrating away from that, where possible. If the driver
is e.g. shared with a platform without clock domains, we obviously cannot
do that.

So you can take out that code again, that's up to you.

> This is also true for many of the peripheral drivers used on rzn1 (Synopsys
> gpio controller, i2c controller, gmac, dmac, Arasan sdio controller). The
> commit to add this clock to the i2c controller driver is my fault, as I was
> following the pattern of the others.
>
> Of the few drivers that don't already get the hclk/pclk used to access the
> peripherals is the Synopsys spi controller (though that currently doesn’t
> support runtime PM) and the USB Host controller.

Good, so the latter will start working magically, I assume? ;-)

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

RE: [PATCH v3 1/2] dt-bindings: clock: renesas,r9a06g032-sysctrl: Document power Domains

[Phil Edworthy]

Hi Geert,

On 03 June 2019 09:39 Geert Uytterhoeven wrote:
> On Mon, Jun 3, 2019 at 10:29 AM Phil Edworthy wrote:
> > On 28 May 2019 08:29 Geert Uytterhoeven wrote:
> > > On Fri, May 24, 2019 at 5:32 PM Gareth Williams wrote:
> > > > The driver is gaining power domain support, so add the new property to
> > > > the DT binding and update the examples.
> > > >
> > > > Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> 
> > > > ---
> > > > a/Documentation/devicetree/bindings/clock/renesas,r9a06g032-
> sysctrl.txt
> > > > +++ b/Documentation/devicetree/bindings/clock/renesas,r9a06g032-
> sysctrl.txt
> > > @@ -40,4 +42,5 @@ Examples
> > > >                 reg-io-width = <4>;
> > > >                 clocks = <&sysctrl R9A06G032_CLK_UART0>;
> > > >                 clock-names = "baudclk";
> > > > +               power-domains = <&sysctrl>;
> > >
> > > This is an interesting example: according to the driver,
> > > R9A06G032_CLK_UART0, is not clock used for power management?
> > >
> > > Oh, the real uart0 node in arch/arm/boot/dts/r9a06g032.dtsi uses
> > >
> > >     clocks = <&sysctrl R9A06G032_CLK_UART0>, <&sysctrl
> > > R9A06G032_HCLK_UART0>;
> > >     clock-names = "baudclk", "apb_pclk";
> > >
> > > That does make sense...
> > Note that the Synopsys DW uart driver already gets the "apb_pclk" clock,
> so
> > we don’t actually need to use clock domains to enable this clock.
> 
> That is not necessarily a problem:
>   1) DT describes hardware, not software policy,
>   2) It doesn't hurt to enable a clock twice.
Yes, that was my take as well.

> There are still some R-Car drivers that manage clocks themselves, but
> we're slowly migrating away from that, where possible. If the driver
> is e.g. shared with a platform without clock domains, we obviously cannot
> do that.
> 
> So you can take out that code again, that's up to you.
I think leaving it as is best.

> > This is also true for many of the peripheral drivers used on rzn1 (Synopsys
> > gpio controller, i2c controller, gmac, dmac, Arasan sdio controller). The
> > commit to add this clock to the i2c controller driver is my fault, as I was
> > following the pattern of the others.
> >
> > Of the few drivers that don't already get the hclk/pclk used to access the
> > peripherals is the Synopsys spi controller (though that currently doesn’t
> > support runtime PM) and the USB Host controller.
> 
> Good, so the latter will start working magically, I assume? ;-)
Yes, except for the usb PLL. The rzn1 has a mode bit that switches USB
between 2xHost and 1xHost + 1xFunc, however the PLL must be started after
the mode bit has been set. That still needs to be implemented...

BR
Phil
 
> 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