"cell-index" vs. "index" vs. no index in I2C device nodes

"cell-index" vs. "index" vs. no index in I2C device nodes

[Stefan Roese]

I'm wondering what is currently recommended in the I2C device tree nodes? The 
current IBM I2C driver (i2c-ibm_iic.c) checks "index" and most FSL dts files 
use "cell-index". Some 4xx dts files implement "cell-index" some have no 
index at all.

So what should be used here. Please advise and I'll prepare a patch for it.

Thanks.

Best regards,
Stefan

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Stefan Roese wrote:
> I'm wondering what is currently recommended in the I2C device tree nodes? The 
> current IBM I2C driver (i2c-ibm_iic.c) checks "index" and most FSL dts files 
> use "cell-index". Some 4xx dts files implement "cell-index" some have no 
> index at all.
> 
> So what should be used here. Please advise and I'll prepare a patch for it.

I just posted a patch for the FSL I2C driver to check for cell-index.  I'm under
the impression that cell-index is the standard for enumerating devices in the
device tree.

My vote: any driver that currently checks for index should first check
cell-index, and any device tree that has index should add cell-index.  In time,
we can remove 'index' from the device trees.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Scott Wood]

On Wed, Jun 04, 2008 at 10:24:15AM -0500, Timur Tabi wrote:
> Stefan Roese wrote:
> > I'm wondering what is currently recommended in the I2C device tree nodes? The 
> > current IBM I2C driver (i2c-ibm_iic.c) checks "index" and most FSL dts files 
> > use "cell-index". Some 4xx dts files implement "cell-index" some have no 
> > index at all.
> > 
> > So what should be used here. Please advise and I'll prepare a patch for it.
> 
> I just posted a patch for the FSL I2C driver to check for cell-index.  I'm under
> the impression that cell-index is the standard for enumerating devices in the
> device tree.

No, it's the standard for correlating devices with portions of a shared
register block elsewhere.  Your use in the I2C node is merely a hack to
deal with Linux wanting to deal with indices rather than pointers,
combined with a lack of a decent way to look up a device struct from the
device node.

-Scott

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Josh Boyer]

On Wed, 4 Jun 2008 10:43:51 -0500
Scott Wood <scottwood@freescale.com> wrote:

> On Wed, Jun 04, 2008 at 10:24:15AM -0500, Timur Tabi wrote:
> > Stefan Roese wrote:
> > > I'm wondering what is currently recommended in the I2C device tree no=
des? The=20
> > > current IBM I2C driver (i2c-ibm_iic.c) checks "index" and most FSL dt=
s files=20
> > > use "cell-index". Some 4xx dts files implement "cell-index" some have=
 no=20
> > > index at all.
> > >=20
> > > So what should be used here. Please advise and I'll prepare a patch f=
or it.
> >=20
> > I just posted a patch for the FSL I2C driver to check for cell-index.  =
I'm under
> > the impression that cell-index is the standard for enumerating devices =
in the
> > device tree.
>=20
> No, it's the standard for correlating devices with portions of a shared
> register block elsewhere.  Your use in the I2C node is merely a hack to
> deal with Linux wanting to deal with indices rather than pointers,
> combined with a lack of a decent way to look up a device struct from the
> device node.

So if possible, I'd like to eliminate the *index stuff all together
from the 4xx driver.  The private data structure contains an idx
parameter, but this can be populated based on probe order or something.

=46rom a device tree perspective, index and cell-index are both
incorrect.  The IIC macros don't share register blocks with anything,
are enumerated as unique instances per macro in the device tree, and
should be able to be distinguished by "regs" and/or unit address.

Does anyone disagree with that?

josh

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[David Gibson]

On Wed, Jun 04, 2008 at 09:19:42PM -0500, Josh Boyer wrote:
> On Wed, 4 Jun 2008 10:43:51 -0500
> Scott Wood <scottwood@freescale.com> wrote:
> 
> > On Wed, Jun 04, 2008 at 10:24:15AM -0500, Timur Tabi wrote:
> > > Stefan Roese wrote:
> > > > I'm wondering what is currently recommended in the I2C device tree nodes? The 
> > > > current IBM I2C driver (i2c-ibm_iic.c) checks "index" and most FSL dts files 
> > > > use "cell-index". Some 4xx dts files implement "cell-index" some have no 
> > > > index at all.
> > > > 
> > > > So what should be used here. Please advise and I'll prepare a patch for it.
> > > 
> > > I just posted a patch for the FSL I2C driver to check for cell-index.  I'm under
> > > the impression that cell-index is the standard for enumerating devices in the
> > > device tree.
> > 
> > No, it's the standard for correlating devices with portions of a shared
> > register block elsewhere.  Your use in the I2C node is merely a hack to
> > deal with Linux wanting to deal with indices rather than pointers,
> > combined with a lack of a decent way to look up a device struct from the
> > device node.
> 
> So if possible, I'd like to eliminate the *index stuff all together
> from the 4xx driver.  The private data structure contains an idx
> parameter, but this can be populated based on probe order or something.
> 
> >From a device tree perspective, index and cell-index are both
> incorrect.  The IIC macros don't share register blocks with anything,
> are enumerated as unique instances per macro in the device tree, and
> should be able to be distinguished by "regs" and/or unit address.
> 
> Does anyone disagree with that?

Hear, hear.

Aliases can also provide a reasonable way of enumerating devices, if
"reg" isn't suitable on its own.  Though obviously, drivers will need
some sort of fallback if suitable aliases don't exist in the tree.

-- 
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: "cell-index" vs. "index" vs. no index in I2C device nodes

[Sean MacLennan]

On Wed, 4 Jun 2008 21:19:42 -0500
"Josh Boyer" <jwboyer@linux.vnet.ibm.com> wrote:

> From a device tree perspective, index and cell-index are both
> incorrect.  The IIC macros don't share register blocks with anything,
> are enumerated as unique instances per macro in the device tree, and
> should be able to be distinguished by "regs" and/or unit address.

So say I have an i2c bus and I want to call i2c_get_adapter.
Would I have to know the register address of the i2c bus to lookup the
adapter?

Or would they have aliases like serial ports do?

Cheers,
   Sean

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Josh Boyer]

On Wed, 4 Jun 2008 22:54:32 -0400
Sean MacLennan <smaclennan@pikatech.com> wrote:

> On Wed, 4 Jun 2008 21:19:42 -0500
> "Josh Boyer" <jwboyer@linux.vnet.ibm.com> wrote:
> 
> > From a device tree perspective, index and cell-index are both
> > incorrect.  The IIC macros don't share register blocks with anything,
> > are enumerated as unique instances per macro in the device tree, and
> > should be able to be distinguished by "regs" and/or unit address.
> 
> So say I have an i2c bus and I want to call i2c_get_adapter.
> Would I have to know the register address of the i2c bus to lookup the
> adapter?
> 
> Or would they have aliases like serial ports do?

I have no idea.  But if you look at the actual driver code, the only
thing the current "index" property is used for is to fill the idx
member of the ibm_iic_private structure.

I'm not proposing we remove that.  I'm just proposing that it can be
derived from something other than an "index" property.  Fill it in
using a static integer that gets incremented for each new device
found.  It's not like we have an indeterminate probe order, or these
IIC macros can be hot-plugged.

josh

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Sean MacLennan]

On Wed, 4 Jun 2008 22:05:55 -0500
Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:

> I'm not proposing we remove that.  I'm just proposing that it can be
> derived from something other than an "index" property.  Fill it in
> using a static integer that gets incremented for each new device
> found.  It's not like we have an indeterminate probe order, or these
> IIC macros can be hot-plugged.

That's how it used to work by default. It was decided to drop that and
enforce an index. The following is a quote from Jean Delvare from a
post from 8/2/16 4:31:

> I don't like this static index thing much. Can't you just make the
> "index" OF property mandatory? Mixing ways to number things can become
> very confusing. In particular as you are using dev->idx later to call
> i2c_add_numbered_adapter(), the caller is really supposed to know what
> they are doing with the bus numbers.

Maybe it is time to remove the index, or maybe we should go back to
using both a static and the index. But at the time we decided to enforce
an index.

Cheers,
   Sean

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Stefan Roese]

On Thursday 05 June 2008, Sean MacLennan wrote:
> On Wed, 4 Jun 2008 22:05:55 -0500
>
> Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> > I'm not proposing we remove that.  I'm just proposing that it can be
> > derived from something other than an "index" property.  Fill it in
> > using a static integer that gets incremented for each new device
> > found.  It's not like we have an indeterminate probe order, or these
> > IIC macros can be hot-plugged.
>
> That's how it used to work by default. It was decided to drop that and
> enforce an index. The following is a quote from Jean Delvare from a

I added Jean to CC now.

> post from 8/2/16 4:31:
> > I don't like this static index thing much. Can't you just make the
> > "index" OF property mandatory? Mixing ways to number things can become
> > very confusing. In particular as you are using dev->idx later to call
> > i2c_add_numbered_adapter(), the caller is really supposed to know what
> > they are doing with the bus numbers.
>
> Maybe it is time to remove the index, or maybe we should go back to
> using both a static and the index. But at the time we decided to enforce
> an index.

