diff mbox series

[2/3] clk: mvebu: Use of_get_cpu_hwid() to read CPU ID

Message ID 20230327-mvebu-clk-fixes-v1-2-438de1026efd@kernel.org
State New
Headers
Series clk: Fix/cleanup mvebu CPU DT node accesses |

Commit Message

Rob Herring March 27, 2023, 6:43 p.m. UTC 
  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(-)

Comments

Stephen Boyd April 10, 2023, 11:36 p.m. UTC | #1 
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?
Rob Herring April 11, 2023, 2:04 p.m. UTC | #2 
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 mbox series

Patch

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;