Re: [PATCHv5 1 of 8] v4l2_subdev i2c: Add v4l2_i2c_new_subdev_board i2c helper function

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

 



On Saturday 06 June 2009 13:59:19 Hans Verkuil wrote:
> On Friday 29 May 2009 09:33:21 Eduardo Valentin wrote:
> > # HG changeset patch
> > # User Eduardo Valentin <eduardo.valentin@xxxxxxxxx>
> > # Date 1243414605 -10800
> > # Branch export
> > # Node ID 4fb354645426f8b187c2c90cd8528b2518461005
> > # Parent  142fd6020df3b4d543068155e49a2618140efa49
> > Device drivers of v4l2_subdev devices may want to have
> > board specific data. This patch adds an helper function
> > to allow bridge drivers to pass board specific data to
> > v4l2_subdev drivers.
> >
> > For those drivers which need to support kernel versions
> > bellow 2.6.26, a .s_config callback was added. The
> > idea of this callback is to pass board configuration
> > as well. In that case, subdev driver should set .s_config
> > properly, because v4l2_i2c_new_subdev_board will call
> > the .s_config directly. Of course, if we are >= 2.6.26,
> > the same data will be passed through i2c board info as well.
>
> Hi Eduardo,
>
> I finally had some time to look at this. After some thought I realized
> that the main problem is really that the API is becoming quite messy.
> Basically there are 9 different ways of loading and initializing a
> subdev:
>
> First there are three basic initialization calls: no initialization,
> passing irq and platform_data, and passing the i2c_board_info struct
> directly (preferred for drivers that don't need pre-2.6.26
> compatibility).
>
> And for each flavor you would like to see three different versions as
> well: one with a fixed known i2c address, one where you probe for a list
> of addresses, and one where you can probe for a single i2c address.
>
> I propose to change the API as follows:
>
> #define V4L2_I2C_ADDRS(addr, addrs...) \
> 	((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
>
> struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
>                 struct i2c_adapter *adapter,
>                 const char *module_name, const char *client_type,
> 		u8 addr, const unsigned short *addrs);
>
> struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
>                 struct i2c_adapter *adapter,
>                 const char *module_name, const char *client_type,
>                 int irq, void *platform_data,
>                 u8 addr, const unsigned short *addrs);
>
> /* Only available for kernels >= 2.6.26 */
> struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device
> *v4l2_dev, struct i2c_adapter *adapter, const char *module_name, struct
> i2c_board_info *info, const unsigned short *addrs);
>
> If you use a fixed address, then only set addr (or info.addr) and set
> addrs to NULL. If you want to probe for a list of addrs, then set addrs
> to the list of addrs. If you want to probe for a single addr, then use
> V4L2_I2C_ADDRS(addr) as the addrs argument. This constructs an array with
> just two entries. Actually, this macro can also create arrays with more
> entries.
>
> Note that v4l2_i2c_new_subdev will be an inline that calls
> v4l2_i2c_new_subdev_cfg with 0, NULL for the irq and platform_data.
>
> And for kernels >= 2.6.26 v4l2_i2c_new_subdev_cfg can be an inline
> calling v4l2_i2c_new_subdev_board.
>
> This approach reduces the number of functions to just one (not counting
> the inlines) and simplifies things all around. It does mean that all
> sources need to be changed, but if we go this route, then now is the time
> before the 2.6.31 window is closed. And I would also like to remove the
> '_new' from these function names. I never thought it added anything
> useful.
>
> Comments? If we decide to go this way, then I need to know soon so that I
> can make the changes before the 2.6.31 window closes.
>
> BTW, if the new s_config subdev call is present, then it should always be
> called. That way the subdev driver can safely do all of its
> initialization in s_config, no matter how it was initialized.
>
> Sorry about the long delay in replying to this: it's been very hectic
> lately at the expense of my v4l-dvb work.

I've done the initial conversion to the new API (no _cfg or _board version 
yet) in my ~hverkuil/v4l-dvb-subdev tree. It really simplifies things and 
if nobody objects then I'd like to get this in before 2.6.31.

Regards,

	Hans

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