linux-amlogic.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND v9 0/4] clk: meson: add a sub EMMC clock controller support
@ 2019-01-08 13:50 Jianxin Pan
  2019-01-08 13:50 ` [PATCH v9 1/4] clk: meson: add one based divider support for sclk divider Jianxin Pan
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Jianxin Pan @ 2019-01-08 13:50 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong
  Cc: Rob Herring, Hanjie Lin, Victor Wan, Jianxin Pan, devicetree,
	Stephen Boyd, Kevin Hilman, Michael Turquette, Yixun Lan,
	linux-kernel, Boris Brezillon, Liang Yang, Jian Hu,
	Miquel Raynal, Carlo Caione, linux-amlogic, Martin Blumenstingl,
	linux-clk, linux-arm-kernel, Qiufang Dai

This driver will add a MMC clock controller driver support.
The original idea about adding a clock controller is during the
discussion in the NAND driver mainline effort[1].

This driver is tested in the S400 board (AXG platform) with NAND driver.


Changes since v8 [9]
 - fix auto build test ERROR with ARCH=i386

Changes since v7 [8]
 - move meson_clk_get_phase_delay_data() from header to driver
 - CONFIG sclk-div with COMMON_CLK_AMLOGIC instead of COMMON_CLK_AMLOGIC_AUDIO
 - remove onecell date and ID for internal MUX clk
 - use helper for functions for ONE_BASED in sclk-div
 - add ONE_BASED support for duty cycle

Changes since v6 [7]:
 - add one based support for sclk divier
 - alloc sclk in probe for multiple instance
 - fix coding styles

Changes since v5 [6]:
 - remove divider ops with .init and use sclk_div instead
 - drop CLK_DIVIDER_ROUND_CLOSEST in mux and div
 - drop the useless type cast 

Changes since v4 [5]:
 - use struct parm in phase delay driver
 - remove 0 delay releted part in phase delay driver
 - don't rebuild the parent name once again
 - add divider ops with .init

Changes since v3 [4]:
 - separate clk-phase-delay driver
 - replace clk_get_rate() with clk_hw_get_rate()
 - collect Rob's R-Y
 - drop 'meson-' prefix from compatible string

 Changes since v2 [3]:
 - squash dt-binding clock-id patch
 - update license
 - fix alignment
 - construct a clk register helper() function

Changes since v1 [2]:
 - implement phase clock
 - update compatible name
 - adjust file name
 - divider probe() into small functions, and re-use them

[1] https://lkml.kernel.org/r/20180628090034.0637a062@xps13
[2] https://lkml.kernel.org/r/20180703145716.31860-1-yixun.lan@amlogic.com
[3] https://lkml.kernel.org/r/20180710163658.6175-1-yixun.lan@amlogic.com
[4] https://lkml.kernel.org/r/20180712211244.11428-1-yixun.lan@amlogic.com
[5] https://lkml.kernel.org/r/20180809070724.11935-4-yixun.lan@amlogic.com
[6] https://lkml.kernel.org/r/1539839245-13793-1-git-send-email-jianxin.pan@amlogic.com
[7] https://lkml.kernel.org/r/1541089855-19356-1-git-send-email-jianxin.pan@amlogic.com
[8] https://lkml.kernel.org/r/1544457877-51301-1-git-send-email-jianxin.pan@amlogic.com
[9] https://lkml.kernel.org/r/1545063850-21504-1-git-send-email-jianxin.pan@amlogic.com
Jianxin Pan (1):
  clk: meson: add one based divider support for sclk divider

Yixun Lan (3):
  clk: meson: add emmc sub clock phase delay driver
  clk: meson: add DT documentation for emmc clock controller
  clk: meson: add sub MMC clock controller driver

 .../devicetree/bindings/clock/amlogic,mmc-clkc.txt |  39 +++
 drivers/clk/meson/Kconfig                          |  10 +
 drivers/clk/meson/Makefile                         |   5 +-
 drivers/clk/meson/clk-phase-delay.c                |  73 +++++
 drivers/clk/meson/clkc-audio.h                     |   8 -
 drivers/clk/meson/clkc.h                           |  17 +-
 drivers/clk/meson/mmc-clkc.c                       | 304 +++++++++++++++++++++
 drivers/clk/meson/sclk-div.c                       |  59 ++--
 include/dt-bindings/clock/amlogic,mmc-clkc.h       |  17 ++
 9 files changed, 502 insertions(+), 30 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
 create mode 100644 drivers/clk/meson/clk-phase-delay.c
 create mode 100644 drivers/clk/meson/mmc-clkc.c
 create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h

-- 
1.9.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* [PATCH v9 1/4] clk: meson: add one based divider support for sclk divider
  2019-01-08 13:50 [PATCH RESEND v9 0/4] clk: meson: add a sub EMMC clock controller support Jianxin Pan
@ 2019-01-08 13:50 ` Jianxin Pan
  2019-01-22  9:25   ` Jerome Brunet
  2019-01-08 13:50 ` [PATCH v9 2/4] clk: meson: add emmc sub clock phase delay driver Jianxin Pan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Jianxin Pan @ 2019-01-08 13:50 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong
  Cc: Rob Herring, Hanjie Lin, Victor Wan, Jianxin Pan, devicetree,
	Stephen Boyd, Kevin Hilman, Michael Turquette, Yixun Lan,
	linux-kernel, Boris Brezillon, Liang Yang, Jian Hu,
	Miquel Raynal, Carlo Caione, linux-amlogic, Martin Blumenstingl,
	linux-clk, linux-arm-kernel, Qiufang Dai

When CLK_DIVIDER_ONE_BASED flag is set, the sclk divider will be:
one based divider (div = val), and zero value gates the clock

Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
---
 drivers/clk/meson/Makefile     |  3 ++-
 drivers/clk/meson/clkc-audio.h |  8 ------
 drivers/clk/meson/clkc.h       | 10 ++++++-
 drivers/clk/meson/sclk-div.c   | 59 ++++++++++++++++++++++++++++--------------
 4 files changed, 50 insertions(+), 30 deletions(-)

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index a849aa8..acd8694 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -4,7 +4,8 @@
 
 obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-phase.o vid-pll-div.o
 obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-input.o
-obj-$(CONFIG_COMMON_CLK_AMLOGIC_AUDIO)	+= clk-triphase.o sclk-div.o
+obj-$(CONFIG_COMMON_CLK_AMLOGIC) += sclk-div.o
+obj-$(CONFIG_COMMON_CLK_AMLOGIC_AUDIO) += clk-triphase.o
 obj-$(CONFIG_COMMON_CLK_MESON_AO) += meson-aoclk.o
 obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
 obj-$(CONFIG_COMMON_CLK_GXBB)	 += gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
diff --git a/drivers/clk/meson/clkc-audio.h b/drivers/clk/meson/clkc-audio.h
index 0a7c157..286ff12 100644
--- a/drivers/clk/meson/clkc-audio.h
+++ b/drivers/clk/meson/clkc-audio.h
@@ -15,14 +15,6 @@ struct meson_clk_triphase_data {
 	struct parm ph2;
 };
 
-struct meson_sclk_div_data {
-	struct parm div;
-	struct parm hi;
-	unsigned int cached_div;
-	struct clk_duty cached_duty;
-};
-
 extern const struct clk_ops meson_clk_triphase_ops;
-extern const struct clk_ops meson_sclk_div_ops;
 
 #endif /* __MESON_CLKC_AUDIO_H */
diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h
index 6183b22..00b3320 100644
--- a/drivers/clk/meson/clkc.h
+++ b/drivers/clk/meson/clkc.h
@@ -27,6 +27,14 @@ struct parm {
 	u8	width;
 };
 
