2020-09-02 06:52:12

by zhangqing

[permalink] [raw]
Subject: [PATCH v1 1/6] clk: rockchip: Use clk_hw_register_composite instead of clk_register_composite calls

clk_hw_register_composite it's already exported.
Preparation for compilation of rK common clock drivers into modules.

Signed-off-by: Elaine Zhang <[email protected]>
---
drivers/clk/rockchip/clk-half-divider.c | 12 +++++----
drivers/clk/rockchip/clk.c | 35 ++++++++++++++-----------
2 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/clk/rockchip/clk-half-divider.c b/drivers/clk/rockchip/clk-half-divider.c
index b333fc28c94b..35db0651ea1d 100644
--- a/drivers/clk/rockchip/clk-half-divider.c
+++ b/drivers/clk/rockchip/clk-half-divider.c
@@ -166,6 +166,7 @@ struct clk *rockchip_clk_register_halfdiv(const char *name,
unsigned long flags,
spinlock_t *lock)
{
+ struct clk_hw *hw;
struct clk *clk;
struct clk_mux *mux = NULL;
struct clk_gate *gate = NULL;
@@ -212,12 +213,13 @@ struct clk *rockchip_clk_register_halfdiv(const char *name,
div_ops = &clk_half_divider_ops;
}

- clk = clk_register_composite(NULL, name, parent_names, num_parents,
- mux ? &mux->hw : NULL, mux_ops,
- div ? &div->hw : NULL, div_ops,
- gate ? &gate->hw : NULL, gate_ops,
- flags);
+ hw = clk_hw_register_composite(NULL, name, parent_names, num_parents,
+ mux ? &mux->hw : NULL, mux_ops,
+ div ? &div->hw : NULL, div_ops,
+ gate ? &gate->hw : NULL, gate_ops,
+ flags);

+ clk = hw->clk;
return clk;
err_div:
kfree(gate);
diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index 546e810c3560..2cfebfb61814 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -43,6 +43,7 @@ static struct clk *rockchip_clk_register_branch(const char *name,
u8 gate_shift, u8 gate_flags, unsigned long flags,
spinlock_t *lock)
{
+ struct clk_hw *hw;
struct clk *clk;
struct clk_mux *mux = NULL;
struct clk_gate *gate = NULL;
@@ -100,12 +101,12 @@ static struct clk *rockchip_clk_register_branch(const char *name,
: &clk_divider_ops;
}

- clk = clk_register_composite(NULL, name, parent_names, num_parents,
- mux ? &mux->hw : NULL, mux_ops,
- div ? &div->hw : NULL, div_ops,
- gate ? &gate->hw : NULL, gate_ops,
- flags);
-
+ hw = clk_hw_register_composite(NULL, name, parent_names, num_parents,
+ mux ? &mux->hw : NULL, mux_ops,
+ div ? &div->hw : NULL, div_ops,
+ gate ? &gate->hw : NULL, gate_ops,
+ flags);
+ clk = hw->clk;
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
goto err_composite;
@@ -214,6 +215,7 @@ static struct clk *rockchip_clk_register_frac_branch(
unsigned long flags, struct rockchip_clk_branch *child,
spinlock_t *lock)
{
+ struct clk_hw *hw;
struct rockchip_clk_frac *frac;
struct clk *clk;
struct clk_gate *gate = NULL;
@@ -255,11 +257,12 @@ static struct clk *rockchip_clk_register_frac_branch(
div->approximation = rockchip_fractional_approximation;
div_ops = &clk_fractional_divider_ops;

- clk = clk_register_composite(NULL, name, parent_names, num_parents,
- NULL, NULL,
- &div->hw, div_ops,
- gate ? &gate->hw : NULL, gate_ops,
- flags | CLK_SET_RATE_UNGATE);
+ hw = clk_hw_register_composite(NULL, name, parent_names, num_parents,
+ NULL, NULL,
+ &div->hw, div_ops,
+ gate ? &gate->hw : NULL, gate_ops,
+ flags | CLK_SET_RATE_UNGATE);
+ clk = hw->clk;
if (IS_ERR(clk)) {
kfree(frac);
return clk;
@@ -320,6 +323,7 @@ static struct clk *rockchip_clk_register_factor_branch(const char *name,
int gate_offset, u8 gate_shift, u8 gate_flags,
unsigned long flags, spinlock_t *lock)
{
+ struct clk_hw *hw;
struct clk *clk;
struct clk_gate *gate = NULL;
struct clk_fixed_factor *fix = NULL;
@@ -349,10 +353,11 @@ static struct clk *rockchip_clk_register_factor_branch(const char *name,
fix->mult = mult;
fix->div = div;

- clk = clk_register_composite(NULL, name, parent_names, num_parents,
- NULL, NULL,
- &fix->hw, &clk_fixed_factor_ops,
- &gate->hw, &clk_gate_ops, flags);
+ hw = clk_hw_register_composite(NULL, name, parent_names, num_parents,
+ NULL, NULL,
+ &fix->hw, &clk_fixed_factor_ops,
+ &gate->hw, &clk_gate_ops, flags);
+ clk = hw->clk;
if (IS_ERR(clk)) {
kfree(fix);
kfree(gate);
--
2.17.1




2020-09-02 15:55:25

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v1 1/6] clk: rockchip: Use clk_hw_register_composite instead of clk_register_composite calls

On 2020-09-02 07:48, Elaine Zhang wrote:
> clk_hw_register_composite it's already exported.
> Preparation for compilation of rK common clock drivers into modules.
>
> Signed-off-by: Elaine Zhang <[email protected]>
> ---
> drivers/clk/rockchip/clk-half-divider.c | 12 +++++----
> drivers/clk/rockchip/clk.c | 35 ++++++++++++++-----------
> 2 files changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/clk/rockchip/clk-half-divider.c b/drivers/clk/rockchip/clk-half-divider.c
> index b333fc28c94b..35db0651ea1d 100644
> --- a/drivers/clk/rockchip/clk-half-divider.c
> +++ b/drivers/clk/rockchip/clk-half-divider.c
> @@ -166,6 +166,7 @@ struct clk *rockchip_clk_register_halfdiv(const char *name,
> unsigned long flags,
> spinlock_t *lock)
> {
> + struct clk_hw *hw;
> struct clk *clk;
> struct clk_mux *mux = NULL;
> struct clk_gate *gate = NULL;
> @@ -212,12 +213,13 @@ struct clk *rockchip_clk_register_halfdiv(const char *name,
> div_ops = &clk_half_divider_ops;
> }
>
> - clk = clk_register_composite(NULL, name, parent_names, num_parents,
> - mux ? &mux->hw : NULL, mux_ops,
> - div ? &div->hw : NULL, div_ops,
> - gate ? &gate->hw : NULL, gate_ops,
> - flags);
> + hw = clk_hw_register_composite(NULL, name, parent_names, num_parents,
> + mux ? &mux->hw : NULL, mux_ops,
> + div ? &div->hw : NULL, div_ops,
> + gate ? &gate->hw : NULL, gate_ops,
> + flags);
>
> + clk = hw->clk;
> return clk;

Nit: there's really no point keeping the "clk" variable here, you could
simply "return hw->clk" if registration succeeds - note that you also
need the rest of the logic from clk_register_composite() to check that
"hw" isn't an error value.

> err_div:
> kfree(gate);
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index 546e810c3560..2cfebfb61814 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -43,6 +43,7 @@ static struct clk *rockchip_clk_register_branch(const char *name,
> u8 gate_shift, u8 gate_flags, unsigned long flags,
> spinlock_t *lock)
> {
> + struct clk_hw *hw;
> struct clk *clk;
> struct clk_mux *mux = NULL;
> struct clk_gate *gate = NULL;
> @@ -100,12 +101,12 @@ static struct clk *rockchip_clk_register_branch(const char *name,
> : &clk_divider_ops;
> }
>
> - clk = clk_register_composite(NULL, name, parent_names, num_parents,
> - mux ? &mux->hw : NULL, mux_ops,
> - div ? &div->hw : NULL, div_ops,
> - gate ? &gate->hw : NULL, gate_ops,
> - flags);
> -
> + hw = clk_hw_register_composite(NULL, name, parent_names, num_parents,
> + mux ? &mux->hw : NULL, mux_ops,
> + div ? &div->hw : NULL, div_ops,
> + gate ? &gate->hw : NULL, gate_ops,
> + flags);
> + clk = hw->clk;
> if (IS_ERR(clk)) {

Similar to above, this is totally broken - you need to rework all the
error handling in terms of "hw" rather than "clk" - dereferencing an
ERR_PTR value does not yield another ERR_PTR value, it yields a crash ;)

Robin.

> ret = PTR_ERR(clk);
> goto err_composite;
> @@ -214,6 +215,7 @@ static struct clk *rockchip_clk_register_frac_branch(
> unsigned long flags, struct rockchip_clk_branch *child,
> spinlock_t *lock)
> {
> + struct clk_hw *hw;
> struct rockchip_clk_frac *frac;
> struct clk *clk;
> struct clk_gate *gate = NULL;
> @@ -255,11 +257,12 @@ static struct clk *rockchip_clk_register_frac_branch(
> div->approximation = rockchip_fractional_approximation;
> div_ops = &clk_fractional_divider_ops;
>
> - clk = clk_register_composite(NULL, name, parent_names, num_parents,
> - NULL, NULL,
> - &div->hw, div_ops,
> - gate ? &gate->hw : NULL, gate_ops,
> - flags | CLK_SET_RATE_UNGATE);
> + hw = clk_hw_register_composite(NULL, name, parent_names, num_parents,
> + NULL, NULL,
> + &div->hw, div_ops,
> + gate ? &gate->hw : NULL, gate_ops,
> + flags | CLK_SET_RATE_UNGATE);
> + clk = hw->clk;
> if (IS_ERR(clk)) {
> kfree(frac);
> return clk;
> @@ -320,6 +323,7 @@ static struct clk *rockchip_clk_register_factor_branch(const char *name,
> int gate_offset, u8 gate_shift, u8 gate_flags,
> unsigned long flags, spinlock_t *lock)
> {
> + struct clk_hw *hw;
> struct clk *clk;
> struct clk_gate *gate = NULL;
> struct clk_fixed_factor *fix = NULL;
> @@ -349,10 +353,11 @@ static struct clk *rockchip_clk_register_factor_branch(const char *name,
> fix->mult = mult;
> fix->div = div;
>
> - clk = clk_register_composite(NULL, name, parent_names, num_parents,
> - NULL, NULL,
> - &fix->hw, &clk_fixed_factor_ops,
> - &gate->hw, &clk_gate_ops, flags);
> + hw = clk_hw_register_composite(NULL, name, parent_names, num_parents,
> + NULL, NULL,
> + &fix->hw, &clk_fixed_factor_ops,
> + &gate->hw, &clk_gate_ops, flags);
> + clk = hw->clk;
> if (IS_ERR(clk)) {
> kfree(fix);
> kfree(gate);
>