Date: Thu, 3 Nov 2011 13:22:13 +0530 From: "Jayachandran C." <jchandra@freebsd.org> To: Rafal Jaworowski <raj@semihalf.com>, Nathan Whitehorn <nwhitehorn@freebsd.org>, freebsd-arm@freebsd.org, freebsd-ppc@freebsd.org, FreeBSD Current <freebsd-current@freebsd.org> Cc: Marcel Moolenaar <marcel@freebsd.org> Subject: [RFC] Fix OF_finddevice return code for FDT Message-ID: <CA%2B7sy7DMHQgo8f%2Byj7oNOkti28Hg8zOtwJwhNCgVUdksMPru1g@mail.gmail.com> In-Reply-To: <CA%2B7sy7DJEHNjSQvL3mySEoN0KQyCbhwQF8odaoysTDkv1AQLRg@mail.gmail.com> References: <CA%2B7sy7DJEHNjSQvL3mySEoN0KQyCbhwQF8odaoysTDkv1AQLRg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
[I had posted this to freebsd-ppc@ and freebsd-arm@, did not see any
comments, posting to freebsd-current@ to see if there is any interest
or comments]
While reviewing the previous FDT patch, nwhitehorn@ noted that the
return code of OF_finddevice was not correct in case of FDT. According
to the 1275 standard, we should return a phandle value of -1 in case
of error, but the ofw_fdt_finddevice implementation now returns 0.
The attached patches fixes this in the FDT code, and makes changes in
the callers to check the return code correctly. Since most of the
callers are in ARM, any testing on ARM would be really appreciated.
Thanks,
JC.
[-- Attachment #2 --]
commit 81076e8aa4cbf23c8ad890ec8515f7917bae2ab8
Author: Jayachandran C <jayachandranc@netlogicmicro.com>
Date: Tue Oct 18 00:57:46 2011 +0530
OF_finddevice should return -1 on error
Fix the implementation, and fixup the callers.
diff --git a/sys/dev/fdt/fdt_common.c b/sys/dev/fdt/fdt_common.c
index d25715b..8702df2 100644
--- a/sys/dev/fdt/fdt_common.c
+++ b/sys/dev/fdt/fdt_common.c
@@ -74,13 +74,13 @@ fdt_immr_addr(vm_offset_t immr_va)
/*
* Try to access the SOC node directly i.e. through /aliases/.
*/
- if ((node = OF_finddevice("soc")) != 0)
+ if ((node = OF_finddevice("soc")) != -1)
if (fdt_is_compatible_strict(node, "simple-bus"))
goto moveon;
/*
* Find the node the long way.
*/
- if ((node = OF_finddevice("/")) == 0)
+ if ((node = OF_finddevice("/")) == -1)
return (ENXIO);
if ((node = fdt_find_compatible(node, "simple-bus", 1)) == 0)
@@ -576,7 +576,7 @@ fdt_get_mem_regions(struct mem_region *mr, int *mrcnt, uint32_t *memsize)
max_size = sizeof(reg);
memory = OF_finddevice("/memory");
- if (memory <= 0) {
+ if (memory == -1) {
rv = ENXIO;
goto out;
}
diff --git a/sys/dev/fdt/fdtbus.c b/sys/dev/fdt/fdtbus.c
index e2808d1..d5296a9 100644
--- a/sys/dev/fdt/fdtbus.c
+++ b/sys/dev/fdt/fdtbus.c
@@ -177,7 +177,7 @@ fdtbus_attach(device_t dev)
u_long start, end;
int error;
- if ((root = OF_peer(0)) == 0)
+ if ((root = OF_finddevice("/")) == -1)
panic("fdtbus_attach: no root node.");
sc = device_get_softc(dev);
diff --git a/sys/dev/ofw/ofw_fdt.c b/sys/dev/ofw/ofw_fdt.c
index 806f17c..7b3b0e9 100644
--- a/sys/dev/ofw/ofw_fdt.c
+++ b/sys/dev/ofw/ofw_fdt.c
@@ -392,6 +392,8 @@ ofw_fdt_finddevice(ofw_t ofw, const char *device)
int offset;
offset = fdt_path_offset(fdtp, device);
+ if (offset < 0)
+ return (-1);
return (fdt_offset_phandle(offset));
}
@@ -420,7 +422,7 @@ ofw_fdt_fixup(ofw_t ofw)
ssize_t len;
int i;
- if ((root = ofw_fdt_finddevice(ofw, "/")) == 0)
+ if ((root = ofw_fdt_finddevice(ofw, "/")) == -1)
return (ENODEV);
if ((len = ofw_fdt_getproplen(ofw, root, "model")) <= 0)
diff --git a/sys/dev/ofw/openfirm.c b/sys/dev/ofw/openfirm.c
index a8cb8f7..f543dce 100644
--- a/sys/dev/ofw/openfirm.c
+++ b/sys/dev/ofw/openfirm.c
@@ -131,7 +131,7 @@ OF_init(void *cookie)
rv = OFW_INIT(ofw_obj, cookie);
- if ((chosen = OF_finddevice("/chosen")) > 0)
+ if ((chosen = OF_finddevice("/chosen")) != -1)
if (OF_getprop(chosen, "stdout", &stdout, sizeof(stdout)) == -1)
stdout = -1;
diff --git a/sys/dev/uart/uart_bus_fdt.c b/sys/dev/uart/uart_bus_fdt.c
index 4209866..8bb62ea 100644
--- a/sys/dev/uart/uart_bus_fdt.c
+++ b/sys/dev/uart/uart_bus_fdt.c
@@ -155,11 +155,11 @@ uart_cpu_getdev(int devtype, struct uart_devinfo *di)
/*
* Retrieve /chosen/std{in,out}.
*/
- if ((chosen = OF_finddevice("/chosen")) == 0)
+ if ((chosen = OF_finddevice("/chosen")) == -1)
return (ENXIO);
if (OF_getprop(chosen, "stdin", buf, sizeof(buf)) <= 0)
return (ENXIO);
- if ((node = OF_finddevice(buf)) == 0)
+ if ((node = OF_finddevice(buf)) == -1)
return (ENXIO);
if (OF_getprop(chosen, "stdout", buf, sizeof(buf)) <= 0)
return (ENXIO);
[-- Attachment #3 --]
diff --git a/sys/arm/mv/common.c b/sys/arm/mv/common.c
index 4596a20..c7beae8 100644
--- a/sys/arm/mv/common.c
+++ b/sys/arm/mv/common.c
@@ -1693,7 +1693,7 @@ fdt_get_ranges(const char *nodename, void *buf, int size, int *tuples,
int len, tuple_size, tuples_count;
node = OF_finddevice(nodename);
- if (node <= 0)
+ if (node == -1)
return (EINVAL);
if ((fdt_addrsize_cells(node, &addr_cells, &size_cells)) != 0)
@@ -1762,11 +1762,11 @@ win_cpu_from_dt(void)
/*
* Retrieve CESA SRAM data.
*/
- if ((node = OF_finddevice("sram")) != 0)
+ if ((node = OF_finddevice("sram")) != -1)
if (fdt_is_compatible(node, "mrvl,cesa-sram"))
goto moveon;
- if ((node = OF_finddevice("/")) == 0)
+ if ((node = OF_finddevice("/")) != -1)
return (ENXIO);
if ((node = fdt_find_compatible(node, "mrvl,cesa-sram", 0)) == 0)
@@ -1796,7 +1796,7 @@ fdt_win_setup(void)
int err, i;
node = OF_finddevice("/");
- if (node == 0)
+ if (node == -1)
panic("fdt_win_setup: no root node");
node = fdt_find_compatible(node, "simple-bus", 1);
diff --git a/sys/arm/mv/mv_machdep.c b/sys/arm/mv/mv_machdep.c
index fd17692..8839740 100644
--- a/sys/arm/mv/mv_machdep.c
+++ b/sys/arm/mv/mv_machdep.c
@@ -617,13 +617,13 @@ platform_mpp_init(void)
/*
* Try to access the MPP node directly i.e. through /aliases/mpp.
*/
- if ((node = OF_finddevice("mpp")) != 0)
+ if ((node = OF_finddevice("mpp")) != -1)
if (fdt_is_compatible(node, "mrvl,mpp"))
goto moveon;
/*
* Find the node the long way.
*/
- if ((node = OF_finddevice("/")) == 0)
+ if ((node = OF_finddevice("/")) == -1)
return (ENXIO);
if ((node = fdt_find_compatible(node, "simple-bus", 0)) == 0)
@@ -752,7 +752,7 @@ platform_devmap_init(void)
/*
* PCI range(s).
*/
- if ((root = OF_finddevice("/")) == 0)
+ if ((root = OF_finddevice("/")) == -1)
return (ENXIO);
for (child = OF_child(root); child != 0; child = OF_peer(child))
@@ -779,7 +779,7 @@ platform_devmap_init(void)
/*
* CESA SRAM range.
*/
- if ((child = OF_finddevice("sram")) != 0)
+ if ((child = OF_finddevice("sram")) != -1)
if (fdt_is_compatible(child, "mrvl,cesa-sram"))
goto moveon;
[-- Attachment #4 --]
diff --git a/sys/dev/fdt/fdt_powerpc.c b/sys/dev/fdt/fdt_powerpc.c
index ea614db..ac81c08 100644
--- a/sys/dev/fdt/fdt_powerpc.c
+++ b/sys/dev/fdt/fdt_powerpc.c
@@ -62,7 +62,7 @@ fdt_fixup_busfreq(phandle_t root)
* This fixup uses /cpus/ bus-frequency prop value to set simple-bus
* bus-frequency property.
*/
- if ((cpus = OF_finddevice("/cpus")) == 0)
+ if ((cpus = OF_finddevice("/cpus")) == -1)
return;
if ((child = OF_child(cpus)) == 0)
diff --git a/sys/powerpc/booke/platform_bare.c b/sys/powerpc/booke/platform_bare.c
index ca3cfa2..f04bf96 100644
--- a/sys/powerpc/booke/platform_bare.c
+++ b/sys/powerpc/booke/platform_bare.c
@@ -189,7 +189,7 @@ bare_timebase_freq(platform_t plat, struct cpuref *cpuref)
} else
ticks = 0;
- if ((cpus = OF_finddevice("/cpus")) == 0)
+ if ((cpus = OF_finddevice("/cpus")) == -1)
goto out;
if ((child = OF_child(cpus)) == 0)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2B7sy7DMHQgo8f%2Byj7oNOkti28Hg8zOtwJwhNCgVUdksMPru1g>