+struct meson_sclk_div_data {
+	struct parm div;
+	struct parm hi;
+	unsigned int cached_div;
+	struct clk_duty cached_duty;
+	u8 flags;
+};
+
 static inline unsigned int meson_parm_read(struct regmap *map, struct parm *p)
 {
 	unsigned int val;
@@ -118,10 +126,10 @@ struct clk_regmap _name = {						\
 extern const struct clk_ops meson_clk_mpll_ops;
 extern const struct clk_ops meson_clk_phase_ops;
 extern const struct clk_ops meson_vid_pll_div_ro_ops;
+extern const struct clk_ops meson_sclk_div_ops;
 
 struct clk_hw *meson_clk_hw_register_input(struct device *dev,
 					   const char *of_name,
 					   const char *clk_name,
 					   unsigned long flags);
-
 #endif /* __CLKC_H */
diff --git a/drivers/clk/meson/sclk-div.c b/drivers/clk/meson/sclk-div.c
index bc64019..a6c425b 100644
--- a/drivers/clk/meson/sclk-div.c
+++ b/drivers/clk/meson/sclk-div.c
@@ -4,42 +4,60 @@
  * Author: Jerome Brunet <jbrunet@baylibre.com>
  *
  * Sample clock generator divider:
- * This HW divider gates with value 0 but is otherwise a zero based divider:
+ * This HW divider gates with value 0
  *
  * val >= 1
- * divider = val + 1
+ * divider = val + 1 if ONE_BASED is not set, otherwise divider = val.
  *
  * The duty cycle may also be set for the LR clock variant. The duty cycle
  * ratio is:
  *
  * hi = [0 - val]
- * duty_cycle = (1 + hi) / (1 + val)
+ * duty_cycle = (1 + hi) / (1 + val) if ONE_BASED is not set, otherwise:
+ * duty_cycle = hi / (1 + val)
  */
 
-#include "clkc-audio.h"
+#include "clkc.h"
 
-static inline struct meson_sclk_div_data *
-meson_sclk_div_data(struct clk_regmap *clk)
+static inline int get_reg(int val, unsigned char flag)
 {
-	return (struct meson_sclk_div_data *)clk->data;
+	WARN_ON(val < 1);
+	if ((flag & CLK_DIVIDER_ONE_BASED) || !val)
+		return val;
+	else
+		return val - 1;
+}
+
+static inline int get_value(int reg, unsigned char flag)
+{
+	if (flag & CLK_DIVIDER_ONE_BASED)
+		return reg;
+	else
+		return reg + 1;
 }
 
-static int sclk_div_maxval(struct meson_sclk_div_data *sclk)
+static inline struct meson_sclk_div_data *
+meson_sclk_div_data(struct clk_regmap *clk)
 {
-	return (1 << sclk->div.width) - 1;
+	return (struct meson_sclk_div_data *)clk->data;
 }
 
 static int sclk_div_maxdiv(struct meson_sclk_div_data *sclk)
 {
-	return sclk_div_maxval(sclk) + 1;
+	unsigned int reg = clk_div_mask(sclk->div.width);
+
+	return get_value(reg, sclk->flags);
 }
 
 static int sclk_div_getdiv(struct clk_hw *hw, unsigned long rate,
 			   unsigned long prate, int maxdiv)
 {
 	int div = DIV_ROUND_CLOSEST_ULL((u64)prate, rate);
+	struct clk_regmap *clk = to_clk_regmap(hw);
+	struct meson_sclk_div_data *sclk = meson_sclk_div_data(clk);
+	int mindiv = get_value(1, sclk->flags);
 
-	return clamp(div, 2, maxdiv);
+	return clamp(div, mindiv, maxdiv);
 }
 
 static int sclk_div_bestdiv(struct clk_hw *hw, unsigned long rate,
@@ -47,7 +65,7 @@ static int sclk_div_bestdiv(struct clk_hw *hw, unsigned long rate,
 			    struct meson_sclk_div_data *sclk)
 {
 	struct clk_hw *parent = clk_hw_get_parent(hw);
-	int bestdiv = 0, i;
+	int bestdiv = 0, i, mindiv;
 	unsigned long maxdiv, now, parent_now;
 	unsigned long best = 0, best_parent = 0;
 
@@ -64,8 +82,9 @@ static int sclk_div_bestdiv(struct clk_hw *hw, unsigned long rate,
 	 * unsigned long in rate * i below
 	 */
 	maxdiv = min(ULONG_MAX / rate, maxdiv);
+	mindiv = get_value(1, sclk->flags);
 
-	for (i = 2; i <= maxdiv; i++) {
+	for (i = mindiv; i <= maxdiv; i++) {
 		/*
 		 * It's the most ideal case if the requested rate can be
 		 * divided from parent clock without needing to change
@@ -111,10 +130,7 @@ static void sclk_apply_ratio(struct clk_regmap *clk,
 					    sclk->cached_duty.num,
 					    sclk->cached_duty.den);
 
-	if (hi)
-		hi -= 1;
-
-	meson_parm_write(clk->map, &sclk->hi, hi);
+	meson_parm_write(clk->map, &sclk->hi, get_reg(hi, sclk->flags));
 }
 
 static int sclk_div_set_duty_cycle(struct clk_hw *hw,
@@ -145,7 +161,7 @@ static int sclk_div_get_duty_cycle(struct clk_hw *hw,
 	}
 
 	hi = meson_parm_read(clk->map, &sclk->hi);
-	duty->num = hi + 1;
+	duty->num = get_value(hi, sclk->flags);
 	duty->den = sclk->cached_div;
 	return 0;
 }
@@ -153,10 +169,13 @@ static int sclk_div_get_duty_cycle(struct clk_hw *hw,
 static void sclk_apply_divider(struct clk_regmap *clk,
 			       struct meson_sclk_div_data *sclk)
 {
+	unsigned int div;
+
 	if (MESON_PARM_APPLICABLE(&sclk->hi))
 		sclk_apply_ratio(clk, sclk);
 
-	meson_parm_write(clk->map, &sclk->div, sclk->cached_div - 1);
+	div = get_reg(sclk->cached_div, sclk->flags);
+	meson_parm_write(clk->map, &sclk->div, div);
 }
 
 static int sclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -224,7 +243,7 @@ static void sclk_div_init(struct clk_hw *hw)
 	if (!val)
 		sclk->cached_div = sclk_div_maxdiv(sclk);
 	else
-		sclk->cached_div = val + 1;
+		sclk->cached_div = get_value(val, sclk->flags);
 
 	sclk_div_get_duty_cycle(hw, &sclk->cached_duty);
 }
-- 
1.9.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* [PATCH v9 2/4] clk: meson: add emmc sub clock phase delay driver
  2019-01-08 13:50 [PATCH RESEND v9 0/4] clk: meson: add a sub EMMC clock controller support Jianxin Pan
  2019-01-08 13:50 ` [PATCH v9 1/4] clk: meson: add one based divider support for sclk divider Jianxin Pan
@ 2019-01-08 13:50 ` Jianxin Pan
  2019-01-08 13:50 ` [PATCH v9 3/4] clk: meson: add DT documentation for emmc clock controller Jianxin Pan
  2019-01-08 13:50 ` [PATCH v9 4/4] clk: meson: add sub MMC clock controller driver Jianxin Pan
  3 siblings, 0 replies; 9+ messages in thread
From: Jianxin Pan @ 2019-01-08 13:50 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong
  Cc: Rob Herring, Hanjie Lin, Victor Wan, Jianxin Pan, Stephen Boyd,
	Kevin Hilman, Michael Turquette, Yixun Lan, linux-kernel,
	Boris Brezillon, Liang Yang, Jian Hu, Miquel Raynal,
	Carlo Caione, linux-amlogic, Martin Blumenstingl, linux-clk,
	linux-arm-kernel, Qiufang Dai

From: Yixun Lan <yixun.lan@amlogic.com>

Export the emmc sub clock phase delay ops which will be used
by the emmc sub clock driver itself.

Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
---
 drivers/clk/meson/Makefile          |  1 +
 drivers/clk/meson/clk-phase-delay.c | 73 +++++++++++++++++++++++++++++++++++++
 drivers/clk/meson/clkc.h            |  7 ++++
 3 files changed, 81 insertions(+)
 create mode 100644 drivers/clk/meson/clk-phase-delay.c

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index acd8694..d59620d 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -3,6 +3,7 @@
 #
 
 obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-phase.o vid-pll-div.o
+obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-phase-delay.o
 obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-input.o
 obj-$(CONFIG_COMMON_CLK_AMLOGIC) += sclk-div.o
 obj-$(CONFIG_COMMON_CLK_AMLOGIC_AUDIO) += clk-triphase.o
diff --git a/drivers/clk/meson/clk-phase-delay.c b/drivers/clk/meson/clk-phase-delay.c
new file mode 100644
index 0000000..a6e6600
--- /dev/null
+++ b/drivers/clk/meson/clk-phase-delay.c
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Amlogic Meson MMC Sub Clock Controller Driver
+ *
+ * Copyright (c) 2017 Baylibre SAS.
+ * Author: Jerome Brunet <jbrunet@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Yixun Lan <yixun.lan@amlogic.com>
+ * Author: Jianxin Pan <jianxin.pan@amlogic.com>
+ */
+
+#include <linux/clk-provider.h>
+#include "clkc.h"
+
+static inline struct meson_clk_phase_delay_data *
+meson_clk_get_phase_delay_data(struct clk_regmap *clk)
+{
+	return clk->data;
+}
+
+static int meson_clk_phase_delay_get_phase(struct clk_hw *hw)
+{
+	struct clk_regmap *clk = to_clk_regmap(hw);
+	struct meson_clk_phase_delay_data *ph;
+	unsigned long period_ps, p, d;
+	int degrees;
+
+	ph = meson_clk_get_phase_delay_data(clk);
+	p = meson_parm_read(clk->map, &ph->phase);
+	degrees = p * 360 / (1 << (ph->phase.width));
+
+	period_ps = DIV_ROUND_UP_ULL(NSEC_PER_SEC * 1000ull,
+				     clk_hw_get_rate(hw));
+
+	d = meson_parm_read(clk->map, &ph->delay);
+	degrees += d * ph->delay_step_ps * 360 / period_ps;
+	degrees %= 360;
+
+	return degrees;
+}
+
+static int meson_clk_phase_delay_set_phase(struct clk_hw *hw, int degrees)
+{
+	struct clk_regmap *clk = to_clk_regmap(hw);
+	struct meson_clk_phase_delay_data *ph;
+	unsigned long period_ps, d = 0;
+	unsigned int p;
+
+	ph = meson_clk_get_phase_delay_data(clk);
+	period_ps = DIV_ROUND_UP_ULL(NSEC_PER_SEC * 1000ull,
+				     clk_hw_get_rate(hw));
+
+	/*
+	 * First compute the phase index (p), the remainder (r) is the
+	 * part we'll try to acheive using the delays (d).
+	 */
+	p = 360 / 1 << (ph->phase.width);
+	degrees = degrees / p;
+	d = DIV_ROUND_CLOSEST((degrees % p) * period_ps,
+			      360 * ph->delay_step_ps);
+	d = min(d, PMASK(ph->delay.width));
+
+	meson_parm_write(clk->map, &ph->phase, degrees);
+	meson_parm_write(clk->map, &ph->delay, d);
+	return 0;
+}
+
+const struct clk_ops meson_clk_phase_delay_ops = {
+	.get_phase = meson_clk_phase_delay_get_phase,
+	.set_phase = meson_clk_phase_delay_set_phase,
+};
+EXPORT_SYMBOL_GPL(meson_clk_phase_delay_ops);
diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h
index 00b3320..e6f0905 100644
--- a/drivers/clk/meson/clkc.h
+++ b/drivers/clk/meson/clkc.h
@@ -118,6 +118,12 @@ struct clk_regmap _name = {						\
 	},								\
 };
 
+struct meson_clk_phase_delay_data {
+	struct parm	phase;
+	struct parm	delay;
+	unsigned int	delay_step_ps;
+};
+
 /* clk_ops */
 extern const struct clk_ops meson_clk_pll_ro_ops;
 extern const struct clk_ops meson_clk_pll_ops;
@@ -127,6 +133,7 @@ struct clk_regmap _name = {						\
 extern const struct clk_ops meson_clk_phase_ops;
 extern const struct clk_ops meson_vid_pll_div_ro_ops;
 extern const struct clk_ops meson_sclk_div_ops;
+extern const struct clk_ops meson_clk_phase_delay_ops;
 
 struct clk_hw *meson_clk_hw_register_input(struct device *dev,
 					   const char *of_name,
-- 
1.9.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* [PATCH v9 3/4] clk: meson: add DT documentation for emmc clock controller
  2019-01-08 13:50 [PATCH RESEND v9 0/4] clk: meson: add a sub EMMC clock controller support Jianxin Pan
  2019-01-08 13:50 ` [PATCH v9 1/4] clk: meson: add one based divider support for sclk divider Jianxin Pan
  2019-01-08 13:50 ` [PATCH v9 2/4] clk: meson: add emmc sub clock phase delay driver Jianxin Pan
@ 2019-01-08 13:50 ` Jianxin Pan
  2019-01-22  9:27   ` Jerome Brunet
  2019-01-08 13:50 ` [PATCH v9 4/4] clk: meson: add sub MMC clock controller driver Jianxin Pan
  3 siblings, 1 reply; 9+ messages in thread
From: Jianxin Pan @ 2019-01-08 13:50 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong
  Cc: Rob Herring, Hanjie Lin, Victor Wan, Jianxin Pan, Stephen Boyd,
	Kevin Hilman, Michael Turquette, Yixun Lan, linux-kernel,
	Boris Brezillon, Liang Yang, Jian Hu, Miquel Raynal,
	Carlo Caione, linux-amlogic, Martin Blumenstingl, linux-clk,
	linux-arm-kernel, Qiufang Dai

From: Yixun Lan <yixun.lan@amlogic.com>

Document the MMC sub clock controller driver, the potential consumer
of this driver is MMC or NAND. Also add four clock bindings IDs which
provided by this driver.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
---
 .../devicetree/bindings/clock/amlogic,mmc-clkc.txt | 39 ++++++++++++++++++++++
 include/dt-bindings/clock/amlogic,mmc-clkc.h       | 17 ++++++++++
 2 files changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
 create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h

diff --git a/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
new file mode 100644
index 0000000..0f518e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
@@ -0,0 +1,39 @@
+* Amlogic MMC Sub Clock Controller Driver
+
+The Amlogic MMC clock controller generates and supplies clock to support
+MMC and NAND controller
+
+Required Properties:
+
+- compatible: should be:
+		"amlogic,gx-mmc-clkc"
+		"amlogic,axg-mmc-clkc"
+
+- #clock-cells: should be 1.
+- clocks: phandles to clocks corresponding to the clock-names property
+- clock-names: list of parent clock names
+	- "clkin0", "clkin1"
+
+- reg: address of emmc sub clock register
+
+Example: Clock controller node:
+
+sd_mmc_c_clkc: clock-controller@7000 {
+	compatible = "amlogic,axg-mmc-clkc", "syscon";
+	reg = <0x0 0x7000 0x0 0x4>;
+	#clock-cells = <1>;
+
+	clock-names = "clkin0", "clkin1";
+	clocks = <&clkc CLKID_SD_MMC_C_CLK0>,
+		 <&clkc CLKID_FCLK_DIV2>;
+};
+
+sd_emmc_b_clkc: clock-controller@5000 {
+	compatible = "amlogic,axg-mmc-clkc", "syscon";
+	reg = <0x0 0x5000 0x0 0x4>;
+
+	#clock-cells = <1>;
+	clock-names = "clkin0", "clkin1";
+	clocks = <&clkc CLKID_SD_EMMC_B_CLK0>,
+		 <&clkc CLKID_FCLK_DIV2>;
+};
diff --git a/include/dt-bindings/clock/amlogic,mmc-clkc.h b/include/dt-bindings/clock/amlogic,mmc-clkc.h
new file mode 100644
index 0000000..34a3c56
--- /dev/null
+++ b/include/dt-bindings/clock/amlogic,mmc-clkc.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Meson MMC sub clock tree IDs
+ *
+ * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
+ * Author: Yixun Lan <yixun.lan@amlogic.com>
+ */
+
+#ifndef __MMC_CLKC_H
+#define __MMC_CLKC_H
+
+#define CLKID_MMC_DIV		0
+#define CLKID_MMC_PHASE_CORE	1
+#define CLKID_MMC_PHASE_TX	2
+#define CLKID_MMC_PHASE_RX	3
+
+#endif
-- 
1.9.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* [PATCH v9 4/4] clk: meson: add sub MMC clock controller driver
  2019-01-08 13:50 [PATCH RESEND v9 0/4] clk: meson: add a sub EMMC clock controller support Jianxin Pan
                   ` (2 preceding siblings ...)
  2019-01-08 13:50 ` [PATCH v9 3/4] clk: meson: add DT documentation for emmc clock controller Jianxin Pan
@ 2019-01-08 13:50 ` Jianxin Pan
  2019-01-22  9:25   ` Jerome Brunet
  3 siblings, 1 reply; 9+ messages in thread
From: Jianxin Pan @ 2019-01-08 13:50 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong
  Cc: Rob Herring, Hanjie Lin, Victor Wan, Jianxin Pan, Stephen Boyd,
	Kevin Hilman, Michael Turquette, Yixun Lan, linux-kernel,
	Boris Brezillon, Liang Yang, Jian Hu, Miquel Raynal,
	Carlo Caione, linux-amlogic, Martin Blumenstingl, linux-clk,
	linux-arm-kernel, Qiufang Dai

From: Yixun Lan <yixun.lan@amlogic.com>

The patch will add a MMC clock controller driver which used by MMC or NAND,
It provide a mux and divider clock, and three phase clocks - core, tx, tx.

Two clocks are provided as the parent of MMC clock controller from
upper layer clock controller - eg "amlogic,axg-clkc" in AXG platform.

To specify which clock the MMC or NAND driver may consume,
the preprocessor macros in the dt-bindings/clock/amlogic,mmc-clkc.h header
can be used in the device tree sources.

Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
---
 drivers/clk/meson/Kconfig    |  10 ++
 drivers/clk/meson/Makefile   |   1 +
 drivers/clk/meson/mmc-clkc.c | 304 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 315 insertions(+)
 create mode 100644 drivers/clk/meson/mmc-clkc.c

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index efaa70f..3555f9d 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -15,6 +15,16 @@ config COMMON_CLK_MESON_AO
 	select COMMON_CLK_REGMAP_MESON
 	select RESET_CONTROLLER
 
+config COMMON_CLK_MMC_MESON
+	tristate "Meson MMC Sub Clock Controller Driver"
+	depends on ARCH_MESON || COMPILE_TEST
+	select MFD_SYSCON
+	select COMMON_CLK_AMLOGIC
+	help
+	  Support for the MMC sub clock controller on Amlogic Meson Platform,
+	  which include S905 (GXBB, GXL), A113D/X (AXG) devices.
+	  Say Y if you want this clock enabled.
+
 config COMMON_CLK_REGMAP_MESON
 	bool
 	select REGMAP
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index d59620d..54416a2 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -12,4 +12,5 @@ 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 axg-aoclk.o
 obj-$(CONFIG_COMMON_CLK_AXG_AUDIO)	+= axg-audio.o
+obj-$(CONFIG_COMMON_CLK_MMC_MESON) 	+= mmc-clkc.o
 obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)	+= clk-regmap.o
diff --git a/drivers/clk/meson/mmc-clkc.c b/drivers/clk/meson/mmc-clkc.c
new file mode 100644
index 0000000..2582a98
--- /dev/null
+++ b/drivers/clk/meson/mmc-clkc.c
@@ -0,0 +1,304 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Amlogic Meson MMC Sub Clock Controller Driver
+ *
+ * Copyright (c) 2017 Baylibre SAS.
+ * Author: Jerome Brunet <jbrunet@baylibre.com>
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Yixun Lan <yixun.lan@amlogic.com>
+ * Author: Jianxin Pan <jianxin.pan@amlogic.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/of_device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/platform_device.h>
+#include <dt-bindings/clock/amlogic,mmc-clkc.h>
+
+#include "clkc.h"
+
+/* clock ID used by internal driver */
+
+#define SD_EMMC_CLOCK		0
+#define CLK_DELAY_STEP_PS	200
+#define MUX_CLK_NUM_PARENTS	2
+#define MMC_MAX_CLKS		4
+
+struct mmc_clkc_data {
+	struct meson_clk_phase_delay_data tx;
+	struct meson_clk_phase_delay_data rx;
+};
+
+static struct clk_regmap_mux_data mmc_clkc_mux_data = {
+	.offset = SD_EMMC_CLOCK,
+	.mask   = 0x3,
+	.shift  = 6,
+};
+
+static const struct meson_sclk_div_data mmc_clkc_div_data = {
+	.div = {
+		.reg_off = SD_EMMC_CLOCK,
+		.width   = 6,
+	},
+	.flags = CLK_DIVIDER_ONE_BASED,
+};
+
+static struct meson_clk_phase_data mmc_clkc_core_phase = {
+	.ph = {
+		.reg_off = SD_EMMC_CLOCK,
+		.shift   = 8,
+		.width   = 2,
+	}
+};
+
+static const struct mmc_clkc_data mmc_clkc_gx_data = {
+	.tx = {
+		.phase = {
+			.reg_off = SD_EMMC_CLOCK,
+			.shift   = 10,
+			.width   = 2,
+		},
+		.delay = {
+			.reg_off = SD_EMMC_CLOCK,
+			.shift   = 16,
+			.width   = 4,
+		},
+		.delay_step_ps = CLK_DELAY_STEP_PS,
+	},
+	.rx = {
+		.phase = {
+			.reg_off = SD_EMMC_CLOCK,
+			.shift   = 12,
+			.width   = 2,
+		},
+		.delay = {
+			.reg_off = SD_EMMC_CLOCK,
+			.shift   = 20,
+			.width   = 4,
+		},
+		.delay_step_ps   = CLK_DELAY_STEP_PS,
+	},
+};
+
+static const struct mmc_clkc_data mmc_clkc_axg_data = {
+	.tx = {
+		.phase = {
+			.reg_off = SD_EMMC_CLOCK,
+			.shift   = 10,
+			.width   = 2,
+		},
+		.delay = {
+			.reg_off = SD_EMMC_CLOCK,
+			.shift   = 16,
+			.width   = 6,
+		},
+		.delay_step_ps   = CLK_DELAY_STEP_PS,
+	},
+	.rx = {
+		.phase = {
+			.reg_off = SD_EMMC_CLOCK,
+			.shift   = 12,
+			.width   = 2,
+		},
+		.delay = {
+			.reg_off = SD_EMMC_CLOCK,
+			.shift   = 22,
+			.width   = 6,
+		},
+		.delay_step_ps   = CLK_DELAY_STEP_PS,
+	},
+};
+
+static const struct of_device_id mmc_clkc_match_table[] = {
+	{
+		.compatible	= "amlogic,gx-mmc-clkc",
+		.data		= &mmc_clkc_gx_data
+	},
+	{
+		.compatible	= "amlogic,axg-mmc-clkc",
+		.data		= &mmc_clkc_axg_data
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, mmc_clkc_match_table);
+
+static struct clk_regmap *
+mmc_clkc_register_clk(struct device *dev, struct regmap *map,
+		      struct clk_init_data *init,
+		      const char *suffix, void *data)
+{
+	struct clk_regmap *clk;
+	char *name;
+	int ret;
+
+	clk = devm_kzalloc(dev, sizeof(*clk), GFP_KERNEL);
+	if (!clk)
+		return ERR_PTR(-ENOMEM);
+
+	name = kasprintf(GFP_KERNEL, "%s#%s", dev_name(dev), suffix);
+	if (!name)
+		return ERR_PTR(-ENOMEM);
+
+	init->name = name;
+	clk->map = map;
+	clk->data = data;
+	clk->hw.init = init;
+	ret = devm_clk_hw_register(dev, &clk->hw);
+	if (ret)
+		clk = ERR_PTR(ret);
+
+	kfree(name);
+	return clk;
+}
+
+static struct clk_regmap *mmc_clkc_register_mux(struct device *dev,
+						struct regmap *map)
+{
+	const char *parent_names[MUX_CLK_NUM_PARENTS];
+	struct clk_init_data init;
+	struct clk_regmap *mux;
+	struct clk *clk;
+	int i;
+
+	for (i = 0; i < MUX_CLK_NUM_PARENTS; i++) {
+		char name[8];
+
+		snprintf(name, sizeof(name), "clkin%d", i);
+		clk = devm_clk_get(dev, name);
+		if (IS_ERR(clk)) {
+			if (clk != ERR_PTR(-EPROBE_DEFER))
+				dev_err(dev, "Missing clock %s\n", name);
+			return ERR_CAST(clk);
+		}
+
+		parent_names[i] = __clk_get_name(clk);
+	}
+
+	init.ops = &clk_regmap_mux_ops;
+	init.flags = CLK_SET_RATE_PARENT;
+	init.parent_names = parent_names;
+	init.num_parents = MUX_CLK_NUM_PARENTS;
+
+	mux = mmc_clkc_register_clk(dev, map, &init, "mux", &mmc_clkc_mux_data);
+	if (IS_ERR(mux))
+		dev_err(dev, "Mux clock registration failed\n");
+
+	return mux;
+}
+
+static struct clk_regmap *
+mmc_clkc_register_clk_with_parent(struct device *dev, struct regmap *map,
+				  char *suffix, const struct clk_hw *hw,
+				  unsigned long flags,
+				  const struct clk_ops *ops, void *data)
+{
+	struct clk_init_data init;
+	struct clk_regmap *clk;
+	const char *parent_name = clk_hw_get_name(hw);
+
+	init.ops = ops;
+	init.flags = flags;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	clk = mmc_clkc_register_clk(dev, map, &init, suffix, data);
+	if (IS_ERR(clk))
+		dev_err(dev, "%s clock registration failed\n", suffix);
+
+	return clk;
+}
+
+static int mmc_clkc_probe(struct platform_device *pdev)
+{
+	struct clk_hw_onecell_data *onecell_data;
+	struct device *dev = &pdev->dev;
+	struct mmc_clkc_data *data;
+	struct regmap *map;
+	struct clk_regmap *clk, *core;
+	struct meson_sclk_div_data *div_data;
+
+	/*cast to drop the const in match->data*/
+	data = (struct mmc_clkc_data *)of_device_get_match_data(dev);
+	if (!data)
+		return -ENODEV;
+
+	map = syscon_node_to_regmap(dev->of_node);
+	if (IS_ERR(map)) {
+		dev_err(dev, "could not find mmc clock controller\n");
+		return PTR_ERR(map);
+	}
+
+	onecell_data = devm_kzalloc(dev, sizeof(*onecell_data) +
+				    sizeof(*onecell_data->hws) * MMC_MAX_CLKS,
+				    GFP_KERNEL);
+	if (!onecell_data)
+		return -ENOMEM;
+
+	clk = mmc_clkc_register_mux(dev, map);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	div_data = devm_kzalloc(dev, sizeof(*div_data), GFP_KERNEL);
+	if (!div_data)
+		return -ENOMEM;
+
+	memcpy(div_data, &mmc_clkc_div_data, sizeof(*div_data));
+	clk = mmc_clkc_register_clk_with_parent(dev, map, "div",
+						&clk->hw,
+						CLK_SET_RATE_PARENT,
+						&meson_sclk_div_ops,
+						div_data);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	onecell_data->hws[CLKID_MMC_DIV] = &clk->hw,
+
+	core = mmc_clkc_register_clk_with_parent(dev, map, "core",
+						 &clk->hw,
+						 CLK_SET_RATE_PARENT,
+						 &meson_clk_phase_ops,
+						 &mmc_clkc_core_phase);
+	if (IS_ERR(core))
+		return PTR_ERR(core);
+
+	onecell_data->hws[CLKID_MMC_PHASE_CORE] = &core->hw,
+
+	clk = mmc_clkc_register_clk_with_parent(dev, map, "rx",
+						&core->hw,  0,
+						&meson_clk_phase_delay_ops,
+						&data->rx);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	onecell_data->hws[CLKID_MMC_PHASE_RX] = &clk->hw,
+	clk = mmc_clkc_register_clk_with_parent(dev, map, "tx",
+						&core->hw,  0,
+						&meson_clk_phase_delay_ops,
+						&data->tx);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	onecell_data->hws[CLKID_MMC_PHASE_TX] = &clk->hw,
+	onecell_data->num = MMC_MAX_CLKS;
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+					   onecell_data);
+}
+
+static struct platform_driver mmc_clkc_driver = {
+	.probe		= mmc_clkc_probe,
+	.driver		= {
+		.name	= "meson-mmc-clkc",
+		.of_match_table = of_match_ptr(mmc_clkc_match_table),
+	},
+};
+
+module_platform_driver(mmc_clkc_driver);
+
+MODULE_DESCRIPTION("Amlogic AXG MMC clock driver");
+MODULE_AUTHOR("Jianxin Pan <jianxin.pan@amlogic.com>");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH v9 1/4] clk: meson: add one based divider support for sclk divider
  2019-01-08 13:50 ` [PATCH v9 1/4] clk: meson: add one based divider support for sclk divider Jianxin Pan
@ 2019-01-22  9:25   ` Jerome Brunet
  2019-01-24  9:10     ` Jianxin Pan
  0 siblings, 1 reply; 9+ messages in thread
From: Jerome Brunet @ 2019-01-22  9:25 UTC (permalink / raw)
  To: Jianxin Pan, Neil Armstrong
  Cc: Rob Herring, Hanjie Lin, Victor Wan, devicetree, Stephen Boyd,
	Kevin Hilman, Michael Turquette, Yixun Lan, linux-kernel,
	Boris Brezillon, Liang Yang, Jian Hu, Miquel Raynal,
	Carlo Caione, linux-amlogic, Martin Blumenstingl, linux-clk,
	linux-arm-kernel, Qiufang Dai

On Tue, 2019-01-08 at 21:50 +0800, Jianxin Pan wrote:
> When CLK_DIVIDER_ONE_BASED flag is set, the sclk divider will be:
> one based divider (div = val), and zero value gates the clock
> 
> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
> ---
>  drivers/clk/meson/Makefile     |  3 ++-
>  drivers/clk/meson/clkc-audio.h |  8 ------
>  drivers/clk/meson/clkc.h       | 10 ++++++-
>  drivers/clk/meson/sclk-div.c   | 59 ++++++++++++++++++++++++++++-----------
> ---
>  4 files changed, 50 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index a849aa8..acd8694 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -4,7 +4,8 @@
>  
>  obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-phase.o vid-
> pll-div.o
>  obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-input.o
> -obj-$(CONFIG_COMMON_CLK_AMLOGIC_AUDIO)	+= clk-triphase.o sclk-div.o
> +obj-$(CONFIG_COMMON_CLK_AMLOGIC) += sclk-div.o
> +obj-$(CONFIG_COMMON_CLK_AMLOGIC_AUDIO) += clk-triphase.o
>  obj-$(CONFIG_COMMON_CLK_MESON_AO) += meson-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
>  obj-$(CONFIG_COMMON_CLK_GXBB)	 += gxbb.o gxbb-aoclk.o gxbb-aoclk-
> 32k.o
> diff --git a/drivers/clk/meson/clkc-audio.h b/drivers/clk/meson/clkc-audio.h
> index 0a7c157..286ff12 100644
> --- a/drivers/clk/meson/clkc-audio.h
> +++ b/drivers/clk/meson/clkc-audio.h
> @@ -15,14 +15,6 @@ struct meson_clk_triphase_data {
>  	struct parm ph2;
>  };
>  
> -struct meson_sclk_div_data {
> -	struct parm div;
> -	struct parm hi;
> -	unsigned int cached_div;
> -	struct clk_duty cached_duty;
> -};
> -
>  extern const struct clk_ops meson_clk_triphase_ops;
> -extern const struct clk_ops meson_sclk_div_ops;
>  
>  #endif /* __MESON_CLKC_AUDIO_H */
> diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h
> index 6183b22..00b3320 100644
> --- a/drivers/clk/meson/clkc.h
> +++ b/drivers/clk/meson/clkc.h
> @@ -27,6 +27,14 @@ struct parm {
>  	u8	width;
>  };
>  
> +struct meson_sclk_div_data {
> +	struct parm div;
> +	struct parm hi;
> +	unsigned int cached_div;
> +	struct clk_duty cached_duty;
> +	u8 flags;
> +};
> +
>  static inline unsigned int meson_parm_read(struct regmap *map, struct parm
> *p)
>  {
>  	unsigned int val;
> @@ -118,10 +126,10 @@ struct clk_regmap _name = {				
> 		\
>  extern const struct clk_ops meson_clk_mpll_ops;
>  extern const struct clk_ops meson_clk_phase_ops;
>  extern const struct clk_ops meson_vid_pll_div_ro_ops;
> +extern const struct clk_ops meson_sclk_div_ops;
>  
>  struct clk_hw *meson_clk_hw_register_input(struct device *dev,
>  					   const char *of_name,
>  					   const char *clk_name,
>  					   unsigned long flags);
> -
>  #endif /* __CLKC_H */
> diff --git a/drivers/clk/meson/sclk-div.c b/drivers/clk/meson/sclk-div.c
> index bc64019..a6c425b 100644
> --- a/drivers/clk/meson/sclk-div.c
> +++ b/drivers/clk/meson/sclk-div.c
> @@ -4,42 +4,60 @@
>   * Author: Jerome Brunet <jbrunet@baylibre.com>
>   *
>   * Sample clock generator divider:
> - * This HW divider gates with value 0 but is otherwise a zero based
> divider:
> + * This HW divider gates with value 0
>   *
>   * val >= 1
> - * divider = val + 1
> + * divider = val + 1 if ONE_BASED is not set, otherwise divider = val.
>   *
>   * The duty cycle may also be set for the LR clock variant. The duty cycle
>   * ratio is:
>   *
>   * hi = [0 - val]
> - * duty_cycle = (1 + hi) / (1 + val)
> + * duty_cycle = (1 + hi) / (1 + val) if ONE_BASED is not set, otherwise:
> + * duty_cycle = hi / (1 + val)
>   */
>  
> -#include "clkc-audio.h"
> +#include "clkc.h"
>  
> -static inline struct meson_sclk_div_data *
> -meson_sclk_div_data(struct clk_regmap *clk)
> +static inline int get_reg(int val, unsigned char flag)

s/get_reg/sclk_get_reg

>  {
> -	return (struct meson_sclk_div_data *)clk->data;
> +	WARN_ON(val < 1);

I don't think this WARN is justified, especially since you are using this
function to set the hi value.

> +	if ((flag & CLK_DIVIDER_ONE_BASED) || !val)

I don't like that you make a dependency on the generic divider just for this.
Please make your own flag like MESON_SCLK_ONE_BASED

> +		return val;
> +	else
> +		return val - 1;
> +}
> +
> +static inline int get_value(int reg, unsigned char flag)

s/get_value/sclk_get_divider

> +{
> +	if (flag & CLK_DIVIDER_ONE_BASED)
> +		return reg;
> +	else
> +		return reg + 1;
>  }
>  
> -static int sclk_div_maxval(struct meson_sclk_div_data *sclk)
> +static inline struct meson_sclk_div_data *
> +meson_sclk_div_data(struct clk_regmap *clk)
>  {
> -	return (1 << sclk->div.width) - 1;
> +	return (struct meson_sclk_div_data *)clk->data;
>  }
>  
>  static int sclk_div_maxdiv(struct meson_sclk_div_data *sclk)
>  {
> -	return sclk_div_maxval(sclk) + 1;
> +	unsigned int reg = clk_div_mask(sclk->div.width);

same here, leave the generic divider alone.
 
`(1 << sclk->div.width) - 1` was fine to get max register value.

> +
> +	return get_value(reg, sclk->flags);
>  }
>  
>  static int sclk_div_getdiv(struct clk_hw *hw, unsigned long rate,
>  			   unsigned long prate, int maxdiv)
>  {
>  	int div = DIV_ROUND_CLOSEST_ULL((u64)prate, rate);
> +	struct clk_regmap *clk = to_clk_regmap(hw);
> +	struct meson_sclk_div_data *sclk = meson_sclk_div_data(clk);
> +	int mindiv = get_value(1, sclk->flags);

maxdiv is provided as a param while mindiv is computed inside this function.
let's be coherent and pick one approach.

>  
> -	return clamp(div, 2, maxdiv);
> +	return clamp(div, mindiv, maxdiv);
>  }
>  
>  static int sclk_div_bestdiv(struct clk_hw *hw, unsigned long rate,
> @@ -47,7 +65,7 @@ static int sclk_div_bestdiv(struct clk_hw *hw, unsigned
> long rate,
>  			    struct meson_sclk_div_data *sclk)
>  {
>  	struct clk_hw *parent = clk_hw_get_parent(hw);
> -	int bestdiv = 0, i;
> +	int bestdiv = 0, i, mindiv;
>  	unsigned long maxdiv, now, parent_now;
>  	unsigned long best = 0, best_parent = 0;
>  
> @@ -64,8 +82,9 @@ static int sclk_div_bestdiv(struct clk_hw *hw, unsigned
> long rate,
>  	 * unsigned long in rate * i below
>  	 */
>  	maxdiv = min(ULONG_MAX / rate, maxdiv);
> +	mindiv = get_value(1, sclk->flags);
>  
> -	for (i = 2; i <= maxdiv; i++) {
> +	for (i = mindiv; i <= maxdiv; i++) {
>  		/*
>  		 * It's the most ideal case if the requested rate can be
>  		 * divided from parent clock without needing to change
> @@ -111,10 +130,7 @@ static void sclk_apply_ratio(struct clk_regmap *clk,
>  					    sclk->cached_duty.num,
>  					    sclk->cached_duty.den);
>  
> -	if (hi)
> -		hi -= 1;
> -
> -	meson_parm_write(clk->map, &sclk->hi, hi);
> +	meson_parm_write(clk->map, &sclk->hi, get_reg(hi, sclk->flags));
>  }
>  
>  static int sclk_div_set_duty_cycle(struct clk_hw *hw,
> @@ -145,7 +161,7 @@ static int sclk_div_get_duty_cycle(struct clk_hw *hw,
>  	}
>  
>  	hi = meson_parm_read(clk->map, &sclk->hi);
> -	duty->num = hi + 1;
> +	duty->num = get_value(hi, sclk->flags);
>  	duty->den = sclk->cached_div;
>  	return 0;
>  }
> @@ -153,10 +169,13 @@ static int sclk_div_get_duty_cycle(struct clk_hw *hw,
>  static void sclk_apply_divider(struct clk_regmap *clk,
>  			       struct meson_sclk_div_data *sclk)
>  {
> +	unsigned int div;
> +
>  	if (MESON_PARM_APPLICABLE(&sclk->hi))
>  		sclk_apply_ratio(clk, sclk);
>  
> -	meson_parm_write(clk->map, &sclk->div, sclk->cached_div - 1);
> +	div = get_reg(sclk->cached_div, sclk->flags);
> +	meson_parm_write(clk->map, &sclk->div, div);
>  }
>  
>  static int sclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
> @@ -224,7 +243,7 @@ static void sclk_div_init(struct clk_hw *hw)
>  	if (!val)
>  		sclk->cached_div = sclk_div_maxdiv(sclk);
>  	else
> -		sclk->cached_div = val + 1;
> +		sclk->cached_div = get_value(val, sclk->flags);
>  
>  	sclk_div_get_duty_cycle(hw, &sclk->cached_duty);
>  }



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH v9 4/4] clk: meson: add sub MMC clock controller driver
  2019-01-08 13:50 ` [PATCH v9 4/4] clk: meson: add sub MMC clock controller driver Jianxin Pan
@ 2019-01-22  9:25   ` Jerome Brunet
  0 siblings, 0 replies; 9+ messages in thread
From: Jerome Brunet @ 2019-01-22  9:25 UTC (permalink / raw)
  To: Jianxin Pan, Neil Armstrong
  Cc: Rob Herring, Hanjie Lin, Victor Wan, Stephen Boyd, Kevin Hilman,
	Michael Turquette, Yixun Lan, linux-kernel, Boris Brezillon,
	Liang Yang, Jian Hu, Miquel Raynal, Carlo Caione, linux-amlogic,
	Martin Blumenstingl, linux-clk, linux-arm-kernel, Qiufang Dai

On Tue, 2019-01-08 at 21:50 +0800, Jianxin Pan wrote:
> From: Yixun Lan <yixun.lan@amlogic.com>
> 
> The patch will add a MMC clock controller driver which used by MMC or NAND,
> It provide a mux and divider clock, and three phase clocks - core, tx, tx.
> 
> Two clocks are provided as the parent of MMC clock controller from
> upper layer clock controller - eg "amlogic,axg-clkc" in AXG platform.
> 
> To specify which clock the MMC or NAND driver may consume,
> the preprocessor macros in the dt-bindings/clock/amlogic,mmc-clkc.h header
> can be used in the device tree sources.
> 
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
> ---
>  drivers/clk/meson/Kconfig    |  10 ++
>  drivers/clk/meson/Makefile   |   1 +
>  drivers/clk/meson/mmc-clkc.c | 304
> +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 315 insertions(+)
>  create mode 100644 drivers/clk/meson/mmc-clkc.c
> 
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index efaa70f..3555f9d 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -15,6 +15,16 @@ config COMMON_CLK_MESON_AO
>  	select COMMON_CLK_REGMAP_MESON
>  	select RESET_CONTROLLER
>  
> +config COMMON_CLK_MMC_MESON
> +	tristate "Meson MMC Sub Clock Controller Driver"
> +	depends on ARCH_MESON || COMPILE_TEST
> +	select MFD_SYSCON
> +	select COMMON_CLK_AMLOGIC
> +	help
> +	  Support for the MMC sub clock controller on Amlogic Meson Platform,
> +	  which include S905 (GXBB, GXL), A113D/X (AXG) devices.
> +	  Say Y if you want this clock enabled.
> +
>  config COMMON_CLK_REGMAP_MESON
>  	bool
>  	select REGMAP
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index d59620d..54416a2 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -12,4 +12,5 @@ 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 axg-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_AXG_AUDIO)	+= axg-audio.o
> +obj-$(CONFIG_COMMON_CLK_MMC_MESON) 	+= mmc-clkc.o
>  obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)	+= clk-regmap.o
> diff --git a/drivers/clk/meson/mmc-clkc.c b/drivers/clk/meson/mmc-clkc.c
> new file mode 100644
> index 0000000..2582a98
> --- /dev/null
> +++ b/drivers/clk/meson/mmc-clkc.c
> @@ -0,0 +1,304 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Amlogic Meson MMC Sub Clock Controller Driver
> + *
> + * Copyright (c) 2017 Baylibre SAS.
> + * Author: Jerome Brunet <jbrunet@baylibre.com>
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Yixun Lan <yixun.lan@amlogic.com>
> + * Author: Jianxin Pan <jianxin.pan@amlogic.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/of_device.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/platform_device.h>
> +#include <dt-bindings/clock/amlogic,mmc-clkc.h>
> +
> +#include "clkc.h"
> +
> +/* clock ID used by internal driver */
> +
> +#define SD_EMMC_CLOCK		0
> +#define CLK_DELAY_STEP_PS	200
> +#define MUX_CLK_NUM_PARENTS	2
> +#define MMC_MAX_CLKS		4
> +
> +struct mmc_clkc_data {
> +	struct meson_clk_phase_delay_data tx;
> +	struct meson_clk_phase_delay_data rx;
> +};
> +
> +static struct clk_regmap_mux_data mmc_clkc_mux_data = {
> +	.offset = SD_EMMC_CLOCK,
> +	.mask   = 0x3,
> +	.shift  = 6,
> +};
> +
> +static const struct meson_sclk_div_data mmc_clkc_div_data = {
> +	.div = {
> +		.reg_off = SD_EMMC_CLOCK,
> +		.width   = 6,
> +	},
> +	.flags = CLK_DIVIDER_ONE_BASED,
> +};
> +
> +static struct meson_clk_phase_data mmc_clkc_core_phase = {
> +	.ph = {
> +		.reg_off = SD_EMMC_CLOCK,
> +		.shift   = 8,
> +		.width   = 2,
> +	}
> +};
> +
> +static const struct mmc_clkc_data mmc_clkc_gx_data = {
> +	.tx = {
> +		.phase = {
> +			.reg_off = SD_EMMC_CLOCK,
> +			.shift   = 10,
> +			.width   = 2,
> +		},
> +		.delay = {
> +			.reg_off = SD_EMMC_CLOCK,
> +			.shift   = 16,
> +			.width   = 4,
> +		},
> +		.delay_step_ps = CLK_DELAY_STEP_PS,
> +	},
> +	.rx = {
> +		.phase = {
> +			.reg_off = SD_EMMC_CLOCK,
> +			.shift   = 12,
> +			.width   = 2,
> +		},
> +		.delay = {
> +			.reg_off = SD_EMMC_CLOCK,
> +			.shift   = 20,
> +			.width   = 4,
> +		},
> +		.delay_step_ps   = CLK_DELAY_STEP_PS,
> +	},
> +};
> +
> +static const struct mmc_clkc_data mmc_clkc_axg_data = {
> +	.tx = {
> +		.phase = {
> +			.reg_off = SD_EMMC_CLOCK,
> +			.shift   = 10,
> +			.width   = 2,
> +		},
> +		.delay = {
> +			.reg_off = SD_EMMC_CLOCK,
> +			.shift   = 16,
> +			.width   = 6,
> +		},
> +		.delay_step_ps   = CLK_DELAY_STEP_PS,

Please check this. The delay step is not 200ns on the AXG.

> +	},
> +	.rx = {
> +		.phase = {
> +			.reg_off = SD_EMMC_CLOCK,
> +			.shift   = 12,
> +			.width   = 2,
> +		},
> +		.delay = {
> +			.reg_off = SD_EMMC_CLOCK,
> +			.shift   = 22,
> +			.width   = 6,
> +		},
> +		.delay_step_ps   = CLK_DELAY_STEP_PS,
> +	},
> +};
> +
> +static const struct of_device_id mmc_clkc_match_table[] = {
> +	{
> +		.compatible	= "amlogic,gx-mmc-clkc",
> +		.data		= &mmc_clkc_gx_data
> +	},
> +	{
> +		.compatible	= "amlogic,axg-mmc-clkc",
> +		.data		= &mmc_clkc_axg_data
> +	},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, mmc_clkc_match_table);
> +
> +static struct clk_regmap *
> +mmc_clkc_register_clk(struct device *dev, struct regmap *map,
> +		      struct clk_init_data *init,
> +		      const char *suffix, void *data)
> +{
> +	struct clk_regmap *clk;
> +	char *name;
> +	int ret;
> +
> +	clk = devm_kzalloc(dev, sizeof(*clk), GFP_KERNEL);
> +	if (!clk)
> +		return ERR_PTR(-ENOMEM);
> +
> +	name = kasprintf(GFP_KERNEL, "%s#%s", dev_name(dev), suffix);
> +	if (!name)
> +		return ERR_PTR(-ENOMEM);
> +
> +	init->name = name;
> +	clk->map = map;
> +	clk->data = data;
> +	clk->hw.init = init;
> +	ret = devm_clk_hw_register(dev, &clk->hw);
> +	if (ret)
> +		clk = ERR_PTR(ret);
> +
> +	kfree(name);
> +	return clk;
> +}
> +
> +static struct clk_regmap *mmc_clkc_register_mux(struct device *dev,
> +						struct regmap *map)
> +{
> +	const char *parent_names[MUX_CLK_NUM_PARENTS];
> +	struct clk_init_data init;
> +	struct clk_regmap *mux;
> +	struct clk *clk;
> +	int i;
> +
> +	for (i = 0; i < MUX_CLK_NUM_PARENTS; i++) {
> +		char name[8];
> +
> +		snprintf(name, sizeof(name), "clkin%d", i);
> +		clk = devm_clk_get(dev, name);
> +		if (IS_ERR(clk)) {
> +			if (clk != ERR_PTR(-EPROBE_DEFER))
> +				dev_err(dev, "Missing clock %s\n", name);
> +			return ERR_CAST(clk);
> +		}
> +
> +		parent_names[i] = __clk_get_name(clk);
> +	}
> +
> +	init.ops = &clk_regmap_mux_ops;
> +	init.flags = CLK_SET_RATE_PARENT;
> +	init.parent_names = parent_names;
> +	init.num_parents = MUX_CLK_NUM_PARENTS;
> +
> +	mux = mmc_clkc_register_clk(dev, map, &init, "mux",
> &mmc_clkc_mux_data);
> +	if (IS_ERR(mux))
> +		dev_err(dev, "Mux clock registration failed\n");
> +
> +	return mux;
> +}
> +
> +static struct clk_regmap *
> +mmc_clkc_register_clk_with_parent(struct device *dev, struct regmap *map,
> +				  char *suffix, const struct clk_hw *hw,
> +				  unsigned long flags,
> +				  const struct clk_ops *ops, void *data)
> +{
> +	struct clk_init_data init;
> +	struct clk_regmap *clk;
> +	const char *parent_name = clk_hw_get_name(hw);
> +
> +	init.ops = ops;
> +	init.flags = flags;
> +	init.parent_names = &parent_name;
> +	init.num_parents = 1;
> +
> +	clk = mmc_clkc_register_clk(dev, map, &init, suffix, data);
> +	if (IS_ERR(clk))
> +		dev_err(dev, "%s clock registration failed\n", suffix);
> +
> +	return clk;
> +}
> +
> +static int mmc_clkc_probe(struct platform_device *pdev)
> +{
> +	struct clk_hw_onecell_data *onecell_data;
> +	struct device *dev = &pdev->dev;
> +	struct mmc_clkc_data *data;
> +	struct regmap *map;
> +	struct clk_regmap *clk, *core;
> +	struct meson_sclk_div_data *div_data;
> +
> +	/*cast to drop the const in match->data*/
> +	data = (struct mmc_clkc_data *)of_device_get_match_data(dev);
> +	if (!data)
> +		return -ENODEV;
> +
> +	map = syscon_node_to_regmap(dev->of_node);
> +	if (IS_ERR(map)) {
> +		dev_err(dev, "could not find mmc clock controller\n");
> +		return PTR_ERR(map);
> +	}
> +
> +	onecell_data = devm_kzalloc(dev, sizeof(*onecell_data) +
> +				    sizeof(*onecell_data->hws) * MMC_MAX_CLKS,
> +				    GFP_KERNEL);
> +	if (!onecell_data)
> +		return -ENOMEM;
> +
> +	clk = mmc_clkc_register_mux(dev, map);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	div_data = devm_kzalloc(dev, sizeof(*div_data), GFP_KERNEL);
> +	if (!div_data)
> +		return -ENOMEM;
> +
> +	memcpy(div_data, &mmc_clkc_div_data, sizeof(*div_data));
> +	clk = mmc_clkc_register_clk_with_parent(dev, map, "div",
> +						&clk->hw,
> +						CLK_SET_RATE_PARENT,
> +						&meson_sclk_div_ops,
> +						div_data);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	onecell_data->hws[CLKID_MMC_DIV] = &clk->hw,
> +
> +	core = mmc_clkc_register_clk_with_parent(dev, map, "core",
> +						 &clk->hw,
> +						 CLK_SET_RATE_PARENT,
> +						 &meson_clk_phase_ops,
> +						 &mmc_clkc_core_phase);
> +	if (IS_ERR(core))
> +		return PTR_ERR(core);
> +
> +	onecell_data->hws[CLKID_MMC_PHASE_CORE] = &core->hw,
> +
> +	clk = mmc_clkc_register_clk_with_parent(dev, map, "rx",
> +						&core->hw,  0,
> +						&meson_clk_phase_delay_ops,
> +						&data->rx);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	onecell_data->hws[CLKID_MMC_PHASE_RX] = &clk->hw,
> +	clk = mmc_clkc_register_clk_with_parent(dev, map, "tx",
> +						&core->hw,  0,
> +						&meson_clk_phase_delay_ops,
> +						&data->tx);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	onecell_data->hws[CLKID_MMC_PHASE_TX] = &clk->hw,
> +	onecell_data->num = MMC_MAX_CLKS;
> +	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> +					   onecell_data);
> +}
> +
> +static struct platform_driver mmc_clkc_driver = {
> +	.probe		= mmc_clkc_probe,
> +	.driver		= {
> +		.name	= "meson-mmc-clkc",
> +		.of_match_table = of_match_ptr(mmc_clkc_match_table),
> +	},
> +};
> +
> +module_platform_driver(mmc_clkc_driver);
> +
> +MODULE_DESCRIPTION("Amlogic AXG MMC clock driver");
> +MODULE_AUTHOR("Jianxin Pan <jianxin.pan@amlogic.com>");
> +MODULE_LICENSE("GPL v2");



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH v9 3/4] clk: meson: add DT documentation for emmc clock controller
  2019-01-08 13:50 ` [PATCH v9 3/4] clk: meson: add DT documentation for emmc clock controller Jianxin Pan
@ 2019-01-22  9:27   ` Jerome Brunet
  0 siblings, 0 replies; 9+ messages in thread
From: Jerome Brunet @ 2019-01-22  9:27 UTC (permalink / raw)
  To: Jianxin Pan, Neil Armstrong
  Cc: Rob Herring, Hanjie Lin, Victor Wan, Stephen Boyd, Kevin Hilman,
	Michael Turquette, Yixun Lan, linux-kernel, Boris Brezillon,
	Liang Yang, Jian Hu, Miquel Raynal, Carlo Caione, linux-amlogic,
	Martin Blumenstingl, linux-clk, linux-arm-kernel, Qiufang Dai

On Tue, 2019-01-08 at 21:50 +0800, Jianxin Pan wrote:
> From: Yixun Lan <yixun.lan@amlogic.com>
> 
> Document the MMC sub clock controller driver, the potential consumer
> of this driver is MMC or NAND. Also add four clock bindings IDs which
> provided by this driver.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
> ---
>  .../devicetree/bindings/clock/amlogic,mmc-clkc.txt | 39
> ++++++++++++++++++++++
>  include/dt-bindings/clock/amlogic,mmc-clkc.h       | 17 ++++++++++
>  2 files changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-
> clkc.txt
>  create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
> b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
> new file mode 100644
> index 0000000..0f518e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
> @@ -0,0 +1,39 @@
> +* Amlogic MMC Sub Clock Controller Driver
> +
> +The Amlogic MMC clock controller generates and supplies clock to support
> +MMC and NAND controller
> +
> +Required Properties:
> +
> +- compatible: should be:
> +		"amlogic,gx-mmc-clkc"
> +		"amlogic,axg-mmc-clkc"

nitpick, missing is something to tell that only one is required, not both
for example `should be one of the following`

> +
> +- #clock-cells: should be 1.
> +- clocks: phandles to clocks corresponding to the clock-names property
> +- clock-names: list of parent clock names
> +	- "clkin0", "clkin1"
> +
> +- reg: address of emmc sub clock register
> +
> +Example: Clock controller node:
> +
> +sd_mmc_c_clkc: clock-controller@7000 {
> +	compatible = "amlogic,axg-mmc-clkc", "syscon";
> +	reg = <0x0 0x7000 0x0 0x4>;
> +	#clock-cells = <1>;
> +
> +	clock-names = "clkin0", "clkin1";
> +	clocks = <&clkc CLKID_SD_MMC_C_CLK0>,
> +		 <&clkc CLKID_FCLK_DIV2>;
> +};
> +
> +sd_emmc_b_clkc: clock-controller@5000 {
> +	compatible = "amlogic,axg-mmc-clkc", "syscon";
> +	reg = <0x0 0x5000 0x0 0x4>;
> +
> +	#clock-cells = <1>;
> +	clock-names = "clkin0", "clkin1";
> +	clocks = <&clkc CLKID_SD_EMMC_B_CLK0>,
> +		 <&clkc CLKID_FCLK_DIV2>;
> +};
> diff --git a/include/dt-bindings/clock/amlogic,mmc-clkc.h b/include/dt-
> bindings/clock/amlogic,mmc-clkc.h
> new file mode 100644
> index 0000000..34a3c56
> --- /dev/null
> +++ b/include/dt-bindings/clock/amlogic,mmc-clkc.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Meson MMC sub clock tree IDs
> + *
> + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
> + * Author: Yixun Lan <yixun.lan@amlogic.com>
> + */
> +
> +#ifndef __MMC_CLKC_H
> +#define __MMC_CLKC_H
> +
> +#define CLKID_MMC_DIV		0
> +#define CLKID_MMC_PHASE_CORE	1
> +#define CLKID_MMC_PHASE_TX	2
> +#define CLKID_MMC_PHASE_RX	3
> +
> +#endif



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH v9 1/4] clk: meson: add one based divider support for sclk divider
  2019-01-22  9:25   ` Jerome Brunet
@ 2019-01-24  9:10     ` Jianxin Pan
  0 siblings, 0 replies; 9+ messages in thread
From: Jianxin Pan @ 2019-01-24  9:10 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong
  Cc: Rob Herring, Hanjie Lin, Victor Wan, devicetree, Stephen Boyd,
	Kevin Hilman, Michael Turquette, Yixun Lan, linux-kernel,
	Boris Brezillon, Liang Yang, Jian Hu, Miquel Raynal,
	Carlo Caione, linux-amlogic, Martin Blumenstingl, linux-clk,
	linux-arm-kernel, Qiufang Dai

Hi Jerome,

On 2019/1/22 17:25, Jerome Brunet wrote:
> On Tue, 2019-01-08 at 21:50 +0800, Jianxin Pan wrote:
>> When CLK_DIVIDER_ONE_BASED flag is set, the sclk divider will be:
>> one based divider (div = val), and zero value gates the clock
>>
>> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
>> ---
>>  drivers/clk/meson/Makefile     |  3 ++-
>>  drivers/clk/meson/clkc-audio.h |  8 ------
>>  drivers/clk/meson/clkc.h       | 10 ++++++-
>>  drivers/clk/meson/sclk-div.c   | 59 ++++++++++++++++++++++++++++-----------
>> ---
>>  4 files changed, 50 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index a849aa8..acd8694 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -4,7 +4,8 @@
>>  
>>  obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-phase.o vid-
>> pll-div.o
>>  obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-input.o
>> -obj-$(CONFIG_COMMON_CLK_AMLOGIC_AUDIO)	+= clk-triphase.o sclk-div.o
>> +obj-$(CONFIG_COMMON_CLK_AMLOGIC) += sclk-div.o
>> +obj-$(CONFIG_COMMON_CLK_AMLOGIC_AUDIO) += clk-triphase.o
>>  obj-$(CONFIG_COMMON_CLK_MESON_AO) += meson-aoclk.o
>>  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
>>  obj-$(CONFIG_COMMON_CLK_GXBB)	 += gxbb.o gxbb-aoclk.o gxbb-aoclk-
>> 32k.o
>> diff --git a/drivers/clk/meson/clkc-audio.h b/drivers/clk/meson/clkc-audio.h
>> index 0a7c157..286ff12 100644
>> --- a/drivers/clk/meson/clkc-audio.h
>> +++ b/drivers/clk/meson/clkc-audio.h
>> @@ -15,14 +15,6 @@ struct meson_clk_triphase_data {
>>  	struct parm ph2;
>>  };
>>  
>> -struct meson_sclk_div_data {
>> -	struct parm div;
>> -	struct parm hi;
>> -	unsigned int cached_div;
>> -	struct clk_duty cached_duty;
>> -};
>> -
>>  extern const struct clk_ops meson_clk_triphase_ops;
>> -extern const struct clk_ops meson_sclk_div_ops;
>>  
>>  #endif /* __MESON_CLKC_AUDIO_H */
>> diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h
>> index 6183b22..00b3320 100644
>> --- a/drivers/clk/meson/clkc.h
>> +++ b/drivers/clk/meson/clkc.h
>> @@ -27,6 +27,14 @@ struct parm {
>>  	u8	width;
>>  };
>>  
>> +struct meson_sclk_div_data {
>> +	struct parm div;
>> +	struct parm hi;
>> +	unsigned int cached_div;
>> +	struct clk_duty cached_duty;
>> +	u8 flags;
>> +};
>> +
>>  static inline unsigned int meson_parm_read(struct regmap *map, struct parm
>> *p)
>>  {
>>  	unsigned int val;
>> @@ -118,10 +126,10 @@ struct clk_regmap _name = {				
>> 		\
>>  extern const struct clk_ops meson_clk_mpll_ops;
>>  extern const struct clk_ops meson_clk_phase_ops;
>>  extern const struct clk_ops meson_vid_pll_div_ro_ops;
>> +extern const struct clk_ops meson_sclk_div_ops;
>>  
>>  struct clk_hw *meson_clk_hw_register_input(struct device *dev,
>>  					   const char *of_name,
>>  					   const char *clk_name,
>>  					   unsigned long flags);
>> -
>>  #endif /* __CLKC_H */
>> diff --git a/drivers/clk/meson/sclk-div.c b/drivers/clk/meson/sclk-div.c
>> index bc64019..a6c425b 100644
>> --- a/drivers/clk/meson/sclk-div.c
>> +++ b/drivers/clk/meson/sclk-div.c
>> @@ -4,42 +4,60 @@
>>   * Author: Jerome Brunet <jbrunet@baylibre.com>
>>   *
>>   * Sample clock generator divider:
>> - * This HW divider gates with value 0 but is otherwise a zero based
>> divider:
>> + * This HW divider gates with value 0
>>   *
>>   * val >= 1
>> - * divider = val + 1
>> + * divider = val + 1 if ONE_BASED is not set, otherwise divider = val.
>>   *
>>   * The duty cycle may also be set for the LR clock variant. The duty cycle
>>   * ratio is:
>>   *
>>   * hi = [0 - val]
>> - * duty_cycle = (1 + hi) / (1 + val)
>> + * duty_cycle = (1 + hi) / (1 + val) if ONE_BASED is not set, otherwise:
>> + * duty_cycle = hi / (1 + val)
>>   */
>>  
>> -#include "clkc-audio.h"
>> +#include "clkc.h"
>>  
>> -static inline struct meson_sclk_div_data *
>> -meson_sclk_div_data(struct clk_regmap *clk)
>> +static inline int get_reg(int val, unsigned char flag)
> 
> s/get_reg/sclk_get_reg
OK, I will rename get_reg and get_value in next version.
Thank you for the review.
> 
>>  {
>> -	return (struct meson_sclk_div_data *)clk->data;
>> +	WARN_ON(val < 1);
> 
> I don't think this WARN is justified, especially since you are using this
> function to set the hi value.
OK, it will remove it. Thank you.
> 
>> +	if ((flag & CLK_DIVIDER_ONE_BASED) || !val)
> 
> I don't like that you make a dependency on the generic divider just for this.
> Please make your own flag like MESON_SCLK_ONE_BASED
OK, I will define a new flag MESON_SCLK_ONE_BASED in drivers/clk/meson/clkc.h.
> 
>> +		return val;
>> +	else
>> +		return val - 1;
>> +}
>> +
>> +static inline int get_value(int reg, unsigned char flag)
> 
> s/get_value/sclk_get_divider
OK, I will rename it.
> 
>> +{
>> +	if (flag & CLK_DIVIDER_ONE_BASED)
>> +		return reg;
>> +	else
>> +		return reg + 1;
>>  }
>>  
>> -static int sclk_div_maxval(struct meson_sclk_div_data *sclk)
>> +static inline struct meson_sclk_div_data *
>> +meson_sclk_div_data(struct clk_regmap *clk)
>>  {
>> -	return (1 << sclk->div.width) - 1;
>> +	return (struct meson_sclk_div_data *)clk->data;
>>  }
>>  
>>  static int sclk_div_maxdiv(struct meson_sclk_div_data *sclk)
>>  {
>> -	return sclk_div_maxval(sclk) + 1;
>> +	unsigned int reg = clk_div_mask(sclk->div.width);
> 
> same here, leave the generic divider alone.
>  
> `(1 << sclk->div.width) - 1` was fine to get max register value.
> 
>> +
>> +	return get_value(reg, sclk->flags);
>>  }
>>  
>>  static int sclk_div_getdiv(struct clk_hw *hw, unsigned long rate,
>>  			   unsigned long prate, int maxdiv)
>>  {
>>  	int div = DIV_ROUND_CLOSEST_ULL((u64)prate, rate);
>> +	struct clk_regmap *clk = to_clk_regmap(hw);
>> +	struct meson_sclk_div_data *sclk = meson_sclk_div_data(clk);
>> +	int mindiv = get_value(1, sclk->flags);
> 
> maxdiv is provided as a param while mindiv is computed inside this function.
> let's be coherent and pick one approach.
OK, I will compute min and max in this function both. 
Thank you for your review.
> 
>>  
>> -	return clamp(div, 2, maxdiv);
>> +	return clamp(div, mindiv, maxdiv);
>>  }
>>  
>>  static int sclk_div_bestdiv(struct clk_hw *hw, unsigned long rate,
>> @@ -47,7 +65,7 @@ static int sclk_div_bestdiv(struct clk_hw *hw, unsigned
>> long rate,
>>  			    struct meson_sclk_div_data *sclk)
>>  {
>>  	struct clk_hw *parent = clk_hw_get_parent(hw);
>> -	int bestdiv = 0, i;
>> +	int bestdiv = 0, i, mindiv;
>>  	unsigned long maxdiv, now, parent_now;
>>  	unsigned long best = 0, best_parent = 0;
>>  
>> @@ -64,8 +82,9 @@ static int sclk_div_bestdiv(struct clk_hw *hw, unsigned
>> long rate,
>>  	 * unsigned long in rate * i below
>>  	 */
>>  	maxdiv = min(ULONG_MAX / rate, maxdiv);
>> +	mindiv = get_value(1, sclk->flags);
>>  
>> -	for (i = 2; i <= maxdiv; i++) {
>> +	for (i = mindiv; i <= maxdiv; i++) {
>>  		/*
>>  		 * It's the most ideal case if the requested rate can be
>>  		 * divided from parent clock without needing to change
>> @@ -111,10 +130,7 @@ static void sclk_apply_ratio(struct clk_regmap *clk,
>>  					    sclk->cached_duty.num,
>>  					    sclk->cached_duty.den);
>>  
>> -	if (hi)
>> -		hi -= 1;
>> -
>> -	meson_parm_write(clk->map, &sclk->hi, hi);
>> +	meson_parm_write(clk->map, &sclk->hi, get_reg(hi, sclk->flags));
>>  }
>>  
>>  static int sclk_div_set_duty_cycle(struct clk_hw *hw,
>> @@ -145,7 +161,7 @@ static int sclk_div_get_duty_cycle(struct clk_hw *hw,
>>  	}
>>  
>>  	hi = meson_parm_read(clk->map, &sclk->hi);
>> -	duty->num = hi + 1;
>> +	duty->num = get_value(hi, sclk->flags);
>>  	duty->den = sclk->cached_div;
>>  	return 0;
>>  }
>> @@ -153,10 +169,13 @@ static int sclk_div_get_duty_cycle(struct clk_hw *hw,
>>  static void sclk_apply_divider(struct clk_regmap *clk,
>>  			       struct meson_sclk_div_data *sclk)
>>  {
>> +	unsigned int div;
>> +
>>  	if (MESON_PARM_APPLICABLE(&sclk->hi))
>>  		sclk_apply_ratio(clk, sclk);
>>  
>> -	meson_parm_write(clk->map, &sclk->div, sclk->cached_div - 1);
>> +	div = get_reg(sclk->cached_div, sclk->flags);
>> +	meson_parm_write(clk->map, &sclk->div, div);
>>  }
>>  
>>  static int sclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
>> @@ -224,7 +243,7 @@ static void sclk_div_init(struct clk_hw *hw)
>>  	if (!val)
>>  		sclk->cached_div = sclk_div_maxdiv(sclk);
>>  	else
>> -		sclk->cached_div = val + 1;
>> +		sclk->cached_div = get_value(val, sclk->flags);
>>  
>>  	sclk_div_get_duty_cycle(hw, &sclk->cached_duty);
>>  }
> 
> 
> .
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

end of thread, other threads:[~2019-01-24  9:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-08 13:50 [PATCH RESEND v9 0/4] clk: meson: add a sub EMMC clock controller support Jianxin Pan
2019-01-08 13:50 ` [PATCH v9 1/4] clk: meson: add one based divider support for sclk divider Jianxin Pan
2019-01-22  9:25   ` Jerome Brunet
2019-01-24  9:10     ` Jianxin Pan
2019-01-08 13:50 ` [PATCH v9 2/4] clk: meson: add emmc sub clock phase delay driver Jianxin Pan
2019-01-08 13:50 ` [PATCH v9 3/4] clk: meson: add DT documentation for emmc clock controller Jianxin Pan
2019-01-22  9:27   ` Jerome Brunet
2019-01-08 13:50 ` [PATCH v9 4/4] clk: meson: add sub MMC clock controller driver Jianxin Pan
2019-01-22  9:25   ` Jerome Brunet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).