New style i2c drivers for ALSA SoC

New style i2c drivers for ALSA SoC

[Ryan Mallon]

I am writing drivers to support ALSA SoC for an ARM based system. The
system has an i2c codec. I want to use the new style driver for the
codec driver, but I cannot get it to work properly. In my codec driver
(sound/soc/codecs/tlv320aic2x.c) I have:

static struct i2c_driver aic2x_i2c_driver = {
	.driver		= {
		.name	= "tlv320aic2x",
		.owner	= THIS_MODULE,
	},
	.probe		= aic2x_i2c_probe,
	.remove		= aic2x_i2c_remove,
};

static int __init aic2x_init(void)
{
	return i2c_add_driver(&aic2x_i2c_driver);
}

static void __exit aic2x_exit(void)
{
	i2c_del_driver(&aic2x_i2c_driver);
}

module_init(aic2x_init);
module_exit(aic2x_exit);

Putting i2c_register_board_info either in my machine file in the
arch/arm/ directory, or the machine file in the sound/soc/ directory
does not seem to work. The driver does correctly register, ie I see the
following in dmesg (although much later than the other i2c drivers):

i2c-core: driver [tlv320aic2x] registered

But its probe function is never called. The other i2c codec drivers in
the sound/soc/codecs directory are all using the legacy i2c driver model.

I also looked at the i2c_new_device function. I'm not sure if this is
what I want, but I don't know where to get the adapter structure to pass
to it. The other drivers I found which use this function are creating
their own i2c adapter (usually bit-bashed) for talking to the i2c
device, which is not what I want to do.

Finally, a stylistic question: Should the i2c_board_info (or similar)
for a codec device be defined in the machine initialisation code
(arch/arm/ directory), or in the sound/soc machine file?

Thanks,
~Ryan

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: New style i2c drivers for ALSA SoC

[Jean Delvare]

Hi Ryan,

On Mon, 12 May 2008 12:57:11 +1200, Ryan Mallon wrote:
> I am writing drivers to support ALSA SoC for an ARM based system. The
> system has an i2c codec. I want to use the new style driver for the
> codec driver, but I cannot get it to work properly. In my codec driver
> (sound/soc/codecs/tlv320aic2x.c) I have:
> 
> static struct i2c_driver aic2x_i2c_driver = {
> 	.driver		= {
> 		.name	= "tlv320aic2x",
> 		.owner	= THIS_MODULE,
> 	},
> 	.probe		= aic2x_i2c_probe,
> 	.remove		= aic2x_i2c_remove,
> };
> 
> static int __init aic2x_init(void)
> {
> 	return i2c_add_driver(&aic2x_i2c_driver);
> }
> 
> static void __exit aic2x_exit(void)
> {
> 	i2c_del_driver(&aic2x_i2c_driver);
> }
> 
> module_init(aic2x_init);
> module_exit(aic2x_exit);
> 
> Putting i2c_register_board_info either in my machine file in the
> arch/arm/ directory, or the machine file in the sound/soc/ directory
> does not seem to work. The driver does correctly register, ie I see the
> following in dmesg (although much later than the other i2c drivers):
> 
> i2c-core: driver [tlv320aic2x] registered
> 
> But its probe function is never called. The other i2c codec drivers in
> the sound/soc/codecs directory are all using the legacy i2c driver model.

What data are you passing to i2c_register_board_info?

Since 2.6.26-rc1, you can and should provide a list of devices
supported by your driver. Your driver definition would look like:

static const struct i2c_device_id aic2x_i2c_id[] = {
	{ "tlv320aic2x", 0 },
	{ }
};
MODULE_DEVICE_TABLE(i2c, aic2x_i2c_id);

static struct i2c_driver aic2x_i2c_driver = {
	.driver		= {
		.name	= "tlv320aic2x",
		.owner	= THIS_MODULE,
	},
	.probe		= aic2x_i2c_probe,
	.remove		= aic2x_i2c_remove,
	.id_table	= aic2x_i2c_id,
};

Also note that there's already a driver in the kernel tree
(drivers/media/video/tlv320aic23b.c) for apparently the same hardware,
so maybe there's some code to share.

