* [PATCH 1/2][OF] Add of_device_is_disabled function @ 2008-02-23 21:58 Josh Boyer 2008-02-23 22:00 ` [PATCH 2/2][POWERPC] Ignore disabled serial ports Josh Boyer 2008-02-24 0:59 ` [PATCH 1/2][OF] Add of_device_is_disabled function Josh Boyer 0 siblings, 2 replies; 12+ messages in thread From: Josh Boyer @ 2008-02-23 21:58 UTC (permalink / raw) To: sfr, davem; +Cc: linuxppc-dev IEEE 1275 defined a standard "status" property to indicate the operational status of a device. The property has four possible values: okay, disabled, fail, fail-xxx. The absence of this property means the operational status of the device is unknown or okay. This adds a function called of_device_is_disabled that checks to see if a node has the status property set to "disabled". This can be quite useful for devices that may be present but disabled due to pin sharing, etc. Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com> --- drivers/of/base.c | 18 ++++++++++++++++++ include/linux/of.h | 1 + 2 files changed, 19 insertions(+) --- linux-2.6.orig/drivers/of/base.c +++ linux-2.6/drivers/of/base.c @@ -117,6 +117,24 @@ int of_device_is_compatible(const struct EXPORT_SYMBOL(of_device_is_compatible); /** + * of_device_is_disabled - Check if a device's status is disabled + * @device: device node to check + * + * Returns true or false depending on the value of the staus property + */ +int of_device_is_disabled(const struct device_node *device) +{ + const char *status; + + status = of_get_property(device, "status", NULL); + if (status == NULL) + return 0; + + return !(strcmp(status, "disabled")); +} +EXPORT_SYMBOL(of_device_is_disabled); + +/** * of_get_parent - Get a node's parent if any * @node: Node to get parent * --- linux-2.6.orig/include/linux/of.h +++ linux-2.6/include/linux/of.h @@ -62,6 +62,7 @@ extern struct property *of_find_property int *lenp); extern int of_device_is_compatible(const struct device_node *device, const char *); +extern int of_device_is_disabled(const struct device_node *device); extern const void *of_get_property(const struct device_node *node, const char *name, int *lenp); ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2][POWERPC] Ignore disabled serial ports 2008-02-23 21:58 [PATCH 1/2][OF] Add of_device_is_disabled function Josh Boyer @ 2008-02-23 22:00 ` Josh Boyer 2008-02-24 0:59 ` [PATCH 1/2][OF] Add of_device_is_disabled function Josh Boyer 1 sibling, 0 replies; 12+ messages in thread From: Josh Boyer @ 2008-02-23 22:00 UTC (permalink / raw) To: arnd; +Cc: sfr, davem, linuxppc-dev Some SoC chips have multiple serial ports on board. The usability of these ports can rely on various factors, ranging from pin sharing to unpopulated connectors. This uses the new of_device_is_disabled function to check for and ignore disabled UARTs. Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com> --- arch/powerpc/kernel/legacy_serial.c | 4 ++++ drivers/serial/of_serial.c | 5 +++++ 2 files changed, 9 insertions(+) --- linux-2.6.orig/drivers/serial/of_serial.c +++ linux-2.6/drivers/serial/of_serial.c @@ -72,6 +72,11 @@ static int __devinit of_platform_serial_ int port_type; int ret; + if (of_device_is_disabled(ofdev->node)) { + dev_info(&ofdev->dev, "Disabled serial port. Ignored\n"); + return -ENODEV; + } + if (of_find_property(ofdev->node, "used-by-rtas", NULL)) return -EBUSY; --- linux-2.6.orig/arch/powerpc/kernel/legacy_serial.c +++ linux-2.6/arch/powerpc/kernel/legacy_serial.c @@ -54,6 +54,10 @@ static int __init add_legacy_port(struct u32 clock = BASE_BAUD * 16; int index; + /* Check the status property if present. Ignore disabled devices */ + if (of_device_is_disabled(np)) + return -1; + /* get clock freq. if present */ clk = of_get_property(np, "clock-frequency", NULL); if (clk && *clk) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2][OF] Add of_device_is_disabled function 2008-02-23 21:58 [PATCH 1/2][OF] Add of_device_is_disabled function Josh Boyer 2008-02-23 22:00 ` [PATCH 2/2][POWERPC] Ignore disabled serial ports Josh Boyer @ 2008-02-24 0:59 ` Josh Boyer 2008-02-24 2:23 ` [PATCH][OF] Add of_device_is_available function Josh Boyer ` (2 more replies) 1 sibling, 3 replies; 12+ messages in thread From: Josh Boyer @ 2008-02-24 0:59 UTC (permalink / raw) To: sfr, davem, benh; +Cc: linuxppc-dev On Sat, 23 Feb 2008 15:58:23 -0600 Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote: > IEEE 1275 defined a standard "status" property to indicate the operational > status of a device. The property has four possible values: okay, disabled, > fail, fail-xxx. The absence of this property means the operational status > of the device is unknown or okay. > > This adds a function called of_device_is_disabled that checks to see if a > node has the status property set to "disabled". This can be quite useful > for devices that may be present but disabled due to pin sharing, etc. > > Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com> Talking with Ben H a bit, he suggested to reverse this API. Basically, create an of_device_is_available that returns 1 if the status property is completely missing, or if it's set to "okay" or "ok". The latter is to cope with some broken firmwares. I can do either really. Eventually you could embed the is_available check in the of_platform code so that devices don't even get presented to drivers if they aren't available. Dave, I'm not sure how applicable this all is to sparc. But for some of the "newer" embedded ports that are coming into powerpc I can see it being very useful. Thoughts? josh ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH][OF] Add of_device_is_available function 2008-02-24 0:59 ` [PATCH 1/2][OF] Add of_device_is_disabled function Josh Boyer @ 2008-02-24 2:23 ` Josh Boyer 2008-02-26 9:04 ` Paul Mackerras 2008-02-24 2:45 ` [PATCH 1/2][OF] Add of_device_is_disabled function David Miller 2008-02-24 20:12 ` Nathan Lynch 2 siblings, 1 reply; 12+ messages in thread From: Josh Boyer @ 2008-02-24 2:23 UTC (permalink / raw) To: sfr, davem, benh; +Cc: linuxppc-dev On Sat, 23 Feb 2008 18:59:04 -0600 Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote: > On Sat, 23 Feb 2008 15:58:23 -0600 > Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote: > > > IEEE 1275 defined a standard "status" property to indicate the operational > > status of a device. The property has four possible values: okay, disabled, > > fail, fail-xxx. The absence of this property means the operational status > > of the device is unknown or okay. > > > > This adds a function called of_device_is_disabled that checks to see if a > > node has the status property set to "disabled". This can be quite useful > > for devices that may be present but disabled due to pin sharing, etc. > > > > Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com> > > Talking with Ben H a bit, he suggested to reverse this API. Basically, > create an of_device_is_available that returns 1 if the status property > is completely missing, or if it's set to "okay" or "ok". The latter is > to cope with some broken firmwares. And since I seem to be talking to myself and have nothing better to do on a Saturday evening, here's the code. josh IEEE 1275 defined a standard "status" property to indicate the operational status of a device. The property has four possible values: okay, disabled, fail, fail-xxx. The absence of this property means the operational status of the device is unknown or okay. This adds a function called of_device_is_available that checks the state of the status property of a device. If the property is absent or set to either "okay" or "ok", it returns 1. Otherwise it returns 0. Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com> --- drivers/of/base.c | 23 +++++++++++++++++++++++ include/linux/of.h | 1 + 2 files changed, 24 insertions(+) --- linux-2.6.orig/drivers/of/base.c +++ linux-2.6/drivers/of/base.c @@ -117,6 +117,29 @@ int of_device_is_compatible(const struct EXPORT_SYMBOL(of_device_is_compatible); /** + * of_device_is_available - check if a device is available for use + * + * @device: Node to check for availability + * + * Returns 1 if the status property is absent or set to "okay" or "ok", + * 0 otherwise + */ +int of_device_is_available(const struct device_node *device) +{ + const char *status; + + status = of_get_property(device, "status", NULL); + if (status == NULL) + return 1; + + if (!strcmp(status, "okay") || !strcmp(status, "ok")) + return 1; + + return 0; +} +EXPORT_SYMBOL(of_device_is_available); + +/** * of_get_parent - Get a node's parent if any * @node: Node to get parent * --- linux-2.6.orig/include/linux/of.h +++ linux-2.6/include/linux/of.h @@ -62,6 +62,7 @@ extern struct property *of_find_property int *lenp); extern int of_device_is_compatible(const struct device_node *device, const char *); +extern int of_device_is_available(const struct device_node *device); extern const void *of_get_property(const struct device_node *node, const char *name, int *lenp); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][OF] Add of_device_is_available function 2008-02-24 2:23 ` [PATCH][OF] Add of_device_is_available function Josh Boyer @ 2008-02-26 9:04 ` Paul Mackerras 2008-02-26 21:34 ` David Miller 0 siblings, 1 reply; 12+ messages in thread From: Paul Mackerras @ 2008-02-26 9:04 UTC (permalink / raw) To: Josh Boyer; +Cc: linuxppc-dev, sfr, davem Josh Boyer writes: > + status = of_get_property(device, "status", NULL); > + if (status == NULL) > + return 1; > + > + if (!strcmp(status, "okay") || !strcmp(status, "ok")) It would probably be good to defend against the possibility that the property isn't null-terminated (for example if its length is zero). Paul. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][OF] Add of_device_is_available function 2008-02-26 9:04 ` Paul Mackerras @ 2008-02-26 21:34 ` David Miller 2008-02-26 21:45 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 12+ messages in thread From: David Miller @ 2008-02-26 21:34 UTC (permalink / raw) To: paulus; +Cc: linuxppc-dev, sfr From: Paul Mackerras <paulus@samba.org> Date: Tue, 26 Feb 2008 20:04:12 +1100 > It would probably be good to defend against the possibility that the > property isn't null-terminated (for example if its length is zero). FWIW, when I pull in the device tree on sparc I eliminate any need for those kinds of checks by putting a '\0' at the end of every property blob. I copied a lot of this code from you guys, so it wouldn't surprise me if ppc does this too. :-) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][OF] Add of_device_is_available function 2008-02-26 21:34 ` David Miller @ 2008-02-26 21:45 ` Benjamin Herrenschmidt 2008-02-26 22:44 ` David Miller 0 siblings, 1 reply; 12+ messages in thread From: Benjamin Herrenschmidt @ 2008-02-26 21:45 UTC (permalink / raw) To: David Miller; +Cc: sfr, paulus, linuxppc-dev On Tue, 2008-02-26 at 13:34 -0800, David Miller wrote: > From: Paul Mackerras <paulus@samba.org> > Date: Tue, 26 Feb 2008 20:04:12 +1100 > > > It would probably be good to defend against the possibility that the > > property isn't null-terminated (for example if its length is zero). > > FWIW, when I pull in the device tree on sparc I eliminate any need for > those kinds of checks by putting a '\0' at the end of every property > blob. > > I copied a lot of this code from you guys, so it wouldn't surprise > me if ppc does this too. :-) I doubt we do that. Properties that contain things like ranges, or "reg" properties are expected to be of a size that is a multiple of #size-cells/#address-cells and I'm not sure that won't break things here or there if they suddenly get one more byte.. Or do you mean you/we are appending that-without- changing the length field ? Ben. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][OF] Add of_device_is_available function 2008-02-26 21:45 ` Benjamin Herrenschmidt @ 2008-02-26 22:44 ` David Miller 2008-02-26 22:58 ` Benjamin Herrenschmidt 2008-02-26 23:53 ` Josh Boyer 0 siblings, 2 replies; 12+ messages in thread From: David Miller @ 2008-02-26 22:44 UTC (permalink / raw) To: benh; +Cc: sfr, paulus, linuxppc-dev From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: Wed, 27 Feb 2008 08:45:37 +1100 > I doubt we do that. Properties that contain things like ranges, or "reg" > properties are expected to be of a size that is a multiple of > #size-cells/#address-cells and I'm not sure that won't break things here > or there if they suddenly get one more byte.. > > Or do you mean you/we are appending that-without- changing the length > field ? Right, simply don't change the length field. Put the zero byte at offset "length + 1" It's stupid to validate NULL termination everywhere when we can make it an invariant in one spot. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][OF] Add of_device_is_available function 2008-02-26 22:44 ` David Miller @ 2008-02-26 22:58 ` Benjamin Herrenschmidt 2008-02-26 23:53 ` Josh Boyer 1 sibling, 0 replies; 12+ messages in thread From: Benjamin Herrenschmidt @ 2008-02-26 22:58 UTC (permalink / raw) To: David Miller; +Cc: sfr, paulus, linuxppc-dev On Tue, 2008-02-26 at 14:44 -0800, David Miller wrote: > From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Date: Wed, 27 Feb 2008 08:45:37 +1100 > > > I doubt we do that. Properties that contain things like ranges, or "reg" > > properties are expected to be of a size that is a multiple of > > #size-cells/#address-cells and I'm not sure that won't break things here > > or there if they suddenly get one more byte.. > > > > Or do you mean you/we are appending that-without- changing the length > > field ? > > Right, simply don't change the length field. Put the zero byte > at offset "length + 1" > > It's stupid to validate NULL termination everywhere when we > can make it an invariant in one spot. Fair enough. Ben. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH][OF] Add of_device_is_available function 2008-02-26 22:44 ` David Miller 2008-02-26 22:58 ` Benjamin Herrenschmidt @ 2008-02-26 23:53 ` Josh Boyer 1 sibling, 0 replies; 12+ messages in thread From: Josh Boyer @ 2008-02-26 23:53 UTC (permalink / raw) To: David Miller; +Cc: linuxppc-dev, paulus, sfr On Tue, 26 Feb 2008 14:44:23 -0800 (PST) David Miller <davem@davemloft.net> wrote: > From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Date: Wed, 27 Feb 2008 08:45:37 +1100 > > > I doubt we do that. Properties that contain things like ranges, or "reg" > > properties are expected to be of a size that is a multiple of > > #size-cells/#address-cells and I'm not sure that won't break things here > > or there if they suddenly get one more byte.. > > > > Or do you mean you/we are appending that-without- changing the length > > field ? > > Right, simply don't change the length field. Put the zero byte > at offset "length + 1" > > It's stupid to validate NULL termination everywhere when we > can make it an invariant in one spot. I don't mind fixing up the function to use strncmp and checking for a 0 length from of_get_property. However, I'm almost certain that other places in the code have the same issue so what you're saying here seems to make sense. josh ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2][OF] Add of_device_is_disabled function 2008-02-24 0:59 ` [PATCH 1/2][OF] Add of_device_is_disabled function Josh Boyer 2008-02-24 2:23 ` [PATCH][OF] Add of_device_is_available function Josh Boyer @ 2008-02-24 2:45 ` David Miller 2008-02-24 20:12 ` Nathan Lynch 2 siblings, 0 replies; 12+ messages in thread From: David Miller @ 2008-02-24 2:45 UTC (permalink / raw) To: jwboyer; +Cc: linuxppc-dev, sfr From: Josh Boyer <jwboyer@linux.vnet.ibm.com> Date: Sat, 23 Feb 2008 18:59:04 -0600 > Dave, I'm not sure how applicable this all is to sparc. But for some > of the "newer" embedded ports that are coming into powerpc I can see it > being very useful. I think I've seen this property on sparc boxes too, I'm fine with whatever you guys come up with. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2][OF] Add of_device_is_disabled function 2008-02-24 0:59 ` [PATCH 1/2][OF] Add of_device_is_disabled function Josh Boyer 2008-02-24 2:23 ` [PATCH][OF] Add of_device_is_available function Josh Boyer 2008-02-24 2:45 ` [PATCH 1/2][OF] Add of_device_is_disabled function David Miller @ 2008-02-24 20:12 ` Nathan Lynch 2 siblings, 0 replies; 12+ messages in thread From: Nathan Lynch @ 2008-02-24 20:12 UTC (permalink / raw) To: Josh Boyer; +Cc: linuxppc-dev, sfr, davem Josh Boyer wrote: > On Sat, 23 Feb 2008 15:58:23 -0600 > Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote: > > > IEEE 1275 defined a standard "status" property to indicate the operational > > status of a device. The property has four possible values: okay, disabled, > > fail, fail-xxx. The absence of this property means the operational status > > of the device is unknown or okay. > > > > This adds a function called of_device_is_disabled that checks to see if a > > node has the status property set to "disabled". This can be quite useful > > for devices that may be present but disabled due to pin sharing, etc. > > > > Talking with Ben H a bit, he suggested to reverse this API. Basically, > create an of_device_is_available that returns 1 if the status property > is completely missing, or if it's set to "okay" or "ok". The latter is > to cope with some broken firmwares. I agree with Ben's suggestion. The rtas_pci and eeh code could be converted to use this, which gives a net savings of a few bytes with ppc64_defconfig: diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c index 433a0a0..39a752b 100644 --- a/arch/powerpc/kernel/rtas_pci.c +++ b/arch/powerpc/kernel/rtas_pci.c @@ -56,21 +56,6 @@ static inline int config_access_valid(struct pci_dn *dn, int where) return 0; } -static int of_device_available(struct device_node * dn) -{ - const char *status; - - status = of_get_property(dn, "status", NULL); - - if (!status) - return 1; - - if (!strcmp(status, "okay")) - return 1; - - return 0; -} - int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val) { int returnval = -1; @@ -117,7 +102,7 @@ static int rtas_pci_read_config(struct pci_bus *bus, for (dn = busdn->child; dn; dn = dn->sibling) { struct pci_dn *pdn = PCI_DN(dn); if (pdn && pdn->devfn == devfn - && of_device_available(dn)) + && of_device_is_available(dn)) return rtas_read_config(pdn, where, size, val); } @@ -164,7 +149,7 @@ static int rtas_pci_write_config(struct pci_bus *bus, for (dn = busdn->child; dn; dn = dn->sibling) { struct pci_dn *pdn = PCI_DN(dn); if (pdn && pdn->devfn == devfn - && of_device_available(dn)) + && of_device_is_available(dn)) return rtas_write_config(pdn, where, size, val); } return PCIBIOS_DEVICE_NOT_FOUND; diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c index 9eb539e..550b2f7 100644 --- a/arch/powerpc/platforms/pseries/eeh.c +++ b/arch/powerpc/platforms/pseries/eeh.c @@ -945,7 +945,6 @@ static void *early_enable_eeh(struct device_node *dn, void *data) unsigned int rets[3]; struct eeh_early_enable_info *info = data; int ret; - const char *status = of_get_property(dn, "status", NULL); const u32 *class_code = of_get_property(dn, "class-code", NULL); const u32 *vendor_id = of_get_property(dn, "vendor-id", NULL); const u32 *device_id = of_get_property(dn, "device-id", NULL); @@ -959,8 +958,8 @@ static void *early_enable_eeh(struct device_node *dn, void *data) pdn->eeh_freeze_count = 0; pdn->eeh_false_positives = 0; - if (status && strncmp(status, "ok", 2) != 0) - return NULL; /* ignore devices with bad status */ + if (!of_device_is_available(dn)) + return NULL; /* Ignore bad nodes. */ if (!class_code || !vendor_id || !device_id) ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2008-02-26 23:54 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-02-23 21:58 [PATCH 1/2][OF] Add of_device_is_disabled function Josh Boyer 2008-02-23 22:00 ` [PATCH 2/2][POWERPC] Ignore disabled serial ports Josh Boyer 2008-02-24 0:59 ` [PATCH 1/2][OF] Add of_device_is_disabled function Josh Boyer 2008-02-24 2:23 ` [PATCH][OF] Add of_device_is_available function Josh Boyer 2008-02-26 9:04 ` Paul Mackerras 2008-02-26 21:34 ` David Miller 2008-02-26 21:45 ` Benjamin Herrenschmidt 2008-02-26 22:44 ` David Miller 2008-02-26 22:58 ` Benjamin Herrenschmidt 2008-02-26 23:53 ` Josh Boyer 2008-02-24 2:45 ` [PATCH 1/2][OF] Add of_device_is_disabled function David Miller 2008-02-24 20:12 ` Nathan Lynch
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).