[PATCH 3/5] powerpc: Document device nodes for I2C devices.

[PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Scott Wood]

Document the use of device trees to describe devices on an I2C bus, which
will be used with David Brownell's "new style" I2C binding patches.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 Documentation/powerpc/booting-without-of.txt |   34 ++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index b49ce16..67026ad 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -1257,6 +1257,8 @@ platforms are moved over to use the flattened-device-tree model.
 
    e) I2C
 
+   e1) I2C Controller
+
    Required properties :
 
     - device_type : Should be "i2c"
@@ -1277,6 +1279,10 @@ platforms are moved over to use the flattened-device-tree model.
       a digital filter sampling rate register
     - fsl5200-clocking : boolean; if defined, indicated that this device
       uses the FSL 5200 clocking mechanism.
+    - #address-cells : should exist and be 1 if I2C devices are declared
+      in the device tree.
+    - #size-cells : should exist and be 0 if I2C devices are declared
+      in the device tree.
 
    Example :
 
@@ -1289,6 +1295,34 @@ platforms are moved over to use the flattened-device-tree model.
 		dfsrr;
 	};
 
+   e2) I2C Devices
+
+   Required properties :
+
+    - reg : Unshifted 7-bit I2C address for the device
+
+   Recommended properties :
+
+    - compatible : The name of the Linux device driver that
+      handles this device.  If unspecified, the name of the
+      node will be used.
+    - interrupts : <a b> where a is the interrupt number and b is a
+      field that represents an encoding of the sense and level
+      information for the interrupt.  This should be encoded based on
+      the information in section 2) depending on the type of interrupt
+      controller you have.
+    - interrupt-parent : the phandle for the interrupt controller that
+      services interrupts for this device.
+
+   Example :
+
+	rtc@68 {
+		device_type = "rtc";
+		compatible = "ds1374";
+		reg = <68>;
+		interrupts = <13 8>;
+		interrupt-parent = <700>;
+	};
 
    f) Freescale SOC USB controllers
 
-- 
1.5.0.3

Re: [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Kumar Gala]

On May 17, 2007, at 9:38 AM, Scott Wood wrote:

> Document the use of device trees to describe devices on an I2C bus,  
> which
> will be used with David Brownell's "new style" I2C binding patches.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
>  Documentation/powerpc/booting-without-of.txt |   34 +++++++++++++++ 
> +++++++++++
>  1 files changed, 34 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/powerpc/booting-without-of.txt b/ 
> Documentation/powerpc/booting-without-of.txt
> index b49ce16..67026ad 100644
> --- a/Documentation/powerpc/booting-without-of.txt
> +++ b/Documentation/powerpc/booting-without-of.txt
> @@ -1257,6 +1257,8 @@ platforms are moved over to use the flattened- 
> device-tree model.
>
>     e) I2C
>
> +   e1) I2C Controller
> +
>     Required properties :
>
>      - device_type : Should be "i2c"
> @@ -1277,6 +1279,10 @@ platforms are moved over to use the  
> flattened-device-tree model.
>        a digital filter sampling rate register
>      - fsl5200-clocking : boolean; if defined, indicated that this  
> device
>        uses the FSL 5200 clocking mechanism.
> +    - #address-cells : should exist and be 1 if I2C devices are  
> declared
> +      in the device tree.
> +    - #size-cells : should exist and be 0 if I2C devices are declared
> +      in the device tree.

As I've stated before, we need a bus number as well so we can handle  
things like I2C switches and muxes.

- k

Re: [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Scott Wood]

Kumar Gala wrote:
>> diff --git a/Documentation/powerpc/booting-without-of.txt b/ 
>> Documentation/powerpc/booting-without-of.txt
>> index b49ce16..67026ad 100644
>> --- a/Documentation/powerpc/booting-without-of.txt
>> +++ b/Documentation/powerpc/booting-without-of.txt
>> @@ -1257,6 +1257,8 @@ platforms are moved over to use the flattened- 
>> device-tree model.
>>
>>     e) I2C
>>
>> +   e1) I2C Controller
>> +
>>     Required properties :
>>
>>      - device_type : Should be "i2c"
>> @@ -1277,6 +1279,10 @@ platforms are moved over to use the  
>> flattened-device-tree model.
>>        a digital filter sampling rate register
>>      - fsl5200-clocking : boolean; if defined, indicated that this  
>> device
>>        uses the FSL 5200 clocking mechanism.
>> +    - #address-cells : should exist and be 1 if I2C devices are  
>> declared
>> +      in the device tree.
>> +    - #size-cells : should exist and be 0 if I2C devices are declared
>> +      in the device tree.
> 
> 
> As I've stated before, we need a bus number as well so we can handle  
> things like I2C switches and muxes.

Is this something we handle now?  If not, then it's really not within 
the scope of this patchset.  If so, how am I breaking it?

-Scott

Re: [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Kumar Gala]

On May 17, 2007, at 11:17 AM, Scott Wood wrote:

> Kumar Gala wrote:
>>> diff --git a/Documentation/powerpc/booting-without-of.txt b/  
>>> Documentation/powerpc/booting-without-of.txt
>>> index b49ce16..67026ad 100644
>>> --- a/Documentation/powerpc/booting-without-of.txt
>>> +++ b/Documentation/powerpc/booting-without-of.txt
>>> @@ -1257,6 +1257,8 @@ platforms are moved over to use the  
>>> flattened- device-tree model.
>>>
>>>     e) I2C
>>>
>>> +   e1) I2C Controller
>>> +
>>>     Required properties :
>>>
>>>      - device_type : Should be "i2c"
>>> @@ -1277,6 +1279,10 @@ platforms are moved over to use the   
>>> flattened-device-tree model.
>>>        a digital filter sampling rate register
>>>      - fsl5200-clocking : boolean; if defined, indicated that  
>>> this  device
>>>        uses the FSL 5200 clocking mechanism.
>>> +    - #address-cells : should exist and be 1 if I2C devices are   
>>> declared
>>> +      in the device tree.
>>> +    - #size-cells : should exist and be 0 if I2C devices are  
>>> declared
>>> +      in the device tree.
>> As I've stated before, we need a bus number as well so we can  
>> handle  things like I2C switches and muxes.
>
> Is this something we handle now?  If not, then it's really not  
> within the scope of this patchset.  If so, how am I breaking it?

We don't handle i2c devices in the dev tree today.  If you are going  
to propose a solution it should work for all cases that people are  
aware of even if linux doesn't support the functionality.

If only some subset of cases are handled what good is the device tree  
to a user?  They will just have to figure out if their usage is  
supported or not and if not find some other solution that works for  
them.

- k

Re: [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Scott Wood]

Kumar Gala wrote:
> On May 17, 2007, at 11:17 AM, Scott Wood wrote:
>> Kumar Gala wrote:
>>> As I've stated before, we need a bus number as well so we can  
>>> handle  things like I2C switches and muxes.
>>
>> Is this something we handle now?  If not, then it's really not  within 
>> the scope of this patchset.  If so, how am I breaking it?
> 
> We don't handle i2c devices in the dev tree today.  If you are going  to 
> propose a solution it should work for all cases that people are  aware 
> of even if linux doesn't support the functionality.

But we do handle i2c *controllers* in the device tree, and that's where 
a bus number property would go.  Given that we don't have a binding for 
non-toplevel i2c buses, and I'm not adding one, I don't see the 
relevance.  Note that adding a bus number property makes zero sense for 
toplevel buses, as at that level the bus number is just a fiction 
maintained by Linux for user API and device preregistration purposes.

It's not a matter of the binding only covering some cases; it's a matter 
of the binding being for one thing (i2c devices) and not another 
(multiplexed i2c buses).

> If only some subset of cases are handled what good is the device tree  
> to a user?  They will just have to figure out if their usage is  
> supported or not and if not find some other solution that works for  them.

...just as they'll have to figure out if a binding exists for device 
type $FOO.

-Scott

Re: [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Kumar Gala]

On May 17, 2007, at 11:47 AM, Scott Wood wrote:

> Kumar Gala wrote:
>> On May 17, 2007, at 11:17 AM, Scott Wood wrote:
>>> Kumar Gala wrote:
>>>> As I've stated before, we need a bus number as well so we can   
>>>> handle  things like I2C switches and muxes.
>>>
>>> Is this something we handle now?  If not, then it's really not   
>>> within the scope of this patchset.  If so, how am I breaking it?
>> We don't handle i2c devices in the dev tree today.  If you are  
>> going  to propose a solution it should work for all cases that  
>> people are  aware of even if linux doesn't support the functionality.
>
> But we do handle i2c *controllers* in the device tree, and that's  
> where a bus number property would go.  Given that we don't have a  
> binding for non-toplevel i2c buses, and I'm not adding one, I don't  
> see the relevance.  Note that adding a bus number property makes  
> zero sense for toplevel buses, as at that level the bus number is  
> just a fiction maintained by Linux for user API and device  
> preregistration purposes.

