linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2 v2] [OF] Add of_device_is_available function
@ 2008-03-01 14:16 Josh Boyer
  2008-03-01 14:17 ` [PATCH 2/2 v2] [POWERPC] Ignore disabled serial ports Josh Boyer
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Josh Boyer @ 2008-03-01 14:16 UTC (permalink / raw)
  To: paulus, sfr; +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_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  |   26 ++++++++++++++++++++++++++
 include/linux/of.h |    1 +
 2 files changed, 27 insertions(+)

--- linux-2.6.orig/drivers/of/base.c
+++ linux-2.6/drivers/of/base.c
@@ -117,6 +117,32 @@ 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;
+	int statlen;
+
+	status = of_get_property(device, "status", &statlen);
+	if (status == NULL)
+		return 1;
+
+	if (statlen > 0) {
+		if (!strncmp(status, "okay", 4) || !strncmp(status,
"ok", 2))
+			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] 21+ messages in thread

* [PATCH 2/2 v2] [POWERPC] Ignore disabled serial ports
  2008-03-01 14:16 [PATCH 1/2 v2] [OF] Add of_device_is_available function Josh Boyer
@ 2008-03-01 14:17 ` Josh Boyer
  2008-03-03  3:43   ` Arnd Bergmann
  2008-03-01 14:41 ` [PATCH 3/2][NEWEMAC] Use status property for unused/unwired EMACs Josh Boyer
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 21+ messages in thread
From: Josh Boyer @ 2008-03-01 14:17 UTC (permalink / raw)
  To: paulus; +Cc: linuxppc-dev, arnd

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 checks for a status property in the serial node and ignores
the port if it is set to "disabled".

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_available(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_available(np))
+		return -1;
+
 	/* get clock freq. if present */
 	clk = of_get_property(np, "clock-frequency", NULL);
 	if (clk && *clk)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 3/2][NEWEMAC] Use status property for unused/unwired EMACs
  2008-03-01 14:16 [PATCH 1/2 v2] [OF] Add of_device_is_available function Josh Boyer
  2008-03-01 14:17 ` [PATCH 2/2 v2] [POWERPC] Ignore disabled serial ports Josh Boyer
@ 2008-03-01 14:41 ` Josh Boyer
  2008-03-01 21:33   ` Benjamin Herrenschmidt
  2008-03-02 19:43   ` Sean MacLennan
  2008-03-01 22:02 ` [PATCH 1/2 v2] [OF] Add of_device_is_available function Stephen Rothwell
  2008-03-01 23:48 ` [RESEND] " Josh Boyer
  3 siblings, 2 replies; 21+ messages in thread
From: Josh Boyer @ 2008-03-01 14:41 UTC (permalink / raw)
  To: paulus, benh; +Cc: linuxppc-dev

Convert ibm_newemac to use the of_device_is_available function when checking
for unused/unwired EMACs.  We leave the current check for an "unused" property
to maintain backwards compatibility for older device trees.  Newer device
trees should simply use the standard "status" property in the EMAC node.

The taishan DTS file is updated to reflect this.

Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>

---
 arch/powerpc/boot/dts/taishan.dts |    4 ++--
 drivers/net/ibm_newemac/core.c    |    7 +++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

--- linux-2.6.orig/arch/powerpc/boot/dts/taishan.dts
+++ linux-2.6/arch/powerpc/boot/dts/taishan.dts
@@ -234,7 +234,6 @@
 
 
 			EMAC0: ethernet@40000800 {
-				unused = <1>;
 				linux,network-index = <2>;
 				device_type = "network";
 				compatible = "ibm,emac-440gx", "ibm,emac4";
@@ -253,9 +252,9 @@
 				phy-map = <00000001>;
 				zmii-device = <&ZMII0>;
 				zmii-channel = <0>;
+				status = "disabled";
 			};
 		 	EMAC1: ethernet@40000900 {
-				unused = <1>;
 				linux,network-index = <3>;
 				device_type = "network";
 				compatible = "ibm,emac-440gx", "ibm,emac4";
@@ -274,6 +273,7 @@
 				phy-map = <00000001>;
  				zmii-device = <&ZMII0>;
 				zmii-channel = <1>;
+				status = "disabled";
 			};
 
 		 	EMAC2: ethernet@40000c00 {
--- linux-2.6.orig/drivers/net/ibm_newemac/core.c
+++ linux-2.6/drivers/net/ibm_newemac/core.c
@@ -2552,8 +2552,11 @@ static int __devinit emac_probe(struct o
 	struct device_node **blist = NULL;
 	int err, i;
 
-	/* Skip unused/unwired EMACS */
-	if (of_get_property(np, "unused", NULL))
+	/* Skip unused/unwired EMACS.  We leave the check for an unused
+	 * property here for now, but new flat device trees should set a
+	 * status property to "disabled" instead.
+	 */
+	if (of_get_property(np, "unused", NULL) || !of_device_is_available(np))
 		return -ENODEV;
 
 	/* Find ourselves in the bootlist if we are there */

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/2][NEWEMAC] Use status property for unused/unwired EMACs
  2008-03-01 14:41 ` [PATCH 3/2][NEWEMAC] Use status property for unused/unwired EMACs Josh Boyer
@ 2008-03-01 21:33   ` Benjamin Herrenschmidt
  2008-03-01 23:50     ` Josh Boyer
  2008-03-02 19:43   ` Sean MacLennan
  1 sibling, 1 reply; 21+ messages in thread
From: Benjamin Herrenschmidt @ 2008-03-01 21:33 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, paulus


On Sat, 2008-03-01 at 08:41 -0600, Josh Boyer wrote:
> Convert ibm_newemac to use the of_device_is_available function when checking
> for unused/unwired EMACs.  We leave the current check for an "unused" property
> to maintain backwards compatibility for older device trees.  Newer device
> trees should simply use the standard "status" property in the EMAC node.
> 
> The taishan DTS file is updated to reflect this.
> 
> Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>

I don't think any other DTS than Taishan uses "unused" so we may be able
to get rid of that backward compat check. I think the Cell DT just
ommits EMACs when they are unused.

Ben.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2 v2] [OF] Add of_device_is_available function
  2008-03-01 14:16 [PATCH 1/2 v2] [OF] Add of_device_is_available function Josh Boyer
  2008-03-01 14:17 ` [PATCH 2/2 v2] [POWERPC] Ignore disabled serial ports Josh Boyer
  2008-03-01 14:41 ` [PATCH 3/2][NEWEMAC] Use status property for unused/unwired EMACs Josh Boyer
@ 2008-03-01 22:02 ` Stephen Rothwell
  2008-03-01 22:25   ` Josh Boyer
  2008-03-01 23:48 ` [RESEND] " Josh Boyer
  3 siblings, 1 reply; 21+ messages in thread
From: Stephen Rothwell @ 2008-03-01 22:02 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, paulus

[-- Attachment #1: Type: text/plain, Size: 848 bytes --]

Hi Josh,

On Sat, 1 Mar 2008 08:16:00 -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_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>

Looks good to me - except the patch is word wrapped (which is weird for
Claws) ...

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

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2 v2] [OF] Add of_device_is_available function
  2008-03-01 22:02 ` [PATCH 1/2 v2] [OF] Add of_device_is_available function Stephen Rothwell
