| Message ID | 20230327-mvebu-clk-fixes-v1-2-438de1026efd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | clk: Fix/cleanup mvebu CPU DT node accesses | expand |
Quoting Rob Herring (2023-03-27 11:43:19) > Use of_get_cpu_hwid() rather than the open coded reading of the CPU > nodes "reg" property. The existing code is in fact wrong as the "reg" > address cells size is 2 cells for arm64. The existing code happens to > work because the DTS files are wrong as well. > > Signed-off-by: Rob Herring <robh@kernel.org> > --- > Note this should be marked for stable so that if/when the DTS files are > fixed, then at least stable kernels will work. This is untested, so I > didn't mark for stable. That makes it sound like it breaks for existing DTS files. Is that the case?
On Mon, Apr 10, 2023 at 04:36:21PM -0700, Stephen Boyd wrote: > Quoting Rob Herring (2023-03-27 11:43:19) > > Use of_get_cpu_hwid() rather than the open coded reading of the CPU > > nodes "reg" property. The existing code is in fact wrong as the "reg" > > address cells size is 2 cells for arm64. The existing code happens to > > work because the DTS files are wrong as well. > > > > Signed-off-by: Rob Herring <robh@kernel.org> > > --- > > Note this should be marked for stable so that if/when the DTS files are > > fixed, then at least stable kernels will work. This is untested, so I > > didn't mark for stable. > > That makes it sound like it breaks for existing DTS files. Is that the > case? No, if the DTS files are fixed, then they will not work with the existing code. This change should work for both existing and fixed DTS files. Rob
diff --git a/drivers/clk/mvebu/ap-cpu-clk.c b/drivers/clk/mvebu/ap-cpu-clk.c index 71bdd7c3ff03..d8a7a4c90d54 100644 --- a/drivers/clk/mvebu/ap-cpu-clk.c +++ b/drivers/clk/mvebu/ap-cpu-clk.c @@ -253,12 +253,12 @@ static int ap_cpu_clock_probe(struct platform_device *pdev) */ nclusters = 1; for_each_of_cpu_node(dn) { - int cpu, err; + u64 cpu; - err = of_property_read_u32(dn, "reg", &cpu); - if (WARN_ON(err)) { + cpu = of_get_cpu_hwid(dn, 0); + if (WARN_ON(cpu == OF_BAD_ADDR)) { of_node_put(dn); - return err; + return -EINVAL; } /* If cpu2 or cpu3 is enabled */ @@ -288,12 +288,12 @@ static int ap_cpu_clock_probe(struct platform_device *pdev) struct clk_init_data init; const char *parent_name; struct clk *parent; - int cpu, err; + u64 cpu; - err = of_property_read_u32(dn, "reg", &cpu); - if (WARN_ON(err)) { + cpu = of_get_cpu_hwid(dn, 0); + if (WARN_ON(cpu == OF_BAD_ADDR)) { of_node_put(dn); - return err; + return -EINVAL; } cluster_index = cpu & APN806_CLUSTER_NUM_MASK;
Use of_get_cpu_hwid() rather than the open coded reading of the CPU nodes "reg" property. The existing code is in fact wrong as the "reg" address cells size is 2 cells for arm64. The existing code happens to work because the DTS files are wrong as well. Signed-off-by: Rob Herring <robh@kernel.org> --- Note this should be marked for stable so that if/when the DTS files are fixed, then at least stable kernels will work. This is untested, so I didn't mark for stable. --- drivers/clk/mvebu/ap-cpu-clk.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)