| Message ID | 1447958344-836-24-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Simon Horman |
| Headers | show |
Hi Geert, Thank you for the patch. On Thursday 19 November 2015 19:39:02 Geert Uytterhoeven wrote: > Add the device node for the external SCIF_CLK. > The presence of the SCIF_CLK crystal and its clock frequency depend on > the actual board. > > Add the two optional clock sources (ZS_CLK and SCIF_CLK for the internal > resp. external clock) for the Baud Rate Generator for External Clock > (BRG) to all SCIF and HSCIF device nodes. > > This increases the range and accuracy of supported baud rates on > (H)SCIF. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > arch/arm64/boot/dts/renesas/r8a7795.dtsi | 74 +++++++++++++++++++---------- > 1 file changed, 52 insertions(+), 22 deletions(-) > > diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi > b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index > 53a2a8fb42b7480c..25900761cfde201e 100644 > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi > @@ -84,6 +84,14 @@ > status = "disabled"; > }; > > + /* External SCIF clock - to be overridden by boards that provide it */ > + scif_clk: scif { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <0>; > + status = "disabled"; > + }; I have mixed feelings about this. Defining an external clock that isn't present on the board isn't very clean, even more so when the clock has such a generic name. Wouldn't it be better to let board files define the clock when they need it ? I know it would require board files to override the clocks and clock-names property too. Maybe we need to extend the DTS syntax to allow extending list properties instead of overriding them completely ? > soc { > compatible = "simple-bus"; > interrupt-parent = <&gic>; > @@ -362,8 +370,10 @@ > compatible = "renesas,hscif-r8a7795", "renesas,hscif"; > reg = <0 0xe6540000 0 96>; > interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&cpg CPG_MOD 520>; > - clock-names = "fck"; > + clocks = <&cpg CPG_MOD 520>, > + <&cpg CPG_CORE R8A7795_CLK_S3D1>, > + <&scif_clk>; > + clock-names = "fck", "int_clk", "scif_clk"; > dmas = <&dmac1 0x31>, <&dmac1 0x30>; > dma-names = "tx", "rx"; > power-domains = <&cpg>; > @@ -374,8 +384,10 @@ > compatible = "renesas,hscif-r8a7795", "renesas,hscif"; > reg = <0 0xe6550000 0 96>; > interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&cpg CPG_MOD 519>; > - clock-names = "fck"; > + clocks = <&cpg CPG_MOD 519>, > + <&cpg CPG_CORE R8A7795_CLK_S3D1>, > + <&scif_clk>; > + clock-names = "fck", "int_clk", "scif_clk"; > dmas = <&dmac1 0x33>, <&dmac1 0x32>; > dma-names = "tx", "rx"; > power-domains = <&cpg>; > @@ -386,8 +398,10 @@ > compatible = "renesas,hscif-r8a7795", "renesas,hscif"; > reg = <0 0xe6560000 0 96>; > interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&cpg CPG_MOD 518>; > - clock-names = "fck"; > + clocks = <&cpg CPG_MOD 518>, > + <&cpg CPG_CORE R8A7795_CLK_S3D1>, > + <&scif_clk>; > + clock-names = "fck", "int_clk", "scif_clk"; > dmas = <&dmac1 0x35>, <&dmac1 0x34>; > dma-names = "tx", "rx"; > power-domains = <&cpg>; > @@ -398,8 +412,10 @@ > compatible = "renesas,hscif-r8a7795", "renesas,hscif"; > reg = <0 0xe66a0000 0 96>; > interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&cpg CPG_MOD 517>; > - clock-names = "fck"; > + clocks = <&cpg CPG_MOD 517>, > + <&cpg CPG_CORE R8A7795_CLK_S3D1>, > + <&scif_clk>; > + clock-names = "fck", "int_clk", "scif_clk"; > dmas = <&dmac0 0x37>, <&dmac0 0x36>; > dma-names = "tx", "rx"; > power-domains = <&cpg>; > @@ -410,8 +426,10 @@ > compatible = "renesas,hscif-r8a7795", "renesas,hscif"; > reg = <0 0xe66b0000 0 96>; > interrupts = <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&cpg CPG_MOD 516>; > - clock-names = "fck"; > + clocks = <&cpg CPG_MOD 516>, > + <&cpg CPG_CORE R8A7795_CLK_S3D1>, > + <&scif_clk>; > + clock-names = "fck", "int_clk", "scif_clk"; > dmas = <&dmac0 0x39>, <&dmac0 0x38>; > dma-names = "tx", "rx"; > power-domains = <&cpg>; > @@ -422,8 +440,10 @@ > compatible = "renesas,scif-r8a7795", "renesas,scif"; > reg = <0 0xe6e60000 0 64>; > interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&cpg CPG_MOD 207>; > - clock-names = "fck"; > + clocks = <&cpg CPG_MOD 207>, > + <&cpg CPG_CORE R8A7795_CLK_S3D1>, > + <&scif_clk>; > + clock-names = "fck", "int_clk", "scif_clk"; > dmas = <&dmac1 0x51>, <&dmac1 0x50>; > dma-names = "tx", "rx"; > power-domains = <&cpg>; > @@ -434,8 +454,10 @@ > compatible = "renesas,scif-r8a7795", "renesas,scif"; > reg = <0 0xe6e68000 0 64>; > interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&cpg CPG_MOD 206>; > - clock-names = "fck"; > + clocks = <&cpg CPG_MOD 206>, > + <&cpg CPG_CORE R8A7795_CLK_S3D1>, > + <&scif_clk>; > + clock-names = "fck", "int_clk", "scif_clk"; > dmas = <&dmac1 0x53>, <&dmac1 0x52>; > dma-names = "tx", "rx"; > power-domains = <&cpg>; > @@ -446,8 +468,10 @@ > compatible = "renesas,scif-r8a7795", "renesas,scif"; > reg = <0 0xe6e88000 0 64>; > interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&cpg CPG_MOD 310>; > - clock-names = "fck"; > + clocks = <&cpg CPG_MOD 310>, > + <&cpg CPG_CORE R8A7795_CLK_S3D1>, > + <&scif_clk>; > + clock-names = "fck", "int_clk", "scif_clk"; > dmas = <&dmac1 0x13>, <&dmac1 0x12>; > dma-names = "tx", "rx"; > power-domains = <&cpg>; > @@ -458,8 +482,10 @@ > compatible = "renesas,scif-r8a7795", "renesas,scif"; > reg = <0 0xe6c50000 0 64>; > interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&cpg CPG_MOD 204>; > - clock-names = "fck"; > + clocks = <&cpg CPG_MOD 204>, > + <&cpg CPG_CORE R8A7795_CLK_S3D1>, > + <&scif_clk>; > + clock-names = "fck", "int_clk", "scif_clk"; > dmas = <&dmac0 0x57>, <&dmac0 0x56>; > dma-names = "tx", "rx"; > power-domains = <&cpg>; > @@ -470,8 +496,10 @@ > compatible = "renesas,scif-r8a7795", "renesas,scif"; > reg = <0 0xe6c40000 0 64>; > interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&cpg CPG_MOD 203>; > - clock-names = "fck"; > + clocks = <&cpg CPG_MOD 203>, > + <&cpg CPG_CORE R8A7795_CLK_S3D1>, > + <&scif_clk>; > + clock-names = "fck", "int_clk", "scif_clk"; > dmas = <&dmac0 0x59>, <&dmac0 0x58>; > dma-names = "tx", "rx"; > power-domains = <&cpg>; > @@ -482,8 +510,10 @@ > compatible = "renesas,scif-r8a7795", "renesas,scif"; > reg = <0 0xe6f30000 0 64>; > interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&cpg CPG_MOD 202>; > - clock-names = "fck"; > + clocks = <&cpg CPG_MOD 202>, > + <&cpg CPG_CORE R8A7795_CLK_S3D1>, > + <&scif_clk>; > + clock-names = "fck", "int_clk", "scif_clk"; > dmas = <&dmac1 0x5b>, <&dmac1 0x5a>; > dma-names = "tx", "rx"; > power-domains = <&cpg>;
Hi Laurent, On Thu, Nov 19, 2015 at 10:07 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Thursday 19 November 2015 19:39:02 Geert Uytterhoeven wrote: >> Add the device node for the external SCIF_CLK. >> The presence of the SCIF_CLK crystal and its clock frequency depend on >> the actual board. >> >> Add the two optional clock sources (ZS_CLK and SCIF_CLK for the internal >> resp. external clock) for the Baud Rate Generator for External Clock >> (BRG) to all SCIF and HSCIF device nodes. >> >> This increases the range and accuracy of supported baud rates on >> (H)SCIF. >> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >> --- >> arch/arm64/boot/dts/renesas/r8a7795.dtsi | 74 +++++++++++++++++++---------- >> 1 file changed, 52 insertions(+), 22 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi >> b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index >> 53a2a8fb42b7480c..25900761cfde201e 100644 >> --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi >> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi >> @@ -84,6 +84,14 @@ >> status = "disabled"; >> }; >> >> + /* External SCIF clock - to be overridden by boards that provide it */ >> + scif_clk: scif { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <0>; >> + status = "disabled"; >> + }; > > I have mixed feelings about this. Defining an external clock that isn't > present on the board isn't very clean, even more so when the clock has such a We have precedence of optional external clocks (can_clk, audio_clk_*). The SoC datasheet clearly calls it "scif_clk", so that makes it an ABI, IMHO. > generic name. Wouldn't it be better to let board files define the clock when > they need it ? I know it would require board files to override the clocks and > clock-names property too. Maybe we need to extend the DTS syntax to allow > extending list properties instead of overriding them completely ? As scif_clk is shared between all (H)SCIF instances, that would mean overriding the clock and clock-names for all of them, which is quite a tedious task. Most boards seem to provide a SCIF_CLK, to allow having "perfect" standard baud rates. Combined all of the above, I think it's sufficiently generic to keep it that way. Note that it's different for (H)SCK: these are per-(H)SCIF inputs, and depend even more on board layout. Adding individual zero-frequency clock nodes for them would preclude e.g. connecting all (H)SCK inputs to the same crystal. Hence I didn't add them, and you do have to override all clocks and clock-names of a node if you want to add an (H)SCK clock input (been there, done that for testing; long live DT overlays). 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index 53a2a8fb42b7480c..25900761cfde201e 100644 --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi @@ -84,6 +84,14 @@ status = "disabled"; }; + /* External SCIF clock - to be overridden by boards that provide it */ + scif_clk: scif { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <0>; + status = "disabled"; + }; + soc { compatible = "simple-bus"; interrupt-parent = <&gic>; @@ -362,8 +370,10 @@ compatible = "renesas,hscif-r8a7795", "renesas,hscif"; reg = <0 0xe6540000 0 96>; interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&cpg CPG_MOD 520>; - clock-names = "fck"; + clocks = <&cpg CPG_MOD 520>, + <&cpg CPG_CORE R8A7795_CLK_S3D1>, + <&scif_clk>; + clock-names = "fck", "int_clk", "scif_clk"; dmas = <&dmac1 0x31>, <&dmac1 0x30>; dma-names = "tx", "rx"; power-domains = <&cpg>; @@ -374,8 +384,10 @@ compatible = "renesas,hscif-r8a7795", "renesas,hscif"; reg = <0 0xe6550000 0 96>; interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&cpg CPG_MOD 519>; - clock-names = "fck"; + clocks = <&cpg CPG_MOD 519>, + <&cpg CPG_CORE R8A7795_CLK_S3D1>, + <&scif_clk>; + clock-names = "fck", "int_clk", "scif_clk"; dmas = <&dmac1 0x33>, <&dmac1 0x32>; dma-names = "tx", "rx"; power-domains = <&cpg>; @@ -386,8 +398,10 @@ compatible = "renesas,hscif-r8a7795", "renesas,hscif"; reg = <0 0xe6560000 0 96>; interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&cpg CPG_MOD 518>; - clock-names = "fck"; + clocks = <&cpg CPG_MOD 518>, + <&cpg CPG_CORE R8A7795_CLK_S3D1>, + <&scif_clk>; + clock-names = "fck", "int_clk", "scif_clk"; dmas = <&dmac1 0x35>, <&dmac1 0x34>; dma-names = "tx", "rx"; power-domains = <&cpg>; @@ -398,8 +412,10 @@ compatible = "renesas,hscif-r8a7795", "renesas,hscif"; reg = <0 0xe66a0000 0 96>; interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&cpg CPG_MOD 517>; - clock-names = "fck"; + clocks = <&cpg CPG_MOD 517>, + <&cpg CPG_CORE R8A7795_CLK_S3D1>, + <&scif_clk>; + clock-names = "fck", "int_clk", "scif_clk"; dmas = <&dmac0 0x37>, <&dmac0 0x36>; dma-names = "tx", "rx"; power-domains = <&cpg>; @@ -410,8 +426,10 @@ compatible = "renesas,hscif-r8a7795", "renesas,hscif"; reg = <0 0xe66b0000 0 96>; interrupts = <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&cpg CPG_MOD 516>; - clock-names = "fck"; + clocks = <&cpg CPG_MOD 516>, + <&cpg CPG_CORE R8A7795_CLK_S3D1>, + <&scif_clk>; + clock-names = "fck", "int_clk", "scif_clk"; dmas = <&dmac0 0x39>, <&dmac0 0x38>; dma-names = "tx", "rx"; power-domains = <&cpg>; @@ -422,8 +440,10 @@ compatible = "renesas,scif-r8a7795", "renesas,scif"; reg = <0 0xe6e60000 0 64>; interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&cpg CPG_MOD 207>; - clock-names = "fck"; + clocks = <&cpg CPG_MOD 207>, + <&cpg CPG_CORE R8A7795_CLK_S3D1>, + <&scif_clk>; + clock-names = "fck", "int_clk", "scif_clk"; dmas = <&dmac1 0x51>, <&dmac1 0x50>; dma-names = "tx", "rx"; power-domains = <&cpg>; @@ -434,8 +454,10 @@ compatible = "renesas,scif-r8a7795", "renesas,scif"; reg = <0 0xe6e68000 0 64>; interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&cpg CPG_MOD 206>; - clock-names = "fck"; + clocks = <&cpg CPG_MOD 206>, + <&cpg CPG_CORE R8A7795_CLK_S3D1>, + <&scif_clk>; + clock-names = "fck", "int_clk", "scif_clk"; dmas = <&dmac1 0x53>, <&dmac1 0x52>; dma-names = "tx", "rx"; power-domains = <&cpg>; @@ -446,8 +468,10 @@ compatible = "renesas,scif-r8a7795", "renesas,scif"; reg = <0 0xe6e88000 0 64>; interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&cpg CPG_MOD 310>; - clock-names = "fck"; + clocks = <&cpg CPG_MOD 310>, + <&cpg CPG_CORE R8A7795_CLK_S3D1>, + <&scif_clk>; + clock-names = "fck", "int_clk", "scif_clk"; dmas = <&dmac1 0x13>, <&dmac1 0x12>; dma-names = "tx", "rx"; power-domains = <&cpg>; @@ -458,8 +482,10 @@ compatible = "renesas,scif-r8a7795", "renesas,scif"; reg = <0 0xe6c50000 0 64>; interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&cpg CPG_MOD 204>; - clock-names = "fck"; + clocks = <&cpg CPG_MOD 204>, + <&cpg CPG_CORE R8A7795_CLK_S3D1>, + <&scif_clk>; + clock-names = "fck", "int_clk", "scif_clk"; dmas = <&dmac0 0x57>, <&dmac0 0x56>; dma-names = "tx", "rx"; power-domains = <&cpg>; @@ -470,8 +496,10 @@ compatible = "renesas,scif-r8a7795", "renesas,scif"; reg = <0 0xe6c40000 0 64>; interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&cpg CPG_MOD 203>; - clock-names = "fck"; + clocks = <&cpg CPG_MOD 203>, + <&cpg CPG_CORE R8A7795_CLK_S3D1>, + <&scif_clk>; + clock-names = "fck", "int_clk", "scif_clk"; dmas = <&dmac0 0x59>, <&dmac0 0x58>; dma-names = "tx", "rx"; power-domains = <&cpg>; @@ -482,8 +510,10 @@ compatible = "renesas,scif-r8a7795", "renesas,scif"; reg = <0 0xe6f30000 0 64>; interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&cpg CPG_MOD 202>; - clock-names = "fck"; + clocks = <&cpg CPG_MOD 202>, + <&cpg CPG_CORE R8A7795_CLK_S3D1>, + <&scif_clk>; + clock-names = "fck", "int_clk", "scif_clk"; dmas = <&dmac1 0x5b>, <&dmac1 0x5a>; dma-names = "tx", "rx"; power-domains = <&cpg>;
Add the device node for the external SCIF_CLK. The presence of the SCIF_CLK crystal and its clock frequency depend on the actual board. Add the two optional clock sources (ZS_CLK and SCIF_CLK for the internal resp. external clock) for the Baud Rate Generator for External Clock (BRG) to all SCIF and HSCIF device nodes. This increases the range and accuracy of supported baud rates on (H)SCIF. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- arch/arm64/boot/dts/renesas/r8a7795.dtsi | 74 ++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 22 deletions(-)