* [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
0 siblings, 1 reply; 4+ messages in thread
From: Jianxin Pan @ 2019-01-08 13:50 UTC (permalink / raw)
To: Jerome Brunet, Neil Armstrong
Cc: Jianxin Pan, Kevin Hilman, Carlo Caione, Michael Turquette,
Stephen Boyd, Rob Herring, Miquel Raynal, Boris Brezillon,
Martin Blumenstingl, Yixun Lan, Liang Yang, Jian Hu, Qiufang Dai,
Hanjie Lin, Victor Wan, linux-clk, linux-amlogic,
linux-arm-kernel, 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 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
^ permalink raw reply [flat|nested] 4+ 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
0 siblings, 1 reply; 4+ messages in thread
From: Jianxin Pan @ 2019-01-08 13:50 UTC (permalink / raw)
To: Jerome Brunet, Neil Armstrong
Cc: Jianxin Pan, Kevin Hilman, Carlo Caione, Michael Turquette,
Stephen Boyd, Rob Herring, Miquel Raynal, Boris Brezillon,
Martin Blumenstingl, Yixun Lan, Liang Yang, Jian Hu, Qiufang Dai,
Hanjie Lin, Victor Wan, linux-clk, linux-amlogic,
linux-arm-kernel, linux-kernel, devicetree
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
^ permalink raw reply related [flat|nested] 4+ 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; 4+ messages in thread
From: Jerome Brunet @ 2019-01-22 9:25 UTC (permalink / raw)
To: Jianxin Pan, Neil Armstrong
Cc: Kevin Hilman, Carlo Caione, Michael Turquette, Stephen Boyd,
Rob Herring, Miquel Raynal, Boris Brezillon, Martin Blumenstingl,
Yixun Lan, Liang Yang, Jian Hu, Qiufang Dai, Hanjie Lin,
Victor Wan, linux-clk, linux-amlogic, linux-arm-kernel,
linux-kernel, devicetree
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);
> }
^ permalink raw reply [flat|nested] 4+ 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; 4+ messages in thread
From: Jianxin Pan @ 2019-01-24 9:10 UTC (permalink / raw)
To: Jerome Brunet, Neil Armstrong
Cc: Kevin Hilman, Carlo Caione, Michael Turquette, Stephen Boyd,
Rob Herring, Miquel Raynal, Boris Brezillon, Martin Blumenstingl,
Yixun Lan, Liang Yang, Jian Hu, Qiufang Dai, Hanjie Lin,
Victor Wan, linux-clk, linux-amlogic, linux-arm-kernel,
linux-kernel, devicetree
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);
>> }
>
>
> .
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-01-24 9:10 UTC | newest]
Thread overview: 4+ 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
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).