@ 2008-03-01 22:25   ` Josh Boyer
  0 siblings, 0 replies; 21+ messages in thread
From: Josh Boyer @ 2008-03-01 22:25 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linuxppc-dev, paulus

On Sun, 2 Mar 2008 09:02:18 +1100
Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Josh,
> 
> On Sat, 1 Mar 2008 08:16:00 -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_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>
> 
> Looks good to me - except the patch is word wrapped (which is weird for
> Claws) ...

That's weird indeed...  I've never had that happen before.  I'll resend.

josh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RESEND] [PATCH 1/2 v2] [OF] Add of_device_is_available function
  2008-03-01 14:16 [PATCH 1/2 v2] [OF] Add of_device_is_available function Josh Boyer
                   ` (2 preceding siblings ...)
  2008-03-01 22:02 ` [PATCH 1/2 v2] [OF] Add of_device_is_available function Stephen Rothwell
@ 2008-03-01 23:48 ` Josh Boyer
  2008-03-24 11:39   ` Paul Mackerras
  3 siblings, 1 reply; 21+ messages in thread
From: Josh Boyer @ 2008-03-01 23:48 UTC (permalink / raw)
  To: paulus, sfr; +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_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  |   26 ++++++++++++++++++++++++++
 include/linux/of.h |    1 +
 2 files changed, 27 insertions(+)

--- linux-2.6.orig/drivers/of/base.c
+++ linux-2.6/drivers/of/base.c
@@ -117,6 +117,32 @@ 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;
+	int statlen;
+
+	status = of_get_property(device, "status", &statlen);
+	if (status == NULL)
+		return 1;
+
+	if (statlen > 0) {
+		if (!strncmp(status, "okay", 4) || !strncmp(status, "ok", 2))
+			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] 21+ messages in thread

* Re: [PATCH 3/2][NEWEMAC] Use status property for unused/unwired EMACs
  2008-03-01 21:33   ` Benjamin Herrenschmidt
@ 2008-03-01 23:50     ` Josh Boyer
  0 siblings, 0 replies; 21+ messages in thread
From: Josh Boyer @ 2008-03-01 23:50 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev, paulus

On Sun, 02 Mar 2008 08:33:15 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> 
> On Sat, 2008-03-01 at 08:41 -0600, Josh Boyer wrote:
> > Convert ibm_newemac to use the of_device_is_available function when checking
> > for unused/unwired EMACs.  We leave the current check for an "unused" property
> > to maintain backwards compatibility for older device trees.  Newer device
> > trees should simply use the standard "status" property in the EMAC node.
> > 
> > The taishan DTS file is updated to reflect this.
> > 
> > Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
> 
> I don't think any other DTS than Taishan uses "unused" so we may be able
> to get rid of that backward compat check. I think the Cell DT just
> ommits EMACs when they are unused.

I wasn't sure on that aspect and I can't check until next week.  If you
can verify before I do, let me know.  I'll remove the "unused" check
assuming the of_device_is_available patch goes in and the Axon DT
doesn't use it.

josh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/2][NEWEMAC] Use status property for unused/unwired EMACs
  2008-03-01 14:41 ` [PATCH 3/2][NEWEMAC] Use status property for unused/unwired EMACs Josh Boyer
  2008-03-01 21:33   ` Benjamin Herrenschmidt
@ 2008-03-02 19:43   ` Sean MacLennan
  2008-03-02 22:23     ` Josh Boyer
  1 sibling, 1 reply; 21+ messages in thread
From: Sean MacLennan @ 2008-03-02 19:43 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev

Josh Boyer wrote:
> Convert ibm_newemac to use the of_device_is_available function when checking
> for unused/unwired EMACs.  We leave the current check for an "unused" property
> to maintain backwards compatibility for older device trees.  Newer device
> trees should simply use the standard "status" property in the EMAC node.
>
> The taishan DTS file is updated to reflect this.
>   
What is the advantage of documenting unwired devices? I ask because the 
taco does not have emac1 connected. I handle it by not defining emac1 in 
the dts file.

Is it better to document it and disable it? Or is this mainly for 
reference boards to show that while the PPC supports to emacs, only one 
is enabled on the reference board?

i.e. We want reference design boards to show what *could* be done, but 
production boards to show what *is* done?

Cheers,
   Sean

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/2][NEWEMAC] Use status property for unused/unwired EMACs
  2008-03-02 19:43   ` Sean MacLennan