So what should we do now? Currently I2C doesn't work at all on 4xx since the 
driver expects the "index" property and no dts sets this property. Personally 
I would like to move to using cell-index here, since this seems to be more 
common. But I could also life with removing the index property and using 
the "static index" if this is preferred and/or acceptable.

Please advise. Thanks.

Best regards,
Stefan

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Jean Delvare]

On Thu, 5 Jun 2008 08:22:00 +0200, Stefan Roese wrote:
> On Thursday 05 June 2008, Sean MacLennan wrote:
> > On Wed, 4 Jun 2008 22:05:55 -0500
> >
> > Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> > > I'm not proposing we remove that.  I'm just proposing that it can be
> > > derived from something other than an "index" property.  Fill it in
> > > using a static integer that gets incremented for each new device
> > > found.  It's not like we have an indeterminate probe order, or these
> > > IIC macros can be hot-plugged.
> >
> > That's how it used to work by default. It was decided to drop that and
> > enforce an index. The following is a quote from Jean Delvare from a
> 
> I added Jean to CC now.
> 
> > post from 8/2/16 4:31:
> > > I don't like this static index thing much. Can't you just make the
> > > "index" OF property mandatory? Mixing ways to number things can become
> > > very confusing. In particular as you are using dev->idx later to call
> > > i2c_add_numbered_adapter(), the caller is really supposed to know what
> > > they are doing with the bus numbers.
> >
> > Maybe it is time to remove the index, or maybe we should go back to
> > using both a static and the index. But at the time we decided to enforce
> > an index.
> 
> So what should we do now? Currently I2C doesn't work at all on 4xx since the 
> driver expects the "index" property and no dts sets this property. Personally 
> I would like to move to using cell-index here, since this seems to be more 
> common. But I could also life with removing the index property and using 
> the "static index" if this is preferred and/or acceptable.
> 
> Please advise. Thanks.

As far as I am concerned, it's really up to the maintainers and users
of this platform. All I am asking for is that you do not call
i2c_add_numbered_adapter() on an adapter with an automatically
generated number. This function must only be used for adapter's those
number is well defined. If an adapter doesn't have a well-defined
number then use i2c_add_adapter() (but then you can no longer declare
your I2C devices as part of the platform data.)

Thanks,
-- 
Jean Delvare

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Stefan Roese]

On Thursday 05 June 2008, Jean Delvare wrote:
> > > Maybe it is time to remove the index, or maybe we should go back to
> > > using both a static and the index. But at the time we decided to
> > > enforce an index.
> >
> > So what should we do now? Currently I2C doesn't work at all on 4xx since
> > the driver expects the "index" property and no dts sets this property.
> > Personally I would like to move to using cell-index here, since this
> > seems to be more common. But I could also life with removing the index
> > property and using the "static index" if this is preferred and/or
> > acceptable.
> >
> > Please advise. Thanks.
>
> As far as I am concerned, it's really up to the maintainers and users
> of this platform. All I am asking for is that you do not call
> i2c_add_numbered_adapter() on an adapter with an automatically
> generated number. This function must only be used for adapter's those
> number is well defined. If an adapter doesn't have a well-defined
> number then use i2c_add_adapter() (but then you can no longer declare
> your I2C devices as part of the platform data.)

Full ack from me. So I suggest to use "cell-index" if available and otherwise 
use an incremented number, same as the FSL i2c driver does now:

http://ozlabs.org/pipermail/linuxppc-dev/2008-June/057254.html

If nobody objects I'll send a patch to add the cell-index to all 4xx dts files 
in a short while.

Best regards,
Stefan

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[David Gibson]

On Thu, Jun 05, 2008 at 10:45:42AM +0200, Stefan Roese wrote:
> On Thursday 05 June 2008, Jean Delvare wrote:
> > > > Maybe it is time to remove the index, or maybe we should go back to
> > > > using both a static and the index. But at the time we decided to
> > > > enforce an index.
> > >
> > > So what should we do now? Currently I2C doesn't work at all on 4xx since
> > > the driver expects the "index" property and no dts sets this property.
> > > Personally I would like to move to using cell-index here, since this
> > > seems to be more common. But I could also life with removing the index
> > > property and using the "static index" if this is preferred and/or
> > > acceptable.
> > >
> > > Please advise. Thanks.
> >
> > As far as I am concerned, it's really up to the maintainers and users
> > of this platform. All I am asking for is that you do not call
> > i2c_add_numbered_adapter() on an adapter with an automatically
> > generated number. This function must only be used for adapter's those
> > number is well defined. If an adapter doesn't have a well-defined
> > number then use i2c_add_adapter() (but then you can no longer declare
> > your I2C devices as part of the platform data.)
> 
> Full ack from me. So I suggest to use "cell-index" if available and otherwise 
> use an incremented number, same as the FSL i2c driver does now:
> 
> http://ozlabs.org/pipermail/linuxppc-dev/2008-June/057254.html
> 
> If nobody objects I'll send a patch to add the cell-index to all 4xx
> dts files in a short while.

Do *not* add cell-index unless it is used to correlate to a global
register somewhere.  Although, for the 4xx IIC adaptor, I think there
usually will be shared registers, and a cell-index property.

The proper device tree mechanism for providing a standard numbering
for similar devices is the /aliases node.  So instead, what you should
do is:
	- If a suitable alias exists pointing to this device, use that
number
	- Otherwise, if cell-index is present, use that (this is a
fallback heuristic, not recommended practice)
	- Otherwise, use i2c_add_adapter() instead of
i2c_add_numbered_adapter()

If a platform has a conventional way of numbering the i2c busses,
which you want to represent in the device tree, you should add
aliases, not cell-index.

-- 
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: "cell-index" vs. "index" vs. no index in I2C device nodes

[Josh Boyer]

On Thu, 5 Jun 2008 10:45:42 +0200
Stefan Roese <sr@denx.de> wrote:

> On Thursday 05 June 2008, Jean Delvare wrote:
> > > > Maybe it is time to remove the index, or maybe we should go back to
> > > > using both a static and the index. But at the time we decided to
> > > > enforce an index.
> > >
> > > So what should we do now? Currently I2C doesn't work at all on 4xx since
> > > the driver expects the "index" property and no dts sets this property.
> > > Personally I would like to move to using cell-index here, since this
> > > seems to be more common. But I could also life with removing the index
> > > property and using the "static index" if this is preferred and/or
> > > acceptable.
> > >
> > > Please advise. Thanks.
> >
> > As far as I am concerned, it's really up to the maintainers and users
> > of this platform. All I am asking for is that you do not call
> > i2c_add_numbered_adapter() on an adapter with an automatically
> > generated number. This function must only be used for adapter's those
> > number is well defined. If an adapter doesn't have a well-defined
> > number then use i2c_add_adapter() (but then you can no longer declare
> > your I2C devices as part of the platform data.)
> 
> Full ack from me. So I suggest to use "cell-index" if available and otherwise 
> use an incremented number, same as the FSL i2c driver does now:
> 
> http://ozlabs.org/pipermail/linuxppc-dev/2008-June/057254.html
> 
> If nobody objects I'll send a patch to add the cell-index to all 4xx dts files 
> in a short while.

I have no large objections.  Though as Scott pointed out, this isn't
really a proper use of "cell-index".  Something like:

"linux,i2c-index"

seems to be a more distinct definition of what this is.  But I have no
idea how well that would go over, and it would probably need to be
changed in all the fsl boards as well.

josh

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Josh Boyer wrote:

> From a device tree perspective, index and cell-index are both
> incorrect.  The IIC macros don't share register blocks with anything,
> are enumerated as unique instances per macro in the device tree, and
> should be able to be distinguished by "regs" and/or unit address.

I think we should just expand the definition of cell-index to include standard
device enumeration for when it's needed.  The original definition is too
limited, IMHO.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Stefan Roese wrote:

> So what should we do now? Currently I2C doesn't work at all on 4xx since the 
> driver expects the "index" property and no dts sets this property. Personally 
> I would like to move to using cell-index here, since this seems to be more 
> common. But I could also life with removing the index property and using 
> the "static index" if this is preferred and/or acceptable.

My opinion:

In situations where it doesn't matter which I2C bus is #1 and which one is #2,
then I think the code should just initialize idx based on the order the nodes
are found in the tree.

In situations where it does matter, then we should use cell-index.

The patch I posted ("Update fsl_soc to use cell-index property of I2C nodes")
does both.  If the cell-index property is present, then its value is used in the
call to platform_device_register_simple().  Otherwise, it just keeps count of
each node, and uses that count.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Josh Boyer wrote:

> seems to be a more distinct definition of what this is.  But I have no
> idea how well that would go over, and it would probably need to be
> changed in all the fsl boards as well.

Which would end up breaking backwards compatibility with older device trees.
Like I said earlier, I'm in favor of expanding the definition of "cell-index".

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Grant Likely]

