Registering I2C devices on X86

Registering I2C devices on X86

[Richard Röjfors]

Hi,

I have a general question regarding the best way of registering I2C devices on a X86 system.

On ARM I would have done it in the board config, pretty straight forward.

On this X86 system the I2C bus is a PCI device, and different I2C devices might be tied into the bus 
depending on which board the device is populated on.

So my idea is to create a "mapping" driver. It opens the I2C adapter, creates platform data for each 
I2C device and add them by calling i2c_new_device. I don't find any better way since platform data 
must be created and also translation from GPIO pins to interrupt numbers.

Any ideas or suggestions?

Thanks in advance
--Richard

Re: Registering I2C devices on X86

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 329 bytes --]

> Any ideas or suggestions?

Do it in userspace, or is this too late?

Documentation/i2c/instantiating-devices lists all options you have.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Registering I2C devices on X86

[Jean Delvare]

On Wed, 2 Jun 2010 12:36:50 +0200, Wolfram Sang wrote:
> > Any ideas or suggestions?
> 
> Do it in userspace, or is this too late?

The user-space interface only works for simple cases: you can't pass
platform data or irq numbers. If someone needs to do this, we need an
extended interface, probably using configfs.

> Documentation/i2c/instantiating-devices lists all options you have.

Richard, look at drivers/i2c/busses/i2c-i801.c, function i801_probe():
you'll see an example of per-platform I2C device instantiation on x86.
I'm not claiming it is elegant, but it works.

-- 
Jean Delvare

Re: Registering I2C devices on X86

[Richard Röjfors]

Hi,

Thanks for quick feedback!

On 06/02/2010 01:04 PM, Jean Delvare wrote:
> On Wed, 2 Jun 2010 12:36:50 +0200, Wolfram Sang wrote:
>>> Any ideas or suggestions?
>>
>> Do it in userspace, or is this too late?
>
> The user-space interface only works for simple cases: you can't pass
> platform data or irq numbers. If someone needs to do this, we need an
> extended interface, probably using configfs.

Yeah I think it's a common case to provide platform data and interrupt numbers. At least I see the 
need for it on our platforms.

>
>> Documentation/i2c/instantiating-devices lists all options you have.
>
> Richard, look at drivers/i2c/busses/i2c-i801.c, function i801_probe():
> you'll see an example of per-platform I2C device instantiation on x86.
> I'm not claiming it is elegant, but it works.

The "problem" I see is that the CPU and the chipset + I2C chip will be populated on several 
different boards. It would mean that the I2C bus driver would need knowledge of all the boards where 
it is used. And the bus driver itself can not really detect which board it's running on.

That is why I kept the I2C device setup in a separate driver. So each board would have a separate 
"setup" driver. Isn't that the most clean solution right now?

Thanks
--Richard

Re: Registering I2C devices on X86

[Jean Delvare]

On Wed, 02 Jun 2010 13:29:36 +0200, Richard Röjfors wrote:
> On 06/02/2010 01:04 PM, Jean Delvare wrote:
> > Richard, look at drivers/i2c/busses/i2c-i801.c, function i801_probe():
> > you'll see an example of per-platform I2C device instantiation on x86.
> > I'm not claiming it is elegant, but it works.
> 
> The "problem" I see is that the CPU and the chipset + I2C chip will be populated on several 
> different boards. It would mean that the I2C bus driver would need knowledge of all the boards where 
> it is used. And the bus driver itself can not really detect which board it's running on.
> 
> That is why I kept the I2C device setup in a separate driver. So each board would have a separate 
> "setup" driver. Isn't that the most clean solution right now?

I have to admit I don't clearly understand what you are doing. It
should become clearer when I see your code.

-- 
Jean Delvare

Re: Registering I2C devices on X86

[Richard Röjfors]

