* [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support
@ 2022-01-21 7:45 Liang Yang
2022-01-21 7:45 ` [PATCH v10 1/4] clk: meson: add one based divider support for sclk Liang Yang
2022-01-25 14:54 ` [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support Neil Armstrong
0 siblings, 2 replies; 6+ messages in thread
From: Liang Yang @ 2022-01-21 7:45 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Michael Turquette,
Stephen Boyd, Rob Herring, linux-clk
Cc: Liang Yang, Martin Blumenstingl, Jianxin Pan, Victor Wan,
XianWei Zhao, Kelvin Zhang, BiChao Zheng, YongHui Yu,
linux-arm-kernel, linux-amlogic, linux-kernel, devicetree
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/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
[10] https://lore.kernel.org/all/20220113115745.45826-1-liang.yang@amlogic.com/
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v10 1/4] clk: meson: add one based divider support for sclk
2022-01-21 7:45 [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support Liang Yang
@ 2022-01-21 7:45 ` Liang Yang
2022-01-25 14:54 ` [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support Neil Armstrong
1 sibling, 0 replies; 6+ messages in thread
From: Liang Yang @ 2022-01-21 7:45 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Michael Turquette,
Stephen Boyd, Rob Herring, linux-clk
Cc: Liang Yang, Martin Blumenstingl, Jianxin Pan, Victor Wan,
XianWei Zhao, Kelvin Zhang, BiChao Zheng, YongHui Yu,
linux-arm-kernel, linux-amlogic, linux-kernel, devicetree
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 <liang.yang@amlogic.com>
---
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 <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 <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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support
2022-01-21 7:45 [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support Liang Yang
2022-01-21 7:45 ` [PATCH v10 1/4] clk: meson: add one based divider support for sclk Liang Yang
@ 2022-01-25 14:54 ` Neil Armstrong
2022-01-26 9:08 ` Liang Yang
1 sibling, 1 reply; 6+ messages in thread
From: Neil Armstrong @ 2022-01-25 14:54 UTC (permalink / raw)
To: Liang Yang, Jerome Brunet, Kevin Hilman, Michael Turquette,
Stephen Boyd, Rob Herring, linux-clk
Cc: Martin Blumenstingl, Jianxin Pan, Victor Wan, XianWei Zhao,
Kelvin Zhang, BiChao Zheng, YongHui Yu, linux-arm-kernel,
linux-amlogic, linux-kernel, devicetree
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/20220106033130.37623-1-liang.yang@amlogic.com
[2] https://lore.kernel.org/r/20220106032504.23310-1-liang.yang@amlogic.com
>
> 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/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
> [10] https://lore.kernel.org/all/20220113115745.45826-1-liang.yang@amlogic.com/
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support
2022-01-25 14:54 ` [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support Neil Armstrong
@ 2022-01-26 9:08 ` Liang Yang
2022-01-26 9:14 ` Jerome Brunet
0 siblings, 1 reply; 6+ messages in thread
From: Liang Yang @ 2022-01-26 9:08 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Michael Turquette,
Stephen Boyd, Rob Herring, linux-clk
Cc: Martin Blumenstingl, Jianxin Pan, Victor Wan, XianWei Zhao,
Kelvin Zhang, BiChao Zheng, YongHui Yu, linux-arm-kernel,
linux-amlogic, linux-kernel, devicetree
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/20220106033130.37623-1-liang.yang@amlogic.com
> [2] https://lore.kernel.org/r/20220106032504.23310-1-liang.yang@amlogic.com
>
>>
>> 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/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
>> [10] https://lore.kernel.org/all/20220113115745.45826-1-liang.yang@amlogic.com/
>> 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
>>
>
> .
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support
2022-01-26 9:08 ` Liang Yang
@ 2022-01-26 9:14 ` Jerome Brunet
2022-01-26 9:42 ` Liang Yang
0 siblings, 1 reply; 6+ messages in thread
From: Jerome Brunet @ 2022-01-26 9:14 UTC (permalink / raw)
To: Liang Yang, Neil Armstrong, Kevin Hilman, Michael Turquette,
Stephen Boyd, Rob Herring, linux-clk
Cc: Martin Blumenstingl, Jianxin Pan, Victor Wan, XianWei Zhao,
Kelvin Zhang, BiChao Zheng, YongHui Yu, linux-arm-kernel,
linux-amlogic, linux-kernel, devicetree
On Wed 26 Jan 2022 at 17:08, Liang Yang <liang.yang@amlogic.com> 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/20220106033130.37623-1-liang.yang@amlogic.com
>> [2] https://lore.kernel.org/r/20220106032504.23310-1-liang.yang@amlogic.com
>>
>>>
>>> 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/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
>>> [10] https://lore.kernel.org/all/20220113115745.45826-1-liang.yang@amlogic.com/
>>> 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
>>>
>> .
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support
2022-01-26 9:14 ` Jerome Brunet
@ 2022-01-26 9:42 ` Liang Yang
0 siblings, 0 replies; 6+ messages in thread
From: Liang Yang @ 2022-01-26 9:42 UTC (permalink / raw)
To: Jerome Brunet, Neil Armstrong, Kevin Hilman, Michael Turquette,
Stephen Boyd, Rob Herring, linux-clk
Cc: Martin Blumenstingl, Jianxin Pan, Victor Wan, XianWei Zhao,
Kelvin Zhang, BiChao Zheng, YongHui Yu, linux-arm-kernel,
linux-amlogic, linux-kernel, devicetree
Hi Jerome,
On 2022/1/26 17:14, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
>
> On Wed 26 Jan 2022 at 17:08, Liang Yang <liang.yang@amlogic.com> 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/20220106033130.37623-1-liang.yang@amlogic.com
>>> [2] https://lore.kernel.org/r/20220106032504.23310-1-liang.yang@amlogic.com
>>>
>>>>
>>>> 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/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
>>>> [10] https://lore.kernel.org/all/20220113115745.45826-1-liang.yang@amlogic.com/
>>>> 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
>>>>
>>> .
>
> .
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-26 9:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-21 7:45 [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support Liang Yang
2022-01-21 7:45 ` [PATCH v10 1/4] clk: meson: add one based divider support for sclk Liang Yang
2022-01-25 14:54 ` [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support Neil Armstrong
2022-01-26 9:08 ` Liang Yang
2022-01-26 9:14 ` Jerome Brunet
2022-01-26 9:42 ` Liang Yang
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).