Re: [PATCH 08/10 v2] v4l2-subdev: add a v4l2_i2c_subdev_board() function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Friday 22 May 2009 10:58:27 Eduardo Valentin wrote:
> Hi Hans and Guennadi,
>
> On Thu, May 21, 2009 at 05:33:48PM +0200, ext Guennadi Liakhovetski wrote:
> > Hi Hans,
> >
> > On Thu, 21 May 2009, Hans Verkuil wrote:
> > > On Friday 15 May 2009 19:20:10 Guennadi Liakhovetski wrote:
> > > > Introduce a function similar to v4l2_i2c_new_subdev() but taking a
> > > > pointer to a struct i2c_board_info as a parameter instead of a
> > > > client type and an I2C address, and make v4l2_i2c_new_subdev() a
> > > > wrapper around it.
> > > >
> > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> > > > ---
> > > >
> > > > Hans, renamed as you requested and updated to a (more) current
> > > > state.
> > >
> > > NAK. Not because it is a bad idea, but because you need to patch
> > > against the version in the v4l-dvb repo. The version in the kernel is
> > > missing a lot of the compatibility code which we unfortunately need
> > > to keep.
> > >
> > > Any function passing the board_info will be valid for kernels >=
> > > 2.6.26 only.
> >
> > Here's a quote from your earlier email.
> >
> > On Tue, 21 Apr 2009, Hans Verkuil wrote:
> > > The board_info struct didn't appear until 2.6.22, so that's certainly
> > > a cut-off point. Since the probe version of this call does not work
> > > on kernels < 2.6.26 the autoprobing mechanism is still used for those
> > > older kernels. I think it makes life much easier to require that
> > > everything that uses board_info needs kernel 2.6.26 at the minimum. I
> > > don't think that is an issue anyway for soc-camera. Unless there is a
> > > need to use soc-camera from v4l-dvb with kernels <2.6.26?
> >
> > So, will this my patch build and work with >= 2.6.22 or not? I really
> > would not like to consciously make code uglier now because of
> > compatibility with < 2.6.26 to make it better some time later again.
>
> I've to agree with Guennadi, I believe newer code should not suffer
> because of compatibility code, at least if it is possible. I also agree
> with you that we must keep compatibility with older drivers.

Let there be no doubt about it: it's very unfortunate that we have to do 
this. I really hope that in, say, one year we can just drop support for 
kernels pre-2.6.26. But my proposal from the beginning of the year to drop 
support for kernels < 2.6.22 demonstrated that not everyone is happy about 
that.

A quick note for Guennadi: the i2c_board_info and the new i2c API has been 
available since 2.6.22, but for the subdev support in v4l2 I've decided not 
to use the new i2c API for kernels < 2.6.26 due to a serious i2c core 
kernel bug that wasn't fixed until 2.6.26 (probing for the existence of an 
i2c device at certain addresses can cause an oops). Strictly speaking it 
would be possible to support board_info from 2.6.22 onwards, but going that 
way makes it very messy with lots of #ifdefs. I want to keep the simple 
rule to only support the new i2c API for 2.6.26 onwards.

> What I propose it to have the mechanism of .s_config available only for
> LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 26). Newer version can take
> advance of the new i2c api features.
>
> This is slightly different from what Hans proposed. The difference here
> is that we do not force newer drivers to use a callback only because
> of backward compatibility.
>
> Well, this is what I think of this problem, you may have a different
> point of view. What do you think?

No, that's not going to work. None of the USB and PCI drivers use 
i2c_board_info since they all can run on older kernels as well. So all 
those drivers need an s_config ops that they can call. Now, embedded 
drivers usually have no need to support older kernels, so these can use 
i2c_board_info directly.

If you know that an i2c driver is only ever called using this new 
v4l2_i2c_new_subdev_board_info function (I prefer the shorter name 
v4l2_i2c_subdev_board() BTW), then you can choose to not implement s_config 
in that i2c driver.

But i2c drivers that have to support older kernels as well should use 
s_config.

Note that s_config also needs an 'int irq' argument besides the 'void 
*platform_data'. And that we should also add a 
v4l2_i2c_probed_subdev_board() call.

Regards,

	Hans

>
> > Thanks
> > Guennadi
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
> > --
> > 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



-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
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

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux