| Message ID | 1417686899-30149-6-git-send-email-hverkuil@xs4all.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Hans, Thank you for the patch. On Thursday 04 December 2014 10:54:56 Hans Verkuil wrote: > From: Hans Verkuil <hans.verkuil@cisco.com> > > These drivers depend on VIDEO_V4L2_SUBDEV_API, which in turn > depends on MEDIA_CONTROLLER. So it is sufficient to just depend > on VIDEO_V4L2_SUBDEV_API. Shouldn't the VIDEO_V4L2_SUBDEV_API dependency be dropped from those (and other) subdev drivers ? They don't require the userspace API, just the kernel part. > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > --- > drivers/media/i2c/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index f40b4cf..29276fd 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -196,7 +196,7 @@ config VIDEO_ADV7183 > > config VIDEO_ADV7604 > tristate "Analog Devices ADV7604 decoder" > - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API > ---help--- > Support for the Analog Devices ADV7604 video decoder. > > @@ -208,7 +208,7 @@ config VIDEO_ADV7604 > > config VIDEO_ADV7842 > tristate "Analog Devices ADV7842 decoder" > - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API > ---help--- > Support for the Analog Devices ADV7842 video decoder. > > @@ -431,7 +431,7 @@ config VIDEO_ADV7393 > > config VIDEO_ADV7511 > tristate "Analog Devices ADV7511 encoder" > - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API > ---help--- > Support for the Analog Devices ADV7511 video encoder.
On 12/08/2014 12:38 AM, Laurent Pinchart wrote: > Hi Hans, > > Thank you for the patch. > > On Thursday 04 December 2014 10:54:56 Hans Verkuil wrote: >> From: Hans Verkuil <hans.verkuil@cisco.com> >> >> These drivers depend on VIDEO_V4L2_SUBDEV_API, which in turn >> depends on MEDIA_CONTROLLER. So it is sufficient to just depend >> on VIDEO_V4L2_SUBDEV_API. > > Shouldn't the VIDEO_V4L2_SUBDEV_API dependency be dropped from those (and > other) subdev drivers ? They don't require the userspace API, just the kernel > part. They set V4L2_SUBDEV_FL_HAS_DEVNODE and use v4l2_subdev_get_try_format, so they do need VIDEO_V4L2_SUBDEV_API. Or am I missing something? Regards, Hans > >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> >> --- >> drivers/media/i2c/Kconfig | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig >> index f40b4cf..29276fd 100644 >> --- a/drivers/media/i2c/Kconfig >> +++ b/drivers/media/i2c/Kconfig >> @@ -196,7 +196,7 @@ config VIDEO_ADV7183 >> >> config VIDEO_ADV7604 >> tristate "Analog Devices ADV7604 decoder" >> - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER >> + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API >> ---help--- >> Support for the Analog Devices ADV7604 video decoder. >> >> @@ -208,7 +208,7 @@ config VIDEO_ADV7604 >> >> config VIDEO_ADV7842 >> tristate "Analog Devices ADV7842 decoder" >> - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER >> + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API >> ---help--- >> Support for the Analog Devices ADV7842 video decoder. >> >> @@ -431,7 +431,7 @@ config VIDEO_ADV7393 >> >> config VIDEO_ADV7511 >> tristate "Analog Devices ADV7511 encoder" >> - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER >> + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API >> ---help--- >> Support for the Analog Devices ADV7511 video encoder. > -- 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
Hi Hans, On Friday 19 December 2014 12:44:46 Hans Verkuil wrote: > On 12/08/2014 12:38 AM, Laurent Pinchart wrote: > > On Thursday 04 December 2014 10:54:56 Hans Verkuil wrote: > >> From: Hans Verkuil <hans.verkuil@cisco.com> > >> > >> These drivers depend on VIDEO_V4L2_SUBDEV_API, which in turn > >> depends on MEDIA_CONTROLLER. So it is sufficient to just depend > >> on VIDEO_V4L2_SUBDEV_API. > > > > Shouldn't the VIDEO_V4L2_SUBDEV_API dependency be dropped from those (and > > other) subdev drivers ? They don't require the userspace API, just the > > kernel part. > > They set V4L2_SUBDEV_FL_HAS_DEVNODE and use v4l2_subdev_get_try_format, > so they do need VIDEO_V4L2_SUBDEV_API. Or am I missing something? VIDEO_V4L2_SUBDEV_API was initially designed to cover both the subdev userspace API and the subdev in-kernel pad-level API. Now that the latter has been found useful without the former, I think we should revisit the idea. Does it still make sense to have a single Kconfig option to cover both concepts ? Should it be kept a-is, split in two, or redefined to cover the userspace API only (with the v4l2_subdev_get_try_* functions being then always available) ? As the idea is to standardize on pad-level operations for in- kernel communication between bridges and subdevs the v4l2_subdev_get_try_* functions will get increasingly used in most (if not all) subdev drivers.
On 12/19/2014 01:18 PM, Laurent Pinchart wrote: > Hi Hans, > > On Friday 19 December 2014 12:44:46 Hans Verkuil wrote: >> On 12/08/2014 12:38 AM, Laurent Pinchart wrote: >>> On Thursday 04 December 2014 10:54:56 Hans Verkuil wrote: >>>> From: Hans Verkuil <hans.verkuil@cisco.com> >>>> >>>> These drivers depend on VIDEO_V4L2_SUBDEV_API, which in turn >>>> depends on MEDIA_CONTROLLER. So it is sufficient to just depend >>>> on VIDEO_V4L2_SUBDEV_API. >>> >>> Shouldn't the VIDEO_V4L2_SUBDEV_API dependency be dropped from those (and >>> other) subdev drivers ? They don't require the userspace API, just the >>> kernel part. >> >> They set V4L2_SUBDEV_FL_HAS_DEVNODE and use v4l2_subdev_get_try_format, >> so they do need VIDEO_V4L2_SUBDEV_API. Or am I missing something? > > VIDEO_V4L2_SUBDEV_API was initially designed to cover both the subdev > userspace API and the subdev in-kernel pad-level API. Now that the latter has > been found useful without the former, I think we should revisit the idea. > > Does it still make sense to have a single Kconfig option to cover both > concepts ? Should it be kept a-is, split in two, or redefined to cover the > userspace API only (with the v4l2_subdev_get_try_* functions being then always > available) ? As the idea is to standardize on pad-level operations for in- > kernel communication between bridges and subdevs the v4l2_subdev_get_try_* > functions will get increasingly used in most (if not all) subdev drivers. OK, but if you don't mind I would make such changes in a separate patch. This patch just removes an obviously superfluous dependency and brings these drivers in line with the others. Removing it altogether is a separate issue. Regards, Hans -- 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
Hi Hans, On Friday 19 December 2014 13:21:29 Hans Verkuil wrote: > On 12/19/2014 01:18 PM, Laurent Pinchart wrote: > > On Friday 19 December 2014 12:44:46 Hans Verkuil wrote: > >> On 12/08/2014 12:38 AM, Laurent Pinchart wrote: > >>> On Thursday 04 December 2014 10:54:56 Hans Verkuil wrote: > >>>> From: Hans Verkuil <hans.verkuil@cisco.com> > >>>> > >>>> These drivers depend on VIDEO_V4L2_SUBDEV_API, which in turn > >>>> depends on MEDIA_CONTROLLER. So it is sufficient to just depend > >>>> on VIDEO_V4L2_SUBDEV_API. > >>> > >>> Shouldn't the VIDEO_V4L2_SUBDEV_API dependency be dropped from those > >>> (and other) subdev drivers ? They don't require the userspace API, just > >>> the kernel part. > >> > >> They set V4L2_SUBDEV_FL_HAS_DEVNODE and use v4l2_subdev_get_try_format, > >> so they do need VIDEO_V4L2_SUBDEV_API. Or am I missing something? > > > > VIDEO_V4L2_SUBDEV_API was initially designed to cover both the subdev > > userspace API and the subdev in-kernel pad-level API. Now that the latter > > has been found useful without the former, I think we should revisit the > > idea. > > > > Does it still make sense to have a single Kconfig option to cover both > > concepts ? Should it be kept a-is, split in two, or redefined to cover the > > userspace API only (with the v4l2_subdev_get_try_* functions being then > > always available) ? As the idea is to standardize on pad-level operations > > for in- kernel communication between bridges and subdevs the > > v4l2_subdev_get_try_* functions will get increasingly used in most (if > > not all) subdev drivers. > > OK, but if you don't mind I would make such changes in a separate patch. Sure. I sometimes think one step ahead :-) We can certainly fix that in a separate patch or patch series. What's your opinion regarding repurposing or splitting CONFIG_VIDEO_V4L2_SUBDEV_API ? > This patch just removes an obviously superfluous dependency and brings these > drivers in line with the others. > > Removing it altogether is a separate issue.
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index f40b4cf..29276fd 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -196,7 +196,7 @@ config VIDEO_ADV7183 config VIDEO_ADV7604 tristate "Analog Devices ADV7604 decoder" - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API ---help--- Support for the Analog Devices ADV7604 video decoder. @@ -208,7 +208,7 @@ config VIDEO_ADV7604 config VIDEO_ADV7842 tristate "Analog Devices ADV7842 decoder" - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API ---help--- Support for the Analog Devices ADV7842 video decoder. @@ -431,7 +431,7 @@ config VIDEO_ADV7393 config VIDEO_ADV7511 tristate "Analog Devices ADV7511 encoder" - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API ---help--- Support for the Analog Devices ADV7511 video encoder.