@ 2008-03-02 22:23     ` Josh Boyer
  0 siblings, 0 replies; 21+ messages in thread
From: Josh Boyer @ 2008-03-02 22:23 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev

On Sun, 02 Mar 2008 14:43:31 -0500
Sean MacLennan <smaclennan@pikatech.com> wrote:

> Josh Boyer wrote:
> > Convert ibm_newemac to use the of_device_is_available function when checking
> > for unused/unwired EMACs.  We leave the current check for an "unused" property
> > to maintain backwards compatibility for older device trees.  Newer device
> > trees should simply use the standard "status" property in the EMAC node.
> >
> > The taishan DTS file is updated to reflect this.
> >   
> What is the advantage of documenting unwired devices? I ask because the 
> taco does not have emac1 connected. I handle it by not defining emac1 in 
> the dts file.

If it never has the possibility of becoming usable without some kind
of hardware rework, then that's probably best.

> Is it better to document it and disable it? Or is this mainly for 
> reference boards to show that while the PPC supports to emacs, only one 
> is enabled on the reference board?
> 
> i.e. We want reference design boards to show what *could* be done, but 
> production boards to show what *is* done?

Sort of.  A lot of these eval boards have DIP switches that change pins
to various devices.  So for those, we enumerate most things and we can
adjust the status property based on those settings in the
firmware/wrapper.

josh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2 v2] [POWERPC] Ignore disabled serial ports
  2008-03-01 14:17 ` [PATCH 2/2 v2] [POWERPC] Ignore disabled serial ports Josh Boyer
@ 2008-03-03  3:43   ` Arnd Bergmann
  2008-03-03  4:43     ` Josh Boyer
  0 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2008-03-03  3:43 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, paulus

On Saturday 01 March 2008, Josh Boyer wrote:
> --- 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_
> =A0=A0=A0=A0=A0=A0=A0=A0int port_type;
> =A0=A0=A0=A0=A0=A0=A0=A0int ret;
> =A0
> +=A0=A0=A0=A0=A0=A0=A0if (!of_device_is_available(ofdev->node)) {
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0dev_info(&ofdev->dev, "Disa=
bled serial port. =A0Ignored\n");
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return -ENODEV;
> +=A0=A0=A0=A0=A0=A0=A0}
> +
> =A0=A0=A0=A0=A0=A0=A0=A0if (of_find_property(ofdev->node, "used-by-rtas",=
 NULL))
> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return -EBUSY;

I wonder whether we should move the check for "used-by-rtas" into the
of_device_is_available function. I understand that used-by-rtas is
another way of expressing the idea that the kernel is not supposed to
access the specific device. In this case, the device is physically
present, but is not available to the OS.

	Arnd <><

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2 v2] [POWERPC] Ignore disabled serial ports
  2008-03-03  3:43   ` Arnd Bergmann
@ 2008-03-03  4:43     ` Josh Boyer
  2008-03-03 19:09       ` Scott Wood
  0 siblings, 1 reply; 21+ messages in thread
From: Josh Boyer @ 2008-03-03  4:43 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, paulus

On Mon, 3 Mar 2008 04:43:42 +0100
Arnd Bergmann <arnd@arndb.de> wrote:

> On Saturday 01 March 2008, Josh Boyer wrote:
> > --- 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_
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int port_type;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int ret;
> > =C2=A0
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (!of_device_is_available(=
ofdev->node)) {
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0dev_info(&ofdev->dev, "Disabled serial port. =C2=A0Ign=
ored\n");
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0return -ENODEV;
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
> > +
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (of_find_property(of=
dev->node, "used-by-rtas", NULL))
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0return -EBUSY;
>=20
> I wonder whether we should move the check for "used-by-rtas" into the
> of_device_is_available function. I understand that used-by-rtas is
> another way of expressing the idea that the kernel is not supposed to
> access the specific device. In this case, the device is physically
> present, but is not available to the OS.

I'd rather not at the moment.  My intention was to only look at the
status property for now.  I'd like to avoid this function growing into
a huge switch statement for $random_firmware's way of flagging
something as "don't touch".

josh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2 v2] [POWERPC] Ignore disabled serial ports
  2008-03-03  4:43     ` Josh Boyer
