[PATCH 1/2] cell: don't cast the result of of_get_property()

Subsystems: cell broadband engine architecture, linux for powerpc (32-bit and 64-bit), the rest

STALE6902d

7 messages, 4 authors, 2007-09-26 · open the first message on its own page

[PATCH 1/2] cell: don't cast the result of of_get_property()

From: Jeremy Kerr <jk@ozlabs.org>
Date: 2007-09-14 05:46:40

The cast to u32 * isn't required, of_get_property returns a void *.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

---
 arch/powerpc/platforms/cell/spu_manage.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spu_manage.c b/arch/powerpc/platforms/cell/spu_manage.c
index 0e14f53..1b01070 100644
--- a/arch/powerpc/platforms/cell/spu_manage.c
+++ b/arch/powerpc/platforms/cell/spu_manage.c

@@ -377,10 +377,10 @@ static int qs20_reg_memory[QS20_SPES_PER_BE] = { 1, 1, 0, 0, 0, 0, 0, 0 };
 static struct spu *spu_lookup_reg(int node, u32 reg)
 {
 	struct spu *spu;
-	u32 *spu_reg;
+	const u32 *spu_reg;
 
 	list_for_each_entry(spu, &cbe_spu_info[node].spus, cbe_list) {
-		spu_reg = (u32*)of_get_property(spu_devnode(spu), "reg", NULL);
+		spu_reg = of_get_property(spu_devnode(spu), "reg", NULL);
 		if (*spu_reg == reg)
 			return spu;
 	}

[PATCH 2/2] fsl/embedded6xx: don't cast the result of of_get_property

From: Jeremy Kerr <jk@ozlabs.org>
Date: 2007-09-14 05:46:40

Use a temporary variable of the correct type instead.

Also, this allows us to check the return value in ls_uarts_init, to
avoid dereferencing a null pointer if required properties aren't
present.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

---
 arch/powerpc/platforms/embedded6xx/ls_uart.c |   14 +++++++++-----
 arch/powerpc/sysdev/fsl_soc.c                |   18 ++++++++++--------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/platforms/embedded6xx/ls_uart.c b/arch/powerpc/platforms/embedded6xx/ls_uart.c
index d0bee9f..9d4b40c 100644
--- a/arch/powerpc/platforms/embedded6xx/ls_uart.c
+++ b/arch/powerpc/platforms/embedded6xx/ls_uart.c

@@ -103,20 +103,24 @@ static void __init ls_uart_init(void)
 static int __init ls_uarts_init(void)
 {
 	struct device_node *avr;
-	phys_addr_t phys_addr;
+	const u32 *avr_clock_prop, *phys_addr;
 	int len;
 
 	avr = of_find_node_by_path("/soc10x/serial@80004500");
 	if (!avr)
 		return -EINVAL;
 
-	avr_clock = *(u32*)of_get_property(avr, "clock-frequency", &len);
-	phys_addr = ((u32*)of_get_property(avr, "reg", &len))[0];
+	phys_addr = of_get_property(avr, "reg", &len);
+	avr_clock_prop = of_get_property(avr, "clock-frequency", &len);
 
-	if (!avr_clock || !phys_addr)
+	if (!avr_clock_prop || !phys_addr)
 		return -EINVAL;
 
-	avr_addr = ioremap(phys_addr, 32);
+	avr_clock = *avr_clock_prop;
+	if (!avr_clock || !phys_addr[0])
+		return -EINVAL;
+
+	avr_addr = ioremap(phys_addr[0], 32);
 	if (!avr_addr)
 		return -EFAULT;

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 1cf29c9..564d5e5 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c

@@ -673,6 +673,7 @@ static int __init fs_enet_of_init(void)
 		struct fs_platform_info fs_enet_data;
 		const unsigned int *id, *phy_addr, *phy_irq;
 		const void *mac_addr;
+		const u32 *clk;
 		const phandle *ph;
 		const char *model;

@@ -740,10 +741,10 @@ static int __init fs_enet_of_init(void)
                         goto unreg;
                 }
 
-		fs_enet_data.clk_rx = *((u32 *)of_get_property(np,
-						"rx-clock", NULL));
-		fs_enet_data.clk_tx = *((u32 *)of_get_property(np,
-						"tx-clock", NULL));
+		clk = of_get_property(np, "rx-clock", NULL);
+		fs_enet_data.clk_rx = *clk;
+		clk = of_get_property(np, "tx-clock", NULL);
+		fs_enet_data.clk_tx = *clk;
 
 		if (strstr(model, "FCC")) {
 			int fcc_index = *id - 1;

@@ -844,6 +845,7 @@ static int __init cpm_uart_of_init(void)
 		struct resource r[3];
 		struct fs_uart_platform_info cpm_uart_data;
 		const int *id;
+		const u32 *clk;
 		const char *model;
 
 		memset(r, 0, sizeof(r));

@@ -882,10 +884,10 @@ static int __init cpm_uart_of_init(void)
 		cpm_uart_data.tx_buf_size = 32;
 		cpm_uart_data.rx_num_fifo = 4;
 		cpm_uart_data.rx_buf_size = 32;
-		cpm_uart_data.clk_rx = *((u32 *)of_get_property(np,
-						"rx-clock", NULL));
-		cpm_uart_data.clk_tx = *((u32 *)of_get_property(np,
-						"tx-clock", NULL));
+		clk = of_get_property(np, "rx-clock", NULL);
+		cpm_uart_data.clk_rx = *clk;
+		clk = of_get_property(np, "tx-clock", NULL);
+		cpm_uart_data.clk_tx = *clk;
 
 		ret =
 		    platform_device_add_data(cpm_uart_dev, &cpm_uart_data,

Re: [PATCH 2/2] fsl/embedded6xx: don't cast the result of of_get_property

From: Kumar Gala <hidden>
Date: 2007-09-25 20:27:30

On Sep 14, 2007, at 12:46 AM, Jeremy Kerr wrote:

Use a temporary variable of the correct type instead.

Also, this allows us to check the return value in ls_uarts_init, to
avoid dereferencing a null pointer if required properties aren't
present.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

Can I get you to respin this against for-2.6.24.  For some reason  
this doesn't apply cleanly for me.

- k

Re: [PATCH 2/2] fsl/embedded6xx: don't cast the result of of_get_property

From: Stephen Rothwell <hidden>
Date: 2007-09-26 00:44:19

On Fri, 14 Sep 2007 15:46:40 +1000 Jeremy Kerr [off-list ref] wrote:

-	avr_clock = *(u32*)of_get_property(avr, "clock-frequency", &len);
-	phys_addr = ((u32*)of_get_property(avr, "reg", &len))[0];
+	phys_addr = of_get_property(avr, "reg", &len);
+	avr_clock_prop = of_get_property(avr, "clock-frequency", &len);

If you don't use the value of the len variable, then you can pass NULL to
of_get_property.

+	if (!avr_clock || !phys_addr[0])

Not *phys_addr?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

Re: [PATCH 2/2] fsl/embedded6xx: don't cast the result of of_get_property

From: Jeremy Kerr <jk@ozlabs.org>
Date: 2007-09-26 00:52:23

Stephen,

If you don't use the value of the len variable, then you can pass
NULL to of_get_property.

Sure, but that's probably a separate patch, no ?

Not *phys_addr?

That's what the existing code does. I'm guessing that it treats 
phys_addr as an array with one entry, not a single u32.

Cheers,


Jeremy

Re: [PATCH 2/2] fsl/embedded6xx: don't cast the result of of_get_property

From: Stephen Rothwell <hidden>
Date: 2007-09-26 00:59:18

On Wed, 26 Sep 2007 10:52:23 +1000 Jeremy Kerr [off-list ref] wrote:

Stephen,

quoted

If you don't use the value of the len variable, then you can pass
NULL to of_get_property.

Sure, but that's probably a separate patch, no ?

OK.

quoted

Not *phys_addr?

That's what the existing code does. I'm guessing that it treats 
phys_addr as an array with one entry, not a single u32.

OK.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

Re: [PATCH 2/2] fsl/embedded6xx: don't cast the result of of_get_property

From: Segher Boessenkool <hidden>
Date: 2007-09-26 12:55:18

quoted

-	avr_clock = *(u32*)of_get_property(avr, "clock-frequency", &len);
-	phys_addr = ((u32*)of_get_property(avr, "reg", &len))[0];
+	phys_addr = of_get_property(avr, "reg", &len);
+	avr_clock_prop = of_get_property(avr, "clock-frequency", &len);

If you don't use the value of the len variable, then you can pass NULL 
to
of_get_property.

OTOH, it is almost always an error to not use the length (missing
error check).


Segher

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help