MPC Adapter: read class attribute from device tree
Subsystems (2)
PATCH 1/1 MPC Adapter: read class attribute from device tree
For MPC adapter there is no class assigned as it is done in other adapters. This way no new-style client will ever be instantiated. With this patch class assignment is read from device tree. Necessary entry:
class = <1>; /* I2C_CLASS_HWMON (iic.h) */
Based on 2.6.29
--- drivers/i2c/busses/i2c-mpc.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
drivers/i2c/busses/i2c-mpc.c +0 -0
@@ -318,6 +318,7 @@ static int __devinit fsl_i2c_probe(struct of_device
*op, const struct of_device_
{
int result = 0;
struct mpc_i2c *i2c;
+ int *of_val;
i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
if (!i2c)
@@ -354,6 +355,10 @@ static int __devinit fsl_i2c_probe(struct of_device *op, const struct of_device_
dev_set_drvdata(&op->dev, i2c);
i2c->adap = mpc_ops;
+ of_val = of_get_property(op->node, "class", NULL);
+ if(of_val)
+ i2c->adap.class = *of_val;
+
i2c_set_adapdata(&i2c->adap, i2c);
i2c->adap.dev.parent = &op->dev;
Signature · 1 line
Michael Lawnick
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hello,
sorry for jumping in late. I just recently subscribed to this list.
Quoted 5 lines · Michael Lawnick
For MPC adapter there is no class assigned as it is done in other adapters. This way no new-style client will ever be instantiated. With this patch class assignment is read from device tree. Necessary entry:
class = <1>; /*I2C_CLASS_HWMON(iic.h) */
Do we really need that? Probing is dangerous and not necessary. Does it not work with a proper DTS entry? But maybe I have missed something?
Quoted 22 lines
Based on 2.6.29
Signed-off-by Michael Lawnick <[email protected]>Cc Jean Delvare <[email protected]>Cc Sang, Wolfram <[email protected]>--- drivers/i2c/busses/i2c-mpc.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)drivers/i2c/busses/i2c-mpc.c +1
-0@@ -318,6 +318,7 @@ static int __devinit fsl_i2c_probe(struct of_device *op, const struct of_device_ { int result = 0;struct mpc_i2c *i2c; +int *of_val; i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);drivers/i2c/busses/i2c-mpc.c +0
-0if (!i2c)
drivers/i2c/busses/i2c-mpc.c static int __devinit fsl_i2c_probe(struct of_device · line 364
Quoted 10 lines
drivers/i2c/busses/i2c-mpc.c +4
-0@@ -354,6 +355,10 @@ static int __devinit fsl_i2c_probe(struct of_device *op, const struct of_device_ dev_set_drvdata(&op->dev, i2c); i2c->adap = mpc_ops; + of_val =of_get_property (op->node,"class" ,NULL ); +if (of_val) + i2c->adap.class = *of_val; +i2c_set_adapdata (&i2c->adap, i2c); i2c->adap.dev.parent = &op->dev;
The CPM I2C bus driver uses "linux,i2c-class" for that purpose. See:
http://lxr.linux.no/linux+v2.6.29/Documentation/powerpc/dts-bindings/fsl/cpm_qe/cpm/i2c.txt
A consistent binding would be nice and the default should be "0", if we really need legacy probing.
Furthermore, the new binding needs to be documented and published on the Devicetree-discuss ML.
Wolfgang.
A message that is not in the archive (sent off-list or not yet received): [email protected]
Wolfgang Grandegger said the following:
Quoted 5 lines
Hello,
sorry for jumping in late. I just recently subscribed to this list.
Michael Lawnick wrote:
Quoted 8 lines
Quoted 5 lines
For MPC adapter there is no class assigned as it is done in other adapters. This way no new-style client will ever be instantiated. With this patch class assignment is read from device tree. Necessary entry:
class = <1>; /*I2C_CLASS_HWMON(iic.h) */Do we really need that? Probing is dangerous and not necessary. Does it not work with a proper DTS entry? But maybe I have missed something?
Our current and our next system consists of two flavors of the same board with different devices. To minimize maintenance and testing we use a reduced kernel and load the needed modules at runtime. Furthermore we will have to handle hot-plugged I2C-devices. Whether this strategy is best could be discussed in another thread but is rather OT in this mailing list. Nevertheless loading modules at runtime is legal and generally supported by LINUX.
Defining all possible (I2C-)devices in DTS would give a mess. E.g. on one board there will be ~30 temperature sensors, on the other none. As every DTS entry will force a sysFs subdirectory there would be a bunch of functionless directories - rather ugly.
If probing of a device is dangerous, it can be defined in DTS anyway and the device driver can easily omit this part by early return.
Because of the situation above I try to keep the ability of dynamic instantiation. Jean hesitates, I feel because he sees I2C solely in static manner.
Quoted 36 lines
Quoted 23 lines
Based on 2.6.29
Signed-off-by Michael Lawnick <[email protected]>Cc Jean Delvare <[email protected]>Cc Sang, Wolfram <[email protected]>--- drivers/i2c/busses/i2c-mpc.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)drivers/i2c/busses/i2c-mpc.c +1
-0@@ -318,6 +318,7 @@ static int __devinit fsl_i2c_probe(struct of_device *op, const struct of_device_ { int result = 0;struct mpc_i2c *i2c; +int *of_val; i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);drivers/i2c/busses/i2c-mpc.c +0
-0if (!i2c)drivers/i2c/busses/i2c-mpc.c static int __devinit fsl_i2c_probe(struct of_device · line 364
Quoted 10 lines
drivers/i2c/busses/i2c-mpc.c +4
-0@@ -354,6 +355,10 @@ static int __devinit fsl_i2c_probe(struct of_device *op, const struct of_device_ dev_set_drvdata(&op->dev, i2c); i2c->adap = mpc_ops; + of_val =of_get_property (op->node,"class" ,NULL ); +if (of_val) + i2c->adap.class = *of_val; +i2c_set_adapdata (&i2c->adap, i2c); i2c->adap.dev.parent = &op->dev;The CPM I2C bus driver uses "linux,i2c-class" for that purpose. See:
Quoted 1 line
http://lxr.linux.no/linux+v2.6.29/Documentation/powerpc/dts-bindings/fsl/cpm_qe/cpm/i2c.txt
I love this distributed information :-( - I will fix.
Quoted 3 lines
A consistent binding would be nice and the default should be "0", if we really need legacy probing.
If linux,i2c-class is not set, 0 will be used.
Quoted 4 lines
Furthermore, the new binding needs to be documented and published on the Devicetree-discuss ML.
Hmm, how are they involved if linux,i2c-class is used - it's already defined as you showed above.
Signature · 2 lines
Kind regards, Michael
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 3 lines
mailing list. Nevertheless loading modules at runtime is legal and generally supported by LINUX.
Ack. And it does so...
Quoted 4 lines
Defining all possible (I2C-)devices in DTS would give a mess. E.g. on one board there will be ~30 temperature sensors, on the other none. As every DTS entry will force a sysFs subdirectory there would be a bunch of functionless directories - rather ugly.
Well, you should have a DTS for every flavour; that is what DTS are about. You do know that, do you?
Even if not, there would be a slighty messed directory on one specific device with how many units? Let's take the latest example of extending the DTS file with OS-specific information - this one is ugly, too, but has impact _for everyone using dts_ files. Think about the number of affected systems and you will hopefully understand why such patches are handled with extreme caution.
Quoted 3 lines
Because of the situation above I try to keep the ability of dynamic instantiation. Jean hesitates, I feel because he sees I2C solely in static manner.
He is taking care for every I2C communication on every Linux machine out there. He can't please everyone. On the other hand, you are free to modify the kernel as you please. It doesn't really have to be in mainline, does it?
BTW, I hope you saw that the "linux,i2c-class" property is considered deprecated. Thus, expect a patch from me removing it entirely as soon as the old binding model went away (there aren't even any users).
Signature · 2 lines
Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
A message that is not in the archive (sent off-list or not yet received): [email protected]
Wolfram Sang said the following:
Quoted 5 lines
Quoted 2 lines
mailing list. Nevertheless loading modules at runtime is legal and generally supported by LINUX.
Ack. And it does so...
Quoted 7 lines
Quoted 4 lines
Defining all possible (I2C-)devices in DTS would give a mess. E.g. on one board there will be ~30 temperature sensors, on the other none. As every DTS entry will force a sysFs subdirectory there would be a bunch of functionless directories - rather ugly.
Well, you should have a DTS for every flavour; that is what DTS are about. You do know that, do you?
You like it more complicated? Ok, here it is:
As Jean indicated in his reply, we will have to support hotplug. Most of the above sensors won't exist a life time on some systems, on others all could exist. I tried to keep out this issue to avoid a drift to the discussion that hotplug is not _yet_ supported. I will^Wmust support hotplug, either by using tree's code or by doing my own modifications.
Quoted 7 lines
Even if not, there would be a slighty messed directory on one specific device with how many units? Let's take the latest example of extending the DTS file with OS-specific information - this one is ugly, too, but has impact
_foreveryone usingdts_files. Think about the number of affected systems and you will hopefully understand why such patches are handled with extreme caution.
You talk about my reading of the class entry from DTS? No, I don't understand how other systems are affected by this. They don't read this entry of _my_ .dts file.
Seems there is a general misunderstanding...
Quoted 9 lines
Quoted 4 lines
Because of the situation above I try to keep the ability of dynamic instantiation. Jean hesitates, I feel because he sees I2C solely in static manner.
He is taking care for every I2C communication on every Linux machine out there. He can't please everyone. On the other hand, you are free to modify the kernel as you please. It doesn't really have to be in mainline, does it?
If this would be a private problem, yes.
But I see this as a more general flaw:
Many adapters do initialization of .class in adapter code. A few do not. I haven't checked, but assume the PPC ones, as they have DTS. Thats Ok, as long DTS is really used. In at least one other adapter it is: Exactly Wolfgang Grandegger, who is responsible for removal of the original default initialization, showed me, that it is done in i2c-cpm!
Quoted 5 lines
BTW, I hope you saw that the "linux,i2c-class" property is considered deprecated. Thus, expect a patch from me removing it entirely as soon as the old binding model went away (there aren't even any users).
And how do you want to implement setting the class attribute? Allowing only clients that are mentioned in DTS? How should it work on systems without DTS?
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 36 lines · Michael Lawnick
Wolfram Sang said the following:
Quoted 10 lines
Quoted 2 lines
mailing list. Nevertheless loading modules at runtime is legal and generally supported by LINUX.
Ack. And it does so...
Quoted 4 lines
Defining all possible (I2C-)devices in DTS would give a mess. E.g. on one board there will be ~30 temperature sensors, on the other none. As every DTS entry will force a sysFs subdirectory there would be a bunch of functionless directories - rather ugly.
Well, you should have a DTS for every flavour; that is what DTS are about. You do know that, do you?
You like it more complicated? Ok, here it is:
As Jean indicated in his reply, we will have to support hotplug. Most of the above sensors won't exist a life time on some systems, on others all could exist. I tried to keep out this issue to avoid a drift to the discussion that hotplug is not _yet_ supported. I will^Wmust support hotplug, either by using tree's code or by doing my own modifications.Quoted 6 lines
Even if not, there would be a slighty messed directory on one specific device with how many units? Let's take the latest example of extending the DTS file with OS-specific information - this one is ugly, too, but has impact
_foreveryone usingdts_files. Think about the number of affected systems and you will hopefully understand why such patches are handled with extreme caution.You talk about my reading of the class entry from DTS? No, I don't understand how other systems are affected by this. They don't read this entry of _my_ .dts file.
Seems there is a general misunderstanding...Quoted 7 lines
Quoted 3 lines
Because of the situation above I try to keep the ability of dynamic instantiation. Jean hesitates, I feel because he sees I2C solely in static manner.
He is taking care for every I2C communication on every Linux machine out there. He can't please everyone. On the other hand, you are free to modify the kernel as you please. It doesn't really have to be in mainline, does it?
Quoted 7 lines
If this would be a private problem, yes.
But I see this as a more general flaw:
Many adapters do initialization of .class in adapter code. A few do not. I haven't checked, but assume the PPC ones, as they have DTS. Thats Ok, as long DTS is really used. In at least one other adapter it is: Exactly Wolfgang Grandegger, who is responsible for removal of the original default initialization, showed me, that it is done in i2c-cpm!
This property in i2c-cpm existed before the conversion to new style I2C drivers. I actually proposed the following patch:
http://ozlabs.org/pipermail/linuxppc-dev/2008-July/060016.html
But after some discussion we came to the conclusion, that legacy device probing should be avoided for FDT based systems.
Wolfgang.
A message that is not in the archive (sent off-list or not yet received): [email protected]
Wolfgang Grandegger said the following:
Quoted 6 lines
Quoted 5 lines
Many adapters do initialization of .class in adapter code. A few do not. I haven't checked, but assume the PPC ones, as they have DTS. Thats Ok, as long DTS is really used. In at least one other adapter it is: Exactly Wolfgang Grandegger, who is responsible for removal of the original default initialization, showed me, that it is done in i2c-cpm!
Quoted 7 lines
This property in i2c-cpm existed before the conversion to new style I2C drivers. I actually proposed the following patch:
http://ozlabs.org/pipermail/linuxppc-dev/2008-July/060016.html
But after some discussion we came to the conclusion, that legacy device probing should be avoided for FDT based systems.
What does this ^^^ mean?
I am really without emotion about how it is solved, but dynamic instantiation should be possible in a clean way. Generating an entry for every possible device is no clean way, IMHO.
BTW: I can't see what this has to do with legacy, I my case we talk about new-style ones.
Signature · 1 line
Michael
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 6 lines · Michael Lawnick
Wolfgang Grandegger said the following:
Quoted 5 lines
Quoted 5 lines
Many adapters do initialization of .class in adapter code. A few do not. I haven't checked, but assume the PPC ones, as they have DTS. Thats Ok, as long DTS is really used. In at least one other adapter it is: Exactly Wolfgang Grandegger, who is responsible for removal of the original default initialization, showed me, that it is done in i2c-cpm!
Quoted 8 lines
Quoted 7 lines
This property in i2c-cpm existed before the conversion to new style I2C drivers. I actually proposed the following patch:
http://ozlabs.org/pipermail/linuxppc-dev/2008-July/060016.html
But after some discussion we came to the conclusion, that legacy device probing should be avoided for FDT based systems.
What does this ^^^ mean?
"flattened device-tree", see
linux-2.6/Documentation/powerpc/booting-without-of.txt
Wolfgang.
Quoted 2 lines
You like it more complicated? Ok, here it is:
Thanks, that gave me a bigger picture.
Quoted 4 lines
You talk about my reading of the class entry from DTS? No, I don't understand how other systems are affected by this. They don't read this entry of _my_ .dts file.
Seems there is a general misunderstanding...
DTS is convention. One convention is to keep it OS-neutral. If you use something out of this convention like "linux,i2c-class", you motivate others to follow you. In fact, this is exactly what happened because i2c-cpm defined such a thing (and people who are watching the DTS not drifting away seemed to have missed it). If other people use it, other OS could be forced to evaluate a property which is named "linux" and may be, in addition, inadequate for their purpose.
DTS (or better the OF devicetree) faces the danger of so many private extensions that there is no common thing (= convention) at the end of the day rendering it unusable. This is why adding new properties is handled with care (and should always be CCed to devicetree-discuss).
Quoted 2 lines
Many adapters do initialization of .class in adapter code. A few do not. I haven't checked, but assume the PPC ones, as they have DTS. Thats Ok,
(Would be nice if you did. Knowledge is better than assumption in this case)
I think most drivers are old enough to have a .class as it was useful back then and nobody cared to remove it. Newer drivers, especially embedded ones, usually don't have it right from the beginning, also on ARM..., as probing is not wanted.
Quoted 3 lines
as long DTS is really used. In at least one other adapter it is: Exactly Wolfgang Grandegger, who is responsible for removal of the original default initialization, showed me, that it is done in i2c-cpm!
You mean setting the class variable through DTS? Yeah, this was a good outcome of this thread. I will submit a patch to remove it soonish. It just slipped through, should have never been there IMHO.
Quoted 1 line
And how do you want to implement setting the class attribute?
Not at all. I see ".class" as a reminiscent to overcome some flaws of the old binding model. If we finally got rid of that we can surely think of a more elegant way to enforce instanciation of clients run-time. And with that, probably get rid of .class altogether. D'accord, Jean?
Quoted 1 line
Allowing only clients that are mentioned in DTS?
For now, yes. This is still the common case (=avoid probing). Until a mainline solution comes up (which is needed, I agree on that), not so common cases could simply patch the driver to add a .class field.
Quoted 1 line
How should it work on systems without DTS?
i2c_board_info? (no offence, sometimes it is hard to tell if you know all details and still see a valid problem or if you are just missing details. If the first one is the case, please be a bit more elaborative on the problem.)
Regards,
Wolfram
Signature · 2 lines
Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hi Wolfram,
Quoted 4 lines · Wolfram Sang
Quoted 1 line
And how do you want to implement setting the class attribute?
Not at all. I see ".class" as a reminiscent to overcome some flaws of the old binding model.
Not really. The old binding model didn't have this concept of class originally, it was added later but never enforced and only sparsely used.
Nowadays classes are an extension of the new binding model, used to restrict the field of application of .detect() callbacks (because we definitely do not want probing to be enabled by default.)
Quoted 3 lines
If we finally got rid of that we can surely think
of a more elegant way to enforce instanciation of clients run-time. And with that, probably get rid of .class altogether. D'accord, Jean?
In an ideal world, yes. In the real world, no. There are several thousand PC mainboards out there with devices on the SMBus and we don't have a list, and in most cases no way to ask the system about them. Probing is the only realistic way there, so at least I2C_CLASS_HWMON, I2C_CLASS_DDC and I2C_CLASS_SPD are there to stay. We should be able to get rid of I2C_CLASS_TV_ANALOG and I2C_CLASS_TV_DIGITAL though.
While classes will stay for PC mainboards, it should be possible (and desirable) to stop using them completely on embedded systems.
Signature · 1 line
Jean Delvare
A message that is not in the archive (sent off-list or not yet received): [email protected]
Jean Delvare said the following:
Quoted 3 lines
While classes will stay for PC mainboards, it should be possible (and desirable) to stop using them completely on embedded systems.
This means no probing on embedded systems, doesn't it?
Do you really expect a customized kernel space code to do that? IMHO there should be at least a way for triggering from user space.
Signature · 2 lines
Regards, Michael
Wolfram Sang said the following:
...
Quoted 6 lines
Quoted 1 line
Allowing only clients that are mentioned in DTS?
For now, yes. This is still the common case (=avoid probing). Until a mainline solution comes up (which is needed, I agree on that), not so common cases could simply patch the driver to add a .class field.
Especially it is ugly that devices which are mentioned in DTS but are not present (because the component isn't plugged in or powered) get an directory entry in /sys/bus/i2c/devices with bind, unbind, ... This is my 'problem': I search for a clean way to instantiate on demand with no zombies in case of missing H/W. I know there are problems in identification of devices, but to remove all possibilities to try it seems not a good way for me.
Quoted 6 lines
Quoted 1 line
How should it work on systems without DTS?
i2c_board_info? (no offence, sometimes it is hard to tell if you know all details and still see a valid problem or if you are just missing details. If the first one is the case, please be a bit more elaborative on the problem.)
I am new in the sense that I jumped in at 2.4 making some non i2c-drivers, rejoind at 2.6.24 for i2c-drivers and a non i2c-driver. Currently I try to catch up to current state to prepare management of all i2c-devices on our next project.
Even i2c_board_info needs to be filled in and passed to subsystem via a function call. Which part of kernel is supposed to do that without changing code beside board specific configuration files?
(I do really mean this as a question.)
Signature · 2 lines
Kind regards, Michael
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 8 lines · Michael Lawnick
Wolfram Sang said the following:
...Quoted 6 lines
Quoted 1 line
Allowing only clients that are mentioned in DTS?
For now, yes. This is still the common case (=avoid probing). Until a mainline solution comes up (which is needed, I agree on that), not so common cases could simply patch the driver to add a .class field.
Quoted 7 lines
Especially it is ugly that devices which are mentioned in DTS but are not present (because the component isn't plugged in or powered) get an directory entry in
/sys/bus/i2c/deviceswith bind, unbind, ... This is my 'problem': I search for a clean way to instantiate on demand with no zombies in case of missing H/W. I know there are problems in identification of devices, but to remove all possibilities to try it seems not a good way for me.
You might want to ask about this on the linux-ppc mailing list.
You're stuck because i2c hardware does not support hotplug. Since the hardware doesn't support hotplug there's no software support for it in the kernel. So you want to fall back to polled probing.
But probing is bad because it isn't standardized and coordinated, one device's probe can cause another device to destroy things (like set a voltage too high on a motherboard). Because of this probing can't be turned on in general.
It's possible to get to the i2c bus from user space. You could write a user space app that polls in your controlled environment. When you detect the new devices you can load the appropriate modules. Now you need to instantiate these new devices.
Jean, is it still possible to instantiate i2c devices from the module command line?
Quoted 26 lines
Quoted 7 lines
Quoted 2 lines
How should it work on systems without DTS?
i2c_board_info? (no offence, sometimes it is hard to tell if you know all details and still see a valid problem or if you are just missing details. If the first one is the case, please be a bit more elaborative on the problem.)I am new in the sense that I jumped in at 2.4 making some non i2c-drivers, rejoind at 2.6.24 for i2c-drivers and a non i2c-driver. Currently I try to catch up to current state to prepare management of all i2c-devices on our next project.
Even
i2c_board_infoneeds to be filled in and passed to subsystem via a function call. Which part of kernel is supposed to do that without changing code beside board specific configuration files?
(I do really mean this as a question.)--
Kind regards,
Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
Jon Smirl [email protected]
A message that is not in the archive (sent off-list or not yet received): 9e4733910904240838k5f425d7m849cd6b7fad19f27-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Jon Smirl said the following:
Quoted 17 lines · Michael Lawnick
Quoted 8 lines
Wolfram Sang said the following:
...Quoted 6 lines
Quoted 1 line
Allowing only clients that are mentioned in DTS?
For now, yes. This is still the common case (=avoid probing). Until a mainline solution comes up (which is needed, I agree on that), not so common cases could simply patch the driver to add a .class field.
Quoted 7 lines
Especially it is ugly that devices which are mentioned in DTS but are not present (because the component isn't plugged in or powered) get an directory entry in
/sys/bus/i2c/deviceswith bind, unbind, ... This is my 'problem': I search for a clean way to instantiate on demand with no zombies in case of missing H/W. I know there are problems in identification of devices, but to remove all possibilities to try it seems not a good way for me.
Quoted 1 line
You might want to ask about this on the linux-ppc mailing list.
I can't see how they should be able to help me more than this list.
Quoted 4 lines
You're stuck because i2c hardware does not support hotplug. Since the hardware doesn't support hotplug there's no software support for it in the kernel. So you want to fall back to polled probing.
That's what I do. But being able to detect a new device via /dev/i2c is only halve of the job. The device needs to be identified and a client instantiated. Doing this job in the user space polling program would double the code, as detection is already done in the drivers.
Quoted 5 lines
But probing is bad because it isn't standardized and coordinated, one device's probe can cause another device to destroy things (like set a voltage too high on a motherboard). Because of this probing can't be turned on in general.
That's not the problem. The problem is, that in current code state it is not possible at all to do proper probing on MPC85xx architecture, even if the possible I2C devices / bus layout would allow it. You must force instantiation, so if the driver detects an error on .probe and stops, you have dead entries in sysFs. This would not be the case if there would be a way to go via .detect
Quoted 3 lines
It's possible to get to the i2c bus from user space. You could write a user space app that polls in your controlled environment.
As said above, this is already done in this way.
Quoted 3 lines
When you
detect the new devices you can load the appropriate modules. Now you need to instantiate these new devices.
The modules are already loaded, there is missing a proper way to start instantiation of *new* clients.
Quoted 3 lines
Jean, is it still possible to instantiate i2c devices from the module command line?
This question is missing the point. The module is already up and running. The situation to handle is that there is powered-on a device after module load. Now a client for this device needs instantiation. As code for detection of client-type should IMHO not be part of user space (as it *is* already part of drivers) I am currently fighting for a way to trigger a driver's detect function.
--
Kind regards,
Michael
Quoted 18 lines · Jon Smirl
Quoted 7 lines · Michael Lawnick
Wolfram Sang said the following:
...Quoted 5 lines
Quoted 1 line
Allowing only clients that are mentioned in DTS?
For now, yes. This is still the common case (=avoid probing). Until a mainline solution comes up (which is needed, I agree on that), not so common cases could simply patch the driver to add a .class field.
Quoted 7 lines
Especially it is ugly that devices which are mentioned in DTS but are not present (because the component isn't plugged in or powered) get an directory entry in
/sys/bus/i2c/deviceswith bind, unbind, ... This is my 'problem': I search for a clean way to instantiate on demand with no zombies in case of missing H/W. I know there are problems in identification of devices, but to remove all possibilities to try it seems not a good way for me.You might want to ask about this on the linux-ppc mailing list.
Quoted 8 lines
You're stuck because i2c hardware does not support hotplug. Since the hardware doesn't support hotplug there's no software support for it in the kernel. So you want to fall back to polled probing.
But probing is bad because it isn't standardized and coordinated, one device's probe can cause another device to destroy things (like set a voltage too high on a motherboard). Because of this probing can't be turned on in general.
To decide if a device exists and responds at a specified I2C address, it would be sufficient to just read some data without causing trouble. Is that assumption correct or have I missed something? Trying to identify the device is a different issue and should be avoided.
Wolfgang.
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 28 lines · Wolfgang Grandegger
Quoted 18 lines · Jon Smirl
Quoted 7 lines · Michael Lawnick
Wolfram Sang said the following:
...Quoted 5 lines
Quoted 1 line
Allowing only clients that are mentioned in DTS?
For now, yes. This is still the common case (=avoid probing). Until a mainline solution comes up (which is needed, I agree on that), not so common cases could simply patch the driver to add a .class field.
Quoted 7 lines
Especially it is ugly that devices which are mentioned in DTS but are not present (because the component isn't plugged in or powered) get an directory entry in
/sys/bus/i2c/deviceswith bind, unbind, ... This is my 'problem': I search for a clean way to instantiate on demand with no zombies in case of missing H/W. I know there are problems in identification of devices, but to remove all possibilities to try it seems not a good way for me.You might want to ask about this on the linux-ppc mailing list.
Quoted 8 lines
You're stuck because i2c hardware does not support hotplug. Since the hardware doesn't support hotplug there's no software support for it in the kernel. So you want to fall back to polled probing.
But probing is bad because it isn't standardized and coordinated, one device's probe can cause another device to destroy things (like set a voltage too high on a motherboard). Because of this probing can't be turned on in general.
Quoted 4 lines
To decide if a device exists and responds at a specified I2C address, it would be sufficient to just read some data without causing trouble. Is that assumption correct or have I missed something? Trying to identify the device is a different issue and should be avoided.
It depends if you know what you are expecting to find there, due to the limited addressing space used by i2c, a number of devices use the same address and often do not have any indentification information.
Worse, doing a read of the chip requires knowing whether you need to issue an register address or not, so this makes life even harder if you are trying to do some form of autodetect.
Quoted 5 lines
Wolfgang.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
Ben ([email protected], http://www.fluff.org/) 'a smiley only costs 4 bytes'
Hi Jon,
Quoted 9 lines · Jon Smirl
You're stuck because i2c hardware does not support hotplug. Since the hardware doesn't support hotplug there's no software support for it in the kernel. So you want to fall back to polled probing.
But probing is bad because it isn't standardized and coordinated, one device's probe can cause another device to destroy things (like set a voltage too high on a motherboard). Because of this probing can't be turned on in general.
Quoted 7 lines
It's possible to get to the i2c bus from user space. You could write a user space app that polls in your controlled environment. When you detect the new devices you can load the appropriate modules. Now you need to instantiate these new devices.
Jean, is it still possible to instantiate i2c devices from the module command line?
It is still possible if the driver implements the .detect() callback (even without any address being probed by default) and uses one of the I2C_CLIENT_INSMOD* macros. However, these macros are planned for removal. Their replacement is the sysfs entries I have been discussing with Michael in the past few weeks and for which he sent a patch for me to review. This is still on my todo list, but I am very busy these days :(
Signature · 1 line
Jean Delvare
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 11 lines · Jean Delvare
Quoted 2 lines · Jon Smirl
Jean, is it still possible to instantiate i2c devices from the module command line?
It is still possible if the driver implements the .detect() callback (even without any address being probed by default) and uses one of the I2C_CLIENT_INSMOD* macros. However, these macros are planned for removal. Their replacement is the sysfs entries I have been discussing with Michael in the past few weeks and for which he sent a patch for me to review. This is still on my todo list, but I am very busy these days :(
For the record, it finally happened. I posted a patch doing this on
the linux-i2c list on May 4th, which can be temporarily seen at:
ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/i2c-sysfs-interface-to-instantiate-devices.patch
It will be merged in 2.6.31. Potential users are invited to give it a try and comment by then. Hopefully we can then deprecate the ugly I2C_CLIENT_INSMOD* macros, and get rid of them 2 kernels later.
Thanks,
Signature · 1 line
Jean Delvare
Quoted 15 lines · Michael Lawnick
Wolfgang Grandegger said the following:
Quoted 5 lines
Hello,
sorry for jumping in late. I just recently subscribed to this list.
Michael Lawnick wrote:
Quoted 8 lines
Quoted 5 lines
For MPC adapter there is no class assigned as it is done in other adapters. This way no new-style client will ever be instantiated. With this patch class assignment is read from device tree. Necessary entry:
class = <1>; /*I2C_CLASS_HWMON(iic.h) */Do we really need that? Probing is dangerous and not necessary. Does it not work with a proper DTS entry? But maybe I have missed something?
Quoted 7 lines
Our current and our next system consists of two flavors of the same board with different devices. To minimize maintenance and testing we use a reduced kernel and load the needed modules at runtime. Furthermore we will have to handle hot-plugged I2C-devices. Whether this strategy is best could be discussed in another thread but is rather OT in this mailing list. Nevertheless loading modules at runtime is legal and generally supported by LINUX.
You apparently still have a hard time differentiating between devices and drivers. Loading modules at runtime != instantiating devices at runtime.
(And both can be done, but if you can't define your needs, we won't go very far.)
Quoted 4 lines
Defining all possible (I2C-)devices in DTS would give a mess. E.g. on one board there will be ~30 temperature sensors, on the other none. As every DTS entry will force a sysFs subdirectory there would be a bunch of functionless directories - rather ugly.
Why don't you define one DTS per board? I'm no embedded expert, but my understanding is that this is what other developers/designers do.
Quoted 2 lines
If probing of a device is dangerous, it can be defined in DTS anyway and the device driver can easily omit this part by early return.
This is correct.
Quoted 3 lines
Because of the situation above I try to keep the ability of dynamic instantiation. Jean hesitates, I feel because he sees I2C solely in static manner.
Well there are 2 different issues here. First issue is relying on probing when a DTS would do. Many developers have done that before, and stopped doing it quickly, for a reason. We simply want to save you from doing the same mistake.
Second issue is the warm-plugging of I2C devices. Linux doesn't support this at this point, we are working on it (actually I am working on the dependencies), but as long as support for basic multiplexing it isn't there, there's little point in discussing more complex (dynamic) schemes.
I had been explaining exactly this already, hadn't I?
Signature · 1 line
Jean Delvare
A message that is not in the archive (sent off-list or not yet received): [email protected]
Jean Delvare said the following:
Quoted 4 lines
You apparently still have a hard time differentiating between devices and drivers. Loading modules at runtime != instantiating devices at runtime.
Well, till now I assumed that you can't instantiate a device who's driver isn't loaded. But I am eager to learn.
Quoted 2 lines
(And both can be done, but if you can't define your needs, we won't go very far.)
I thought I had explained very often. I'll do it once again:
We load drivers at runtime directly after boot which does automatic instantiation. There will occur further devices on bus after some time (hotplug) and therefore we need later on instantiation without reloading drivers. We cannot define which devices will exactly be plugged in, it might even be possible that on same bus address there occur different devices (as an possible option and not at same time of course). So device detection is highly appreciated.
Do you need more info?
Quoted 9 lines
Quoted 5 lines
Defining all possible (I2C-)devices in DTS would give a mess. E.g. on one board there will be ~30 temperature sensors, on the other none. As every DTS entry will force a sysFs subdirectory there would be a bunch of functionless directories - rather ugly.
Why don't you define one DTS per board? I'm no embedded expert, but my understanding is that this is what other developers/designers do.
--> my answer to W.G. at 15:05 (<[email protected]>)
Quoted 8 lines
Quoted 3 lines
Because of the situation above I try to keep the ability of dynamic instantiation. Jean hesitates, I feel because he sees I2C solely in static manner.
Well there are 2 different issues here. First issue is relying on probing when a DTS would do. Many developers have done that before, and stopped doing it quickly, for a reason. We simply want to save you from doing the same mistake.
If you don't tell me the issue, I won't understand. DTS is no good option as described multiple times.
Quoted 9 lines
Second issue is the warm-plugging of I2C devices. Linux doesn't support this at this point, we are working on it (actually I am working on the dependencies), but as long as support for basic multiplexing it isn't there, there's little point in discussing more complex (dynamic) schemes.
I had been explaining exactly this already, hadn't I?
I took a look into the code of Rodolfo Giometti and it seemed to be in a way I would have done it, if I wouldn't have been too late. In my current working I assume this code as a basis.
Nevertheless my mailed patch has *nothing* to do with multiplexing. It works on the problem that i2c-mpc has 0 as default class and so there is no way to instantiate a new-style client that is not mentioned in DTS.
Signature · 1 line
Michael
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 11 lines · Michael Lawnick
Jean Delvare said the following:
Quoted 4 lines
You apparently still have a hard time differentiating between devices and drivers. Loading modules at runtime != instantiating devices at runtime.
Well, till now I assumed that you can't instantiate a device who's driver isn't loaded. But I am eager to learn.
Quoted 2 lines
(And both can be done, but if you can't define your needs, we won't go very far.)
Quoted 9 lines
I thought I had explained very often. I'll do it once again:
We load drivers at runtime directly after boot which does automatic instantiation. There will occur further devices on bus after some time (hotplug) and therefore we need later on instantiation without reloading drivers. We cannot define which devices will exactly be plugged in, it might even be possible that on same bus address there occur different devices (as an possible option and not at same time of course). So device detection is highly appreciated.
Do you need more info?
Jean, haven't you told me multiple times that hotplug doesn't work for i2c? How can you tell when a new device has been added to an i2c bus?
There is already code in the kernel that will dynamically load the correct i2c device drivers based on the device tree. The i2c drivers don't need to be build into the kernel. Just put drivers for all of your devices onto the initrd.
Signature · 2 lines
Jon Smirl [email protected]
A message that is not in the archive (sent off-list or not yet received): 9e4733910904210900r4c5d1a8en178ad106134b7e6d-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 22 lines · Jon Smirl
Quoted 11 lines · Michael Lawnick
Jean Delvare said the following:
Quoted 4 lines
You apparently still have a hard time differentiating between devices and drivers. Loading modules at runtime != instantiating devices at runtime.
Well, till now I assumed that you can't instantiate a device who's driver isn't loaded. But I am eager to learn.
Quoted 2 lines
(And both can be done, but if you can't define your needs, we won't go very far.)
Quoted 9 lines
I thought I had explained very often. I'll do it once again:
We load drivers at runtime directly after boot which does automatic instantiation. There will occur further devices on bus after some time (hotplug) and therefore we need later on instantiation without reloading drivers. We cannot define which devices will exactly be plugged in, it might even be possible that on same bus address there occur different devices (as an possible option and not at same time of course). So device detection is highly appreciated.
Do you need more info?
Quoted 2 lines
Jean, haven't you told me multiple times that hotplug doesn't work for i2c? How can you tell when a new device has been added to an i2c bus?
I did say so, and I maintain this assertion. Warm-plug, on the other hand, could be supported (but isn't yet.) By warm-plug, I mean: switch off one "channel" of a multiplexer, add one chip to that segment, and enable the channel again. Once we have multiplexing support, this should be doable, if we implement the proper interface. But this really is per-segment, not per-device.
Of course this can never be hot-plug like e.g. USB has it: you're not going to get a signal when a new I2C device is connected. This has to be a partly manual process, where you tell the system that a new I2C segment is available, and that the usual rules (per-board instantiation or device detection) have to be applied.
Quoted 4 lines
There is already code in the kernel that will dynamically load the correct i2c device drivers based on the device tree. The i2c drivers don't need to be build into the kernel. Just put drivers for all of your devices onto the initrd.
Signature · 1 line
Jean Delvare
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 33 lines · Jean Delvare
Quoted 22 lines · Jon Smirl
Quoted 11 lines · Michael Lawnick
Jean Delvare said the following:
Quoted 4 lines
You apparently still have a hard time differentiating between devices and drivers. Loading modules at runtime != instantiating devices at runtime.
Well, till now I assumed that you can't instantiate a device who's driver isn't loaded. But I am eager to learn.
Quoted 2 lines
(And both can be done, but if you can't define your needs, we won't go very far.)
Quoted 9 lines
I thought I had explained very often. I'll do it once again:
We load drivers at runtime directly after boot which does automatic instantiation. There will occur further devices on bus after some time (hotplug) and therefore we need later on instantiation without reloading drivers. We cannot define which devices will exactly be plugged in, it might even be possible that on same bus address there occur different devices (as an possible option and not at same time of course). So device detection is highly appreciated.
Do you need more info?Quoted 2 lines
Jean, haven't you told me multiple times that hotplug doesn't work for i2c? How can you tell when a new device has been added to an i2c bus?
I did say so, and I maintain this assertion. Warm-plug, on the other hand, could be supported (but isn't yet.) By warm-plug, I mean: switch off one "channel" of a multiplexer, add one chip to that segment, and enable the channel again. Once we have multiplexing support, this should be doable, if we implement the proper interface. But this really is per-segment, not per-device.
Quoted 5 lines
Of course this can never be hot-plug like e.g. USB has it: you're not going to get a signal when a new I2C device is connected. This has to be a partly manual process, where you tell the system that a new I2C segment is available, and that the usual rules (per-board instantiation or device detection) have to be applied.
So a solution might lie down the path of having whatever catches the hotplug event (pci, usb, ..) run code that modifies the device tree and inserts the new i2c nodes. After the device tree has been modified trigger a rescan of devices so that the i2c drivers get loaded. I've never tried doing that but it should be doable in theory.
Something has to trigger an interrupt (or the user types a command) indicating that new hardware has been added. The driver of that device can modify the device tree. The rescan of the device tree will cause the i2c modules to automatically load. This shouldn't require any changes to the current i2c sub-system.
Quoted 9 lines
Quoted 5 lines
There is already code in the kernel that will dynamically load the correct i2c device drivers based on the device tree. The i2c drivers don't need to be build into the kernel. Just put drivers for all of your devices onto the initrd.
--
Jean Delvare
Signature · 2 lines
Jon Smirl [email protected]
A message that is not in the archive (sent off-list or not yet received): 9e4733910904211218l8428128k1f8dd021c50c7846-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Jon Smirl said the following:
Quoted 11 lines
So a solution might lie down the path of having whatever catches the hotplug event (pci, usb, ..) run code that modifies the device tree and inserts the new i2c nodes. After the device tree has been modified trigger a rescan of devices so that the i2c drivers get loaded. I've never tried doing that but it should be doable in theory.
Something has to trigger an interrupt (or the user types a command) indicating that new hardware has been added. The driver of that device can modify the device tree. The rescan of the device tree will cause the i2c modules to automatically load. This shouldn't require any changes to the current i2c sub-system.
This is an solution, although it feels a bit more complicated than necessary. My original plan is to check the bus in intervals of some seconds via /dev/i2c for a specific device. When the device is detected its module is loaded and the driver/device initialized. A simple user space script, running in background.
Signature · 2 lines
Regards, Michael
Quoted 6 lines
Something has to trigger an interrupt (or the user types a command) indicating that new hardware has been added. The driver of that device can modify the device tree. The rescan of the device tree will cause the i2c modules to automatically load. This shouldn't require any changes to the current i2c sub-system.
As I understood it so far, the purpose of the devicetree is done as soon it is presented to the linux-kernel and once parsed during boot. Changing it at runtime and reacting to that looks like a bigger step to me. Hints for that are that most fdt-functions are __init and I couldn't quickly find an add_node-function...
Shouldn't really matter anyhow, should it? As soon as we are run-time, there shouldn't be any difference between DTS and platform_devices. It is just driver-model then...
Signature · 2 lines
Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
Quoted 7 lines · Michael Lawnick
I took a look into the code of Rodolfo Giometti and it seemed to be in a way I would have done it, if I wouldn't have been too late. In my current working I assume this code as a basis.
Nevertheless my mailed patch has *nothing* to do with multiplexing. It works on the problem that i2c-mpc has 0 as default class and so there is no way to instantiate a new-style client that is not mentioned in DTS.
If you didn't want to handle warm-plug then that wouldn't be a problem, because you would be able to put all the chip information in the DTS. But you want warm-plug. And warm-plug needs multiplexing support. So your problem is very much related to multiplexing.
Now I'll ignore you until I am done with the prerequisites to accept Rodolfo's patches, and with Rodolfo's patches themselves. Discussing your problems when there's nothing I can do about them at the time being, is just a waste of everybody's time.
Signature · 1 line
Jean Delvare
Quoted 22 lines · Michael Lawnick
Wolfgang Grandegger said the following:
Quoted 4 lines
Hello,
sorry for jumping in late. I just recently subscribed to this list.
Quoted 8 lines
Quoted 5 lines · Michael Lawnick
For MPC adapter there is no class assigned as it is done in other adapters. This way no new-style client will ever be instantiated. With this patch class assignment is read from device tree. Necessary entry:
class = <1>; /*I2C_CLASS_HWMON(iic.h) */Do we really need that? Probing is dangerous and not necessary. Does it not work with a proper DTS entry? But maybe I have missed something?
Our current and our next system consists of two flavors of the same board with different devices. To minimize maintenance and testing we use a reduced kernel and load the needed modules at runtime. Furthermore we will have to handle hot-plugged I2C-devices. Whether this strategy is best could be discussed in another thread but is rather OT in this mailing list. Nevertheless loading modules at runtime is legal and generally supported by LINUX.
Quoted 2 lines
Defining all possible (I2C-)devices in DTS would give a mess. E.g. on one board there will be ~30 temperature sensors, on the other none.
The legacy device probing has it's limitations and I doubt that it will be able to find 30 DTT devices properly. E.g. the legacy probing algorithm is not able to distinguish between a LM75 and DS75. Let's first try to find out why it does not work as expected with the DTS entry.
Quoted 2 lines
As every DTS entry will force a sysFs subdirectory there would be a bunch of functionless directories - rather ugly.
Hopefully not. They will be created when the probing was successful, I assume. I think it is OK to define multiple I2C nodes also for non-existing devices in the DTS file if the I2C address is unique.
Wolfgang.
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 21 lines · Wolfgang Grandegger
Quoted 12 lines · Michael Lawnick
Wolfgang Grandegger said the following:
Quoted 2 lines
Do we really need that? Probing is dangerous and not necessary. Does it not work with a proper DTS entry? But maybe I have missed something?
Our current and our next system consists of two flavors of the same board with different devices. To minimize maintenance and testing we use a reduced kernel and load the needed modules at runtime. Furthermore we will have to handle hot-plugged I2C-devices. Whether this strategy is best could be discussed in another thread but is rather OT in this mailing list. Nevertheless loading modules at runtime is legal and generally supported by LINUX.
Quoted 2 lines
Defining all possible (I2C-)devices in DTS would give a mess. E.g. on one board there will be ~30 temperature sensors, on the other none.
The legacy device probing has it's limitations and I doubt that it will be able to find 30 DTT devices properly. E.g. the legacy probing algorithm is not able to distinguish between a LM75 and DS75. Let's first try to find out why it does not work as expected with the DTS entry.
Quoted 5 lines
Quoted 2 lines
As every DTS entry will force a sysFs subdirectory there would be a bunch of functionless directories - rather ugly.
Hopefully not. They will be created when the probing was successful, I assume.
No, device creation is unconditional. It is assumed that the DTS file author knew what he/she was doing.
There was a discussion about adding a flag to state the "may not be present" nature of devices, which would result in a probe before instantiating the device (on success), but it wasn't implemented yet. I have no objection is this mechanism is deemed useful.
Quoted 2 lines
I think it is OK to define multiple I2C nodes also for non-existing devices in the DTS file if the I2C address is unique.
Signature · 1 line
Jean Delvare
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 25 lines · Jean Delvare
Quoted 19 lines · Wolfgang Grandegger
Quoted 11 lines · Michael Lawnick
Wolfgang Grandegger said the following:
Quoted 2 lines
Do we really need that? Probing is dangerous and not necessary. Does it not work with a proper DTS entry? But maybe I have missed something?
Our current and our next system consists of two flavors of the same board with different devices. To minimize maintenance and testing we use a reduced kernel and load the needed modules at runtime. Furthermore we will have to handle hot-plugged I2C-devices. Whether this strategy is best could be discussed in another thread but is rather OT in this mailing list. Nevertheless loading modules at runtime is legal and generally supported by LINUX.
Quoted 2 lines
Defining all possible (I2C-)devices in DTS would give a mess. E.g. on one board there will be ~30 temperature sensors, on the other none.
The legacy device probing has it's limitations and I doubt that it will be able to find 30 DTT devices properly. E.g. the legacy probing algorithm is not able to distinguish between a LM75 and DS75. Let's first try to find out why it does not work as expected with the DTS entry.
Quoted 4 lines
Quoted 2 lines
As every DTS entry will force a sysFs subdirectory there would be a bunch of functionless directories - rather ugly.
Hopefully not. They will be created when the probing was successful, I assume.
Quoted 7 lines
No, device creation is unconditional. It is assumed that the DTS file author knew what he/she was doing.
There was a discussion about adding a flag to state the "may not be present" nature of devices, which would result in a probe before instantiating the device (on success), but it wasn't implemented yet. I have no objection is this mechanism is deemed useful.
Ah, I see. If the software can reliably find out if a device exists it should probe for it. It would be useful to limit the amount of required DTS files for the hardware variants. Often the RTC or other I2C devices are optional. I added the Deviceterr-disccuss ML to the CC to reach the the device-tree gurus.
Wolfgang.
Hi Wolfgang,
Quoted 1 line
I added the Deviceterr-disccuss ML to the CC to reach the the
~~~~~~~~~~
Quoted 1 line
device-tree gurus.
Freud or "nickname among friends"? :)
Cheers
Detlev
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: [email protected]
Quoted 3 lines
For MPC adapter there is no class assigned as it is done in other adapters.
Still, the trend is to use the class field less, not more often. Probing causes more pain than gain. Most of us have been through that, try to believe us.
Quoted 1 line
This way no new-style client will ever be instantiated.
Sorry, this seems wrong to me:
1) The client _instance_ does come from the dts, as Jean explained before. Did you get the difference? You said yourself that you could see the entries in sysfs. It seems just that it does not get connected to the driver. This should be the place to fix the error.
2) When I showed you the output of modprobing the rtc-ds1307 driver, I was using exactly this i2c-master driver (the board uses an MPC5200B). Plus, I often use at24 as a module. Doesn't look like a problem in the infrastructure to me.
Quoted 3 lines
this patch class assignment is read from device tree. Necessary entry:
class = <1>; /*I2C_CLASS_HWMON(iic.h) */
The device tree is OS independent. You are encoding something linux-specific into it. This is regularly rejected. I am quite surprised to see that there is such a thing in i2c-cpm. Lucky punch, I guess ;) Should be removed IMHO.
Have you already posted the DTS snipplet for your I2C setup?
Regards,
Wolfram
Signature · 2 lines
Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
Quoted 2 lines · Michael Lawnick
For MPC adapter there is no class assigned as it is done in other adapters. This way no new-style client will ever be instantiated.
This statement is incorrect. How many times will I have to repeat myself?
Quoted 4 lines
With
this patch class assignment is read from device tree. Necessary entry:
class = <1>; /*I2C_CLASS_HWMON(iic.h) */
Hard-coding the value of Linux-specific flags in the openfirmware device tree doesn't look too good to me.
Quoted 23 lines
Based on 2.6.29
Signed-off-by Michael Lawnick <[email protected]>Cc Jean Delvare <[email protected]>Cc Sang, Wolfram <[email protected]>--- drivers/i2c/busses/i2c-mpc.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)drivers/i2c/busses/i2c-mpc.c +1
-0@@ -318,6 +318,7 @@ static int __devinit fsl_i2c_probe(struct of_device *op, const struct of_device_ { int result = 0;struct mpc_i2c *i2c; +int *of_val; i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);drivers/i2c/busses/i2c-mpc.c +0
-0if (!i2c)
drivers/i2c/busses/i2c-mpc.c static int __devinit fsl_i2c_probe(struct of_device · line 360
Quoted 6 lines
drivers/i2c/busses/i2c-mpc.c +2
-0@@ -354,6 +355,10 @@ static int __devinit fsl_i2c_probe(struct of_device *op, const struct of_device_ dev_set_drvdata(&op->dev, i2c); i2c->adap = mpc_ops; + of_val =of_get_property (op->node,"class" ,NULL ); +if (of_val)
Incorrect coding style. Didn't you run scripts/checkpatch.pl on your patch before submitting it?
drivers/i2c/busses/i2c-mpc.c static int __devinit fsl_i2c_probe(struct of_device · line 364
Quoted 5 lines
drivers/i2c/busses/i2c-mpc.c +2
-0+ i2c->adap.class = *of_val; +i2c_set_adapdata (&i2c->adap, i2c); i2c->adap.dev.parent = &op->dev;
Anyway: Wolfram and Wolfgang have explained to you how to do it right and the above isn't that. Declaring the I2C devices in the device tree works for everybody else, it has to work for you as well, if you do it right.
Signature · 1 line
Jean Delvare