| Message ID | 1459070777-18049-5-git-send-email-thomas.petazzoni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Mar 27, 2016 at 11:26:16AM +0200, Thomas Petazzoni wrote: > This commit adds the DT binding documentation for the Marvell CP110 > system controller, which is part of the CP110 HW block, itself used in > the Marvell Armada 7K and 8K SoCs. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > .../arm/marvell/cp110-system-controller0.txt | 88 ++++++++++++++++++++++ > 1 file changed, 88 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt > > diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt > new file mode 100644 > index 0000000..e8883da > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt > @@ -0,0 +1,88 @@ > +Marvell Armada CP110 System Controller 0 > +======================================== > + > +The CP110 is one of the two core HW blocks of the Marvell Armada 7K/8K > +SoCs. It contains two sets of system control registers, System > +Controller 0 and System Controller 1. This Device Tree binding allows > +to describe the first system controller, which provides registers to > +configure various aspects of the SoC. > + > +The Device Tree node representing this System Controller 0 provides a > +number of clocks: > + > + - a set of core clocks > + - a set of gatable clocks > + > +Those clocks can be referenced by other Device Tree nodes using two > +cells: > + - The first cell must be 0 or 1. 0 for the core clocks and 1 for the > + gatable clocks. > + - The second cell identifies the particular core clock or gatable > + clocks. > + > +The following clocks are available: > + - Core clocks > + - 0 0 APLL > + - 0 1 PPv2 core > + - 0 2 EIP > + - 0 3 Core > + - 0 4 NAND core > + - Gatable clocks > + - 1 0 Audio > + - 1 1 Comm Unit > + - 1 2 NAND > + - 1 3 PPv2 > + - 1 4 SDIO > + - 1 5 MG Domain > + - 1 6 MG Core > + - 1 7 XOR1 > + - 1 8 XOR0 > + - 1 9 GOP DP > + - 1 11 PCIe x1 0 > + - 1 12 PCIe x1 1 > + - 1 13 PCIe x4 > + - 1 14 PCIe / XOR > + - 1 15 SATA > + - 1 16 SATA USB > + - 1 18 GOP Macro > + - 1 22 USB3H0 > + - 1 23 USB3H1 > + - 1 24 USB3 Device > + - 1 25 EIP150 > + - 1 26 EIP197 > + > +Required properties: > + > + - compatible: must be: > + "marvell,cp110-system-controller0", "syscon"; This block is really the same across SoCs? > + - reg: register area of the CP110 system controller 0 > + - #clock-cells: must be set to 2 > + - core-clock-output-names must be set to: > + "cpm-apll", "cpm-ppv2-core", "cpm-eip", "cpm-core", "cpm-nand-core" > + - gatable-clock-indices must be set to: > + <0>, <1>, <2>, <3>, <4>, <5>, <6>, <7>, <8>, > + <9>, <11>, <12>, <13>, <14>, <15>, <16>, <18>, > + <22>, <23>, <24>, <25>, <26> You aren't skipping very many spots. I'd just fill the unused names in with "none" or something. > + - gatable-clock-output-names must be set to: > + "cpm-audio", "cpm-communit", "cpm-nand", "cpm-ppv2", "cpm-sdio", > + "cpm-mg-domain", "cpm-mg-core", "cpm-xor1", "cpm-xor0", "cpm-gop-dp", "cpm-pcie_x10", > + "cpm-pcie_x11", "cpm-pcie_x4", "cpm-pcie-xor", "cpm-sata", "cpm-sata-usb", > + "cpm-gop-macro", "cpm-usb3h0", "cpm-usb3h1", "cpm-usb3dev", "cpm-eip150", > + "cpm-eip197" > + > +Example: > + > + cpm_syscon0: system-controller@440000 { > + compatible = "marvell,cp110-system-controller0", "syscon"; > + reg = <0x440000 0x1000>; > + #clock-cells = <2>; > + core-clock-output-names = "cpm-apll", "cpm-ppv2-core", "cpm-eip", "cpm-core", "cpm-nand-core"; > + gate-clock-indices = <0>, <1>, <2>, <3>, <4>, <5>, <6>, <7>, <8>, > + <9>, <11>, <12>, <13>, <14>, <15>, <16>, <18>, > + <22>, <23>, <24>, <25>, <26>; > + gate-clock-output-names = "cpm-audio", "cpm-communit", "cpm-nand", "cpm-ppv2", "cpm-sdio", > + "cpm-mg-domain", "cpm-mg-core", "cpm-xor1", "cpm-xor0", "cpm-gop-dp", "cpm-pcie_x10", > + "cpm-pcie_x11", "cpm-pcie_x4", "cpm-pcie-xor", "cpm-sata", "cpm-sata-usb", > + "cpm-gop-macro", "cpm-usb3h0", "cpm-usb3h1", "cpm-usb3dev", "cpm-eip150", > + "cpm-eip197"; > + }; > -- > 2.6.4 >
Hello, On Mon, 28 Mar 2016 14:47:56 -0500, Rob Herring wrote: > > +Required properties: > > + > > + - compatible: must be: > > + "marvell,cp110-system-controller0", "syscon"; > > This block is really the same across SoCs? As per my knowledge, it is the same across 7020, 7040, 8020 and 8040, where the CP part is named CP110. My understanding is that in future SoCs, when the CP part will change, the CP part will have a different name, i.e CP115 or 120 or something (these are invented names, I have no idea how Marvell will name the future CPs). So I believe cp110-system-controller0 properly uniquely identifies this IP block. > > + - reg: register area of the CP110 system controller 0 > > + - #clock-cells: must be set to 2 > > + - core-clock-output-names must be set to: > > + "cpm-apll", "cpm-ppv2-core", "cpm-eip", "cpm-core", "cpm-nand-core" > > + - gatable-clock-indices must be set to: > > + <0>, <1>, <2>, <3>, <4>, <5>, <6>, <7>, <8>, > > + <9>, <11>, <12>, <13>, <14>, <15>, <16>, <18>, > > + <22>, <23>, <24>, <25>, <26> > > You aren't skipping very many spots. I'd just fill the unused names in > with "none" or something. and then remove the gatable-clock-indices property altogether? Thanks for the review! Thomas
Hello, On Mon, 28 Mar 2016 14:47:56 -0500, Rob Herring wrote: > > + - reg: register area of the CP110 system controller 0 > > + - #clock-cells: must be set to 2 > > + - core-clock-output-names must be set to: > > + "cpm-apll", "cpm-ppv2-core", "cpm-eip", "cpm-core", "cpm-nand-core" > > + - gatable-clock-indices must be set to: > > + <0>, <1>, <2>, <3>, <4>, <5>, <6>, <7>, <8>, > > + <9>, <11>, <12>, <13>, <14>, <15>, <16>, <18>, > > + <22>, <23>, <24>, <25>, <26> > > You aren't skipping very many spots. I'd just fill the unused names in > with "none" or something. > > > + - gatable-clock-output-names must be set to: > > + "cpm-audio", "cpm-communit", "cpm-nand", "cpm-ppv2", "cpm-sdio", > > + "cpm-mg-domain", "cpm-mg-core", "cpm-xor1", "cpm-xor0", "cpm-gop-dp", "cpm-pcie_x10", > > + "cpm-pcie_x11", "cpm-pcie_x4", "cpm-pcie-xor", "cpm-sata", "cpm-sata-usb", > > + "cpm-gop-macro", "cpm-usb3h0", "cpm-usb3h1", "cpm-usb3dev", "cpm-eip150", > > + "cpm-eip197" In fact, the more I think of it, the less I find encoding the clock output names in the DT to be useful for such a driver. For generic clock drivers, it makes complete sense, but here the driver is really tied to the specific system controller of that SoC, so the clock names will not change. In addition, those gatable clocks are not independent from each other: many of them are parent from others (this wasn't encoded into the driver until now, because I didn't had this info until now). So the driver will anyway have to have a lot of knowledge about which clock is child/parent of which one. Which means the driver is anyway really tied to that specific system controller. Does it still make sense to encode the clock names in the DT in such a case, or should the driver be providing the clock names? (I don't have a strong opinion either way, I'm just asking what is the preference of the DT and clock maintainers on the matter.). Thanks! Thomas
Hello, On Wed, 13 Apr 2016 18:01:22 +0200, Thomas Petazzoni wrote: > In fact, the more I think of it, the less I find encoding the clock > output names in the DT to be useful for such a driver. For generic > clock drivers, it makes complete sense, but here the driver is really > tied to the specific system controller of that SoC, so the clock names > will not change. My bad: there will be two instances of the CP110 system controller, one for the master CP110 and one for the slave CP110. So having the possibility of setting the clock names in the DT is actually a good thing, so I'll keep it. Thanks! Thomas
Bonsoir Thomas, On Wed, Apr 13, 2016 at 6:01 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > On Mon, 28 Mar 2016 14:47:56 -0500, Rob Herring wrote: > >> > + - reg: register area of the CP110 system controller 0 >> > + - #clock-cells: must be set to 2 >> > + - core-clock-output-names must be set to: >> > + "cpm-apll", "cpm-ppv2-core", "cpm-eip", "cpm-core", "cpm-nand-core" >> > + - gatable-clock-indices must be set to: >> > + <0>, <1>, <2>, <3>, <4>, <5>, <6>, <7>, <8>, >> > + <9>, <11>, <12>, <13>, <14>, <15>, <16>, <18>, >> > + <22>, <23>, <24>, <25>, <26> >> >> You aren't skipping very many spots. I'd just fill the unused names in >> with "none" or something. >> >> > + - gatable-clock-output-names must be set to: >> > + "cpm-audio", "cpm-communit", "cpm-nand", "cpm-ppv2", "cpm-sdio", >> > + "cpm-mg-domain", "cpm-mg-core", "cpm-xor1", "cpm-xor0", "cpm-gop-dp", "cpm-pcie_x10", >> > + "cpm-pcie_x11", "cpm-pcie_x4", "cpm-pcie-xor", "cpm-sata", "cpm-sata-usb", >> > + "cpm-gop-macro", "cpm-usb3h0", "cpm-usb3h1", "cpm-usb3dev", "cpm-eip150", >> > + "cpm-eip197" > > In fact, the more I think of it, the less I find encoding the clock > output names in the DT to be useful for such a driver. For generic > clock drivers, it makes complete sense, but here the driver is really > tied to the specific system controller of that SoC, so the clock names > will not change. > > In addition, those gatable clocks are not independent from each other: > many of them are parent from others (this wasn't encoded into the > driver until now, because I didn't had this info until now). So the > driver will anyway have to have a lot of knowledge about which clock is > child/parent of which one. Which means the driver is anyway really > tied to that specific system controller. > > Does it still make sense to encode the clock names in the DT in such a > case, or should the driver be providing the clock names? > > (I don't have a strong opinion either way, I'm just asking what is the > preference of the DT and clock maintainers on the matter.). From my experience with clock drivers for Renesas SoCs, life is much easier when handling all of this (clock names, parents, ...) in C. The same is true BTW for PM Domains. The clock controllers of r8a7791 and r8a7795 are very similar, and could be handled similarly. Compare the DT hierarchy for CPG and MSTP/MSSR in arch/arm/boot/dts/r8a7791.dtsi and arch/arm64/boot/dts/renesas/r8a7795.dtsi. Binding definitions are include/dt-bindings/clock/r8a7791-clock.h vs include/dt-bindings/clock/r8a7795-cpg-mssr.h Corresponding drivers are drivers/clk/renesas/clk-rcar-gen2.c drivers/clk/renesas/clk-mstp.c vs. drivers/clk/renesas/r8a7795-cpg-mssr.c drivers/clk/renesas/renesas-cpg-mssr.c drivers/clk/renesas/renesas-cpg-mssr.h 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
Hello, On Thu, 14 Apr 2016 21:19:54 +0200, Geert Uytterhoeven wrote: > From my experience with clock drivers for Renesas SoCs, life is much easier > when handling all of this (clock names, parents, ...) in C. The same is true > BTW for PM Domains. > > The clock controllers of r8a7791 and r8a7795 are very similar, and could > be handled similarly. > Compare the DT hierarchy for CPG and MSTP/MSSR in > arch/arm/boot/dts/r8a7791.dtsi and arch/arm64/boot/dts/renesas/r8a7795.dtsi. Thanks for your feedback. However in practice, there will ultimately be two instances of this clock controller, as some variants of the SoC will contain two CP parts. Due to this, it is going to be quite convenient to have the clock names configurable from the DT, as I will be able to give the same clock on the master CP and the slave CP a different name. That's actually why my binding documentation uses names like "cpm-<foo>" for all the clocks, where "cpm" stands for "CP Master". In the future, another instance will have clocks named "cps-<foo>" for "CP Slave". Best regards, Thomas
Hi Thomas, On Sat, Apr 23, 2016 at 4:22 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > On Thu, 14 Apr 2016 21:19:54 +0200, Geert Uytterhoeven wrote: >> From my experience with clock drivers for Renesas SoCs, life is much easier >> when handling all of this (clock names, parents, ...) in C. The same is true >> BTW for PM Domains. >> >> The clock controllers of r8a7791 and r8a7795 are very similar, and could >> be handled similarly. >> Compare the DT hierarchy for CPG and MSTP/MSSR in >> arch/arm/boot/dts/r8a7791.dtsi and arch/arm64/boot/dts/renesas/r8a7795.dtsi. > > Thanks for your feedback. However in practice, there will ultimately be > two instances of this clock controller, as some variants of the SoC > will contain two CP parts. Due to this, it is going to be quite > convenient to have the clock names configurable from the DT, as I will > be able to give the same clock on the master CP and the slave CP a > different name. You could still prefix the names by e.g. an instance number. > That's actually why my binding documentation uses names like > "cpm-<foo>" for all the clocks, where "cpm" stands for "CP Master". In > the future, another instance will have clocks named "cps-<foo>" for "CP > Slave". So the names are actually part of the binding, while nothing really relies on the actual names, right? Clocks are referred to by clock specifier (phandle + some optonal bits). 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/arm/marvell/cp110-system-controller0.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt new file mode 100644 index 0000000..e8883da --- /dev/null +++ b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt @@ -0,0 +1,88 @@ +Marvell Armada CP110 System Controller 0 +======================================== + +The CP110 is one of the two core HW blocks of the Marvell Armada 7K/8K +SoCs. It contains two sets of system control registers, System +Controller 0 and System Controller 1. This Device Tree binding allows +to describe the first system controller, which provides registers to +configure various aspects of the SoC. + +The Device Tree node representing this System Controller 0 provides a +number of clocks: + + - a set of core clocks + - a set of gatable clocks + +Those clocks can be referenced by other Device Tree nodes using two +cells: + - The first cell must be 0 or 1. 0 for the core clocks and 1 for the + gatable clocks. + - The second cell identifies the particular core clock or gatable + clocks. + +The following clocks are available: + - Core clocks + - 0 0 APLL + - 0 1 PPv2 core + - 0 2 EIP + - 0 3 Core + - 0 4 NAND core + - Gatable clocks + - 1 0 Audio + - 1 1 Comm Unit + - 1 2 NAND + - 1 3 PPv2 + - 1 4 SDIO + - 1 5 MG Domain + - 1 6 MG Core + - 1 7 XOR1 + - 1 8 XOR0 + - 1 9 GOP DP + - 1 11 PCIe x1 0 + - 1 12 PCIe x1 1 + - 1 13 PCIe x4 + - 1 14 PCIe / XOR + - 1 15 SATA + - 1 16 SATA USB + - 1 18 GOP Macro + - 1 22 USB3H0 + - 1 23 USB3H1 + - 1 24 USB3 Device + - 1 25 EIP150 + - 1 26 EIP197 + +Required properties: + + - compatible: must be: + "marvell,cp110-system-controller0", "syscon"; + - reg: register area of the CP110 system controller 0 + - #clock-cells: must be set to 2 + - core-clock-output-names must be set to: + "cpm-apll", "cpm-ppv2-core", "cpm-eip", "cpm-core", "cpm-nand-core" + - gatable-clock-indices must be set to: + <0>, <1>, <2>, <3>, <4>, <5>, <6>, <7>, <8>, + <9>, <11>, <12>, <13>, <14>, <15>, <16>, <18>, + <22>, <23>, <24>, <25>, <26> + - gatable-clock-output-names must be set to: + "cpm-audio", "cpm-communit", "cpm-nand", "cpm-ppv2", "cpm-sdio", + "cpm-mg-domain", "cpm-mg-core", "cpm-xor1", "cpm-xor0", "cpm-gop-dp", "cpm-pcie_x10", + "cpm-pcie_x11", "cpm-pcie_x4", "cpm-pcie-xor", "cpm-sata", "cpm-sata-usb", + "cpm-gop-macro", "cpm-usb3h0", "cpm-usb3h1", "cpm-usb3dev", "cpm-eip150", + "cpm-eip197" + +Example: + + cpm_syscon0: system-controller@440000 { + compatible = "marvell,cp110-system-controller0", "syscon"; + reg = <0x440000 0x1000>; + #clock-cells = <2>; + core-clock-output-names = "cpm-apll", "cpm-ppv2-core", "cpm-eip", "cpm-core", "cpm-nand-core"; + gate-clock-indices = <0>, <1>, <2>, <3>, <4>, <5>, <6>, <7>, <8>, + <9>, <11>, <12>, <13>, <14>, <15>, <16>, <18>, + <22>, <23>, <24>, <25>, <26>; + gate-clock-output-names = "cpm-audio", "cpm-communit", "cpm-nand", "cpm-ppv2", "cpm-sdio", + "cpm-mg-domain", "cpm-mg-core", "cpm-xor1", "cpm-xor0", "cpm-gop-dp", "cpm-pcie_x10", + "cpm-pcie_x11", "cpm-pcie_x4", "cpm-pcie-xor", "cpm-sata", "cpm-sata-usb", + "cpm-gop-macro", "cpm-usb3h0", "cpm-usb3h1", "cpm-usb3dev", "cpm-eip150", + "cpm-eip197"; + };
This commit adds the DT binding documentation for the Marvell CP110 system controller, which is part of the CP110 HW block, itself used in the Marvell Armada 7K and 8K SoCs. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- .../arm/marvell/cp110-system-controller0.txt | 88 ++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt