[PATCH 0/7] Some trivial patches for rockchip clk stuff

[PATCH 0/7] Some trivial patches for rockchip clk stuff

[Shawn Lin]

Hi Heiko and Xing,

After reviewing some clk patches for rk3399, I found some
trivial problems. So this is another round of patches to
slightly improve the code.

Based on v4.7-clk/next branch
git://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git



Shawn Lin (7):
  clk: rockchip: remove mux_core_reg from rockchip_cpuclk_reg_data
  clk: rockchip: fix warning reported by kernel-doc
  clk: rockchip: release io resource when rk3036_clk_init failed
  clk: rockchip: release io resource when rk3188_common_clk_init failed
  clk: rockchip: release io resource when rk3368_clk_init failed
  clk: rockchip: remove redundant checking of device_node
  clk: rockchip: fix a typo for rockchip_rk3399_pll_set_params

 drivers/clk/rockchip/clk-pll.c    |  2 +-
 drivers/clk/rockchip/clk-rk3036.c |  1 +
 drivers/clk/rockchip/clk-rk3188.c |  1 +
 drivers/clk/rockchip/clk-rk3368.c |  1 +
 drivers/clk/rockchip/clk.c        |  8 +++-----
 drivers/clk/rockchip/clk.h        | 10 +++++-----
 6 files changed, 12 insertions(+), 11 deletions(-)

-- 
2.3.7

[PATCH 1/7] clk: rockchip: remove mux_core_reg from rockchip_cpuclk_reg_data

[Shawn Lin]