> I also looked at the i2c_new_device function. I'm not sure if this is
> what I want, but I don't know where to get the adapter structure to pass
> to it. The other drivers I found which use this function are creating
> their own i2c adapter (usually bit-bashed) for talking to the i2c
> device, which is not what I want to do.

i2c_new_device() is meant to instantiate I2C devices at run-time. It is
mostly useful for TV adapters and similar where you don't know in
advance if it will be there and what i2c bus number it will receive.
For embedded designs where you know exactly which devices you have at
build time, i2c_register_board_info() is the way to go.

> Finally, a stylistic question: Should the i2c_board_info (or similar)
> for a codec device be defined in the machine initialisation code
> (arch/arm/ directory), or in the sound/soc machine file?

Machine initialization code.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: New style i2c drivers for ALSA SoC

[Ryan Mallon]

Jean Delvare wrote:
> Hi Ryan,
> 
> On Mon, 12 May 2008 12:57:11 +1200, Ryan Mallon wrote:
>> I am writing drivers to support ALSA SoC for an ARM based system. The
>> system has an i2c codec. I want to use the new style driver for the
>> codec driver, but I cannot get it to work properly. In my codec driver
>> (sound/soc/codecs/tlv320aic2x.c) I have:
>>
>> static struct i2c_driver aic2x_i2c_driver = {
>> 	.driver		= {
>> 		.name	= "tlv320aic2x",
>> 		.owner	= THIS_MODULE,
>> 	},
>> 	.probe		= aic2x_i2c_probe,
>> 	.remove		= aic2x_i2c_remove,
>> };
>>
>> static int __init aic2x_init(void)
>> {
>> 	return i2c_add_driver(&aic2x_i2c_driver);
>> }
>>
>> static void __exit aic2x_exit(void)
>> {
>> 	i2c_del_driver(&aic2x_i2c_driver);
>> }
>>
>> module_init(aic2x_init);
>> module_exit(aic2x_exit);
>>
>> Putting i2c_register_board_info either in my machine file in the
>> arch/arm/ directory, or the machine file in the sound/soc/ directory
>> does not seem to work. The driver does correctly register, ie I see the
>> following in dmesg (although much later than the other i2c drivers):
>>
>> i2c-core: driver [tlv320aic2x] registered
>>
>> But its probe function is never called. The other i2c codec drivers in
>> the sound/soc/codecs directory are all using the legacy i2c driver model.
> 
> What data are you passing to i2c_register_board_info?
> 
> Since 2.6.26-rc1, you can and should provide a list of devices
> supported by your driver. Your driver definition would look like:
> 
> static const struct i2c_device_id aic2x_i2c_id[] = {
> 	{ "tlv320aic2x", 0 },
> 	{ }
> };
> MODULE_DEVICE_TABLE(i2c, aic2x_i2c_id);

Done. I had thought this was option if a driver only supported a single
device?

> static struct i2c_driver aic2x_i2c_driver = {
> 	.driver		= {
> 		.name	= "tlv320aic2x",
> 		.owner	= THIS_MODULE,
> 	},
> 	.probe		= aic2x_i2c_probe,
> 	.remove		= aic2x_i2c_remove,
> 	.id_table	= aic2x_i2c_id,
> };
> 
> Also note that there's already a driver in the kernel tree
> (drivers/media/video/tlv320aic23b.c) for apparently the same hardware,
> so maybe there's some code to share.

Yes. The tlv320aic23b is both an audio codec and a video decoder of some
sort. We only use the audio codec part on our particular board. The
driver under drivers/media/video is pretty minimal, and appears that it
may have been hard coded for a particular graphics card that it appears
on? The driver I have for the audio codec is already substantially
larger. There is also the problem that the soc codec stuff should live
under sound/soc/codecs and video decoders should live under
drivers/media/video. Assuming I post patches for my work at some stage
(which I intend to do), perhaps it is easier to have two drivers to
start with, and then merge them if people have hardware which is using
both the audio and video codec functionality?