The only support we have for i2c controllers is to support one  
specific i2c controller from Freescale.

If you aren't going to provide a complete solution why are you  
prosing one?  I'm tired of this put stuff in the device tree but only  
as much as I need to do my particular thing.  The device tree is just  
as important an interface point as the kernel/user space interfaces  
and we should treat it as such.  If people aren't willing to work to  
a complete solution than they should stop proposing changes.

> It's not a matter of the binding only covering some cases; it's a  
> matter of the binding being for one thing (i2c devices) and not  
> another (multiplexed i2c buses).

But once you introduce the concept of i2c devices you introduce the  
possibility of hierarchies.  For example, tell me how I'd describe  
the following device http://www.nxp.com/pip/PCA9548ABS.html and any  
i2c devices connected to it?

>> If only some subset of cases are handled what good is the device  
>> tree  to a user?  They will just have to figure out if their usage  
>> is  supported or not and if not find some other solution that  
>> works for  them.
>
> ...just as they'll have to figure out if a binding exists for  
> device type $FOO.

True, but if I go look at the PCI OF spec I have some faith that its  
complete and will cover my needs.

As I've been thinking about this I think trying to even describe i2c  
devices in the device tree is point less.  Since I2C devices have no  
way of uniquely identifying themselves we are looking at taking on  
the role of device name registrar and if someone is willing to do  
that great, but I doubt anyone truly is.

- k

Re: [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Scott Wood]

Kumar Gala wrote:
> On May 17, 2007, at 11:47 AM, Scott Wood wrote:
>> But we do handle i2c *controllers* in the device tree, and that's  
>> where a bus number property would go.  Given that we don't have a  
>> binding for non-toplevel i2c buses, and I'm not adding one, I don't  
>> see the relevance.  Note that adding a bus number property makes  zero 
>> sense for toplevel buses, as at that level the bus number is  just a 
>> fiction maintained by Linux for user API and device  preregistration 
>> purposes.
> 
> The only support we have for i2c controllers is to support one  specific 
> i2c controller from Freescale.

That's not what booting-without-of.txt says.

> If you aren't going to provide a complete solution why are you  prosing 
> one?

Because if we can't do everything that anyone could ever need, we 
shouldn't do anything?  There's nothing in what I proposed that prevents 
i2c muxes; it just doesn't explicitly specify what extra things would 
need to be specified.

 >> I'm tired of this put stuff in the device tree but only  as much
> as I need to do my particular thing.

I'm tired of unconstructive whining that something that accomplishes 
something useful doesn't do everything you want it to.  If you want a 
device tree binding for i2c muxes, write one.  If you think it's 
pointless, then stop complaining about bindings that *are* useful.

 > The device tree is just  as
> important an interface point as the kernel/user space interfaces  and we 
> should treat it as such.

I agree.  And you will note that the entire set of kernel/user 
interfaces didn't spring into existence in one instant.  In both cases, 
adding is much easier than changing, and only additions would be needed 
to support i2c muxes.

> If people aren't willing to work to  a 
> complete solution than they should stop proposing changes.

By that token, if you're not willing to work toward any solution 
(complete or otherwise), you should stop proposing changes (or the 
absence thereof).  If there's a specific change that you would like to 
suggest that you believe would improve the binding, then please say what 
it is.

>> It's not a matter of the binding only covering some cases; it's a  
>> matter of the binding being for one thing (i2c devices) and not  
>> another (multiplexed i2c buses).
> 
> 
> But once you introduce the concept of i2c devices you introduce the  
> possibility of hierarchies.  For example, tell me how I'd describe  the 
> following device http://www.nxp.com/pip/PCA9548ABS.html and any  i2c 
> devices connected to it?

i2c-switch@70 {
	#address-cells = <1>;
	#size-cells = <0>;
	compatible = "pca9548a";
	reg = <70>;

	i2c@0 {
		#address-cells = <1>;
		#size-cells = <0>;
		reg = <0>;
		
		rtc@68 {
			device_type = "rtc";
			compatible = "ds1374";
			reg = <68>;
		};
	};

	i2c@1 {
		#address-cells = <1>;
		#size-cells = <0>;
		reg = <1>;
	
		// more devices here
	};

	// i2c@2-i2c@7 here
};

>>> If only some subset of cases are handled what good is the device  
>>> tree  to a user?  They will just have to figure out if their usage  
>>> is  supported or not and if not find some other solution that  works 
>>> for  them.
>>
>>
>> ...just as they'll have to figure out if a binding exists for  device 
>> type $FOO.
> 
> 
> True, but if I go look at the PCI OF spec I have some faith that its  
> complete and will cover my needs.

So what does the PCI OF spec have to say about devices on i2c 
controllers on PCI cards?

> As I've been thinking about this I think trying to even describe i2c  
> devices in the device tree is point less.

Well then device trees aren't a complete solution to the problem, so 
let's just ditch the concept entirely!  That's the way you want to do 
things, right?

 > Since I2C devices have no
> way of uniquely identifying themselves we are looking at taking on  the 
> role of device name registrar 

How's that different from any other random chip or logic block that gets 
  hooked up through something that isn't i2c?

> and if someone is willing to do  that 
> great, but I doubt anyone truly is.

Perhaps something could be done through Power.org?

-Scott

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Jean Delvare]

Hi Kumar,

On Thu, 17 May 2007 12:21:02 -0500, Kumar Gala wrote:
> The only support we have for i2c controllers is to support one  
> specific i2c controller from Freescale.
> 
> If you aren't going to provide a complete solution why are you  
> prosing one?  I'm tired of this put stuff in the device tree but only  
> as much as I need to do my particular thing.

This is exactly how free software development works. If people were
only proposing complete solutions, Linux would not even exist. Things
happen exactly because people write what they need and contribute what
they wrote. If you think it's not enough for your own needs (present or
future), then _you_ get to do the extra work.

Thanks,
-- 
Jean Delvare

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Kumar Gala]

On May 18, 2007, at 10:15 AM, Jean Delvare wrote:

> Hi Kumar,
>
> On Thu, 17 May 2007 12:21:02 -0500, Kumar Gala wrote:
>> The only support we have for i2c controllers is to support one
>> specific i2c controller from Freescale.
>>
>> If you aren't going to provide a complete solution why are you
>> prosing one?  I'm tired of this put stuff in the device tree but only
>> as much as I need to do my particular thing.
>
> This is exactly how free software development works. If people were
> only proposing complete solutions, Linux would not even exist. Things
> happen exactly because people write what they need and contribute what
> they wrote. If you think it's not enough for your own needs  
> (present or
> future), then _you_ get to do the extra work.

I guess my gripe is about proposing a solution and not willing to  
extend it in light of people providing issues with it.  Last time I  
check we don't put things into the kernel w/o any review and if  
people have issues that are reasonable they get hashed out.  It seems  
that the onus is on the initial submitter to either show that what  
they are providing is sufficient and w/o issue or incorporate the  
feedback.

More specifically, we have a way to specify what devices are connect  
on I2C today.  I'm not convinced there is any value in creating yet  
another mechanism, especially in an interface that in theory should  
be linux agnostic.

- k

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Scott Wood]

Kumar Gala wrote:
> I guess my gripe is about proposing a solution and not willing to  
> extend it in light of people providing issues with it. 

I'm perfectly willing to extend it if you let me know what you think is 
needed, rather than just saying "switches and muxes".  What 
*specifically* would they need beyond what I proposed?

> Last time I  
> check we don't put things into the kernel w/o any review and if  people 
> have issues that are reasonable they get hashed out.  It seems  that the 
> onus is on the initial submitter to either show that what  they are 
> providing is sufficient and w/o issue or incorporate the  feedback.

Give me something I can incorporate, then.  My gripe is when the 
feedback is "don't bother" based on unspecified problems with a 
configuration more complex than what it was intended to address (but 
still, AFAICT, not outside its ability to address).

> More specifically, we have a way to specify what devices are connect  on 
> I2C today.  I'm not convinced there is any value in creating yet  
> another mechanism, especially in an interface that in theory should  be 
> linux agnostic.

We had a way to specify platform devices before, too.  If the device 
tree isn't worthwhile for i2c devices, why is it worthwhile for soc 
devices?  It seems to me that non-probable chips like i2c devices are 
precisely the kind of thing that the device tree is useful for.

-Scott

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Kumar Gala]

On May 18, 2007, at 11:35 AM, Scott Wood wrote:

