On Sun, 2009-03-15 at 18:12 +0100, Jean Delvare wrote:
> Hi Hans,
>
> On Sun, 15 Mar 2009 13:44:01 +0100, Hans Verkuil wrote:
> > Hi Mauro, Jean,
> >
> > When converting the bttv driver to v4l2_subdev I found one probing conflict
> > between tvaudio and ir-kbd-i2c: address 0x96 (or 0x4b in 7-bit notation).
> >
> > It turns out that this is one and the same PIC16C54 device used on the
> > ProVideo PV951 board. This chip is used for both audio input selection and
> > for IR handling.
> >
> > But the tvaudio module does the audio part and the ir-kbd-i2c module does
> > the IR part. I have truly no idea how this should be handled in the new
> > situation. For that matter, I wonder whether it ever worked at all since my
> > understanding is that once you called i2c_attach_client for a particular
> > address, you cannot do that a second time. So depending on which module
> > happens to register itself first, you either have working audio or working
> > IR, but not both.
>
> You are right.
>
> This is the typical multifunction device problem. It isn't specifically
> related to I2C,
But the specific problem that Hans' brings up is precisely a Linux
kernel I2C subsystem *software* prohibition on two i2c_clients binding
to the same address on the same adapter.
>From linux/drivers/i2c/i2c-core.c:
static int __i2c_check_addr(struct device *dev, void *addrp)
{
struct i2c_client *client = i2c_verify_client(dev);
int addr = *(int *)addrp;
if (client && client->addr == addr)
return -EBUSY;
return 0;
}
[...]
int i2c_attach_client(struct i2c_client *client)
{
struct i2c_adapter *adapter = client->adapter;
int res;
/* Check for address business */
res = i2c_check_addr(adapter, client->addr);
if (res)
return res;
[...]
It seems like an artificial restriction: intended for safety, but
getting in the way when something like that is a valid need.
> the exact same problem happens for other devices, for
> example a PCI south bridge including hardware monitoring and SMBus, or
> a Super-I/O chip including hardware monitoring, parallel port,
> infrared, watchdog, etc. Linux currently only allows one driver to bind
> to a given device, so it becomes very difficult to make per-function
> drivers for such devices.
> I know that there was some work in progress to allow multiple drivers
> to bind to the same device. However it seems to be very slow because it
> is fundamentally incompatible with the device driver model as it was
> originally designed.
The driver model outside of the I2C subsystem?
Looking at the rest of i2c_attach_client() (that I didn't paste in
above), I dont' see how the call to device_register(&client->dev) would
care, as each i2c_client has it's own dev. Although I guess you might
get duplicately named sysfs directory entries like
/sys/devices/.../i2c-adapter/i2c-3/3-0096
Which could be a problem for accessing via the sysfs filesystem. But
that could be fixed in i2c_attach_client?
Then there's a matter of accessing the I2C device only by the address
which means the wrong client might be used. But since they both point
to the same address on the same device, does that really matter?
> In the meantime, one workaround is to list the multifunction device as
> supported by several drivers, and make the probe functions for this
> device fail, while still keeping a reference to the device. The
> reference lets you access the device, and is freed when you remove the
> drivers. See for example the via686a, vt8231 and i2c-viapro drivers.
> This approach may or may not be suitable for the ir-kbd-i2c and tvaudio
> drivers. One drawback is that you can't do power management on the
> device.
To me it would be more forward looking to add support in the I2C
subsystem for allowing multiple client drivers to use the same address
on the same adapter, instead of adding non-intuitive behavior to module
probe routines as a workaround. Integration of discrete I2C chip cores
into multifunction devices is likely to be a continuing trend.
The PCI subsystem handles single devices with multiple functions.
There, of course, the function number is in the logical device address.
For an single I2C chip with multiple functions, I've seen two types of
functional block separation provided: a separate I2C address per
functional block, and functions are separated by register address
ranges. The CX25843 leaps to mind as being of the second type. There
are register blocks for the basic device, the analog front end, the
consumer IR device, the video decoding, the broadcast audio decoding,
and AC97 interface functions.
> As far as the PIC16C54 is concerned, another possibility would be to
> move support to a dedicated driver. Depending on how much code is common
> between the PIC16C54 and the other supported devices, the new driver
> may either be standalone, or rely on functions exported by the
> ir-kbd-i2c and tvaudio modules.
I'll guess that solution is probably the path of least resistance for
the problem at hand. It seems like a workaround for design decision
made in the I2C subsystem long ago though.
Regards,
Andy
> But this is all said without having much knowledge of the bttv, tvaudio
> and ir-kbd-i2c drivers, so I might as well be completely off track.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
