2019-10-29 13:42:01

by Peng Fan

[permalink] [raw]
Subject: [PATCH 0/7] clk: imx: switch to clk_hw based API

From: Peng Fan <[email protected]>

This is a preparation patch set to switch i.MX8MM/N/Q clk
driver to clk_hw based API.

There are some patches under reviewing for i.MX8M clk driver,
to avoid conflicts, so not include i.MX8M clk_hw patches in this
pach set.

The patch set covers the APIs used by i.MX8M clk driver.

Peng Fan (7):
clk: imx: clk-pll14xx: Switch to clk_hw based API
clk: imx: clk-composite-8m: Switch to clk_hw based API
clk: imx: add imx_unregister_hw_clocks
clk: imx: add hw API imx_clk_hw_mux2_flags
clk: imx: frac-pll: Switch to clk_hw based API
clk: imx: sccg-pll: Switch to clk_hw based API
clk: imx: gate3: Switch to clk_hw based API

drivers/clk/imx/clk-composite-8m.c | 4 +--
drivers/clk/imx/clk-frac-pll.c | 4 +--
drivers/clk/imx/clk-pll14xx.c | 22 ++++++++-----
drivers/clk/imx/clk-sccg-pll.c | 4 +--
drivers/clk/imx/clk.c | 8 +++++
drivers/clk/imx/clk.h | 67 ++++++++++++++++++++++++++++++--------
6 files changed, 80 insertions(+), 29 deletions(-)

--
2.16.4


2019-10-29 13:42:08

by Peng Fan

[permalink] [raw]
Subject: [PATCH 1/7] clk: imx: clk-pll14xx: Switch to clk_hw based API

From: Peng Fan <[email protected]>

Switch the imx_clk_pll14xx function to clk_hw based API, rename
accordingly and add a macro for clk based legacy. This allows us to
move closer to a clear split between consumer and provider clk APIs.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk-pll14xx.c | 22 +++++++++++++---------
drivers/clk/imx/clk.h | 8 ++++++--
2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 5c458199060a..fa76e04251c4 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -369,13 +369,14 @@ static const struct clk_ops clk_pll1443x_ops = {
.set_rate = clk_pll1443x_set_rate,
};

-struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
- void __iomem *base,
- const struct imx_pll14xx_clk *pll_clk)
+struct clk_hw *imx_clk_hw_pll14xx(const char *name, const char *parent_name,
+ void __iomem *base,
+ const struct imx_pll14xx_clk *pll_clk)
{
struct clk_pll14xx *pll;
- struct clk *clk;
+ struct clk_hw *hw;
struct clk_init_data init;
+ int ret;
u32 val;

pll = kzalloc(sizeof(*pll), GFP_KERNEL);
@@ -412,12 +413,15 @@ struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
val &= ~BYPASS_MASK;
writel_relaxed(val, pll->base + GNRL_CTL);

- clk = clk_register(NULL, &pll->hw);
- if (IS_ERR(clk)) {
- pr_err("%s: failed to register pll %s %lu\n",
- __func__, name, PTR_ERR(clk));
+ hw = &pll->hw;
+
+ ret = clk_hw_register(NULL, hw);
+ if (ret) {
+ pr_err("%s: failed to register pll %s %d\n",
+ __func__, name, ret);
kfree(pll);
+ return ERR_PTR(ret);
}

- return clk;
+ return hw;
}
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index bc5bb6ac8636..5038622f1a72 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -97,8 +97,12 @@ extern struct imx_pll14xx_clk imx_1443x_pll;
#define imx_clk_mux(name, reg, shift, width, parents, num_parents) \
imx_clk_hw_mux(name, reg, shift, width, parents, num_parents)->clk

-struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
- void __iomem *base, const struct imx_pll14xx_clk *pll_clk);
+#define imx_clk_pll14xx(name, parent_name, base, pll_clk) \
+ imx_clk_hw_pll14xx(name, parent_name, base, pll_clk)->clk
+
+struct clk_hw *imx_clk_hw_pll14xx(const char *name, const char *parent_name,
+ void __iomem *base,
+ const struct imx_pll14xx_clk *pll_clk);

struct clk *imx_clk_pllv1(enum imx_pllv1_type type, const char *name,
const char *parent, void __iomem *base);
--
2.16.4

2019-10-29 13:43:31

by Peng Fan

[permalink] [raw]
Subject: [PATCH 5/7] clk: imx: frac-pll: Switch to clk_hw based API

From: Peng Fan <[email protected]>

Switch the imx_clk_frac_pll function to clk_hw based API, rename
accordingly and add a macro for clk based legacy. This allows us to
move closer to a clear split between consumer and provider clk APIs.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk-frac-pll.c | 4 ++--
drivers/clk/imx/clk.h | 6 ++++--
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/imx/clk-frac-pll.c b/drivers/clk/imx/clk-frac-pll.c
index fece503e3610..818ddb35329f 100644
--- a/drivers/clk/imx/clk-frac-pll.c
+++ b/drivers/clk/imx/clk-frac-pll.c
@@ -201,7 +201,7 @@ static const struct clk_ops clk_frac_pll_ops = {
.set_rate = clk_pll_set_rate,
};

-struct clk *imx_clk_frac_pll(const char *name, const char *parent_name,
+struct clk_hw *imx_clk_hw_frac_pll(const char *name, const char *parent_name,
void __iomem *base)
{
struct clk_init_data init;
@@ -230,5 +230,5 @@ struct clk *imx_clk_frac_pll(const char *name, const char *parent_name,
return ERR_PTR(ret);
}

- return hw->clk;
+ return hw;
}
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 49cbf6e20ad8..a260b8dd3256 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -111,8 +111,10 @@ struct clk *imx_clk_pllv1(enum imx_pllv1_type type, const char *name,
struct clk *imx_clk_pllv2(const char *name, const char *parent,
void __iomem *base);

-struct clk *imx_clk_frac_pll(const char *name, const char *parent_name,
- void __iomem *base);
+struct clk_hw *imx_clk_hw_frac_pll(const char *name, const char *parent_name,
+ void __iomem *base);
+#define imx_clk_frac_pll(name, parent_name, base) \
+ imx_clk_hw_frac_pll(name, parent_name, base)->clk

struct clk *imx_clk_sccg_pll(const char *name,
const char * const *parent_names,
--
2.16.4

2019-10-29 13:44:06

by Peng Fan

[permalink] [raw]
Subject: [PATCH 6/7] clk: imx: sccg-pll: Switch to clk_hw based API

From: Peng Fan <[email protected]>

Switch the imx_clk_sccg_pll function to clk_hw based API, rename
accordingly and add a macro for clk based legacy. This allows us to
move closer to a clear split between consumer and provider clk APIs.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk-sccg-pll.c | 4 ++--
drivers/clk/imx/clk.h | 8 ++++++--
2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/imx/clk-sccg-pll.c b/drivers/clk/imx/clk-sccg-pll.c
index 5d65f65c512e..2cf874813458 100644
--- a/drivers/clk/imx/clk-sccg-pll.c
+++ b/drivers/clk/imx/clk-sccg-pll.c
@@ -506,7 +506,7 @@ static const struct clk_ops clk_sccg_pll_ops = {
.determine_rate = clk_sccg_pll_determine_rate,
};

-struct clk *imx_clk_sccg_pll(const char *name,
+struct clk_hw *imx_clk_hw_sccg_pll(const char *name,
const char * const *parent_names,
u8 num_parents,
u8 parent, u8 bypass1, u8 bypass2,
@@ -545,5 +545,5 @@ struct clk *imx_clk_sccg_pll(const char *name,
return ERR_PTR(ret);
}

- return hw->clk;
+ return hw;
}
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index a260b8dd3256..9de6bc590638 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -116,13 +116,17 @@ struct clk_hw *imx_clk_hw_frac_pll(const char *name, const char *parent_name,
#define imx_clk_frac_pll(name, parent_name, base) \
imx_clk_hw_frac_pll(name, parent_name, base)->clk

-struct clk *imx_clk_sccg_pll(const char *name,
+struct clk_hw *imx_clk_hw_sccg_pll(const char *name,
const char * const *parent_names,
u8 num_parents,
u8 parent, u8 bypass1, u8 bypass2,
void __iomem *base,
unsigned long flags);
-
+#define imx_clk_sccg_pll(name, parent_names, num_parents, parent, \
+ bypass1, bypass2, base, flags) \
+ imx_clk_hw_sccg_pll(name, parent_names, num_parents, parent, \
+ bypass1, bypass2, base, flags)->clk
+
enum imx_pllv3_type {
IMX_PLLV3_GENERIC,
IMX_PLLV3_SYS,
--
2.16.4

2019-10-29 13:46:32

by Peng Fan

[permalink] [raw]
Subject: [PATCH 7/7] clk: imx: gate3: Switch to clk_hw based API

From: Peng Fan <[email protected]>

Switch the imx_clk_hw_gate3_flags function to clk_hw based API, rename
accordingly and add a macro for clk based legacy. This allows us to
move closer to a clear split between consumer and provider clk APIs.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 9de6bc590638..cd92d9fdccf4 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -366,15 +366,18 @@ static inline struct clk_hw *imx_clk_hw_gate3(const char *name, const char *pare
reg, shift, 0, &imx_ccm_lock);
}

-static inline struct clk *imx_clk_gate3_flags(const char *name,
+static inline struct clk_hw *imx_clk_hw_gate3_flags(const char *name,
const char *parent, void __iomem *reg, u8 shift,
unsigned long flags)
{
- return clk_register_gate(NULL, name, parent,
+ return clk_hw_register_gate(NULL, name, parent,
flags | CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
reg, shift, 0, &imx_ccm_lock);
}

+#define imx_clk_gate3_flags(name, parent, reg, shift, flags) \
+ imx_clk_hw_gate3_flags(name, parent, reg, shift, flags)->clk
+
static inline struct clk_hw *imx_clk_hw_gate4(const char *name, const char *parent,
void __iomem *reg, u8 shift)
{
--
2.16.4

2019-10-29 19:41:49

by Peng Fan

[permalink] [raw]
Subject: [PATCH 2/7] clk: imx: clk-composite-8m: Switch to clk_hw based API

From: Peng Fan <[email protected]>

Switch the imx8m_clk_hw_composite_flags function to clk_hw based API,
rename accordingly and add a macro for clk based legacy. This allows
us to move closer to a clear split between consumer and provider clk
APIs.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk-composite-8m.c | 4 ++--
drivers/clk/imx/clk.h | 29 ++++++++++++++++++++++-------
2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c
index 388bdb94f841..e0f25983e80f 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -123,7 +123,7 @@ static const struct clk_ops imx8m_clk_composite_divider_ops = {
.set_rate = imx8m_clk_composite_divider_set_rate,
};

-struct clk *imx8m_clk_composite_flags(const char *name,
+struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,
const char * const *parent_names,
int num_parents, void __iomem *reg,
unsigned long flags)
@@ -169,7 +169,7 @@ struct clk *imx8m_clk_composite_flags(const char *name,
if (IS_ERR(hw))
goto fail;

- return hw->clk;
+ return hw;

fail:
kfree(gate);
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 5038622f1a72..e67f7d4ab1dd 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -450,19 +450,34 @@ struct clk_hw *imx_clk_hw_cpu(const char *name, const char *parent_name,
struct clk *div, struct clk *mux, struct clk *pll,
struct clk *step);

-struct clk *imx8m_clk_composite_flags(const char *name,
- const char * const *parent_names,
- int num_parents, void __iomem *reg,
- unsigned long flags);
-
-#define __imx8m_clk_composite(name, parent_names, reg, flags) \
- imx8m_clk_composite_flags(name, parent_names, \
+struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,
+ const char * const *parent_names,
+ int num_parents,
+ void __iomem *reg,
+ unsigned long flags);
+
+#define imx8m_clk_composite_flags(name, parent_names, num_parents, reg, \
+ flags) \
+ imx8m_clk_hw_composite_flags(name, parent_names, num_parents, \
+ reg, flags)->clk
+
+#define __imx8m_clk_hw_composite(name, parent_names, reg, flags) \
+ imx8m_clk_hw_composite_flags(name, parent_names, \
ARRAY_SIZE(parent_names), reg, \
flags | CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE)

+#define __imx8m_clk_composite(name, parent_names, reg, flags) \
+ __imx8m_clk_hw_composite(name, parent_names, reg, flags)->clk
+
+#define imx8m_clk_hw_composite(name, parent_names, reg) \
+ __imx8m_clk_hw_composite(name, parent_names, reg, 0)
+
#define imx8m_clk_composite(name, parent_names, reg) \
__imx8m_clk_composite(name, parent_names, reg, 0)

+#define imx8m_clk_hw_composite_critical(name, parent_names, reg) \
+ __imx8m_clk_hw_composite(name, parent_names, reg, CLK_IS_CRITICAL)
+
#define imx8m_clk_composite_critical(name, parent_names, reg) \
__imx8m_clk_composite(name, parent_names, reg, CLK_IS_CRITICAL)

--
2.16.4

2019-10-29 19:41:51

by Peng Fan

[permalink] [raw]
Subject: [PATCH 4/7] clk: imx: add hw API imx_clk_hw_mux2_flags

From: Peng Fan <[email protected]>

Introduce hw based API imx_clk_hw_mux2_flags, then we could
convert i.MX8MN clk driver to use hw based APIs.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk.h | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 7d6157f6dcf9..49cbf6e20ad8 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -425,6 +425,16 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
&imx_ccm_lock);
}

+static inline struct clk_hw *imx_clk_hw_mux2_flags(const char *name,
+ void __iomem *reg, u8 shift, u8 width,
+ const char * const *parents,
+ int num_parents, unsigned long flags)
+{
+ return clk_hw_register_mux(NULL, name, parents, num_parents,
+ flags | CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
+ reg, shift, width, 0, &imx_ccm_lock);
+}
+
static inline struct clk *imx_clk_mux2_flags(const char *name,
void __iomem *reg, u8 shift, u8 width,
const char * const *parents,
--
2.16.4

2019-10-29 19:41:51

by Peng Fan

[permalink] [raw]
Subject: [PATCH 3/7] clk: imx: add imx_unregister_hw_clocks

From: Peng Fan <[email protected]>

There is a non hw API based imx_unregister_clocks to unregister clocks
when of_clk_add_provider failed. Add a hw API based
imx_unregister_hw_clocks when of_clk_add_hw_provider failed.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk.c | 8 ++++++++
drivers/clk/imx/clk.h | 1 +
2 files changed, 9 insertions(+)

diff --git a/drivers/clk/imx/clk.c b/drivers/clk/imx/clk.c
index cfc05e43c343..d0ce29f2c417 100644
--- a/drivers/clk/imx/clk.c
+++ b/drivers/clk/imx/clk.c
@@ -22,6 +22,14 @@ void imx_unregister_clocks(struct clk *clks[], unsigned int count)
clk_unregister(clks[i]);
}

+void imx_unregister_hw_clocks(struct clk_hw *hws[], unsigned int count)
+{
+ unsigned int i;
+
+ for (i = 0; i < count; i++)
+ clk_hw_unregister(hws[i]);
+}
+
void __init imx_mmdc_mask_handshake(void __iomem *ccm_base,
unsigned int chn)
{
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index e67f7d4ab1dd..7d6157f6dcf9 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -12,6 +12,7 @@ void imx_check_clk_hws(struct clk_hw *clks[], unsigned int count);
void imx_register_uart_clocks(struct clk ** const clks[]);
void imx_mmdc_mask_handshake(void __iomem *ccm_base, unsigned int chn);
void imx_unregister_clocks(struct clk *clks[], unsigned int count);
+void imx_unregister_hw_clocks(struct clk_hw *hws[], unsigned int count);

extern void imx_cscmr1_fixup(u32 *val);

--
2.16.4

2019-10-30 10:23:26

by Abel Vesa

[permalink] [raw]
Subject: Re: [PATCH 0/7] clk: imx: switch to clk_hw based API

On 19-10-29 13:40:49, Peng Fan wrote:
> From: Peng Fan <[email protected]>
>
> This is a preparation patch set to switch i.MX8MM/N/Q clk
> driver to clk_hw based API.
>
> There are some patches under reviewing for i.MX8M clk driver,
> to avoid conflicts, so not include i.MX8M clk_hw patches in this
> pach set.
>
> The patch set covers the APIs used by i.MX8M clk driver.
>

Thanks for working on this.

The entire series looks good.

Reviewed-by: Abel Vesa <[email protected]>

> Peng Fan (7):
> clk: imx: clk-pll14xx: Switch to clk_hw based API
> clk: imx: clk-composite-8m: Switch to clk_hw based API
> clk: imx: add imx_unregister_hw_clocks
> clk: imx: add hw API imx_clk_hw_mux2_flags
> clk: imx: frac-pll: Switch to clk_hw based API
> clk: imx: sccg-pll: Switch to clk_hw based API
> clk: imx: gate3: Switch to clk_hw based API
>
> drivers/clk/imx/clk-composite-8m.c | 4 +--
> drivers/clk/imx/clk-frac-pll.c | 4 +--
> drivers/clk/imx/clk-pll14xx.c | 22 ++++++++-----
> drivers/clk/imx/clk-sccg-pll.c | 4 +--
> drivers/clk/imx/clk.c | 8 +++++
> drivers/clk/imx/clk.h | 67 ++++++++++++++++++++++++++++++--------
> 6 files changed, 80 insertions(+), 29 deletions(-)
>
> --
> 2.16.4
>

2019-12-02 01:19:43

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH 1/7] clk: imx: clk-pll14xx: Switch to clk_hw based API

On Tue, Oct 29, 2019 at 01:40:54PM +0000, Peng Fan wrote:
> From: Peng Fan <[email protected]>
>
> Switch the imx_clk_pll14xx function to clk_hw based API, rename
> accordingly and add a macro for clk based legacy. This allows us to
> move closer to a clear split between consumer and provider clk APIs.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/clk/imx/clk-pll14xx.c | 22 +++++++++++++---------
> drivers/clk/imx/clk.h | 8 ++++++--
> 2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> index 5c458199060a..fa76e04251c4 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -369,13 +369,14 @@ static const struct clk_ops clk_pll1443x_ops = {
> .set_rate = clk_pll1443x_set_rate,
> };
>
> -struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
> - void __iomem *base,
> - const struct imx_pll14xx_clk *pll_clk)
> +struct clk_hw *imx_clk_hw_pll14xx(const char *name, const char *parent_name,
> + void __iomem *base,
> + const struct imx_pll14xx_clk *pll_clk)
> {
> struct clk_pll14xx *pll;
> - struct clk *clk;
> + struct clk_hw *hw;
> struct clk_init_data init;
> + int ret;
> u32 val;
>
> pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> @@ -412,12 +413,15 @@ struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
> val &= ~BYPASS_MASK;
> writel_relaxed(val, pll->base + GNRL_CTL);
>
> - clk = clk_register(NULL, &pll->hw);
> - if (IS_ERR(clk)) {
> - pr_err("%s: failed to register pll %s %lu\n",
> - __func__, name, PTR_ERR(clk));
> + hw = &pll->hw;
> +
> + ret = clk_hw_register(NULL, hw);
> + if (ret) {
> + pr_err("%s: failed to register pll %s %d\n",
> + __func__, name, ret);
> kfree(pll);
> + return ERR_PTR(ret);
> }
>
> - return clk;
> + return hw;
> }
> diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
> index bc5bb6ac8636..5038622f1a72 100644
> --- a/drivers/clk/imx/clk.h
> +++ b/drivers/clk/imx/clk.h
> @@ -97,8 +97,12 @@ extern struct imx_pll14xx_clk imx_1443x_pll;
> #define imx_clk_mux(name, reg, shift, width, parents, num_parents) \
> imx_clk_hw_mux(name, reg, shift, width, parents, num_parents)->clk
>
> -struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
> - void __iomem *base, const struct imx_pll14xx_clk *pll_clk);
> +#define imx_clk_pll14xx(name, parent_name, base, pll_clk) \
> + imx_clk_hw_pll14xx(name, parent_name, base, pll_clk)->clk
> +

I would suggest to use an inline function for this, which will be more
readable and complying to kernel coding style.

Shawn

> +struct clk_hw *imx_clk_hw_pll14xx(const char *name, const char *parent_name,
> + void __iomem *base,
> + const struct imx_pll14xx_clk *pll_clk);
>
> struct clk *imx_clk_pllv1(enum imx_pllv1_type type, const char *name,
> const char *parent, void __iomem *base);
> --
> 2.16.4
>

2019-12-02 02:30:47

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH 1/7] clk: imx: clk-pll14xx: Switch to clk_hw based API

Hi Shawn,

> Subject: Re: [PATCH 1/7] clk: imx: clk-pll14xx: Switch to clk_hw based API
>
> On Tue, Oct 29, 2019 at 01:40:54PM +0000, Peng Fan wrote:
> > From: Peng Fan <[email protected]>
> >
> > Switch the imx_clk_pll14xx function to clk_hw based API, rename
> > accordingly and add a macro for clk based legacy. This allows us to
> > move closer to a clear split between consumer and provider clk APIs.
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> > drivers/clk/imx/clk-pll14xx.c | 22 +++++++++++++---------
> > drivers/clk/imx/clk.h | 8 ++++++--
> > 2 files changed, 19 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-pll14xx.c
> > b/drivers/clk/imx/clk-pll14xx.c index 5c458199060a..fa76e04251c4
> > 100644
> > --- a/drivers/clk/imx/clk-pll14xx.c
> > +++ b/drivers/clk/imx/clk-pll14xx.c
> > @@ -369,13 +369,14 @@ static const struct clk_ops clk_pll1443x_ops = {
> > .set_rate = clk_pll1443x_set_rate,
> > };
> >
> > -struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
> > - void __iomem *base,
> > - const struct imx_pll14xx_clk *pll_clk)
> > +struct clk_hw *imx_clk_hw_pll14xx(const char *name, const char
> *parent_name,
> > + void __iomem *base,
> > + const struct imx_pll14xx_clk *pll_clk)
> > {
> > struct clk_pll14xx *pll;
> > - struct clk *clk;
> > + struct clk_hw *hw;
> > struct clk_init_data init;
> > + int ret;
> > u32 val;
> >
> > pll = kzalloc(sizeof(*pll), GFP_KERNEL); @@ -412,12 +413,15 @@
> > struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
> > val &= ~BYPASS_MASK;
> > writel_relaxed(val, pll->base + GNRL_CTL);
> >
> > - clk = clk_register(NULL, &pll->hw);
> > - if (IS_ERR(clk)) {
> > - pr_err("%s: failed to register pll %s %lu\n",
> > - __func__, name, PTR_ERR(clk));
> > + hw = &pll->hw;
> > +
> > + ret = clk_hw_register(NULL, hw);
> > + if (ret) {
> > + pr_err("%s: failed to register pll %s %d\n",
> > + __func__, name, ret);
> > kfree(pll);
> > + return ERR_PTR(ret);
> > }
> >
> > - return clk;
> > + return hw;
> > }
> > diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h index
> > bc5bb6ac8636..5038622f1a72 100644
> > --- a/drivers/clk/imx/clk.h
> > +++ b/drivers/clk/imx/clk.h
> > @@ -97,8 +97,12 @@ extern struct imx_pll14xx_clk imx_1443x_pll;
> > #define imx_clk_mux(name, reg, shift, width, parents, num_parents) \
> > imx_clk_hw_mux(name, reg, shift, width, parents, num_parents)->clk
> >
> > -struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
> > - void __iomem *base, const struct imx_pll14xx_clk *pll_clk);
> > +#define imx_clk_pll14xx(name, parent_name, base, pll_clk) \
> > + imx_clk_hw_pll14xx(name, parent_name, base, pll_clk)->clk
> > +
>
> I would suggest to use an inline function for this, which will be more readable
> and complying to kernel coding style.

ok, I'll send out v2.

Thanks,
Peng.

>
> Shawn
>
> > +struct clk_hw *imx_clk_hw_pll14xx(const char *name, const char
> *parent_name,
> > + void __iomem *base,
> > + const struct imx_pll14xx_clk *pll_clk);
> >
> > struct clk *imx_clk_pllv1(enum imx_pllv1_type type, const char *name,
> > const char *parent, void __iomem *base);
> > --
> > 2.16.4
> >

2019-12-02 05:10:27

by Leonard Crestez

[permalink] [raw]
Subject: Re: [PATCH 1/7] clk: imx: clk-pll14xx: Switch to clk_hw based API

On 2019-12-02 4:26 AM, Peng Fan wrote:
>> Subject: Re: [PATCH 1/7] clk: imx: clk-pll14xx: Switch to clk_hw based API
>> On Tue, Oct 29, 2019 at 01:40:54PM +0000, Peng Fan wrote:
>>> From: Peng Fan <[email protected]>
>>>
>>> Switch the imx_clk_pll14xx function to clk_hw based API, rename
>>> accordingly and add a macro for clk based legacy. This allows us to
>>> move closer to a clear split between consumer and provider clk APIs.
>>>
>>> diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h index
>>> bc5bb6ac8636..5038622f1a72 100644
>>> --- a/drivers/clk/imx/clk.h
>>> +++ b/drivers/clk/imx/clk.h
>>> @@ -97,8 +97,12 @@ extern struct imx_pll14xx_clk imx_1443x_pll;
>>> #define imx_clk_mux(name, reg, shift, width, parents, num_parents) \
>>> imx_clk_hw_mux(name, reg, shift, width, parents, num_parents)->clk
>>>
>>> -struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
>>> - void __iomem *base, const struct imx_pll14xx_clk *pll_clk);
>>> +#define imx_clk_pll14xx(name, parent_name, base, pll_clk) \
>>> + imx_clk_hw_pll14xx(name, parent_name, base, pll_clk)->clk
>>> +
>>
>> I would suggest to use an inline function for this, which will be more readable
>> and complying to kernel coding style.
>
> ok, I'll send out v2.

Blindly dereferencing ->clk will crash instead of propagating errors so
you might want to use the to_clk helper from here:

https://patchwork.kernel.org/patch/11257811/

--
Regards,
Leonard

2019-12-02 05:13:23

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH 1/7] clk: imx: clk-pll14xx: Switch to clk_hw based API

> Subject: Re: [PATCH 1/7] clk: imx: clk-pll14xx: Switch to clk_hw based API
>
> On 2019-12-02 4:26 AM, Peng Fan wrote:
> >> Subject: Re: [PATCH 1/7] clk: imx: clk-pll14xx: Switch to clk_hw
> >> based API On Tue, Oct 29, 2019 at 01:40:54PM +0000, Peng Fan wrote:
> >>> From: Peng Fan <[email protected]>
> >>>
> >>> Switch the imx_clk_pll14xx function to clk_hw based API, rename
> >>> accordingly and add a macro for clk based legacy. This allows us to
> >>> move closer to a clear split between consumer and provider clk APIs.
> >>>
> >>> diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h index
> >>> bc5bb6ac8636..5038622f1a72 100644
> >>> --- a/drivers/clk/imx/clk.h
> >>> +++ b/drivers/clk/imx/clk.h
> >>> @@ -97,8 +97,12 @@ extern struct imx_pll14xx_clk imx_1443x_pll;
> >>> #define imx_clk_mux(name, reg, shift, width, parents, num_parents) \
> >>> imx_clk_hw_mux(name, reg, shift, width, parents,
> >>> num_parents)->clk
> >>>
> >>> -struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
> >>> - void __iomem *base, const struct imx_pll14xx_clk *pll_clk);
> >>> +#define imx_clk_pll14xx(name, parent_name, base, pll_clk) \
> >>> + imx_clk_hw_pll14xx(name, parent_name, base, pll_clk)->clk
> >>> +
> >>
> >> I would suggest to use an inline function for this, which will be
> >> more readable and complying to kernel coding style.
> >
> > ok, I'll send out v2.
>
> Blindly dereferencing ->clk will crash instead of propagating errors so you
> might want to use the to_clk helper from here:
>
> https://patchwork.kernel.org/patch/11257811/

I not see Abel's patch applied, should I include Abel's patchset in my v2?
Or I posted out my v2 with only my changes based on Abel's patchset?

Thanks,
Peng.

>
> --
> Regards,
> Leonard