* [PATCH v2 0/2] clk: keystone: Add new driver to handle ehrpwm tbclk @ 2020-02-07 4:44 Vignesh Raghavendra 2020-02-07 4:44 ` [PATCH v2 1/2] dt-bindings: clock: Add binding documentation for TI syscon gate clock Vignesh Raghavendra 2020-02-07 4:44 ` [PATCH v2 2/2] clk: keystone: Add new driver to handle syscon based clocks Vignesh Raghavendra 0 siblings, 2 replies; 7+ messages in thread From: Vignesh Raghavendra @ 2020-02-07 4:44 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland, Santosh Shilimkar Cc: linux-clk, devicetree, linux-kernel, Vignesh Raghavendra, Tero Kristo On TI's AM654 and J721e SoCs, certain clocks can be gated/ungated by setting a single bit in SoC's System Control registers. Sometime more than one clock control can be in the same register. But these registers might also have bits to control other SoC functionalities. For example, Time Base clock(TBclk) enable bits for various EPWM IPs are all in EPWM_CTRL Syscon registers on K2G SoC. This series adds a new clk driver to support controlling tbclk. Registers which control clocks will be grouped into a syscon DT node, thus enabling sharing of register across clk drivers and other drivers. v2: Simplify driver to have only one clock node per group of syscon controller registers instead of one per clock instance. v1: https://patchwork.kernel.org/cover/10848783/ Vignesh Raghavendra (2): dt-bindings: clock: Add binding documentation for TI syscon gate clock clk: keystone: Add new driver to handle syscon based clocks .../bindings/clock/ti,am654-ehrpwm-tbclk.yaml | 47 +++++ drivers/clk/keystone/Kconfig | 8 + drivers/clk/keystone/Makefile | 1 + drivers/clk/keystone/syscon-clk.c | 177 ++++++++++++++++++ 4 files changed, 233 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml create mode 100644 drivers/clk/keystone/syscon-clk.c -- 2.25.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] dt-bindings: clock: Add binding documentation for TI syscon gate clock 2020-02-07 4:44 [PATCH v2 0/2] clk: keystone: Add new driver to handle ehrpwm tbclk Vignesh Raghavendra @ 2020-02-07 4:44 ` Vignesh Raghavendra 2020-02-10 18:45 ` Stephen Boyd 2020-02-07 4:44 ` [PATCH v2 2/2] clk: keystone: Add new driver to handle syscon based clocks Vignesh Raghavendra 1 sibling, 1 reply; 7+ messages in thread From: Vignesh Raghavendra @ 2020-02-07 4:44 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland, Santosh Shilimkar Cc: linux-clk, devicetree, linux-kernel, Vignesh Raghavendra, Tero Kristo Add dt bindings for TI syscon gate clock driver that is used to control EHRPWM's TimeBase clock (TBCLK) on TI's AM654 SoC. Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> --- .../bindings/clock/ti,am654-ehrpwm-tbclk.yaml | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml diff --git a/Documentation/devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml b/Documentation/devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml new file mode 100644 index 000000000000..98fcac2b41f3 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml @@ -0,0 +1,47 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/ti,am654-ehrpwm-tbclk.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TI syscon gate clock driver + +maintainers: + - Vignesh Raghavendra <vigneshr@ti.com> + +description: | + + This binding uses common clock bindings + Documentation/devicetree/bindings/clock/clock-bindings.txt + +properties: + compatible: + items: + - const: ti,am654-ehrpwm-tbclk + + "#clock-cells": + const: 1 + + ti,tbclk-syscon: + $ref: /schemas/types.yaml#/definitions/phandle + description: + Phandle to the system controller node that has bits to + control eHRPWM's TBCLK + +required: + - compatible + - "#clock-cells" + - ti,tbclk-syscon + +examples: + - | + tbclk_ctrl: tbclk_ctrl@4140 { + compatible = "syscon"; + reg = <0x4140 0x18>; + }; + + ehrpwm_tbclk: clk0 { + compatible = "ti,am654-ehrpwm-tbclk"; + #clock-cells = <1>; + ti,tbclk-syscon = <&tbclk_ctrl>; + }; -- 2.25.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: clock: Add binding documentation for TI syscon gate clock 2020-02-07 4:44 ` [PATCH v2 1/2] dt-bindings: clock: Add binding documentation for TI syscon gate clock Vignesh Raghavendra @ 2020-02-10 18:45 ` Stephen Boyd 2020-02-15 12:37 ` Vignesh Raghavendra 0 siblings, 1 reply; 7+ messages in thread From: Stephen Boyd @ 2020-02-10 18:45 UTC (permalink / raw) To: Mark Rutland, Michael Turquette, Rob Herring, Santosh Shilimkar, Vignesh Raghavendra Cc: linux-clk, devicetree, linux-kernel, Vignesh Raghavendra, Tero Kristo Quoting Vignesh Raghavendra (2020-02-06 20:44:24) > diff --git a/Documentation/devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml b/Documentation/devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml > new file mode 100644 > index 000000000000..98fcac2b41f3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml > @@ -0,0 +1,47 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/ti,am654-ehrpwm-tbclk.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: TI syscon gate clock driver > + > +maintainers: > + - Vignesh Raghavendra <vigneshr@ti.com> > + > +description: | > + > + This binding uses common clock bindings > + Documentation/devicetree/bindings/clock/clock-bindings.txt Maybe have a real description instead of this line which is mostly useless. > + > +properties: > + compatible: > + items: I think you can drop items. > + - const: ti,am654-ehrpwm-tbclk > + > + "#clock-cells": > + const: 1 > + > + ti,tbclk-syscon: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + Phandle to the system controller node that has bits to > + control eHRPWM's TBCLK > + > +required: > + - compatible > + - "#clock-cells" > + - ti,tbclk-syscon > + > +examples: > + - | > + tbclk_ctrl: tbclk_ctrl@4140 { > + compatible = "syscon"; > + reg = <0x4140 0x18>; > + }; > + > + ehrpwm_tbclk: clk0 { > + compatible = "ti,am654-ehrpwm-tbclk"; > + #clock-cells = <1>; > + ti,tbclk-syscon = <&tbclk_ctrl>; > + }; I don't understand the binding. Why can't the syscon node register clks and have #clock-cells? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: clock: Add binding documentation for TI syscon gate clock 2020-02-10 18:45 ` Stephen Boyd @ 2020-02-15 12:37 ` Vignesh Raghavendra 0 siblings, 0 replies; 7+ messages in thread From: Vignesh Raghavendra @ 2020-02-15 12:37 UTC (permalink / raw) To: Stephen Boyd, Mark Rutland, Michael Turquette, Rob Herring, Santosh Shilimkar Cc: linux-clk, devicetree, linux-kernel, Tero Kristo Hi, On 2/11/2020 12:15 AM, Stephen Boyd wrote: > Quoting Vignesh Raghavendra (2020-02-06 20:44:24) [...] >> + - Vignesh Raghavendra <vigneshr@ti.com> >> + >> +description: | >> + >> + This binding uses common clock bindings >> + Documentation/devicetree/bindings/clock/clock-bindings.txt > > Maybe have a real description instead of this line which is mostly > useless. > Will drop this line.. >> + >> +properties: >> + compatible: >> + items: > > I think you can drop items. > >> + - const: ti,am654-ehrpwm-tbclk >> + >> + "#clock-cells": >> + const: 1 >> + >> + ti,tbclk-syscon: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: >> + Phandle to the system controller node that has bits to >> + control eHRPWM's TBCLK >> + >> +required: >> + - compatible >> + - "#clock-cells" >> + - ti,tbclk-syscon >> + >> +examples: >> + - | >> + tbclk_ctrl: tbclk_ctrl@4140 { >> + compatible = "syscon"; >> + reg = <0x4140 0x18>; >> + }; >> + >> + ehrpwm_tbclk: clk0 { >> + compatible = "ti,am654-ehrpwm-tbclk"; >> + #clock-cells = <1>; >> + ti,tbclk-syscon = <&tbclk_ctrl>; >> + }; > > I don't understand the binding. Why can't the syscon node register clks > and have #clock-cells? > I did not know that would work. Will make syscon code to register clks.. Thanks! Regards Vignesh ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] clk: keystone: Add new driver to handle syscon based clocks 2020-02-07 4:44 [PATCH v2 0/2] clk: keystone: Add new driver to handle ehrpwm tbclk Vignesh Raghavendra 2020-02-07 4:44 ` [PATCH v2 1/2] dt-bindings: clock: Add binding documentation for TI syscon gate clock Vignesh Raghavendra @ 2020-02-07 4:44 ` Vignesh Raghavendra 2020-02-10 18:59 ` Stephen Boyd 1 sibling, 1 reply; 7+ messages in thread From: Vignesh Raghavendra @ 2020-02-07 4:44 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland, Santosh Shilimkar Cc: linux-clk, devicetree, linux-kernel, Vignesh Raghavendra, Tero Kristo On TI's AM654/J721e SoCs, certain clocks can be gated/ungated by setting a single bit in SoC's System Control Module registers. Sometime more than one clock control can be in the same register. Add driver to support such clocks. Registers that control clocks will be grouped into a syscon regmap. Clock provider node will be child of the syscon node. Driver currently supports controlling EHRPWM's TimeBase clock(TBCLK) for AM654 SoC. Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> --- drivers/clk/keystone/Kconfig | 8 ++ drivers/clk/keystone/Makefile | 1 + drivers/clk/keystone/syscon-clk.c | 177 ++++++++++++++++++++++++++++++ 3 files changed, 186 insertions(+) create mode 100644 drivers/clk/keystone/syscon-clk.c diff --git a/drivers/clk/keystone/Kconfig b/drivers/clk/keystone/Kconfig index 38aeefb1e808..69ca3db1a99e 100644 --- a/drivers/clk/keystone/Kconfig +++ b/drivers/clk/keystone/Kconfig @@ -26,3 +26,11 @@ config TI_SCI_CLK_PROBE_FROM_FW This is mostly only useful for debugging purposes, and will increase the boot time of the device. If you want the clocks probed from firmware, say Y. Otherwise, say N. + +config TI_SYSCON_CLK + tristate "Syscon based clock driver for K2/K3 SoCs" + depends on (ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST) && OF + default (ARCH_KEYSTONE || ARCH_K3) + help + This adds clock driver support for syscon based gate + clocks on TI's K2 and K3 SoCs. diff --git a/drivers/clk/keystone/Makefile b/drivers/clk/keystone/Makefile index d044de6f965c..0e426e648f7c 100644 --- a/drivers/clk/keystone/Makefile +++ b/drivers/clk/keystone/Makefile @@ -1,3 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only obj-$(CONFIG_COMMON_CLK_KEYSTONE) += pll.o gate.o obj-$(CONFIG_TI_SCI_CLK) += sci-clk.o +obj-$(CONFIG_TI_SYSCON_CLK) += syscon-clk.o diff --git a/drivers/clk/keystone/syscon-clk.c b/drivers/clk/keystone/syscon-clk.c new file mode 100644 index 000000000000..42e7416371ff --- /dev/null +++ b/drivers/clk/keystone/syscon-clk.c @@ -0,0 +1,177 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/ +// + +#include <linux/clk-provider.h> +#include <linux/clk.h> +#include <linux/mfd/syscon.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> + +struct ti_syscon_gate_clk_priv { + struct clk_hw hw; + struct regmap *regmap; + u32 reg; + u32 idx; +}; + +struct ti_syscon_gate_clk_data { + char *name; + u32 offset; + u32 bit_idx; +}; + +static struct +ti_syscon_gate_clk_priv *to_ti_syscon_gate_clk_priv(struct clk_hw *hw) +{ + return container_of(hw, struct ti_syscon_gate_clk_priv, hw); +} + +static int ti_syscon_gate_clk_enable(struct clk_hw *hw) +{ + struct ti_syscon_gate_clk_priv *priv = to_ti_syscon_gate_clk_priv(hw); + + return regmap_write_bits(priv->regmap, priv->reg, priv->idx, + priv->idx); +} + +static void ti_syscon_gate_clk_disable(struct clk_hw *hw) +{ + struct ti_syscon_gate_clk_priv *priv = to_ti_syscon_gate_clk_priv(hw); + + regmap_write_bits(priv->regmap, priv->reg, priv->idx, 0); +} + +static int ti_syscon_gate_clk_is_enabled(struct clk_hw *hw) +{ + unsigned int val; + struct ti_syscon_gate_clk_priv *priv = to_ti_syscon_gate_clk_priv(hw); + + regmap_read(priv->regmap, priv->reg, &val); + + return !!(val & priv->idx); +} + +static const struct clk_ops ti_syscon_gate_clk_ops = { + .enable = ti_syscon_gate_clk_enable, + .disable = ti_syscon_gate_clk_disable, + .is_enabled = ti_syscon_gate_clk_is_enabled, +}; + +static struct clk_hw +*ti_syscon_gate_clk_register(struct device *dev, struct regmap *regmap, + const struct ti_syscon_gate_clk_data *data) +{ + struct ti_syscon_gate_clk_priv *priv; + struct clk_init_data init; + int ret; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return ERR_PTR(-ENOMEM); + + init.name = data->name; + init.ops = &ti_syscon_gate_clk_ops; + init.parent_names = NULL; + init.num_parents = 0; + init.flags = 0; + + priv->regmap = regmap; + priv->reg = data->offset; + priv->idx = BIT(data->bit_idx); + priv->hw.init = &init; + + ret = devm_clk_hw_register(dev, &priv->hw); + if (ret) + return ERR_PTR(ret); + + return &priv->hw; +} + +static int ti_syscon_gate_clk_probe(struct platform_device *pdev) +{ + const struct ti_syscon_gate_clk_data *data, *p; + struct clk_hw_onecell_data *hw_data; + struct device *dev = &pdev->dev; + struct regmap *regmap; + int num_clks = 0; + int i; + + data = of_device_get_match_data(dev); + if (!data) + return -EINVAL; + + regmap = syscon_regmap_lookup_by_phandle(dev->of_node, + "ti,tbclk-syscon"); + if (IS_ERR(regmap)) { + if (PTR_ERR(regmap) == -EPROBE_DEFER) + return -EPROBE_DEFER; + dev_err(dev, "failed to find parent regmap\n"); + return PTR_ERR(regmap); + } + + for (p = data; p->name; p++) + num_clks++; + + hw_data = devm_kzalloc(dev, struct_size(hw_data, hws, num_clks), + GFP_KERNEL); + if (!hw_data) + return -ENOMEM; + + hw_data->num = num_clks; + + for (i = 0; i < num_clks; i++) { + hw_data->hws[i] = ti_syscon_gate_clk_register(dev, regmap, + &data[i]); + if (IS_ERR(hw_data->hws[i])) + dev_err(dev, "failed to register %s", + data[i].name); + } + + return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, + hw_data); +} + +#define TI_SYSCON_CLK_GATE(_name, _offset, _bit_idx) \ + { \ + .name = _name, \ + .offset = (_offset), \ + .bit_idx = (_bit_idx), \ + } + +static const struct ti_syscon_gate_clk_data am654_clk_data[] = { + TI_SYSCON_CLK_GATE("ehrpwm_tbclk0", 0x0, 0), + TI_SYSCON_CLK_GATE("ehrpwm_tbclk1", 0x4, 0), + TI_SYSCON_CLK_GATE("ehrpwm_tbclk2", 0x8, 0), + TI_SYSCON_CLK_GATE("ehrpwm_tbclk3", 0xc, 0), + TI_SYSCON_CLK_GATE("ehrpwm_tbclk4", 0x10, 0), + TI_SYSCON_CLK_GATE("ehrpwm_tbclk5", 0x14, 0), + { /* Sentinel */ }, +}; + +static const struct of_device_id ti_syscon_gate_clk_ids[] = { + { + .compatible = "ti,am654-ehrpwm-tbclk", + .data = &am654_clk_data, + }, + { } +}; +MODULE_DEVICE_TABLE(of, ti_syscon_gate_clk_ids); + +static struct platform_driver ti_syscon_gate_clk_driver = { + .probe = ti_syscon_gate_clk_probe, + .driver = { + .name = "ti-syscon-gate-clk", + .of_match_table = ti_syscon_gate_clk_ids, + }, +}; + +module_platform_driver(ti_syscon_gate_clk_driver); + +MODULE_AUTHOR("Vignesh Raghavendra <vigneshr@ti.com>"); +MODULE_DESCRIPTION("Syscon backed gate-clock driver"); +MODULE_LICENSE("GPL"); -- 2.25.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] clk: keystone: Add new driver to handle syscon based clocks 2020-02-07 4:44 ` [PATCH v2 2/2] clk: keystone: Add new driver to handle syscon based clocks Vignesh Raghavendra @ 2020-02-10 18:59 ` Stephen Boyd 2020-02-15 12:42 ` Vignesh Raghavendra 0 siblings, 1 reply; 7+ messages in thread From: Stephen Boyd @ 2020-02-10 18:59 UTC (permalink / raw) To: Mark Rutland, Michael Turquette, Rob Herring, Santosh Shilimkar, Vignesh Raghavendra Cc: linux-clk, devicetree, linux-kernel, Vignesh Raghavendra, Tero Kristo Quoting Vignesh Raghavendra (2020-02-06 20:44:25) > diff --git a/drivers/clk/keystone/Kconfig b/drivers/clk/keystone/Kconfig > index 38aeefb1e808..69ca3db1a99e 100644 > --- a/drivers/clk/keystone/Kconfig > +++ b/drivers/clk/keystone/Kconfig > @@ -26,3 +26,11 @@ config TI_SCI_CLK_PROBE_FROM_FW > This is mostly only useful for debugging purposes, and will > increase the boot time of the device. If you want the clocks probed > from firmware, say Y. Otherwise, say N. > + > +config TI_SYSCON_CLK > + tristate "Syscon based clock driver for K2/K3 SoCs" > + depends on (ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST) && OF > + default (ARCH_KEYSTONE || ARCH_K3) Drop parenthesis. It's not useful. Also, not sure why OF is a build dependency. Please drop it. depends on ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST default ARCH_KEYSTONE || ARCH_K3 > + help > + This adds clock driver support for syscon based gate > + clocks on TI's K2 and K3 SoCs. > diff --git a/drivers/clk/keystone/Makefile b/drivers/clk/keystone/Makefile > index d044de6f965c..0e426e648f7c 100644 > --- a/drivers/clk/keystone/Makefile > +++ b/drivers/clk/keystone/Makefile > @@ -1,3 +1,4 @@ > # SPDX-License-Identifier: GPL-2.0-only > obj-$(CONFIG_COMMON_CLK_KEYSTONE) += pll.o gate.o > obj-$(CONFIG_TI_SCI_CLK) += sci-clk.o > +obj-$(CONFIG_TI_SYSCON_CLK) += syscon-clk.o > diff --git a/drivers/clk/keystone/syscon-clk.c b/drivers/clk/keystone/syscon-clk.c > new file mode 100644 > index 000000000000..42e7416371ff > --- /dev/null > +++ b/drivers/clk/keystone/syscon-clk.c > @@ -0,0 +1,177 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// > +// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/ > +// These last three comment lines should be normal kernel style. /* */ > + > +#include <linux/clk-provider.h> > +#include <linux/clk.h> Is this used? > +#include <linux/mfd/syscon.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> Hopefully these two includes aren't needed. > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + [...] > + > +static int ti_syscon_gate_clk_probe(struct platform_device *pdev) > +{ > + const struct ti_syscon_gate_clk_data *data, *p; > + struct clk_hw_onecell_data *hw_data; > + struct device *dev = &pdev->dev; > + struct regmap *regmap; > + int num_clks = 0; Please don't initialize here. > + int i; > + > + data = of_device_get_match_data(dev); Use device_get_match_data() instead? > + if (!data) > + return -EINVAL; > + > + regmap = syscon_regmap_lookup_by_phandle(dev->of_node, > + "ti,tbclk-syscon"); > + if (IS_ERR(regmap)) { > + if (PTR_ERR(regmap) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + dev_err(dev, "failed to find parent regmap\n"); > + return PTR_ERR(regmap); > + } > + > + for (p = data; p->name; p++) Initialize num_clks here so we know it's a loop that's counting. > + num_clks++; > + > + hw_data = devm_kzalloc(dev, struct_size(hw_data, hws, num_clks), > + GFP_KERNEL); > + if (!hw_data) > + return -ENOMEM; > + > + hw_data->num = num_clks; > + > + for (i = 0; i < num_clks; i++) { > + hw_data->hws[i] = ti_syscon_gate_clk_register(dev, regmap, > + &data[i]); > + if (IS_ERR(hw_data->hws[i])) > + dev_err(dev, "failed to register %s", Add a newline? > + data[i].name); And we don't fail? So it really isn't a problem? Maybe dev_warn() instead? > + } > + > + return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, > + hw_data); > +} > + > +#define TI_SYSCON_CLK_GATE(_name, _offset, _bit_idx) \ > + { \ > + .name = _name, \ > + .offset = (_offset), \ > + .bit_idx = (_bit_idx), \ > + } > + > +static const struct ti_syscon_gate_clk_data am654_clk_data[] = { > + TI_SYSCON_CLK_GATE("ehrpwm_tbclk0", 0x0, 0), > + TI_SYSCON_CLK_GATE("ehrpwm_tbclk1", 0x4, 0), > + TI_SYSCON_CLK_GATE("ehrpwm_tbclk2", 0x8, 0), > + TI_SYSCON_CLK_GATE("ehrpwm_tbclk3", 0xc, 0), > + TI_SYSCON_CLK_GATE("ehrpwm_tbclk4", 0x10, 0), > + TI_SYSCON_CLK_GATE("ehrpwm_tbclk5", 0x14, 0), > + { /* Sentinel */ }, > +}; > + > +static const struct of_device_id ti_syscon_gate_clk_ids[] = { > + { > + .compatible = "ti,am654-ehrpwm-tbclk", > + .data = &am654_clk_data, > + }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ti_syscon_gate_clk_ids); > + > +static struct platform_driver ti_syscon_gate_clk_driver = { > + .probe = ti_syscon_gate_clk_probe, > + .driver = { > + .name = "ti-syscon-gate-clk", > + .of_match_table = ti_syscon_gate_clk_ids, > + }, > +}; > + Nitpick: Drop the newline. > +module_platform_driver(ti_syscon_gate_clk_driver); > + ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] clk: keystone: Add new driver to handle syscon based clocks 2020-02-10 18:59 ` Stephen Boyd @ 2020-02-15 12:42 ` Vignesh Raghavendra 0 siblings, 0 replies; 7+ messages in thread From: Vignesh Raghavendra @ 2020-02-15 12:42 UTC (permalink / raw) To: Stephen Boyd, Mark Rutland, Michael Turquette, Rob Herring, Santosh Shilimkar Cc: linux-clk, devicetree, linux-kernel, Tero Kristo On 2/11/2020 12:29 AM, Stephen Boyd wrote: > Quoting Vignesh Raghavendra (2020-02-06 20:44:25) >> diff --git a/drivers/clk/keystone/Kconfig b/drivers/clk/keystone/Kconfig >> index 38aeefb1e808..69ca3db1a99e 100644 >> --- a/drivers/clk/keystone/Kconfig >> +++ b/drivers/clk/keystone/Kconfig >> @@ -26,3 +26,11 @@ config TI_SCI_CLK_PROBE_FROM_FW >> This is mostly only useful for debugging purposes, and will >> increase the boot time of the device. If you want the clocks probed >> from firmware, say Y. Otherwise, say N. >> + >> +config TI_SYSCON_CLK >> + tristate "Syscon based clock driver for K2/K3 SoCs" >> + depends on (ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST) && OF >> + default (ARCH_KEYSTONE || ARCH_K3) > > Drop parenthesis. It's not useful. Also, not sure why OF is a build > dependency. Please drop it. > Yes, will drop.. > depends on ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST > default ARCH_KEYSTONE || ARCH_K3 > >> + help >> + This adds clock driver support for syscon based gate >> + clocks on TI's K2 and K3 SoCs. >> diff --git a/drivers/clk/keystone/Makefile b/drivers/clk/keystone/Makefile >> index d044de6f965c..0e426e648f7c 100644 >> --- a/drivers/clk/keystone/Makefile >> +++ b/drivers/clk/keystone/Makefile >> @@ -1,3 +1,4 @@ >> # SPDX-License-Identifier: GPL-2.0-only >> obj-$(CONFIG_COMMON_CLK_KEYSTONE) += pll.o gate.o >> obj-$(CONFIG_TI_SCI_CLK) += sci-clk.o >> +obj-$(CONFIG_TI_SYSCON_CLK) += syscon-clk.o >> diff --git a/drivers/clk/keystone/syscon-clk.c b/drivers/clk/keystone/syscon-clk.c >> new file mode 100644 >> index 000000000000..42e7416371ff >> --- /dev/null >> +++ b/drivers/clk/keystone/syscon-clk.c >> @@ -0,0 +1,177 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +// >> +// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/ >> +// > > These last three comment lines should be normal kernel style. /* */ > >> + >> +#include <linux/clk-provider.h> >> +#include <linux/clk.h> > > Is this used? > Will drop >> +#include <linux/mfd/syscon.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/of_device.h> > > Hopefully these two includes aren't needed. > Not anymore >> +#include <linux/platform_device.h> >> +#include <linux/regmap.h> >> + > [...] >> + >> +static int ti_syscon_gate_clk_probe(struct platform_device *pdev) >> +{ >> + const struct ti_syscon_gate_clk_data *data, *p; >> + struct clk_hw_onecell_data *hw_data; >> + struct device *dev = &pdev->dev; >> + struct regmap *regmap; >> + int num_clks = 0; > > Please don't initialize here. Ok > >> + int i; >> + >> + data = of_device_get_match_data(dev); > > Use device_get_match_data() instead? Sure > >> + if (!data) >> + return -EINVAL; >> + >> + regmap = syscon_regmap_lookup_by_phandle(dev->of_node, >> + "ti,tbclk-syscon"); >> + if (IS_ERR(regmap)) { >> + if (PTR_ERR(regmap) == -EPROBE_DEFER) >> + return -EPROBE_DEFER; >> + dev_err(dev, "failed to find parent regmap\n"); >> + return PTR_ERR(regmap); >> + } >> + >> + for (p = data; p->name; p++) > > Initialize num_clks here so we know it's a loop that's counting. OK > >> + num_clks++; >> + >> + hw_data = devm_kzalloc(dev, struct_size(hw_data, hws, num_clks), >> + GFP_KERNEL); >> + if (!hw_data) >> + return -ENOMEM; >> + >> + hw_data->num = num_clks; >> + >> + for (i = 0; i < num_clks; i++) { >> + hw_data->hws[i] = ti_syscon_gate_clk_register(dev, regmap, >> + &data[i]); >> + if (IS_ERR(hw_data->hws[i])) >> + dev_err(dev, "failed to register %s", > > Add a newline? Yes, will add > >> + data[i].name); > > And we don't fail? So it really isn't a problem? Maybe dev_warn() > instead? > OK >> + } >> + >> + return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, >> + hw_data); >> +} >> + >> +#define TI_SYSCON_CLK_GATE(_name, _offset, _bit_idx) \ >> + { \ >> + .name = _name, \ >> + .offset = (_offset), \ >> + .bit_idx = (_bit_idx), \ >> + } >> + >> +static const struct ti_syscon_gate_clk_data am654_clk_data[] = { >> + TI_SYSCON_CLK_GATE("ehrpwm_tbclk0", 0x0, 0), >> + TI_SYSCON_CLK_GATE("ehrpwm_tbclk1", 0x4, 0), >> + TI_SYSCON_CLK_GATE("ehrpwm_tbclk2", 0x8, 0), >> + TI_SYSCON_CLK_GATE("ehrpwm_tbclk3", 0xc, 0), >> + TI_SYSCON_CLK_GATE("ehrpwm_tbclk4", 0x10, 0), >> + TI_SYSCON_CLK_GATE("ehrpwm_tbclk5", 0x14, 0), >> + { /* Sentinel */ }, >> +}; >> + >> +static const struct of_device_id ti_syscon_gate_clk_ids[] = { >> + { >> + .compatible = "ti,am654-ehrpwm-tbclk", >> + .data = &am654_clk_data, >> + }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(of, ti_syscon_gate_clk_ids); >> + >> +static struct platform_driver ti_syscon_gate_clk_driver = { >> + .probe = ti_syscon_gate_clk_probe, >> + .driver = { >> + .name = "ti-syscon-gate-clk", >> + .of_match_table = ti_syscon_gate_clk_ids, >> + }, >> +}; >> + > > Nitpick: Drop the newline. > Ok >> +module_platform_driver(ti_syscon_gate_clk_driver); >> + Thanks for the review! Regards Vignesh ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-02-15 12:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-07 4:44 [PATCH v2 0/2] clk: keystone: Add new driver to handle ehrpwm tbclk Vignesh Raghavendra 2020-02-07 4:44 ` [PATCH v2 1/2] dt-bindings: clock: Add binding documentation for TI syscon gate clock Vignesh Raghavendra 2020-02-10 18:45 ` Stephen Boyd 2020-02-15 12:37 ` Vignesh Raghavendra 2020-02-07 4:44 ` [PATCH v2 2/2] clk: keystone: Add new driver to handle syscon based clocks Vignesh Raghavendra 2020-02-10 18:59 ` Stephen Boyd 2020-02-15 12:42 ` Vignesh Raghavendra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).