On Thu, Jun 5, 2008 at 9:13 AM, Timur Tabi <timur@freescale.com> wrote:
> Josh Boyer wrote:
>
>> From a device tree perspective, index and cell-index are both
>> incorrect.  The IIC macros don't share register blocks with anything,
>> are enumerated as unique instances per macro in the device tree, and
>> should be able to be distinguished by "regs" and/or unit address.
>
> I think we should just expand the definition of cell-index to include standard
> device enumeration for when it's needed.  The original definition is too
> limited, IMHO.

nak

if you need explicit indexing then use an alias.  My opinion however
is that explicit indexing is unnecessary and is just an artifact of
current i2c subsystem internals.  There is already enough information
in the device tree to match i2c devices with i2c busses without
resorting to indexes.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Grant Likely wrote:

> if you need explicit indexing then use an alias.  My opinion however
> is that explicit indexing is unnecessary and is just an artifact of
> current i2c subsystem internals.  There is already enough information
> in the device tree to match i2c devices with i2c busses without
> resorting to indexes.

Not for ALSA SoC V2 devices.  In ASoC V2, the "fabric" driver needs to identify
the codec by its specific I2C bus and address number.  The codec driver is not
an OF driver (normally), so it doesn't have access to any OF data.  It's just an
I2C driver, so its given an I2C address and some number that represents an adapter.

Therefore, the fabric driver and the codec driver need to independently
determine the I2C bus number, and they need to match.  The fabric driver parses
the OF tree and looks up the cell-index property.  The codec driver uses the
adapter->nr variable.  The patch I posted ensures that the two contain the same
number.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Jochen Friedrich]

Hi Timur,

> In situations where it doesn't matter which I2C bus is #1 and which one is #2,
> then I think the code should just initialize idx based on the order the nodes
> are found in the tree.
> 
> In situations where it does matter, then we should use cell-index.

that's what I did in i2c-cpm, as well. However, here I use the property
"linux,i2c-index" instead (see http://patchwork.ozlabs.org/linuxppc/patch?id=18603).

Thanks,
Jochen

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Segher Boessenkool]

>> I think we should just expand the definition of cell-index to include 
>> standard
>> device enumeration for when it's needed.  The original definition is 
>> too
>> limited, IMHO.
>
> nak
>
> if you need explicit indexing then use an alias.  My opinion however
> is that explicit indexing is unnecessary and is just an artifact of
> current i2c subsystem internals.  There is already enough information
> in the device tree to match i2c devices with i2c busses without
> resorting to indexes.

Fully agreed.  Let me say it as well, it makes me feel powerful:

NAK


Segher

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Jochen Friedrich wrote:
> Hi Timur,
> 
>> In situations where it doesn't matter which I2C bus is #1 and which one is #2,
>> then I think the code should just initialize idx based on the order the nodes
>> are found in the tree.
>>
>> In situations where it does matter, then we should use cell-index.
> 
> that's what I did in i2c-cpm, as well. However, here I use the property
> "linux,i2c-index" instead (see http://patchwork.ozlabs.org/linuxppc/patch?id=18603).

Well, I just don't see the point of having two different properties that say the
same thing.  I'm not an IEE 1275 purist, so I don't think we should be hampered
by old node definitions.  I especially don't like having a property specifically
for indexing I2C nodes that can't be used to enumerate other nodes.

The DMA and SSI controllers on Freescale parts use cell-index to enumerate them.
 It just seems dumb to invent a new property.

Will there ever be a situation where a node will contain "cell-index" and
"linux,i2c-index"?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Jochen Friedrich]

Hi Grant,

> if you need explicit indexing then use an alias.  My opinion however
> is that explicit indexing is unnecessary and is just an artifact of
> current i2c subsystem internals.  There is already enough information
> in the device tree to match i2c devices with i2c busses without
> resorting to indexes.

True. However, there are currently drivers which need platform data for
its initialisation (like drivers/gpio/pca953x.c). Unless these drivers
are rewritten, they can't be loaded just by parsing the device tree, so
they must be loaded from platform init code and here the adapter index
is needed to attach the driver.

Thanks,
Jochen

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Segher Boessenkool]

>> if you need explicit indexing then use an alias.  My opinion however
>> is that explicit indexing is unnecessary and is just an artifact of
>> current i2c subsystem internals.  There is already enough information
>> in the device tree to match i2c devices with i2c busses without
>> resorting to indexes.
>
> Not for ALSA SoC V2 devices.  In ASoC V2, the "fabric" driver needs to 
> identify
> the codec by its specific I2C bus and address number.  The codec 
> driver is not
> an OF driver (normally), so it doesn't have access to any OF data.  
> It's just an
> I2C driver, so its given an I2C address and some number that 
> represents an adapter.
>
> Therefore, the fabric driver and the codec driver need to independently
> determine the I2C bus number, and they need to match.  The fabric 
> driver parses
> the OF tree and looks up the cell-index property.  The codec driver 
> uses the
> adapter->nr variable.  The patch I posted ensures that the two contain 
> the same
> number.

Sounds to me like both simply need to use adapter->nr.  For access to
Linux-internal data structures (and that is what this "index" is), you
shouldn't have to go via the device tree.  If the Linux data structures
do not have the information you need, well, fix that.


Segher

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Grant Likely]

On Thu, Jun 5, 2008 at 9:52 AM, Jochen Friedrich <jochen@scram.de> wrote:
> Hi Grant,
>
>> if you need explicit indexing then use an alias.  My opinion however
>> is that explicit indexing is unnecessary and is just an artifact of
>> current i2c subsystem internals.  There is already enough information
>> in the device tree to match i2c devices with i2c busses without
>> resorting to indexes.
>
> True. However, there are currently drivers which need platform data for
> its initialisation (like drivers/gpio/pca953x.c). Unless these drivers
> are rewritten, they can't be loaded just by parsing the device tree, so
> they must be loaded from platform init code and here the adapter index
> is needed to attach the driver.

... which means that platform code is responsible to figure it out.
Don't encode it into the device tree.

Besides, this is just a binding.  I see absolutely no problem adding
probe code to the bindings to extract data from the device tree.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Grant Likely]

On Thu, Jun 5, 2008 at 9:43 AM, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
>> if you need explicit indexing then use an alias.  My opinion however
>> is that explicit indexing is unnecessary and is just an artifact of
>> current i2c subsystem internals.  There is already enough information
>> in the device tree to match i2c devices with i2c busses without
>> resorting to indexes.
>
> Not for ALSA SoC V2 devices.  In ASoC V2, the "fabric" driver needs to identify
> the codec by its specific I2C bus and address number.  The codec driver is not
> an OF driver (normally), so it doesn't have access to any OF data.  It's just an
> I2C driver, so its given an I2C address and some number that represents an adapter.
>
> Therefore, the fabric driver and the codec driver need to independently
> determine the I2C bus number, and they need to match.  The fabric driver parses
> the OF tree and looks up the cell-index property.  The codec driver uses the
> adapter->nr variable.  The patch I posted ensures that the two contain the same
> number.

That is still Linux internal artifacts leaking out.  Don't encode that
data into the device tree.

Besides, the whole thing looks bass ackwards and doesn't match the
Linux driver model.  'busses' should register 'devices'.  'drivers'
should then bind to those 'devices'.  Where ASoC is concerned, the i2c
bus should register its own child devices, which includes the codecs,
and an ASoC codec driver should bind against it.  The fabric driver
should then use the codec's exported API.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Segher Boessenkool wrote:

> Sounds to me like both simply need to use adapter->nr. 

How can a non-I2C driver get the adapter structure for another driver that is an
I2C driver?

> For access to
> Linux-internal data structures (and that is what this "index" is), you
> shouldn't have to go via the device tree.  If the Linux data structures
> do not have the information you need, well, fix that.

The fabric driver doesn't have access to any I2C structures when it starts
looking for the codec driver.  The fabric driver is like an OF platform driver,
in that it's OF-aware and machine-specific.  By parsing the device tree (which
is the only tool I have to know how the board is laid out), the fabric driver
can determine that SSI1 is attached to a CS4270 on I2C bus #1, address 0x4F.
That's the only information it has.

The CS4270 driver, on the other hand, knows nothing about OF.  It just knows
whatever is passed to its I2C probe function, i.e. the i2c_adapter and
i2c_client structures.

So in order for the two drivers to be able to link to each other, there needs to
be some code that takes data from the device tree and places it into the I2C
data structures.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Grant Likely]

On Thu, Jun 5, 2008 at 9:50 AM, Timur Tabi <timur@freescale.com> wrote:
> Jochen Friedrich wrote:
>> Hi Timur,
>>
>>> In situations where it doesn't matter which I2C bus is #1 and which one is #2,
>>> then I think the code should just initialize idx based on the order the nodes
>>> are found in the tree.
>>>
>>> In situations where it does matter, then we should use cell-index.
>>
>> that's what I did in i2c-cpm, as well. However, here I use the property
>> "linux,i2c-index" instead (see http://patchwork.ozlabs.org/linuxppc/patch?id=18603).
>
> Well, I just don't see the point of having two different properties that say the
> same thing.  I'm not an IEE 1275 purist, so I don't think we should be hampered
> by old node definitions.  I especially don't like having a property specifically
> for indexing I2C nodes that can't be used to enumerate other nodes.
>
> The DMA and SSI controllers on Freescale parts use cell-index to enumerate them.
>  It just seems dumb to invent a new property.
>
> Will there ever be a situation where a node will contain "cell-index" and
> "linux,i2c-index"?

You are trying to describe 2 different things.  cell-index is purely
for identifying multiple devices within a silicon block that share
resources.  Indexing devices has a very different scope.  The whole
scheme breaks the moment you put down 2 identical multifunction
peripherals into the same system.  If the chip has multiple devices
that share resources, and those resources are described with cell
index; then you'll get something like this (notice how cell-index
values are duplicated):