@ 2008-03-03 19:09       ` Scott Wood
  2008-03-03 19:21         ` Josh Boyer
  0 siblings, 1 reply; 21+ messages in thread
From: Scott Wood @ 2008-03-03 19:09 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, paulus, Arnd Bergmann

On Sun, Mar 02, 2008 at 10:43:17PM -0600, Josh Boyer wrote:
> On Mon, 3 Mar 2008 04:43:42 +0100
> Arnd Bergmann <arnd@arndb.de> wrote:
> > I wonder whether we should move the check for "used-by-rtas" into the
> > of_device_is_available function. I understand that used-by-rtas is
> > another way of expressing the idea that the kernel is not supposed to
> > access the specific device. In this case, the device is physically
> > present, but is not available to the OS.
> 
> I'd rather not at the moment.  My intention was to only look at the
> status property for now.  I'd like to avoid this function growing into
> a huge switch statement for $random_firmware's way of flagging
> something as "don't touch".

Better that than having the "huge" list of tests in every driver...

-Scott

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2 v2] [POWERPC] Ignore disabled serial ports
  2008-03-03 19:09       ` Scott Wood
@ 2008-03-03 19:21         ` Josh Boyer
  2008-03-03 21:40           ` Nathan Lynch
  0 siblings, 1 reply; 21+ messages in thread
From: Josh Boyer @ 2008-03-03 19:21 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, paulus, Arnd Bergmann

On Mon, 3 Mar 2008 13:09:25 -0600
Scott Wood <scottwood@freescale.com> wrote:

> On Sun, Mar 02, 2008 at 10:43:17PM -0600, Josh Boyer wrote:
> > On Mon, 3 Mar 2008 04:43:42 +0100
> > Arnd Bergmann <arnd@arndb.de> wrote:
> > > I wonder whether we should move the check for "used-by-rtas" into the
> > > of_device_is_available function. I understand that used-by-rtas is
> > > another way of expressing the idea that the kernel is not supposed to
> > > access the specific device. In this case, the device is physically
> > > present, but is not available to the OS.
> > 
> > I'd rather not at the moment.  My intention was to only look at the
> > status property for now.  I'd like to avoid this function growing into
> > a huge switch statement for $random_firmware's way of flagging
> > something as "don't touch".
> 
> Better that than having the "huge" list of tests in every driver...

Perhaps.

This isn't set in stone.  I'd rather get what's in the patch in-tree
now and massage it as we go.  Otherwise this bike shed will wind up
being rainbow colored yet totally useless because it's a never-ending
patch rework.

josh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2 v2] [POWERPC] Ignore disabled serial ports
  2008-03-03 19:21         ` Josh Boyer
@ 2008-03-03 21:40           ` Nathan Lynch
  2008-03-04  0:42             ` Arnd Bergmann
  0 siblings, 1 reply; 21+ messages in thread
From: Nathan Lynch @ 2008-03-03 21:40 UTC (permalink / raw)
  To: Josh Boyer; +Cc: Scott Wood, linuxppc-dev, paulus, Arnd Bergmann