> Kumar Gala wrote:
>> I guess my gripe is about proposing a solution and not willing to   
>> extend it in light of people providing issues with it.
>
> I'm perfectly willing to extend it if you let me know what you  
> think is needed, rather than just saying "switches and muxes".   
> What *specifically* would they need beyond what I proposed?

I provided you an example device and asked you to explain how it  
would be described in what you are proposing.

>> Last time I  check we don't put things into the kernel w/o any  
>> review and if  people have issues that are reasonable they get  
>> hashed out.  It seems  that the onus is on the initial submitter  
>> to either show that what  they are providing is sufficient and w/o  
>> issue or incorporate the  feedback.
>
> Give me something I can incorporate, then.  My gripe is when the  
> feedback is "don't bother" based on unspecified problems with a  
> configuration more complex than what it was intended to address  
> (but still, AFAICT, not outside its ability to address).

I never said don't bother because you didn't cover the switch/mux  
case.  I said don't bother because I don't see what the value is  
creating a namespace that no one is going to manage and thus will end  
up most likely being linux specific, and linux already provides a  
solution for the problem.

>> More specifically, we have a way to specify what devices are  
>> connect  on I2C today.  I'm not convinced there is any value in  
>> creating yet  another mechanism, especially in an interface that  
>> in theory should  be linux agnostic.
>
> We had a way to specify platform devices before, too.  If the  
> device tree isn't worthwhile for i2c devices, why is it worthwhile  
> for soc devices?  It seems to me that non-probable chips like i2c  
> devices are precisely the kind of thing that the device tree is  
> useful for.

I dont believe anyone has ever said that platform devices have to be  
in the device tree.  We've been putting them their because we are  
going to act as the registry for the devices.  The number of devices  
on all the various Freescale/AMCC/IBM PPC SoCs is likely a very small  
number compared to all I2C devices.

For I2C specifically we already have both a dynamic way (kernel cmd  
line) and static (i2c_board_info) to specify the i2c devices, why do  
we need yet another?

- k

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Scott Wood]

Kumar Gala wrote:
> 
> On May 18, 2007, at 11:35 AM, Scott Wood wrote:
> 
>> Kumar Gala wrote:
>>
>>> I guess my gripe is about proposing a solution and not willing to   
>>> extend it in light of people providing issues with it.
>>
>>
>> I'm perfectly willing to extend it if you let me know what you  think 
>> is needed, rather than just saying "switches and muxes".   What 
>> *specifically* would they need beyond what I proposed?
> 
> 
> I provided you an example device and asked you to explain how it  would 
> be described in what you are proposing.

And I did.  What did you find lacking in the device tree fragment I 
suggested?

> I never said don't bother because you didn't cover the switch/mux  
> case.  I said don't bother because I don't see what the value is  
> creating a namespace that no one is going to manage and thus will end  
> up most likely being linux specific, and linux already provides a  
> solution for the problem.

Given that power.org is attempting to do further standardization of the 
device tree for embedded applications, I'd be surprised if there weren't 
a way we could have them act as a registry.

> For I2C specifically we already have both a dynamic way (kernel cmd  
> line) and static (i2c_board_info) to specify the i2c devices, why do  we 
> need yet another?

This uses i2c_board_info; it doesn't replace it.

-Scott

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Kumar Gala]

On May 18, 2007, at 12:17 PM, Scott Wood wrote:

> Kumar Gala wrote:
>> On May 18, 2007, at 11:35 AM, Scott Wood wrote:
>>> Kumar Gala wrote:
>>>
>>>> I guess my gripe is about proposing a solution and not willing  
>>>> to   extend it in light of people providing issues with it.
>>>
>>>
>>> I'm perfectly willing to extend it if you let me know what you   
>>> think is needed, rather than just saying "switches and muxes".    
>>> What *specifically* would they need beyond what I proposed?
>> I provided you an example device and asked you to explain how it   
>> would be described in what you are proposing.
>
> And I did.  What did you find lacking in the device tree fragment I  
> suggested?

Once you expand the beyond just a root node for the controller I'd  
like to see how you suggest we handle the case where a particular  
child ends up having children as well.  You example, is sufficient  
the majority of devices, but I'd like to know that we'll be able to  
handle the case where a node is both a device and controller.

>> I never said don't bother because you didn't cover the switch/mux   
>> case.  I said don't bother because I don't see what the value is   
>> creating a namespace that no one is going to manage and thus will  
>> end  up most likely being linux specific, and linux already  
>> provides a  solution for the problem.
>
> Given that power.org is attempting to do further standardization of  
> the device tree for embedded applications, I'd be surprised if  
> there weren't a way we could have them act as a registry.

If/when they sign up for this I'd be more inclined to have kernel  
support for it.

>> For I2C specifically we already have both a dynamic way (kernel  
>> cmd  line) and static (i2c_board_info) to specify the i2c devices,  
>> why do  we need yet another?
>
> This uses i2c_board_info; it doesn't replace it.

Ok, but what functionality does it give us that we dont already have  
today?

- k

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Scott Wood]

Kumar Gala wrote:
> Once you expand the beyond just a root node for the controller I'd  like 
> to see how you suggest we handle the case where a particular  child ends 
> up having children as well.  You example, is sufficient  the majority of 
> devices, but I'd like to know that we'll be able to  handle the case 
> where a node is both a device and controller.

The example I gave *was* a controller and an i2c device at the same time 
(note the i2c-style reg = <70>).  Just take the fragment and stick it in 
an existing i2c controller node; I omitted the latter for brevity, as a 
result of making the mistake of typing it in Mozilla rather than mutt, 
and thus having no autoindent.

>> Given that power.org is attempting to do further standardization of  
>> the device tree for embedded applications, I'd be surprised if  there 
>> weren't a way we could have them act as a registry.
> 
> If/when they sign up for this I'd be more inclined to have kernel  
> support for it.

Fair enough.  I'm still interested in what you think would need to be 
done to support switches and muxes, from the context of standardizing it 
in ePAPR.  The bus numbering shouldn't be an issue as long as you keep 
the bus numbers local to the switch/mux, and don't pretend that they 
have anything to do with any global i2c bus number that the OS may or 
may not have.

>>> For I2C specifically we already have both a dynamic way (kernel  cmd  
>>> line) and static (i2c_board_info) to specify the i2c devices,  why 
>>> do  we need yet another?
>>
>>
>> This uses i2c_board_info; it doesn't replace it.
> 
> 
> Ok, but what functionality does it give us that we dont already have  
> today?

The convenience of putting the data in a dts rather than in 
board-specific code/structs.  It's not a huge deal, but I find the 
former to be a more pleasant way of doing things, and others have 
similarly expressed interest in it.

It would be significantly more useful as an ePAPR standard that can be 
used by multiple OSes, but they seem to be in 
standardize-what-Linux-does mode, hence why I proposed it here first.

-Scott

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Jean Delvare]

Hi Scott,

On Fri, 18 May 2007 12:55:45 -0500, Scott Wood wrote:
> Fair enough.  I'm still interested in what you think would need to be 
> done to support switches and muxes, from the context of standardizing it 
> in ePAPR.  The bus numbering shouldn't be an issue as long as you keep 
> the bus numbers local to the switch/mux, and don't pretend that they 
> have anything to do with any global i2c bus number that the OS may or 
> may not have.

But then you cannot declare devices on these segments and expect Linux
to instantiate them. You'll have to wait for the segments to be created
and only then you'll be able to create the devices on them (using
i2c_new_device()).

Also, what's the point of giving numbers to the segments in the first
place, if they don't correspond to anything?

-- 
Jean Delvare

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Scott Wood]

Jean Delvare wrote:
> Hi Scott,
> 
> On Fri, 18 May 2007 12:55:45 -0500, Scott Wood wrote:
> 
>>Fair enough.  I'm still interested in what you think would need to be 
>>done to support switches and muxes, from the context of standardizing it 
>>in ePAPR.  The bus numbering shouldn't be an issue as long as you keep 
>>the bus numbers local to the switch/mux, and don't pretend that they 
>>have anything to do with any global i2c bus number that the OS may or 
>>may not have.
> 
> 
> But then you cannot declare devices on these segments and expect Linux
> to instantiate them. You'll have to wait for the segments to be created
> and only then you'll be able to create the devices on them (using
> i2c_new_device()).

You can declare devices by having platform code assign Linux bus numbers 
to them, just as with non-switched buses.  Of course, platform code 
would have to know about the switch to do that; it may be easier to just 
have the platform code register the switch driver and pass it a device 
tree node (or more generally, something opaque in platform data that 
gets passed back to platform code for registration) to do 
i2c_new_device()-based enumeration.

> Also, what's the point of giving numbers to the segments in the first
> place, if they don't correspond to anything?

The switch-local bus numbers are used to tell the switch which bus is 
being accessed.  The Linux bus numbers are used for device 
preregistration and user API.

They're both used, just for different purposes.

-Scott

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Matt Sealey]

Kumar Gala wrote:
> On May 18, 2007, at 11:35 AM, Scott Wood wrote:
> 
>> device tree isn't worthwhile for i2c devices, why is it worthwhile  
>> for soc devices?  It seems to me that non-probable chips like i2c  
>> devices are precisely the kind of thing that the device tree is  
>> useful for.
> 
> I dont believe anyone has ever said that platform devices have to be  
> in the device tree.  We've been putting them their because we are  
> going to act as the registry for the devices.  The number of devices  
> on all the various Freescale/AMCC/IBM PPC SoCs is likely a very small  
> number compared to all I2C devices.
> 
> For I2C specifically we already have both a dynamic way (kernel cmd  
> line) and static (i2c_board_info) to specify the i2c devices, why do  
> we need yet another?

Linux doesn't but it might be nice to specify this kind of thing in a
way that other operating systems might or may support.

Essentially I think since there are a lot of ways to support I2C
(including bitbanging a GPIO pair), the only real way to support it
is to do something like;

i2c@blah {
	name = "i2c"
	compatible = "mpc52xx-i2c,someother-i2c"
	regs = "address:range"
}

The PURPOSE of this node is not to describe how the i2c controller
works, but to advertise it's presence and AUTHORIZE it's use. If a
node is not in the device tree, a driver author should probably
think twice about using it, especially on chips where pin muxing
is the modus operandii.

If it's in the device tree, then the chances of it being a wildly
crazy type of device external to the board design is probably very
low. If you have an SPI controller on - for example - an MPC52xx
chip, there are only 2 or 3 ways to have it implemented. Either
the inbuilt SPI, a PSC-based SPI, or perhaps using a bunch of GPIOs
to mimic an SPI interface very closely. The same applies to i2c.

You're not ever going to be able to specify in the device tree
exactly how to handle a driver, encompassing both implementation,
bugs in revisions, quirks of board design, but you can specify
for a driver a very accurate, very educated guess on it (any
quirks, bugs or implementation differences would be board/chip
specific, and are easily derived from the other device nodes
like the cpu node, soc node, and so on)

To carry on from the previous paragraph, with that in mind, if
it is an external device (perhaps bridged through another
chip or bus) it will be a child of the external bus. This also
gives a big clue about it's operation. If it is USB, or PCI,
or i2c based device, it will be marked with vendor/device/subsystem
ids or even an i2c slave address. Also very, very big clues.

(the only way you can accurately do all of the above and take
out all of the guesswork, is provide the driver in the firmware.
U-Boot and FDT's, you can forget it!)

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Segher Boessenkool]

> Linux doesn't but it might be nice to specify this kind of thing in a
> way that other operating systems might or may support.
>
> Essentially I think since there are a lot of ways to support I2C
> (including bitbanging a GPIO pair), the only real way to support it
> is to do something like;
>
> i2c@blah {
> 	name = "i2c"
> 	compatible = "mpc52xx-i2c,someother-i2c"
> 	regs = "address:range"
> }

Yes, all devices should be in the device tree.  That's
what it's for.

> You're not ever going to be able to specify in the device tree
> exactly how to handle a driver, encompassing both implementation,
> bugs in revisions, quirks of board design, but you can specify
> for a driver a very accurate, very educated guess on it (any
> quirks, bugs or implementation differences would be board/chip
> specific, and are easily derived from the other device nodes
> like the cpu node, soc node, and so on)

Actually, you can, and should.  All this information is
contained in the "compatible" and "model" properties.
"Quirks of board design" can be described too, on a case-
by-case basis.

All the knowledge about how to drive the device resides
in the kernel, but the device tree describes exactly what
device this is, so the kernel can match a driver to it
uniquely, and the driver can know exactly what revision
chip this is and what quirks to apply.


Segher

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Matt Sealey]

Segher Boessenkool wrote:
> 
> Yes, all devices should be in the device tree.  That's
> what it's for.
> 
>> You're not ever going to be able to specify in the device tree
>> exactly how to handle a driver, encompassing both implementation,
>> bugs in revisions, quirks of board design, but you can specify
>> for a driver a very accurate, very educated guess on it (any
>> quirks, bugs or implementation differences would be board/chip
>> specific, and are easily derived from the other device nodes
>> like the cpu node, soc node, and so on)
> 
> Actually, you can, and should.  All this information is
> contained in the "compatible" and "model" properties.
> "Quirks of board design" can be described too, on a case-
> by-case basis.
>
> All the knowledge about how to drive the device resides
> in the kernel, but the device tree describes exactly what
> device this is, so the kernel can match a driver to it
> uniquely, and the driver can know exactly what revision
> chip this is and what quirks to apply.

That's what I said wasn't it?

If you have a buggy i2c controller or one that has a strange
quirk, but it's present as fsl-i2c in those device trees,
would you specify that it is fsl-i2c-less-bugs later?
Would you add property after property to describe errata,
quirks in the nodes themselves?

I don't see the point in that, when such information is
nearly always derivable from other parts of the device
tree. An ATA node does not contain the timings for devices
on that bus. The driver usually has to work that out. If
there is a quirk to be implemented, this will be described
by the PVR/SVR and other system information already in the
device tree. The clock distribution module in the MPC52xx
changed some bit settings between the original and B versions,
I don't see any benefit in having a property or a name
change (from mpc5200-cdm or so in this example) when the
differences are well documented and also easy to work out
from the device tree without additional properties, or
name changes. If the register location changes, the register
property handles it. If the operation changes this is defined
by both the soc svr, possibly a board model, and also
possibly by the pvr (cache sizes etc.).

I'll take an example of putting useless information in
the device tree - how about the CPU node? It has all the
information for cache sizes etc. but does Linux use it?
No.. the only ONLY chip I have seen this information be
relevant on is the MPC7447/MPC7457 where it is impossible
to determine by PVR which chip you have, and even then
impossible to determine the L3 cache size. And Linux
does not even handle this; the firmware sets it up and
the L3 registers are already there for Linux to pick up
on.

This is what I mean by 'describing exactly what the device
is' being rather a tedious and time-wasting concept.

I might be a little less noisy about it if there was
some kind of edict for devices never to wander outside
of their own node in the device tree, but there isn't.
You have the entire board description available and when
that information doesn't exist, it is usually in the
chip or board itself.

I don't think the device tree has much use beyond the
advertisement and authorisation of use of system devices,
and as the most basic and essential automatic driver
processes (probe and initialisation). It is quite another
matter to make it a kind of Linux-programmers errata
replacement framework and artificially recreate already
easily-accessible information.

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Segher Boessenkool]

>> Actually, you can, and should.  All this information is
>> contained in the "compatible" and "model" properties.
>> "Quirks of board design" can be described too, on a case-
>> by-case basis.
>>
>> All the knowledge about how to drive the device resides
>> in the kernel, but the device tree describes exactly what
>> device this is, so the kernel can match a driver to it
>> uniquely, and the driver can know exactly what revision
>> chip this is and what quirks to apply.
>
> That's what I said wasn't it?

Not at all, no.

> If you have a buggy i2c controller or one that has a strange
> quirk, but it's present as fsl-i2c in those device trees,
> would you specify that it is fsl-i2c-less-bugs later?
> Would you add property after property to describe errata,
> quirks in the nodes themselves?

No.  All this can be easily derived from the "model"
properties in the relevant nodes.

> I'll take an example of putting useless information in
> the device tree - how about the CPU node? It has all the
> information for cache sizes etc. but does Linux use it?

It *should* use it though.  But it cannot really do that,
since many/most device trees are broken in this respect.

Linux *does* use some of the "cpu" properties though.
Maybe in the future it will use more.

> This is what I mean by 'describing exactly what the device
> is' being rather a tedious and time-wasting concept.

This is equivalent to stating the device tree is a useless
concept.  You are free to your opinion of course.

> I might be a little less noisy about it if there was
> some kind of edict for devices never to wander outside
> of their own node in the device tree, but there isn't.

I'm not sure what you mean here.  It is best practice
for device nodes to be reasonably self-contained though.
Of course not completely; every node always has to refer
to its parent bus, etc.  Device drivers will sometimes
have to refer to board model for board-specific workarounds.

> I don't think the device tree has much use beyond the
> advertisement and authorisation of use of system devices,
> and as the most basic and essential automatic driver
> processes (probe and initialisation).

Again, you are free to your own opinion.

> It is quite another
> matter to make it a kind of Linux-programmers errata
> replacement framework and artificially recreate already
> easily-accessible information.

