Re: [PATCH v12 17/21] h8300: clock driver
Hello Sato-san, Unfortunately this patch did not Cc myself, Stephen Boyd or the linux-...@vger.kernel.org mailing list. As such Stephen and I did not have a chance to review it. Even more unfortunate was that it was ninja merged by maintainers without our ack. :-/ Quoting Yoshinori Sato (2015-05-10 23:26:36) > Signed-off-by: Yoshinori Sato > --- > .../bindings/clock/renesas,h8300-div-clock.txt | 24 > .../bindings/clock/renesas,h8s2678-pll-clock.txt | 23 > drivers/clk/Makefile | 1 + > drivers/clk/h8300/Makefile | 2 + > drivers/clk/h8300/clk-div.c| 53 > drivers/clk/h8300/clk-h8s2678.c| 147 > + > 6 files changed, 250 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt > create mode 100644 > Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt > create mode 100644 drivers/clk/h8300/Makefile > create mode 100644 drivers/clk/h8300/clk-div.c > create mode 100644 drivers/clk/h8300/clk-h8s2678.c > > diff --git > a/Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt > b/Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt > new file mode 100644 > index 000..36c2b52 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt > @@ -0,0 +1,24 @@ > +* Renesas H8/300 divider clock > + > +Required Properties: > + > + - compatible: Must be "renesas,sh73a0-h8300-div-clock" > + > + - clocks: Reference to the parent clocks ("extal1" and "extal2") > + > + - #clock-cells: Must be 1 > + > + - reg: Base address and length of the divide rate selector > + > + - renesas,width: bit width of selector > + > +Example > +--- > + > + cclk: cclk { > + compatible = "renesas,h8300-div-clock"; > + clocks = <>; > + #clock-cells = <0>; > + reg = <0xfee01b 2>; > + renesas,width = <2>; > + }; I could not find any info on this clock in the H8S/2678 reference manual[0]. Could you point me to the right documentation? > diff --git > a/Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt > b/Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt > new file mode 100644 > index 000..500cdadb > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt > @@ -0,0 +1,23 @@ > +Renesas H8S2678 PLL clock > + > +This device is Clock multiplyer > + > +Required Properties: > + > + - compatible: Must be "renesas,h8s2678-pll-clock" > + > + - clocks: Reference to the parent clocks > + > + - #clock-cells: Must be 0 > + > + - reg: Two rate selector (Multiply / Divide) register address > + > +Example > +--- > + > + pllclk: pllclk { > + compatible = "renesas,h8s2678-pll-clock"; > + clocks = <>; > + #clock-cells = <0>; > + reg = <0xfee03b 2>, <0xfee045 2>; > + }; Is there really only one clock output? According to figure 21.1 there is the "System clock to φ pin" output and the "Internal clock to peripheral modules" output. I am wondering if clock-cells should be 1 instead of zero and support both of these output signals? As a nitpick, I think it would have been better to name the node "cpg" as it is listed in Section 21. pllclk is only one of the two registers that make up the cpg. Something like: cpg: clock-controller@fee03b If you do decide to have clock-cells greater than zero, you might find the following threads helpful. They describe how to craft a clock-controller style binding: http://lkml.kernel.org/r/<20150411001231.18916.93186@quantum> http://lkml.kernel.org/r/<20150724034229.642.88156@quantum> As an additional thought, it looks like the module stop registers are mixed in with the clock registers. When you decide to write a reset driver for these platforms you might want to re-use this existing dt binding description and put the reset code into your clock provider driver. Grep for reset.h in the drivers/clk/ directory for some examples. [0] http://documentation.renesas.com/doc/products/mpumcu/rej09b0283_2678hm.pdf Regards, Mike -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v12 17/21] h8300: clock driver
Hello Sato-san, Unfortunately this patch did not Cc myself, Stephen Boyd or the linux-...@vger.kernel.org mailing list. As such Stephen and I did not have a chance to review it. Even more unfortunate was that it was ninja merged by maintainers without our ack. :-/ Quoting Yoshinori Sato (2015-05-10 23:26:36) Signed-off-by: Yoshinori Sato ys...@users.sourceforge.jp --- .../bindings/clock/renesas,h8300-div-clock.txt | 24 .../bindings/clock/renesas,h8s2678-pll-clock.txt | 23 drivers/clk/Makefile | 1 + drivers/clk/h8300/Makefile | 2 + drivers/clk/h8300/clk-div.c| 53 drivers/clk/h8300/clk-h8s2678.c| 147 + 6 files changed, 250 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt create mode 100644 Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt create mode 100644 drivers/clk/h8300/Makefile create mode 100644 drivers/clk/h8300/clk-div.c create mode 100644 drivers/clk/h8300/clk-h8s2678.c diff --git a/Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt b/Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt new file mode 100644 index 000..36c2b52 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt @@ -0,0 +1,24 @@ +* Renesas H8/300 divider clock + +Required Properties: + + - compatible: Must be renesas,sh73a0-h8300-div-clock + + - clocks: Reference to the parent clocks (extal1 and extal2) + + - #clock-cells: Must be 1 + + - reg: Base address and length of the divide rate selector + + - renesas,width: bit width of selector + +Example +--- + + cclk: cclk { + compatible = renesas,h8300-div-clock; + clocks = xclk; + #clock-cells = 0; + reg = 0xfee01b 2; + renesas,width = 2; + }; I could not find any info on this clock in the H8S/2678 reference manual[0]. Could you point me to the right documentation? diff --git a/Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt b/Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt new file mode 100644 index 000..500cdadb --- /dev/null +++ b/Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt @@ -0,0 +1,23 @@ +Renesas H8S2678 PLL clock + +This device is Clock multiplyer + +Required Properties: + + - compatible: Must be renesas,h8s2678-pll-clock + + - clocks: Reference to the parent clocks + + - #clock-cells: Must be 0 + + - reg: Two rate selector (Multiply / Divide) register address + +Example +--- + + pllclk: pllclk { + compatible = renesas,h8s2678-pll-clock; + clocks = xclk; + #clock-cells = 0; + reg = 0xfee03b 2, 0xfee045 2; + }; Is there really only one clock output? According to figure 21.1 there is the System clock to φ pin output and the Internal clock to peripheral modules output. I am wondering if clock-cells should be 1 instead of zero and support both of these output signals? As a nitpick, I think it would have been better to name the node cpg as it is listed in Section 21. pllclk is only one of the two registers that make up the cpg. Something like: cpg: clock-controller@fee03b If you do decide to have clock-cells greater than zero, you might find the following threads helpful. They describe how to craft a clock-controller style binding: http://lkml.kernel.org/r/20150411001231.18916.93186@quantum http://lkml.kernel.org/r/20150724034229.642.88156@quantum As an additional thought, it looks like the module stop registers are mixed in with the clock registers. When you decide to write a reset driver for these platforms you might want to re-use this existing dt binding description and put the reset code into your clock provider driver. Grep for reset.h in the drivers/clk/ directory for some examples. [0] http://documentation.renesas.com/doc/products/mpumcu/rej09b0283_2678hm.pdf Regards, Mike -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v12 17/21] h8300: clock driver
Signed-off-by: Yoshinori Sato --- .../bindings/clock/renesas,h8300-div-clock.txt | 24 .../bindings/clock/renesas,h8s2678-pll-clock.txt | 23 drivers/clk/Makefile | 1 + drivers/clk/h8300/Makefile | 2 + drivers/clk/h8300/clk-div.c| 53 drivers/clk/h8300/clk-h8s2678.c| 147 + 6 files changed, 250 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt create mode 100644 Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt create mode 100644 drivers/clk/h8300/Makefile create mode 100644 drivers/clk/h8300/clk-div.c create mode 100644 drivers/clk/h8300/clk-h8s2678.c diff --git a/Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt b/Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt new file mode 100644 index 000..36c2b52 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt @@ -0,0 +1,24 @@ +* Renesas H8/300 divider clock + +Required Properties: + + - compatible: Must be "renesas,sh73a0-h8300-div-clock" + + - clocks: Reference to the parent clocks ("extal1" and "extal2") + + - #clock-cells: Must be 1 + + - reg: Base address and length of the divide rate selector + + - renesas,width: bit width of selector + +Example +--- + + cclk: cclk { + compatible = "renesas,h8300-div-clock"; + clocks = <>; + #clock-cells = <0>; + reg = <0xfee01b 2>; + renesas,width = <2>; + }; diff --git a/Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt b/Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt new file mode 100644 index 000..500cdadb --- /dev/null +++ b/Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt @@ -0,0 +1,23 @@ +Renesas H8S2678 PLL clock + +This device is Clock multiplyer + +Required Properties: + + - compatible: Must be "renesas,h8s2678-pll-clock" + + - clocks: Reference to the parent clocks + + - #clock-cells: Must be 0 + + - reg: Two rate selector (Multiply / Divide) register address + +Example +--- + + pllclk: pllclk { + compatible = "renesas,h8s2678-pll-clock"; + clocks = <>; + #clock-cells = <0>; + reg = <0xfee03b 2>, <0xfee045 2>; + }; diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 3d00c25..9df871d 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -73,3 +73,4 @@ obj-$(CONFIG_ARCH_U8500) += ux500/ obj-$(CONFIG_COMMON_CLK_VERSATILE) += versatile/ obj-$(CONFIG_X86) += x86/ obj-$(CONFIG_ARCH_ZYNQ)+= zynq/ +obj-$(CONFIG_H8300)+= h8300/ diff --git a/drivers/clk/h8300/Makefile b/drivers/clk/h8300/Makefile new file mode 100644 index 000..b86427c3 --- /dev/null +++ b/drivers/clk/h8300/Makefile @@ -0,0 +1,2 @@ +obj-y += clk-div.o +obj-$(CONFIG_H8S2678) += clk-h8s2678.o diff --git a/drivers/clk/h8300/clk-div.c b/drivers/clk/h8300/clk-div.c new file mode 100644 index 000..56f9eba --- /dev/null +++ b/drivers/clk/h8300/clk-div.c @@ -0,0 +1,53 @@ +/* + * H8/300 divide clock driver + * + * Copyright 2015 Yoshinori Sato + */ + +#include +#include +#include +#include +#include +#include + +static DEFINE_SPINLOCK(clklock); + +static void __init h8300_div_clk_setup(struct device_node *node) +{ + unsigned int num_parents; + struct clk *clk; + const char *clk_name = node->name; + const char *parent_name; + void __iomem *divcr = NULL; + int width; + + num_parents = of_clk_get_parent_count(node); + if (num_parents < 1) { + pr_err("%s: no parent found", clk_name); + return; + } + + divcr = of_iomap(node, 0); + if (divcr == NULL) { + pr_err("%s: failed to map divide register", clk_name); + goto error; + } + + parent_name = of_clk_get_parent_name(node, 0); + of_property_read_u32(node, "renesas,width", ); + clk = clk_register_divider(NULL, clk_name, parent_name, + CLK_SET_RATE_GATE, divcr, 0, width, + CLK_DIVIDER_POWER_OF_TWO, ); + if (!IS_ERR(clk)) { + of_clk_add_provider(node, of_clk_src_simple_get, clk); + return; + } + pr_err("%s: failed to register %s div clock (%ld)\n", + __func__, clk_name, PTR_ERR(clk)); +error: + if (divcr) + iounmap(divcr); +} + +CLK_OF_DECLARE(h8300_div_clk, "renesas,h8300-div-clock", h8300_div_clk_setup); diff --git a/drivers/clk/h8300/clk-h8s2678.c b/drivers/clk/h8300/clk-h8s2678.c
[PATCH v12 17/21] h8300: clock driver
Signed-off-by: Yoshinori Sato ys...@users.sourceforge.jp --- .../bindings/clock/renesas,h8300-div-clock.txt | 24 .../bindings/clock/renesas,h8s2678-pll-clock.txt | 23 drivers/clk/Makefile | 1 + drivers/clk/h8300/Makefile | 2 + drivers/clk/h8300/clk-div.c| 53 drivers/clk/h8300/clk-h8s2678.c| 147 + 6 files changed, 250 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt create mode 100644 Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt create mode 100644 drivers/clk/h8300/Makefile create mode 100644 drivers/clk/h8300/clk-div.c create mode 100644 drivers/clk/h8300/clk-h8s2678.c diff --git a/Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt b/Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt new file mode 100644 index 000..36c2b52 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/renesas,h8300-div-clock.txt @@ -0,0 +1,24 @@ +* Renesas H8/300 divider clock + +Required Properties: + + - compatible: Must be renesas,sh73a0-h8300-div-clock + + - clocks: Reference to the parent clocks (extal1 and extal2) + + - #clock-cells: Must be 1 + + - reg: Base address and length of the divide rate selector + + - renesas,width: bit width of selector + +Example +--- + + cclk: cclk { + compatible = renesas,h8300-div-clock; + clocks = xclk; + #clock-cells = 0; + reg = 0xfee01b 2; + renesas,width = 2; + }; diff --git a/Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt b/Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt new file mode 100644 index 000..500cdadb --- /dev/null +++ b/Documentation/devicetree/bindings/clock/renesas,h8s2678-pll-clock.txt @@ -0,0 +1,23 @@ +Renesas H8S2678 PLL clock + +This device is Clock multiplyer + +Required Properties: + + - compatible: Must be renesas,h8s2678-pll-clock + + - clocks: Reference to the parent clocks + + - #clock-cells: Must be 0 + + - reg: Two rate selector (Multiply / Divide) register address + +Example +--- + + pllclk: pllclk { + compatible = renesas,h8s2678-pll-clock; + clocks = xclk; + #clock-cells = 0; + reg = 0xfee03b 2, 0xfee045 2; + }; diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 3d00c25..9df871d 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -73,3 +73,4 @@ obj-$(CONFIG_ARCH_U8500) += ux500/ obj-$(CONFIG_COMMON_CLK_VERSATILE) += versatile/ obj-$(CONFIG_X86) += x86/ obj-$(CONFIG_ARCH_ZYNQ)+= zynq/ +obj-$(CONFIG_H8300)+= h8300/ diff --git a/drivers/clk/h8300/Makefile b/drivers/clk/h8300/Makefile new file mode 100644 index 000..b86427c3 --- /dev/null +++ b/drivers/clk/h8300/Makefile @@ -0,0 +1,2 @@ +obj-y += clk-div.o +obj-$(CONFIG_H8S2678) += clk-h8s2678.o diff --git a/drivers/clk/h8300/clk-div.c b/drivers/clk/h8300/clk-div.c new file mode 100644 index 000..56f9eba --- /dev/null +++ b/drivers/clk/h8300/clk-div.c @@ -0,0 +1,53 @@ +/* + * H8/300 divide clock driver + * + * Copyright 2015 Yoshinori Sato ys...@users.sourceforge.jp + */ + +#include linux/clk.h +#include linux/clkdev.h +#include linux/clk-provider.h +#include linux/err.h +#include linux/of.h +#include linux/of_address.h + +static DEFINE_SPINLOCK(clklock); + +static void __init h8300_div_clk_setup(struct device_node *node) +{ + unsigned int num_parents; + struct clk *clk; + const char *clk_name = node-name; + const char *parent_name; + void __iomem *divcr = NULL; + int width; + + num_parents = of_clk_get_parent_count(node); + if (num_parents 1) { + pr_err(%s: no parent found, clk_name); + return; + } + + divcr = of_iomap(node, 0); + if (divcr == NULL) { + pr_err(%s: failed to map divide register, clk_name); + goto error; + } + + parent_name = of_clk_get_parent_name(node, 0); + of_property_read_u32(node, renesas,width, width); + clk = clk_register_divider(NULL, clk_name, parent_name, + CLK_SET_RATE_GATE, divcr, 0, width, + CLK_DIVIDER_POWER_OF_TWO, clklock); + if (!IS_ERR(clk)) { + of_clk_add_provider(node, of_clk_src_simple_get, clk); + return; + } + pr_err(%s: failed to register %s div clock (%ld)\n, + __func__, clk_name, PTR_ERR(clk)); +error: + if (divcr) + iounmap(divcr); +} + +CLK_OF_DECLARE(h8300_div_clk,