Re: [PATCH 1/2][OF] Add of_device_is_disabled function

2008-02-24 Thread Nathan Lynch 
Josh Boyer wrote:
 On Sat, 23 Feb 2008 15:58:23 -0600
 Josh Boyer [EMAIL PROTECTED] 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)
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH 1/2][OF] Add of_device_is_disabled function

2008-02-23 Thread Josh Boyer 
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 [EMAIL PROTECTED]

---
 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);
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 1/2][OF] Add of_device_is_disabled function

2008-02-23 Thread Josh Boyer 
On Sat, 23 Feb 2008 15:58:23 -0600
Josh Boyer [EMAIL PROTECTED] 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 [EMAIL PROTECTED]

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
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 1/2][OF] Add of_device_is_disabled function

2008-02-23 Thread David Miller 
From: Josh Boyer [EMAIL PROTECTED]
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.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev