[patch 0/3] fsl_soc / mpc8349emitx patches

STALE6904d

18 messages, 5 authors, 2007-09-28 · open the first message on its own page

[patch 0/3] fsl_soc / mpc8349emitx patches

From: Peter Korsgaard <jacmet@sunsite.dk>
Date: 2007-09-20 10:43:16

Various small fsl_soc / mpc8349emitx patches.

-- 
Bye, Peter Korsgaard

[patch 2/3] fsl_soc: rtc-ds1307 support

From: Peter Korsgaard <jacmet@sunsite.dk>
Date: 2007-09-20 10:43:16

Add support for the I2C devices handled by the rtc-ds1307 driver to
of_register_i2c_devices.

Cc: G. Liakhovetski <redacted>
Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
 arch/powerpc/sysdev/fsl_soc.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux/arch/powerpc/sysdev/fsl_soc.c
===================================================================
--- linux.orig/arch/powerpc/sysdev/fsl_soc.c
+++ linux/arch/powerpc/sysdev/fsl_soc.c
@@ -319,6 +319,12 @@
 	{"ricoh,rs5c372b", "rtc-rs5c372", "rs5c372b",},
 	{"ricoh,rv5c386",  "rtc-rs5c372", "rv5c386",},
 	{"ricoh,rv5c387a", "rtc-rs5c372", "rv5c387a",},
+	{"dallas,ds1307",  "rtc-ds1307",  "ds1307",},
+	{"dallas,ds1337",  "rtc-ds1307",  "ds1337",},
+	{"dallas,ds1338",  "rtc-ds1307",  "ds1338",},
+	{"dallas,ds1339",  "rtc-ds1307",  "ds1339",},
+	{"dallas,ds1340",  "rtc-ds1307",  "ds1340",},
+	{"stm,m41t00",     "rtc-ds1307",  "m41t00"},
 };
 
 static int __init of_find_i2c_driver(struct device_node *node, struct i2c_board_info *info)
-- 
Bye, Peter Korsgaard

[patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: Peter Korsgaard <jacmet@sunsite.dk>
Date: 2007-09-20 10:43:16

Add ds1339 I2C RTC chip as child of 2nd I2C controller.

Cc: Timur Tabi <redacted>
Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
 arch/powerpc/boot/dts/mpc8349emitx.dts |    7 +++++++
 1 file changed, 7 insertions(+)

Index: linux/arch/powerpc/boot/dts/mpc8349emitx.dts
===================================================================
--- linux.orig/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ linux/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -68,6 +68,13 @@
 			interrupts = <f 8>;
 			interrupt-parent = < &ipic >;
 			dfsrr;
+
+			rtc@68 {
+				device_type = "rtc";
+				compatible = "dallas,ds1339";
+				reg = <68 4>;
+			};
+
 		};
 
 		spi@7000 {
-- 
Bye, Peter Korsgaard

[patch 1/3] fsl_soc: Fix trivial printk typo.

From: Peter Korsgaard <jacmet@sunsite.dk>
Date: 2007-09-20 10:43:16

Fix a trivial printk typo in fsl_soc.

Cc: G. Liakhovetski <redacted>
Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
 arch/powerpc/sysdev/fsl_soc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/arch/powerpc/sysdev/fsl_soc.c
===================================================================
--- linux.orig/arch/powerpc/sysdev/fsl_soc.c
+++ linux/arch/powerpc/sysdev/fsl_soc.c
@@ -346,7 +346,7 @@
 
 		addr = of_get_property(node, "reg", &len);
 		if (!addr || len < sizeof(int) || *addr > (1 << 10) - 1) {
-			printk(KERN_WARNING "fsl_ioc.c: invalid i2c device entry\n");
+			printk(KERN_WARNING "fsl_soc.c: invalid i2c device entry\n");
 			continue;
 		}
 
-- 
Bye, Peter Korsgaard

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: Scott Wood <hidden>
Date: 2007-09-20 13:35:38

On Thu, Sep 20, 2007 at 12:42:14PM +0200, Peter Korsgaard wrote:
quoted hunk
Index: linux/arch/powerpc/boot/dts/mpc8349emitx.dts
===================================================================
--- linux.orig/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ linux/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -68,6 +68,13 @@
 			interrupts = <f 8>;
 			interrupt-parent = < &ipic >;
 			dfsrr;
+
+			rtc@68 {
+				device_type = "rtc";
+				compatible = "dallas,ds1339";
+				reg = <68 4>;
+			};
#size-cells is zero on i2c, so it should just be reg = <68>.

You'll probably need to add #address-cells and #size-cells to the
controller node, as well.

-Scott

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: Peter Korsgaard <jacmet@sunsite.dk>
Date: 2007-09-21 07:35:15

quoted
quoted
quoted
quoted
"Scott" == Scott Wood [off-list ref] writes:
Hi,

 Scott> #size-cells is zero on i2c, so it should just be reg = <68>.

 Scott> You'll probably need to add #address-cells and #size-cells to the
 Scott> controller node, as well.

Ahh - Thanks. This should be better.
---

[PATCH] mpc8349emitx.dts: Add ds1339 RTC

Add ds1339 I2C RTC chip as child of 2nd I2C controller.

Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
 arch/powerpc/boot/dts/mpc8349emitx.dts |    9 +++++++++
 1 file changed, 9 insertions(+)

Index: linux/arch/powerpc/boot/dts/mpc8349emitx.dts
===================================================================
--- linux.orig/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ linux/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -62,12 +62,21 @@
 		};
 
 		i2c@3100 {
+			#address-cells = <1>;
+			#size-cells = <0>;
 			device_type = "i2c";
 			compatible = "fsl-i2c";
 			reg = <3100 100>;
 			interrupts = <f 8>;
 			interrupt-parent = < &ipic >;
 			dfsrr;
+
+			rtc@68 {
+				device_type = "rtc";
+				compatible = "dallas,ds1339";
+				reg = <68>;
+			};
+
 		};
 
 		spi@7000 {
-- 
Bye, Peter Korsgaard

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: David Gibson <hidden>
Date: 2007-09-24 05:07:09

On Fri, Sep 21, 2007 at 09:35:03AM +0200, Peter Korsgaard wrote:
quoted
quoted
quoted
quoted
quoted
"Scott" == Scott Wood [off-list ref] writes:
Hi,

 Scott> #size-cells is zero on i2c, so it should just be reg = <68>.

 Scott> You'll probably need to add #address-cells and #size-cells to the
 Scott> controller node, as well.
Uh.. yes.. i2c interfaces should really always have #a and #s.
quoted hunk
Ahh - Thanks. This should be better.
---

[PATCH] mpc8349emitx.dts: Add ds1339 RTC

Add ds1339 I2C RTC chip as child of 2nd I2C controller.

Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
 arch/powerpc/boot/dts/mpc8349emitx.dts |    9 +++++++++
 1 file changed, 9 insertions(+)

Index: linux/arch/powerpc/boot/dts/mpc8349emitx.dts
===================================================================
--- linux.orig/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ linux/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -62,12 +62,21 @@
 		};
 
 		i2c@3100 {
+			#address-cells = <1>;
+			#size-cells = <0>;
 			device_type = "i2c";
Hrm... we probably want an "i2c" device_type class, but I don't think
we've actually defined one, which is a problem
 			compatible = "fsl-i2c";
 			reg = <3100 100>;
 			interrupts = <f 8>;
 			interrupt-parent = < &ipic >;
 			dfsrr;
+
+			rtc@68 {
+				device_type = "rtc";
+				compatible = "dallas,ds1339";
+				reg = <68>;
+			};
I think we want to think a bit more carefully about how to do bindings
for RTC devices.  No "rtc" device_type is defined, but again we might
want to.

I did find one real OF binding for a different Dallas RTC (and NVRAM),
see:

http://playground.sun.com/1275/proposals/Closed/Remanded/Accepted/346-it.txt

It's a little different from the example above.

The fact that NVRAM+RTC chips are so common is a bit of an issue from
the point of view of defining a device class binding - a device can't
have type "rtc" and "nvram".
+
 		};
 
 		spi@7000 {
-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: Peter Korsgaard <jacmet@sunsite.dk>
Date: 2007-09-24 05:52:29

quoted
quoted
quoted
quoted
"David" == David Gibson [off-list ref] writes:
Hi

 >> compatible = "fsl-i2c";
 >> reg = <3100 100>;
 >> interrupts = <f 8>;
 >> interrupt-parent = < &ipic >;
 >> dfsrr;
 >> +
 >> +			rtc@68 {
 >> +				device_type = "rtc";
 >> +				compatible = "dallas,ds1339";
 >> +				reg = <68>;
 >> +			};

 David> I think we want to think a bit more carefully about how to do bindings
 David> for RTC devices.  No "rtc" device_type is defined, but again we might
 David> want to.

Could be. I've simply done it like kuroboxHD.dts already does and
fsl_soc.c expects.

 David> I did find one real OF binding for a different Dallas RTC (and NVRAM),
 David> see:

 David> http://playground.sun.com/1275/proposals/Closed/Remanded/Accepted/346-it.txt

 David> It's a little different from the example above.

 David> The fact that NVRAM+RTC chips are so common is a bit of an issue from
 David> the point of view of defining a device class binding - a device can't
 David> have type "rtc" and "nvram".

True. I think we should primarily focus on the RTC part rather than
NVRAM as that's the "main" functionality and leave a NVRAM class for
I2C EEPROMs.

The Linux driver for the chip (rtc-1307.c) doesn't expose the NVRAM
bytes either.

But I'm open for suggestions.

-- 
Bye, Peter Korsgaard

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: Kumar Gala <hidden>
Date: 2007-09-24 06:13:05

On Sep 24, 2007, at 12:07 AM, David Gibson wrote:
On Fri, Sep 21, 2007 at 09:35:03AM +0200, Peter Korsgaard wrote:
quoted
quoted
quoted
quoted
quoted
quoted
"Scott" == Scott Wood [off-list ref] writes:
Hi,

 Scott> #size-cells is zero on i2c, so it should just be reg = <68>.

 Scott> You'll probably need to add #address-cells and #size-cells  
to the
 Scott> controller node, as well.
Uh.. yes.. i2c interfaces should really always have #a and #s.
quoted
Ahh - Thanks. This should be better.
---

[PATCH] mpc8349emitx.dts: Add ds1339 RTC

Add ds1339 I2C RTC chip as child of 2nd I2C controller.

Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
 arch/powerpc/boot/dts/mpc8349emitx.dts |    9 +++++++++
 1 file changed, 9 insertions(+)

Index: linux/arch/powerpc/boot/dts/mpc8349emitx.dts
===================================================================
--- linux.orig/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ linux/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -62,12 +62,21 @@
 		};

 		i2c@3100 {
+			#address-cells = <1>;
+			#size-cells = <0>;
 			device_type = "i2c";
Hrm... we probably want an "i2c" device_type class, but I don't think
we've actually defined one, which is a prob
suggestions on how to handle this?  We've described that fsl-i2c  
nodes in booting-without-of.txt should have it. (which may have been  
wrong at the time).

- k
quoted
 			compatible = "fsl-i2c";
 			reg = <3100 100>;
 			interrupts = <f 8>;
 			interrupt-parent = < &ipic >;
 			dfsrr;

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: Scott Wood <hidden>
Date: 2007-09-24 14:53:00

David Gibson wrote:
quoted
 		i2c@3100 {
+			#address-cells = <1>;
+			#size-cells = <0>;
 			device_type = "i2c";
Hrm... we probably want an "i2c" device_type class, but I don't think
we've actually defined one, which is a problem
Right... but we need to get the kernel to stop expecting the device type 
to be there before we yell at people for including it. :-)
The fact that NVRAM+RTC chips are so common is a bit of an issue from
the point of view of defining a device class binding - a device can't
have type "rtc" and "nvram".
This is one of the reasons that I'd prefer to use compatible for such 
things.

-Scott

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: Segher Boessenkool <hidden>
Date: 2007-09-24 21:11:28

quoted
 Scott> #size-cells is zero on i2c, so it should just be reg = <68>.

 Scott> You'll probably need to add #address-cells and #size-cells to 
the
 Scott> controller node, as well.
Uh.. yes.. i2c interfaces should really always have #a and #s.
More generally, every node that defines a bus needs it (unless the
defaults of 2 resp. 1 are correct for this bus, but even then you
might want it because it makes things more explicit).
quoted
 		i2c@3100 {
+			#address-cells = <1>;
+			#size-cells = <0>;
 			device_type = "i2c";
Hrm... we probably want an "i2c" device_type class, but I don't think
we've actually defined one, which is a problem
By defining new device_type's, or new semantics for device_type,
you open the door to (accidentally) becoming incompatible with
"real" OF.

And you don't need to: "real" OF has a mechanism for specifying
the "generic device class" already, if you use the "generic names"
recommended practice (and you do, for both this node and the rtc
node): it's the generic name itself!
quoted
+			rtc@68 {
+				device_type = "rtc";
+				compatible = "dallas,ds1339";
+				reg = <68>;
+			};
I think we want to think a bit more carefully about how to do bindings
for RTC devices.  No "rtc" device_type is defined, but again we might
want to.
Actually, "device_type" = "rtc" _is_ defined (in the "device support
extensions" recommended practice), and there is no useful way a flat
device tree can implement it (it merely defines get-time and set-time
methods).
The fact that NVRAM+RTC chips are so common is a bit of an issue from
the point of view of defining a device class binding - a device can't
have type "rtc" and "nvram".
You should match OS drivers on "compatible" only anyway.

Those cases where OS drivers don't nicely 1-1 match device nodes are a
bit of a headache; for RTC/NVRAM devices, these problems are nicely
side-stepped by handling this from platform code.


Segher

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: David Gibson <hidden>
Date: 2007-09-25 02:04:59

On Mon, Sep 24, 2007 at 09:52:31AM -0500, Scott Wood wrote:
David Gibson wrote:
quoted
quoted
 		i2c@3100 {
+			#address-cells = <1>;
+			#size-cells = <0>;
 			device_type = "i2c";
Hrm... we probably want an "i2c" device_type class, but I don't think
we've actually defined one, which is a problem
Right... but we need to get the kernel to stop expecting the device type 
to be there before we yell at people for including it. :-)
Obviously.  We should make sure all the corresponding compatibles are
specific enough, change the drivers, then think about getting rid of
it.
quoted
The fact that NVRAM+RTC chips are so common is a bit of an issue from
the point of view of defining a device class binding - a device can't
have type "rtc" and "nvram".
This is one of the reasons that I'd prefer to use compatible for such 
things.
Yeah, fair enough.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: David Gibson <hidden>
Date: 2007-09-25 02:11:44

On Mon, Sep 24, 2007 at 11:11:28PM +0200, Segher Boessenkool wrote:
quoted
quoted
 Scott> #size-cells is zero on i2c, so it should just be reg = <68>.

 Scott> You'll probably need to add #address-cells and #size-cells to 
the
 Scott> controller node, as well.
Uh.. yes.. i2c interfaces should really always have #a and #s.
More generally, every node that defines a bus needs it (unless the
defaults of 2 resp. 1 are correct for this bus, but even then you
might want it because it makes things more explicit).
Yes.  Actually I think we should make explicit #a and #s at least
strongly recommended in the flat tree documentation.
quoted
quoted
 		i2c@3100 {
+			#address-cells = <1>;
+			#size-cells = <0>;
 			device_type = "i2c";
Hrm... we probably want an "i2c" device_type class, but I don't think
we've actually defined one, which is a problem
By defining new device_type's, or new semantics for device_type,
you open the door to (accidentally) becoming incompatible with
"real" OF.
Hrm... perhaps.  But is it a realistic danger - I'll have to think
more about that.
And you don't need to: "real" OF has a mechanism for specifying
the "generic device class" already, if you use the "generic names"
recommended practice (and you do, for both this node and the rtc
node): it's the generic name itself!
Hmm, I guess.
quoted
quoted
+			rtc@68 {
+				device_type = "rtc";
+				compatible = "dallas,ds1339";
+				reg = <68>;
+			};
I think we want to think a bit more carefully about how to do bindings
for RTC devices.  No "rtc" device_type is defined, but again we might
want to.
Actually, "device_type" = "rtc" _is_ defined (in the "device support
extensions" recommended practice), and there is no useful way a flat
device tree can implement it (it merely defines get-time and set-time
methods).
Ah.. right.  That changes things a bit, in that we might want to
include device_type purely for similarity with real OF tree.

Real OF has a device_type == "nvram" too, doesn't it?  AFAICT the real
OF systems I have (which I think all have ISA-like CMOS RTC/NVRAM
chips) the RTC is labelled as "nvram" rather than "rtc".
quoted
The fact that NVRAM+RTC chips are so common is a bit of an issue from
the point of view of defining a device class binding - a device can't
have type "rtc" and "nvram".
You should match OS drivers on "compatible" only anyway.
Absolutely.  I was only thinking of defining "device classes" where
for some reason it is useful to examine them without needing to pick a
particular driver.
Those cases where OS drivers don't nicely 1-1 match device nodes are a
bit of a headache; for RTC/NVRAM devices, these problems are nicely
side-stepped by handling this from platform code.
Not necessarily.  The new RTC class drivers are just drivers like
anything other and are not especially instantiated from the platform
code.


And drat.  I was only really mentioning stuff about device_type in
passing, but it's the only thing anyone's responded to.  I was also
mostly suggesting changing the format of compatible, for greater
similarity with the existing ds1385 binding.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: David Gibson <hidden>
Date: 2007-09-25 02:13:40

On Mon, Sep 24, 2007 at 07:52:22AM +0200, Peter Korsgaard wrote:
quoted
quoted
quoted
quoted
quoted
"David" == David Gibson [off-list ref] writes:
Hi

 >> compatible = "fsl-i2c";
 >> reg = <3100 100>;
 >> interrupts = <f 8>;
 >> interrupt-parent = < &ipic >;
 >> dfsrr;
 >> +
 >> +			rtc@68 {
 >> +				device_type = "rtc";
 >> +				compatible = "dallas,ds1339";
 >> +				reg = <68>;
 >> +			};

 David> I think we want to think a bit more carefully about how to do bindings
 David> for RTC devices.  No "rtc" device_type is defined, but again we might
 David> want to.

Could be. I've simply done it like kuroboxHD.dts already does and
fsl_soc.c expects.

 David> I did find one real OF binding for a different Dallas RTC (and NVRAM),
 David> see:

 David> http://playground.sun.com/1275/proposals/Closed/Remanded/Accepted/346-it.txt

 David> It's a little different from the example above.

 David> The fact that NVRAM+RTC chips are so common is a bit of an issue from
 David> the point of view of defining a device class binding - a device can't
 David> have type "rtc" and "nvram".

True. I think we should primarily focus on the RTC part rather than
NVRAM as that's the "main" functionality and leave a NVRAM class for
I2C EEPROMs.

The Linux driver for the chip (rtc-1307.c) doesn't expose the NVRAM
bytes either.
Incidentally how are you planning on instantiating the driver?  AFAIK
all the rtc-* drivers are platform drivers rather than of_platform
drivers.  I had been thinking of an rtc helper function that would go
through the tree instantiating platform devices for any RTCs based on
a compatible -> platform device name table.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: Peter Korsgaard <jacmet@sunsite.dk>
Date: 2007-09-25 05:33:23

quoted
quoted
quoted
quoted
"David" == David Gibson [off-list ref] writes:
Hi,

 >> The Linux driver for the chip (rtc-1307.c) doesn't expose the NVRAM
 >> bytes either.

 David> Incidentally how are you planning on instantiating the driver?  AFAIK
 David> all the rtc-* drivers are platform drivers rather than of_platform
 David> drivers.  I had been thinking of an rtc helper function that would go
 David> through the tree instantiating platform devices for any RTCs based on
 David> a compatible -> platform device name table.

Please see patch #2 in the series:

http://ozlabs.org/pipermail/linuxppc-dev/2007-September/042896.html

That helper function more or less already exists in fsl_soc.c.

-- 
Bye, Peter Korsgaard

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: David Gibson <hidden>
Date: 2007-09-25 05:47:16

On Tue, Sep 25, 2007 at 07:33:15AM +0200, Peter Korsgaard wrote:
quoted
quoted
quoted
quoted
quoted
"David" == David Gibson [off-list ref] writes:
Hi,

 >> The Linux driver for the chip (rtc-1307.c) doesn't expose the NVRAM
 >> bytes either.

 David> Incidentally how are you planning on instantiating the driver?  AFAIK
 David> all the rtc-* drivers are platform drivers rather than of_platform
 David> drivers.  I had been thinking of an rtc helper function that would go
 David> through the tree instantiating platform devices for any RTCs based on
 David> a compatible -> platform device name table.

Please see patch #2 in the series:

http://ozlabs.org/pipermail/linuxppc-dev/2007-September/042896.html

That helper function more or less already exists in fsl_soc.c.
Ah, I see.  Well... it exists for i2c devices (possibly including
RTCs).  Whereas I was thinking of a version for RTCs (possibly
including i2c devices).  Actually that won't quite work - looks like
the i2c RTC class drivers are probed differently from the RTC drivers
I was looking at which are pure platform devices.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: Segher Boessenkool <hidden>
Date: 2007-09-25 20:33:58

quoted
quoted
Hrm... we probably want an "i2c" device_type class, but I don't think
we've actually defined one, which is a problem
By defining new device_type's, or new semantics for device_type,
you open the door to (accidentally) becoming incompatible with
"real" OF.
Hrm... perhaps.  But is it a realistic danger - I'll have to think
more about that.
It is trivial to avoid these dangers completely by not overloading
the meaning of "device_type".
quoted
quoted
I think we want to think a bit more carefully about how to do  
bindings
for RTC devices.  No "rtc" device_type is defined, but again we might
want to.
Actually, "device_type" = "rtc" _is_ defined (in the "device support
extensions" recommended practice), and there is no useful way a flat
device tree can implement it (it merely defines get-time and set-time
methods).
Ah.. right.  That changes things a bit, in that we might want to
include device_type purely for similarity with real OF tree.
You should include the device_type only if you implement its binding,
and a flat device tree does not, and cannot.  (In almost all cases,
a flat device tree cannot implement device_type's semantics -- this
means that pretty much the only case where a flat tree should use
device_type is to have it as a workaround for bad kernel requirements).
Real OF has a device_type == "nvram" too, doesn't it?
Yes, same "device support extensions" document.
AFAICT the real
OF systems I have (which I think all have ISA-like CMOS RTC/NVRAM
chips) the RTC is labelled as "nvram" rather than "rtc".
Sounds buggy.
quoted
quoted
The fact that NVRAM+RTC chips are so common is a bit of an issue from
the point of view of defining a device class binding - a device can't
have type "rtc" and "nvram".
You should match OS drivers on "compatible" only anyway.
Absolutely.  I was only thinking of defining "device classes" where
for some reason it is useful to examine them without needing to pick a
particular driver.
Yeah I understand.  In what situations would this be useful?
Answering that question will make the requirements for this more
clear; or maybe it will show we do not need this at all.
quoted
Those cases where OS drivers don't nicely 1-1 match device nodes are a
bit of a headache; for RTC/NVRAM devices, these problems are nicely
side-stepped by handling this from platform code.
Not necessarily.  The new RTC class drivers are just drivers like
anything other and are not especially instantiated from the platform
code.
I meant "can be nicely side-stepped", or "usually are ..." :-)

Obviously, when you cannot avoid the problem, you have a problem.
And drat.  I was only really mentioning stuff about device_type in
passing, but it's the only thing anyone's responded to.  I was also
mostly suggesting changing the format of compatible, for greater
similarity with the existing ds1385 binding.
Okay, quoting from your earlier message:
I did find one real OF binding for a different Dallas RTC (and NVRAM),
see:

http://playground.sun.com/1275/proposals/Closed/Remanded/Accepted/346- 
it.txt

It's a little different from the example above.
That is a binding for the nvram part only, not for the RTC.


Segher

Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC

From: David Gibson <hidden>
Date: 2007-09-28 02:45:53

On Tue, Sep 25, 2007 at 10:33:58PM +0200, Segher Boessenkool wrote:
quoted
quoted
quoted
Hrm... we probably want an "i2c" device_type class, but I don't think
we've actually defined one, which is a problem
By defining new device_type's, or new semantics for device_type,
you open the door to (accidentally) becoming incompatible with
"real" OF.
Hrm... perhaps.  But is it a realistic danger - I'll have to think
more about that.
It is trivial to avoid these dangers completely by not overloading
the meaning of "device_type".
Hrm.  Perhaps.
quoted
quoted
quoted
I think we want to think a bit more carefully about how to do  
bindings
for RTC devices.  No "rtc" device_type is defined, but again we might
want to.
Actually, "device_type" = "rtc" _is_ defined (in the "device support
extensions" recommended practice), and there is no useful way a flat
device tree can implement it (it merely defines get-time and set-time
methods).
Ah.. right.  That changes things a bit, in that we might want to
include device_type purely for similarity with real OF tree.
You should include the device_type only if you implement its binding,
and a flat device tree does not, and cannot.  (In almost all cases,
a flat device tree cannot implement device_type's semantics -- this
means that pretty much the only case where a flat tree should use
device_type is to have it as a workaround for bad kernel requirements).
I really don't think there's an ambiguity here.  A flat-tree can
clearly never implement runtime binding features.  This is also true
for a flat tree derived from a real OF, and so full of device_type all
over the place.
quoted
Real OF has a device_type == "nvram" too, doesn't it?
Yes, same "device support extensions" document.
Erm.. I've lost track amongst our various threads.  Which same
document?
quoted
AFAICT the real
OF systems I have (which I think all have ISA-like CMOS RTC/NVRAM
chips) the RTC is labelled as "nvram" rather than "rtc".
Sounds buggy.
Why?

[snip]
quoted
I did find one real OF binding for a different Dallas RTC (and NVRAM),
see:

http://playground.sun.com/1275/proposals/Closed/Remanded/Accepted/346- 
it.txt

It's a little different from the example above.
That is a binding for the nvram part only, not for the RTC.
Hrm.  So how do you suggest we do bindings for combined devices?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help