No one is proposing that I hope.  This information indeed
is already easily available in most cases -- namely, in
the device tree.


Segher

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Matt Sealey]

Segher Boessenkool wrote:
>>> Actually, you can, and should.  All this information is
>> I'll take an example of putting useless information in
>> the device tree - how about the CPU node? It has all the
>> information for cache sizes etc. but does Linux use it?
> 
> It *should* use it though.  But it cannot really do that,
> since many/most device trees are broken in this respect.

But if it does work, you could use it. There is information
in the device tree enough, I think, to determine which board
you are running on, which firmware version, and then simply
use that data.

However it doesn't. But as an aside, maybe it's not the best
example, since this is part of the Open Firmware device tree
standard and it would be a shame not to implement in any
device tree information which is dictated by a real standard.

The Linux flat device tree is no previously-ratified IEEE
standard in use for 15 years though :)

> Linux *does* use some of the "cpu" properties though.
> Maybe in the future it will use more.





>> This is what I mean by 'describing exactly what the device
>> is' being rather a tedious and time-wasting concept.
> 
> This is equivalent to stating the device tree is a useless
> concept.  You are free to your opinion of course.

The device tree is a great concept for what it was invented
for; it describes the hardware and it ALSO provides access
to rudimentary abstraction of hardware device drivers. OF
is not just a machine description, it is supposed to involve
using those devices through standardised I/O and function
calls.

The device tree in OF and the FDT are relegated to machine
descriptions in Linux because a) Linux kills the OF on boot
and doesn't use any of the abstraction and b) U-Boot doesn't
have any.

>> I might be a little less noisy about it if there was
>> some kind of edict for devices never to wander outside
>> of their own node in the device tree, but there isn't.
> 
> I'm not sure what you mean here.  It is best practice
> for device nodes to be reasonably self-contained though.
> Of course not completely; every node always has to refer
> to its parent bus, etc.  Device drivers will sometimes
> have to refer to board model for board-specific workarounds.

Right. So, my question is, what is the difference between
device A and B on the same chipset or using the same IP core,
which cannot be described by board model or board-specific
handling in other places in the device tree?

I see a lot of clutter on my particular tree here; the USB
controller has 'ohci-bigendian, ohci-be, mpc5200-ohci,
mpc5200-usb' in it's compatible nodes. The model is mpc5200-ohci.

I would love to know why ohci-bigendian isn't just used. It
is perfectly obvious from the location of the node (it's
parent is /builtin or /soc, which has model 'mpc5200b').

The various names and compatibles I think are just information
for information's sake. Prepending mpc5200 to it is useless
information. The ONLY differentiation I see relevant here is
that the device is big-endian. Isn't that a standard property
though in most device trees, and not part of the device name?
I don't think you could put the information in there any other
way, but then again, if you are running an MPC5200B, how many
other ways can you configure the USB device? You can practically
assume big-endianess.

The same goes for the Ethernet (mpc5200b-fec, mpc5200-fec)
which is a ridiculous differentiation. It's parent node
as defined in the same device-tree bindings document
is ALWAYS going to be the 'soc' node, which has a model
which perfectly describes the SoC. Even THAT node has
compatible properties (mpc5200-soc, mpc5200b-soc) in
the documentation.

Again it is information for information's sake. A driver for
platform USB as implemented in Linux only needs one of them,
and none of the detailing on how much of it pertains to the
chip revision is required in-node.

The information in my device tree on my Efika here, then,
is being cluttered up with properties it doesn't even need
to expose for even the dumbest driver. And most of them
are in there to support the very dumbest drivers, over 3
or 4 revisions, and then been removed.

The device tree has become a dumping ground for decisions
made by Linux developers ("what shall I call this node?"
and changing their mind each time), and is no better for
driver support for it if you only track the mainline.

I2S is another one on this particular chip example;

~~
PSC in i2s mode:  The mpc5200 and mpc5200b PSCs are not compatible when in
i2s mode.  An 'mpc5200b-psc-i2s' node cannot include 'mpc5200-psc-i2s' in the
compatible field.
~~

All you need to know is that it's an i2s controller as defined
by the board designer. Secondary to that, you need to know
that it is a PSC I2S controller (the same distinction is true
of MPC52xx SPI). This is what your model attribute is for,
right? That you are running on an MPC5200 or MPC5200B is
defined by the SoC node already. To be extremely wary you
may have the SVR or revision property in the SoC node.

All I see is the dubious 'advantage' to make Linux probe
and device driver code wonderfully succinct - it only has
to look for one particular node from a list and then makes
some brilliant assumptions based on that. But if it is
differentiating between different compatibilities, names
and models, it will be doing a lot of if/elsing around
things, different setup and so on, by checking other
properties and other nodes. If you want to use any of
the devices on the chip, you NEED to ioremap the register
space, which entails accessing the parent SoC node.

So the advantage is lost; driver code isn't succinct,
and certainly having lists of devices which 'fit'
certain names is a wasteful concept. You trade off
20 lines of code to differentiate the chip with 20
lines of compatibility tables.

In fact, this kind of device naming and using compatibility
properties is practically discouraged by Ben's device
tree document (Section III, note 3)

>> It is quite another
>> matter to make it a kind of Linux-programmers errata
>> replacement framework and artificially recreate already
>> easily-accessible information.
> 
> No one is proposing that I hope.  This information indeed
> is already easily available in most cases -- namely, in
> the device tree.

And if it isn't, it is always accessible by the chip in
question anyway. Want the SVR of the SoC? Well, accessing
the SVR on the SoC is something any driver can do, the same
for any general purpose register, and even if the device
node is not in the tree.. it can still access it because
the registers are there.

WhileI think it would be cute to put it in the device tree,
it is far from an essential value. There may be a small
advantage in "faking" devices towards the OS in order to
encourage other behaviours or even for testing. Give a
device a different name and see if it will still try to
handle it. If you are concerned about the resiliance of
driver probing and setups, here is the way to do it.

The functionality of the device tree I think is summed up
at the bottom of Grant's 52xx bindings doc;

~~
Some SoC devices share registers between them.  ie. the i2c devices use
a single clock control register, and almost all device are affected by
the port_config register.  Devices which need to manipulate shared regs
should look to the parent SoC node.  The soc node is responsible
for arbitrating all shared register access.
~~

However that seems fairly naive a distinction. I prefer to
look at this from a pin muxing point of view; If it's not in
the device tree, the board designer probably either didn't
connect it up, or it's multiplexed with something else.

In the case of devices sharing registers, you cannot
arbitrate access sometimes in individual device nodes. But
if you know the board you know the mechanisms..

The other option is, the board designer was too lazy to add the
node, but if you can accurately determine the exact board
you're running on (this, I think, is very important to have
in the device tree).

So, when designing device trees, I think the documentation
sucks, and what was originally good about trees has gotten
lost by having drivers and DT's mutually track each other.
I don't think that spurs adoption of the format, I don't
think any of the major decisions on "how much info should
I put in the tree" are relevant. Where it is board
specific, you have already defined the board, you don't
need to redefine that the component is a part of the
board. You don't need to bring in strings and properties
where the operation is fixed or obvious, just so that
it can be "detected" in the device tree.

All in all I think the flat device tree is being abused,
and the effect is, that it is making dynamic device
trees no more useful either, in fact adding development
time for *real* firmware developers.

Like you said, that's just my opinion on it, but I have
seen too many people nag at Genesi/bplan for the quirks
in our device tree, and a lot of effort went into making
our device trees fit the expectations of Linux where it
was absolutely forced upon us by Linux driver writers.

Yes, we missed out the interrupt stuff in a couple of
places and made a mistake here or there, but they are
bugs. What I saw is a lot of effort being expended into
looking at device drivers past and present and trying
to interpret a complete lack of coherent documentation
into a device tree everyone likes, and enables existing
drivers out of the box, and then getting whined at for
it.. :D

When you look at the documentation, the recommended
practises, and 10 years of Open Firmware device tree
usage, and even the booting-without-of document, it
seems the direction of FDTs is being lost in the
scramble to add more and more and more functionality
to it, and going back to the original goal of the
device tree - it describes the system, but it mostly
exists to abstract devices for -drivers in the
firmware- and not in Linux. /soc/usb/storage/disk@0,0
was a block device handle far, far before it was
simply an advertisement of an attached disk..

--
Matt Sealey <matt@genesi-usa.com>

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Segher Boessenkool]

