2022-01-22 00:37:59

by Liang Yang

[permalink] [raw]
Subject: [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support

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 v9 [10]
- use clk_parent_data instead of parent_names

Changes since v8 [9]
- use MESON_SCLK_ONE_BASED instead of CLK_DIVIDER_ONE_BASED
- use struct_size to caculate onecell_data
- add clk-phase-delay.h
- define CLK_DELAY_STEP_PS_GX and CLK_DELAY_STEP_PS_AXG

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/[email protected]
[3] https://lkml.kernel.org/r/[email protected]
[4] https://lkml.kernel.org/r/[email protected]
[5] https://lkml.kernel.org/r/[email protected]
[6] https://lkml.kernel.org/r/[email protected]
[7] https://lkml.kernel.org/r/[email protected]
[8] https://lkml.kernel.org/r/[email protected]
[9] https://lkml.kernel.org/r/[email protected]
[10] https://lore.kernel.org/all/[email protected]/
Liang Yang (4):
clk: meson: add one based divider support for sclk
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

.../bindings/clock/amlogic,mmc-clkc.yaml | 64 ++++
drivers/clk/meson/Kconfig | 18 ++
drivers/clk/meson/Makefile | 2 +
drivers/clk/meson/clk-phase-delay.c | 69 ++++
drivers/clk/meson/clk-phase-delay.h | 20 ++
drivers/clk/meson/mmc-clkc.c | 302 ++++++++++++++++++
drivers/clk/meson/sclk-div.c | 59 ++--
drivers/clk/meson/sclk-div.h | 3 +
include/dt-bindings/clock/amlogic,mmc-clkc.h | 14 +
9 files changed, 529 insertions(+), 22 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.yaml
create mode 100644 drivers/clk/meson/clk-phase-delay.c
create mode 100644 drivers/clk/meson/clk-phase-delay.h
create mode 100644 drivers/clk/meson/mmc-clkc.c
create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h

--
2.34.1


2022-01-22 00:38:07

by Liang Yang

[permalink] [raw]
Subject: [PATCH v10 1/4] clk: meson: add one based divider support for sclk

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

Signed-off-by: Liang Yang <[email protected]>
---
drivers/clk/meson/sclk-div.c | 59 ++++++++++++++++++++++--------------
drivers/clk/meson/sclk-div.h | 3 ++
2 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/drivers/clk/meson/sclk-div.c b/drivers/clk/meson/sclk-div.c
index 76d31c0a3342..4ddc1763a12d 100644
--- a/drivers/clk/meson/sclk-div.c
+++ b/drivers/clk/meson/sclk-div.c
@@ -4,16 +4,17 @@
* Author: Jerome Brunet <[email protected]>
*
* 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 <linux/clk-provider.h>
@@ -28,22 +29,37 @@ meson_sclk_div_data(struct clk_regmap *clk)
return (struct meson_sclk_div_data *)clk->data;
}

-static int sclk_div_maxval(struct meson_sclk_div_data *sclk)
+static inline int sclk_get_reg(int val, unsigned char flag)
{
- return (1 << sclk->div.width) - 1;
+ if ((flag & MESON_SCLK_ONE_BASED) || !val)
+ return val;
+ return val - 1;
+}
+
+static inline int sclk_get_divider(int reg, unsigned char flag)
+{
+ if (flag & MESON_SCLK_ONE_BASED)
+ return reg;
+ return reg + 1;
}

static int sclk_div_maxdiv(struct meson_sclk_div_data *sclk)
{
- return sclk_div_maxval(sclk) + 1;
+ unsigned int reg = (1 << sclk->div.width) - 1;
+
+ return sclk_get_divider(reg, sclk->flags);
}

static int sclk_div_getdiv(struct clk_hw *hw, unsigned long rate,
- unsigned long prate, int maxdiv)
+ unsigned long prate)
{
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 = sclk_get_divider(1, sclk->flags);
+ int maxdiv = sclk_div_maxdiv(sclk);

- return clamp(div, 2, maxdiv);
+ return clamp(div, mindiv, maxdiv);
}

