Again on virtual i2c adapter support.
Subsystems (2)
Hello Jean,
I'm working again on virtual i2c adapter support :) since I received several letters asking for this support. That's why I'd like to have from you some advice in order to propose a working code.
Looking at latest i2c code I suppose we can modify the i2c_del_driver() function as follow:
drivers/i2c/i2c-core.c +2 -2
@@ -799,10 +799,10 @@ void i2c_del_driver(struct i2c_driver *driver)class_for_each_device (&i2c_adapter_class,NULL , driver, __detach_adapter); +mutex_unlock (&core_lock); +driver_unregister (&driver->driver);pr_debug ("i2c-core: driver [%s] unregistered\n" , driver->driver.name); - -mutex_unlock (&core_lock); }EXPORT_SYMBOL (i2c_del_driver);
Unluckely I'm not an i2c guru so I'd like to ask to you. If such modification is correct I can provide a simple support with no modifications to the i2c core! Apart this one, of course... :)
If you agree I can send a patchset, but everything is based on the above change...
Thanks for your suggestions,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: giometti-AVVDYK/kqiJWk0Htik3J/[email protected] Linux Device Driver [email protected] Embedded Systems phone: +39 349 2432127 UNIX programming skype: rodolfo.giometti
Hi Rodolfo,
Quoted 5 lines · Rodolfo Giometti
Hello Jean,
I'm working again on virtual i2c adapter support :) since I received several letters asking for this support. That's why I'd like to have from you some advice in order to propose a working code.
Thanks for keeping up with this despite my lack of support. I really hope that we can get this done this year.
Did you speak with Dave Miller already? Some months ago he proposed an horrible hack to implement support for I2C bus multiplexing, which I nacked and I pointed him to you.
Last year I have been working with David Brownell of cleaning up i2c-core so that adding things like multiplexing support is easier, in particular when it comes to the locking model. It's not completely done yet but I think we're on the right track. My hope was that we could finish the cleanups first and add multiplexing support only then, but maybe I was too optimistic, the cleanups take longer than I expected.
Quoted 21 lines
Looking at latest i2c code I suppose we can modify the
i2c_del_driver()function as follow:drivers/i2c/i2c-core.c +2
-2@@ -799,10 +799,10 @@ void i2c_del_driver(struct i2c_driver *driver)class_for_each_device (&i2c_adapter_class,NULL , driver, __detach_adapter); +mutex_unlock (&core_lock); +driver_unregister (&driver->driver);pr_debug ("i2c-core: driver [%s] unregistered\n" , driver->driver.name); - -mutex_unlock (&core_lock); }EXPORT_SYMBOL (i2c_del_driver);
Quoted 3 lines
Unluckely I'm not an i2c guru so I'd like to ask to you. If such modification is correct I can provide a simple support with no modifications to the i2c core! Apart this one, of course... :)
Honestly, I am not sure either. The locking model isn't completely cleared yet, we still need to get rid of the 3 dozen remaining legacy i2c chip drivers before we can finish it. I _think_ the above it correct, after all we do not hold the lock when calling driver_register() so I can't see why we would need to hold it when calling driver_unregister()... unless either i2c_client_release() or i2c_client_dev_release() needs this.
David should know better, as he designed the thing in the first place. David, can you please comment on the proposed change?
Quoted 2 lines
If you agree I can send a patchset, but everything is based on the above change...
I would like to propose that you create yourself an account on i2c.wiki.kernel.org. I've just added a "Work in progress" section on the main page so that other interested developers can keep track of what is happening. You could create a page describing your progress and things that need to be done. And I should do the same for my part.
Oh, and please stop calling the thing "virtual i2c adapter support". These adapters are very real. What you are working on is better described as "i2c bus multiplexing support".
Thanks,
Signature · 1 line
Jean Delvare
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jean" == Jean Delvare <[email protected]> writes:
Hi,
Jean> Oh, and please stop calling the thing "virtual i2c adapter support". Jean> These adapters are very real. What you are working on is better Jean> described as "i2c bus multiplexing support".
Not only that, it's afaik about doing i2c bus multiplexing where the multiplexers are themselves i2c devices - E.G. stuff where you might need to recursively call into the i2c core.
I have been doing i2c multiplexers where the multiplexers themselves are not controlled through i2c for years without problems.
I afaik even posted an example driver back when Dave posted his multiplex hack.
Signature · 1 line
Bye, Peter Korsgaard
Hi Peter,
Quoted 12 lines · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jean" == Jean Delvare <[email protected]> writes:
Hi,
Jean> Oh, and please stop calling the thing "virtual i2c adapter support". Jean> These adapters are very real. What you are working on is better Jean> described as "i2c bus multiplexing support".
Not only that, it's afaik about doing i2c bus multiplexing where the multiplexers are themselves i2c devices - E.G. stuff where you might need to recursively call into the i2c core.
Quoted 5 lines
I have been doing i2c multiplexers where the multiplexers themselves are not controlled through i2c for years without problems.
I afaik even posted an example driver back when Dave posted his multiplex hack.
There are still a few caveats when doing this. In particular, if you use drivers which probe the I2C bus for devices, you must make sure that the devices will be properly found on either the trunk or one of the multiplexer branches but not both. In fact this is with this specific case in mind that I decided to wait for the i2c device driver binding model to be cleaned up before going on with full multiplexing support.
That being said, yes, you're correct, multiplexing where the multiplexer chip isn't on the bus itself is a lot easier and can already be done today in some cases.
Signature · 1 line
Jean Delvare
Quoted 16 lines · Jean Delvare
Quoted 16 lines
drivers/i2c/i2c-core.c +2
-2@@ -799,10 +799,10 @@ void i2c_del_driver(struct i2c_driver *driver) class_for_each_device(&i2c_adapter_class, NULL, driver, __detach_adapter); + mutex_unlock(&core_lock); + driver_unregister(&driver->driver); pr_debug("i2c-core: driver [%s] unregistered\n", driver->driver.name); - - mutex_unlock(&core_lock); } EXPORT_SYMBOL(i2c_del_driver);
Quoted 6 lines
Quoted 3 lines
Unluckely I'm not an i2c guru so I'd like to ask to you. If such modification is correct I can provide a simple support with no modifications to the i2c core! Apart this one, of course... :)
Honestly, I am not sure either. The locking model isn't completely cleared yet,
All legacy locking in the I2C core is problematic -- courtesy of the original (lack-of-)design. The core_lock is the worst offender; it's mostly there to support some wierdness when removing legacy drivers, and I recall thinking that it works more by luck (and lack of heavy use!) than correctness.
Quoted 8 lines
we still need to get rid of the 3 dozen remaining legacy i2c chip drivers before we can finish it. I _think_ the above it correct, after all we do not hold the lock when calling
driver_register()so I can't see why we would need to hold it when callingdriver_unregister()... unless eitheri2c_client_release()ori2c_client_dev_release()needs this.David should know better, as he designed the thing in the first place.
No I didn't ... I worked on making it play better with the driver model, and simplifying a bunch of the rest. I gave up on fixing the legacy locking ... other than that patch to remove i2c_adapter.clist_lock, a while back, I think the only real fix is getting rid of those legacy calls (thus using only the driver-model internal locks).
Quoted 1 line
David, can you please comment on the proposed change?
If it's a legacy driver, it will already have been decoupled from devices, so the nastiest bit of stuff will have gotten the protection it needs. New-style drivers shouldn't care about the legacy stuff at all. So that much looks plausible.
But I don't have the patience to really analyse the other locking implications; there were some rude and un-obvious things lurking there, including abuse of complete() for the legacy i2c_client lifecycle.
- Dave
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jean" == Jean Delvare <[email protected]> writes:
Hi,
Jean> There are still a few caveats when doing this. In particular, Jean> if you use drivers which probe the I2C bus for devices, you Jean> must make sure that the devices will be properly found on Jean> either the trunk or one of the multiplexer branches but not Jean> both. In fact this is with this specific case in mind that I Jean> decided to wait for the i2c device driver binding model to be Jean> cleaned up before going on with full multiplexing support.
True. I unsually solve this by making sure the multiplexer starts in an unconnected state so the trunk probe doesn't find anything, or simply not use the old style probing.
Signature · 1 line
Bye, Peter Korsgaard
Quoted 6 lines · Jean Delvare
Thanks for keeping up with this despite my lack of support. I really hope that we can get this done this year.
Did you speak with Dave Miller already? Some months ago he proposed an horrible hack to implement support for I2C bus multiplexing, which I nacked and I pointed him to you.
No... I don't remember about it...
Quoted 6 lines
Last year I have been working with David Brownell of cleaning up i2c-core so that adding things like multiplexing support is easier, in particular when it comes to the locking model. It's not completely done yet but I think we're on the right track. My hope was that we could finish the cleanups first and add multiplexing support only then, but maybe I was too optimistic, the cleanups take longer than I expected.
I see.
Quoted 7 lines
Honestly, I am not sure either. The locking model isn't completely cleared yet, we still need to get rid of the 3 dozen remaining legacy i2c chip drivers before we can finish it. I _think_ the above it correct, after all we do not hold the lock when calling
driver_register()so I can't see why we would need to hold it when callingdriver_unregister()... unless eitheri2c_client_release()ori2c_client_dev_release()needs this.
That is exactly what I supposed to be. :)
Quoted 5 lines
I would like to propose that you create yourself an account on i2c.wiki.kernel.org. I've just added a "Work in progress" section on the main page so that other interested developers can keep track of what is happening. You could create a page describing your progress and things that need to be done. And I should do the same for my part.
Ok. Thanks.
Quoted 3 lines
Oh, and please stop calling the thing "virtual i2c adapter support". These adapters are very real. What you are working on is better described as "i2c bus multiplexing support".
I do.
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: giometti-AVVDYK/kqiJWk0Htik3J/[email protected] Linux Device Driver [email protected] Embedded Systems phone: +39 349 2432127 UNIX programming skype: rodolfo.giometti
Quoted 11 lines · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jean" == Jean Delvare <[email protected]> writes:
Hi,
Jean> Oh, and please stop calling the thing "virtual i2c adapter support". Jean> These adapters are very real. What you are working on is better Jean> described as "i2c bus multiplexing support".
Not only that, it's afaik about doing i2c bus multiplexing where the multiplexers are themselves i2c devices - E.G. stuff where you might need to recursively call into the i2c core.
I did exactly as you say. I have a custom board with a PCA9540 which is a i2c multiplexer controlled by i2c bus.
Quoted 5 lines
I have been doing i2c multiplexers where the multiplexers themselves are not controlled through i2c for years without problems.
I afaik even posted an example driver back when Dave posted his multiplex hack.
Did you post the code on this list?
However I'm going to publish my job on i2c multiplexers ASAP, maybe you can take a look at it and verify if it fixes your needs.
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: giometti-AVVDYK/kqiJWk0Htik3J/[email protected] Linux Device Driver [email protected] Embedded Systems phone: +39 349 2432127 UNIX programming skype: rodolfo.giometti
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Rodolfo" == Rodolfo Giometti <giometti-AVVDYK/kqiJWk0Htik3J/[email protected]> writes:
Hi,
Quoted 5 lines
Quoted 5 lines
I have been doing i2c multiplexers where the multiplexers themselves are not controlled through i2c for years without problems.
I afaik even posted an example driver back when Dave posted his multiplex hack.
Rodolfo> Did you post the code on this list?
Yes, but notice that it was for a multiplexer NOT controlled over I2C.
http://www.mail-archive.com/[email protected]/msg01539.html
Signature · 1 line
Bye, Peter Korsgaard
Quoted 12 lines · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jean" == Jean Delvare <[email protected]> writes:
Hi,
Jean> There are still a few caveats when doing this. In particular, Jean> if you use drivers which probe the I2C bus for devices, you Jean> must make sure that the devices will be properly found on Jean> either the trunk or one of the multiplexer branches but not Jean> both. In fact this is with this specific case in mind that I Jean> decided to wait for the i2c device driver binding model to be Jean> cleaned up before going on with full multiplexing support.
Quoted 3 lines
True. I unsually solve this by making sure the multiplexer starts in an unconnected state so the trunk probe doesn't find anything, or simply not use the old style probing.
Please keep in mind that the difficulty here is with probing itself, not just with old-style. The new binding model also has a detection mode, which is also affected. Making sure that the multiplexer is in an unconnected state initially isn't sufficient, as you can load I2C chip drivers at any later point in time and this will trigger a new detection cycle. And not all multiplexers can be disabled that way, some have always one outer channel enabled.
I've been thinking about it a bit and my conclusion is that detection is simply not compatible with multiplexed I2C buses in general. In specific cases (for example if there is no chip on the trunk) you can get it to work but I couldn't come up with a logic that always works. If anyone has ideas, these are welcome.
Signature · 1 line
Jean Delvare
Hi David,
Quoted 8 lines · David Brownell
Quoted 1 line · Jean Delvare
David, can you please comment on the proposed change?
If it's a legacy driver, it will already have been decoupled from devices, so the nastiest bit of stuff will have gotten the protection it needs. New-style drivers shouldn't care about the legacy stuff at all. So that much looks plausible.
Quoted 4 lines
But I don't have the patience to really analyse the other locking implications; there were some rude and un-obvious things lurking there, including abuse of
complete()for the legacyi2c_clientlifecycle.
This was my conclusion as well.
I think I'll try Rodoflo's patch locally and see if anything breaks. If things appear to work, I am ready to push the patch upstream, but obviously if anyone reports a locking issue after that, it would have to be reverted.
In the meantime I will try to find time again to convert more legacy drivers. My feeling is that we won't do much progress on multiplexing before we get rid of legacy drivers and their custom locking model.
Signature · 1 line
Jean Delvare
Quoted 16 lines · Rodolfo Giometti
Hello Jean,
I'm working again on virtual i2c adapter support :) since I received several letters asking for this support. That's why I'd like to have from you some advice in order to propose a working code.
Looking at latest i2c code I suppose we can modify the
i2c_del_driver()function as follow:drivers/i2c/i2c-core.c +0
-0@@ -799,10 +799,10 @@ void i2c_del_driver(struct i2c_driver *driver)class_for_each_device (&i2c_adapter_class,NULL , driver, __detach_adapter);
drivers/i2c/i2c-core.c void i2c_del_driver(struct i2c_driver *driver) · line 808
Quoted 10 lines
drivers/i2c/i2c-core.c +2
-2+mutex_unlock (&core_lock); +driver_unregister (&driver->driver);pr_debug ("i2c-core: driver [%s] unregistered\n" , driver->driver.name); - -mutex_unlock (&core_lock); }EXPORT_SYMBOL (i2c_del_driver);
What tree is this patch against? I can't get it to apply on top of 2.6.29-rc2.
Signature · 1 line
Jean Delvare
Quoted 8 lines · Jean Delvare
Quoted 8 lines
drivers/i2c/i2c-core.c +0
-0@@ -799,10 +799,10 @@ void i2c_del_driver(struct i2c_driver *driver)class_for_each_device (&i2c_adapter_class,NULL , driver, __detach_adapter);
Quoted 12 lines
drivers/i2c/i2c-core.c void i2c_del_driver(struct i2c_driver *driver) · line 808
Quoted 9 lines
drivers/i2c/i2c-core.c +2
-2+mutex_unlock (&core_lock); +driver_unregister (&driver->driver);pr_debug ("i2c-core: driver [%s] unregistered\n" , driver->driver.name); - -mutex_unlock (&core_lock); }EXPORT_SYMBOL (i2c_del_driver);What tree is this patch against? I can't get it to apply on top of 2.6.29-rc2.
It was against 2.6.29-rc2... let me attach a patch file, maybe I did cut&paste. :)
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: giometti-AVVDYK/kqiJWk0Htik3J/[email protected] Linux Device Driver [email protected] Embedded Systems phone: +39 349 2432127 UNIX programming skype: rodolfo.giometti
drivers/i2c/i2c-core.c +2 -2
@@ -799,10 +799,10 @@ void i2c_del_driver(struct i2c_driver *driver)class_for_each_device (&i2c_adapter_class,NULL , driver, __detach_adapter); +mutex_unlock (&core_lock); +driver_unregister (&driver->driver);pr_debug ("i2c-core: driver [%s] unregistered\n" , driver->driver.name); - -mutex_unlock (&core_lock); }EXPORT_SYMBOL (i2c_del_driver);
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jean" == Jean Delvare <[email protected]> writes:
True. I unsually solve this by making sure the multiplexer starts in an unconnected state so the trunk probe doesn't find anything, or simply not use the old style probing.
Jean> Please keep in mind that the difficulty here is with probing itself, Jean> not just with old-style. The new binding model also has a detection Jean> mode, which is also affected. Making sure that the multiplexer is in an Jean> unconnected state initially isn't sufficient, as you can load I2C chip Jean> drivers at any later point in time and this will trigger a new Jean> detection cycle. And not all multiplexers can be disabled that way, Jean> some have always one outer channel enabled.
Yes, but my code
(http://www.mail-archive.com/[email protected]/msg01539.html) does:
lock parent mutex
enable mux
do transfer on parent bus
disable mux
unlock parent
So that afaik shouldn't be a problem.
If your mux doesn't have an enable control and there's no unused connection you can use instead, then that ofcourse doesn't work.
Signature · 1 line
Bye, Peter Korsgaard
Am 2009-01-23 14:48:33, schrieb Rodolfo Giometti:
Quoted 7 lines · Peter Korsgaard
Quoted 3 lines
Not only that, it's afaik about doing i2c bus multiplexing where the multiplexers are themselves i2c devices - E.G. stuff where you might need to recursively call into the i2c core.
I did exactly as you say. I have a custom board with a PCA9540 which is a i2c multiplexer controlled by i2c bus.
Here too, but using a Maxim MAX7367 and MAX7369 and here you will run into trouble, because the first one is a Switch/Multiplexer and the second one a Multiplexer only.
Runing an Atmel AT91SAM7S and doing RAW coding is no problem, but using my Atmel AT91SAM7SE (64MB NAND, 32 MB SDRAM) using Linux is (currently) the hell.
Quoted 2 lines
Quoted 2 lines
I have been doing i2c multiplexers where the multiplexers themselves are not controlled through i2c for years without problems.
So you use those Multiplexer only for level switching where even on different ports the Address is unique? This is working with the Maxim chips too.
But in my "24V DC modular ATX PSU" (like a striped down UPS) I have on the first Port the standard ATX outputs and the other three Ports are connected to Expansion slots...
Since no one know, which Expansion modules are connected, I can not have unique addresses for the I²C devices...
Used Devices are:
DS1780 Hardware Monitor
DS28CZ04 4kbit EEPROM (stores module infos)
MAX1036 4-Channel 8-Bit ADC
MAX1038 12-Channel 8-Bit ADC
but not limited too.
Quoted 2 lines
However I'm going to publish my job on i2c multiplexers ASAP, maybe you can take a look at it and verify if it fixes your needs.
I hope I hear you soon...
Thanks, Greetings and nice Day/Evening
Michelle Konzack
Systemadministrator
24V Electronic Engineer
Tamay Dogan Network
Debian GNU/Linux Consultant
Signature · 6 lines
Linux-User #280138 with the Linux Counter, http://counter.li.org/ ##################### Debian GNU/Linux Consultant ##################### <http://www.tamay-dogan.net/> <http://www.can4linux.org/> Michelle Konzack Apt. 917 ICQ #328449886 +49/177/9351947 50, rue de Soultz MSN LinuxMichi +33/6/61925193 67100 Strasbourg/France IRC #Debian (irc.icq.com)
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Michelle" == Michelle Konzack <[email protected]> writes:
Hi,
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
I have been doing i2c multiplexers where the multiplexers themselves are not controlled through i2c for years without problems.
Michelle> So you use those Multiplexer only for level switching where Michelle> even on different ports the Address is unique? This is Michelle> working with the Maxim chips too.
No, there can (and there are) multiple chips with the same address. What I mean is simply that the mux is't controlled by writing I2C commands to it, but through something else (in this case a sram-like bus on a fpga).
Signature · 1 line
Bye, Peter Korsgaard