On 06/03/2010 08:21 AM, Jean Delvare wrote:
> On Wed, 02 Jun 2010 13:29:36 +0200, Richard Röjfors wrote:
>> On 06/02/2010 01:04 PM, Jean Delvare wrote:
>>> Richard, look at drivers/i2c/busses/i2c-i801.c, function i801_probe():
>>> you'll see an example of per-platform I2C device instantiation on x86.
>>> I'm not claiming it is elegant, but it works.
>>
>> The "problem" I see is that the CPU and the chipset + I2C chip will be populated on several
>> different boards. It would mean that the I2C bus driver would need knowledge of all the boards where
>> it is used. And the bus driver itself can not really detect which board it's running on.
>>
>> That is why I kept the I2C device setup in a separate driver. So each board would have a separate
>> "setup" driver. Isn't that the most clean solution right now?
>
> I have to admit I don't clearly understand what you are doing. It
> should become clearer when I see your code.
>

The I2C bus driver used is a standard one and the setup of I2C devices depends on the actual board 
the chips are deployed on.

I do stuff like this, example of a driver for one specific board (there are more I2C devices to come):

#include <linux/gpio.h>
#include <linux/i2c.h>
#include <linux/i2c/tsc2007.h>

#define DRIVER_NAME "the_module"

static int	i2c_bus = 0;
static unsigned	tsc2007_irq_pin = 102;

static __devinitdata struct tsc2007_platform_data tsc2007_platform_data = {
	.model = 2007,
	.x_plate_ohms = 200
};

static __initdata struct i2c_board_info tsc2007_i2c_board_info = {
	I2C_BOARD_INFO("tsc2007", 0x48),
	.platform_data = &tsc2007_platform_data,
	/* irq to be filled in runtime */
};

static struct i2c_client *tsc2007_client;

static __init int the_module_get_irq(unsigned gpio_pin)
{
	int err;

	err = gpio_request(gpio_pin, DRIVER_NAME);
	if (err)
		return err;

	err = gpio_direction_input(gpio_pin);
	if (err)
		goto err;

	err = gpio_to_irq(gpio_pin);
	if (err < 0)
		goto err;

	return err;
err:
	gpio_free(tsc2007_irq_pin);
	return err;
}

static __init int the_module_init(void)
{
	struct i2c_adapter *adapt;
	int err;

	adapt = i2c_get_adapter(i2c_bus);
	if (!adapt) {
		printk(KERN_ERR "%s: Failed to get I2C adapter\n", __func__);
		return -ENODEV;
	}

	err = the_module_get_irq(tsc2007_irq_pin);
	if (err < 0)
		goto put_adapter;
	tsc2007_i2c_board_info.irq = err;

	/* add in the devices on the bus */
	tsc2007_client = i2c_new_device(adapt, &tsc2007_i2c_board_info);
	if (!tsc2007_client)
		goto free_tsc2007_pin;

	i2c_put_adapter(adapt);

	return 0;

free_tsc2007_pin:
	gpio_free(tsc2007_irq_pin);
put_adapter:
	i2c_put_adapter(adapt);

	return err;
}

static void __exit the_module_exit(void)
{
	i2c_unregister_device(tsc2007_client);
}

Re: Registering I2C devices on X86

[Jean Delvare]

Hi Richard,

Sorry for the late reply, your post slipped through the cracks.