Josh Boyer wrote:
> On Mon, 3 Mar 2008 13:09:25 -0600
> Scott Wood <scottwood@freescale.com> wrote:
> 
> > On Sun, Mar 02, 2008 at 10:43:17PM -0600, Josh Boyer wrote:
> > > On Mon, 3 Mar 2008 04:43:42 +0100
> > > Arnd Bergmann <arnd@arndb.de> wrote:
> > > > I wonder whether we should move the check for "used-by-rtas" into the
> > > > of_device_is_available function. I understand that used-by-rtas is
> > > > another way of expressing the idea that the kernel is not supposed to
> > > > access the specific device. In this case, the device is physically
> > > > present, but is not available to the OS.
> > > 
> > > I'd rather not at the moment.  My intention was to only look at the
> > > status property for now.  I'd like to avoid this function growing into
> > > a huge switch statement for $random_firmware's way of flagging
> > > something as "don't touch".
> > 
> > Better that than having the "huge" list of tests in every driver...
> 
> Perhaps.
> 
> This isn't set in stone.  I'd rather get what's in the patch in-tree
> now and massage it as we go.  Otherwise this bike shed will wind up
> being rainbow colored yet totally useless because it's a never-ending
> patch rework.

I agree.  Josh's patch is immediately useful to other code as-is.

used-by-rtas is powerpc-specific and doesn't belong in drivers/of IMO.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2 v2] [POWERPC] Ignore disabled serial ports
  2008-03-03 21:40           ` Nathan Lynch
@ 2008-03-04  0:42             ` Arnd Bergmann
  0 siblings, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2008-03-04  0:42 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Scott Wood, Nathan Lynch, paulus

On Monday 03 March 2008, Nathan Lynch wrote:
> I agree. =A0Josh's patch is immediately useful to other code as-is.
>=20
> used-by-rtas is powerpc-specific and doesn't belong in drivers/of IMO.

Ok, makes sense, plus paulus looked at the PAPR spec with me and we found
that used-by-rtas doesn't necessarily mean "don't touch" in all
circumstances, so original patch

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RESEND] [PATCH 1/2 v2] [OF] Add of_device_is_available function
  2008-03-01 23:48 ` [RESEND] " Josh Boyer
@ 2008-03-24 11:39   ` Paul Mackerras
  2008-03-24 11:45     ` Josh Boyer
  2008-03-25  4:47     ` Sean MacLennan
  0 siblings, 2 replies; 21+ messages in thread
From: Paul Mackerras @ 2008-03-24 11:39 UTC (permalink / raw)
  To: Josh Boyer; +Cc: sfr, linuxppc-dev

Josh Boyer writes:

> 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.

Well actually...

> +	if (statlen > 0) {
> +		if (!strncmp(status, "okay", 4) || !strncmp(status, "ok", 2))
> +			return 1;

The second test will succeed for anything that starts with "ok", so
the first test is redundant.  I suspect you want strcmp instead of
strncmp in both tests.

Paul.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RESEND] [PATCH 1/2 v2] [OF] Add of_device_is_available function
  2008-03-24 11:39   ` Paul Mackerras
@ 2008-03-24 11:45     ` Josh Boyer
  2008-03-25  4:47     ` Sean MacLennan
  1 sibling, 0 replies; 21+ messages in thread
From: Josh Boyer @ 2008-03-24 11:45 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: sfr, linuxppc-dev

On Mon, 24 Mar 2008 22:39:18 +1100
Paul Mackerras <paulus@samba.org> wrote:

> Josh Boyer writes:
> 
> > 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.
> 
> Well actually...
> 
> > +	if (statlen > 0) {
> > +		if (!strncmp(status, "okay", 4) || !strncmp(status, "ok", 2))
> > +			return 1;
> 
> The second test will succeed for anything that starts with "ok", so
> the first test is redundant.  I suspect you want strcmp instead of
> strncmp in both tests.

GRR!  That's what I had in the original patch and you told me to use
strncmp instead.

If you want I can send out a v3.  Otherwise, perhaps you could just
munge the patch?

josh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RESEND] [PATCH 1/2 v2] [OF] Add of_device_is_available function
  2008-03-24 11:39   ` Paul Mackerras
  2008-03-24 11:45     ` Josh Boyer
@ 2008-03-25  4:47     ` Sean MacLennan
  2008-03-25 11:51       ` Josh Boyer
  1 sibling, 1 reply; 21+ messages in thread
From: Sean MacLennan @ 2008-03-25  4:47 UTC (permalink / raw)
  To: linuxppc-dev

On Mon, 24 Mar 2008 22:39:18 +1100
"Paul Mackerras" <paulus@samba.org> wrote:

> The second test will succeed for anything that starts with "ok", so
> the first test is redundant.  I suspect you want strcmp instead of
> strncmp in both tests.

I like the strncmp(status, "ok", 2). Then you can have a status of
okey-dokey ;)

Cheers,
   Sean

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RESEND] [PATCH 1/2 v2] [OF] Add of_device_is_available function
  2008-03-25  4:47     ` Sean MacLennan
