| Message ID | 1558711904-27278-2-git-send-email-gareth.williams.jx@renesas.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | clk: renesas: r9a06g032: Add clock domain support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
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
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
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
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
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>; };
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(+)