* [PATCH v4 1/7] clk: meson: aoclk: refactor common code into dedicated file
2018-04-08 3:19 [PATCH v4 0/7] clk: meson-axg: Add AO Cloclk and Reset driver Yixun Lan
@ 2018-04-08 3:19 ` Yixun Lan
2018-04-09 12:08 ` Jerome Brunet
2018-04-08 3:19 ` [PATCH v4 2/7] clk: meson: migrate to devm_of_clk_add_hw_provider API Yixun Lan
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Yixun Lan @ 2018-04-08 3:19 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Carlo Caione
Cc: Yixun Lan, Rob Herring, Michael Turquette, Stephen Boyd,
Philipp Zabel, Qiufang Dai, linux-amlogic, linux-clk,
linux-arm-kernel, linux-kernel
We try to refactor the common code into one dedicated file,
while preparing to add new Meson-AXG aoclk driver, this would
help us to better share the code by all aoclk drivers.
Suggested-by: Jerome Brunet <jbrunet@baylibre.com>
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
---
drivers/clk/meson/Makefile | 2 +-
drivers/clk/meson/gxbb-aoclk.c | 91 ++++++++++++++---------------------------
drivers/clk/meson/gxbb-aoclk.h | 7 ++++
drivers/clk/meson/meson-aoclk.c | 83 +++++++++++++++++++++++++++++++++++++
drivers/clk/meson/meson-aoclk.h | 35 ++++++++++++++++
5 files changed, 157 insertions(+), 61 deletions(-)
create mode 100644 drivers/clk/meson/meson-aoclk.c
create mode 100644 drivers/clk/meson/meson-aoclk.h
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index ffee82e60b7a..555ab9c6ab64 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -4,6 +4,6 @@
obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
-obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
+obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o meson-aoclk.o gxbb-aoclk.o gxbb-aoclk-32k.o
obj-$(CONFIG_COMMON_CLK_AXG) += axg.o
obj-$(CONFIG_COMMON_CLK_REGMAP_MESON) += clk-regmap.o
diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 9ec23ae9a219..59db8e92f8cf 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -52,39 +52,13 @@
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-#include <linux/clk-provider.h>
-#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/reset-controller.h>
#include <linux/mfd/syscon.h>
-#include <linux/regmap.h>
#include <linux/init.h>
-#include <linux/delay.h>
-#include <dt-bindings/clock/gxbb-aoclkc.h>
-#include <dt-bindings/reset/gxbb-aoclkc.h>
#include "clk-regmap.h"
#include "gxbb-aoclk.h"
-struct gxbb_aoclk_reset_controller {
- struct reset_controller_dev reset;
- unsigned int *data;
- struct regmap *regmap;
-};
-
-static int gxbb_aoclk_do_reset(struct reset_controller_dev *rcdev,
- unsigned long id)
-{
- struct gxbb_aoclk_reset_controller *reset =
- container_of(rcdev, struct gxbb_aoclk_reset_controller, reset);
-
- return regmap_write(reset->regmap, AO_RTI_GEN_CNTL_REG0,
- BIT(reset->data[id]));
-}
-
-static const struct reset_control_ops gxbb_aoclk_reset_ops = {
- .reset = gxbb_aoclk_do_reset,
-};
-
#define GXBB_AO_GATE(_name, _bit) \
static struct clk_regmap _name##_ao = { \
.data = &(struct clk_regmap_gate_data) { \
@@ -117,7 +91,7 @@ static struct aoclk_cec_32k cec_32k_ao = {
},
};
-static unsigned int gxbb_aoclk_reset[] = {
+static const unsigned int gxbb_aoclk_reset[] = {
[RESET_AO_REMOTE] = 16,
[RESET_AO_I2C_MASTER] = 18,
[RESET_AO_I2C_SLAVE] = 19,
@@ -135,7 +109,7 @@ static struct clk_regmap *gxbb_aoclk_gate[] = {
[CLKID_AO_IR_BLASTER] = &ir_blaster_ao,
};
-static struct clk_hw_onecell_data gxbb_aoclk_onecell_data = {
+static const struct clk_hw_onecell_data gxbb_aoclk_onecell_data = {
.hws = {
[CLKID_AO_REMOTE] = &remote_ao.hw,
[CLKID_AO_I2C_MASTER] = &i2c_master_ao.hw,
@@ -145,19 +119,15 @@ static struct clk_hw_onecell_data gxbb_aoclk_onecell_data = {
[CLKID_AO_IR_BLASTER] = &ir_blaster_ao.hw,
[CLKID_AO_CEC_32K] = &cec_32k_ao.hw,
},
- .num = 7,
+ .num = NR_CLKS,
};
-static int gxbb_aoclkc_probe(struct platform_device *pdev)
+static int gxbb_aoclkc_register_specific_clk(
+ struct platform_device *pdev)
{
- struct gxbb_aoclk_reset_controller *rstc;
struct device *dev = &pdev->dev;
struct regmap *regmap;
- int ret, clkid;
-
- rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
- if (!rstc)
- return -ENOMEM;
+ int ret;
regmap = syscon_node_to_regmap(of_get_parent(dev->of_node));
if (IS_ERR(regmap)) {
@@ -165,38 +135,39 @@ static int gxbb_aoclkc_probe(struct platform_device *pdev)
return -ENODEV;
}
- /* Reset Controller */
- rstc->regmap = regmap;
- rstc->data = gxbb_aoclk_reset;
- rstc->reset.ops = &gxbb_aoclk_reset_ops;
- rstc->reset.nr_resets = ARRAY_SIZE(gxbb_aoclk_reset);
- rstc->reset.of_node = dev->of_node;
- ret = devm_reset_controller_register(dev, &rstc->reset);
-
- /*
- * Populate regmap and register all clks
- */
- for (clkid = 0; clkid < ARRAY_SIZE(gxbb_aoclk_gate); clkid++) {
- gxbb_aoclk_gate[clkid]->map = regmap;
-
- ret = devm_clk_hw_register(dev,
- gxbb_aoclk_onecell_data.hws[clkid]);
- if (ret)
- return ret;
- }
-
/* Specific clocks */
cec_32k_ao.regmap = regmap;
ret = devm_clk_hw_register(dev, &cec_32k_ao.hw);
- if (ret)
+ if (ret) {
+ dev_err(&pdev->dev, "clk cec_32k_ao register failed.\n");
return ret;
+ }
+
+ return 0;
+}
+
+static const struct meson_aoclk_data gxbb_aoclkc_data = {
+ .reset_reg = AO_RTI_GEN_CNTL_REG0,
+ .num_reset = ARRAY_SIZE(gxbb_aoclk_reset),
+ .reset = gxbb_aoclk_reset,
+ .num_clks = ARRAY_SIZE(gxbb_aoclk_gate),
+ .clks = gxbb_aoclk_gate,
+ .hw_data = &gxbb_aoclk_onecell_data,
+};
+
+static int gxbb_aoclkc_probe(struct platform_device *pdev)
+{
+ if (gxbb_aoclkc_register_specific_clk(pdev))
+ return -ENODEV;
- return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
- &gxbb_aoclk_onecell_data);
+ return meson_aoclkc_probe(pdev);
}
static const struct of_device_id gxbb_aoclkc_match_table[] = {
- { .compatible = "amlogic,meson-gx-aoclkc" },
+ {
+ .compatible = "amlogic,meson-gx-aoclkc",
+ .data = &gxbb_aoclkc_data,
+ },
{ }
};
diff --git a/drivers/clk/meson/gxbb-aoclk.h b/drivers/clk/meson/gxbb-aoclk.h
index badc4c22b4ee..b031f1a0213e 100644
--- a/drivers/clk/meson/gxbb-aoclk.h
+++ b/drivers/clk/meson/gxbb-aoclk.h
@@ -8,6 +8,10 @@
#ifndef __GXBB_AOCLKC_H
#define __GXBB_AOCLKC_H
+#include "meson-aoclk.h"
+
+#define NR_CLKS 7
+
/* AO Configuration Clock registers offsets */
#define AO_RTI_PWR_CNTL_REG1 0x0c
#define AO_RTI_PWR_CNTL_REG0 0x10
@@ -26,4 +30,7 @@ struct aoclk_cec_32k {
extern const struct clk_ops meson_aoclk_cec_32k_ops;
+#include <dt-bindings/clock/gxbb-aoclkc.h>
+#include <dt-bindings/reset/gxbb-aoclkc.h>
+
#endif /* __GXBB_AOCLKC_H */
diff --git a/drivers/clk/meson/meson-aoclk.c b/drivers/clk/meson/meson-aoclk.c
new file mode 100644
index 000000000000..14862c22e20d
--- /dev/null
+++ b/drivers/clk/meson/meson-aoclk.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Amlogic Meson-AXG Clock Controller Driver
+ *
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <qiufang.dai@amlogic.com>
+ * Author: Yixun Lan <yixun.lan@amlogic.com>
+ */
+
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/mfd/syscon.h>
+#include <linux/init.h>
+#include <linux/of_device.h>
+#include "clk-regmap.h"
+#include "meson-aoclk.h"
+
+static int meson_aoclk_do_reset(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct meson_aoclk_reset_controller *rstc =
+ container_of(rcdev, struct meson_aoclk_reset_controller, reset);
+
+ return regmap_write(rstc->regmap, rstc->data->reset_reg,
+ BIT(rstc->data->reset[id]));
+}
+
+static const struct reset_control_ops meson_aoclk_reset_ops = {
+ .reset = meson_aoclk_do_reset,
+};
+
+int meson_aoclkc_probe(struct platform_device *pdev)
+{
+ struct meson_aoclk_reset_controller *rstc;
+ struct meson_aoclk_data *data;
+ struct device *dev = &pdev->dev;
+ struct regmap *regmap;
+ int ret, clkid;
+
+ data = (struct meson_aoclk_data *)
+ of_device_get_match_data(dev);
+ if (!data)
+ return -ENODEV;
+
+ rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
+ if (!rstc)
+ return -ENOMEM;
+
+ regmap = syscon_node_to_regmap(of_get_parent(dev->of_node));
+ if (IS_ERR(regmap)) {
+ dev_err(dev, "failed to get regmap\n");
+ return -ENODEV;
+ }
+
+ /* Reset Controller */
+ rstc->data = data;
+ rstc->regmap = regmap;
+ rstc->reset.ops = &meson_aoclk_reset_ops;
+ rstc->reset.nr_resets = data->num_reset,
+ rstc->reset.of_node = dev->of_node;
+ ret = devm_reset_controller_register(dev, &rstc->reset);
+ if (ret) {
+ dev_err(dev, "failed to register reset controller\n");
+ return ret;
+ }
+
+ /*
+ * Populate regmap and register all clks
+ */
+ for (clkid = 0; clkid < data->num_clks; clkid++) {
+ data->clks[clkid]->map = regmap;
+
+ ret = devm_clk_hw_register(dev, data->hw_data->hws[clkid]);
+ if (ret)
+ return ret;
+ }
+
+ return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
+ (void *) data->hw_data);
+}
diff --git a/drivers/clk/meson/meson-aoclk.h b/drivers/clk/meson/meson-aoclk.h
new file mode 100644
index 000000000000..f0fba19fff0a
--- /dev/null
+++ b/drivers/clk/meson/meson-aoclk.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2017 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <qiufang.dai@amlogic.com>
+ * Author: Yixun Lan <yixun.lan@amlogic.com>
+ */
+
+#ifndef __MESON_AOCLK_H__
+#define __MESON_AOCLK_H__
+
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include "clk-regmap.h"
+
+struct meson_aoclk_data {
+ const unsigned int reset_reg;
+ const int num_reset;
+ const unsigned int *reset;
+ int num_clks;
+ struct clk_regmap **clks;
+ const struct clk_hw_onecell_data *hw_data;
+};
+
+struct meson_aoclk_reset_controller {
+ struct reset_controller_dev reset;
+ const struct meson_aoclk_data *data;
+ struct regmap *regmap;
+};
+
+int meson_aoclkc_probe(struct platform_device *pdev);
+#endif
+
--
2.15.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 1/7] clk: meson: aoclk: refactor common code into dedicated file
2018-04-08 3:19 ` [PATCH v4 1/7] clk: meson: aoclk: refactor common code into dedicated file Yixun Lan
@ 2018-04-09 12:08 ` Jerome Brunet
2018-04-09 14:29 ` Yixun Lan
0 siblings, 1 reply; 12+ messages in thread
From: Jerome Brunet @ 2018-04-09 12:08 UTC (permalink / raw)
To: Yixun Lan, Neil Armstrong, Kevin Hilman, Carlo Caione
Cc: Rob Herring, Michael Turquette, Stephen Boyd, Philipp Zabel,
Qiufang Dai, linux-amlogic, linux-clk, linux-arm-kernel,
linux-kernel
On Sun, 2018-04-08 at 11:19 +0800, Yixun Lan wrote:
> We try to refactor the common code into one dedicated file,
> while preparing to add new Meson-AXG aoclk driver, this would
> help us to better share the code by all aoclk drivers.
>
> Suggested-by: Jerome Brunet <jbrunet@baylibre.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> ---
> drivers/clk/meson/Makefile | 2 +-
> drivers/clk/meson/gxbb-aoclk.c | 91 ++++++++++++++---------------------------
> drivers/clk/meson/gxbb-aoclk.h | 7 ++++
> drivers/clk/meson/meson-aoclk.c | 83 +++++++++++++++++++++++++++++++++++++
> drivers/clk/meson/meson-aoclk.h | 35 ++++++++++++++++
> 5 files changed, 157 insertions(+), 61 deletions(-)
> create mode 100644 drivers/clk/meson/meson-aoclk.c
> create mode 100644 drivers/clk/meson/meson-aoclk.h
>
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index ffee82e60b7a..555ab9c6ab64 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -4,6 +4,6 @@
>
> obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
> -obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
> +obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o meson-aoclk.o gxbb-aoclk.o gxbb-aoclk-32k.o
^
This is not going to end well ========================|
> obj-$(CONFIG_COMMON_CLK_AXG) += axg.o
> obj-$(CONFIG_COMMON_CLK_REGMAP_MESON) += clk-regmap.o
> diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
> index 9ec23ae9a219..59db8e92f8cf 100644
> --- a/drivers/clk/meson/gxbb-aoclk.c
> +++ b/drivers/clk/meson/gxbb-aoclk.c
> @@ -52,39 +52,13 @@
> * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
> -#include <linux/clk-provider.h>
> -#include <linux/of_address.h>
> #include <linux/platform_device.h>
> #include <linux/reset-controller.h>
> #include <linux/mfd/syscon.h>
> -#include <linux/regmap.h>
> #include <linux/init.h>
> -#include <linux/delay.h>
> -#include <dt-bindings/clock/gxbb-aoclkc.h>
> -#include <dt-bindings/reset/gxbb-aoclkc.h>
> #include "clk-regmap.h"
> #include "gxbb-aoclk.h"
>
> -struct gxbb_aoclk_reset_controller {
> - struct reset_controller_dev reset;
> - unsigned int *data;
> - struct regmap *regmap;
> -};
> -
> -static int gxbb_aoclk_do_reset(struct reset_controller_dev *rcdev,
> - unsigned long id)
> -{
> - struct gxbb_aoclk_reset_controller *reset =
> - container_of(rcdev, struct gxbb_aoclk_reset_controller, reset);
> -
> - return regmap_write(reset->regmap, AO_RTI_GEN_CNTL_REG0,
> - BIT(reset->data[id]));
> -}
> -
> -static const struct reset_control_ops gxbb_aoclk_reset_ops = {
> - .reset = gxbb_aoclk_do_reset,
> -};
> -
> #define GXBB_AO_GATE(_name, _bit) \
> static struct clk_regmap _name##_ao = { \
> .data = &(struct clk_regmap_gate_data) { \
> @@ -117,7 +91,7 @@ static struct aoclk_cec_32k cec_32k_ao = {
> },
> };
>
> -static unsigned int gxbb_aoclk_reset[] = {
> +static const unsigned int gxbb_aoclk_reset[] = {
> [RESET_AO_REMOTE] = 16,
> [RESET_AO_I2C_MASTER] = 18,
> [RESET_AO_I2C_SLAVE] = 19,
> @@ -135,7 +109,7 @@ static struct clk_regmap *gxbb_aoclk_gate[] = {
> [CLKID_AO_IR_BLASTER] = &ir_blaster_ao,
> };
>
> -static struct clk_hw_onecell_data gxbb_aoclk_onecell_data = {
> +static const struct clk_hw_onecell_data gxbb_aoclk_onecell_data = {
> .hws = {
> [CLKID_AO_REMOTE] = &remote_ao.hw,
> [CLKID_AO_I2C_MASTER] = &i2c_master_ao.hw,
> @@ -145,19 +119,15 @@ static struct clk_hw_onecell_data gxbb_aoclk_onecell_data = {
> [CLKID_AO_IR_BLASTER] = &ir_blaster_ao.hw,
> [CLKID_AO_CEC_32K] = &cec_32k_ao.hw,
> },
> - .num = 7,
> + .num = NR_CLKS,
> };
>
> -static int gxbb_aoclkc_probe(struct platform_device *pdev)
> +static int gxbb_aoclkc_register_specific_clk(
Please call this something more explicit, gxbb_register_cec_ao_32k for example
> + struct platform_device *pdev)
Why do you put this on a new line ? it was not over 80 char ...
> {
> - struct gxbb_aoclk_reset_controller *rstc;
> struct device *dev = &pdev->dev;
> struct regmap *regmap;
> - int ret, clkid;
> -
> - rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
> - if (!rstc)
> - return -ENOMEM;
> + int ret;
>
> regmap = syscon_node_to_regmap(of_get_parent(dev->of_node));
> if (IS_ERR(regmap)) {
> @@ -165,38 +135,39 @@ static int gxbb_aoclkc_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> - /* Reset Controller */
> - rstc->regmap = regmap;
> - rstc->data = gxbb_aoclk_reset;
> - rstc->reset.ops = &gxbb_aoclk_reset_ops;
> - rstc->reset.nr_resets = ARRAY_SIZE(gxbb_aoclk_reset);
> - rstc->reset.of_node = dev->of_node;
> - ret = devm_reset_controller_register(dev, &rstc->reset);
> -
> - /*
> - * Populate regmap and register all clks
> - */
> - for (clkid = 0; clkid < ARRAY_SIZE(gxbb_aoclk_gate); clkid++) {
> - gxbb_aoclk_gate[clkid]->map = regmap;
> -
> - ret = devm_clk_hw_register(dev,
> - gxbb_aoclk_onecell_data.hws[clkid]);
> - if (ret)
> - return ret;
> - }
> -
> /* Specific clocks */
> cec_32k_ao.regmap = regmap;
> ret = devm_clk_hw_register(dev, &cec_32k_ao.hw);
> - if (ret)
> + if (ret) {
> + dev_err(&pdev->dev, "clk cec_32k_ao register failed.\n");
> return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct meson_aoclk_data gxbb_aoclkc_data = {
> + .reset_reg = AO_RTI_GEN_CNTL_REG0,
> + .num_reset = ARRAY_SIZE(gxbb_aoclk_reset),
> + .reset = gxbb_aoclk_reset,
> + .num_clks = ARRAY_SIZE(gxbb_aoclk_gate),
> + .clks = gxbb_aoclk_gate,
> + .hw_data = &gxbb_aoclk_onecell_data,
> +};
> +
> +static int gxbb_aoclkc_probe(struct platform_device *pdev)
> +{
> + if (gxbb_aoclkc_register_specific_clk(pdev))
> + return -ENODEV;
It is not OK to just put your error code like that. If a function, such as
devm_clk_hw_register() return an error, you should fwd that error code.
>
> - return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
> - &gxbb_aoclk_onecell_data);
> + return meson_aoclkc_probe(pdev);
> }
>
> static const struct of_device_id gxbb_aoclkc_match_table[] = {
> - { .compatible = "amlogic,meson-gx-aoclkc" },
> + {
> + .compatible = "amlogic,meson-gx-aoclkc",
> + .data = &gxbb_aoclkc_data,
> + },
> { }
> };
>
> diff --git a/drivers/clk/meson/gxbb-aoclk.h b/drivers/clk/meson/gxbb-aoclk.h
> index badc4c22b4ee..b031f1a0213e 100644
> --- a/drivers/clk/meson/gxbb-aoclk.h
> +++ b/drivers/clk/meson/gxbb-aoclk.h
> @@ -8,6 +8,10 @@
> #ifndef __GXBB_AOCLKC_H
> #define __GXBB_AOCLKC_H
>
> +#include "meson-aoclk.h"
> +
> +#define NR_CLKS 7
> +
> /* AO Configuration Clock registers offsets */
> #define AO_RTI_PWR_CNTL_REG1 0x0c
> #define AO_RTI_PWR_CNTL_REG0 0x10
> @@ -26,4 +30,7 @@ struct aoclk_cec_32k {
>
> extern const struct clk_ops meson_aoclk_cec_32k_ops;
>
> +#include <dt-bindings/clock/gxbb-aoclkc.h>
> +#include <dt-bindings/reset/gxbb-aoclkc.h>
> +
> #endif /* __GXBB_AOCLKC_H */
> diff --git a/drivers/clk/meson/meson-aoclk.c b/drivers/clk/meson/meson-aoclk.c
> new file mode 100644
> index 000000000000..14862c22e20d
> --- /dev/null
> +++ b/drivers/clk/meson/meson-aoclk.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Amlogic Meson-AXG Clock Controller Driver
> + *
> + * Copyright (c) 2016 BayLibre, SAS.
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
> + * Author: Yixun Lan <yixun.lan@amlogic.com>
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/init.h>
> +#include <linux/of_device.h>
> +#include "clk-regmap.h"
> +#include "meson-aoclk.h"
> +
> +static int meson_aoclk_do_reset(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct meson_aoclk_reset_controller *rstc =
> + container_of(rcdev, struct meson_aoclk_reset_controller, reset);
> +
> + return regmap_write(rstc->regmap, rstc->data->reset_reg,
> + BIT(rstc->data->reset[id]));
> +}
> +
> +static const struct reset_control_ops meson_aoclk_reset_ops = {
> + .reset = meson_aoclk_do_reset,
> +};
> +
> +int meson_aoclkc_probe(struct platform_device *pdev)
> +{
> + struct meson_aoclk_reset_controller *rstc;
> + struct meson_aoclk_data *data;
> + struct device *dev = &pdev->dev;
> + struct regmap *regmap;
> + int ret, clkid;
> +
> + data = (struct meson_aoclk_data *)
> + of_device_get_match_data(dev);
Again, line would not be over 80 char ...
> + if (!data)
> + return -ENODEV;
> +
> + rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
> + if (!rstc)
> + return -ENOMEM;
> +
> + regmap = syscon_node_to_regmap(of_get_parent(dev->of_node));
> + if (IS_ERR(regmap)) {
> + dev_err(dev, "failed to get regmap\n");
> + return -ENODEV;
Same as above, you can't decide to return error code like this ... use PTR_ERR.
> + }
> +
> + /* Reset Controller */
> + rstc->data = data;
> + rstc->regmap = regmap;
> + rstc->reset.ops = &meson_aoclk_reset_ops;
> + rstc->reset.nr_resets = data->num_reset,
> + rstc->reset.of_node = dev->of_node;
> + ret = devm_reset_controller_register(dev, &rstc->reset);
> + if (ret) {
> + dev_err(dev, "failed to register reset controller\n");
> + return ret;
> + }
> +
> + /*
> + * Populate regmap and register all clks
> + */
> + for (clkid = 0; clkid < data->num_clks; clkid++) {
> + data->clks[clkid]->map = regmap;
> +
> + ret = devm_clk_hw_register(dev, data->hw_data->hws[clkid]);
> + if (ret)
> + return ret;
> + }
> +
> + return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
> + (void *) data->hw_data);
> +}
> diff --git a/drivers/clk/meson/meson-aoclk.h b/drivers/clk/meson/meson-aoclk.h
> new file mode 100644
> index 000000000000..f0fba19fff0a
> --- /dev/null
> +++ b/drivers/clk/meson/meson-aoclk.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Copyright (c) 2017 BayLibre, SAS
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
> + * Author: Yixun Lan <yixun.lan@amlogic.com>
> + */
> +
> +#ifndef __MESON_AOCLK_H__
> +#define __MESON_AOCLK_H__
> +
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include "clk-regmap.h"
> +
> +struct meson_aoclk_data {
> + const unsigned int reset_reg;
> + const int num_reset;
> + const unsigned int *reset;
> + int num_clks;
> + struct clk_regmap **clks;
> + const struct clk_hw_onecell_data *hw_data;
If you align structure member names, please do it for all them.
> +};
> +
> +struct meson_aoclk_reset_controller {
> + struct reset_controller_dev reset;
> + const struct meson_aoclk_data *data;
> + struct regmap *regmap;
> +};
> +
> +int meson_aoclkc_probe(struct platform_device *pdev);
> +#endif
> +
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 1/7] clk: meson: aoclk: refactor common code into dedicated file
2018-04-09 12:08 ` Jerome Brunet
@ 2018-04-09 14:29 ` Yixun Lan
0 siblings, 0 replies; 12+ messages in thread
From: Yixun Lan @ 2018-04-09 14:29 UTC (permalink / raw)
To: Jerome Brunet, Neil Armstrong, Kevin Hilman, Carlo Caione
Cc: yixun.lan, Rob Herring, Michael Turquette, Stephen Boyd,
Philipp Zabel, Qiufang Dai, linux-amlogic, linux-clk,
linux-arm-kernel, linux-kernel
Hi Jerome
thanks for the review, I will fix them in next version
On 04/09/2018 08:08 PM, Jerome Brunet wrote:
> On Sun, 2018-04-08 at 11:19 +0800, Yixun Lan wrote:
>> We try to refactor the common code into one dedicated file,
>> while preparing to add new Meson-AXG aoclk driver, this would
>> help us to better share the code by all aoclk drivers.
>>
>> Suggested-by: Jerome Brunet <jbrunet@baylibre.com>
>> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
>> ---
>> drivers/clk/meson/Makefile | 2 +-
>> drivers/clk/meson/gxbb-aoclk.c | 91 ++++++++++++++---------------------------
>> drivers/clk/meson/gxbb-aoclk.h | 7 ++++
>> drivers/clk/meson/meson-aoclk.c | 83 +++++++++++++++++++++++++++++++++++++
>> drivers/clk/meson/meson-aoclk.h | 35 ++++++++++++++++
>> 5 files changed, 157 insertions(+), 61 deletions(-)
>> create mode 100644 drivers/clk/meson/meson-aoclk.c
>> create mode 100644 drivers/clk/meson/meson-aoclk.h
>>
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index ffee82e60b7a..555ab9c6ab64 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -4,6 +4,6 @@
>>
>> obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
>> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
>> -obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
>> +obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o meson-aoclk.o gxbb-aoclk.o gxbb-aoclk-32k.o
> ^
> This is not going to end well ========================|
>
will address this
>
>> obj-$(CONFIG_COMMON_CLK_AXG) += axg.o
>> obj-$(CONFIG_COMMON_CLK_REGMAP_MESON) += clk-regmap.o
>> diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
>> index 9ec23ae9a219..59db8e92f8cf 100644
>> --- a/drivers/clk/meson/gxbb-aoclk.c
>> +++ b/drivers/clk/meson/gxbb-aoclk.c
>> @@ -52,39 +52,13 @@
>> * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>> * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> */
>> -#include <linux/clk-provider.h>
>> -#include <linux/of_address.h>
>> #include <linux/platform_device.h>
>> #include <linux/reset-controller.h>
>> #include <linux/mfd/syscon.h>
>> -#include <linux/regmap.h>
>> #include <linux/init.h>
>> -#include <linux/delay.h>
>> -#include <dt-bindings/clock/gxbb-aoclkc.h>
>> -#include <dt-bindings/reset/gxbb-aoclkc.h>
>> #include "clk-regmap.h"
>> #include "gxbb-aoclk.h"
>>
>> -struct gxbb_aoclk_reset_controller {
>> - struct reset_controller_dev reset;
>> - unsigned int *data;
>> - struct regmap *regmap;
>> -};
>> -
>> -static int gxbb_aoclk_do_reset(struct reset_controller_dev *rcdev,
>> - unsigned long id)
>> -{
>> - struct gxbb_aoclk_reset_controller *reset =
>> - container_of(rcdev, struct gxbb_aoclk_reset_controller, reset);
>> -
>> - return regmap_write(reset->regmap, AO_RTI_GEN_CNTL_REG0,
>> - BIT(reset->data[id]));
>> -}
>> -
>> -static const struct reset_control_ops gxbb_aoclk_reset_ops = {
>> - .reset = gxbb_aoclk_do_reset,
>> -};
>> -
>> #define GXBB_AO_GATE(_name, _bit) \
>> static struct clk_regmap _name##_ao = { \
>> .data = &(struct clk_regmap_gate_data) { \
>> @@ -117,7 +91,7 @@ static struct aoclk_cec_32k cec_32k_ao = {
>> },
>> };
>>
>> -static unsigned int gxbb_aoclk_reset[] = {
>> +static const unsigned int gxbb_aoclk_reset[] = {
>> [RESET_AO_REMOTE] = 16,
>> [RESET_AO_I2C_MASTER] = 18,
>> [RESET_AO_I2C_SLAVE] = 19,
>> @@ -135,7 +109,7 @@ static struct clk_regmap *gxbb_aoclk_gate[] = {
>> [CLKID_AO_IR_BLASTER] = &ir_blaster_ao,
>> };
>>
>> -static struct clk_hw_onecell_data gxbb_aoclk_onecell_data = {
>> +static const struct clk_hw_onecell_data gxbb_aoclk_onecell_data = {
>> .hws = {
>> [CLKID_AO_REMOTE] = &remote_ao.hw,
>> [CLKID_AO_I2C_MASTER] = &i2c_master_ao.hw,
>> @@ -145,19 +119,15 @@ static struct clk_hw_onecell_data gxbb_aoclk_onecell_data = {
>> [CLKID_AO_IR_BLASTER] = &ir_blaster_ao.hw,
>> [CLKID_AO_CEC_32K] = &cec_32k_ao.hw,
>> },
>> - .num = 7,
>> + .num = NR_CLKS,
>> };
>>
>> -static int gxbb_aoclkc_probe(struct platform_device *pdev)
>> +static int gxbb_aoclkc_register_specific_clk(
>
> Please call this something more explicit, gxbb_register_cec_ao_32k for example
>
ok
>> + struct platform_device *pdev)
>
> Why do you put this on a new line ? it was not over 80 char ...
>
>> {
>> - struct gxbb_aoclk_reset_controller *rstc;
>> struct device *dev = &pdev->dev;
>> struct regmap *regmap;
>> - int ret, clkid;
>> -
>> - rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
>> - if (!rstc)
>> - return -ENOMEM;
>> + int ret;
>>
>> regmap = syscon_node_to_regmap(of_get_parent(dev->of_node));
>> if (IS_ERR(regmap)) {
>> @@ -165,38 +135,39 @@ static int gxbb_aoclkc_probe(struct platform_device *pdev)
>> return -ENODEV;
>> }
>>
>> - /* Reset Controller */
>> - rstc->regmap = regmap;
>> - rstc->data = gxbb_aoclk_reset;
>> - rstc->reset.ops = &gxbb_aoclk_reset_ops;
>> - rstc->reset.nr_resets = ARRAY_SIZE(gxbb_aoclk_reset);
>> - rstc->reset.of_node = dev->of_node;
>> - ret = devm_reset_controller_register(dev, &rstc->reset);
>> -
>> - /*
>> - * Populate regmap and register all clks
>> - */
>> - for (clkid = 0; clkid < ARRAY_SIZE(gxbb_aoclk_gate); clkid++) {
>> - gxbb_aoclk_gate[clkid]->map = regmap;
>> -
>> - ret = devm_clk_hw_register(dev,
>> - gxbb_aoclk_onecell_data.hws[clkid]);
>> - if (ret)
>> - return ret;
>> - }
>> -
>> /* Specific clocks */
>> cec_32k_ao.regmap = regmap;
>> ret = devm_clk_hw_register(dev, &cec_32k_ao.hw);
>> - if (ret)
>> + if (ret) {
>> + dev_err(&pdev->dev, "clk cec_32k_ao register failed.\n");
>> return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct meson_aoclk_data gxbb_aoclkc_data = {
>> + .reset_reg = AO_RTI_GEN_CNTL_REG0,
>> + .num_reset = ARRAY_SIZE(gxbb_aoclk_reset),
>> + .reset = gxbb_aoclk_reset,
>> + .num_clks = ARRAY_SIZE(gxbb_aoclk_gate),
>> + .clks = gxbb_aoclk_gate,
>> + .hw_data = &gxbb_aoclk_onecell_data,
>> +};
>> +
>> +static int gxbb_aoclkc_probe(struct platform_device *pdev)
>> +{
>> + if (gxbb_aoclkc_register_specific_clk(pdev))
>> + return -ENODEV;
>
> It is not OK to just put your error code like that. If a function, such as
> devm_clk_hw_register() return an error, you should fwd that error code.
>
ok, will do
>>
>> - return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
>> - &gxbb_aoclk_onecell_data);
>> + return meson_aoclkc_probe(pdev);
>> }
>>
>> static const struct of_device_id gxbb_aoclkc_match_table[] = {
>> - { .compatible = "amlogic,meson-gx-aoclkc" },
>> + {
>> + .compatible = "amlogic,meson-gx-aoclkc",
>> + .data = &gxbb_aoclkc_data,
>> + },
>> { }
>> };
>>
>> diff --git a/drivers/clk/meson/gxbb-aoclk.h b/drivers/clk/meson/gxbb-aoclk.h
>> index badc4c22b4ee..b031f1a0213e 100644
>> --- a/drivers/clk/meson/gxbb-aoclk.h
>> +++ b/drivers/clk/meson/gxbb-aoclk.h
>> @@ -8,6 +8,10 @@
>> #ifndef __GXBB_AOCLKC_H
>> #define __GXBB_AOCLKC_H
>>
>> +#include "meson-aoclk.h"
>> +
>> +#define NR_CLKS 7
>> +
>> /* AO Configuration Clock registers offsets */
>> #define AO_RTI_PWR_CNTL_REG1 0x0c
>> #define AO_RTI_PWR_CNTL_REG0 0x10
>> @@ -26,4 +30,7 @@ struct aoclk_cec_32k {
>>
>> extern const struct clk_ops meson_aoclk_cec_32k_ops;
>>
>> +#include <dt-bindings/clock/gxbb-aoclkc.h>
>> +#include <dt-bindings/reset/gxbb-aoclkc.h>
>> +
>> #endif /* __GXBB_AOCLKC_H */
>> diff --git a/drivers/clk/meson/meson-aoclk.c b/drivers/clk/meson/meson-aoclk.c
>> new file mode 100644
>> index 000000000000..14862c22e20d
>> --- /dev/null
>> +++ b/drivers/clk/meson/meson-aoclk.c
>> @@ -0,0 +1,83 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Amlogic Meson-AXG Clock Controller Driver
>> + *
>> + * Copyright (c) 2016 BayLibre, SAS.
>> + * Author: Neil Armstrong <narmstrong@baylibre.com>
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
>> + * Author: Yixun Lan <yixun.lan@amlogic.com>
>> + */
>> +
>> +#include <linux/platform_device.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/init.h>
>> +#include <linux/of_device.h>
>> +#include "clk-regmap.h"
>> +#include "meson-aoclk.h"
>> +
>> +static int meson_aoclk_do_reset(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + struct meson_aoclk_reset_controller *rstc =
>> + container_of(rcdev, struct meson_aoclk_reset_controller, reset);
>> +
>> + return regmap_write(rstc->regmap, rstc->data->reset_reg,
>> + BIT(rstc->data->reset[id]));
>> +}
>> +
>> +static const struct reset_control_ops meson_aoclk_reset_ops = {
>> + .reset = meson_aoclk_do_reset,
>> +};
>> +
>> +int meson_aoclkc_probe(struct platform_device *pdev)
>> +{
>> + struct meson_aoclk_reset_controller *rstc;
>> + struct meson_aoclk_data *data;
>> + struct device *dev = &pdev->dev;
>> + struct regmap *regmap;
>> + int ret, clkid;
>> +
>> + data = (struct meson_aoclk_data *)
>> + of_device_get_match_data(dev);
>
> Again, line would not be over 80 char ...
>
will adjust
>> + if (!data)
>> + return -ENODEV;
>> +
>> + rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
>> + if (!rstc)
>> + return -ENOMEM;
>> +
>> + regmap = syscon_node_to_regmap(of_get_parent(dev->of_node));
>> + if (IS_ERR(regmap)) {
>> + dev_err(dev, "failed to get regmap\n");
>> + return -ENODEV;
>
> Same as above, you can't decide to return error code like this ... use PTR_ERR.
>
ok
>> + }
>> +
>> + /* Reset Controller */
>> + rstc->data = data;
>> + rstc->regmap = regmap;
>> + rstc->reset.ops = &meson_aoclk_reset_ops;
>> + rstc->reset.nr_resets = data->num_reset,
>> + rstc->reset.of_node = dev->of_node;
>> + ret = devm_reset_controller_register(dev, &rstc->reset);
>> + if (ret) {
>> + dev_err(dev, "failed to register reset controller\n");
>> + return ret;
>> + }
>> +
>> + /*
>> + * Populate regmap and register all clks
>> + */
>> + for (clkid = 0; clkid < data->num_clks; clkid++) {
>> + data->clks[clkid]->map = regmap;
>> +
>> + ret = devm_clk_hw_register(dev, data->hw_data->hws[clkid]);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
>> + (void *) data->hw_data);
>> +}
>> diff --git a/drivers/clk/meson/meson-aoclk.h b/drivers/clk/meson/meson-aoclk.h
>> new file mode 100644
>> index 000000000000..f0fba19fff0a
>> --- /dev/null
>> +++ b/drivers/clk/meson/meson-aoclk.h
>> @@ -0,0 +1,35 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Copyright (c) 2017 BayLibre, SAS
>> + * Author: Neil Armstrong <narmstrong@baylibre.com>
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
>> + * Author: Yixun Lan <yixun.lan@amlogic.com>
>> + */
>> +
>> +#ifndef __MESON_AOCLK_H__
>> +#define __MESON_AOCLK_H__
>> +
>> +#include <linux/platform_device.h>
>> +#include <linux/reset-controller.h>
>> +#include "clk-regmap.h"
>> +
>> +struct meson_aoclk_data {
>> + const unsigned int reset_reg;
>> + const int num_reset;
>> + const unsigned int *reset;
>> + int num_clks;
>> + struct clk_regmap **clks;
>> + const struct clk_hw_onecell_data *hw_data;
>
> If you align structure member names, please do it for all them.
>
ok,
>> +};
>> +
>> +struct meson_aoclk_reset_controller {
>> + struct reset_controller_dev reset;
>> + const struct meson_aoclk_data *data;
>> + struct regmap *regmap;
>> +};
>> +
>> +int meson_aoclkc_probe(struct platform_device *pdev);
>> +#endif
>> +
>
> .
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 2/7] clk: meson: migrate to devm_of_clk_add_hw_provider API
2018-04-08 3:19 [PATCH v4 0/7] clk: meson-axg: Add AO Cloclk and Reset driver Yixun Lan
2018-04-08 3:19 ` [PATCH v4 1/7] clk: meson: aoclk: refactor common code into dedicated file Yixun Lan
@ 2018-04-08 3:19 ` Yixun Lan
2018-04-09 12:03 ` Jerome Brunet
2018-04-08 3:19 ` [PATCH v4 3/7] dt-bindings: clock: axg-aoclkc: New binding for Meson-AXG SoC Yixun Lan
` (4 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Yixun Lan @ 2018-04-08 3:19 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Carlo Caione
Cc: Yixun Lan, Rob Herring, Michael Turquette, Stephen Boyd,
Philipp Zabel, Qiufang Dai, linux-amlogic, linux-clk,
linux-arm-kernel, linux-kernel
There is a protential memory leak, as of_clk_del_provider is
never called if of_clk_add_hw_provider has been executed.
Fix this by using devm variant API.
Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
---
drivers/clk/meson/meson-aoclk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/meson/meson-aoclk.c b/drivers/clk/meson/meson-aoclk.c
index 14862c22e20d..14a1e46a4de6 100644
--- a/drivers/clk/meson/meson-aoclk.c
+++ b/drivers/clk/meson/meson-aoclk.c
@@ -78,6 +78,6 @@ int meson_aoclkc_probe(struct platform_device *pdev)
return ret;
}
- return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
+ return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
(void *) data->hw_data);
}
--
2.15.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 2/7] clk: meson: migrate to devm_of_clk_add_hw_provider API
2018-04-08 3:19 ` [PATCH v4 2/7] clk: meson: migrate to devm_of_clk_add_hw_provider API Yixun Lan
@ 2018-04-09 12:03 ` Jerome Brunet
0 siblings, 0 replies; 12+ messages in thread
From: Jerome Brunet @ 2018-04-09 12:03 UTC (permalink / raw)
To: Yixun Lan, Neil Armstrong, Kevin Hilman, Carlo Caione
Cc: Rob Herring, Michael Turquette, Stephen Boyd, Philipp Zabel,
Qiufang Dai, linux-amlogic, linux-clk, linux-arm-kernel,
linux-kernel
On Sun, 2018-04-08 at 11:19 +0800, Yixun Lan wrote:
> There is a protential memory leak, as of_clk_del_provider is
> never called if of_clk_add_hw_provider has been executed.
> Fix this by using devm variant API.
>
> Suggested-by: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
Please make this the 1st patch of the series and add the proper fixes tag.
> ---
> drivers/clk/meson/meson-aoclk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/meson/meson-aoclk.c b/drivers/clk/meson/meson-aoclk.c
> index 14862c22e20d..14a1e46a4de6 100644
> --- a/drivers/clk/meson/meson-aoclk.c
> +++ b/drivers/clk/meson/meson-aoclk.c
> @@ -78,6 +78,6 @@ int meson_aoclkc_probe(struct platform_device *pdev)
> return ret;
> }
>
> - return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
> + return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> (void *) data->hw_data);
> }
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 3/7] dt-bindings: clock: axg-aoclkc: New binding for Meson-AXG SoC
2018-04-08 3:19 [PATCH v4 0/7] clk: meson-axg: Add AO Cloclk and Reset driver Yixun Lan
2018-04-08 3:19 ` [PATCH v4 1/7] clk: meson: aoclk: refactor common code into dedicated file Yixun Lan
2018-04-08 3:19 ` [PATCH v4 2/7] clk: meson: migrate to devm_of_clk_add_hw_provider API Yixun Lan
@ 2018-04-08 3:19 ` Yixun Lan
2018-04-08 3:19 ` [PATCH v4 4/7] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings Yixun Lan
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Yixun Lan @ 2018-04-08 3:19 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Carlo Caione
Cc: Yixun Lan, Rob Herring, Michael Turquette, Stephen Boyd,
Philipp Zabel, Qiufang Dai, linux-amlogic, linux-clk,
linux-arm-kernel, linux-kernel, devicetree
Update the dt-binding documentation to support new compatible string
for the Amlogic's Meson-AXG SoC.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
---
Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt b/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt
index 786dc39ca904..3a880528030e 100644
--- a/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt
+++ b/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt
@@ -9,6 +9,7 @@ Required Properties:
- GXBB (S905) : "amlogic,meson-gxbb-aoclkc"
- GXL (S905X, S905D) : "amlogic,meson-gxl-aoclkc"
- GXM (S912) : "amlogic,meson-gxm-aoclkc"
+ - AXG (A113D, A113X) : "amlogic,meson-axg-aoclkc"
followed by the common "amlogic,meson-gx-aoclkc"
- #clock-cells: should be 1.
--
2.15.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 4/7] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
2018-04-08 3:19 [PATCH v4 0/7] clk: meson-axg: Add AO Cloclk and Reset driver Yixun Lan
` (2 preceding siblings ...)
2018-04-08 3:19 ` [PATCH v4 3/7] dt-bindings: clock: axg-aoclkc: New binding for Meson-AXG SoC Yixun Lan
@ 2018-04-08 3:19 ` Yixun Lan
2018-04-08 3:19 ` [PATCH v4 5/7] clk: meson-axg: Add AO Clock and Reset controller driver Yixun Lan
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Yixun Lan @ 2018-04-08 3:19 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Carlo Caione
Cc: Yixun Lan, Rob Herring, Michael Turquette, Stephen Boyd,
Philipp Zabel, Qiufang Dai, linux-amlogic, linux-clk,
linux-arm-kernel, linux-kernel, devicetree
Add dt-bindings headers for the Meson-AXG's AO clock and
reset controller.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
---
include/dt-bindings/clock/axg-aoclkc.h | 26 ++++++++++++++++++++++++++
include/dt-bindings/reset/axg-aoclkc.h | 20 ++++++++++++++++++++
2 files changed, 46 insertions(+)
create mode 100644 include/dt-bindings/clock/axg-aoclkc.h
create mode 100644 include/dt-bindings/reset/axg-aoclkc.h
diff --git a/include/dt-bindings/clock/axg-aoclkc.h b/include/dt-bindings/clock/axg-aoclkc.h
new file mode 100644
index 000000000000..61955016a55b
--- /dev/null
+++ b/include/dt-bindings/clock/axg-aoclkc.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/*
+ * Copyright (c) 2016 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <qiufang.dai@amlogic.com>
+ */
+
+#ifndef DT_BINDINGS_CLOCK_AMLOGIC_MESON_AXG_AOCLK
+#define DT_BINDINGS_CLOCK_AMLOGIC_MESON_AXG_AOCLK
+
+#define CLKID_AO_REMOTE 0
+#define CLKID_AO_I2C_MASTER 1
+#define CLKID_AO_I2C_SLAVE 2
+#define CLKID_AO_UART1 3
+#define CLKID_AO_UART2 4
+#define CLKID_AO_IR_BLASTER 5
+#define CLKID_AO_SAR_ADC 6
+#define CLKID_AO_CLK81 7
+#define CLKID_AO_SAR_ADC_SEL 8
+#define CLKID_AO_SAR_ADC_DIV 9
+#define CLKID_AO_SAR_ADC_CLK 10
+#define CLKID_AO_ALT_XTAL 11
+
+#endif
diff --git a/include/dt-bindings/reset/axg-aoclkc.h b/include/dt-bindings/reset/axg-aoclkc.h
new file mode 100644
index 000000000000..d342c0b6b2a7
--- /dev/null
+++ b/include/dt-bindings/reset/axg-aoclkc.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/*
+ * Copyright (c) 2016 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <qiufang.dai@amlogic.com>
+ */
+
+#ifndef DT_BINDINGS_RESET_AMLOGIC_MESON_AXG_AOCLK
+#define DT_BINDINGS_RESET_AMLOGIC_MESON_AXG_AOCLK
+
+#define RESET_AO_REMOTE 0
+#define RESET_AO_I2C_MASTER 1
+#define RESET_AO_I2C_SLAVE 2
+#define RESET_AO_UART1 3
+#define RESET_AO_UART2 4
+#define RESET_AO_IR_BLASTER 5
+
+#endif
--
2.15.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 5/7] clk: meson-axg: Add AO Clock and Reset controller driver
2018-04-08 3:19 [PATCH v4 0/7] clk: meson-axg: Add AO Cloclk and Reset driver Yixun Lan
` (3 preceding siblings ...)
2018-04-08 3:19 ` [PATCH v4 4/7] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings Yixun Lan
@ 2018-04-08 3:19 ` Yixun Lan
2018-04-09 12:08 ` Jerome Brunet
2018-04-08 3:19 ` [PATCH v4 6/7] clk: meson: drop CLK_SET_RATE_PARENT flag Yixun Lan
2018-04-08 3:19 ` [PATCH v4 7/7] clk: meson: drop CLK_IGNORE_UNUSED flag Yixun Lan
6 siblings, 1 reply; 12+ messages in thread
From: Yixun Lan @ 2018-04-08 3:19 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Carlo Caione
Cc: Qiufang Dai, Yixun Lan, Rob Herring, Michael Turquette,
Stephen Boyd, Philipp Zabel, linux-amlogic, linux-clk,
linux-arm-kernel, linux-kernel
From: Qiufang Dai <qiufang.dai@amlogic.com>
Adds a Clock and Reset controller driver for the Always-On part
of the Amlogic Meson-AXG SoC.
Signed-off-by: Qiufang Dai <qiufang.dai@amlogic.com>
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
---
drivers/clk/meson/Makefile | 2 +-
drivers/clk/meson/axg-aoclk.c | 164 ++++++++++++++++++++++++++++++++++++++++++
drivers/clk/meson/axg-aoclk.h | 31 ++++++++
3 files changed, 196 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/meson/axg-aoclk.c
create mode 100644 drivers/clk/meson/axg-aoclk.h
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 555ab9c6ab64..fa6d1e36cef6 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -5,5 +5,5 @@
obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o meson-aoclk.o gxbb-aoclk.o gxbb-aoclk-32k.o
-obj-$(CONFIG_COMMON_CLK_AXG) += axg.o
+obj-$(CONFIG_COMMON_CLK_AXG) += axg.o meson-aoclk.o axg-aoclk.o
obj-$(CONFIG_COMMON_CLK_REGMAP_MESON) += clk-regmap.o
diff --git a/drivers/clk/meson/axg-aoclk.c b/drivers/clk/meson/axg-aoclk.c
new file mode 100644
index 000000000000..cb56d809d3df
--- /dev/null
+++ b/drivers/clk/meson/axg-aoclk.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Amlogic Meson-AXG Clock Controller Driver
+ *
+ * Copyright (c) 2016 Baylibre SAS.
+ * Author: Michael Turquette <mturquette@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <qiufang.dai@amlogic.com>
+ */
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/mfd/syscon.h>
+#include <linux/init.h>
+#include "clkc.h"
+#include "axg-aoclk.h"
+
+#define AXG_AO_GATE(_name, _bit) \
+static struct clk_regmap _name##_ao = { \
+ .data = &(struct clk_regmap_gate_data) { \
+ .offset = (AO_RTI_GEN_CNTL_REG0), \
+ .bit_idx = (_bit), \
+ }, \
+ .hw.init = &(struct clk_init_data) { \
+ .name = #_name "_ao", \
+ .ops = &clk_regmap_gate_ops, \
+ .parent_names = (const char *[]){ "clk81" }, \
+ .num_parents = 1, \
+ .flags = CLK_IGNORE_UNUSED, \
+ }, \
+}
+
+AXG_AO_GATE(remote, 0);
+AXG_AO_GATE(i2c_master, 1);
+AXG_AO_GATE(i2c_slave, 2);
+AXG_AO_GATE(uart1, 3);
+AXG_AO_GATE(uart2, 5);
+AXG_AO_GATE(ir_blaster, 6);
+AXG_AO_GATE(saradc, 7);
+
+static struct clk_regmap ao_clk81 = {
+ .data = &(struct clk_regmap_mux_data) {
+ .offset = AO_RTI_PWR_CNTL_REG0,
+ .mask = 0x1,
+ .shift = 8,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "ao_clk81",
+ .ops = &clk_regmap_mux_ro_ops,
+ .parent_names = (const char *[]){ "clk81", "ao_alt_xtal"},
+ .num_parents = 2,
+ },
+};
+
+static struct clk_regmap axg_saradc_mux = {
+ .data = &(struct clk_regmap_mux_data) {
+ .offset = AO_SAR_CLK,
+ .mask = 0x3,
+ .shift = 9,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "axg_saradc_mux",
+ .ops = &clk_regmap_mux_ops,
+ .parent_names = (const char *[]){ "xtal", "ao_clk81" },
+ .num_parents = 2,
+ },
+};
+
+static struct clk_regmap axg_saradc_div = {
+ .data = &(struct clk_regmap_div_data) {
+ .offset = AO_SAR_CLK,
+ .shift = 0,
+ .width = 8,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "axg_saradc_div",
+ .ops = &clk_regmap_divider_ops,
+ .parent_names = (const char *[]){ "axg_saradc_mux" },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_regmap axg_saradc_gate = {
+ .data = &(struct clk_regmap_gate_data) {
+ .offset = AO_SAR_CLK,
+ .bit_idx = 8,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "axg_saradc_gate",
+ .ops = &clk_regmap_gate_ops,
+ .parent_names = (const char *[]){ "axg_saradc_div" },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static const unsigned int axg_aoclk_reset[] = {
+ [RESET_AO_REMOTE] = 16,
+ [RESET_AO_I2C_MASTER] = 18,
+ [RESET_AO_I2C_SLAVE] = 19,
+ [RESET_AO_UART1] = 17,
+ [RESET_AO_UART2] = 22,
+ [RESET_AO_IR_BLASTER] = 23,
+};
+
+static struct clk_regmap *axg_aoclk_regmap[] = {
+ [CLKID_AO_REMOTE] = &remote_ao,
+ [CLKID_AO_I2C_MASTER] = &i2c_master_ao,
+ [CLKID_AO_I2C_SLAVE] = &i2c_slave_ao,
+ [CLKID_AO_UART1] = &uart1_ao,
+ [CLKID_AO_UART2] = &uart2_ao,
+ [CLKID_AO_IR_BLASTER] = &ir_blaster_ao,
+ [CLKID_AO_SAR_ADC] = &saradc_ao,
+ [CLKID_AO_CLK81] = &ao_clk81,
+ [CLKID_AO_SAR_ADC_SEL] = &axg_saradc_mux,
+ [CLKID_AO_SAR_ADC_DIV] = &axg_saradc_div,
+ [CLKID_AO_SAR_ADC_CLK] = &axg_saradc_gate,
+};
+
+static const struct clk_hw_onecell_data axg_aoclk_onecell_data = {
+ .hws = {
+ [CLKID_AO_REMOTE] = &remote_ao.hw,
+ [CLKID_AO_I2C_MASTER] = &i2c_master_ao.hw,
+ [CLKID_AO_I2C_SLAVE] = &i2c_slave_ao.hw,
+ [CLKID_AO_UART1] = &uart1_ao.hw,
+ [CLKID_AO_UART2] = &uart2_ao.hw,
+ [CLKID_AO_IR_BLASTER] = &ir_blaster_ao.hw,
+ [CLKID_AO_SAR_ADC] = &saradc_ao.hw,
+ [CLKID_AO_CLK81] = &ao_clk81.hw,
+ [CLKID_AO_SAR_ADC_SEL] = &axg_saradc_mux.hw,
+ [CLKID_AO_SAR_ADC_DIV] = &axg_saradc_div.hw,
+ [CLKID_AO_SAR_ADC_CLK] = &axg_saradc_gate.hw,
+ },
+ .num = NR_CLKS,
+};
+
+static const struct meson_aoclk_data axg_aoclkc_data = {
+ .reset_reg = AO_RTI_GEN_CNTL_REG0,
+ .num_reset = ARRAY_SIZE(axg_aoclk_reset),
+ .reset = axg_aoclk_reset,
+ .num_clks = ARRAY_SIZE(axg_aoclk_regmap),
+ .clks = axg_aoclk_regmap,
+ .hw_data = &axg_aoclk_onecell_data,
+};
+
+static const struct of_device_id axg_aoclkc_match_table[] = {
+ {
+ .compatible = "amlogic,meson-axg-aoclkc",
+ .data = &axg_aoclkc_data,
+ },
+ { }
+};
+
+static struct platform_driver axg_aoclkc_driver = {
+ .probe = meson_aoclkc_probe,
+ .driver = {
+ .name = "axg-aoclkc",
+ .of_match_table = axg_aoclkc_match_table,
+ },
+};
+
+builtin_platform_driver(axg_aoclkc_driver);
diff --git a/drivers/clk/meson/axg-aoclk.h b/drivers/clk/meson/axg-aoclk.h
new file mode 100644
index 000000000000..396cd3023064
--- /dev/null
+++ b/drivers/clk/meson/axg-aoclk.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2017 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <qiufang.dai@amlogic.com>
+ */
+
+#ifndef __AXG_AOCLKC_H
+#define __AXG_AOCLKC_H
+
+#include "meson-aoclk.h"
+
+#define NR_CLKS 11
+/* AO Configuration Clock registers offsets
+ * Register offsets from the data sheet must be multiplied by 4.
+ */
+#define AO_RTI_PWR_CNTL_REG1 0x0C
+#define AO_RTI_PWR_CNTL_REG0 0x10
+#define AO_RTI_GEN_CNTL_REG0 0x40
+#define AO_OSCIN_CNTL 0x58
+#define AO_CRT_CLK_CNTL1 0x68
+#define AO_SAR_CLK 0x90
+#define AO_RTC_ALT_CLK_CNTL0 0x94
+#define AO_RTC_ALT_CLK_CNTL1 0x98
+
+#include <dt-bindings/clock/axg-aoclkc.h>
+#include <dt-bindings/reset/axg-aoclkc.h>
+
+#endif /* __AXG_AOCLKC_H */
--
2.15.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 5/7] clk: meson-axg: Add AO Clock and Reset controller driver
2018-04-08 3:19 ` [PATCH v4 5/7] clk: meson-axg: Add AO Clock and Reset controller driver Yixun Lan
@ 2018-04-09 12:08 ` Jerome Brunet
0 siblings, 0 replies; 12+ messages in thread
From: Jerome Brunet @ 2018-04-09 12:08 UTC (permalink / raw)
To: Yixun Lan, Neil Armstrong, Kevin Hilman, Carlo Caione
Cc: Qiufang Dai, Rob Herring, Michael Turquette, Stephen Boyd,
Philipp Zabel, linux-amlogic, linux-clk, linux-arm-kernel,
linux-kernel
On Sun, 2018-04-08 at 11:19 +0800, Yixun Lan wrote:
> From: Qiufang Dai <qiufang.dai@amlogic.com>
>
> Adds a Clock and Reset controller driver for the Always-On part
> of the Amlogic Meson-AXG SoC.
>
> Signed-off-by: Qiufang Dai <qiufang.dai@amlogic.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> ---
> drivers/clk/meson/Makefile | 2 +-
> drivers/clk/meson/axg-aoclk.c | 164 ++++++++++++++++++++++++++++++++++++++++++
> drivers/clk/meson/axg-aoclk.h | 31 ++++++++
> 3 files changed, 196 insertions(+), 1 deletion(-)
> create mode 100644 drivers/clk/meson/axg-aoclk.c
> create mode 100644 drivers/clk/meson/axg-aoclk.h
>
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 555ab9c6ab64..fa6d1e36cef6 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -5,5 +5,5 @@
> obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o meson-aoclk.o gxbb-aoclk.o gxbb-aoclk-32k.o
> -obj-$(CONFIG_COMMON_CLK_AXG) += axg.o
> +obj-$(CONFIG_COMMON_CLK_AXG) += axg.o meson-aoclk.o axg-aoclk.o
^
As hinted in patch 1, not ending well =======|
You are putting this object in 2 modules.
What you should have done is :
- Add a Kconfig CONFIG_COMMON_CLK_MESON_AO (same as CONFIG_COMMON_CLK_AMLOGIC)
selected by GXBB and AXG.
- Compile meson-aoclk.o according to this new configuration
> obj-$(CONFIG_COMMON_CLK_REGMAP_MESON) += clk-regmap.o
> diff --git a/drivers/clk/meson/axg-aoclk.c b/drivers/clk/meson/axg-aoclk.c
> new file mode 100644
> index 000000000000..cb56d809d3df
> --- /dev/null
> +++ b/drivers/clk/meson/axg-aoclk.c
> @@ -0,0 +1,164 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Amlogic Meson-AXG Clock Controller Driver
> + *
> + * Copyright (c) 2016 Baylibre SAS.
> + * Author: Michael Turquette <mturquette@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
> + */
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/init.h>
> +#include "clkc.h"
> +#include "axg-aoclk.h"
> +
> +#define AXG_AO_GATE(_name, _bit) \
> +static struct clk_regmap _name##_ao = { \
> + .data = &(struct clk_regmap_gate_data) { \
> + .offset = (AO_RTI_GEN_CNTL_REG0), \
> + .bit_idx = (_bit), \
> + }, \
> + .hw.init = &(struct clk_init_data) { \
> + .name = #_name "_ao", \
> + .ops = &clk_regmap_gate_ops, \
> + .parent_names = (const char *[]){ "clk81" }, \
> + .num_parents = 1, \
> + .flags = CLK_IGNORE_UNUSED, \
> + }, \
> +}
> +
> +AXG_AO_GATE(remote, 0);
> +AXG_AO_GATE(i2c_master, 1);
> +AXG_AO_GATE(i2c_slave, 2);
> +AXG_AO_GATE(uart1, 3);
> +AXG_AO_GATE(uart2, 5);
> +AXG_AO_GATE(ir_blaster, 6);
> +AXG_AO_GATE(saradc, 7);
> +
> +static struct clk_regmap ao_clk81 = {
> + .data = &(struct clk_regmap_mux_data) {
> + .offset = AO_RTI_PWR_CNTL_REG0,
> + .mask = 0x1,
> + .shift = 8,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "ao_clk81",
> + .ops = &clk_regmap_mux_ro_ops,
> + .parent_names = (const char *[]){ "clk81", "ao_alt_xtal"},
> + .num_parents = 2,
> + },
> +};
> +
> +static struct clk_regmap axg_saradc_mux = {
> + .data = &(struct clk_regmap_mux_data) {
> + .offset = AO_SAR_CLK,
> + .mask = 0x3,
> + .shift = 9,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "axg_saradc_mux",
> + .ops = &clk_regmap_mux_ops,
> + .parent_names = (const char *[]){ "xtal", "ao_clk81" },
> + .num_parents = 2,
> + },
> +};
> +
> +static struct clk_regmap axg_saradc_div = {
> + .data = &(struct clk_regmap_div_data) {
> + .offset = AO_SAR_CLK,
> + .shift = 0,
> + .width = 8,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "axg_saradc_div",
> + .ops = &clk_regmap_divider_ops,
> + .parent_names = (const char *[]){ "axg_saradc_mux" },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +static struct clk_regmap axg_saradc_gate = {
> + .data = &(struct clk_regmap_gate_data) {
> + .offset = AO_SAR_CLK,
> + .bit_idx = 8,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "axg_saradc_gate",
> + .ops = &clk_regmap_gate_ops,
> + .parent_names = (const char *[]){ "axg_saradc_div" },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +static const unsigned int axg_aoclk_reset[] = {
> + [RESET_AO_REMOTE] = 16,
> + [RESET_AO_I2C_MASTER] = 18,
> + [RESET_AO_I2C_SLAVE] = 19,
> + [RESET_AO_UART1] = 17,
> + [RESET_AO_UART2] = 22,
> + [RESET_AO_IR_BLASTER] = 23,
> +};
> +
> +static struct clk_regmap *axg_aoclk_regmap[] = {
> + [CLKID_AO_REMOTE] = &remote_ao,
> + [CLKID_AO_I2C_MASTER] = &i2c_master_ao,
> + [CLKID_AO_I2C_SLAVE] = &i2c_slave_ao,
> + [CLKID_AO_UART1] = &uart1_ao,
> + [CLKID_AO_UART2] = &uart2_ao,
> + [CLKID_AO_IR_BLASTER] = &ir_blaster_ao,
> + [CLKID_AO_SAR_ADC] = &saradc_ao,
> + [CLKID_AO_CLK81] = &ao_clk81,
> + [CLKID_AO_SAR_ADC_SEL] = &axg_saradc_mux,
> + [CLKID_AO_SAR_ADC_DIV] = &axg_saradc_div,
> + [CLKID_AO_SAR_ADC_CLK] = &axg_saradc_gate,
> +};
> +
> +static const struct clk_hw_onecell_data axg_aoclk_onecell_data = {
> + .hws = {
> + [CLKID_AO_REMOTE] = &remote_ao.hw,
> + [CLKID_AO_I2C_MASTER] = &i2c_master_ao.hw,
> + [CLKID_AO_I2C_SLAVE] = &i2c_slave_ao.hw,
> + [CLKID_AO_UART1] = &uart1_ao.hw,
> + [CLKID_AO_UART2] = &uart2_ao.hw,
> + [CLKID_AO_IR_BLASTER] = &ir_blaster_ao.hw,
> + [CLKID_AO_SAR_ADC] = &saradc_ao.hw,
> + [CLKID_AO_CLK81] = &ao_clk81.hw,
> + [CLKID_AO_SAR_ADC_SEL] = &axg_saradc_mux.hw,
> + [CLKID_AO_SAR_ADC_DIV] = &axg_saradc_div.hw,
> + [CLKID_AO_SAR_ADC_CLK] = &axg_saradc_gate.hw,
> + },
> + .num = NR_CLKS,
> +};
> +
> +static const struct meson_aoclk_data axg_aoclkc_data = {
> + .reset_reg = AO_RTI_GEN_CNTL_REG0,
> + .num_reset = ARRAY_SIZE(axg_aoclk_reset),
> + .reset = axg_aoclk_reset,
> + .num_clks = ARRAY_SIZE(axg_aoclk_regmap),
> + .clks = axg_aoclk_regmap,
> + .hw_data = &axg_aoclk_onecell_data,
> +};
> +
> +static const struct of_device_id axg_aoclkc_match_table[] = {
> + {
> + .compatible = "amlogic,meson-axg-aoclkc",
> + .data = &axg_aoclkc_data,
> + },
> + { }
> +};
> +
> +static struct platform_driver axg_aoclkc_driver = {
> + .probe = meson_aoclkc_probe,
> + .driver = {
> + .name = "axg-aoclkc",
> + .of_match_table = axg_aoclkc_match_table,
> + },
> +};
> +
> +builtin_platform_driver(axg_aoclkc_driver);
> diff --git a/drivers/clk/meson/axg-aoclk.h b/drivers/clk/meson/axg-aoclk.h
> new file mode 100644
> index 000000000000..396cd3023064
> --- /dev/null
> +++ b/drivers/clk/meson/axg-aoclk.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Copyright (c) 2017 BayLibre, SAS
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
> + */
> +
> +#ifndef __AXG_AOCLKC_H
> +#define __AXG_AOCLKC_H
> +
> +#include "meson-aoclk.h"
> +
> +#define NR_CLKS 11
> +/* AO Configuration Clock registers offsets
> + * Register offsets from the data sheet must be multiplied by 4.
> + */
> +#define AO_RTI_PWR_CNTL_REG1 0x0C
> +#define AO_RTI_PWR_CNTL_REG0 0x10
> +#define AO_RTI_GEN_CNTL_REG0 0x40
> +#define AO_OSCIN_CNTL 0x58
> +#define AO_CRT_CLK_CNTL1 0x68
> +#define AO_SAR_CLK 0x90
> +#define AO_RTC_ALT_CLK_CNTL0 0x94
> +#define AO_RTC_ALT_CLK_CNTL1 0x98
> +
> +#include <dt-bindings/clock/axg-aoclkc.h>
> +#include <dt-bindings/reset/axg-aoclkc.h>
> +
> +#endif /* __AXG_AOCLKC_H */
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 6/7] clk: meson: drop CLK_SET_RATE_PARENT flag
2018-04-08 3:19 [PATCH v4 0/7] clk: meson-axg: Add AO Cloclk and Reset driver Yixun Lan
` (4 preceding siblings ...)
2018-04-08 3:19 ` [PATCH v4 5/7] clk: meson-axg: Add AO Clock and Reset controller driver Yixun Lan
@ 2018-04-08 3:19 ` Yixun Lan
2018-04-08 3:19 ` [PATCH v4 7/7] clk: meson: drop CLK_IGNORE_UNUSED flag Yixun Lan
6 siblings, 0 replies; 12+ messages in thread
From: Yixun Lan @ 2018-04-08 3:19 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Carlo Caione
Cc: Yixun Lan, Rob Herring, Michael Turquette, Stephen Boyd,
Philipp Zabel, Qiufang Dai, linux-amlogic, linux-clk,
linux-arm-kernel, linux-kernel
The clk81 is not expected to be changed, so drop this flag.
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
---
drivers/clk/meson/gxbb-aoclk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 59db8e92f8cf..0a2641d6fd33 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -70,7 +70,7 @@ static struct clk_regmap _name##_ao = { \
.ops = &clk_regmap_gate_ops, \
.parent_names = (const char *[]){ "clk81" }, \
.num_parents = 1, \
- .flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED), \
+ .flags = CLK_IGNORE_UNUSED, \
}, \
}
--
2.15.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 7/7] clk: meson: drop CLK_IGNORE_UNUSED flag
2018-04-08 3:19 [PATCH v4 0/7] clk: meson-axg: Add AO Cloclk and Reset driver Yixun Lan
` (5 preceding siblings ...)
2018-04-08 3:19 ` [PATCH v4 6/7] clk: meson: drop CLK_SET_RATE_PARENT flag Yixun Lan
@ 2018-04-08 3:19 ` Yixun Lan
6 siblings, 0 replies; 12+ messages in thread
From: Yixun Lan @ 2018-04-08 3:19 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Carlo Caione
Cc: Yixun Lan, Rob Herring, Michael Turquette, Stephen Boyd,
Philipp Zabel, Qiufang Dai, linux-amlogic, linux-clk,
linux-arm-kernel, linux-kernel
Rely on drivers to request the clock explicitly.
Previous the kernel will leave the clock on while
bootloader adready initilized the clock, this wasn't
optimal way, so fix it here.
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
---
drivers/clk/meson/axg-aoclk.c | 1 -
drivers/clk/meson/gxbb-aoclk.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/clk/meson/axg-aoclk.c b/drivers/clk/meson/axg-aoclk.c
index cb56d809d3df..8457b64b9b7c 100644
--- a/drivers/clk/meson/axg-aoclk.c
+++ b/drivers/clk/meson/axg-aoclk.c
@@ -27,7 +27,6 @@ static struct clk_regmap _name##_ao = { \
.ops = &clk_regmap_gate_ops, \
.parent_names = (const char *[]){ "clk81" }, \
.num_parents = 1, \
- .flags = CLK_IGNORE_UNUSED, \
}, \
}
diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 0a2641d6fd33..aa655044011a 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -70,7 +70,6 @@ static struct clk_regmap _name##_ao = { \
.ops = &clk_regmap_gate_ops, \
.parent_names = (const char *[]){ "clk81" }, \
.num_parents = 1, \
- .flags = CLK_IGNORE_UNUSED, \
}, \
}
--
2.15.1
^ permalink raw reply related [flat|nested] 12+ messages in thread