static int sclk_div_bestdiv(struct clk_hw *hw, unsigned long rate,
@@ -51,25 +67,25 @@ 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;

if (!rate)
rate = 1;

- maxdiv = sclk_div_maxdiv(sclk);
-
if (!(clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT))
- return sclk_div_getdiv(hw, rate, *prate, maxdiv);
+ return sclk_div_getdiv(hw, rate, *prate);

/*
* The maximum divider we can use without overflowing
* unsigned long in rate * i below
*/
+ maxdiv = sclk_div_maxdiv(sclk);
maxdiv = min(ULONG_MAX / rate, maxdiv);
+ mindiv = sclk_get_divider(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
@@ -115,10 +131,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, sclk_get_reg(hi, sclk->flags));
}

static int sclk_div_set_duty_cycle(struct clk_hw *hw,
@@ -149,7 +162,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 = sclk_get_divider(hi, sclk->flags);
duty->den = sclk->cached_div;
return 0;
}
@@ -157,10 +170,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 = sclk_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,
@@ -168,9 +184,8 @@ static int sclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
{
struct clk_regmap *clk = to_clk_regmap(hw);
struct meson_sclk_div_data *sclk = meson_sclk_div_data(clk);
- unsigned long maxdiv = sclk_div_maxdiv(sclk);

- sclk->cached_div = sclk_div_getdiv(hw, rate, prate, maxdiv);
+ sclk->cached_div = sclk_div_getdiv(hw, rate, prate);

if (clk_hw_is_enabled(hw))
sclk_apply_divider(clk, sclk);
@@ -228,7 +243,7 @@ static int 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 = sclk_get_divider(val, sclk->flags);

sclk_div_get_duty_cycle(hw, &sclk->cached_duty);

diff --git a/drivers/clk/meson/sclk-div.h b/drivers/clk/meson/sclk-div.h
index b64b2a32005f..944dab5ec0cf 100644
--- a/drivers/clk/meson/sclk-div.h
+++ b/drivers/clk/meson/sclk-div.h
@@ -10,11 +10,14 @@
#include <linux/clk-provider.h>
#include "parm.h"

+#define MESON_SCLK_ONE_BASED BIT(0)
+
struct meson_sclk_div_data {
struct parm div;
struct parm hi;
unsigned int cached_div;
struct clk_duty cached_duty;
+ u8 flags;
};

extern const struct clk_ops meson_sclk_div_ops;
--
2.34.1

2022-01-22 00:38:13

by Liang Yang

[permalink] [raw]
Subject: [PATCH v10 2/4] clk: meson: add emmc sub clock phase delay driver

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

Signed-off-by: Liang Yang <[email protected]>
---
drivers/clk/meson/Kconfig | 4 ++
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/clk-phase-delay.c | 69 +++++++++++++++++++++++++++++
drivers/clk/meson/clk-phase-delay.h | 20 +++++++++
4 files changed, 94 insertions(+)
create mode 100644 drivers/clk/meson/clk-phase-delay.c
create mode 100644 drivers/clk/meson/clk-phase-delay.h

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 3014e2f1fbb4..bb0f59eea366 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -18,6 +18,10 @@ config COMMON_CLK_MESON_PHASE
tristate
select COMMON_CLK_MESON_REGMAP

+config COMMON_CLK_MESON_PHASE_DELAY
+ tristate
+ select COMMON_CLK_MESON_REGMAP
+
config COMMON_CLK_MESON_PLL
tristate
select COMMON_CLK_MESON_REGMAP
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index b3ef5f67820f..99fe4eeed000 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_MPLL) += clk-mpll.o
obj-$(CONFIG_COMMON_CLK_MESON_PHASE) += clk-phase.o
obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
+obj-$(CONFIG_COMMON_CLK_MESON_PHASE_DELAY) += clk-phase-delay.o
obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o