>> I also looked at the i2c_new_device function. I'm not sure if this is
>> what I want, but I don't know where to get the adapter structure to pass
>> to it. The other drivers I found which use this function are creating
>> their own i2c adapter (usually bit-bashed) for talking to the i2c
>> device, which is not what I want to do.
> 
> i2c_new_device() is meant to instantiate I2C devices at run-time. It is
> mostly useful for TV adapters and similar where you don't know in
> advance if it will be there and what i2c bus number it will receive.
> For embedded designs where you know exactly which devices you have at
> build time, i2c_register_board_info() is the way to go.
> 
>> Finally, a stylistic question: Should the i2c_board_info (or similar)
>> for a codec device be defined in the machine initialisation code
>> (arch/arm/ directory), or in the sound/soc machine file?
>
> Machine initialization code.
> 

Okay, thanks. I got it working. Part of the problem turned out to be a
dodgy i2c bus on the board I was using.

~Ryan

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: New style i2c drivers for ALSA SoC

[Jean Delvare]

Hi Ryan,

On Tue, 13 May 2008 08:52:50 +1200, Ryan Mallon wrote:
> Jean Delvare wrote:
> > Since 2.6.26-rc1, you can and should provide a list of devices
> > supported by your driver. Your driver definition would look like:
> > 
> > static const struct i2c_device_id aic2x_i2c_id[] = {
> > 	{ "tlv320aic2x", 0 },
> > 	{ }
> > };
> > MODULE_DEVICE_TABLE(i2c, aic2x_i2c_id);
> 
> Done. I had thought this was option if a driver only supported a single
> device?

It is optional up to and including 2.6.26-rc2, but hopefully I'll be
done with the cleanups for 2.6.26-rc3 and using .id_table will be the
only way for a driver to match its devices.

> 
> > static struct i2c_driver aic2x_i2c_driver = {
> > 	.driver		= {
> > 		.name	= "tlv320aic2x",
> > 		.owner	= THIS_MODULE,
> > 	},
> > 	.probe		= aic2x_i2c_probe,
> > 	.remove		= aic2x_i2c_remove,
> > 	.id_table	= aic2x_i2c_id,
> > };
> > 
> > Also note that there's already a driver in the kernel tree
> > (drivers/media/video/tlv320aic23b.c) for apparently the same hardware,
> > so maybe there's some code to share.
> 
> Yes. The tlv320aic23b is both an audio codec and a video decoder of some
> sort. We only use the audio codec part on our particular board. The
> driver under drivers/media/video is pretty minimal, and appears that it
> may have been hard coded for a particular graphics card that it appears
> on? The driver I have for the audio codec is already substantially
> larger. There is also the problem that the soc codec stuff should live
> under sound/soc/codecs and video decoders should live under
> drivers/media/video. Assuming I post patches for my work at some stage
> (which I intend to do), perhaps it is easier to have two drivers to
> start with, and then merge them if people have hardware which is using
> both the audio and video codec functionality?

I don't know what it the best approach myself. I just wanted you to be
aware that this other driver existed.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: New style i2c drivers for ALSA SoC

[Ben Dooks]

On Mon, May 12, 2008 at 12:57:11PM +1200, Ryan Mallon wrote:
> I am writing drivers to support ALSA SoC for an ARM based system. The
> system has an i2c codec. I want to use the new style driver for the
> codec driver, but I cannot get it to work properly. In my codec driver
> (sound/soc/codecs/tlv320aic2x.c) I have:
> 
> static struct i2c_driver aic2x_i2c_driver = {
> 	.driver		= {
> 		.name	= "tlv320aic2x",
> 		.owner	= THIS_MODULE,
> 	},
> 	.probe		= aic2x_i2c_probe,
> 	.remove		= aic2x_i2c_remove,
> };
> 
> static int __init aic2x_init(void)
> {
> 	return i2c_add_driver(&aic2x_i2c_driver);
> }
> 
> static void __exit aic2x_exit(void)
> {
> 	i2c_del_driver(&aic2x_i2c_driver);
> }
> 
> module_init(aic2x_init);
> module_exit(aic2x_exit);

I belive that you need a bit of glue to bind your SoC system
to the codec and audio hardware. ASoC is littered with a number
of examples of how to do it.

> Finally, a stylistic question: Should the i2c_board_info (or similar)
> for a codec device be defined in the machine initialisation code
> (arch/arm/ directory), or in the sound/soc machine file?

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

  'a smiley only costs 4 bytes'

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c