clk: qcom: gcc: Fix board clock node name
Subsystems (3)
PATCH 1/1 clk: qcom: gcc: Fix board clock node name
Device tree node name are not supposed to have "_" in them so fix the node name use of xo_board to xo-board
---
Steve: RobH pointed this on DTS patches, would be great if you can pick this as a fix
drivers/clk/qcom/gcc-qcs404.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clk/qcom/gcc-qcs404.c +1 -1
@@ -265,7 +265,7 @@ static struct clk_fixed_factor cxo = { .div = 1, .hw.init = &(struct clk_init_data ){ .name ="cxo" , - .parent_names = (const char *[]){"xo_board" }, + .parent_names = (const char *[]){"xo-board" }, .num_parents =1 , .ops = &clk_fixed_factor_ops, },
Signature · 1 line
2.14.4
Stephen Boyd
maintainerQuoting Vinod Koul (2018-11-09 01:50:54)
Quoted 9 lines
Device tree node name are not supposed to have "_" in them so fix the node name use of
xo_boardto xo-boardFixes 652f1813c113 ("clk: qcom: gcc: Add global clock controller driver for QCS404")Signed-off-by Vinod Koul <[email protected]>---
Steve: RobH pointed this on DTS patches, would be great if you can pick this as a fix
Ok.
Quoted 9 lines
drivers/clk/qcom/gcc-qcs404.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/clk/qcom/gcc-qcs404.c +0
-0
drivers/clk/qcom/gcc-qcs404.c static struct clk_fixed_factor cxo = { · line 268
Quoted 5 lines
drivers/clk/qcom/gcc-qcs404.c +1
-1@@ -265,7 +265,7 @@ static struct clk_fixed_factor cxo = { .div = 1, .hw.init = &(struct clk_init_data ){ .name ="cxo" , - .parent_names = (const char *[]){"xo_board" }, + .parent_names = (const char *[]){"xo-board" },
We have xo_board used everywhere else in drivers/clk/qcom/ so this makes me concerned. Wouldn't a better answer be to add clock-output-names to the xo-board DT node and have arm-soc merge that through. I mostly want to keep things consistent here so that if we need to inject an xo_board clk into the system then we can do so generically instead of making it per-platform. Of course, if we're never going to have this problem on qcs404 then it will be fine to start differing here. So I'm leaning towards merge this patch, just please ack my concern here and tell me it won't be a problem and I'll be happy to merge to clk-fixes.
BTW, can you also specify a 'clocks' property in the GCC node and send the xo_board node there? In fact, we should do that for every GCC node in the tree. Care to do that and also add sleep_clk to each clock controller node that uses it? This is useful to do so that we can more easily see where clocks are going between clock controller nodes.
(Add Rob & Bjorn)
Hi Steve,
Quoted 29 lines · Stephen Boyd
Quoting Vinod Koul (2018-11-09 01:50:54)
Quoted 9 lines
Device tree node name are not supposed to have "_" in them so fix the node name use of
xo_boardto xo-boardFixes 652f1813c113 ("clk: qcom: gcc: Add global clock controller driver for QCS404")Signed-off-by Vinod Koul <[email protected]>---
Steve: RobH pointed this on DTS patches, would be great if you can pick this as a fix
Ok.
Quoted 9 lines
drivers/clk/qcom/gcc-qcs404.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/clk/qcom/gcc-qcs404.c +0
-0drivers/clk/qcom/gcc-qcs404.c static struct clk_fixed_factor cxo = { · line 268
Quoted 5 lines
drivers/clk/qcom/gcc-qcs404.c +1
-1@@ -265,7 +265,7 @@ static struct clk_fixed_factor cxo = { .div = 1, .hw.init = &(struct clk_init_data ){ .name ="cxo" , - .parent_names = (const char *[]){"xo_board" }, + .parent_names = (const char *[]){"xo-board" },We have
xo_boardused everywhere else indrivers/clk/qcom/so this makes
Quoted 8 lines
me concerned. Wouldn't a better answer be to add clock-output-names to the xo-board DT node and have arm-soc merge that through. I mostly want to keep things consistent here so that if we need to inject an
xo_boardclk into the system then we can do so generically instead of making it per-platform. Of course, if we're never going to have this problem on qcs404 then it will be fine to start differing here. So I'm leaning towards merge this patch, just please ack my concern here and tell me it won't be a problem and I'll be happy to merge to clk-fixes.
So this is a warning from DT compiler and triggered with W=12, I see tons of examples using "_" in node names. Clearly someone realized it (Rob ?) added a warning for it.
As you rightly thought, qcs404 will be okay as we are starting out and following few conventions so keeping this saner :)
Quoted 5 lines
BTW, can you also specify a 'clocks' property in the GCC node and send the
xo_boardnode there? In fact, we should do that for every GCC node in the tree. Care to do that and also addsleep_clkto each clock controller node that uses it? This is useful to do so that we can more easily see where clocks are going between clock controller nodes.
I agree that it makes sense to add the property in gcc node. I will add this in my list and chase if after my current task completes, if that is fine by you
Thanks
Signature · 1 line
~Vinod
Fix Rob's email id... and looping him correctly :)
Quoted 63 lines · Vinod Koul
(Add Rob & Bjorn)
Hi Steve,
Quoted 29 lines · Stephen Boyd
Quoting Vinod Koul (2018-11-09 01:50:54)
Quoted 9 lines
Device tree node name are not supposed to have "_" in them so fix the node name use of
xo_boardto xo-boardFixes 652f1813c113 ("clk: qcom: gcc: Add global clock controller driver for QCS404")Signed-off-by Vinod Koul <[email protected]>---
Steve: RobH pointed this on DTS patches, would be great if you can pick this as a fix
Ok.
Quoted 9 lines
drivers/clk/qcom/gcc-qcs404.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/clk/qcom/gcc-qcs404.c +0
-0drivers/clk/qcom/gcc-qcs404.c static struct clk_fixed_factor cxo = { · line 268
Quoted 5 lines
drivers/clk/qcom/gcc-qcs404.c +1
-1@@ -265,7 +265,7 @@ static struct clk_fixed_factor cxo = { .div = 1, .hw.init = &(struct clk_init_data ){ .name ="cxo" , - .parent_names = (const char *[]){"xo_board" }, + .parent_names = (const char *[]){"xo-board" },We have
xo_boardused everywhere else indrivers/clk/qcom/so this makesQuoted 8 lines
me concerned. Wouldn't a better answer be to add clock-output-names to the xo-board DT node and have arm-soc merge that through. I mostly want to keep things consistent here so that if we need to inject an
xo_boardclk into the system then we can do so generically instead of making it per-platform. Of course, if we're never going to have this problem on qcs404 then it will be fine to start differing here. So I'm leaning towards merge this patch, just please ack my concern here and tell me it won't be a problem and I'll be happy to merge to clk-fixes.So this is a warning from DT compiler and triggered with W=12, I see tons of examples using "_" in node names. Clearly someone realized it (Rob ?) added a warning for it.
As you rightly thought, qcs404 will be okay as we are starting out and following few conventions so keeping this saner :)
Quoted 5 lines
BTW, can you also specify a 'clocks' property in the GCC node and send the
xo_boardnode there? In fact, we should do that for every GCC node in the tree. Care to do that and also addsleep_clkto each clock controller node that uses it? This is useful to do so that we can more easily see where clocks are going between clock controller nodes.I agree that it makes sense to add the property in gcc node. I will add this in my list and chase if after my current task completes, if that is fine by you
Thanks
Signature · 1 line
~Vinod
Signature · 1 line
~Vinod
Stephen Boyd
maintainerQuoting Vinod Koul (2018-11-09 09:51:05)
Quoted 40 lines · Vinod Koul
Quoted 39 lines · Stephen Boyd
Quoted 16 lines
Quoting Vinod Koul (2018-11-09 01:50:54)
drivers/clk/qcom/gcc-qcs404.c static struct clk_fixed_factor cxo = { · line 268
Quoted 13 lines
drivers/clk/qcom/gcc-qcs404.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/clk/qcom/gcc-qcs404.c +1
-1@@ -265,7 +265,7 @@ static struct clk_fixed_factor cxo = { .div = 1, .hw.init = &(struct clk_init_data ){ .name ="cxo" , - .parent_names = (const char *[]){"xo_board" }, + .parent_names = (const char *[]){"xo-board" },We have
xo_boardused everywhere else indrivers/clk/qcom/so this makesQuoted 8 lines
me concerned. Wouldn't a better answer be to add clock-output-names to the xo-board DT node and have arm-soc merge that through. I mostly want to keep things consistent here so that if we need to inject an
xo_boardclk into the system then we can do so generically instead of making it per-platform. Of course, if we're never going to have this problem on qcs404 then it will be fine to start differing here. So I'm leaning towards merge this patch, just please ack my concern here and tell me it won't be a problem and I'll be happy to merge to clk-fixes.So this is a warning from DT compiler and triggered with W=12, I see tons of examples using "_" in node names. Clearly someone realized it (Rob ?) added a warning for it.
As you rightly thought, qcs404 will be okay as we are starting out and following few conventions so keeping this saner :)
Quoted 5 lines
BTW, can you also specify a 'clocks' property in the GCC node and send the
xo_boardnode there? In fact, we should do that for every GCC node in the tree. Care to do that and also addsleep_clkto each clock controller node that uses it? This is useful to do so that we can more easily see where clocks are going between clock controller nodes.
Quoted 4 lines
Quoted 4 lines
I agree that it makes sense to add the property in gcc node. I will add this in my list and chase if after my current task completes, if that is fine by you
Ok. Thanks for checking. Applied to clk-fixes.
Hello Vinod,
Quoted 19 lines · Vinod Koul
Device tree node name are not supposed to have "_" in them so fix the node name use of
xo_boardto xo-boardFixes 652f1813c113 ("clk: qcom: gcc: Add global clock controller driver for QCS404")Signed-off-by Vinod Koul <[email protected]>---
Steve: RobH pointed this on DTS patches, would be great if you can pick this as a fix
drivers/clk/qcom/gcc-qcs404.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/clk/qcom/gcc-qcs404.c +0
-0
drivers/clk/qcom/gcc-qcs404.c static struct clk_fixed_factor cxo = { · line 271
Quoted 8 lines
drivers/clk/qcom/gcc-qcs404.c +1
-1@@ -265,7 +265,7 @@ static struct clk_fixed_factor cxo = { .div = 1, .hw.init = &(struct clk_init_data ){ .name ="cxo" , - .parent_names = (const char *[]){"xo_board" }, + .parent_names = (const char *[]){"xo-board" }, .num_parents =1 , .ops = &clk_fixed_factor_ops, },
This fixed clock needs to be removed, once the RPM<->SMD clocks are added. Why not have this clock part of the device Tree?
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Stephen Boyd
maintainerQuoting Taniya Das (2018-11-10 18:12:28)
Quoted 25 lines
Hello Vinod,
Quoted 22 lines · Vinod Koul
Device tree node name are not supposed to have "_" in them so fix the node name use of
xo_boardto xo-boardFixes 652f1813c113 ("clk: qcom: gcc: Add global clock controller driver for QCS404")Signed-off-by Vinod Koul <[email protected]>---
Steve: RobH pointed this on DTS patches, would be great if you can pick this as a fix
drivers/clk/qcom/gcc-qcs404.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/clk/qcom/gcc-qcs404.c +0
-1@@ -265,7 +265,7 @@ static struct clk_fixed_factor cxo = { .div = 1, .hw.init = &(struct clk_init_data ){ .name ="cxo" , - .parent_names = (const char *[]){"xo_board" },
Quoted 9 lines
drivers/clk/qcom/gcc-qcs404.c static struct clk_fixed_factor cxo = { · line 271
Quoted 5 lines
drivers/clk/qcom/gcc-qcs404.c +1
-0+ .parent_names = (const char *[]){ "xo-board" }, .num_parents = 1, .ops = &clk_fixed_factor_ops, },This fixed clock needs to be removed, once the RPM<->SMD clocks are added. Why not have this clock part of the device Tree?
If the clk needs to be removed at some point, then putting it into DT instead of leaving it in the driver is the wrong direction to take. We don't want to have to change DT as often as we change driver code.