diff --git a/drivers/clk/meson/clk-phase-delay.c b/drivers/clk/meson/clk-phase-delay.c
new file mode 100644
index 000000000000..3c1ae0ee2a24
--- /dev/null
+++ b/drivers/clk/meson/clk-phase-delay.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+
+#include "clk-regmap.h"
+#include "clk-phase-delay.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/clk-phase-delay.h b/drivers/clk/meson/clk-phase-delay.h
new file mode 100644
index 000000000000..b4f211d02c84
--- /dev/null
+++ b/drivers/clk/meson/clk-phase-delay.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#ifndef __MESON_CLK_PHASE_DELAY_H
+#define __MESON_CLK_PHASE_DELAY_H
+
+#include <linux/clk-provider.h>
+#include "parm.h"
+
+struct meson_clk_phase_delay_data {
+ struct parm phase;
+ struct parm delay;
+ unsigned int delay_step_ps;
+};
+
+extern const struct clk_ops meson_clk_phase_delay_ops;
+
+#endif /* __MESON_CLK_PHASE_DELAY_H */
--
2.34.1

2022-01-22 00:38:17

by Liang Yang

[permalink] [raw]
Subject: [PATCH v10 4/4] clk: meson: add sub MMC clock controller driver

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: Liang Yang <[email protected]>
---
drivers/clk/meson/Kconfig | 14 ++
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/mmc-clkc.c | 302 +++++++++++++++++++++++++++++++++++
3 files changed, 317 insertions(+)
create mode 100644 drivers/clk/meson/mmc-clkc.c

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index bb0f59eea366..5f344a0892cb 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -39,6 +39,20 @@ config COMMON_CLK_MESON_AO_CLKC
select COMMON_CLK_MESON_REGMAP
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
+ select COMMON_CLK_MESON_PHASE
+ select COMMON_CLK_MESON_PHASE_DELAY
+ select COMMON_CLK_MESON_SCLK_DIV
+ help
+ Support for the MMC sub clock controller on
+ Amlogic Meson Platform, which includes S905 (GXBB, GXL),
+ A113D/X (AXG) devices. Say Y if you want this
+ clock enabled.
+
config COMMON_CLK_MESON_EE_CLKC
tristate
select COMMON_CLK_MESON_REGMAP
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 99fe4eeed000..7a7ffb37a726 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.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_GXBB) += gxbb.o gxbb-aoclk.o
obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
diff --git a/drivers/clk/meson/mmc-clkc.c b/drivers/clk/meson/mmc-clkc.c
new file mode 100644
index 000000000000..e42c4015c7a3
--- /dev/null
+++ b/drivers/clk/meson/mmc-clkc.c
@@ -0,0 +1,302 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#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 "sclk-div.h"
+#include "clk-phase-delay.h"
+#include "clk-regmap.h"
+#include "clk-phase.h"
+
+/* clock ID used by internal driver */
+
+#define SD_EMMC_CLOCK 0
+#define CLK_DELAY_STEP_PS_GX 200
+#define CLK_DELAY_STEP_PS_AXG 78
+#define MUX_CLK_NUM_PARENTS 2
+#define MMC_MAX_CLKS 4
+
+static struct clk_parent_data mmc_clkc_parent_data[MUX_CLK_NUM_PARENTS];
+
+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 = MESON_SCLK_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_GX,
+ },
+ .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_GX,
+ },
+};
+
+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_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_AXG,
+ },
+};
+
+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)
+{
+ struct clk_init_data init = {0};
+ 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_probe(dev, PTR_ERR(clk),
+ "Missing clock\n");
+ return ERR_CAST(clk);
+ }
+
+ mmc_clkc_parent_data[i].fw_name = __clk_get_name(clk);
+ }
+
+ init.ops = &clk_regmap_mux_ops;
+ init.flags = CLK_SET_RATE_PARENT;
+ init.parent_data = mmc_clkc_parent_data;
+ 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;
+ const struct mmc_clkc_data *data;
+ struct regmap *map;
+ struct clk_regmap *clk, *core;
+ struct meson_sclk_div_data *div_data, *tx, *rx;
+
+ data = of_device_get_match_data(dev);
+ if (!data)
+ return -ENODEV;
+
+ /* cast to drop the const */
+ tx = (struct meson_sclk_div_data *)&data->tx;
+ rx = (struct meson_sclk_div_data *)&data->rx;
+
+ 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,
+ struct_size(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,
+ 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,
+ 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 <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.34.1

2022-01-22 00:39:19

by Liang Yang

[permalink] [raw]
Subject: [PATCH v10 3/4] clk: meson: add DT documentation for emmc clock controller

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.

Signed-off-by: Liang Yang <[email protected]>
---
.../bindings/clock/amlogic,mmc-clkc.yaml | 64 +++++++++++++++++++
include/dt-bindings/clock/amlogic,mmc-clkc.h | 14 ++++
2 files changed, 78 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.yaml
create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h

diff --git a/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.yaml
new file mode 100644
index 000000000000..0fe2e33c2082
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/amlogic,mmc-clkc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic MMC Sub Clock Controller Driver Device Tree Bindings
+
+maintainers:
+ - [email protected]
+ - [email protected]
+
+properties:
+ compatible:
+ enum:
+ - "amlogic,axg-mmc-clkc", "syscon"
+ - "amlogic,gx-mmc-clkc", "syscon"
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 2
+
+ clock-names:
+ items:
+ - const: "clkin0", "clkin1"
+
+ "#clock-cells":
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - "#clock-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ 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_EMMC_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>;
+ };
+
+...
\ No newline at end of file
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 000000000000..71301517b183
--- /dev/null
+++ b/include/dt-bindings/clock/amlogic,mmc-clkc.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#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
--
2.34.1