On Thu, 03 Jun 2010 14:39:08 +0200, Richard Röjfors wrote:
> On 06/03/2010 08:21 AM, Jean Delvare wrote:
> > On Wed, 02 Jun 2010 13:29:36 +0200, Richard Röjfors wrote:
> >> The "problem" I see is that the CPU and the chipset + I2C chip will be populated on several
> >> different boards. It would mean that the I2C bus driver would need knowledge of all the boards where
> >> it is used. And the bus driver itself can not really detect which board it's running on.
> >>
> >> That is why I kept the I2C device setup in a separate driver. So each board would have a separate
> >> "setup" driver. Isn't that the most clean solution right now?
> >
> > I have to admit I don't clearly understand what you are doing. It
> > should become clearer when I see your code.
> 
> The I2C bus driver used is a standard one and the setup of I2C devices depends on the actual board 
> the chips are deployed on.
> 
> I do stuff like this, example of a driver for one specific board (there are more I2C devices to come):
> 
> #include <linux/gpio.h>
> #include <linux/i2c.h>
> #include <linux/i2c/tsc2007.h>
> 
> #define DRIVER_NAME "the_module"
> 
> static int	i2c_bus = 0;
> static unsigned	tsc2007_irq_pin = 102;
> 
> static __devinitdata struct tsc2007_platform_data tsc2007_platform_data = {
> 	.model = 2007,
> 	.x_plate_ohms = 200
> };
> 
> static __initdata struct i2c_board_info tsc2007_i2c_board_info = {
> 	I2C_BOARD_INFO("tsc2007", 0x48),
> 	.platform_data = &tsc2007_platform_data,
> 	/* irq to be filled in runtime */
> };
> 
> static struct i2c_client *tsc2007_client;
> 
> static __init int the_module_get_irq(unsigned gpio_pin)
> {
> 	int err;
> 
> 	err = gpio_request(gpio_pin, DRIVER_NAME);
> 	if (err)
> 		return err;
> 
> 	err = gpio_direction_input(gpio_pin);
> 	if (err)
> 		goto err;
> 
> 	err = gpio_to_irq(gpio_pin);
> 	if (err < 0)
> 		goto err;
> 
> 	return err;
> err:
> 	gpio_free(tsc2007_irq_pin);
> 	return err;
> }
> 
> static __init int the_module_init(void)
> {
> 	struct i2c_adapter *adapt;
> 	int err;
> 
> 	adapt = i2c_get_adapter(i2c_bus);

How do you know the value of i2c_bus? This is the key problem. If you
do not declare your I2C chips at the platform data level, there are no
I2C bus numbers reserved for static numbering. And even if you managed
to change that, the I2C adapters on x86 do not ask for a specific bus
number: they pick the first one available.

Maybe it happens to work for you right now, but this is very fragile.
I2C bus number 0 could become something completely different (for
example a DDC channel on the graphics adapter) at any time, depending
on which drivers are included in your kernel, the order in which they
are loaded/linked, and which exact hardware you run on.

If you want to take this approach, you need to extend the current API
first. You need a way to reserve static I2C bus numbers without
declaring devices on them. And you need to change the bus drivers (e.g.
i2c-i801.c) to request this specific bus number under specific
circumstances. So you won't avoid platform/machine specific code in the
I2C bus
driver.

The above is certainly doable, but if you need to do that kind of
thing, then it's probably time to rethink the whole thing. If x86
starts being used for embedded-style applications, then the
platform/machine handling should be adjusted. I see no reason why the
same approach that works for ARM platforms wouldn't work for x86
platforms as well.


> 	if (!adapt) {
> 		printk(KERN_ERR "%s: Failed to get I2C adapter\n", __func__);
> 		return -ENODEV;
> 	}
> 
> 	err = the_module_get_irq(tsc2007_irq_pin);
> 	if (err < 0)
> 		goto put_adapter;
> 	tsc2007_i2c_board_info.irq = err;
> 
> 	/* add in the devices on the bus */
> 	tsc2007_client = i2c_new_device(adapt, &tsc2007_i2c_board_info);
> 	if (!tsc2007_client)
> 		goto free_tsc2007_pin;
> 
> 	i2c_put_adapter(adapt);
> 
> 	return 0;
> 
> free_tsc2007_pin:
> 	gpio_free(tsc2007_irq_pin);
> put_adapter:
> 	i2c_put_adapter(adapt);
> 
> 	return err;
> }
> 
> static void __exit the_module_exit(void)
> {
> 	i2c_unregister_device(tsc2007_client);
> }


-- 
Jean Delvare

Re: Registering I2C devices on X86

[Ben Dooks]

On Wed, Jun 02, 2010 at 12:12:00PM +0200, Richard R?jfors wrote:
> Hi,
> 
> I have a general question regarding the best way of registering I2C devices on a X86 system.
> 
> On ARM I would have done it in the board config, pretty straight forward.
> 
> On this X86 system the I2C bus is a PCI device, and different I2C
> devices might be tied into the bus depending on which board the
> device is populated on.

Hmm, if it is a PCI device, then surely you should know the PCI ID
you are binding too?
 
> So my idea is to create a "mapping" driver. It opens the I2C
> adapter, creates platform data for each I2C device and add them by
> calling i2c_new_device. I don't find any better way since platform
> data must be created and also translation from GPIO pins to
> interrupt numbers.

That might be a way, however there's not really a good way to add
interrupts dynamically.

-- 
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'