>>> I might be a little less noisy about it if there was
>>> some kind of edict for devices never to wander outside
>>> of their own node in the device tree, but there isn't.
>>
>> I'm not sure what you mean here.  It is best practice
>> for device nodes to be reasonably self-contained though.
>> Of course not completely; every node always has to refer
>> to its parent bus, etc.  Device drivers will sometimes
>> have to refer to board model for board-specific workarounds.
>
> Right. So, my question is, what is the difference between
> device A and B on the same chipset or using the same IP core,
> which cannot be described by board model or board-specific
> handling in other places in the device tree?

I still don't understand the question.

> I see a lot of clutter on my particular tree here; the USB
> controller has 'ohci-bigendian, ohci-be, mpc5200-ohci,
> mpc5200-usb' in it's compatible nodes. The model is mpc5200-ohci.
>
> I would love to know why ohci-bigendian isn't just used. It
> is perfectly obvious from the location of the node (it's
> parent is /builtin or /soc, which has model 'mpc5200b').

"fsl,mpc5200b-usb" is the exact name.  This one can be matched
against if the driver needs to do things specific to this
chip.

"fsl,mpc5200-usb" is one step below; it can be used by a driver
if that driver can drive every USB in the 5200 family.

"usb-ohci-bigendian" can be matched against by the USB OHCI
driver if there is no need for any 5200-specific quirks.

(And yes I know I changed some names here).

> The various names and compatibles I think are just information
> for information's sake.

Lots of people go overboard with them.  "compatible" is a very
important property though: it is the main thing that is used
to match OS device drivers with devices.

> Prepending mpc5200 to it is useless
> information.

Not at all.  If the 5200 needs any special treatment, this
is where the device driver (or some early quirk step, or
whatever) will see it has to deal with that.

> The ONLY differentiation I see relevant here is
> that the device is big-endian. Isn't that a standard property
> though in most device trees, and not part of the device name?

It isn't a standard property.  It would have been a better
way to go about this though, yes -- not put the "big-endian"
in the name, but in a separate property.

> I don't think you could put the information in there any other
> way, but then again, if you are running an MPC5200B, how many
> other ways can you configure the USB device? You can practically
> assume big-endianess.

But then you need to put chipset-specific information in the
kernel, where it isn't necessary at all.  There are quite
a few USB OHCs that are big-endian.

> The same goes for the Ethernet (mpc5200b-fec, mpc5200-fec)
> which is a ridiculous differentiation. It's parent node
> as defined in the same device-tree bindings document
> is ALWAYS going to be the 'soc' node, which has a model
> which perfectly describes the SoC. Even THAT node has
> compatible properties (mpc5200-soc, mpc5200b-soc) in
> the documentation.

You shouldn't have to look at any other nodes if all you're
trying to figure out is what one device is exactly.

> Again it is information for information's sake.

Not at all.  I'm sorry you don't see that.  It seems
you'd rather want one single node that says "5200" but
that's just not how the device tree is supposed to be
used.

> A driver for
> platform USB as implemented in Linux only needs one of them,
> and none of the detailing on how much of it pertains to the
> chip revision is required in-node.

Maybe the 5200b USB is actually *fully* compatible to the
5200 USB.  So you got lucky, and the device driver can
simply match on plain 5200 for both, since the 5200b device
trees state it is compatible to that, too.  Now OTOH if some
sneaky bug would show up that requires a workaround on 5200b
devices, you can match on that instead.

> The device tree has become a dumping ground for decisions
> made by Linux developers ("what shall I call this node?"
> and changing their mind each time), and is no better for
> driver support for it if you only track the mainline.

There are many bad device trees around yes.

> I2S is another one on this particular chip example;
>
> ~~
> PSC in i2s mode:  The mpc5200 and mpc5200b PSCs are not compatible=20
> when in
> i2s mode.  An 'mpc5200b-psc-i2s' node cannot include 'mpc5200-psc-i2s'=20=

> in the
> compatible field.
> ~~
>
> All you need to know is that it's an i2s controller as defined
> by the board designer. Secondary to that, you need to know
> that it is a PSC I2S controller (the same distinction is true
> of MPC52xx SPI). This is what your model attribute is for,
> right?

No, the "model" property is for defining the exact
"part number" (or similar) for the device.  I'll quote:

	Standard property name to define a manufacturer=92s model
	number.

	prop-encoded-array: Text string, encoded with encode-string.
	A manufacturer-dependent string that generally specifies the
	model name and number (including revision level) for this
	device. The format of the text string is arbitrary, although
	in conventional usage the string begins with the name of the
	device=92s manufacturer as with the =93name=94 property. =
Although
	there is no standard interpretation for the value of the
	=93model=94 property, a specific device driver might use it to
	learn, for instance, the revision level of its particular
	device.

> That you are running on an MPC5200 or MPC5200B is
> defined by the SoC node already.

Irrelevant, as I've said many times now.

> If you want to use any of
> the devices on the chip, you NEED to ioremap the register
> space, which entails accessing the parent SoC node.

But the probing routine doesn't have to; there are
helper functions for that.

> So the advantage is lost; driver code isn't succinct,
> and certainly having lists of devices which 'fit'
> certain names is a wasteful concept. You trade off
> 20 lines of code to differentiate the chip with 20
> lines of compatibility tables.
>
> In fact, this kind of device naming and using compatibility
> properties is practically discouraged by Ben's device
> tree document (Section III, note 3)

I think you just read it wrong.  Either way, you should
read the standard OF documentation as well, it would
certainly help you understand some of the concepts here.

>>> It is quite another
>>> matter to make it a kind of Linux-programmers errata
>>> replacement framework and artificially recreate already
>>> easily-accessible information.
>>
>> No one is proposing that I hope.  This information indeed
>> is already easily available in most cases -- namely, in
>> the device tree.
>
> And if it isn't, it is always accessible by the chip in
> question anyway. Want the SVR of the SoC? Well, accessing
> the SVR on the SoC is something any driver can do, the same
> for any general purpose register, and even if the device
> node is not in the tree.. it can still access it because
> the registers are there.

While you could do that for some SoC devices perhaps,
_iff_ you know for sure that no matter what those SVR
or whatever registers live at certain addresses when
all you really know is that you have one specific SoC
device, you cannot for most other devices.  Why would
you special case some SoC devices?  It is an encapsulation
violation no matter what.

> WhileI think it would be cute to put it in the device tree,
> it is far from an essential value.

Nothing is "essential", certainly you *can* boot a Linux
kernel without any device tree.  Duh.

> All in all I think the flat device tree is being abused,

Yes it is, but the things that you complain about aren't
the problem -- things that require implicit knowledge
(about Linux for example) are.

> Like you said, that's just my opinion on it, but I have
> seen too many people nag at Genesi/bplan for the quirks
> in our device tree, and a lot of effort went into making
> our device trees fit the expectations of Linux where it
> was absolutely forced upon us by Linux driver writers.

You could have avoided most of those problems by working
with us from the start.  Developing in a hidden corner
for months and then dropping all your code on us seldom
gives the results you hope for.  Anyway, let's not rehash
*that* discussion.

> What I saw is a lot of effort being expended into
> looking at device drivers past and present and trying
> to interpret a complete lack of coherent documentation

There is *plenty* very good documentation.  And you
can always ask.

> and going back to the original goal of the
> device tree - it describes the system, but it mostly
> exists to abstract devices for -drivers in the
> firmware- and not in Linux.

Rubbish.

> /soc/usb/storage/disk@0,0
> was a block device handle far, far before it was
> simply an advertisement of an attached disk..

It *always* is the second, and it *can* be the first.


Segher

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Matt Sealey]

Segher Boessenkool wrote:
>> Like you said, that's just my opinion on it, but I have
>> seen too many people nag at Genesi/bplan for the quirks
>> in our device tree, and a lot of effort went into making
>> our device trees fit the expectations of Linux where it
>> was absolutely forced upon us by Linux driver writers.
> 
> You could have avoided most of those problems by working
> with us from the start.  Developing in a hidden corner
> for months and then dropping all your code on us seldom
> gives the results you hope for.  Anyway, let's not rehash
> *that* discussion.

*Rubbish/Rehash*. When we started firmware development for
the Efika (ahem, mid-2005) there was NO mpc52xx device tree
specification.

Even if you do have a device tree specification I think it
has been somewhat tainted by it being written by the driver
developers, who are not so much hell-bent on writing a great
spec as hell-bent on making their code easier on the eyes
of the people who ACK it.

And lastly, the real crime here is that absolutely NO input
was attempted to be gained from the developers of the ONLY
open-firmware 5200B board on the planet, at the time. It
was just written, and pushed out there, and then it was
asked "why aren't bplan following our specification?!?!"

~

I really do not think it is a problem for a device to be
called "usb-ohci" in the device tree, with a property for
"big-endian". Or perhaps, "irda" with a compatible for
"psc-irda". Without the chip specifier. I also do not think
it is a problem for the device driver to look at the parent
bus node or soc node or platform node or builtin node or
whatever else, to find out more information about the driver.
Naming these nodes with long-winded device-inclusive names
in the first place is a difficult to understand shortcut
which provides no reasonable benefits.