2022-01-25 22:45:16

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support

Hi Liang,

On 21/01/2022 08:45, Liang Yang wrote:
> 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.

Thanks a lot for providing a fixed and updated version of this serie.

After some chat with Jerome and Kevin, it seems the way the eMMC clock reuse
for NAND was designed nearly 4 years doesn't look accurate anymore.

Having a separate clk driver designed to replace the eMMC node when NAND is
used on the board seems over engineered.

Actually having the clock code you add in this serie _but_ directly in
the NAND looks far better, and more coherent since having Linux runtime
detection of eMMC vs NAND will never happen and even this serie required
some DT modification from the bootloader.

I'll let Jerome or Kevin add more details if they want, but I think you should resurrect
the work you pushed in [1] & [2] but:
- passing the eMMC clk registers as a third "reg" cell
- passing the same "clocks" phandle as the eMMC node
- adding the eMMC clock code in the NAND driver directly

I'm open to discussions if you consider the current approach is still superior.

Thanks,
Neil

[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]

>
> Changes since v9 [10]
> - use clk_parent_data instead of parent_names
>
> Changes since v8 [9]
> - use MESON_SCLK_ONE_BASED instead of CLK_DIVIDER_ONE_BASED
> - use struct_size to caculate onecell_data
> - add clk-phase-delay.h
> - define CLK_DELAY_STEP_PS_GX and CLK_DELAY_STEP_PS_AXG
>
> 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/[email protected]
> [3] https://lkml.kernel.org/r/[email protected]
> [4] https://lkml.kernel.org/r/[email protected]
> [5] https://lkml.kernel.org/r/[email protected]
> [6] https://lkml.kernel.org/r/[email protected]
> [7] https://lkml.kernel.org/r/[email protected]
> [8] https://lkml.kernel.org/r/[email protected]
> [9] https://lkml.kernel.org/r/[email protected]
> [10] https://lore.kernel.org/all/[email protected]/
> Liang Yang (4):
> clk: meson: add one based divider support for sclk
> 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
>
> .../bindings/clock/amlogic,mmc-clkc.yaml | 64 ++++
> drivers/clk/meson/Kconfig | 18 ++
> drivers/clk/meson/Makefile | 2 +
> drivers/clk/meson/clk-phase-delay.c | 69 ++++
> drivers/clk/meson/clk-phase-delay.h | 20 ++
> drivers/clk/meson/mmc-clkc.c | 302 ++++++++++++++++++
> drivers/clk/meson/sclk-div.c | 59 ++--
> drivers/clk/meson/sclk-div.h | 3 +
> include/dt-bindings/clock/amlogic,mmc-clkc.h | 14 +
> 9 files changed, 529 insertions(+), 22 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.yaml
> create mode 100644 drivers/clk/meson/clk-phase-delay.c
> create mode 100644 drivers/clk/meson/clk-phase-delay.h
> create mode 100644 drivers/clk/meson/mmc-clkc.c
> create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h
>