mux_core_reg isn't been used anywhere, let's remove it.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

 drivers/clk/rockchip/clk.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index 4798786..b298f99 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -245,7 +245,6 @@ struct rockchip_cpuclk_reg_data {
 	int		core_reg;
 	u8		div_core_shift;
 	u32		div_core_mask;
-	int		mux_core_reg;
 	u8		mux_core_alt;
 	u8		mux_core_main;
 	u8		mux_core_shift;
-- 
2.3.7

[PATCH 2/7] clk: rockchip: fix warning reported by kernel-doc

[Shawn Lin]

./scripts/kernel-doc -man -v drivers/clk/rockchip/clk.h > /dev/null

drivers/clk/rockchip/clk.h:133: warning: missing initial short
description on line:
 * struct rockchip_clk_provider: information about clock provider
drivers/clk/rockchip/clk.h:133: info: Scanning doc for struct
drivers/clk/rockchip/clk.h:164: warning: missing initial short
description on line:
 * struct rockchip_pll_clock: information about pll clock
drivers/clk/rockchip/clk.h:164: info: Scanning doc for struct
drivers/clk/rockchip/clk.h:194: warning: No description found for
parameter 'parent_names'
drivers/clk/rockchip/clk.h:194: warning: No description found for
parameter 'num_parents'
drivers/clk/rockchip/clk.h:194: warning: Excess struct/union/enum/typedef
member 'parent_name' description in 'rockchip_pll_clock'
drivers/clk/rockchip/clk.h:235: warning: missing initial short
description on line:
 * struct rockchip_cpuclk_reg_data: describes register offsets and
masks of the cpuclock

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/clk/rockchip/clk.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index b298f99..e0f103b 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -130,7 +130,7 @@ enum rockchip_pll_type {
 }
 
 /**
- * struct rockchip_clk_provider: information about clock provider
+ * struct rockchip_clk_provider - information about clock provider
  * @reg_base: virtual address for the register base.
  * @clk_data: holds clock related data like clk* and number of clocks.
  * @cru_node: device-node of the clock-provider
@@ -161,10 +161,11 @@ struct rockchip_pll_rate_table {
 };
 
 /**
- * struct rockchip_pll_clock: information about pll clock
+ * struct rockchip_pll_clock - information about pll clock
  * @id: platform specific id of the clock.
  * @name: name of this pll clock.
- * @parent_name: name of the parent clock.
+ * @parent_names: name of the parent clock.
+ * @num_parents: number of parents
  * @flags: optional flags for basic clock.
  * @con_offset: offset of the register for configuring the PLL.
  * @mode_offset: offset of the register for configuring the PLL-mode.
@@ -232,7 +233,7 @@ struct rockchip_cpuclk_rate_table {
 };
 
 /**
- * struct rockchip_cpuclk_reg_data: describes register offsets and masks of the cpuclock
+ * struct rockchip_cpuclk_reg_data - describes register offsets and masks of the cpuclock
  * @core_reg:		register offset of the core settings register
  * @div_core_shift:	core divider offset used to divide the pll value
  * @div_core_mask:	core divider mask
-- 
2.3.7

[PATCH 3/7] clk: rockchip: release io resource when rk3036_clk_init failed

[Shawn Lin]

We should call iounmap to relase reg_base since it's not going
to be used any more.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/clk/rockchip/clk-rk3036.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
index 8c683cc..284c36f 100644
--- a/drivers/clk/rockchip/clk-rk3036.c
+++ b/drivers/clk/rockchip/clk-rk3036.c
@@ -453,6 +453,7 @@ static void __init rk3036_clk_init(struct device_node *np)
 	ctx = rockchip_clk_init(np, reg_base, CLK_NR_CLKS);
 	if (IS_ERR(ctx)) {
 		pr_err("%s: rockchip clk init failed\n", __func__);
+		iounmap(reg_base);
 		return;
 	}
 
-- 
2.3.7

[PATCH 4/7] clk: rockchip: release io resource when rk3188_common_clk_init failed

[Shawn Lin]

We should call iounmap to relase reg_base since it's not going
to be used any more.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/clk/rockchip/clk-rk3188.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
index 0fcce22..d0e722a 100644
--- a/drivers/clk/rockchip/clk-rk3188.c
+++ b/drivers/clk/rockchip/clk-rk3188.c
@@ -773,6 +773,7 @@ static struct rockchip_clk_provider *__init rk3188_common_clk_init(struct device
 	ctx = rockchip_clk_init(np, reg_base, CLK_NR_CLKS);
 	if (IS_ERR(ctx)) {
 		pr_err("%s: rockchip clk init failed\n", __func__);
+		iounmap(reg_base);
 		return ERR_PTR(-ENOMEM);
 	}
 
-- 
2.3.7

[PATCH 5/7] clk: rockchip: release io resource when rk3368_clk_init failed

[Shawn Lin]

We should call iounmap to relase reg_base since it's not going
to be used any more.w

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/clk/rockchip/clk-rk3368.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/rockchip/clk-rk3368.c b/drivers/clk/rockchip/clk-rk3368.c
index 58690f2..3437289 100644
--- a/drivers/clk/rockchip/clk-rk3368.c
+++ b/drivers/clk/rockchip/clk-rk3368.c
@@ -875,6 +875,7 @@ static void __init rk3368_clk_init(struct device_node *np)
 	ctx = rockchip_clk_init(np, reg_base, CLK_NR_CLKS);
 	if (IS_ERR(ctx)) {
 		pr_err("%s: rockchip clk init failed\n", __func__);
+		iounmap(reg_base);
 		return;
 	}
 
-- 
2.3.7

[PATCH 6/7] clk: rockchip: remove redundant checking of device_node

[Shawn Lin]

rockchip_clk_of_add_provider is used by sub-clk driver which
already call of_iomap before calling it. If device_node does
not exist, of_iomap returns NULL which will fail to init the
sub-clk driver. So really it's redundant.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/clk/rockchip/clk.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index c12db69..7345be4 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -357,11 +357,9 @@ err_free:
 void __init rockchip_clk_of_add_provider(struct device_node *np,
 				struct rockchip_clk_provider *ctx)
 {
-	if (np) {
-		if (of_clk_add_provider(np, of_clk_src_onecell_get,
-					&ctx->clk_data))
-			pr_err("%s: could not register clk provider\n", __func__);
-	}
+	if (of_clk_add_provider(np, of_clk_src_onecell_get,
+				&ctx->clk_data))
+		pr_err("%s: could not register clk provider\n", __func__);
 }
 
 struct regmap *rockchip_clk_get_grf(struct rockchip_clk_provider *ctx)
-- 
2.3.7

[PATCH 7/7] clk: rockchip: fix a typo for rockchip_rk3399_pll_set_params

[Shawn Lin]

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/clk/rockchip/clk-pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
index f490ce4..128a6b2 100644
--- a/drivers/clk/rockchip/clk-pll.c
+++ b/drivers/clk/rockchip/clk-pll.c
@@ -733,7 +733,7 @@ static int rockchip_rk3399_pll_set_params(struct rockchip_clk_pll *pll,
 	/* wait for the pll to lock */
 	ret = rockchip_rk3399_pll_wait_lock(pll);
 	if (ret) {
-		pr_warn("%s: pll update unsucessful, trying to restore old params\n",
+		pr_warn("%s: pll update unsuccessful, trying to restore old params\n",
 			__func__);
 		rockchip_rk3399_pll_set_params(pll, &cur);
 	}
-- 
2.3.7

Re: [PATCH 7/7] clk: rockchip: fix a typo for rockchip_rk3399_pll_set_params

[Heiko Stübner]

Hi Shawn,

Am Sonntag, 13. M=E4rz 2016, 00:26:02 schrieb Shawn Lin:
> --- a/drivers/clk/rockchip/clk-pll.c
> +++ b/drivers/clk/rockchip/clk-pll.c
> @@ -733,7 +733,7 @@ static int rockchip_rk3399_pll_set_params(struct
> rockchip_clk_pll *pll, /* wait for the pll to lock */
>  =09ret =3D rockchip_rk3399_pll_wait_lock(pll);
>  =09if (ret) {
> -=09=09pr_warn("%s: pll update unsucessful, trying to restore old par=
ams\n",
> +=09=09pr_warn("%s: pll update unsuccessful, trying to restore old pa=
rams\n",
>  =09=09=09__func__);
>  =09=09rockchip_rk3399_pll_set_params(pll, &cur);
>  =09}

I've folded that fix into the original pll addition [0]

Thanks for catching that
Heiko


[0] https://git.kernel.org/cgit/linux/kernel/git/mmind/linux-rockchip.g=
it/commit/?h=3Dv4.7-clk/next&id=3D95e0c473a0ac1bdac25f55678dc602eb50dae=
684

Re: [PATCH 3/7] clk: rockchip: release io resource when rk3036_clk_init failed

[Heiko Stübner]

Hi Shawn,

Am Sonntag, 13. M=E4rz 2016, 00:25:25 schrieb Shawn Lin:
> We should call iounmap to relase reg_base since it's not going
> to be used any more.
>=20
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

I see that change for rk3036, rk3188(+rk3066) and rk3368.
But it looks like rk3228 and rk3288 should also get that, or am I just=20=

overlooking something?

Also, I think we can fold these changes into one patch, as it's the com=
pletely=20
same addition for all clock-drivers.


Heiko

> ---
>=20
>  drivers/clk/rockchip/clk-rk3036.c | 1 +
>  1 file changed, 1 insertion(+)
>=20
> diff --git a/drivers/clk/rockchip/clk-rk3036.c
> b/drivers/clk/rockchip/clk-rk3036.c index 8c683cc..284c36f 100644
> --- a/drivers/clk/rockchip/clk-rk3036.c
> +++ b/drivers/clk/rockchip/clk-rk3036.c
> @@ -453,6 +453,7 @@ static void __init rk3036_clk_init(struct device_=
node
> *np) ctx =3D rockchip_clk_init(np, reg_base, CLK_NR_CLKS);
>  =09if (IS_ERR(ctx)) {
>  =09=09pr_err("%s: rockchip clk init failed\n", __func__);
> +=09=09iounmap(reg_base);
>  =09=09return;
>  =09}

Re: [PATCH 3/7] clk: rockchip: release io resource when rk3036_clk_init failed

[Shawn Lin]

在 2016/3/13 0:48, Heiko Stübner 写道:
> Hi Shawn,
>
> Am Sonntag, 13. März 2016, 00:25:25 schrieb Shawn Lin:
>> We should call iounmap to relase reg_base since it's not going
>> to be used any more.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> I see that change for rk3036, rk3188(+rk3066) and rk3368.
> But it looks like rk3228 and rk3288 should also get that, or am I just
> overlooking something?

My fault, we need to do that for 3228/3288 as well.
Sorry for my slippy finger. I assign wrong patch-numbers to my
script.

I will squash up patch[3/7],[4/7],[5/7] as well as for 3228/3288, and
resend a separate patch for it.

>
> Also, I think we can fold these changes into one patch, as it's the completely
> same addition for all clock-drivers.
>
>
> Heiko
>
>> ---
>>
>>   drivers/clk/rockchip/clk-rk3036.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/clk/rockchip/clk-rk3036.c
>> b/drivers/clk/rockchip/clk-rk3036.c index 8c683cc..284c36f 100644
>> --- a/drivers/clk/rockchip/clk-rk3036.c
>> +++ b/drivers/clk/rockchip/clk-rk3036.c
>> @@ -453,6 +453,7 @@ static void __init rk3036_clk_init(struct device_node
>> *np) ctx = rockchip_clk_init(np, reg_base, CLK_NR_CLKS);
>>   	if (IS_ERR(ctx)) {
>>   		pr_err("%s: rockchip clk init failed\n", __func__);
>> +		iounmap(reg_base);
>>   		return;
>>   	}
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>

Re: [PATCH 1/7] clk: rockchip: remove mux_core_reg from rockchip_cpuclk_reg_data

[Heiko Stuebner]

Am Sonntag, 13. M=E4rz 2016, 00:25:00 schrieb Shawn Lin:
> mux_core_reg isn't been used anywhere, let's remove it.
>=20
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

applied for v4.7

Thanks
Heiko

Re: [PATCH 2/7] clk: rockchip: fix warning reported by kernel-doc

[Heiko Stuebner]

Am Sonntag, 13. M=E4rz 2016, 00:25:14 schrieb Shawn Lin:
> ./scripts/kernel-doc -man -v drivers/clk/rockchip/clk.h > /dev/null
>=20
> drivers/clk/rockchip/clk.h:133: warning: missing initial short
> description on line:
>  * struct rockchip_clk_provider: information about clock provider
> drivers/clk/rockchip/clk.h:133: info: Scanning doc for struct
> drivers/clk/rockchip/clk.h:164: warning: missing initial short
> description on line:
>  * struct rockchip_pll_clock: information about pll clock
> drivers/clk/rockchip/clk.h:164: info: Scanning doc for struct
> drivers/clk/rockchip/clk.h:194: warning: No description found for
> parameter 'parent_names'
> drivers/clk/rockchip/clk.h:194: warning: No description found for
> parameter 'num_parents'
> drivers/clk/rockchip/clk.h:194: warning: Excess struct/union/enum/typ=
edef
> member 'parent_name' description in 'rockchip_pll_clock'
> drivers/clk/rockchip/clk.h:235: warning: missing initial short
> description on line:
>  * struct rockchip_cpuclk_reg_data: describes register offsets and
> masks of the cpuclock
>=20
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

applied for v4.7

Thanks
Heiko

Re: [PATCH 6/7] clk: rockchip: remove redundant checking of device_node

[Heiko Stuebner]

Am Sonntag, 13. M=E4rz 2016, 00:25:53 schrieb Shawn Lin:
> rockchip_clk_of_add_provider is used by sub-clk driver which
> already call of_iomap before calling it. If device_node does
> not exist, of_iomap returns NULL which will fail to init the
> sub-clk driver. So really it's redundant.
>=20
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

applied for v4.7

Thanks
Heiko