All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] clk: meson-axg: Add AO Cloclk and Reset driver
@ 2018-02-09  7:00 ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-02-09  7:00 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Carlo Caione,
	Kevin Hilman, Philipp Zabel, Yixun Lan, Qiufang Dai, Jian Hu,
	linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel

  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/20180131180945.18025-1-jbrunet@baylibre.com


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 0/2] clk: meson-axg: Add AO Cloclk and Reset driver
@ 2018-02-09  7:00 ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-02-09  7:00 UTC (permalink / raw)
  To: linux-arm-kernel

  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/20180131180945.18025-1-jbrunet at baylibre.com


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 0/2] clk: meson-axg: Add AO Cloclk and Reset driver
@ 2018-02-09  7:00 ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-02-09  7:00 UTC (permalink / raw)
  To: linus-amlogic

  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/20180131180945.18025-1-jbrunet at baylibre.com


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/2] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
  2018-02-09  7:00 ` Yixun Lan
  (?)
  (?)
@ 2018-02-09  7:00   ` Yixun Lan
  -1 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-02-09  7:00 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Carlo Caione,
	Kevin Hilman, Philipp Zabel, Yixun Lan, Qiufang Dai, Jian Hu,
	linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel,
	devicetree

Add dt-bindings headers for the Meson-AXG's AO clock and
reset controller.

CC: <devicetree@vger.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..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 <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..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 <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	[flat|nested] 26+ messages in thread

* [PATCH 1/2] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
@ 2018-02-09  7:00   ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-02-09  7:00 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet
  Cc: Yixun Lan, Rob Herring, devicetree, Kevin Hilman,
	Michael Turquette, Stephen Boyd, linux-kernel, Qiufang Dai,
	Jian Hu, Philipp Zabel, Carlo Caione, linux-amlogic, linux-clk,
	linux-arm-kernel

Add dt-bindings headers for the Meson-AXG's AO clock and
reset controller.

CC: <devicetree@vger.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..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 <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..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 <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	[flat|nested] 26+ messages in thread

* [PATCH 1/2] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
@ 2018-02-09  7:00   ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-02-09  7:00 UTC (permalink / raw)
  To: linux-arm-kernel

Add dt-bindings headers for the Meson-AXG's AO clock and
reset controller.

CC: <devicetree@vger.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..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 <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..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 <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	[flat|nested] 26+ messages in thread

* [PATCH 1/2] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
@ 2018-02-09  7:00   ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-02-09  7:00 UTC (permalink / raw)
  To: linus-amlogic

Add dt-bindings headers for the Meson-AXG's AO clock and
reset controller.

CC: <devicetree@vger.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..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 <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..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 <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	[flat|nested] 26+ messages in thread

* [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver
  2018-02-09  7:00 ` Yixun Lan
  (?)
@ 2018-02-09  7:00   ` Yixun Lan
  -1 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-02-09  7:00 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Carlo Caione,
	Kevin Hilman, Philipp Zabel, Yixun Lan, Qiufang Dai, Jian Hu,
	linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel

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 | 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 <mturquette@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <qiufang.dai@amlogic.com>
+ */
+#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 <narmstrong@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <qiufang.dai@amlogic.com>
+ */
+
+#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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver
@ 2018-02-09  7:00   ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-02-09  7:00 UTC (permalink / raw)
  To: linux-arm-kernel

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 | 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 <mturquette@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <qiufang.dai@amlogic.com>
+ */
+#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 <narmstrong@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <qiufang.dai@amlogic.com>
+ */
+
+#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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver
@ 2018-02-09  7:00   ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-02-09  7:00 UTC (permalink / raw)
  To: linus-amlogic

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 | 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 <mturquette@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <qiufang.dai@amlogic.com>
+ */
+#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 <narmstrong@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai <qiufang.dai@amlogic.com>
+ */
+
+#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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
  2018-02-09  7:00   ` Yixun Lan
  (?)
@ 2018-02-09 11:07     ` Philippe Ombredanne
  -1 siblings, 0 replies; 26+ messages in thread
From: Philippe Ombredanne @ 2018-02-09 11:07 UTC (permalink / raw)
  To: Yixun Lan
  Cc: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
	Rob Herring, Carlo Caione, Kevin Hilman, Philipp Zabel,
	Qiufang Dai, Jian Hu, linux-amlogic, linux-clk,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, Feb 9, 2018 at 8:00 AM, Yixun Lan <yixun.lan@amlogic.com> wrote:
> Add dt-bindings headers for the Meson-AXG's AO clock and
> reset controller.
>
> CC: <devicetree@vger.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..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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/2] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
@ 2018-02-09 11:07     ` Philippe Ombredanne
  0 siblings, 0 replies; 26+ messages in thread
From: Philippe Ombredanne @ 2018-02-09 11:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 9, 2018 at 8:00 AM, Yixun Lan <yixun.lan@amlogic.com> wrote:
> Add dt-bindings headers for the Meson-AXG's AO clock and
> reset controller.
>
> CC: <devicetree@vger.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..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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/2] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
@ 2018-02-09 11:07     ` Philippe Ombredanne
  0 siblings, 0 replies; 26+ messages in thread