2022-01-26 20:38:47

by Liang Yang

[permalink] [raw]
Subject: Re: [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support

Hi Neil,

On 2022/1/25 22:54, Neil Armstrong wrote:
> [ EXTERNAL EMAIL ]
>
> Hi Liang,
>
> On 21/01/2022 08:45, Liang Yang wrote:
>> 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.
>
> Thanks a lot for providing a fixed and updated version of this serie.
>
> After some chat with Jerome and Kevin, it seems the way the eMMC clock reuse
> for NAND was designed nearly 4 years doesn't look accurate anymore.
>
> Having a separate clk driver designed to replace the eMMC node when NAND is
> used on the board seems over engineered.
>
> Actually having the clock code you add in this serie _but_ directly in
> the NAND looks far better, and more coherent since having Linux runtime
> detection of eMMC vs NAND will never happen and even this serie required
> some DT modification from the bootloader.
>
> I'll let Jerome or Kevin add more details if they want, but I think you should resurrect
> the work you pushed in [1] & [2] but:
> - passing the eMMC clk registers as a third "reg" cell
Does it just need to define a 'reg' resource in NFC node and not
'syscon' here?
> - passing the same "clocks" phandle as the eMMC node
> - adding the eMMC clock code in the NAND driver directly
>
> I'm open to discussions if you consider the current approach is still superior.

I don't have persuasive ideas, but really it shares the common clock
implementation for both NFC and EMMC. and we don't need to paste the
same code in NFC and EMMC.

>
> Thanks,
> Neil
>
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
>
>>
>> Changes since v9 [10]
>> - use clk_parent_data instead of parent_names
>>
>> Changes since v8 [9]
>> - use MESON_SCLK_ONE_BASED instead of CLK_DIVIDER_ONE_BASED
>> - use struct_size to caculate onecell_data
>> - add clk-phase-delay.h
>> - define CLK_DELAY_STEP_PS_GX and CLK_DELAY_STEP_PS_AXG
>>
>> 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/[email protected]
>> [3] https://lkml.kernel.org/r/[email protected]
>> [4] https://lkml.kernel.org/r/[email protected]
>> [5] https://lkml.kernel.org/r/[email protected]
>> [6] https://lkml.kernel.org/r/[email protected]
>> [7] https://lkml.kernel.org/r/[email protected]
>> [8] https://lkml.kernel.org/r/[email protected]
>> [9] https://lkml.kernel.org/r/[email protected]
>> [10] https://lore.kernel.org/all/[email protected]/
>> Liang Yang (4):
>> clk: meson: add one based divider support for sclk
>> 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
>>
>> .../bindings/clock/amlogic,mmc-clkc.yaml | 64 ++++
>> drivers/clk/meson/Kconfig | 18 ++
>> drivers/clk/meson/Makefile | 2 +
>> drivers/clk/meson/clk-phase-delay.c | 69 ++++
>> drivers/clk/meson/clk-phase-delay.h | 20 ++
>> drivers/clk/meson/mmc-clkc.c | 302 ++++++++++++++++++
>> drivers/clk/meson/sclk-div.c | 59 ++--
>> drivers/clk/meson/sclk-div.h | 3 +
>> include/dt-bindings/clock/amlogic,mmc-clkc.h | 14 +
>> 9 files changed, 529 insertions(+), 22 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.yaml
>> create mode 100644 drivers/clk/meson/clk-phase-delay.c
>> create mode 100644 drivers/clk/meson/clk-phase-delay.h
>> create mode 100644 drivers/clk/meson/mmc-clkc.c
>> create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h
>>
>
> .

2022-01-26 20:41:44

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support


On Wed 26 Jan 2022 at 17:08, Liang Yang <[email protected]> wrote:

> Hi Neil,
>
> On 2022/1/25 22:54, Neil Armstrong wrote:
>> [ EXTERNAL EMAIL ]
>> Hi Liang,
>> On 21/01/2022 08:45, Liang Yang wrote:
>>> 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.
>> Thanks a lot for providing a fixed and updated version of this serie.
>> After some chat with Jerome and Kevin, it seems the way the eMMC clock
>> reuse
>> for NAND was designed nearly 4 years doesn't look accurate anymore.
>> Having a separate clk driver designed to replace the eMMC node when NAND
>> is
>> used on the board seems over engineered.
>> Actually having the clock code you add in this serie _but_ directly in
>> the NAND looks far better, and more coherent since having Linux runtime
>> detection of eMMC vs NAND will never happen and even this serie required
>> some DT modification from the bootloader.
>> I'll let Jerome or Kevin add more details if they want, but I think you
>> should resurrect
>> the work you pushed in [1] & [2] but:
>> - passing the eMMC clk registers as a third "reg" cell
> Does it just need to define a 'reg' resource in NFC node and not 'syscon'
> here?

Yes

>> - passing the same "clocks" phandle as the eMMC node
>> - adding the eMMC clock code in the NAND driver directly
>> I'm open to discussions if you consider the current approach is still
>> superior.
>
> I don't have persuasive ideas, but really it shares the common clock
> implementation for both NFC and EMMC. and we don't need to paste the
> same code in NFC and EMMC.

You don't need to copy everything. If I understood correctly, all the
Rx/Tx should not be needed. Yes, there is some duplication as it stands but
it allows to avoid coupling the MMC and NAND driver. We can still think
about optimizing things later on. Let's get something simply working
first.

>
>> Thanks,
>> Neil
>> [1]
>> https://lore.kernel.org/r/[email protected]
>> [2] https://lore.kernel.org/r/[email protected]
>>
>>>
>>> Changes since v9 [10]
>>> - use clk_parent_data instead of parent_names
>>>
>>> Changes since v8 [9]
>>> - use MESON_SCLK_ONE_BASED instead of CLK_DIVIDER_ONE_BASED
>>> - use struct_size to caculate onecell_data
>>> - add clk-phase-delay.h
>>> - define CLK_DELAY_STEP_PS_GX and CLK_DELAY_STEP_PS_AXG
>>>
>>> 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/[email protected]
>>> [3] https://lkml.kernel.org/r/[email protected]
>>> [4] https://lkml.kernel.org/r/[email protected]
>>> [5] https://lkml.kernel.org/r/[email protected]
>>> [6] https://lkml.kernel.org/r/[email protected]
>>> [7] https://lkml.kernel.org/r/[email protected]
>>> [8] https://lkml.kernel.org/r/[email protected]
>>> [9] https://lkml.kernel.org/r/[email protected]
>>> [10] https://lore.kernel.org/all/[email protected]/
>>> Liang Yang (4):
>>> clk: meson: add one based divider support for sclk
>>> 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
>>>
>>> .../bindings/clock/amlogic,mmc-clkc.yaml | 64 ++++
>>> drivers/clk/meson/Kconfig | 18 ++
>>> drivers/clk/meson/Makefile | 2 +
>>> drivers/clk/meson/clk-phase-delay.c | 69 ++++
>>> drivers/clk/meson/clk-phase-delay.h | 20 ++
>>> drivers/clk/meson/mmc-clkc.c | 302 ++++++++++++++++++
>>> drivers/clk/meson/sclk-div.c | 59 ++--
>>> drivers/clk/meson/sclk-div.h | 3 +
>>> include/dt-bindings/clock/amlogic,mmc-clkc.h | 14 +
>>> 9 files changed, 529 insertions(+), 22 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.yaml
>>> create mode 100644 drivers/clk/meson/clk-phase-delay.c
>>> create mode 100644 drivers/clk/meson/clk-phase-delay.h
>>> create mode 100644 drivers/clk/meson/mmc-clkc.c
>>> create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h
>>>
>> .

2022-01-26 20:50:54

by Liang Yang

[permalink] [raw]
Subject: Re: [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support

Hi Jerome,

On 2022/1/26 17:14, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
>
> On Wed 26 Jan 2022 at 17:08, Liang Yang <[email protected]> wrote:
>
>> Hi Neil,
>>
>> On 2022/1/25 22:54, Neil Armstrong wrote:
>>> [ EXTERNAL EMAIL ]
>>> Hi Liang,
>>> On 21/01/2022 08:45, Liang Yang wrote:
>>>> 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.
>>> Thanks a lot for providing a fixed and updated version of this serie.
>>> After some chat with Jerome and Kevin, it seems the way the eMMC clock
>>> reuse
>>> for NAND was designed nearly 4 years doesn't look accurate anymore.
>>> Having a separate clk driver designed to replace the eMMC node when NAND
>>> is
>>> used on the board seems over engineered.
>>> Actually having the clock code you add in this serie _but_ directly in
>>> the NAND looks far better, and more coherent since having Linux runtime
>>> detection of eMMC vs NAND will never happen and even this serie required
>>> some DT modification from the bootloader.
>>> I'll let Jerome or Kevin add more details if they want, but I think you
>>> should resurrect
>>> the work you pushed in [1] & [2] but:
>>> - passing the eMMC clk registers as a third "reg" cell
>> Does it just need to define a 'reg' resource in NFC node and not 'syscon'
>> here?
>
> Yes
>
>>> - passing the same "clocks" phandle as the eMMC node
>>> - adding the eMMC clock code in the NAND driver directly
>>> I'm open to discussions if you consider the current approach is still
>>> superior.
>>
>> I don't have persuasive ideas, but really it shares the common clock
>> implementation for both NFC and EMMC. and we don't need to paste the
>> same code in NFC and EMMC.
>
> You don't need to copy everything. If I understood correctly, all the
> Rx/Tx should not be needed. Yes, there is some duplication as it stands but
> it allows to avoid coupling the MMC and NAND driver. We can still think
> about optimizing things later on. Let's get something simply working
> first.
ok. i will do it. thank you.
>
>>
>>> Thanks,
>>> Neil
>>> [1]
>>> https://lore.kernel.org/r/[email protected]
>>> [2] https://lore.kernel.org/r/[email protected]
>>>
>>>>
>>>> Changes since v9 [10]
>>>> - use clk_parent_data instead of parent_names
>>>>
>>>> Changes since v8 [9]
>>>> - use MESON_SCLK_ONE_BASED instead of CLK_DIVIDER_ONE_BASED
>>>> - use struct_size to caculate onecell_data
>>>> - add clk-phase-delay.h
>>>> - define CLK_DELAY_STEP_PS_GX and CLK_DELAY_STEP_PS_AXG
>>>>
>>>> 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/[email protected]
>>>> [3] https://lkml.kernel.org/r/[email protected]
>>>> [4] https://lkml.kernel.org/r/[email protected]
>>>> [5] https://lkml.kernel.org/r/[email protected]
>>>> [6] https://lkml.kernel.org/r/[email protected]
>>>> [7] https://lkml.kernel.org/r/[email protected]
>>>> [8] https://lkml.kernel.org/r/[email protected]
>>>> [9] https://lkml.kernel.org/r/[email protected]
>>>> [10] https://lore.kernel.org/all/[email protected]/
>>>> Liang Yang (4):
>>>> clk: meson: add one based divider support for sclk
>>>> 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
>>>>
>>>> .../bindings/clock/amlogic,mmc-clkc.yaml | 64 ++++
>>>> drivers/clk/meson/Kconfig | 18 ++
>>>> drivers/clk/meson/Makefile | 2 +
>>>> drivers/clk/meson/clk-phase-delay.c | 69 ++++
>>>> drivers/clk/meson/clk-phase-delay.h | 20 ++
>>>> drivers/clk/meson/mmc-clkc.c | 302 ++++++++++++++++++
>>>> drivers/clk/meson/sclk-div.c | 59 ++--
>>>> drivers/clk/meson/sclk-div.h | 3 +
>>>> include/dt-bindings/clock/amlogic,mmc-clkc.h | 14 +
>>>> 9 files changed, 529 insertions(+), 22 deletions(-)
>>>> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.yaml
>>>> create mode 100644 drivers/clk/meson/clk-phase-delay.c
>>>> create mode 100644 drivers/clk/meson/clk-phase-delay.h
>>>> create mode 100644 drivers/clk/meson/mmc-clkc.c
>>>> create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h
>>>>
>>> .
>
> .