It seems okay to me on first reading.
On 6/13/2013 7:53 AM, Daniel Drake wrote:
> Hi Mitch,
>
> As mentioned, upstream now has APBC/APMU clock drivers, and I think there
> are a couple of things in way that clock-dt.c interacts with the common
> clock framework that upstream wouldn't approve of.
>
> Here is my first attempt at a new approach, using the new upstream clock
> drivers, and using the common clock framework in a way that is more
> according to its design and use on other SoCs. Tested on XO-1.75 and XO-4.
>
> If you have some time for some quick comments on my DT design before I
> submit this, it would be much appreciated.
>
>
> DT tweaks and kernel patch follow.
>
> : +string encode-string encode+ ;
> : +int encode-int encode+ ;
> : +phandle encode-phandle encode+ ;
>
> \ APBC CLOCKS
>
> dev /
> new-device
> \ a.k.a. fast clock
> " vctcxo" device-name
> " fixed-clock" +compatible
> 0 " #clock-cells" integer-property
> d# 26000000 " clock-frequency" integer-property
> finish-device
>
> new-device
> " clk32" device-name
> " fixed-clock" +compatible
> 0 " #clock-cells" integer-property
> d# 3200 " clock-frequency" integer-property
> finish-device
>
> new-device
> " apb-clock" device-name
>
> h# d4015000 h# 1000 reg
>
> new-device
> " twsi-clocks" device-name
> " marvell,mmp-apb-clock" +compatible
> 1 " #clock-cells" integer-property
> " /vctcxo" encode-phandle " clocks" property
> 0 0 encode-bytes
> " TWSI1" +string \ 0
> " TWSI2" +string \ 1
> " TWSI3" +string \ 2
> " TWSI4" +string \ 3
> " TWSI5" +string \ 4
> " TWSI6" +string \ 5
> " clock-output-names" property
>
> 0 0 encode-bytes
> h# 04 +int h# 08 +int h# 0c +int h# 10 +int h# 7c +int h# 80 +int
> " reg" property
> finish-device
>
> new-device
> " uart-clocks" device-name
> " marvell,mmp-apb-clock" +compatible
> 1 " #clock-cells" integer-property
> " /vctcxo" encode-phandle " clocks" property
>
> 0 0 encode-bytes
> " UART1" +string \ 0
> " UART2" +string \ 1
> " UART3" +string \ 2
> " UART4" +string \ 3
> " clock-output-names" property
>
> 0 0 encode-bytes h# 2c +int h# 30 +int h# 34 +int h# 88 +int
> " reg" property
> finish-device
>
> new-device
> " ssp-clocks" device-name
> " marvell,mmp-apb-clock" +compatible
> 1 " #clock-cells" integer-property
> " /vctcxo" encode-phandle " clocks" property
>
> 0 0 encode-bytes
> " SSP0" +string \ 0
> " SSP1" +string \ 1
> " SSP2" +string \ 2
> " SSP3" +string \ 3
> " SSP4" +string \ 4
> " SSP5" +string \ 5
> " clock-output-names" property
>
> 0 0 encode-bytes
> h# 4c +int h# 50 +int h# 54 +int h# 58 +int h# 5c +int h# 60 +int
> " reg" property
> finish-device
>
> new-device
> " misc-vctcxo-clocks" device-name
> " marvell,mmp-apb-clock" +compatible
> 1 " #clock-cells" integer-property
> " /vctcxo" encode-phandle " clocks" property
> " GPIO" " clock-output-names" string-property
> 38 " reg" integer-property
> finish-device
>
> new-device
> " thsens-clocks" device-name
> " marvell,mmp-apb-clock" +compatible
> 1 " #clock-cells" integer-property
> " /vctcxo" encode-phandle " clocks" property
>
> 0 0 encode-bytes
> " THSENS1" +string \ 0
> " THSENS2" +string \ 1
> " THSENS3" +string \ 2
> " THSENS4" +string \ 3
> " clock-output-names" property
>
> 0 0 encode-bytes h# 90 +int h# 98 +int h# 9c +int h# a0 +int
> " reg" property
> finish-device
>
>
> new-device
> " rtc-clock" device-name
> " marvell,mmp-apb-clock" +compatible
> 1 " #clock-cells" integer-property
> " /clk32" encode-phandle " clocks" property
> " RTC" " clock-output-names" string-property
> 0 " reg" integer-property
> finish-device
>
> finish-device
> device-end
>
> dev /i2c@d4011000 \ point to TWSI1
> " /apb-clock/twsi-clocks" encode-phandle d# 0 +int " clocks" property
> device-end
>
> dev /i2c@d4031000 \ point to TWSI2
> " /apb-clock/twsi-clocks" encode-phandle d# 1 +int " clocks" property
> device-end
>
> dev /i2c@d4033000 \ point to TWSI4
> " /apb-clock/twsi-clocks" encode-phandle d# 3 +int " clocks" property
> device-end
>
> dev /i2c@d4034000 \ point to TWSI6
> " /apb-clock/twsi-clocks" encode-phandle d# 5 +int " clocks" property
> device-end
>
> dev /uart@d4018000 \ point to UART3
> " /apb-clock/uart-clocks" encode-phandle d# 2 +int " clocks" property
> device-end
>
> dev /uart@d4017000 \ point to UART2
> " /apb-clock/uart-clocks" encode-phandle d# 1 +int " clocks" property
> device-end
>
> dev /uart@d4030000 \ point to UART1
> " /apb-clock/uart-clocks" encode-phandle d# 0 +int " clocks" property
> device-end
>
> dev /uart@d4016000 \ point to UART4
> " /apb-clock/uart-clocks" encode-phandle d# 3 +int " clocks" property
> device-end
>
> dev /gpio \ point to GPIO
> " /apb-clock/misc-vctcxo-clocks" encode-phandle d# 0 +int " clocks"
> property
> device-end
>
> dev /flash \ point to SSP1
> " /apb-clock/ssp-clocks" encode-phandle d# 1 +int " clocks" property
> device-end
>
> dev /ec-spi \ point to SSP3
> " /apb-clock/ssp-clocks" encode-phandle d# 3 +int " clocks" property
> device-end
>
> dev /wakeup-rtc \ point to RTC
> " /apb-clock/rtc-clock" encode-phandle d# 0 +int " clocks" property
> device-end
>
> [ifdef] mmp3
> dev /thermal
> 0 0 encode-bytes
> " /apb-clock/thsens-clocks" +phandle 0 +int
> " /apb-clock/thsens-clocks" +phandle 1 +int
> " /apb-clock/thsens-clocks" +phandle 2 +int
> " /apb-clock/thsens-clocks" +phandle 3 +int
> " clocks" property
> device-end
> [then]
>
> \ APMU clocks
> dev /
> new-device
> " pll1" device-name
> " fixed-clock" +compatible
> 0 " #clock-cells" integer-property
> d# 800000000 " clock-frequency" integer-property
> finish-device
>
> new-device
> " pll1_2" device-name
> " fixed-factor-clock" +compatible
> 0 " #clock-cells" integer-property
> 2 " clock-div" integer-property
> 1 " clock-mult" integer-property
> " /pll1" encode-phandle " clocks" property
> finish-device
>
> new-device
> " pll1_4" device-name
> " fixed-factor-clock" +compatible
> 0 " #clock-cells" integer-property
> 4 " clock-div" integer-property
> 1 " clock-mult" integer-property
> " /pll1" encode-phandle " clocks" property
> finish-device
>
> new-device
> " pll1_7" device-name
> " fixed-factor-clock" +compatible
> 0 " #clock-cells" integer-property
> 7 " clock-div" integer-property
> 1 " clock-mult" integer-property
> " /pll1" encode-phandle " clocks" property
> finish-device
>
> new-device
> " usb_pll" device-name
> " fixed-clock" +compatible
> 0 " #clock-cells" integer-property
> d# 480000000 " clock-frequency" integer-property
> finish-device
>
> new-device
> " apmu-clock" device-name
> h# d4282800 h# 1000 reg
>
> new-device
> " audio-clock" device-name
> " marvell,mmp-apmu-clock" +compatible
> 0 " #clock-cells" integer-property
> " /pll1_2" encode-phandle " clocks" property
> h# 10c " reg" integer-property
> h# 13 " enable-mask" integer-property
> finish-device
>
> new-device
> " ccic-clock" device-name
> " marvell,mmp-apmu-clock" +compatible
> 0 " #clock-cells" integer-property
> " /pll1_2" encode-phandle " clocks" property
> h# 50 " reg" integer-property
> h# 3f " enable-mask" integer-property
> finish-device
>
> new-device
> " wtm-clock" device-name
> " marvell,mmp-apmu-clock" +compatible
> 0 " #clock-cells" integer-property
> " /pll1_4" encode-phandle " clocks" property
> h# 68 " reg" integer-property
> h# 1b " enable-mask" integer-property
> finish-device
>
> new-device
> " usb-clock" device-name
> " marvell,mmp-apmu-clock" +compatible
> 0 " #clock-cells" integer-property
> " /usb_pll" encode-phandle " clocks" property
> h# 5c " reg" integer-property
> h# 09 " enable-mask" integer-property
> finish-device
>
> new-device
> " sdh1-clock" device-name
> " marvell,mmp-apmu-clock" +compatible
> 0 " #clock-cells" integer-property
> " /pll1_4" encode-phandle " clocks" property
> h# 54 " reg" integer-property
> h# 41b " enable-mask" integer-property
> finish-device
>
> new-device
> " sdh2-clock" device-name
> " marvell,mmp-apmu-clock" +compatible
> 0 " #clock-cells" integer-property
> " /pll1_4" encode-phandle " clocks" property
> h# 58 " reg" integer-property
> h# 1b " enable-mask" integer-property
> finish-device
>
> [ifdef] mmp3
> new-device
> " sdh5-clock" device-name
> " marvell,mmp-apmu-clock" +compatible
> 0 " #clock-cells" integer-property
> " /pll1_4" encode-phandle " clocks" property
> h# 15c " reg" integer-property
> h# 1b " enable-mask" integer-property
> finish-device
> [then]
>
> \ FIXME should be mmp2-only? does appear in XO4 DT
> new-device
> " mspro-clock" device-name
> " marvell,mmp-apmu-clock" +compatible
> 0 " #clock-cells" integer-property
> " /pll1_4" encode-phandle " clocks" property
> h# d8 " reg" integer-property
> h# 3f " enable-mask" integer-property
> finish-device
>
> new-device
> " lcd-clock" device-name
> " marvell,mmp-apmu-clock" +compatible
> 0 " #clock-cells" integer-property
> " /pll1_7" encode-phandle " clocks" property
> h# 4c " reg" integer-property
> h# 71b " enable-mask" integer-property
> finish-device
>
> new-device
> " vmeta-clock" device-name
> " marvell,mmp-apmu-clock" +compatible
> 0 " #clock-cells" integer-property
> " /pll1_2" encode-phandle " clocks" property
> h# a4 " reg" integer-property
> h# 1b " enable-mask" integer-property
> finish-device
>
> new-device
> " gc-clock" device-name
> " marvell,mmp-apmu-clock" +compatible
> 0 " #clock-cells" integer-property
> " /pll1_4" encode-phandle " clocks" property
> h# cc " reg" integer-property
> h# 0f " enable-mask" integer-property
> finish-device
>
> finish-device
> device-end
>
> dev /audio
> " /apmu-clock/audio-clock" encode-phandle " clocks" property
> device-end
>
> dev /sspa
> " /apmu-clock/audio-clock" encode-phandle " clocks" property
> device-end
>
> dev /camera
> " /apmu-clock/ccic-clock" encode-phandle " clocks" property
> device-end
>
> dev /ap-sp
> " /apmu-clock/wtm-clock" encode-phandle " clocks" property
> device-end
>
> dev /usb
> " /apmu-clock/usb-clock" encode-phandle " clocks" property
> device-end
>
> dev /sd/sdhci@d4280000
> " /apmu-clock/sdh1-clock" encode-phandle " clocks" property
> device-end
>
> dev /sd/sdhci@d4281000
> \ FIXME clock should be mmp2-only? does appear in XO4 DT
> " /apmu-clock/mspro-clock" encode-phandle " clocks" property
> device-end
>
> dev /sd/sdhci@d4280800
> " /apmu-clock/sdh2-clock" encode-phandle " clocks" property
> device-end
>
> [ifdef] mmp3
> dev /sd/sdhci@d4217000
> " /apmu-clock/sdh5-clock" encode-phandle " clocks" property
> device-end
> [then]
>
> dev /display
> " /apmu-clock/lcd-clock" encode-phandle " clocks" property
> device-end
>
> dev /vmeta
> " /apmu-clock/vmeta-clock" encode-phandle " clocks" property
> device-end
>
> [ifdef] mmp3
> dev /gpu
> " /apmu-clock/gc-clock" encode-phandle " clocks" property
> device-end
> [then]
>
>
>
>
> [PATCH] clk: mmp: add support for DT-defined clocks
>
> Add support to the existing mmp clock drivers for clocks to be defined
> in the device tree. This will be used on OLPC MMP2/MMP3-based laptops.
>
> If clock info cannot be found in the device tree, we fall back to the
> static clock initialization already present.
>
> Signed-off-by: Daniel Drake <d...@laptop.org>
> ---
> .../devicetree/bindings/clock/mmp-apbc.txt | 30 ++++++++++
> .../devicetree/bindings/clock/mmp-apmu.txt | 30 ++++++++++
> drivers/clk/mmp/clk-apbc.c | 66
> +++++++++++++++++++++-
> drivers/clk/mmp/clk-apmu.c | 37 +++++++++++-
> drivers/clk/mmp/clk-mmp2.c | 19 ++++++-
> 5 files changed, 179 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clock/mmp-apbc.txt
> create mode 100644 Documentation/devicetree/bindings/clock/mmp-apmu.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/mmp-apbc.txt
> b/Documentation/devicetree/bindings/clock/mmp-apbc.txt
> new file mode 100644
> index 0000000..88e1253
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/mmp-apbc.txt
> @@ -0,0 +1,30 @@
> +* Clock bindings for Marvell MMP Advanced Peripheral Bus clock
> +
> +Parent apb-clock node
> +=====================
> +Required properties:
> +- reg: Address and length of the APB clock unit registers
> +
> +
> +Child peripheral clock nodes
> +============================
> +Required properties:
> +- compatible : shall be "marvell,mmp-apb-clock"
> +- #clock-cells : from common clock binding; shall be set to 1
> +- clocks : parent clock, from common clock binding
> +- clock-output-names : Array of clock names, from common clock binding
> +- reg : Array of control register offsets into APB clock unit register space
> +
> +
> +Example:
> +apbc: apb-clock {
> + reg = <0xd4015000 0x1000>;
> +
> + twsi-clocks {
> + compatible = "marvell,mmp-apb-clock";
> + #clock-cells = <1>;
> + clocks = <&vctvxo-clock>;
> + clock-output-names = "TWSI0", "TWSI1";
> + reg = <0x04 0x08>;
> + };
> +};
> diff --git a/Documentation/devicetree/bindings/clock/mmp-apmu.txt
> b/Documentation/devicetree/bindings/clock/mmp-apmu.txt
> new file mode 100644
> index 0000000..18bb0f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/mmp-apmu.txt
> @@ -0,0 +1,30 @@
> +* Clock bindings for Marvell MMP Application Subsystem Power Management Unit
> +
> +Parent apmu-clock node
> +=====================
> +Required properties:
> +- reg: Address and length of the CPU Subsystem PMU registers
> +
> +
> +Child peripheral clock nodes
> +============================
> +Required properties:
> +- compatible : shall be "marvell,mmp-apmu-clock"
> +- #clock-cells : from common clock binding; shall be set to 0
> +- clocks : parent clock, from common clock binding
> +- reg : Control register offsets into parent register space
> +- enable-mask : The bits to be set in the register to enable the clock, or
> + cleared to disable.
> +
> +Example:
> +apmu-clock {
> + reg = <0xd4282800 0x1000>;
> +
> + usb-clock {
> + compatible = "marvell,mmp-apmu-clock";
> + #clock-cells = <0>;
> + clocks = <&usb_pll>;
> + reg = <0x5c>;
> + enable-mask = <0x09>;
> + };
> +};
> diff --git a/drivers/clk/mmp/clk-apbc.c b/drivers/clk/mmp/clk-apbc.c
> index 89a146a..d53bc79 100644
> --- a/drivers/clk/mmp/clk-apbc.c
> +++ b/drivers/clk/mmp/clk-apbc.c
> @@ -15,6 +15,8 @@
> #include <linux/err.h>
> #include <linux/delay.h>
> #include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
>
> #include "clk.h"
>
> @@ -129,8 +131,70 @@ struct clk *mmp_clk_register_apbc(const char *name,
> const char *parent_name,
> apbc->hw.init = &init;
>
> clk = clk_register(NULL, &apbc->hw);
> - if (IS_ERR(clk))
> + if (WARN_ON(IS_ERR(clk)))
> kfree(apbc);
>
> return clk;
> }
> +
> +static void __init mmp_apbc_dt_init(struct device_node *node)
> +{
> + struct device_node *parent = of_get_parent(node);
> + const __be32 *regs;
> + struct clk *clk;
> + int rc;
> + int len;
> + int num_clocks;
> + int clock_num;
> + void __iomem *baseaddr;
> + const char *clock_name;
> + const char *parent_name;
> + struct clk **clks;
> + struct clk_onecell_data *clk_data;
> +
> + regs = of_get_property(node, "reg", &len);
> + if (WARN_ON(!regs || (len % sizeof(__be32) != 0)))
> + return;
> + num_clocks = len / sizeof(__be32);
> +
> + baseaddr = of_iomap(parent, 0);
> + of_node_put(parent);
> + if (WARN_ON(!baseaddr))
> + return;
> +
> + clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> + if (WARN_ON(!clk_data))
> + goto err_clk_data;
> +
> + clks = kzalloc(num_clocks * sizeof(*clks), GFP_KERNEL);
> + if (WARN_ON(!clks))
> + goto err_clks;
> +
> + clock_name = of_get_property(node, "clock-output-names", NULL);
> + parent_name = of_clk_get_parent_name(node, 0);
> +
> + for (clock_num = 0; clock_num < num_clocks; clock_num++) {
> + void __iomem *clock_addr;
> + clock_addr = baseaddr + be32_to_cpup(regs + clock_num);
> +
> + clk = mmp_clk_register_apbc(clock_name, parent_name,
> + clock_addr, 10, 0);
> + if (IS_ERR(clk))
> + return;
> + clock_name += strlen(clock_name) + 1;
> + clks[clock_num] = clk;
> + }
> +
> + clk_data->clk_num = num_clocks;
> + clk_data->clks = clks;
> + rc = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> + WARN_ON(rc);
> + return;
> +
> +err_clks:
> + kfree(clk_data);
> +err_clk_data:
> + iounmap(baseaddr);
> +
> +}
> +CLK_OF_DECLARE(mmp2_apbc, "marvell,mmp-apb-clock", mmp_apbc_dt_init);
> diff --git a/drivers/clk/mmp/clk-apmu.c b/drivers/clk/mmp/clk-apmu.c
> index 4f4d8c5..dfcecc9 100644
> --- a/drivers/clk/mmp/clk-apmu.c
> +++ b/drivers/clk/mmp/clk-apmu.c
> @@ -15,6 +15,8 @@
> #include <linux/err.h>
> #include <linux/delay.h>
> #include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
>
> #include "clk.h"
>
> @@ -22,7 +24,6 @@
> struct clk_apmu {
> struct clk_hw hw;
> void __iomem *base;
> - u32 rst_mask;
> u32 enable_mask;
> };
>
> @@ -85,3 +86,37 @@ struct clk *mmp_clk_register_apmu(const char *name, const
> char *parent_name,
>
> return clk;
> }
> +
> +static void __init mmp_apmu_dt_init(struct device_node *node)
> +{
> + struct device_node *parent = of_get_parent(node);
> + const char *clk_name = node->name;
> + void __iomem *baseaddr;
> + struct clk *clk;
> + u32 reg;
> + u32 enable_mask;
> + int rc;
> +
> + rc = of_property_read_u32(node, "reg", ®);
> + if (WARN_ON(rc))
> + return;
> +
> + rc = of_property_read_u32(node, "enable-mask", &enable_mask);
> + if (WARN_ON(rc))
> + return;
> +
> + baseaddr = of_iomap(parent, 0);
> + of_node_put(parent);
> + if (WARN_ON(!baseaddr))
> + return;
> +
> + of_property_read_string(node, "clock-output-names", &clk_name);
> + clk = mmp_clk_register_apmu(clk_name, of_clk_get_parent_name(node, 0),
> + baseaddr + reg, enable_mask);
> + if (WARN_ON(IS_ERR(clk)))
> + return;
> +
> + rc = of_clk_add_provider(node, of_clk_src_simple_get, clk);
> + WARN_ON(rc);
> +}
> +CLK_OF_DECLARE(mmp2_apmu, "marvell,mmp-apmu-clock", mmp_apmu_dt_init);
> diff --git a/drivers/clk/mmp/clk-mmp2.c b/drivers/clk/mmp/clk-mmp2.c
> index 6f654c3..6abb083 100644
> --- a/drivers/clk/mmp/clk-mmp2.c
> +++ b/drivers/clk/mmp/clk-mmp2.c
> @@ -15,6 +15,7 @@
> #include <linux/io.h>
> #include <linux/delay.h>
> #include <linux/err.h>
> +#include <linux/of.h>
>
> #include <mach/addr-map.h>
>
> @@ -73,7 +74,7 @@ static const char *sdh_parent[] = {"pll1_4", "pll2",
> "usb_pll", "pll1"};
> static const char *disp_parent[] = {"pll1", "pll1_16", "pll2", "vctcxo"};
> static const char *ccic_parent[] = {"pll1_2", "pll1_16", "vctcxo"};
>
> -void __init mmp2_clk_init(void)
> +static void __init mmp2_clk_init_static(void)
> {
> struct clk *clk;
> struct clk *vctcxo;
> @@ -445,3 +446,19 @@ void __init mmp2_clk_init(void)
> apmu_base + APMU_CCIC1, 0x300);
> clk_register_clkdev(clk, "sphyclk", "mmp-ccic.1");
> }
> +
> +void __init mmp2_clk_init(void)
> +{
> + struct device_node *node;
> +
> + /* The presence of the ABP clock node in the device tree indicates
> + * that we should use the DT for clock setup. */
> + node = of_find_compatible_node(NULL, NULL, "marvell,mmp-apb-clock");
> + if (node) {
> + of_node_put(node);
> + of_clk_init(NULL);
> + } else {
> + /* Fallback to static setup */
> + mmp2_clk_init_static();
> + }
> +}
>
_______________________________________________
Devel mailing list
Devel@lists.laptop.org
http://lists.laptop.org/listinfo/devel