Ostensibly, with an OCHI USB controller, they are all the
same anyway (it's a register definition, the only variable
points being where it starts and whether you use big endian
or little endian values), so why give it a special name?

If you have 6 chips which implement the same Freescale OHCI
controller but with varying levels of compliance, they are
still all OHCI controllers. And you can tell which OHCI
quirks you need to fix up in the driver, by.. looking at
the chip it's running on. The controller itself would never,
ever tell you how broken it is in real life. I don't
think encapsulation problems exist here. You want to support
a device but quirk out some specific chip revisions it's
built into, well, you need to find the device, then find out
the chip it's in, if in fact it's in a chip at all (if it's
not, then it could be a PCI device, in which case the
details of that are in the PCI config space, without a doubt.
Since most x86 chipset drivers do not look at device trees
they are implementing blacklists, id checks and tweaking
values to see the effects - isn't it code sharing to let
the ppc-compiled version of the same driver do the same
tests, rather than compile it out, add all new code to
determine it's capabilities from a device tree, which may
not have been written to fully determine the device quirks
of 10,000,000 PCI devices in the world possible of being
plugged into a PCI slot?). This is why I artificially limit
it to SoC's.

What is the effect if the device never changes over the
lifetime of the chip and you started calling it "mpc5200-usb"
to start with? Well, every device has to list "compatible"
for at least one chip which may well be very far removed,
I think that is an ugly solution in terms of the namespace.

If a device driver knows about certain chip revisions and
needs only to make small quirks and changes happen in the
device driver, naming it "mpc5200-usb" and "mpc5200b-usb"
and "mpc5121e-usb" and "mpc5900-usb" over and over makes
no benefit to the small modifications required.

As compared to, for instance, calling it "usb-ohci", giving
it a "big-endian" property, then looking at the parent
node, seeing if it's an "soc" node, looking at the model
property to check the chip revision or board revision, and
implementing the quirks, I don't see why this is any different
to the way you wanted to have it done in the first place. In
fact I think it's a more portable, more true-to-OF-specs
way of doing it.

And in fact, if there is no point whatsoever to implementing
the soc model property since all the child nodes need to be
encapsulated, why bother having it there? it's an optional
property in the specs.

If I had my way I'd be rewriting your device tree specs for
the MPC52xx now, but that would only burden the guys at
bplan writing the firmware to model new revisions against a
new device tree spec, which is a waste of time. We're stuck
with what you all jotted down one day, and expected all
device vendors to follow without nary a consultation, and
you yourself have admitted there, that some parts of it are
written and the authors seem to have gone a little overboard
on it. Now it is written, it is practically fixed, so what
do we do about it?

Is there some future process that can be determined which
stops brand new devices being released, reverse engineered
by Linux developers and then have the device vendors
hammered with a "linuxised" device tree they must then
certainly match for fear of derision, a device tree which
on a real and unbiased review, would probably be picked to
fault itself?

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Jean Delvare]

On Fri, 18 May 2007 12:10:18 -0500, Kumar Gala wrote:
> For I2C specifically we already have both a dynamic way (kernel cmd  
> line) and static (i2c_board_info) to specify the i2c devices, why do  
> we need yet another?

This reminds me that the new-style drivers lost the ability to specify
i2c devices on the kernel command line. I wonder if it will be a
problem in practice. I planned to create a sysfs interface as a
replacement but never took the time to actually implement it.

-- 
Jean Delvare

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Segher Boessenkool]

>> The only support we have for i2c controllers is to support one
>> specific i2c controller from Freescale.
>>
>> If you aren't going to provide a complete solution why are you
>> prosing one?  I'm tired of this put stuff in the device tree but only
>> as much as I need to do my particular thing.
>
> This is exactly how free software development works. If people were
> only proposing complete solutions, Linux would not even exist. Things
> happen exactly because people write what they need and contribute what
> they wrote. If you think it's not enough for your own needs (present or
> future), then _you_ get to do the extra work.

You propose a generic device tree binding.  This is
a public API/ABI so it is important that it is future-
proofed as far as possible/sane/whatever.  If you're
asked how a not at all uncommon situation should be
expressed using that binding, and you cannot / will
not give an answer, that is basically equivalent to
withdrawing your proposed binding.

If you cannot get yourself to think about the general
situation, please don't try to draft generic bindings,
but restrict yourself to something board specific or
with that scope at least.  And hope no one else takes
interest in that board or they'll happily NAK your
patches.


Segher

Re: [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Segher Boessenkool]

> +   Required properties :
> +
> +    - reg : Unshifted 7-bit I2C address for the device

What about 10-bit addressing, etc.?

> +   Recommended properties :
> +
> +    - compatible : The name of the Linux device driver that
> +      handles this device.  If unspecified, the name of the
> +      node will be used.

NO WAY

> +    - interrupts : <a b> where a is the interrupt number and b is a

I2C doesn't do interrupts, this doesn't belong in
an I2C binding; it's redundant anyway (and incorrect
as well).


I second most of Kumar's sentiments, too.


Segher

Re: [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Scott Wood]

Segher Boessenkool wrote:
>> +   Required properties :
>> +
>> +    - reg : Unshifted 7-bit I2C address for the device
> 
> What about 10-bit addressing, etc.?

I specified 7-bit to address someone's question back when this first 
came up of whether it was 7-bit unshifted or 8-bit shifted.  Perhaps it 
should just say "Unshifted I2C address for the device"?

>> +   Recommended properties :
>> +
>> +    - compatible : The name of the Linux device driver that
>> +      handles this device.  If unspecified, the name of the
>> +      node will be used.
> 
> NO WAY

Sorry, that was left in there from a while ago and I missed it.  It 
should be defined the same way as any other compatible property (and the 
i2c code in Linux should be fixed to allow drivers to specify multiple 
match names).  No need for shouting. :-)

>> +    - interrupts : <a b> where a is the interrupt number and b is a
> 
> I2C doesn't do interrupts,

...but some I2C devices do.

> this doesn't belong in an I2C binding; it's redundant anyway

I guess it's implicit that any device that generates interrupts will 
have an interrupts property, though there are many other examples of 
this sort of redundancy in booting-without-of.txt.  Its inclusion was 
mainly an example.

 > (and incorrect as well).

How is it incorrect?

-Scott

Re: [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Segher Boessenkool]

>>> +    - reg : Unshifted 7-bit I2C address for the device
>> What about 10-bit addressing, etc.?
>
> I specified 7-bit to address someone's question back when this first 
> came up of whether it was 7-bit unshifted or 8-bit shifted.  Perhaps 
> it should just say "Unshifted I2C address for the device"?

Better, yes.

>>> +    - compatible : The name of the Linux device driver that
>>> +      handles this device.  If unspecified, the name of the
>>> +      node will be used.
>> NO WAY
>
> Sorry, that was left in there from a while ago and I missed it.  It 
> should be defined the same way as any other compatible property (and 
> the i2c code in Linux should be fixed to allow drivers to specify 
> multiple match names).  No need for shouting. :-)

Oh yes there is :-)

>>> +    - interrupts : <a b> where a is the interrupt number and b is a
>> I2C doesn't do interrupts,
>
> ...but some I2C devices do.

So?  They do that outside of the I2C domain.

>> this doesn't belong in an I2C binding; it's redundant anyway
>
> I guess it's implicit that any device that generates interrupts will 
> have an interrupts property,

This is defined in the base spec as well as in the interrupt
mapping spec, yes.  The exact format of the "interrupts"
property for a device is defined in the binding for the
interrupt domain that interrupt lives in.

> though there are many other examples of this sort of redundancy in 
> booting-without-of.txt.  Its inclusion was mainly an example.
>
> > (and incorrect as well).
>
> How is it incorrect?

You specified that an interrupt specifier consists of two
cells.  This is wrong.


Segher

Re: [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Scott Wood]

Segher Boessenkool wrote:
>> How is it incorrect?
> 
> 
> You specified that an interrupt specifier consists of two
> cells.  This is wrong.

Oops...  Apparently, that was copied from other bindings with the same 
problem.

Next time I'll look more carefully at a patch from 6 months ago before 
resending it. :-P

-Scott

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Jean Delvare]

Hi Scott,

On Thu, 17 May 2007 14:32:11 -0500, Scott Wood wrote:
> (and the 
> i2c code in Linux should be fixed to allow drivers to specify multiple 
> match names).