multifunction@0 {
        #size-cells = <1>;
        #address-cells = <1>;
        ranges = <0 0xe00000000 0x1000>;
        i2c@0 {
                cell-index = <0>;
                regs = <0 0x100>;
        }
        i2c@100 {
                cell-index = <1>;
                regs = <0x100 0x100>;
        }
}
multifunction@1 {
        #size-cells = <1>;
        #address-cells = <1>;
        ranges = <0 0xe10000000 0x1000>;
        i2c@0 {
                cell-index = <0>;
                regs = <0 0x100>;
        }
        i2c@100 {
                cell-index = <1>;
                regs = <0x100 0x100>;
        }
}

cell-index must *not* be repurposed as a system level index.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Grant Likely wrote:

> That is still Linux internal artifacts leaking out.  Don't encode that
> data into the device tree.

The I2C bus number is *not* an internal artifact.  On Freescale parts, the one
I2C adapter is specifically designated I2C1, and the 2nd one is specifically
designated I2C2.  This is part of the silicon, and so the device tree should
specify it.

> Besides, the whole thing looks bass ackwards and doesn't match the
> Linux driver model.  'busses' should register 'devices'.  'drivers'
> should then bind to those 'devices'.  Where ASoC is concerned, the i2c
> bus should register its own child devices, which includes the codecs,
> and an ASoC codec driver should bind against it.  The fabric driver
> should then use the codec's exported API.

Sorry, I don't really understand that.  The codec driver is just an I2C driver
that registers callbacks with ASoC.  The fabric driver does not know what codec
driver is present.  In many cases, there could be multiple codecs.  Sometimes
they're the same type of codec, just at different I2C addresses.  Sometimes,
they're completely different codecs.  So I could have any number of codec
drivers loaded.  How would I know which exported API to call?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Grant Likely wrote:

> multifunction@0 {
>         #size-cells = <1>;
>         #address-cells = <1>;
>         ranges = <0 0xe00000000 0x1000>;
>         i2c@0 {
>                 cell-index = <0>;
>                 regs = <0 0x100>;
>         }
>         i2c@100 {
>                 cell-index = <1>;
>                 regs = <0x100 0x100>;
>         }
> }
> multifunction@1 {
>         #size-cells = <1>;
>         #address-cells = <1>;
>         ranges = <0 0xe10000000 0x1000>;
>         i2c@0 {
>                 cell-index = <0>;
>                 regs = <0 0x100>;
>         }
>         i2c@100 {
>                 cell-index = <1>;
>                 regs = <0x100 0x100>;
>         }
> }

What resources are being shared in this example?  Each I2C device has its own
address ranges.  I don't see how cell-index provides any useful info here.

> cell-index must *not* be repurposed as a system level index.

It's a little late for that.  I'm okay with coming up with a new property to
provide system-level indexing, but it needs to be the same property name for
each type of device.  I don't want linux,i2c-index and linux,dma-index and
linux,ssi-index, etc.  I also don't understand why we need the linux, prefix,
since device enumeration is not specific to Linux.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Josh Boyer]

On Thu, 05 Jun 2008 11:13:23 -0500
Timur Tabi <timur@freescale.com> wrote:

> Grant Likely wrote:
> 
> > That is still Linux internal artifacts leaking out.  Don't encode that
> > data into the device tree.
> 
> The I2C bus number is *not* an internal artifact.  On Freescale parts, the one
> I2C adapter is specifically designated I2C1, and the 2nd one is specifically
> designated I2C2.  This is part of the silicon, and so the device tree should
> specify it.

And it does.  It does so by the unique "regs" properties and
unit-names.  You can assign the index that the i2c subsystem needs
based on probe order, like I already said.

I don't know why Jean doesn't like that.  It's not a made up number.

josh

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Jochen Friedrich]

Hi Timur,

> It's a little late for that.  I'm okay with coming up with a new property to
> provide system-level indexing, but it needs to be the same property name for
> each type of device.  I don't want linux,i2c-index and linux,dma-index and
> linux,ssi-index, etc.  I also don't understand why we need the linux, prefix,
> since device enumeration is not specific to Linux.

Full ACK here.

Thanks,
Jochen

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Josh Boyer wrote:

> And it does.  It does so by the unique "regs" properties and
> unit-names.  You can assign the index that the i2c subsystem needs
> based on probe order, like I already said.

The probe order is not sufficient on platforms that specifically enumerate their
I2C (or whatever) devices.  For instance, in order to do audio playback on an
MPC8610, SSI1 needs to use DMA channel 0.  The SSI driver specifically needs to
find the register addresses for DMA channel 0.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Scott Wood]

On Thu, Jun 05, 2008 at 11:09:16AM -0500, Timur Tabi wrote:
> The fabric driver doesn't have access to any I2C structures when it starts
> looking for the codec driver.  The fabric driver is like an OF platform driver,
> in that it's OF-aware and machine-specific.  By parsing the device tree (which
> is the only tool I have to know how the board is laid out), the fabric driver
> can determine that SSI1 is attached to a CS4270 on I2C bus #1, address 0x4F.
> That's the only information it has.

No, it's not.  It can determine that it's at address 0x4f on the i2c bus
at 0xe0003100.  This is exactly how the ethernet phy lookup is done.

-Scott

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Grant Likely]

On Thu, Jun 5, 2008 at 10:22 AM, Jochen Friedrich <jochen@scram.de> wrote:
> Hi Timur,
>
>> It's a little late for that.  I'm okay with coming up with a new property to
>> provide system-level indexing, but it needs to be the same property name for
>> each type of device.  I don't want linux,i2c-index and linux,dma-index and
>> linux,ssi-index, etc.  I also don't understand why we need the linux, prefix,
>> since device enumeration is not specific to Linux.
>
> Full ACK here.

NAK because there is already a mechanism that does what you want.  Its
called the aliases node.

For the record, I'm making 2 arguments here:

1) if you *do* need an enumerated index then use the aliases node.
You don't need to invent a new property
2) for i2c purposes, explicit enumeration is not needed or desired.
All the necessary data is already present in the device tree in that
i2c device nodes are children of i2c bus nodes.  The i2c bus numbers
should be dynamically assigned.

Cheers,
g.


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Grant Likely]

On Thu, Jun 5, 2008 at 10:18 AM, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
>> multifunction@0 {
>>         #size-cells = <1>;
>>         #address-cells = <1>;
>>         ranges = <0 0xe00000000 0x1000>;
>>         i2c@0 {
>>                 cell-index = <0>;
>>                 regs = <0 0x100>;
>>         }
>>         i2c@100 {
>>                 cell-index = <1>;
>>                 regs = <0x100 0x100>;
>>         }
>> }
>> multifunction@1 {
>>         #size-cells = <1>;
>>         #address-cells = <1>;
>>         ranges = <0 0xe10000000 0x1000>;
>>         i2c@0 {
>>                 cell-index = <0>;
>>                 regs = <0 0x100>;
>>         }
>>         i2c@100 {
>>                 cell-index = <1>;
>>                 regs = <0x100 0x100>;
>>         }
>> }
>
> What resources are being shared in this example?  Each I2C device has its own
> address ranges.  I don't see how cell-index provides any useful info here.

As I said; *assume* that the i2c devices have shared resources.  I
didn't explicitly show them in the example, but assume that there are
shared registers in the multifunction nodes.

cell-index provides details about which bits in the shared registers
belong to the device, but since there are 2 identical multifunction
devices in the system there are 2 sets of shared regs.  You cannot now
use the values 0, 1, 2 and 3 for each cell index because '2' and '3'
have no valid meaning on how to reference the shared resource.  So,
cell-index isn't useful for enumerating the i2c busses at the system
level.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Grant Likely]

On Thu, Jun 5, 2008 at 10:25 AM, Timur Tabi <timur@freescale.com> wrote:
> Josh Boyer wrote:
>
>> And it does.  It does so by the unique "regs" properties and
>> unit-names.  You can assign the index that the i2c subsystem needs
>> based on probe order, like I already said.
>
> The probe order is not sufficient on platforms that specifically enumerate their
> I2C (or whatever) devices.  For instance, in order to do audio playback on an
> MPC8610, SSI1 needs to use DMA channel 0.  The SSI driver specifically needs to
> find the register addresses for DMA channel 0.

then you use a phandle for that; but that entirely different from bus
enumeration because it describes a real property of the hardware.

