Hi Hans
Thanks for reviewing.
On Fri, 19 Apr 2013, Hans Verkuil wrote:
> On Thu April 18 2013 23:35:27 Guennadi Liakhovetski wrote:
> > This struct shall be used by subdevice drivers to pass per-subdevice data,
> > e.g. power supplies, to generic V4L2 methods, at the same time allowing
> > optional host-specific extensions via the host_priv pointer. To avoid
> > having to pass two pointers to those methods, add a pointer to this new
> > struct to struct v4l2_subdev.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> > ---
> > include/media/v4l2-subdev.h | 13 +++++++++++++
> > 1 files changed, 13 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index eb91366..b15c6e0 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -561,6 +561,17 @@ struct v4l2_subdev_internal_ops {
> > /* Set this flag if this subdev generates events. */
> > #define V4L2_SUBDEV_FL_HAS_EVENTS (1U << 3)
> >
> > +struct regulator_bulk_data;
> > +
> > +struct v4l2_subdev_platform_data {
> > + /* Optional regulators uset to power on/off the subdevice */
> > + struct regulator_bulk_data *regulators;
> > + int num_regulators;
> > +
> > + /* Per-subdevice data, specific for a certain video host device */
> > + void *host_priv;
> > +};
> > +
> > /* Each instance of a subdev driver should create this struct, either
> > stand-alone or embedded in a larger struct.
> > */
> > @@ -589,6 +600,8 @@ struct v4l2_subdev {
> > /* pointer to the physical device */
> > struct device *dev;
> > struct v4l2_async_subdev_list asdl;
> > + /* common part of subdevice platform data */
> > + struct v4l2_subdev_platform_data *pdata;
> > };
> >
> > static inline struct v4l2_subdev *v4l2_async_to_subdev(
> >
>
> Sorry, this is the wrong approach.
>
> This is data that is of no use to the subdev driver itself. It really is
> v4l2_subdev_host_platform_data, and as such must be maintained by the bridge
> driver.
I don't think so. It has been discussed and agreed upon, that only
subdevice drivers know when to switch power on and off, because only they
know when they need to access the hardware. So, they have to manage
regulators. In fact, those regulators supply power to respective
subdevices, e.g. a camera sensor. Why should the bridge driver manage
them? The V4L2 core can (and probably should) provide helper functions for
that, like soc-camera currently does, but in any case it's the subdevice
driver, that has to call them.
Thanks
Guennadi
> It can use v4l2_get/set_subdev_hostdata() to associate this struct with a
> subdev, though.
>
> Regards,
>
> Hans
---
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