Back when David proposed his new-style i2c code, I had the same
objection. But we addressed the need differently. If you look at struct
i2c_board_info, you'll see two string fields, driver_name and type. The
former specifies the driver name, the second specifies the exact device
variant. For drivers which support several device variants, the
platform code should fill both fields.

-- 
Jean Delvare

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Scott Wood]

Jean Delvare wrote:
> Hi Scott,
> 
> On Thu, 17 May 2007 14:32:11 -0500, Scott Wood wrote:
> 
>>(and the 
>>i2c code in Linux should be fixed to allow drivers to specify multiple 
>>match names).
> 
> 
> Back when David proposed his new-style i2c code, I had the same
> objection. But we addressed the need differently. If you look at struct
> i2c_board_info, you'll see two string fields, driver_name and type. The
> former specifies the driver name, the second specifies the exact device
> variant. For drivers which support several device variants, the
> platform code should fill both fields.

But that still requires the platform to know the driver name, rather 
than matching any driver which knows about the type.  This prevents the 
use of OS-independent device trees (such as in Open Firmware), which 
cannot know specific Linux driver names, without something hacky like a 
type-to-driver table in the device tree code.

-Scott

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Kumar Gala]

On May 18, 2007, at 10:58 AM, Scott Wood wrote:

> Jean Delvare wrote:
>> Hi Scott,
>>
>> On Thu, 17 May 2007 14:32:11 -0500, Scott Wood wrote:
>>
>>> (and the
>>> i2c code in Linux should be fixed to allow drivers to specify  
>>> multiple
>>> match names).
>>
>>
>> Back when David proposed his new-style i2c code, I had the same
>> objection. But we addressed the need differently. If you look at  
>> struct
>> i2c_board_info, you'll see two string fields, driver_name and  
>> type. The
>> former specifies the driver name, the second specifies the exact  
>> device
>> variant. For drivers which support several device variants, the
>> platform code should fill both fields.
>
> But that still requires the platform to know the driver name, rather
> than matching any driver which knows about the type.  This prevents  
> the
> use of OS-independent device trees (such as in Open Firmware), which
> cannot know specific Linux driver names, without something hacky  
> like a
> type-to-driver table in the device tree code.

And this is why I don't think there is any value in trying to put I2C  
devices in the device tree.  The linux mechanism is specific to  
Linux, and is based on Linux created names.  To provide something  
more generic someone would have to take on the task of providing a  
more global registry of names for devices and I just don't see anyone  
doing that.

Can someone explain to me why setting up i2c_board_info in the board  
specific code isn't sufficient?

- k

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Jean Delvare]

On Fri, 18 May 2007 10:58:06 -0500, Scott Wood wrote:
> Jean Delvare wrote:
> > On Thu, 17 May 2007 14:32:11 -0500, Scott Wood wrote:
> > 
> >>(and the 
> >>i2c code in Linux should be fixed to allow drivers to specify multiple 
> >>match names).
> > 
> > 
> > Back when David proposed his new-style i2c code, I had the same
> > objection. But we addressed the need differently. If you look at struct
> > i2c_board_info, you'll see two string fields, driver_name and type. The
> > former specifies the driver name, the second specifies the exact device
> > variant. For drivers which support several device variants, the
> > platform code should fill both fields.
> 
> But that still requires the platform to know the driver name, rather 
> than matching any driver which knows about the type.  This prevents the 
> use of OS-independent device trees (such as in Open Firmware), which 
> cannot know specific Linux driver names, without something hacky like a 
> type-to-driver table in the device tree code.

Oh well, this was also the reason why I objected to David's approach in
the first place. If you dig back in the i2c list archive, you'll find
that I was asking for exactly the same thing you do now: that each i2c
driver would export a list of supported devices, and the i2c-core would
match a device name against that list (independent of the driver name.)
It felt more flexible, but I wondered how useful it would be in
practice, and finally gave up and David had the last word. If you had
shown up back then rather than now...

I am not familiar with Open Firmware. How standard is it? How realistic
would it be to use their device naming in the Linux kernel? Are there
other subsystem doing this? Are there other OSes using it, in
particular for I2C?

We have something which works now, even if that's not what you and I
had in mind, so I don't really want to change it without solid reasons.

-- 
Jean Delvare

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Kumar Gala]

On May 18, 2007, at 11:31 AM, Jean Delvare wrote:

> On Fri, 18 May 2007 10:58:06 -0500, Scott Wood wrote:
>> Jean Delvare wrote:
>>> On Thu, 17 May 2007 14:32:11 -0500, Scott Wood wrote:
>>>
>>>> (and the
>>>> i2c code in Linux should be fixed to allow drivers to specify  
>>>> multiple
>>>> match names).
>>>
>>>
>>> Back when David proposed his new-style i2c code, I had the same
>>> objection. But we addressed the need differently. If you look at  
>>> struct
>>> i2c_board_info, you'll see two string fields, driver_name and  
>>> type. The
>>> former specifies the driver name, the second specifies the exact  
>>> device
>>> variant. For drivers which support several device variants, the
>>> platform code should fill both fields.
>>
>> But that still requires the platform to know the driver name, rather
>> than matching any driver which knows about the type.  This  
>> prevents the
>> use of OS-independent device trees (such as in Open Firmware), which
>> cannot know specific Linux driver names, without something hacky  
>> like a
>> type-to-driver table in the device tree code.
>
> Oh well, this was also the reason why I objected to David's  
> approach in
> the first place. If you dig back in the i2c list archive, you'll find
> that I was asking for exactly the same thing you do now: that each i2c
> driver would export a list of supported devices, and the i2c-core  
> would
> match a device name against that list (independent of the driver  
> name.)
> It felt more flexible, but I wondered how useful it would be in
> practice, and finally gave up and David had the last word. If you had
> shown up back then rather than now...
>
> I am not familiar with Open Firmware. How standard is it? How  
> realistic
> would it be to use their device naming in the Linux kernel? Are there
> other subsystem doing this? Are there other OSes using it, in
> particular for I2C?

OF doesn't have any particular bindings that already exist for I2C.   
If it had I might be more in favor of trying to make Linux work with it.

Here's a link to give you some idea of what bindings exist already  
for OF:

http://openbios.info/Bindings

> We have something which works now, even if that's not what you and I
> had in mind, so I don't really want to change it without solid  
> reasons.

agreed.

- k

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[David Brownell]

On Friday 18 May 2007, Jean Delvare wrote:
> On Fri, 18 May 2007 10:58:06 -0500, Scott Wood wrote:
> > Jean Delvare wrote:
> > > On Thu, 17 May 2007 14:32:11 -0500, Scott Wood wrote:
> > > 
> > >>(and the 
> > >>i2c code in Linux should be fixed to allow drivers to specify multiple 
> > >>match names).
> > > 
> > > 
> > > Back when David proposed his new-style i2c code, I had the same
> > > objection. But we addressed the need differently. If you look at struct
> > > i2c_board_info, you'll see two string fields, driver_name and type. The
> > > former specifies the driver name, the second specifies the exact device
> > > variant. For drivers which support several device variants, the
> > > platform code should fill both fields.
> > 
> > But that still requires the platform to know the driver name, rather 
> > than matching any driver which knows about the type.

Given that the platform (== board/system) may care about the upper layer
of the driver (APIs exposed) not just the lower one (talking-to-hardware),
that seems like a reasonable tradeoff.  It wouldn't want to use the driver
which provides the wrong programming interface!

Plus, to repeat, there is no notion of "type" in I2C.   So in order to
talk about a "type based matching" algorithm you'd have to define one...


> > This prevents the  
> > use of OS-independent device trees (such as in Open Firmware), which 
> > cannot know specific Linux driver names, without something hacky like a 
> > type-to-driver table in the device tree code.
> 
> Oh well, this was also the reason why I objected to David's approach in
> the first place. If you dig back in the i2c list archive, you'll find
> that I was asking for exactly the same thing you do now: that each i2c
> driver would export a list of supported devices, and the i2c-core would
> match a device name against that list (independent of the driver name.)
> It felt more flexible, but I wondered how useful it would be in
> practice, and finally gave up and David had the last word. If you had
> shown up back then rather than now...

With concrete suggestions and patches.  I'd have to dig back in the
list archives to see the details of what was said at that time, but
I don't recall any suggestions being rejected unless they dropped
essential functionality.  (There was one notion to key driver binding
purely on device address, for example...)

- Dave

Re: [i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Jean Delvare]

On Thu, 17 May 2007 21:18:15 +0200, Segher Boessenkool wrote:
> > +   Required properties :
> > +
> > +    - reg : Unshifted 7-bit I2C address for the device
> 
> What about 10-bit addressing, etc.?

Who needs this? Support for 10-bit I2C addressing has been broken for
years and nobody seems to care.

-- 
Jean Delvare