Cheers,
g.

>
> --
> Timur Tabi
> Linux kernel developer at Freescale
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Grant Likely wrote:

> 2) for i2c purposes, explicit enumeration is not needed or desired.
> All the necessary data is already present in the device tree in that
> i2c device nodes are children of i2c bus nodes.  The i2c bus numbers
> should be dynamically assigned.

NACK.  For ASoC driver, they cannot be dynamically assigned.  I2C1 must be
labeled as such.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Scott Wood wrote:

> No, it's not.  It can determine that it's at address 0x4f on the i2c bus
> at 0xe0003100.  This is exactly how the ethernet phy lookup is done.

But how does the fabric driver know whether e0003100 is I2C1 or I2C2?

And how does the codec driver, which sees only I2C information, know that it's
at e0003100?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Scott Wood]

Timur Tabi wrote:
> Scott Wood wrote:
> 
>> No, it's not.  It can determine that it's at address 0x4f on the i2c bus
>> at 0xe0003100.  This is exactly how the ethernet phy lookup is done.
> 
> But how does the fabric driver know whether e0003100 is I2C1 or I2C2?

It shouldn't have to care.

> And how does the codec driver, which sees only I2C information, know that it's
> at e0003100?

This is an internal communications failure within the i2c layer, not 
something that warrants expression in the device tree.  Some solutions, 
in increasing order of desirability, are:

1. Assign the I2C bus number based on the adapter's register offset.
2. Let the adapter provide a more helpful bus_id than a Linux-assigned 
index.
3. Create a way to look up an I2C device by its OF node.

-Scott

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Josh Boyer]

On Thu, 05 Jun 2008 11:25:23 -0500
Timur Tabi <timur@freescale.com> wrote:

> Josh Boyer wrote:
> 
> > And it does.  It does so by the unique "regs" properties and
> > unit-names.  You can assign the index that the i2c subsystem needs
> > based on probe order, like I already said.
> 
> The probe order is not sufficient on platforms that specifically enumerate their
> I2C (or whatever) devices.  For instance, in order to do audio playback on an
> MPC8610, SSI1 needs to use DMA channel 0.  The SSI driver specifically needs to
> find the register addresses for DMA channel 0.

I don't understand this statement.  Are your I2C macros hot-pluggable?
Can you dynamically add/remove an I2C engine on your hardware somehow?
Are you mucking about with the DTB and randomly moving around the I2C
node blobs so they probe order differs from boot to boot?

If not, then the probe order will be static for every boot.  You can
assign the index using a static int that is incremented after each node
is discovered and the ordering of the devices will never change.  Can
you explain why something like that isn't possible or sufficient?

(And I'm talking about I2C, not DMA.  I don't care about DMA because
this conversation will go off into the weeds if we start talking about
cell-index and every possible device out there.)

josh

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Josh Boyer wrote:

> I don't understand this statement.  Are your I2C macros hot-pluggable?
> Can you dynamically add/remove an I2C engine on your hardware somehow?
> Are you mucking about with the DTB and randomly moving around the I2C
> node blobs so they probe order differs from boot to boot?
> 
> If not, then the probe order will be static for every boot. 

You're making two assumptions:

1) That every I2C adapter will be present in the device tree.  Some device trees
don't list I2C adapters if there are no devices on them.
2) That the nodes in the device tree are put in order

Besides, let's say I have a pointer to a specific I2C device node in the tree.
How do I find out the bus number it's on?  With my way, it's easy:

iprop = of_get_property(of_get_parent(codec_np), "cell-index", NULL);
bus = *iprop;

With your way, I'll need to scan the entire device tree, poking inside each I2C
adapter node looking for my I2C device node while keeping track of the I2C adapters.

> You can
> assign the index 

Assign it to where?  When the fabric driver goes to find codec nodes, the only
information it has is the device tree.  So when I assign this index to some
random variable that you're talking about, my fabric driver will know nothing
about that.

> using a static int that is incremented after each node
> is discovered and the ordering of the devices will never change.  Can
> you explain why something like that isn't possible or sufficient?

Yes, I just did.

> (And I'm talking about I2C, not DMA.  I don't care about DMA because
> this conversation will go off into the weeds if we start talking about
> cell-index and every possible device out there.)

But we are talking about every device.  It's the same problem for every device.
 Making this problem I2C-specific is not going to solve anything.


-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Josh Boyer]

On Thu, 05 Jun 2008 13:35:18 -0500
Timur Tabi <timur@freescale.com> wrote:

> Josh Boyer wrote:
> 
> > I don't understand this statement.  Are your I2C macros hot-pluggable?
> > Can you dynamically add/remove an I2C engine on your hardware somehow?
> > Are you mucking about with the DTB and randomly moving around the I2C
> > node blobs so they probe order differs from boot to boot?
> > 
> > If not, then the probe order will be static for every boot. 
> 
> You're making two assumptions:
> 
> 1) That every I2C adapter will be present in the device tree.  Some device trees
> don't list I2C adapters if there are no devices on them.

Which isn't a problem.

> 2) That the nodes in the device tree are put in order

Which isn't a problem.

> Besides, let's say I have a pointer to a specific I2C device node in the tree.
> How do I find out the bus number it's on?  With my way, it's easy:
> 
> iprop = of_get_property(of_get_parent(codec_np), "cell-index", NULL);
> bus = *iprop;
> 
> With your way, I'll need to scan the entire device tree, poking inside each I2C
> adapter node looking for my I2C device node while keeping track of the I2C adapters.
> 
> > You can
> > assign the index 
> 
> Assign it to where?  When the fabric driver goes to find codec nodes, the only
> information it has is the device tree.  So when I assign this index to some
> random variable that you're talking about, my fabric driver will know nothing
> about that.

Your driver is sufficiently different then.  The i2c-ibm_iic driver
keeps an index property in it's private device structure.

> > using a static int that is incremented after each node
> > is discovered and the ordering of the devices will never change.  Can
> > you explain why something like that isn't possible or sufficient?
> 
> Yes, I just did.

Sort of.

> > (And I'm talking about I2C, not DMA.  I don't care about DMA because
> > this conversation will go off into the weeds if we start talking about
> > cell-index and every possible device out there.)
> 
> But we are talking about every device.  It's the same problem for every device.
>  Making this problem I2C-specific is not going to solve anything.

I've been driven to the point of not caring anymore.

If you need a simple index property, then maybe Sean was right to just
call the damn thing "index".  Overloading "cell-index", which already
has a specific semantic associated with it, seems odd to me.

josh

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Grant Likely]

On Thu, Jun 5, 2008 at 12:27 PM, Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> On Thu, 05 Jun 2008 11:25:23 -0500
> Timur Tabi <timur@freescale.com> wrote:
>
>> Josh Boyer wrote:
>>
>> > And it does.  It does so by the unique "regs" properties and
>> > unit-names.  You can assign the index that the i2c subsystem needs
>> > based on probe order, like I already said.
>>
>> The probe order is not sufficient on platforms that specifically enumerate their
>> I2C (or whatever) devices.  For instance, in order to do audio playback on an
>> MPC8610, SSI1 needs to use DMA channel 0.  The SSI driver specifically needs to
>> find the register addresses for DMA channel 0.
>
> I don't understand this statement.  Are your I2C macros hot-pluggable?
> Can you dynamically add/remove an I2C engine on your hardware somehow?
> Are you mucking about with the DTB and randomly moving around the I2C
> node blobs so they probe order differs from boot to boot?
>
> If not, then the probe order will be static for every boot.  You can
> assign the index using a static int that is incremented after each node
> is discovered and the ordering of the devices will never change.  Can
> you explain why something like that isn't possible or sufficient?
>
> (And I'm talking about I2C, not DMA.  I don't care about DMA because
> this conversation will go off into the weeds if we start talking about
> cell-index and every possible device out there.)

I need to disagree here.  Behavior should never be dependent on device
tree order.  It should be absolutely fine for devices to be probed in
a different order and different bus ids to be assigned.

In Timur's case, it is absolutely appropriate to use cell-index and/or
a phandle to make sure it gets the correct DMA registers (which is
what cell-index is intended to solve).  It is not appropriate to
depend on that same number to also be the logical i2c bus number.

Cheers,
g.
>
> josh
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Josh Boyer]

On Thu, 5 Jun 2008 12:46:39 -0600
"Grant Likely" <grant.likely@secretlab.ca> wrote:

> > (And I'm talking about I2C, not DMA.  I don't care about DMA because
> > this conversation will go off into the weeds if we start talking about
> > cell-index and every possible device out there.)
> 
> I need to disagree here.  Behavior should never be dependent on device
> tree order.  It should be absolutely fine for devices to be probed in
> a different order and different bus ids to be assigned.

Meh.  I'll begrudgingly agree.

> In Timur's case, it is absolutely appropriate to use cell-index and/or
> a phandle to make sure it gets the correct DMA registers (which is
> what cell-index is intended to solve).  It is not appropriate to
> depend on that same number to also be the logical i2c bus number.

Hence "index" would be a better fit for the latter then, yes?

josh

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Grant Likely]

