clk: sunxi: allow PLL6 clock to be reused
PATCH 1/1 clk: sunxi: allow PLL6 clock to be reused
Currently we hard-code the base name for the PLL6 clock for both the sun4i and sun6i variants in the driver (pll6 and pll6x2, respectively). This unfortunately denies reusing this clock for the H3 and A64 PLL8 clock, which is the same otherwise.
Remove the hard-coded name in the source code and replace it with an appropriate index into the clock-output-names array in the DT. This is compatible with all current device trees, since they all carry the hard-coded base name in there.
---
Salut Maxime,
does that patch make sense?
It fixes the re-usability problem while still keeping compatibility with existing DTs and also is pretty small.
If you agree with this, I'll send out a v3 of the A64 series including this patch and the H3 PLL8 enablement later.
I tested this on my BananaPi, the clk_summary output was identical with and without this patch.
Cheers,
Andre.
drivers/clk/sunxi/clk-factors.c | 3 ++- drivers/clk/sunxi/clk-factors.h | 1 + drivers/clk/sunxi/clk-sunxi.c | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-)
drivers/clk/sunxi/clk-factors.c +2 -1
@@ -202,7 +202,8 @@ struct clk *sunxi_factors_register(struct device_node *node,if (data->name) clk_name = data->name;else -of_property_read_string (node,"clock-output-names" , &clk_name); +of_property_read_string_index (node,"clock-output-names" , + data->name_idx, &clk_name); factors =kzalloc (sizeof (struct clk_factors ),GFP_KERNEL );if (!factors)
drivers/clk/sunxi/clk-factors.h +1 -0
@@ -36,6 +36,7 @@ struct factors_data {void (*getter)(struct factors_request *req);void (*recalc)(struct factors_request *req);const char *name; +int name_idx; };struct clk_factors {
drivers/clk/sunxi/clk-sunxi.c +2 -2
@@ -530,14 +530,14 @@ static const struct factors_data sun4i_pll6_data __initconst = { .enable = 31, .table = &sun4i_pll5_config, .getter = sun4i_get_pll5_factors, - .name = "pll6", + .name_idx = 2, };static const struct factors_data sun6i_a31_pll6_data__initconst = { .enable =31 , .table = &sun6i_a31_pll6_config, .getter = sun6i_a31_get_pll6_factors, - .name ="pll6x2" , + .name_idx =1 , };static const struct factors_data sun5i_a13_ahb_data__initconst = {
Signature · 1 line
1.8.4
PATCH 1/1 clk: sunxi: allow PLL6 clock to be reused
Quoted 13 lines · Andre Przywara
Currently we hard-code the base name for the PLL6 clock for both the sun4i and sun6i variants in the driver (pll6 and pll6x2, respectively). This unfortunately denies reusing this clock for the H3 and A64 PLL8 clock, which is the same otherwise.
Remove the hard-coded name in the source code and replace it with an appropriate index into the clock-output-names array in the DT. This is compatible with all current device trees, since they all carry the hard-coded base name in there.Signed-off-by Andre Przywara <[email protected]>---
Salut Maxime,
Quoted 7 lines
does that patch make sense?
It fixes the re-usability problem while still keeping compatibility with existing DTs and also is pretty small.
If you agree with this, I'll send out a v3 of the A64 series including this patch and the H3 PLL8 enablement later.
I tested this on my BananaPi, theclk_summaryoutput was identical with and without this patch.
This look OK to me, except one minor detail below.
Quoted 27 lines
Cheers,
Andre.drivers/clk/sunxi/clk-factors.c | 3 ++- drivers/clk/sunxi/clk-factors.h | 1 + drivers/clk/sunxi/clk-sunxi.c | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-)drivers/clk/sunxi/clk-factors.c +2
-1@@ -202,7 +202,8 @@ struct clk *sunxi_factors_register(struct device_node *node,if (data->name) clk_name = data->name;else -of_property_read_string (node,"clock-output-names" , &clk_name); +of_property_read_string_index (node,"clock-output-names" , + data->name_idx, &clk_name); factors =kzalloc (sizeof (struct clk_factors ),GFP_KERNEL );if (!factors)drivers/clk/sunxi/clk-factors.h +0
-0
drivers/clk/sunxi/clk-factors.h struct factors_data { · line 39
Quoted 4 lines
drivers/clk/sunxi/clk-factors.h +1
-0@@ -36,6 +36,7 @@ struct factors_data {void (*getter)(struct factors_request *req);void (*recalc)(struct factors_request *req);const char *name; +int name_idx;
I would drop the .name field. It was a bad workaround due to limitations of the factors clk code at the time by me. We really shouldn't hard-code the name if we want to reuse the driver.
Thanks
ChenYu
Quoted 27 lines
drivers/clk/sunxi/clk-factors.h +0
-0};struct clk_factors {drivers/clk/sunxi/clk-sunxi.c +2
-3@@ -530,14 +530,14 @@ static const struct factors_data sun4i_pll6_data __initconst = { .enable = 31, .table = &sun4i_pll5_config, .getter = sun4i_get_pll5_factors, - .name = "pll6", + .name_idx = 2, };static const struct factors_data sun6i_a31_pll6_data__initconst = { .enable =31 , .table = &sun6i_a31_pll6_config, .getter = sun6i_a31_get_pll6_factors, - .name ="pll6x2" , + .name_idx =1 , };static const struct factors_data sun5i_a13_ahb_data__initconst = { --1.8.4
PATCH 1/1 clk: sunxi: allow PLL6 clock to be reused
Hi Chen-Yu,
Quoted 44 lines · Chen-Yu Tsai
Quoted 13 lines · Andre Przywara
Currently we hard-code the base name for the PLL6 clock for both the sun4i and sun6i variants in the driver (pll6 and pll6x2, respectively). This unfortunately denies reusing this clock for the H3 and A64 PLL8 clock, which is the same otherwise.
Remove the hard-coded name in the source code and replace it with an appropriate index into the clock-output-names array in the DT. This is compatible with all current device trees, since they all carry the hard-coded base name in there.Signed-off-by Andre Przywara <[email protected]>---
Salut Maxime,Quoted 7 lines
does that patch make sense?
It fixes the re-usability problem while still keeping compatibility with existing DTs and also is pretty small.
If you agree with this, I'll send out a v3 of the A64 series including this patch and the H3 PLL8 enablement later.
I tested this on my BananaPi, theclk_summaryoutput was identical with and without this patch.This look OK to me, except one minor detail below.
Quoted 20 lines
Cheers,
Andre.drivers/clk/sunxi/clk-factors.c | 3 ++- drivers/clk/sunxi/clk-factors.h | 1 + drivers/clk/sunxi/clk-sunxi.c | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-)drivers/clk/sunxi/clk-factors.c +2
-1@@ -202,7 +202,8 @@ struct clk *sunxi_factors_register(struct device_node *node,if (data->name) clk_name = data->name;else -of_property_read_string (node,"clock-output-names" , &clk_name); +of_property_read_string_index (node,"clock-output-names" , + data->name_idx, &clk_name);
Quoted 16 lines
drivers/clk/sunxi/clk-factors.h struct factors_data { · line 39
Quoted 11 lines
drivers/clk/sunxi/clk-factors.c +0
-0factors =kzalloc (sizeof (struct clk_factors ),GFP_KERNEL );if (!factors)drivers/clk/sunxi/clk-factors.h +1
-0@@ -36,6 +36,7 @@ struct factors_data {void (*getter)(struct factors_request *req);void (*recalc)(struct factors_request *req);const char *name; +int name_idx;I would drop the .name field. It was a bad workaround due to limitations of the factors clk code at the time by me. We really shouldn't hard-code the name if we want to reuse the driver.
I know what you mean (my first thought, too) and I totally agree, but we need it still for PLL5, which does not carry the original name in the DT output names.
So at least this workaround here does not work, I guess we have to come up with something different - which would be a different patch. I can take a look into this later.
Thanks,
Andre.
Quoted 28 lines
Quoted 27 lines
drivers/clk/sunxi/clk-factors.h +0
-0};struct clk_factors {drivers/clk/sunxi/clk-sunxi.c +2
-3@@ -530,14 +530,14 @@ static const struct factors_data sun4i_pll6_data __initconst = { .enable = 31, .table = &sun4i_pll5_config, .getter = sun4i_get_pll5_factors, - .name = "pll6", + .name_idx = 2, };static const struct factors_data sun6i_a31_pll6_data__initconst = { .enable =31 , .table = &sun6i_a31_pll6_config, .getter = sun6i_a31_get_pll6_factors, - .name ="pll6x2" , + .name_idx =1 , };static const struct factors_data sun5i_a13_ahb_data__initconst = { --1.8.4
PATCH 1/1 clk: sunxi: allow PLL6 clock to be reused
Quoted 15 lines · Andre Przywara
Quoted 6 lines
Quoted 6 lines
drivers/clk/sunxi/clk-factors.h +0
-0@@ -36,6 +36,7 @@ struct factors_data {void (*getter)(struct factors_request *req);Quoted 8 lines
drivers/clk/sunxi/clk-factors.h struct factors_data { · line 39
Quoted 3 lines
drivers/clk/sunxi/clk-factors.h +1
-0void (*recalc)(struct factors_request *req);const char *name; +int name_idx;I would drop the .name field. It was a bad workaround due to limitations of the factors clk code at the time by me. We really shouldn't hard-code the name if we want to reuse the driver.
Quoted 6 lines
I know what you mean (my first thought, too) and I totally agree, but we need it still for PLL5, which does not carry the original name in the DT output names.
So at least this workaround here does not work, I guess we have to come up with something different - which would be a different patch. I can take a look into this later.
Actually, it could be easily worked around. Always take the first clock output name, take whatever is before '_', and you can remove the name field entirely.
Jens did something along these lines here:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/379900.html
Maybe we should simply rebase this patch, and remove the part that falls back on the name field.
Maxime
Signature · 10 lines
Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160225/aba3505b/attachment.sig>
PATCH 1/1 clk: sunxi: allow PLL6 clock to be reused
Quoted 23 lines · Maxime Ripard
Quoted 15 lines · Andre Przywara
Quoted 6 lines
Quoted 6 lines
drivers/clk/sunxi/clk-factors.h +0
-0@@ -36,6 +36,7 @@ struct factors_data {void (*getter)(struct factors_request *req);Quoted 8 lines
drivers/clk/sunxi/clk-factors.h struct factors_data { · line 39
Quoted 3 lines
drivers/clk/sunxi/clk-factors.h +1
-0void (*recalc)(struct factors_request *req);const char *name; +int name_idx;I would drop the .name field. It was a bad workaround due to limitations of the factors clk code at the time by me. We really shouldn't hard-code the name if we want to reuse the driver.
Quoted 6 lines
I know what you mean (my first thought, too) and I totally agree, but we need it still for PLL5, which does not carry the original name in the DT output names.
So at least this workaround here does not work, I guess we have to come up with something different - which would be a different patch. I can take a look into this later.
Quoted 3 lines
Actually, it could be easily worked around. Always take the first clock output name, take whatever is before '_', and you can remove the name field entirely.
Yeah, I was thinking about that, too, but wanted to give an easy patch a try first.
Quoted 2 lines
Jens did something along these lines here:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/379900.html
Indeed, I stumbled upon this by accident today. Looks great on the first
glance: using .self first, then the stub till the underscore. I can review and test this, if needed.
Quoted 2 lines
Maybe we should simply rebase this patch, and remove the part that falls back on the name field.
I am totally fine with this!
Thanks!
Andre.