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

  # HG changeset patch
# User Eduardo Valentin <eduardo.valentin@nokia.com>
# 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.

Signed-off-by: Eduardo Valentin <eduardo.valentin@nokia.com>
---
 drivers/media/video/v4l2-common.c |   37 +++++++++++++++++++++++++++++++++++--
 include/linux/v4l2-common.h       |    8 ++++++++
 include/linux/v4l2-subdev.h       |    1 +
 3 files changed, 44 insertions(+), 2 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

On Sat, 6 Jun 2009, Hans Verkuil wrote:
> On Saturday 06 June 2009 13:59:19 Hans Verkuil wrote:
> > 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);

Maybe "addrs" could be changed to something like "probed_addrs" so it's
easier to tell that the two address fields are used differently?

Is v4l2_i2c_new_subdev() in effect just a wrapper around
v4l2_i2c_new_subdev_cfg() that calls it with NO_IRQ and NULL for irq and
platform_data?

And could v4l2_i2c_new_subdev_cfg() be done like this?

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)
{
	struct i2c_board_info info = {
		.addr = addr, .platform_data = platform_data, .irq = irq, };

	strlcpy(info.type, client_type, sizeof(info.type));
	return v4l2_i2c_new_subdev_board(v4l2_dev, adapter, module_name,
			                 &info, addrs);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

On Sat, 6 Jun 2009, Andy Walls wrote:
> +Alternatively, you can create the unsigned short array dynamically:
> +
> +struct v4l2_subdev *sd = v4l2_i2c_subdev(v4l2_dev, adapter,
> +	       "module_foo", "chipid", 0, V4L2_I2C_ADDRS(0x10, 0x12));
>
> Strictly speaking, that's not "dynamically" in the sense of the
> generated machine code - everything is going to come from the local
> stack and the initialized data space.  The compiler will probably be
> smart enough to generate an unnamed array in the initialized data space
> anyway, avoiding the use of local stack for the array. :)

No such luck, gcc will create an array on the stack and then initialize it
with a series of move word instructions.  It isn't even smart enough to
turn:

        movw    $1, (%esp)
        movw    $2, 2(%esp)
        movw    $3, 4(%esp)
        movw    $-1, 6(%esp)

into:
	movl	$0x00020001, (%esp)
	movl	$0xffff0003, 4(%esp)

Now, if you use a different syntax, and change this:

#define V4L2_I2C_ADDRS(addr, addrs...) \
        ((const unsigned short []){ addr, ## addrs, -1 })
#define bar(addrs...)   _bar(V4L2_I2C_ADDRS(addrs))

into this:

#define bar(addr, addrs...) \
        ({ const unsigned short _a[] = {addr, ## addrs, -1}; _bar(_a); })

If all the values are constants, then for the latter method only gcc will
will create an array in the initialized data segment and use that.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html