From: Philippe Ombredanne @ 2018-02-09 11:07 UTC (permalink / raw)
  To: linus-amlogic

On Fri, Feb 9, 2018 at 8:00 AM, Yixun Lan <yixun.lan@amlogic.com> wrote:
> Add dt-bindings headers for the Meson-AXG's AO clock and
> reset controller.
>
> CC: <devicetree@vger.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..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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver
  2018-02-09  7:00   ` Yixun Lan
  (?)
@ 2018-02-12 15:32     ` Jerome Brunet
  -1 siblings, 0 replies; 26+ messages in thread
From: Jerome Brunet @ 2018-02-12 15:32 UTC (permalink / raw)
  To: Yixun Lan, Neil Armstrong
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Carlo Caione,
	Kevin Hilman, Philipp Zabel, Qiufang Dai, Jian Hu, linux-amlogic,
	linux-clk, linux-arm-kernel, linux-kernel

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 <qiufang.dai@amlogic.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> ---
>  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 <mturquette@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
> + */
> +#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 <narmstrong@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
> + */
> +
> +#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 */

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver
@ 2018-02-12 15:32     ` Jerome Brunet
  0 siblings, 0 replies; 26+ messages in thread
From: Jerome Brunet @ 2018-02-12 15:32 UTC (permalink / raw)
  To: linux-arm-kernel

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 <qiufang.dai@amlogic.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> ---
>  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 <mturquette@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
> + */
> +#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 <narmstrong@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
> + */
> +
> +#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 */

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver
@ 2018-02-12 15:32     ` Jerome Brunet
  0 siblings, 0 replies; 26+ messages in thread
From: Jerome Brunet @ 2018-02-12 15:32 UTC (permalink / raw)
  To: linus-amlogic

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 <qiufang.dai@amlogic.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> ---
>  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 <mturquette@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
> + */
> +#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 <narmstrong@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
> + */
> +
> +#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 */

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver
  2018-02-12 15:32     ` Jerome Brunet
  (?)
@ 2018-02-13 14:51       ` Jerome Brunet
  -1 siblings, 0 replies; 26+ messages in thread
From: Jerome Brunet @ 2018-02-13 14:51 UTC (permalink / raw)
  To: Yixun Lan, Neil Armstrong
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Carlo Caione,
	Kevin Hilman, Philipp Zabel, Qiufang Dai, Jian Hu, linux-amlogic,
	linux-clk, linux-arm-kernel, linux-kernel

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 <qiufang.dai@amlogic.com>
> > Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> > ---
> >   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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver
@ 2018-02-13 14:51       ` Jerome Brunet
  0 siblings, 0 replies; 26+ messages in thread
From: Jerome Brunet @ 2018-02-13 14:51 UTC (permalink / raw)
  To: linux-arm-kernel

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 <qiufang.dai@amlogic.com>
> > Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> > ---
> >   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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver
@ 2018-02-13 14:51       ` Jerome Brunet
  0 siblings, 0 replies; 26+ messages in thread
From: Jerome Brunet @ 2018-02-13 14:51 UTC (permalink / raw)
  To: linus-amlogic

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 <qiufang.dai@amlogic.com>
> > Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> > ---
> >   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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
  2018-02-09 11:07     ` Philippe Ombredanne
  (?)
  (?)
@ 2018-03-23 14:06       ` Yixun Lan
  -1 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-03-23 14:06 UTC (permalink / raw)
  To: Philippe Ombredanne
  Cc: yixun.lan, Neil Armstrong, Jerome Brunet, Michael Turquette,
	Stephen Boyd, Rob Herring, Carlo Caione, Kevin Hilman,
	Philipp Zabel, Qiufang Dai, Jian Hu, linux-amlogic, linux-clk,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Philippe

On 02/09/2018 07:07 PM, Philippe Ombredanne wrote:
> On Fri, Feb 9, 2018 at 8:00 AM, Yixun Lan <yixun.lan@amlogic.com> wrote:
>> Add dt-bindings headers for the Meson-AXG's AO clock and
>> reset controller.
>>
>> CC: <devicetree@vger.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..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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
@ 2018-03-23 14:06       ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-03-23 14:06 UTC (permalink / raw)
  To: Philippe Ombredanne
  Cc: Rob Herring, Neil Armstrong, yixun.lan, Michael Turquette,
	Stephen Boyd, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Qiufang Dai, Jian Hu, Philipp Zabel, Kevin Hilman, Carlo Caione,
	linux-amlogic, linux-clk,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Jerome Brunet