On Thu, Jun 5, 2008 at 12:56 PM, Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> On Thu, 5 Jun 2008 12:46:39 -0600
> "Grant Likely" <grant.likely@secretlab.ca> wrote:
>
>> In Timur's case, it is absolutely appropriate to use cell-index and/or
>> a phandle to make sure it gets the correct DMA registers (which is
>> what cell-index is intended to solve).  It is not appropriate to
>> depend on that same number to also be the logical i2c bus number.
>
> Hence "index" would be a better fit for the latter then, yes?
>

No; use an alias in the aliases node.  That is what aliases is designed
for.  Something like 'index' is a reinvention of the wheel.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Josh Boyer]

On Thu, 5 Jun 2008 13:14:00 -0600
"Grant Likely" <grant.likely@secretlab.ca> wrote:

> On Thu, Jun 5, 2008 at 12:56 PM, Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> > On Thu, 5 Jun 2008 12:46:39 -0600
> > "Grant Likely" <grant.likely@secretlab.ca> wrote:
> >
> >> In Timur's case, it is absolutely appropriate to use cell-index and/or
> >> a phandle to make sure it gets the correct DMA registers (which is
> >> what cell-index is intended to solve).  It is not appropriate to
> >> depend on that same number to also be the logical i2c bus number.
> >
> > Hence "index" would be a better fit for the latter then, yes?
> >
> 
> No; use an alias in the aliases node.  That is what aliases is designed
> for.  Something like 'index' is a reinvention of the wheel.

Ah.  Good point.

josh

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Grant Likely wrote:

> No; use an alias in the aliases node.  That is what aliases is designed
> for.  Something like 'index' is a reinvention of the wheel.

Do aliases work in reverse?  That is, if I have a pointer to a device node, can
I look up its alias directly?  Or do I have to scan the aliases node and do a
comparison of each phandle, one at a time, until I find a match?  And when I
find a match, will I need to do sscanf() in order to extract the actual index
value from the property?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Segher Boessenkool]

>> No; use an alias in the aliases node.  That is what aliases is 
>> designed
>> for.  Something like 'index' is a reinvention of the wheel.
>
> Do aliases work in reverse?  That is, if I have a pointer to a device 
> node, can
> I look up its alias directly?  Or do I have to scan the aliases node 
> and do a
> comparison of each phandle, one at a time, until I find a match?  And 
> when I
> find a match, will I need to do sscanf() in order to extract the 
> actual index
> value from the property?

Aliases are one-way.  You can have multiple aliases point to the same 
node,
as well (and that is quite common, even).

If you need a unique identifier for an OF node, use its phandle.
It sounds to me like you just need to set up a mapping between
phandles and Linux i2c bus ids here?


Segher

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Sean MacLennan]

On Thu, 5 Jun 2008 08:22:00 +0200
"Stefan Roese" <sr@denx.de> wrote:

> So what should we do now? Currently I2C doesn't work at all on 4xx
> since the driver expects the "index" property and no dts sets this
> property. Personally I would like to move to using cell-index here,
> since this seems to be more common. But I could also life with
> removing the index property and using the "static index" if this is
> preferred and/or acceptable.

The warp DTS does ;)

Cheers,
   Sean

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Sean MacLennan]

On Thu, 5 Jun 2008 13:14:00 -0600
"Grant Likely" <grant.likely@secretlab.ca> wrote:

> No; use an alias in the aliases node.  That is what aliases is
> designed for.  Something like 'index' is a reinvention of the wheel.

If we really want to get rid of the index, I like the alias method. I
mainly write drivers, so I don't know register addresses. I just read
the spec and it says device A is on i2c bus 2. That is why I went with
the index. Calling get_i2c_apapter(2) makes sense.

However, calling get_i2c_adapter("IIC2"), just as a made up example,
makes just as much sense.

Cheers,
   Sean

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[David Gibson]

On Thu, Jun 05, 2008 at 11:40:37AM -0500, Timur Tabi wrote:
> Grant Likely wrote:
> 
> > 2) for i2c purposes, explicit enumeration is not needed or desired.
> > All the necessary data is already present in the device tree in that
> > i2c device nodes are children of i2c bus nodes.  The i2c bus numbers
> > should be dynamically assigned.
> 
> NACK.  For ASoC driver, they cannot be dynamically assigned.  I2C1 must be
> labeled as such.

So use aliases, this is exactly what they're for.  How many times do
Grant and I have to say it?

A fallback to cell-index and/or index is acceptable, for easier
compatibility with existing trees, but aliases must be the primary
method for assigning system-wide numbers.


-- 
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: "cell-index" vs. "index" vs. no index in I2C device nodes

[David Gibson]

On Thu, Jun 05, 2008 at 06:52:25AM -0500, Josh Boyer wrote:
> On Thu, 5 Jun 2008 10:45:42 +0200
> Stefan Roese <sr@denx.de> wrote:
> 
> > On Thursday 05 June 2008, Jean Delvare wrote:
> > > > > Maybe it is time to remove the index, or maybe we should go back to
> > > > > using both a static and the index. But at the time we decided to
> > > > > enforce an index.
> > > >
> > > > So what should we do now? Currently I2C doesn't work at all on 4xx since
> > > > the driver expects the "index" property and no dts sets this property.
> > > > Personally I would like to move to using cell-index here, since this
> > > > seems to be more common. But I could also life with removing the index
> > > > property and using the "static index" if this is preferred and/or
> > > > acceptable.
> > > >
> > > > Please advise. Thanks.
> > >
> > > As far as I am concerned, it's really up to the maintainers and users
> > > of this platform. All I am asking for is that you do not call
> > > i2c_add_numbered_adapter() on an adapter with an automatically
> > > generated number. This function must only be used for adapter's those
> > > number is well defined. If an adapter doesn't have a well-defined
> > > number then use i2c_add_adapter() (but then you can no longer declare
> > > your I2C devices as part of the platform data.)
> > 
> > Full ack from me. So I suggest to use "cell-index" if available and otherwise 
> > use an incremented number, same as the FSL i2c driver does now:
> > 
> > http://ozlabs.org/pipermail/linuxppc-dev/2008-June/057254.html
> > 
> > If nobody objects I'll send a patch to add the cell-index to all 4xx dts files 
> > in a short while.
> 
> I have no large objections.  Though as Scott pointed out, this isn't
> really a proper use of "cell-index".  Something like:
> 
> "linux,i2c-index"
> 
> seems to be a more distinct definition of what this is.  But I have no
> idea how well that would go over, and it would probably need to be
> changed in all the fsl boards as well.

Don't do that.  Use aliases, that's what they're for.

-- 
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: "cell-index" vs. "index" vs. no index in I2C device nodes

[David Gibson]

On Thu, Jun 05, 2008 at 02:16:41PM -0500, Timur Tabi wrote:
> Grant Likely wrote:
> 
> > No; use an alias in the aliases node.  That is what aliases is designed
> > for.  Something like 'index' is a reinvention of the wheel.
> 
> Do aliases work in reverse?  That is, if I have a pointer to a
> device node, can I look up its alias directly?  Or do I have to scan
> the aliases node and do a comparison of each phandle, one at a time,
> until I find a match?  And when I find a match, will I need to do
> sscanf() in order to extract the actual index value from the
> property?

Aliases aren't trivially reversible, but it shouldn't be too hard to
write a helper function which will do the scan and parse you describe.

-- 
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: "cell-index" vs. "index" vs. no index in I2C device nodes

[Josh Boyer]

On Thu, 5 Jun 2008 17:37:16 -0400
Sean MacLennan <smaclennan@pikatech.com> wrote:

> On Thu, 5 Jun 2008 08:22:00 +0200
> "Stefan Roese" <sr@denx.de> wrote:
> 
> > So what should we do now? Currently I2C doesn't work at all on 4xx
> > since the driver expects the "index" property and no dts sets this
> > property. Personally I would like to move to using cell-index here,
> > since this seems to be more common. But I could also life with
> > removing the index property and using the "static index" if this is
> > preferred and/or acceptable.
> 
> The warp DTS does ;)

I can't possibly see how.  It lacks the "index" property.

josh
> 
> Cheers,
>    Sean

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Josh Boyer]

On Thu, 05 Jun 2008 10:50:20 -0500
Timur Tabi <timur@freescale.com> wrote:

> Jochen Friedrich wrote:
> > Hi Timur,
> > 
> >> In situations where it doesn't matter which I2C bus is #1 and which one is #2,
> >> then I think the code should just initialize idx based on the order the nodes
> >> are found in the tree.
> >>
> >> In situations where it does matter, then we should use cell-index.
> > 
> > that's what I did in i2c-cpm, as well. However, here I use the property
> > "linux,i2c-index" instead (see http://patchwork.ozlabs.org/linuxppc/patch?id=18603).
> 
> Well, I just don't see the point of having two different properties that say the
> same thing.  I'm not an IEE 1275 purist, so I don't think we should be hampered
> by old node definitions.  I especially don't like having a property specifically
> for indexing I2C nodes that can't be used to enumerate other nodes.

It's not about purity.  It's about overloading something that has
existing semantics just because you have one usecase that you _think_
needs it.

If everyone did that, this whole device tree concept is going to just
be one big cluster f*ck.  Get over it, fix your driver to use the
device model and aliases propertly, and move on.

josh

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Segher Boessenkool]

