| Message ID | 20170508201159.31634-1-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2,v2] clk: Add bindings for the Gemini Clock Controller | expand |
On Mon, May 8, 2017 at 3:11 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > This adds device tree bindings and a header for the Gemini SoC > Clock Controller. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v1->v2: > - Move the clock controller to be directly in the parent syscon > node. > --- > .../clock/cortina,gemini-clock-controller.txt | 22 ++++++++++++++++ > include/dt-bindings/clock/cortina,gemini-clock.h | 29 ++++++++++++++++++++++ > 2 files changed, 51 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/cortina,gemini-clock-controller.txt > create mode 100644 include/dt-bindings/clock/cortina,gemini-clock.h > > diff --git a/Documentation/devicetree/bindings/clock/cortina,gemini-clock-controller.txt b/Documentation/devicetree/bindings/clock/cortina,gemini-clock-controller.txt > new file mode 100644 > index 000000000000..ae0046bccba0 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/cortina,gemini-clock-controller.txt > @@ -0,0 +1,22 @@ > +Clock bindings for the Cortina Systems Gemini SoC Clock Controller > + > +Required properties : > +- compatible : shall contain the following: > + "cortina,gemini-clock-controller" > +- #clock-cells should be <1> > + > +The Gemini clock controller needs to be identical to the system controller > +node. > + > +All available clocks are defined as preprocessor macros in > +dt-bindings/clock/cortina,gemini-clock.h header and can be used in device > +tree sources. > + > +Example: > + > +syscon: syscon@40000000 { > + compatible = "cortina,gemini-syscon", "cortina,gemini-clock-controller", > + "syscon", "simple-mfd"; There are no child nodes, so you don't need simple-mfd. > + reg = <0x40000000 0x1000>; Looks like you have 2 nodes pointing to the same address with your reset binding? You shouldn't have overlapping resources. It's allowed for historical reasons but breaks resource creation in Linux. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 8, 2017 at 11:24 PM, Rob Herring <robh+dt@kernel.org> wrote: >> +Example: >> + >> +syscon: syscon@40000000 { >> + compatible = "cortina,gemini-syscon", "cortina,gemini-clock-controller", >> + "syscon", "simple-mfd"; > > There are no child nodes, so you don't need simple-mfd. The example is taken from an actual device tree (look below), where there are child nodes, I can trim it down. >> + reg = <0x40000000 0x1000>; > > Looks like you have 2 nodes pointing to the same address with your > reset binding? You shouldn't have overlapping resources. It's allowed > for historical reasons but breaks resource creation in Linux. No... they are all in the same node, just sharing the same resource by way of regmap (syscon). In the end, as I think you requested, when you said: >> + clock-controller { >> + compatible = "cortina,gemini-clock-controller"; >> + #clock-cells = <1>; (...) > There's not really much reason to have a child node here. The parent can > be the clock provider. (...) > Same comment as clocks. The parent can be the provider. So as you say, no specific child is needed and syscon provides clocks and resets: syscon: syscon@40000000 { compatible = "cortina,gemini-syscon", "cortina,gemini-clock-controller", "cortina,gemini-reset", "syscon", "simple-mfd"; reg = <0x40000000 0x1000>; #clock-cells = <1>; #reset-cells = <1>; syscon-reboot { compatible = "syscon-reboot"; regmap = <&syscon>; /* GLOBAL_RESET register */ offset = <0x0c>; /* RESET_GLOBAL | RESET_CPU1 */ mask = <0xC0000000>; }; }; Before your (as I percieved it) requested changes to avoid surplus children in the syscon it looked like this: syscon: syscon@40000000 { compatible = "cortina,gemini-syscon", "syscon", "simple-mfd"; reg = <0x40000000 0x1000>; syscon-reboot { compatible = "syscon-reboot"; regmap = <&syscon>; /* GLOBAL_RESET register */ offset = <0x0c>; /* RESET_GLOBAL | RESET_CPU1 */ mask = <0xC0000000>; }; rcon: reset-controller { compatible = "cortina,gemini-reset"; #reset-cells = <1>; }; gcc: clock-controller { compatible = "cortina,gemini-clock-controller"; #clock-cells = <1>; }; }; Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 8, 2017 at 4:41 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Mon, May 8, 2017 at 11:24 PM, Rob Herring <robh+dt@kernel.org> wrote: > >>> +Example: >>> + >>> +syscon: syscon@40000000 { >>> + compatible = "cortina,gemini-syscon", "cortina,gemini-clock-controller", >>> + "syscon", "simple-mfd"; >> >> There are no child nodes, so you don't need simple-mfd. > > The example is taken from an actual device tree (look below), > where there are child nodes, I can trim it down. > >>> + reg = <0x40000000 0x1000>; >> >> Looks like you have 2 nodes pointing to the same address with your >> reset binding? You shouldn't have overlapping resources. It's allowed >> for historical reasons but breaks resource creation in Linux. > > No... they are all in the same node, just sharing the same > resource by way of regmap (syscon). Okay, then please document at least the parent syscon node in a single doc. Splitting it is very confusing. > > In the end, as I think you requested, when you said: > >>> + clock-controller { >>> + compatible = "cortina,gemini-clock-controller"; >>> + #clock-cells = <1>; > (...) >> There's not really much reason to have a child node here. The parent can >> be the clock provider. > (...) >> Same comment as clocks. The parent can be the provider. > > So as you say, no specific child is needed and syscon provides > clocks and resets: > > syscon: syscon@40000000 { > compatible = "cortina,gemini-syscon", > "cortina,gemini-clock-controller", > "cortina,gemini-reset", This mostly looks fine, but you shouldn't need 3 compatible strings for the block. > "syscon", "simple-mfd"; > reg = <0x40000000 0x1000>; > #clock-cells = <1>; > #reset-cells = <1>; > > syscon-reboot { > compatible = "syscon-reboot"; > regmap = <&syscon>; > /* GLOBAL_RESET register */ > offset = <0x0c>; > /* RESET_GLOBAL | RESET_CPU1 */ > mask = <0xC0000000>; > }; > }; > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 9, 2017 at 4:23 PM, Rob Herring <robh+dt@kernel.org> wrote: > On Mon, May 8, 2017 at 4:41 PM, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Mon, May 8, 2017 at 11:24 PM, Rob Herring <robh+dt@kernel.org> wrote: >> >>>> +Example: >>>> + >>>> +syscon: syscon@40000000 { >>>> + compatible = "cortina,gemini-syscon", "cortina,gemini-clock-controller", >>>> + "syscon", "simple-mfd"; >>> >>> There are no child nodes, so you don't need simple-mfd. >> >> The example is taken from an actual device tree (look below), >> where there are child nodes, I can trim it down. >> >>>> + reg = <0x40000000 0x1000>; >>> >>> Looks like you have 2 nodes pointing to the same address with your >>> reset binding? You shouldn't have overlapping resources. It's allowed >>> for historical reasons but breaks resource creation in Linux. >> >> No... they are all in the same node, just sharing the same >> resource by way of regmap (syscon). > > Okay, then please document at least the parent syscon node in a single > doc. Splitting it is very confusing. I'm sorry. :( I'll patch the document in arm/gemini.txt where the syscon node is documented, with a single patch adding both clock and reset bindings. >> syscon: syscon@40000000 { >> compatible = "cortina,gemini-syscon", >> "cortina,gemini-clock-controller", >> "cortina,gemini-reset", > > This mostly looks fine, but you shouldn't need 3 compatible strings > for the block. OK I'll see if I can make it work with just "cortina,gemini-syscon" and skip the two others. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" 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/Documentation/devicetree/bindings/clock/cortina,gemini-clock-controller.txt b/Documentation/devicetree/bindings/clock/cortina,gemini-clock-controller.txt new file mode 100644 index 000000000000..ae0046bccba0 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/cortina,gemini-clock-controller.txt @@ -0,0 +1,22 @@ +Clock bindings for the Cortina Systems Gemini SoC Clock Controller + +Required properties : +- compatible : shall contain the following: + "cortina,gemini-clock-controller" +- #clock-cells should be <1> + +The Gemini clock controller needs to be identical to the system controller +node. + +All available clocks are defined as preprocessor macros in +dt-bindings/clock/cortina,gemini-clock.h header and can be used in device +tree sources. + +Example: + +syscon: syscon@40000000 { + compatible = "cortina,gemini-syscon", "cortina,gemini-clock-controller", + "syscon", "simple-mfd"; + reg = <0x40000000 0x1000>; + #clock-cells = <1>; +}; diff --git a/include/dt-bindings/clock/cortina,gemini-clock.h b/include/dt-bindings/clock/cortina,gemini-clock.h new file mode 100644 index 000000000000..acf5cd550b0c --- /dev/null +++ b/include/dt-bindings/clock/cortina,gemini-clock.h @@ -0,0 +1,29 @@ +#ifndef DT_BINDINGS_CORTINA_GEMINI_CLOCK_H +#define DT_BINDINGS_CORTINA_GEMINI_CLOCK_H + +/* RTC, AHB, APB, CPU, PCI, TVC, UART clocks and 13 gates */ +#define GEMINI_NUM_CLKS 20 + +#define GEMINI_CLK_RTC 0 +#define GEMINI_CLK_AHB 1 +#define GEMINI_CLK_APB 2 +#define GEMINI_CLK_CPU 3 +#define GEMINI_CLK_PCI 4 +#define GEMINI_CLK_TVC 5 +#define GEMINI_CLK_UART 6 +#define GEMINI_CLK_GATES 7 +#define GEMINI_CLK_GATE_SECURITY 7 +#define GEMINI_CLK_GATE_GMAC0 8 +#define GEMINI_CLK_GATE_GMAC1 9 +#define GEMINI_CLK_GATE_SATA0 10 +#define GEMINI_CLK_GATE_SATA1 11 +#define GEMINI_CLK_GATE_USB0 12 +#define GEMINI_CLK_GATE_USB1 13 +#define GEMINI_CLK_GATE_IDE 14 +#define GEMINI_CLK_GATE_PCI 15 +#define GEMINI_CLK_GATE_DDR 16 +#define GEMINI_CLK_GATE_FLASH 17 +#define GEMINI_CLK_GATE_TVC 18 +#define GEMINI_CLK_GATE_BOOT 19 + +#endif /* DT_BINDINGS_CORTINA_GEMINI_CLOCK_H */
This adds device tree bindings and a header for the Gemini SoC Clock Controller. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v1->v2: - Move the clock controller to be directly in the parent syscon node. --- .../clock/cortina,gemini-clock-controller.txt | 22 ++++++++++++++++ include/dt-bindings/clock/cortina,gemini-clock.h | 29 ++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/cortina,gemini-clock-controller.txt create mode 100644 include/dt-bindings/clock/cortina,gemini-clock.h -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html