Hi Philippe

On 02/09/2018 07:07 PM, Philippe Ombredanne wrote:
> On Fri, Feb 9, 2018 at 8:00 AM, Yixun Lan <yixun.lan@amlogic.com> wrote:
>> Add dt-bindings headers for the Meson-AXG's AO clock and
>> reset controller.
>>
>> CC: <devicetree@vger.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..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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/2] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
@ 2018-03-23 14:06       ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-03-23 14:06 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Philippe

On 02/09/2018 07:07 PM, Philippe Ombredanne wrote:
> On Fri, Feb 9, 2018 at 8:00 AM, Yixun Lan <yixun.lan@amlogic.com> wrote:
>> Add dt-bindings headers for the Meson-AXG's AO clock and
>> reset controller.
>>
>> CC: <devicetree@vger.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..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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/2] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings
@ 2018-03-23 14:06       ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-03-23 14:06 UTC (permalink / raw)
  To: linus-amlogic

Hi Philippe

On 02/09/2018 07:07 PM, Philippe Ombredanne wrote:
> On Fri, Feb 9, 2018 at 8:00 AM, Yixun Lan <yixun.lan@amlogic.com> wrote:
>> Add dt-bindings headers for the Meson-AXG's AO clock and
>> reset controller.
>>
>> CC: <devicetree@vger.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..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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver
  2018-02-12 15:32     ` Jerome Brunet
  (?)
@ 2018-03-23 14:13       ` Yixun Lan
  -1 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-03-23 14:13 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong
  Cc: yixun.lan, Michael Turquette, Stephen Boyd, Rob Herring,
	Carlo Caione, Kevin Hilman, Philipp Zabel, Qiufang Dai, Jian Hu,
	linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel

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 <qiufang.dai@amlogic.com>
>> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
>> ---
>>  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 <mturquette@baylibre.com>
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
>> + */
>> +#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 <narmstrong@baylibre.com>
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
>> + */
>> +
>> +#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 */
> 
> .
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver
@ 2018-03-23 14:13       ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-03-23 14:13 UTC (permalink / raw)
  To: linux-arm-kernel

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 <qiufang.dai@amlogic.com>
>> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
>> ---
>>  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 <mturquette@baylibre.com>
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
>> + */
>> +#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 <narmstrong@baylibre.com>
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
>> + */
>> +
>> +#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 */
> 
> .
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver
@ 2018-03-23 14:13       ` Yixun Lan
  0 siblings, 0 replies; 26+ messages in thread
From: Yixun Lan @ 2018-03-23 14:13 UTC (permalink / raw)
  To: linus-amlogic

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 <qiufang.dai@amlogic.com>
>> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
>> ---
>>  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 <mturquette@baylibre.com>
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
>> + */
>> +#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 <narmstrong@baylibre.com>
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Qiufang Dai <qiufang.dai@amlogic.com>
>> + */
>> +
>> +#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 */
> 
> .
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2018-03-23 14:21 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-09  7:00 [PATCH 0/2] clk: meson-axg: Add AO Cloclk and Reset driver Yixun Lan
2018-02-09  7:00 ` Yixun Lan
2018-02-09  7:00 ` Yixun Lan
2018-02-09  7:00 ` [PATCH 1/2] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings Yixun Lan
2018-02-09  7:00   ` Yixun Lan
2018-02-09  7:00   ` Yixun Lan
2018-02-09  7:00   ` Yixun Lan
2018-02-09 11:07   ` Philippe Ombredanne
2018-02-09 11:07     ` Philippe Ombredanne
2018-02-09 11:07     ` Philippe Ombredanne
2018-03-23 14:06     ` Yixun Lan
2018-03-23 14:06       ` Yixun Lan
2018-03-23 14:06       ` Yixun Lan
2018-03-23 14:06       ` Yixun Lan
2018-02-09  7:00 ` [PATCH 2/2] clk: meson-axg: Add AO Clock and Reset controller driver Yixun Lan
2018-02-09  7:00   ` Yixun Lan
2018-02-09  7:00   ` Yixun Lan
2018-02-12 15:32   ` Jerome Brunet
2018-02-12 15:32     ` Jerome Brunet
2018-02-12 15:32     ` Jerome Brunet
2018-02-13 14:51     ` Jerome Brunet
2018-02-13 14:51       ` Jerome Brunet
2018-02-13 14:51       ` Jerome Brunet
2018-03-23 14:13     ` Yixun Lan
2018-03-23 14:13       ` Yixun Lan
2018-03-23 14:13       ` Yixun Lan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.