@ 2008-03-25 11:51       ` Josh Boyer
  2008-03-26 14:25         ` Segher Boessenkool
  0 siblings, 1 reply; 21+ messages in thread
From: Josh Boyer @ 2008-03-25 11:51 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev

On Tue, 25 Mar 2008 00:47:53 -0400
Sean MacLennan <smaclennan@pikatech.com> wrote:

> On Mon, 24 Mar 2008 22:39:18 +1100
> "Paul Mackerras" <paulus@samba.org> wrote:
> 
> > The second test will succeed for anything that starts with "ok", so
> > the first test is redundant.  I suspect you want strcmp instead of
> > strncmp in both tests.
> 
> I like the strncmp(status, "ok", 2). Then you can have a status of
> okey-dokey ;)

You can also have "oklahoma", "okaly-dokaly-do" (I hate Ned Flanders),
and "ok-this-device-is-fubar-don't-ever-touch-it!"

We really want just "okay" and "ok".  Look for a version 3 of the patch
that fixes it later today.

josh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RESEND] [PATCH 1/2 v2] [OF] Add of_device_is_available function
  2008-03-25 11:51       ` Josh Boyer
@ 2008-03-26 14:25         ` Segher Boessenkool
  0 siblings, 0 replies; 21+ messages in thread
From: Segher Boessenkool @ 2008-03-26 14:25 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, Sean MacLennan

> We really want just "okay" and "ok".  Look for a version 3 of the patch
> that fixes it later today.

You might want to put in a comment that says "ok" is only a
workaround for some broken systems, too.


Segher

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2008-03-26 14:29 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-01 14:16 [PATCH 1/2 v2] [OF] Add of_device_is_available function Josh Boyer
2008-03-01 14:17 ` [PATCH 2/2 v2] [POWERPC] Ignore disabled serial ports Josh Boyer
2008-03-03  3:43   ` Arnd Bergmann
2008-03-03  4:43     ` Josh Boyer
2008-03-03 19:09       ` Scott Wood
2008-03-03 19:21         ` Josh Boyer
2008-03-03 21:40           ` Nathan Lynch
2008-03-04  0:42             ` Arnd Bergmann
2008-03-01 14:41 ` [PATCH 3/2][NEWEMAC] Use status property for unused/unwired EMACs Josh Boyer
2008-03-01 21:33   ` Benjamin Herrenschmidt
2008-03-01 23:50     ` Josh Boyer
2008-03-02 19:43   ` Sean MacLennan
2008-03-02 22:23     ` Josh Boyer
2008-03-01 22:02 ` [PATCH 1/2 v2] [OF] Add of_device_is_available function Stephen Rothwell
2008-03-01 22:25   ` Josh Boyer
2008-03-01 23:48 ` [RESEND] " Josh Boyer
2008-03-24 11:39   ` Paul Mackerras
2008-03-24 11:45     ` Josh Boyer
2008-03-25  4:47     ` Sean MacLennan
2008-03-25 11:51       ` Josh Boyer
2008-03-26 14:25         ` Segher Boessenkool

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).