Dynamically-allocated i2c_device_id vs MODULE_DEVICE_TABLE

I currently have this in my code:

static const struct i2c_device_id cs4270_id[] = {
	{"cs4270", 0},
	{}
};

MODULE_DEVICE_TABLE(i2c, cs4270_id);

static struct i2c_driver cs4270_i2c_driver = {
	.driver = {
		.name = "cs4270",
		.owner = THIS_MODULE,
	},
	.id_table = cs4270_id,
	.probe = cs4270_i2c_probe,
	.remove = cs4270_i2c_remove,
};

ret = i2c_add_driver(&cs4270_i2c_driver);

I would like to use the i2c_device_id.driver_data variable to pass private data to my cs4270_i2c_probe() function. So it will look like this:

	socdev = kmalloc(...);
	c24270_id.driver_data = socdev;
	i2c_add_driver(&cs4270_i2c_driver);

And then in my cs4270_i2c_probe(), I would do this:

static int cs4270_i2c_probe(struct i2c_client *i2c_client,
const struct i2c_device_id *id)

{
	struct X *socdev = (struct X *) id->driver_data.

The problem I'm having is the MODULE_DEVICE_TABLE. What I really should be doing is this:

	socdev = kmalloc(...);
	struct i2c_driver *cs4270_i2c_driver = kmalloc(...);
	struct i2c_device_id *cs4270_id = kmalloc(...);
	cs4270_i2c_driver->id_table = cs4270_id;
	c24270_id->driver_data = socdev;
	i2c_add_driver(cs4270_i2c_driver);

But if I do this, then I can't use MODULE_DEVICE_TABLE. So I have two questions:

1) What happens if I don't use MODULE_DEVICE_TABLE to identify my I2C ID table?

2) Is there a way to mimic the behavior of MODULE_DEVICE_TABLE on a dynamically-created ID table?

Timur Tabi
Linux kernel developer at Freescale

I'm not sure what you are trying to achieve. The id table has one entry per chip. It is static data, all instances of the chip should have the same ID entry.

Look at drivers/of_i2c.c and how it passed in dynamic data.

Also, I just added of_find_i2c_device_by_node() so you can follow references in the device tree.
i2s driver loads and uses the codec-handle to locate the i2c entry.

                i2s@2200 { /* PSC2 in i2s mode */
                        compatible =
"fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
                        cell-index = <1>;
                        reg = <0x2200 0x100>;
                        interrupts = <0x2 0x2 0x0>;
                        interrupt-parent = <&mpc5200_pic>;
                        codec-handle = <&tas0>;
                };

               i2c@3d00 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        compatible =
"fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
                        cell-index = <0>;
                        reg = <0x3d00 0x40>;
                        interrupts = <0x2 0xf 0x0>;
                        interrupt-parent = <&mpc5200_pic>;
                        fsl5200-clocking;

                        tas0:codec@1b {
                                compatible = "ti,tas5504";
                                reg = <0x1b>;
                        };
                        clock0:clock@68 {
                                compatible = "maxim,max9485";
                                reg = <0x68>;
                        };
                };

Jon Smirl
[email protected]

This is because the ASoC core still hasn't quite got the idea of instantiating the differnet bits of the device separately. Really what you're having to do here is fairly nasty and there's no good reason for it to be supported - if something is being registered dynamically and passing instantiation specific data around it really should be the device and not the driver. The fact that attempting to do this looks bad is a win on the part of the I2C core.

For now an idiomatic solution for ASoC drivers is to have a single static variable that you use to get the socdev through.

What we need is a way for the codec driver to do its stuff without needing a socdev.

I think I see your point, though. I'm calling i2c_add_driver, which technically could result in my i2c_probe function being called multiple times, if there are multiple matches in the device tree. I'll have to check that out on Monday.

Well, that's what I do today. I was hoping to avoid that, but if I'm right about i2c_add_driver, then this trick doesn't really work either.

Timur Tabi
Linux Kernel Developer @ Freescale

Yes, it will - any device specific data must be registered when the device is registered. You may want to look at how the wm8900 handles this, it makes use of the DAI registration functions to make this a little nicer. It does still have the same problem but is much closer to the end result and means you can start registering the driver normally now.

This really isn't something that can be worked around well in the individual drivers.

you seem to have managed to short-circuit part of the device creation process. You do not need to pass the data via the device driver, you should pass it when creating the device.

If you look at i2c_board_info which can be passed into i2c_new_device or similar functions, there is a platform_data field you can fill out and this is passed in to your probe routine in the i2c device.

Ben ([email protected], http://www.fluff.org/)

  'a smiley only costs 4 bytes'

Yes, I think my code is broken.

I don't think this will work for me. I'm running this on a PowerPC system, and we use a device tree to represent the I2C devices on the various I2C buses. My driver does not call i2c_new_device. This is done for me in of_register_i2c_devices(). As soon as my driver calls i2c_add_driver(), my I2C probe function will be called, once for each I2C device defined in the device tree. So I don't think I can update the i2c_board_info structure.

Timur Tabi
Linux kernel developer at Freescale

Is it possible to use of_find_i2c_device_by_node() during initialization of your I2S device and poke the OF specific info into the codec at that point?

Jon Smirl
[email protected]