This patch try to add AO clock and Reset driver in Amlogic's
Meson-AXG SoC.
Please note this patchset actually depend on the clock regmap
conversion series [1].
[1] clk: meson: use regmap in clock controllers
https://lkml.kernel.org/r/[email protected]
Yixun Lan (2):
dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
clk: meson-axg: Add AO Clock and Reset controller driver
drivers/clk/meson/Makefile | 2 +-
drivers/clk/meson/axg-aoclk.c | 236 +++++++++++++++++++++++++++++++++
drivers/clk/meson/axg-aoclk.h | 25 ++++
include/dt-bindings/clock/axg-aoclkc.h | 26 ++++
include/dt-bindings/reset/axg-aoclkc.h | 20 +++
5 files changed, 308 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/meson/axg-aoclk.c
create mode 100644 drivers/clk/meson/axg-aoclk.h
create mode 100644 include/dt-bindings/clock/axg-aoclkc.h
create mode 100644 include/dt-bindings/reset/axg-aoclkc.h
--
2.15.1
Adds a Clock and Reset controller driver for the Always-On part
of the Amlogic Meson-AXG SoC.
Signed-off-by: Qiufang Dai <[email protected]>
Signed-off-by: Yixun Lan <[email protected]>
---
drivers/clk/meson/Makefile | 2 +-
drivers/clk/meson/axg-aoclk.c | 236 ++++++++++++++++++++++++++++++++++++++++++
drivers/clk/meson/axg-aoclk.h | 25 +++++
3 files changed, 262 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 11f99139b844..c7510744406a 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -6,6 +6,6 @@ obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-phase.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_AXG) += axg.o
+obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.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..832aa19dd76c
--- /dev/null
+++ b/drivers/clk/meson/axg-aoclk.c
@@ -0,0 +1,236 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AmLogic Meson-AXG Clock Controller Driver
+ *
+ * Copyright (c) 2016 Baylibre SAS.
+ * Author: Michael Turquette <[email protected]>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <[email protected]>
+ */
+#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/axg-aoclkc.h>
+#include <dt-bindings/reset/axg-aoclkc.h>
+#include "clkc.h"
+#include "axg-aoclk.h"
+
+struct axg_aoclk_reset_controller {
+ struct reset_controller_dev reset;
+ unsigned int *data;
+ struct regmap *regmap;
+};
+
+static int axg_aoclk_do_reset(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct axg_aoclk_reset_controller *reset =
+ container_of(rcdev, struct axg_aoclk_reset_controller, reset);
+
+ return regmap_write(reset->regmap, AO_RTI_GEN_CNTL_REG0,
+ BIT(reset->data[id]));
+}
+
+static const struct reset_control_ops axg_aoclk_reset_ops = {
+ .reset = axg_aoclk_do_reset,
+};
+
+#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_SET_RATE_PARENT | 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_fixed_rate ao_alt_xtal = {
+ .fixed_rate = 32000,
+ .hw.init = &(struct clk_init_data){
+ .name = "ao_alt_xtal",
+ .num_parents = 0,
+ .ops = &clk_fixed_rate_ops,
+ },
+};
+
+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,
+ },
+};
+
+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 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 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,
+ [CLKID_AO_ALT_XTAL] = &ao_alt_xtal.hw,
+ },
+ .num = 12,
+};
+
+static int axg_aoclkc_probe(struct platform_device *pdev)
+{
+ struct axg_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;
+
+ 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->regmap = regmap;
+ rstc->data = axg_aoclk_reset;
+ rstc->reset.ops = &axg_aoclk_reset_ops;
+ rstc->reset.nr_resets = ARRAY_SIZE(axg_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(axg_aoclk_regmap); clkid++) {
+ axg_aoclk_regmap[clkid]->map = regmap;
+
+ ret = devm_clk_hw_register(dev,
+ axg_aoclk_onecell_data.hws[clkid]);
+ if (ret) {
+ dev_err(dev, "clk register failed.\n");
+ return ret;
+ }
+ }
+
+ /* Specific clocks */
+ ret = devm_clk_hw_register(dev, &ao_alt_xtal.hw);
+ if (ret) {
+ dev_err(dev, "clk alt_xtal register failed.\n");
+ return ret;
+ }
+
+ return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
+ &axg_aoclk_onecell_data);
+}
+
+static const struct of_device_id axg_aoclkc_match_table[] = {
+ { .compatible = "amlogic,meson-axg-aoclkc" },
+ { }
+};
+
+static struct platform_driver axg_aoclkc_driver = {
+ .probe = axg_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..70f82004d3db
--- /dev/null
+++ b/drivers/clk/meson/axg-aoclk.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2017 BayLibre, SAS
+ * Author: Neil Armstrong <[email protected]>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <[email protected]>
+ */
+
+#ifndef __AXG_AOCLKC_H
+#define __AXG_AOCLKC_H
+
+/* AO Configuration Clock registers offsets
+ * Register offsets from the data sheet must be multiplied by 4.
+ */
+#define AO_RTI_PWR_CNTL_REG1 (0x03 << 2)
+#define AO_RTI_PWR_CNTL_REG0 (0x04 << 2)
+#define AO_RTI_GEN_CNTL_REG0 (0x10 << 2)
+#define AO_OSCIN_CNTL (0x16 << 2)
+#define AO_CRT_CLK_CNTL1 (0x1a << 2)
+#define AO_SAR_CLK (0x24 << 2)
+#define AO_RTC_ALT_CLK_CNTL0 (0x25 << 2)
+#define AO_RTC_ALT_CLK_CNTL1 (0x26 << 2)
+
+#endif /* __AXG_AOCLKC_H */
--
2.15.1
Add dt-bindings headers for the Meson-AXG's AO clock and
reset controller.
CC: <[email protected]>
Signed-off-by: Yixun Lan <[email protected]>
---
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..78683abb4247
--- /dev/null
+++ b/include/dt-bindings/clock/axg-aoclkc.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD) */
+/*
+ * Copyright (c) 2016 BayLibre, SAS
+ * Author: Neil Armstrong <[email protected]>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <[email protected]>
+ */
+
+#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..307f58161bbb
--- /dev/null
+++ b/include/dt-bindings/reset/axg-aoclkc.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD) */
+/*
+ * Copyright (c) 2016 BayLibre, SAS
+ * Author: Neil Armstrong <[email protected]>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <[email protected]>
+ */
+
+#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
On Fri, Feb 9, 2018 at 8:00 AM, Yixun Lan <[email protected]> wrote:
> Add dt-bindings headers for the Meson-AXG's AO clock and
> reset controller.
>
> CC: <[email protected]>
> Signed-off-by: Yixun Lan <[email protected]>
> ---
> 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..78683abb4247
> --- /dev/null
> +++ b/include/dt-bindings/clock/axg-aoclkc.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD) */
nit... BSD is not a valid license id as documented in LICENSES [1] :
please use one of these:
BSD-2-Clause or BSD-3-Clause
Thank you!
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/LICENSES/preferred
--
Cordially
Philippe Ombredanne
On Fri, 2018-02-09 at 15:00 +0800, Yixun Lan wrote:
> Adds a Clock and Reset controller driver for the Always-On part
> of the Amlogic Meson-AXG SoC.
>
> Signed-off-by: Qiufang Dai <[email protected]>
> Signed-off-by: Yixun Lan <[email protected]>
> ---
> drivers/clk/meson/Makefile | 2 +-
> drivers/clk/meson/axg-aoclk.c | 236 ++++++++++++++++++++++++++++++++++++++++++
> drivers/clk/meson/axg-aoclk.h | 25 +++++
> 3 files changed, 262 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 11f99139b844..c7510744406a 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -6,6 +6,6 @@ obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
> obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-phase.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_AXG) += axg.o
> +obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
> obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.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..832aa19dd76c
> --- /dev/null
> +++ b/drivers/clk/meson/axg-aoclk.c
> @@ -0,0 +1,236 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * AmLogic Meson-AXG Clock Controller Driver
Unless there a rule behind it, I think we should stop
with the funny camel case and stick to "Amlogic" like written below.
> + *
> + * Copyright (c) 2016 Baylibre SAS.
> + * Author: Michael Turquette <[email protected]>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Qiufang Dai <[email protected]>
> + */
> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
I did not see what is requiring this header. Did I miss something ?
> +#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>
Same question for this one.
> +#include <dt-bindings/clock/axg-aoclkc.h>
> +#include <dt-bindings/reset/axg-aoclkc.h>
I think those should be included in the same fashion as axg.h, gxbb.h and
meson8.h ... which is at the end of the private header.
> +#include "clkc.h"
> +#include "axg-aoclk.h"
> +
> +struct axg_aoclk_reset_controller {
> + struct reset_controller_dev reset;
> + unsigned int *data;
> + struct regmap *regmap;
> +};
This duplicate what is gxbb-aoclk.
Please make something cleaner with a shared definition.
> +
> +static int axg_aoclk_do_reset(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct axg_aoclk_reset_controller *reset =
> + container_of(rcdev, struct axg_aoclk_reset_controller, reset);
> +
> + return regmap_write(reset->regmap, AO_RTI_GEN_CNTL_REG0,
> + BIT(reset->data[id]));
> +}
> +
> +static const struct reset_control_ops axg_aoclk_reset_ops = {
> + .reset = axg_aoclk_do_reset,
> +};
> +
> +#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_SET_RATE_PARENT | CLK_IGNORE_UNUSED), \
These clock are peripheral clocks (meant to enable/disable IPs):
* CLK_SET_RATE_PARENT: why do you need rate propagation here ? do you really
want a consumer of these gate to be able to change clk81 ?
* CLK_IGNORE_UNUSED: Why do need this flag ? All the IPs behind this
definition should be able to claim the pclk properly. If not the driver should
be fixed.
> + }, \
> +}
> +
> +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_fixed_rate ao_alt_xtal = {
> + .fixed_rate = 32000,
> + .hw.init = &(struct clk_init_data){
> + .name = "ao_alt_xtal",
> + .num_parents = 0,
> + .ops = &clk_fixed_rate_ops,
> + },
> +};
Unless this xtal is actually embedded in SoC, this should be defined it the DT,
same as the other xtal.
> +
> +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,
Should the saradc mux change to accomodate the requested rate, if needed ?
If so, you might want to add a CLK_SET_RATE_PARENT here.
> + },
> +};
> +
> +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 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 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,
> + [CLKID_AO_ALT_XTAL] = &ao_alt_xtal.hw,
> + },
> + .num = 12,
I would prefer having a NR_CLKS defined in drivers/clk/meson/axg-aoclk.h
> +};
> +
> +static int axg_aoclkc_probe(struct platform_device *pdev)
> +{
> + struct axg_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;
> +
> + 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->regmap = regmap;
> + rstc->data = axg_aoclk_reset;
> + rstc->reset.ops = &axg_aoclk_reset_ops;
> + rstc->reset.nr_resets = ARRAY_SIZE(axg_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(axg_aoclk_regmap); clkid++) {
> + axg_aoclk_regmap[clkid]->map = regmap;
> +
> + ret = devm_clk_hw_register(dev,
> + axg_aoclk_onecell_data.hws[clkid]);
> + if (ret) {
> + dev_err(dev, "clk register failed.\n");
> + return ret;
> + }
> + }
> +
> + /* Specific clocks */
> + ret = devm_clk_hw_register(dev, &ao_alt_xtal.hw);
> + if (ret) {
> + dev_err(dev, "clk alt_xtal register failed.\n");
> + return ret;
> + }
> +
> + return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
> + &axg_aoclk_onecell_data);
> +}
> +
> +static const struct of_device_id axg_aoclkc_match_table[] = {
> + { .compatible = "amlogic,meson-axg-aoclkc" },
> + { }
> +};
> +
> +static struct platform_driver axg_aoclkc_driver = {
> + .probe = axg_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..70f82004d3db
> --- /dev/null
> +++ b/drivers/clk/meson/axg-aoclk.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Copyright (c) 2017 BayLibre, SAS
> + * Author: Neil Armstrong <[email protected]>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Qiufang Dai <[email protected]>
> + */
> +
> +#ifndef __AXG_AOCLKC_H
> +#define __AXG_AOCLKC_H
> +
> +/* AO Configuration Clock registers offsets
> + * Register offsets from the data sheet must be multiplied by 4.
> + */
> +#define AO_RTI_PWR_CNTL_REG1 (0x03 << 2)
> +#define AO_RTI_PWR_CNTL_REG0 (0x04 << 2)
> +#define AO_RTI_GEN_CNTL_REG0 (0x10 << 2)
> +#define AO_OSCIN_CNTL (0x16 << 2)
> +#define AO_CRT_CLK_CNTL1 (0x1a << 2)
> +#define AO_SAR_CLK (0x24 << 2)
> +#define AO_RTC_ALT_CLK_CNTL0 (0x25 << 2)
> +#define AO_RTC_ALT_CLK_CNTL1 (0x26 << 2)
As already pointed out for the axg clk series, please remove those
calculation from the header file.
> +
> +#endif /* __AXG_AOCLKC_H */
On Mon, 2018-02-12 at 16:32 +0100, Jerome Brunet wrote:
> On Fri, 2018-02-09 at 15:00 +0800, Yixun Lan wrote:
> > Adds a Clock and Reset controller driver for the Always-On part
> > of the Amlogic Meson-AXG SoC.
> >
> > Signed-off-by: Qiufang Dai <[email protected]>
> > Signed-off-by: Yixun Lan <[email protected]>
> > ---
> > drivers/clk/meson/Makefile | 2 +-
> > drivers/clk/meson/axg-aoclk.c | 236 ++++++++++++++++++++++++++++++++++++++++++
> > drivers/clk/meson/axg-aoclk.h | 25 +++++
> > 3 files changed, 262 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/clk/meson/axg-aoclk.c
> > create mode 100644 drivers/clk/meson/axg-aoclk.h
Yixun,
Actually, looking at this driver further, except for some of the data, the
driver is exactly the same as gxbb-aoclk. There is no reason to duplicate all
this code.
I'd prefer if you'd factor all this in a dedicated driver.
This philosophy is more or less the same as meson's pinctrl drivers, which all
provides their own data and bindings but share the driver code.
Regards
Jerome
Hi Jerome:
On 02/12/2018 11:32 PM, Jerome Brunet wrote:
> On Fri, 2018-02-09 at 15:00 +0800, Yixun Lan wrote:
>> Adds a Clock and Reset controller driver for the Always-On part
>> of the Amlogic Meson-AXG SoC.
>>
>> Signed-off-by: Qiufang Dai <[email protected]>
>> Signed-off-by: Yixun Lan <[email protected]>
>> ---
>> drivers/clk/meson/Makefile | 2 +-
>> drivers/clk/meson/axg-aoclk.c | 236 ++++++++++++++++++++++++++++++++++++++++++
>> drivers/clk/meson/axg-aoclk.h | 25 +++++
>> 3 files changed, 262 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 11f99139b844..c7510744406a 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -6,6 +6,6 @@ obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
>> obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-phase.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_AXG) += axg.o
>> +obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>> obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.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..832aa19dd76c
>> --- /dev/null
>> +++ b/drivers/clk/meson/axg-aoclk.c
>> @@ -0,0 +1,236 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * AmLogic Meson-AXG Clock Controller Driver
>
> Unless there a rule behind it, I think we should stop
> with the funny camel case and stick to "Amlogic" like written below.
>
will fix it
>> + *
>> + * Copyright (c) 2016 Baylibre SAS.
>> + * Author: Michael Turquette <[email protected]>
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Qiufang Dai <[email protected]>
>> + */
>> +#include <linux/clk-provider.h>
>> +#include <linux/of_address.h>
> I did not see what is requiring this header. Did I miss something ?
>
will drop this
>> +#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>
> Same question for this one.
>
ditto
>> +#include <dt-bindings/clock/axg-aoclkc.h>
>> +#include <dt-bindings/reset/axg-aoclkc.h>
>
> I think those should be included in the same fashion as axg.h, gxbb.h and
> meson8.h ... which is at the end of the private header.
>
will do
>> +#include "clkc.h"
>> +#include "axg-aoclk.h"
>> +
>> +struct axg_aoclk_reset_controller {
>> + struct reset_controller_dev reset;
>> + unsigned int *data;
>> + struct regmap *regmap;
>> +};
>
> This duplicate what is gxbb-aoclk.
> Please make something cleaner with a shared definition.
>
as you already raised this in another email.
and I will factor the common code, and send out an updated version
>> +
>> +static int axg_aoclk_do_reset(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + struct axg_aoclk_reset_controller *reset =
>> + container_of(rcdev, struct axg_aoclk_reset_controller, reset);
>> +
>> + return regmap_write(reset->regmap, AO_RTI_GEN_CNTL_REG0,
>> + BIT(reset->data[id]));
>> +}
>> +
>> +static const struct reset_control_ops axg_aoclk_reset_ops = {
>> + .reset = axg_aoclk_do_reset,
>> +};
>> +
>> +#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_SET_RATE_PARENT | CLK_IGNORE_UNUSED), \
>
> These clock are peripheral clocks (meant to enable/disable IPs):
> * CLK_SET_RATE_PARENT: why do you need rate propagation here ? do you really
> want a consumer of these gate to be able to change clk81 ?
>
No, we shouldn't change clk81, will drop this flag
> * CLK_IGNORE_UNUSED: Why do need this flag ? All the IPs behind this
> definition should be able to claim the pclk properly. If not the driver should
> be fixed.
>
not really, we could drop this flag
but I'd like to address this in yet another thread (rather than in this
"add ao clock and reset driver")
>> + }, \
>> +}
>> +
>> +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_fixed_rate ao_alt_xtal = {
>> + .fixed_rate = 32000,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "ao_alt_xtal",
>> + .num_parents = 0,
>> + .ops = &clk_fixed_rate_ops,
>> + },
>> +};
>
> Unless this xtal is actually embedded in SoC, this should be defined it the DT,
> same as the other xtal.
>
it's an external crystal, I will put it into DT file, thanks for hint
>> +
>> +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,
>
> Should the saradc mux change to accomodate the requested rate, if needed ?
> If so, you might want to add a CLK_SET_RATE_PARENT here.
>
yes, from hardware perspective, the mux can select clk parent, I will
update this
>> + },
>> +};
>> +
>> +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 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 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,
>> + [CLKID_AO_ALT_XTAL] = &ao_alt_xtal.hw,
>> + },
>> + .num = 12,
>
> I would prefer having a NR_CLKS defined in drivers/clk/meson/axg-aoclk.h
>
can do
>> +};
>> +
>> +static int axg_aoclkc_probe(struct platform_device *pdev)
>> +{
>> + struct axg_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;
>> +
>> + 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->regmap = regmap;
>> + rstc->data = axg_aoclk_reset;
>> + rstc->reset.ops = &axg_aoclk_reset_ops;
>> + rstc->reset.nr_resets = ARRAY_SIZE(axg_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(axg_aoclk_regmap); clkid++) {
>> + axg_aoclk_regmap[clkid]->map = regmap;
>> +
>> + ret = devm_clk_hw_register(dev,
>> + axg_aoclk_onecell_data.hws[clkid]);
>> + if (ret) {
>> + dev_err(dev, "clk register failed.\n");
>> + return ret;
>> + }
>> + }
>> +
>> + /* Specific clocks */
>> + ret = devm_clk_hw_register(dev, &ao_alt_xtal.hw);
>> + if (ret) {
>> + dev_err(dev, "clk alt_xtal register failed.\n");
>> + return ret;
>> + }
>> +
>> + return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
>> + &axg_aoclk_onecell_data);
>> +}
>> +
>> +static const struct of_device_id axg_aoclkc_match_table[] = {
>> + { .compatible = "amlogic,meson-axg-aoclkc" },
>> + { }
>> +};
>> +
>> +static struct platform_driver axg_aoclkc_driver = {
>> + .probe = axg_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..70f82004d3db
>> --- /dev/null
>> +++ b/drivers/clk/meson/axg-aoclk.h
>> @@ -0,0 +1,25 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Copyright (c) 2017 BayLibre, SAS
>> + * Author: Neil Armstrong <[email protected]>
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Qiufang Dai <[email protected]>
>> + */
>> +
>> +#ifndef __AXG_AOCLKC_H
>> +#define __AXG_AOCLKC_H
>> +
>> +/* AO Configuration Clock registers offsets
>> + * Register offsets from the data sheet must be multiplied by 4.
>> + */
>> +#define AO_RTI_PWR_CNTL_REG1 (0x03 << 2)
>> +#define AO_RTI_PWR_CNTL_REG0 (0x04 << 2)
>> +#define AO_RTI_GEN_CNTL_REG0 (0x10 << 2)
>> +#define AO_OSCIN_CNTL (0x16 << 2)
>> +#define AO_CRT_CLK_CNTL1 (0x1a << 2)
>> +#define AO_SAR_CLK (0x24 << 2)
>> +#define AO_RTC_ALT_CLK_CNTL0 (0x25 << 2)
>> +#define AO_RTC_ALT_CLK_CNTL1 (0x26 << 2)
>
> As already pointed out for the axg clk series, please remove those
> calculation from the header file.
>
Ok, will update
>> +
>> +#endif /* __AXG_AOCLKC_H */
>
> .
>
Hi Philippe
On 02/09/2018 07:07 PM, Philippe Ombredanne wrote:
> On Fri, Feb 9, 2018 at 8:00 AM, Yixun Lan <[email protected]> wrote:
>> Add dt-bindings headers for the Meson-AXG's AO clock and
>> reset controller.
>>
>> CC: <[email protected]>
>> Signed-off-by: Yixun Lan <[email protected]>
>> ---
>> 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..78683abb4247
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/axg-aoclkc.h
>> @@ -0,0 +1,26 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD) */
>
> nit... BSD is not a valid license id as documented in LICENSES [1] :
> please use one of these:
> BSD-2-Clause or BSD-3-Clause
>
> Thank you!
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/LICENSES/preferred
>
Thanks for the hint
I'll update to BSD-3-Clause, since this license is just a copy from
previous gxbb-aoclkc.h file.
Yixun