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
mux_core_reg isn't been used anywhere, let's remove it.
Signed-off-by: Shawn Lin <[email protected]>
---
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
./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 <[email protected]>
---
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
We should call iounmap to relase reg_base since it's not going
to be used any more.
Signed-off-by: Shawn Lin <[email protected]>
---
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
We should call iounmap to relase reg_base since it's not going
to be used any more.
Signed-off-by: Shawn Lin <[email protected]>
---
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
We should call iounmap to relase reg_base since it's not going
to be used any more.w
Signed-off-by: Shawn Lin <[email protected]>
---
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
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 <[email protected]>
---
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
Signed-off-by: Shawn Lin <[email protected]>
---
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
Hi Shawn,
Am Sonntag, 13. M?rz 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 */
> 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);
> }
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.git/commit/?h=v4.7-clk/next&id=95e0c473a0ac1bdac25f55678dc602eb50dae684
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 <[email protected]>
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?
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;
> }
在 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 <[email protected]>
>
> 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
Am Sonntag, 13. M?rz 2016, 00:25:00 schrieb Shawn Lin:
> mux_core_reg isn't been used anywhere, let's remove it.
>
> Signed-off-by: Shawn Lin <[email protected]>
applied for v4.7
Thanks
Heiko
Am Sonntag, 13. M?rz 2016, 00:25:14 schrieb 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 <[email protected]>
applied for v4.7
Thanks
Heiko
Am Sonntag, 13. M?rz 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.
>
> Signed-off-by: Shawn Lin <[email protected]>
applied for v4.7
Thanks
Heiko