From: Timur Tabi <hidden> Date: 2011-12-01 17:33:23
An I2C device tree node can contain a 'cell-index' property that can be
used to enumerate the I2C devices. If such a property exists, use it
to specify the I2C adapter number.
This feature is necessary for the Freescale PowerPC audio drivers (e.g.
on the P1022DS). The "machine driver" needs to know the adapter number
for each I2C adapter, but it only has access to the device tree.
Previously, the I2C nodes always appeared in cell-index order, so the
dynamic numbering coincided with the cell-index property. With commit
ab827d97 ("powerpc/85xx: Rework P1022DS device tree"), the I2C nodes are
unintentionally reversed in the device tree, and so the machine driver
guesses the wrong I2C adapter number.
Signed-off-by: Timur Tabi <redacted>
---
drivers/i2c/busses/i2c-mpc.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
From: Scott Wood <hidden> Date: 2011-12-01 18:41:53
On 12/01/2011 11:33 AM, Timur Tabi wrote:
An I2C device tree node can contain a 'cell-index' property that can be
used to enumerate the I2C devices. If such a property exists, use it
to specify the I2C adapter number.
Didn't we decide a long time ago that this was a bad idea?
This feature is necessary for the Freescale PowerPC audio drivers (e.g.
on the P1022DS). The "machine driver" needs to know the adapter number
for each I2C adapter, but it only has access to the device tree.
Previously, the I2C nodes always appeared in cell-index order, so the
dynamic numbering coincided with the cell-index property. With commit
ab827d97 ("powerpc/85xx: Rework P1022DS device tree"), the I2C nodes are
unintentionally reversed in the device tree, and so the machine driver
guesses the wrong I2C adapter number.
What specifically do you need this number for? What does it represent?
-Scott
From: Timur Tabi <hidden> Date: 2011-12-01 20:54:19
Scott Wood wrote:
On 12/01/2011 11:33 AM, Timur Tabi wrote:
quoted
An I2C device tree node can contain a 'cell-index' property that can be
used to enumerate the I2C devices. If such a property exists, use it
to specify the I2C adapter number.
Didn't we decide a long time ago that this was a bad idea?
I don't see what's wrong with it.
quoted
This feature is necessary for the Freescale PowerPC audio drivers (e.g.
on the P1022DS). The "machine driver" needs to know the adapter number
for each I2C adapter, but it only has access to the device tree.
Previously, the I2C nodes always appeared in cell-index order, so the
dynamic numbering coincided with the cell-index property. With commit
ab827d97 ("powerpc/85xx: Rework P1022DS device tree"), the I2C nodes are
unintentionally reversed in the device tree, and so the machine driver
guesses the wrong I2C adapter number.
What specifically do you need this number for? What does it represent?
Well, I thought the above paragraph explained it pretty well.
Audio drivers come in sets of four -- machine, ssi, dma, and codec. The machine driver needs to know which ssi is connected to which codec and which DMA channel(s) it needs to use. So I extract all this information from the device tree.
The individual drivers, however, register only with their own subsystem. So the codec driver, for example, doesn't know anything about device trees -- it just registers as an i2c driver. That means that the machine driver has to guess what name the codec driver will use when it registers. In this case, that's the i2c device name, which include the I2C adapter number (adap->nr). That means that given a device tree node, I need to know what the actual bus number is.
An alternative approach is to create a function like this:
struct i2c_adapter *i2c_adapter_from_node(struct device_node *np);
I could then just use adap->nr directly.
--
Timur Tabi
Linux kernel developer at Freescale
From: Scott Wood <hidden> Date: 2011-12-01 21:31:45
On 12/01/2011 02:54 PM, Timur Tabi wrote:
Scott Wood wrote:
quoted
On 12/01/2011 11:33 AM, Timur Tabi wrote:
quoted
An I2C device tree node can contain a 'cell-index' property that can be
used to enumerate the I2C devices. If such a property exists, use it
to specify the I2C adapter number.
Didn't we decide a long time ago that this was a bad idea?
I don't see what's wrong with it.
Obviously, since you're suggesting doing it. :-P
Did you search for the old discussions?
How is this going to interact with other i2c buses (e.g. on a board
FPGA) that might have a conflicting static numbering scheme? Have you
ensured that no dynamic bus registrations (e.g. an i2c bus on a PCI
device) can happen before the static SoC i2c buses are added?
Global numberspaces of this sort are usually the wrong answer. Look at
the mess it made with IRQ numbers, and the gymnastics we go through to
virtualize it.
quoted
quoted
This feature is necessary for the Freescale PowerPC audio drivers (e.g.
on the P1022DS). The "machine driver" needs to know the adapter number
for each I2C adapter, but it only has access to the device tree.
Previously, the I2C nodes always appeared in cell-index order, so the
dynamic numbering coincided with the cell-index property. With commit
ab827d97 ("powerpc/85xx: Rework P1022DS device tree"), the I2C nodes are
unintentionally reversed in the device tree, and so the machine driver
guesses the wrong I2C adapter number.
What specifically do you need this number for? What does it represent?
Well, I thought the above paragraph explained it pretty well.
All you said was "needs to know the adapter number", nothing about why.
Audio drivers come in sets of four -- machine, ssi, dma, and codec.
The machine driver needs to know which ssi is connected to which
codec and which DMA channel(s) it needs to use. So I extract all
this information from the device tree.
The individual drivers, however, register only with their own
subsystem. So the codec driver, for example, doesn't know anything
about device trees -- it just registers as an i2c driver. That means
that the machine driver has to guess what name the codec driver will
use when it registers. In this case, that's the i2c device name,
which include the I2C adapter number (adap->nr). That means that
given a device tree node, I need to know what the actual bus number
is.
An alternative approach is to create a function like this:
struct i2c_adapter *i2c_adapter_from_node(struct device_node *np);
I could then just use adap->nr directly.
If there isn't a way to get a "struct device" from "struct device_node",
we should add it. From that you should be able to look up the sysfs
name -- no need to mess around with adap->nr as an intermediary.
-Scott
From: Timur Tabi <hidden> Date: 2011-12-01 21:48:03
Scott Wood wrote:
How is this going to interact with other i2c buses (e.g. on a board
FPGA) that might have a conflicting static numbering scheme? Have you
ensured that no dynamic bus registrations (e.g. an i2c bus on a PCI
device) can happen before the static SoC i2c buses are added?
Hmm.... You have a point there.
quoted
An alternative approach is to create a function like this:
struct i2c_adapter *i2c_adapter_from_node(struct device_node *np);
I could then just use adap->nr directly.
If there isn't a way to get a "struct device" from "struct device_node",
we should add it.
How do I do that? Scan all the struct devices until I find one where dev->of_node == np? That seems really inefficient.
--
Timur Tabi
Linux kernel developer at Freescale
From: Scott Wood <hidden> Date: 2011-12-01 21:52:53
On 12/01/2011 03:46 PM, Timur Tabi wrote:
Scott Wood wrote:
quoted
How is this going to interact with other i2c buses (e.g. on a board
FPGA) that might have a conflicting static numbering scheme? Have you
ensured that no dynamic bus registrations (e.g. an i2c bus on a PCI
device) can happen before the static SoC i2c buses are added?
Hmm.... You have a point there.
quoted
quoted
An alternative approach is to create a function like this:
struct i2c_adapter *i2c_adapter_from_node(struct device_node *np);
I could then just use adap->nr directly.
If there isn't a way to get a "struct device" from "struct device_node",
we should add it.
How do I do that? Scan all the struct devices until I find one where dev->of_node == np? That seems really inefficient.
Ideally we would have a field in struct device_node that points to
struct device.
-Scott
Ideally we would have a field in struct device_node that points to
struct device.
No. There are cases where multiple devices may reference the same node.
It is better to walk the list of i2c adapters and look for one that
has the matching node pointer. It really isn't an expensive operation
to do it that way.
g.
From: Timur Tabi <hidden> Date: 2011-12-01 21:59:41
Grant Likely wrote:
It is better to walk the list of i2c adapters and look for one that
has the matching node pointer. It really isn't an expensive operation
to do it that way.
That's what I was thinking. I can't figure out how to walk the list, though. i2c_get_adapter() takes an adapter number, but I don't think this is going to work:
unsigned int i = 0;
struct i2c_adapter adap = i2c_get_adapter(0);
while (adap) {
if (adap->nr == nr)
break;
adap = i2c_get_adapter(++i);
}
--
Timur Tabi
Linux kernel developer at Freescale
From: Timur Tabi <hidden> Date: 2011-12-01 21:59:47
Scott Wood wrote:
Ideally we would have a field in struct device_node that points to
struct device.
I don't think there's a single place in the code where the connection between the device and device_node is made. In fact, I think dev->of_node needs to be manually initialized by the driver during the OF probe.
--
Timur Tabi
Linux kernel developer at Freescale
From: Scott Wood <hidden> Date: 2011-12-01 22:05:50
On 12/01/2011 03:59 PM, Timur Tabi wrote:
Scott Wood wrote:
quoted
Ideally we would have a field in struct device_node that points to
struct device.
I don't think there's a single place in the code where the connection
between the device and device_node is made. In fact, I think
dev->of_node needs to be manually initialized by the driver during
the OF probe.
The i2c core does this.
Does of_find_i2c_device_by_node() do what you want?
-Scott