>>> From a device tree perspective, index and cell-index are both
>> incorrect.  The IIC macros don't share register blocks with anything,
>> are enumerated as unique instances per macro in the device tree, and
>> should be able to be distinguished by "regs" and/or unit address.
>>
>> Does anyone disagree with that?
>
> Hear, hear.

x2.

> Aliases can also provide a reasonable way of enumerating devices, if
> "reg" isn't suitable on its own.

Yes.  In almost all cases, drivers and subsystems do not need this at
all though.  In OF, one device points to another by putting the phandle
of that pointed-to device in some property (and buses are represented
by their parent bridge).  If the Linux subsystem wants to use an integer
for distinguishing between its various buses, that's fine, but it can
just make up these numbers -- the numbers themselves have no actual
meaning, only the relationships expressed via those numbers do.

In a few cases, particularly where those numbers are user-visible, like
in ethN, the aliases construct is a good solution.  If a 
driver/subsystem
is relying on the aliases though, it should document this in a 
(platform?)
binding -- and it would better have a very good reason for it!


Segher

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[David Gibson]

On Fri, Jun 06, 2008 at 04:40:20AM +0200, Segher Boessenkool wrote:
>>>> From a device tree perspective, index and cell-index are both
>>> incorrect.  The IIC macros don't share register blocks with anything,
>>> are enumerated as unique instances per macro in the device tree, and
>>> should be able to be distinguished by "regs" and/or unit address.
>>>
>>> Does anyone disagree with that?
>>
>> Hear, hear.
>
> x2.
>
>> Aliases can also provide a reasonable way of enumerating devices, if
>> "reg" isn't suitable on its own.
>
> Yes.  In almost all cases, drivers and subsystems do not need this at
> all though.  In OF, one device points to another by putting the phandle
> of that pointed-to device in some property (and buses are represented
> by their parent bridge).  If the Linux subsystem wants to use an integer
> for distinguishing between its various buses, that's fine, but it can
> just make up these numbers -- the numbers themselves have no actual
> meaning, only the relationships expressed via those numbers do.

That's true.  However if all the system documentation uses a
particular numbering of the devices, it's convenient if we can use the
same numbering in Linux.

> In a few cases, particularly where those numbers are user-visible, like
> in ethN, the aliases construct is a good solution.  If a  
> driver/subsystem
> is relying on the aliases though, it should document this in a  
> (platform?)
> binding -- and it would better have a very good reason for it!

Incidentally, another word on "cell-index".  Even for its intended
purpose, this was always a compromise.  The strictly correct way to
handle shared registers like this is for the node representing the
shared resource to have a table of phandles pointing back to the
devices controlled by the shared registers from which the appropriate
indices can derived.

On at least some 4xx chips, however, the shared resources are
scattered around various places, but all use the same device
numbering.  Therefore it seemed expedient to encode that numbering in
a single place - 'cell-index' - rather than having several such
tables.

-- 
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: "cell-index" vs. "index" vs. no index in I2C device nodes

[Grant Likely]

On Wed, Jun 4, 2008 at 8:41 PM, David Gibson <dwg@au1.ibm.com> wrote:
> On Wed, Jun 04, 2008 at 09:19:42PM -0500, Josh Boyer wrote:
>> On Wed, 4 Jun 2008 10:43:51 -0500
>> Scott Wood <scottwood@freescale.com> wrote:
>>
>> > On Wed, Jun 04, 2008 at 10:24:15AM -0500, Timur Tabi wrote:
>> > > Stefan Roese wrote:
>> > > > I'm wondering what is currently recommended in the I2C device tree nodes? The
>> > > > current IBM I2C driver (i2c-ibm_iic.c) checks "index" and most FSL dts files
>> > > > use "cell-index". Some 4xx dts files implement "cell-index" some have no
>> > > > index at all.
>> > > >
>> > > > So what should be used here. Please advise and I'll prepare a patch for it.
>> > >
>> > > I just posted a patch for the FSL I2C driver to check for cell-index.  I'm under
>> > > the impression that cell-index is the standard for enumerating devices in the
>> > > device tree.
>> >
>> > No, it's the standard for correlating devices with portions of a shared
>> > register block elsewhere.  Your use in the I2C node is merely a hack to
>> > deal with Linux wanting to deal with indices rather than pointers,
>> > combined with a lack of a decent way to look up a device struct from the
>> > device node.
>>
>> So if possible, I'd like to eliminate the *index stuff all together
>> from the 4xx driver.  The private data structure contains an idx
>> parameter, but this can be populated based on probe order or something.
>>
>> >From a device tree perspective, index and cell-index are both
>> incorrect.  The IIC macros don't share register blocks with anything,
>> are enumerated as unique instances per macro in the device tree, and
>> should be able to be distinguished by "regs" and/or unit address.
>>
>> Does anyone disagree with that?
>
> Hear, hear.
>
> Aliases can also provide a reasonable way of enumerating devices, if
> "reg" isn't suitable on its own.  Though obviously, drivers will need
> some sort of fallback if suitable aliases don't exist in the tree.

The fallback is to just let the i2c layer auto-assign an ID.  The only
reason I can think of to want to assign a specific id to an i2c bus is
so that a userspace application can reference a specific bus.  The
drivers really shouldn't care.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Benjamin Herrenschmidt]

On Wed, 2008-06-04 at 10:43 -0500, Scott Wood wrote:
> > I just posted a patch for the FSL I2C driver to check for cell-index.  I'm under
> > the impression that cell-index is the standard for enumerating devices in the
> > device tree.
> 
> No, it's the standard for correlating devices with portions of a shared
> register block elsewhere.  Your use in the I2C node is merely a hack to
> deal with Linux wanting to deal with indices rather than pointers,
> combined with a lack of a decent way to look up a device struct from the
> device node.

Totally agreed. cell-index should -only- be about correlating a given
device in an ASIC with some shared register (an example coming to mind
is clock control registers with one bit per cell) etc...

For the rest, well, use aliases :-)

Ben.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Benjamin Herrenschmidt]

On Wed, 2008-06-04 at 21:19 -0500, Josh Boyer wrote:
> So if possible, I'd like to eliminate the *index stuff all together
> from the 4xx driver.  The private data structure contains an idx
> parameter, but this can be populated based on probe order or something.
> 
> >From a device tree perspective, index and cell-index are both
> incorrect.  The IIC macros don't share register blocks with anything,
> are enumerated as unique instances per macro in the device tree, and
> should be able to be distinguished by "regs" and/or unit address.
> 
> Does anyone disagree with that?

Not sure what you mean, but some of the 4xx drivers need to know
their cell index to whack the right bits in some SDRs etc... for
things like clock control. That's also the only reason they should
use cell-index :-)

Ben.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Benjamin Herrenschmidt]

On Thu, 2008-06-05 at 09:48 +0200, Jean Delvare wrote:
> As far as I am concerned, it's really up to the maintainers and users
> of this platform. All I am asking for is that you do not call
> i2c_add_numbered_adapter() on an adapter with an automatically
> generated number. This function must only be used for adapter's those
> number is well defined. If an adapter doesn't have a well-defined
> number then use i2c_add_adapter() (but then you can no longer declare
> your I2C devices as part of the platform data.)

Can't we just give those things -names- instead of numbers ?

Ben.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Benjamin Herrenschmidt]

On Thu, 2008-06-05 at 10:45 +0200, Stefan Roese wrote:
> Full ack from me. So I suggest to use "cell-index" if available and
> otherwise 
> use an incremented number, same as the FSL i2c driver does now:
> 
> http://ozlabs.org/pipermail/linuxppc-dev/2008-June/057254.html
> 
> If nobody objects I'll send a patch to add the cell-index to all 4xx
> dts files 
> in a short while.

No, don't use cell-index. If you have a setup with 2 SOCs in
SMP or something like the Axon bridge on cell where you can have
multiple Axons in a system, cell-index will not be unique. They
will give you the index -within-a-chip- as it is used, as I
said in a separate mail, for things like power or clock control
bits & pieces. It will not be system-wide unique.

Ben.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Benjamin Herrenschmidt]

On Thu, 2008-06-05 at 10:13 -0500, Timur Tabi wrote:
> Josh Boyer wrote:
> 
> > From a device tree perspective, index and cell-index are both
> > incorrect.  The IIC macros don't share register blocks with anything,
> > are enumerated as unique instances per macro in the device tree, and
> > should be able to be distinguished by "regs" and/or unit address.
> 
> I think we should just expand the definition of cell-index to include standard
> device enumeration for when it's needed.  The original definition is too
> limited, IMHO.

No. I disagree. Expanding the definition is the wrong thing to do, and
as I pointed out in an earlier mail, will not work well with SMP or
multi-chip setups.

Ben.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Benjamin Herrenschmidt]

