[i2c] How to find an i2c_adapter in the system

[Jean Delvare]

Hi Hendrik,

On Mon, 12 Nov 2007 23:23:34 +0100, Hendrik Sattler wrote:
> Am Montag 12 November 2007 schrieb Jean Delvare:
> > apanel driver which I think I reviewed back then, but that was never
> > merged upstream. Do you happen to know why?
> 
> There was a second approach lately. But that design is kind of limited and all
> mixed up into one file. I had the other parts by the time I suggested the
> patches for smb-alert but he latest posting of apanel drove me to finish it.
> I can post it here, if there is interest.

If the driver is not going to be accepted anyway, there's little
interest. I was just curious. When I spend time reviewing a driver, I
don't much like seeing it discarded, as it gives me the feeling that
I've been wasting my time.

> (...)
> > > However, I face several problems doing so:
> > > 1.
> > > For i2c_new_device(), I need a struct i2c_adapter*. How do I enumerate
> > > the adapters currently present?
> >
> > You don't. Individual I2C chip drivers aren't supposed to deal with
> > this, only i2c-core has access to the list of adapters.
> 
> The thing is that fjkeyinf is neither an i2c bus driver nor an i2c chip driver.
> It's just a database with additional information.
> The current way of calling i2c_get_adapter() doesn't work reliably because the
> module load order can be unlucky: fjkeyinf is loaded prior to i801 and thus
> cannot find it.

I told you not to use i2c_get_adapter() anyway, so this is hardly
relevant.

> I wrote a patch to i2c-core that can enumerate the adapter to interested
> parties outside of it. It used linux/notfier.h to do so. See the attachment.
> Pretty simple, didn't have time to test it. But should work.
> 
> Maybe something like this can be considered as an addition for drivers like
> fjkeyinf.
> 
> BTW: interesting busses are determined by the adapter id (I2C_HW_SMBUS_I801).
> And there can only be one i2c_i801 device in a system, AFAIK.

Right now, yes. However, there's nothing preventing Intel from adding a
second SMBus channel in the next iteration of their ICH south bridge,
and then you lose. Nvidia south bridges already support two SMBus
channels, for example, so it's really possible that Intel does the same
someday. Attempting to identify I2C channels by their adapter's .id is
broken by design. BTW, this .id field will most certainly be dropped as
some point in the future, exactly for this reason.

> ------------------------------------
> Index: git-linville/drivers/i2c/i2c-core.c
> ===================================================================
> --- git-linville.orig/drivers/i2c/i2c-core.c	2007-11-12 19:19:17.570488335 +0100
> +++ git-linville/drivers/i2c/i2c-core.c	2007-11-12 19:58:53.093569447 +0100
> @@ -34,6 +34,7 @@
>  #include <linux/mutex.h>
>  #include <linux/completion.h>
>  #include <asm/uaccess.h>
> +#include <linux/notifier.h>
>  
>  #include "i2c-core.h"
>  
> @@ -42,11 +43,34 @@
>  static LIST_HEAD(drivers);
>  static DEFINE_MUTEX(core_lists);
>  static DEFINE_IDR(i2c_adapter_idr);
> +static RAW_NOTIFIER_HEAD(notifiers);
>  
>  #define is_newstyle_driver(d) ((d)->probe || (d)->remove)
>  
>  /* ------------------------------------------------------------------------- */
>  
> +int i2c_notifier_register (struct notifier_block *notifier)
> +{
> +	struct i2c_adapter *adapter;
> +	int status = raw_notifier_chain_register(&notifiers, notifier);
> +	if (status)
> +		return status;
> +	list_for_each_entry(adapter, &adapters, list) {
> +		if (!try_module_get(adapter->owner))
> +			continue;
> +		(void)raw_notifier_call_chain(&notifiers, I2C_ADAPTER_ADD, adapter);
> +		module_put(adapter->owner);
> +	};
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(i2c_notifier_register);
> +
> +int i2c_notifier_unregister (struct notifier_block *notifier)
> +{
> +	return raw_notifier_chain_unregister(&notifiers, notifier);
> +}
> +EXPORT_SYMBOL_GPL(i2c_notifier_unregister);
> +
>  static int i2c_device_match(struct device *dev, struct device_driver *drv)
>  {
>  	struct i2c_client	*client = to_i2c_client(dev);
> @@ -358,6 +382,9 @@
>  	if (adap->nr < __i2c_first_dynamic_bus_num)
>  		i2c_scan_static_board_info(adap);
>  
> +	/* notify third parties about the new adapter */
> +	(void)raw_notifier_call_chain(&notifiers, I2C_ADAPTER_ADD, adap);
> +
>  	/* let legacy drivers scan this bus for matching devices */
>  	list_for_each(item,&drivers) {
>  		driver = list_entry(item, struct i2c_driver, list);
> Index: git-linville/include/linux/i2c.h
> ===================================================================
> --- git-linville.orig/include/linux/i2c.h	2007-11-12 19:53:45.306949310 +0100
> +++ git-linville/include/linux/i2c.h	2007-11-12 19:55:14.206919349 +0100
> @@ -378,6 +378,10 @@
>  
>  /* administration...
>   */
> +#define I2C_ADAPTER_ADD    (1 << 0)
> +extern int i2c_notifier_register(struct notifier_block *);
> +extern int i2c_notifier_unregister(struct notifier_block *);
> +
>  extern int i2c_add_adapter(struct i2c_adapter *);
>  extern int i2c_del_adapter(struct i2c_adapter *);
>  extern int i2c_add_numbered_adapter(struct i2c_adapter *);

We already have a way for i2c drivers to learn about new i2c_adapters,
it's called .attach_adapter and .detach_adapter (look at i2c-dev for an
example). If you really want to do things that way, just implement
these functions, don't reinvent your own notification system.

But as I explained before, that's certainly not the way to go. If
you're writing a new-style driver, you want to instantiate the device
from platform code, and let i2c-core attach the driver to this device
automatically.

-- 
Jean Delvare