On Thu, 2008-06-05 at 10:43 -0500, Timur Tabi wrote:
> Grant Likely wrote:
> 
> > if you need explicit indexing then use an alias.  My opinion however
> > is that explicit indexing is unnecessary and is just an artifact of
> > current i2c subsystem internals.  There is already enough information
> > in the device tree to match i2c devices with i2c busses without
> > resorting to indexes.
> 
> Not for ALSA SoC V2 devices.  In ASoC V2, the "fabric" driver needs to identify
> the codec by its specific I2C bus and address number.  The codec driver is not
> an OF driver (normally), so it doesn't have access to any OF data.  It's just an
> I2C driver, so its given an I2C address and some number that represents an adapter.

Any struct device can have an OF node. We need to fix things so they get
filled properly.

> Therefore, the fabric driver and the codec driver need to independently
> determine the I2C bus number, and they need to match.  The fabric driver parses
> the OF tree and looks up the cell-index property.  The codec driver uses the
> adapter->nr variable.  The patch I posted ensures that the two contain the same
> number.

No, the fabric driver should get to its device node in a way or another,
and from there, find a pointer (via a phandle) to it's codec, and match
that to an i2c device.

If the current infrastructure doesn't allow that kind of matching, it
needs to be fixed.

Ben.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Sean MacLennan]

On Thu, 5 Jun 2008 22:07:31 -0600
"Grant Likely" <grant.likely@secretlab.ca> wrote:

> The fallback is to just let the i2c layer auto-assign an ID.  The only
> reason I can think of to want to assign a specific id to an i2c bus is
> so that a userspace application can reference a specific bus.  The
> drivers really shouldn't care.

If we switch the iic driver to use i2c_add_adapter rather than
i2c_add_numbered_adapter, this is exactly what it does. i2c will use
the idr_ functions to assign a number.

Cheers,
   Sean

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Jean Delvare]

Hi Ben,

On Fri, 06 Jun 2008 14:16:23 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2008-06-05 at 09:48 +0200, Jean Delvare wrote:
> > As far as I am concerned, it's really up to the maintainers and users
> > of this platform. All I am asking for is that you do not call
> > i2c_add_numbered_adapter() on an adapter with an automatically
> > generated number. This function must only be used for adapter's those
> > number is well defined. If an adapter doesn't have a well-defined
> > number then use i2c_add_adapter() (but then you can no longer declare
> > your I2C devices as part of the platform data.)
> 
> Can't we just give those things -names- instead of numbers ?

Good question. These i2c adapters already have names
(i2c_adapter.name), which are set by their respective drivers with more
or less grace depending on the driver [1]. What we don't have though is
the possibility to instantiate i2c devices by adapter name.

We could certainly add a busname field to struct i2c_devinfo and
implement i2c_register_board_info_by_name() if it seems to be worth the
extra code and memory. I am open to the idea if it solves a problem
with no other clean solution.

[1] radeonfb is notoriously bad at that ;)

-- 
Jean Delvare

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Grant Likely]

On Fri, Jun 6, 2008 at 12:21 AM, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Ben,
>
> On Fri, 06 Jun 2008 14:16:23 +1000, Benjamin Herrenschmidt wrote:
>> On Thu, 2008-06-05 at 09:48 +0200, Jean Delvare wrote:
>> > As far as I am concerned, it's really up to the maintainers and users
>> > of this platform. All I am asking for is that you do not call
>> > i2c_add_numbered_adapter() on an adapter with an automatically
>> > generated number. This function must only be used for adapter's those
>> > number is well defined. If an adapter doesn't have a well-defined
>> > number then use i2c_add_adapter() (but then you can no longer declare
>> > your I2C devices as part of the platform data.)
>>
>> Can't we just give those things -names- instead of numbers ?
>
> Good question. These i2c adapters already have names
> (i2c_adapter.name), which are set by their respective drivers with more
> or less grace depending on the driver [1]. What we don't have though is
> the possibility to instantiate i2c devices by adapter name.
>
> We could certainly add a busname field to struct i2c_devinfo and
> implement i2c_register_board_info_by_name() if it seems to be worth the
> extra code and memory. I am open to the idea if it solves a problem
> with no other clean solution.

Even this isn't necessary.  In arch/powerpc we don't really need to
register the board info separately from the bus driver so we don't
need to jump through hoops to match a board_info with an i2c bus.  The
device tree data structure already links the i2c devices with the bus
they are on, so an OF-aware i2c bus driver will match against a node
in the device tree and then use all the child nodes to register i2c
devices against itself.

This, of course, only applies to platform level i2c busses described
in the device tree.  Plugin devices like PCI cards still can create
their own i2c busses in the usual way.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Benjamin Herrenschmidt]

On Fri, 2008-06-06 at 08:21 +0200, Jean Delvare wrote:
> We could certainly add a busname field to struct i2c_devinfo and
> implement i2c_register_board_info_by_name() if it seems to be worth
> the
> extra code and memory. I am open to the idea if it solves a problem
> with no other clean solution.
> 
> [1] radeonfb is notoriously bad at that ;)

radeonfb is notoriously bad at many things :-)

Ben.

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

David Gibson wrote:

> Aliases aren't trivially reversible, but it shouldn't be too hard to
> write a helper function which will do the scan and parse you describe.

How about I submit a patch adding a function called "of_get_aliased_index()"?

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Stefan Roese]

On Friday 06 June 2008, Timur Tabi wrote:
> David Gibson wrote:
> > Aliases aren't trivially reversible, but it shouldn't be too hard to
> > write a helper function which will do the scan and parse you describe.
>
> How about I submit a patch adding a function called
> "of_get_aliased_index()"?

Would be very welcome. :)

Best regards,
Stefan

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Segher Boessenkool]

>> Well, I just don't see the point of having two different properties 
>> that say the
>> same thing.  I'm not an IEE 1275 purist, so I don't think we should 
>> be hampered
>> by old node definitions.  I especially don't like having a property 
>> specifically
>> for indexing I2C nodes that can't be used to enumerate other nodes.
>
> It's not about purity.  It's about overloading something that has
> existing semantics just because you have one usecase that you _think_
> needs it.
>
> If everyone did that, this whole device tree concept is going to just
> be one big cluster f*ck.

One important way of preventing this overloading and death-by-complexity
is to make most properties specific to a particular binding.  It is good
if other bindings that need similar functionality can use similar
properties, or sometimes even identical ones; but there are only a few
properties that are defined for *every* node.

Trying to make stuff too generic just doesn't work.


Segher

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Segher Boessenkool]

>>> Aliases can also provide a reasonable way of enumerating devices, if
>>> "reg" isn't suitable on its own.
>>
>> Yes.  In almost all cases, drivers and subsystems do not need this at
>> all though.  In OF, one device points to another by putting the 
>> phandle
>> of that pointed-to device in some property (and buses are represented
>> by their parent bridge).  If the Linux subsystem wants to use an 
>> integer
>> for distinguishing between its various buses, that's fine, but it can
>> just make up these numbers -- the numbers themselves have no actual
>> meaning, only the relationships expressed via those numbers do.
>
> That's true.  However if all the system documentation uses a
> particular numbering of the devices, it's convenient if we can use the
> same numbering in Linux.

Sure, it's convenient, and you can use aliases to get the naming you
want accessible to the user.  The drivers shouldn't depend on this
naming internally though, esp. since it doesn't really help anything.

> Incidentally, another word on "cell-index".  Even for its intended
> purpose, this was always a compromise.  The strictly correct way to
> handle shared registers like this is for the node representing the
> shared resource to have a table of phandles pointing back to the
> devices controlled by the shared registers from which the appropriate
> indices can derived.

IMO, there is no really good (let alone "correct") way of handling
this.

> On at least some 4xx chips, however, the shared resources are
> scattered around various places, but all use the same device
> numbering.  Therefore it seemed expedient to encode that numbering in
> a single place - 'cell-index' - rather than having several such
> tables.

Yeah, it seems to work out for those 4xx.  Other platforms would be
well advised to consider the tradeoff for themselves though, many
SoCs have an even more "wild" design than 4xx does ;-)


Segher

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Timur Tabi]

Benjamin Herrenschmidt wrote:

> No, the fabric driver should get to its device node in a way or another,
> and from there, find a pointer (via a phandle) to it's codec, and match
> that to an i2c device.
> 
> If the current infrastructure doesn't allow that kind of matching, it
> needs to be fixed.

Scott and I looked at this just now, and it doesn't seem to be an easy fix
unless we use of_i2c.c, which seems to be unused currently.  Something about not
being able to get the i2c_adapter structure from its corresponding device_node
struct.

Anyone know when of_i2c.c is going to come "online" replace what's in fsl_soc.c?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: "cell-index" vs. "index" vs. no index in I2C device nodes

[Jochen Friedrich]

Hi Timur,

> Anyone know when of_i2c.c is going to come "online" replace what's in fsl_soc.c?

There is a patch being prepared to convert i2c-mpc from a platform driver to an of_platform driver
making the fsl_soc.c code obsolete. See: http://patchwork.ozlabs.org/linuxppc/patch?id=18898

Thanks,
Jochen