* [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format()
2018-11-28 17:19 [PATCH 0/6] media: don't ifdef v4l2_subdev_get_try_format() any more Lubomir Rintel
@ 2018-11-28 17:19 ` Lubomir Rintel
2018-12-03 13:48 ` [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() ^[^[ jacopo mondi
2018-11-28 17:19 ` [PATCH 2/6] media: ov7740: get rid of extra ifdefs Lubomir Rintel
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Lubomir Rintel @ 2018-11-28 17:19 UTC (permalink / raw)
To: Lad, Prabhakar, Mauro Carvalho Chehab, Rui Miguel Silva,
Shunqian Zheng, Jonathan Corbet, Wenyou Yang
Cc: Jacopo Mondi, linux-media, linux-kernel, Lubomir Rintel
Provide a dummy implementation when configured without
CONFIG_VIDEO_V4L2_SUBDEV_API to avoid ifdef dance in the drivers that can
be built either with or without the option.
Suggested-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
include/media/v4l2-subdev.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 9102d6ca566e..906e28011bb4 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -967,6 +967,17 @@ static inline struct v4l2_rect
pad = 0;
return &cfg[pad].try_compose;
}
+
+#else /* !defined(CONFIG_VIDEO_V4L2_SUBDEV_API) */
+
+static inline struct v4l2_mbus_framefmt
+*v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ unsigned int pad)
+{
+ return ERR_PTR(-ENOTTY);
+}
+
#endif
extern const struct v4l2_file_operations v4l2_subdev_fops;
--
2.19.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() ^[^[
2018-11-28 17:19 ` [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() Lubomir Rintel
@ 2018-12-03 13:48 ` jacopo mondi
2018-12-04 15:01 ` Lubomir Rintel
0 siblings, 1 reply; 15+ messages in thread
From: jacopo mondi @ 2018-12-03 13:48 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Lad, Prabhakar, Mauro Carvalho Chehab, Rui Miguel Silva,
Shunqian Zheng, Jonathan Corbet, Wenyou Yang, linux-media,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]
Hi Lubomir,
thanks for the patches
On Wed, Nov 28, 2018 at 06:19:13PM +0100, Lubomir Rintel wrote:
> Provide a dummy implementation when configured without
> CONFIG_VIDEO_V4L2_SUBDEV_API to avoid ifdef dance in the drivers that can
> be built either with or without the option.
>
> Suggested-by: Jacopo Mondi <jacopo@jmondi.org>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
> include/media/v4l2-subdev.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 9102d6ca566e..906e28011bb4 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -967,6 +967,17 @@ static inline struct v4l2_rect
> pad = 0;
> return &cfg[pad].try_compose;
> }
> +
> +#else /* !defined(CONFIG_VIDEO_V4L2_SUBDEV_API) */
> +
> +static inline struct v4l2_mbus_framefmt
> +*v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + unsigned int pad)
> +{
> + return ERR_PTR(-ENOTTY);
> +}
> +
> #endif
While at there, what about doing the same for get_try_crop and
get_try_compose? At lease provide stubs, I let you figure out if
you're willing to fix callers too, it seems there are quite a few of
them though
$ git grep v4l2_subdev_get_try* drivers/media/ | grep -v '_format' | wc -l
44
>
> extern const struct v4l2_file_operations v4l2_subdev_fops;
> --
> 2.19.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() ^[^[
2018-12-03 13:48 ` [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() ^[^[ jacopo mondi
@ 2018-12-04 15:01 ` Lubomir Rintel
2018-12-06 8:30 ` [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() ?? jacopo mondi
2018-12-09 21:17 ` sakari.ailus
0 siblings, 2 replies; 15+ messages in thread
From: Lubomir Rintel @ 2018-12-04 15:01 UTC (permalink / raw)
To: jacopo mondi
Cc: Lad, Prabhakar, Mauro Carvalho Chehab, Rui Miguel Silva,
Shunqian Zheng, Jonathan Corbet, Wenyou Yang, linux-media,
linux-kernel
On Mon, 2018-12-03 at 14:48 +0100, jacopo mondi wrote:
> Hi Lubomir,
>
> thanks for the patches
>
> On Wed, Nov 28, 2018 at 06:19:13PM +0100, Lubomir Rintel wrote:
> > Provide a dummy implementation when configured without
> > CONFIG_VIDEO_V4L2_SUBDEV_API to avoid ifdef dance in the drivers
> > that can
> > be built either with or without the option.
> >
> > Suggested-by: Jacopo Mondi <jacopo@jmondi.org>
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> > include/media/v4l2-subdev.h | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-
> > subdev.h
> > index 9102d6ca566e..906e28011bb4 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -967,6 +967,17 @@ static inline struct v4l2_rect
> > pad = 0;
> > return &cfg[pad].try_compose;
> > }
> > +
> > +#else /* !defined(CONFIG_VIDEO_V4L2_SUBDEV_API) */
> > +
> > +static inline struct v4l2_mbus_framefmt
> > +*v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + unsigned int pad)
> > +{
> > + return ERR_PTR(-ENOTTY);
> > +}
> > +
> > #endif
>
> While at there, what about doing the same for get_try_crop and
> get_try_compose? At lease provide stubs, I let you figure out if
> you're willing to fix callers too, it seems there are quite a few of
> them though
>
> $ git grep v4l2_subdev_get_try* drivers/media/ | grep -v '_format' |
> wc -l
> 44
I'd be happy to do that. However, the drivers that use those seem to
depend on CONFIG_VIDEO_V4L2_SUBDEV_API anyway. Should those
dependencies be eventually done away with?
Please pardon my ignorance -- I don't actually understand why would
anyone disable CONFIG_VIDEO_V4L2_SUBDEV_API.
I'll be following up with a v2 after I get a response from you. It will
address locking issues found with smatch: one introduced by my patch
and one that was there before.
Cheers,
Lubo
> > extern const struct v4l2_file_operations v4l2_subdev_fops;
> > --
> > 2.19.1
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() ??
2018-12-04 15:01 ` Lubomir Rintel
@ 2018-12-06 8:30 ` jacopo mondi
2018-12-07 8:57 ` Hans Verkuil
2018-12-09 21:17 ` sakari.ailus
1 sibling, 1 reply; 15+ messages in thread
From: jacopo mondi @ 2018-12-06 8:30 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Lad, Prabhakar, Mauro Carvalho Chehab, Rui Miguel Silva,
Shunqian Zheng, Jonathan Corbet, Wenyou Yang, linux-media,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3833 bytes --]
Hi Lubomir,
On Tue, Dec 04, 2018 at 04:01:43PM +0100, Lubomir Rintel wrote:
> On Mon, 2018-12-03 at 14:48 +0100, jacopo mondi wrote:
> > Hi Lubomir,
> >
> > thanks for the patches
> >
> > On Wed, Nov 28, 2018 at 06:19:13PM +0100, Lubomir Rintel wrote:
> > > Provide a dummy implementation when configured without
> > > CONFIG_VIDEO_V4L2_SUBDEV_API to avoid ifdef dance in the drivers
> > > that can
> > > be built either with or without the option.
> > >
> > > Suggested-by: Jacopo Mondi <jacopo@jmondi.org>
> > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > > ---
> > > include/media/v4l2-subdev.h | 11 +++++++++++
> > > 1 file changed, 11 insertions(+)
> > >
> > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-
> > > subdev.h
> > > index 9102d6ca566e..906e28011bb4 100644
> > > --- a/include/media/v4l2-subdev.h
> > > +++ b/include/media/v4l2-subdev.h
> > > @@ -967,6 +967,17 @@ static inline struct v4l2_rect
> > > pad = 0;
> > > return &cfg[pad].try_compose;
> > > }
> > > +
> > > +#else /* !defined(CONFIG_VIDEO_V4L2_SUBDEV_API) */
> > > +
> > > +static inline struct v4l2_mbus_framefmt
> > > +*v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
> > > + struct v4l2_subdev_pad_config *cfg,
> > > + unsigned int pad)
> > > +{
> > > + return ERR_PTR(-ENOTTY);
> > > +}
> > > +
> > > #endif
> >
> > While at there, what about doing the same for get_try_crop and
> > get_try_compose? At lease provide stubs, I let you figure out if
> > you're willing to fix callers too, it seems there are quite a few of
> > them though
> >
> > $ git grep v4l2_subdev_get_try* drivers/media/ | grep -v '_format' |
> > wc -l
> > 44
>
> I'd be happy to do that. However, the drivers that use those seem to
> depend on CONFIG_VIDEO_V4L2_SUBDEV_API anyway. Should those
> dependencies be eventually done away with?
>
I don't think it is the case to drop the dependencies. If you go down
that path you would need to be very careful. It's enough to add stubs
for those functions like you've done for v4l2_subdev_get_try_format().
Now, looking around a bit in more detail, most sensor drivers return
-ENOTTY if you require V4L2_SUBDEV_FORMAT_TRY format when
CONFIG_VIDEO_V4L2_SUBDEV_API is not defined. I would say all drivers
but mt9v111.c, which is one of the most recent ones, and that deals
with the issue as:
static struct v4l2_mbus_framefmt *__mt9v111_get_pad_format(
struct mt9v111_dev *mt9v111,
struct v4l2_subdev_pad_config *cfg,
unsigned int pad,
enum v4l2_subdev_format_whence which)
{
switch (which) {
case V4L2_SUBDEV_FORMAT_TRY:
#if IS_ENABLED(CONFIG_VIDEO_V4L2_SUBDEV_API)
return v4l2_subdev_get_try_format(&mt9v111->sd, cfg, pad);
#else
return &cfg->try_fmt;
#endif
case V4L2_SUBDEV_FORMAT_ACTIVE:
return &mt9v111->fmt;
default:
return NULL;
}
}
Since I wrote that part, and I recall it had been suggested to me, I
wonder which one of the two approaches it actually correct :/
> Please pardon my ignorance -- I don't actually understand why would
> anyone disable CONFIG_VIDEO_V4L2_SUBDEV_API.
The config options is described as:
Enables the V4L2 sub-device pad-level userspace API used to configure
video format, size and frame rate between hardware blocks.
Some driver simply do not expose a subdev in userspace. It might be
discussed that if selecting MEDIA_CONTROLLER should in facts be enough
and to imply CONFIG_VIDEO_V4L2_SUBDEV_API, but that's a separate
issue.
>
> I'll be following up with a v2 after I get a response from you. It will
> address locking issues found with smatch: one introduced by my patch
> and one that was there before.
>
Yep, ov2659 was b0rken already, thanks for fixing it while at there.
Thanks
j
> Cheers,
> Lubo
>
> > > extern const struct v4l2_file_operations v4l2_subdev_fops;
> > > --
> > > 2.19.1
> > >
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() ??
2018-12-06 8:30 ` [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() ?? jacopo mondi
@ 2018-12-07 8:57 ` Hans Verkuil
0 siblings, 0 replies; 15+ messages in thread
From: Hans Verkuil @ 2018-12-07 8:57 UTC (permalink / raw)
To: jacopo mondi, Lubomir Rintel
Cc: Lad, Prabhakar, Mauro Carvalho Chehab, Rui Miguel Silva,
Shunqian Zheng, Jonathan Corbet, Wenyou Yang, linux-media,
linux-kernel
On 12/06/2018 09:30 AM, jacopo mondi wrote:
> Hi Lubomir,
>
> On Tue, Dec 04, 2018 at 04:01:43PM +0100, Lubomir Rintel wrote:
>> On Mon, 2018-12-03 at 14:48 +0100, jacopo mondi wrote:
>>> Hi Lubomir,
>>>
>>> thanks for the patches
>>>
>>> On Wed, Nov 28, 2018 at 06:19:13PM +0100, Lubomir Rintel wrote:
>>>> Provide a dummy implementation when configured without
>>>> CONFIG_VIDEO_V4L2_SUBDEV_API to avoid ifdef dance in the drivers
>>>> that can
>>>> be built either with or without the option.
>>>>
>>>> Suggested-by: Jacopo Mondi <jacopo@jmondi.org>
>>>> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
>>>> ---
>>>> include/media/v4l2-subdev.h | 11 +++++++++++
>>>> 1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-
>>>> subdev.h
>>>> index 9102d6ca566e..906e28011bb4 100644
>>>> --- a/include/media/v4l2-subdev.h
>>>> +++ b/include/media/v4l2-subdev.h
>>>> @@ -967,6 +967,17 @@ static inline struct v4l2_rect
>>>> pad = 0;
>>>> return &cfg[pad].try_compose;
>>>> }
>>>> +
>>>> +#else /* !defined(CONFIG_VIDEO_V4L2_SUBDEV_API) */
>>>> +
>>>> +static inline struct v4l2_mbus_framefmt
>>>> +*v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
>>>> + struct v4l2_subdev_pad_config *cfg,
>>>> + unsigned int pad)
>>>> +{
>>>> + return ERR_PTR(-ENOTTY);
>>>> +}
>>>> +
>>>> #endif
>>>
>>> While at there, what about doing the same for get_try_crop and
>>> get_try_compose? At lease provide stubs, I let you figure out if
>>> you're willing to fix callers too, it seems there are quite a few of
>>> them though
>>>
>>> $ git grep v4l2_subdev_get_try* drivers/media/ | grep -v '_format' |
>>> wc -l
>>> 44
>>
>> I'd be happy to do that. However, the drivers that use those seem to
>> depend on CONFIG_VIDEO_V4L2_SUBDEV_API anyway. Should those
>> dependencies be eventually done away with?
>>
>
> I don't think it is the case to drop the dependencies. If you go down
> that path you would need to be very careful. It's enough to add stubs
> for those functions like you've done for v4l2_subdev_get_try_format().
>
> Now, looking around a bit in more detail, most sensor drivers return
> -ENOTTY if you require V4L2_SUBDEV_FORMAT_TRY format when
> CONFIG_VIDEO_V4L2_SUBDEV_API is not defined. I would say all drivers
> but mt9v111.c, which is one of the most recent ones, and that deals
> with the issue as:
>
> static struct v4l2_mbus_framefmt *__mt9v111_get_pad_format(
> struct mt9v111_dev *mt9v111,
> struct v4l2_subdev_pad_config *cfg,
> unsigned int pad,
> enum v4l2_subdev_format_whence which)
> {
> switch (which) {
> case V4L2_SUBDEV_FORMAT_TRY:
> #if IS_ENABLED(CONFIG_VIDEO_V4L2_SUBDEV_API)
> return v4l2_subdev_get_try_format(&mt9v111->sd, cfg, pad);
> #else
> return &cfg->try_fmt;
> #endif
> case V4L2_SUBDEV_FORMAT_ACTIVE:
> return &mt9v111->fmt;
> default:
> return NULL;
> }
> }
>
> Since I wrote that part, and I recall it had been suggested to me, I
> wonder which one of the two approaches it actually correct :/
>
>
>> Please pardon my ignorance -- I don't actually understand why would
>> anyone disable CONFIG_VIDEO_V4L2_SUBDEV_API.
>
> The config options is described as:
> Enables the V4L2 sub-device pad-level userspace API used to configure
> video format, size and frame rate between hardware blocks.
>
> Some driver simply do not expose a subdev in userspace. It might be
> discussed that if selecting MEDIA_CONTROLLER should in facts be enough
> and to imply CONFIG_VIDEO_V4L2_SUBDEV_API, but that's a separate
> issue.
I've thought for quite some time now that CONFIG_VIDEO_V4L2_SUBDEV_API
should be removed. I really do not see any advantage of having that config
option. You save a tiny bit of memory, at the cost of creating confusion.
Complexity is the real enemy of media drivers, and trying to keep that down
is always a good thing.
That said, removing CONFIG_VIDEO_V4L2_SUBDEV_API won't help much since in
the code above it would just replace #if IS_ENABLED(CONFIG_VIDEO_V4L2_SUBDEV_API)
by #if IS_ENABLED(CONFIG_MEDIA_CONTROLLER).
In fact, I would favor to just always enable both MEDIA_CONTROLLER and
V4L2_SUBDEV_API for V4L2 devices. They don't have to use the media controller
if they don't want to, but it's there if needed.
It simplifies a lot of code at the expense of a bit more memory usage. But
given the huge amount of memory that the video buffers take, I really don't
think that is an issue.
Also, as more and more drivers require the media controller anyway, it is likely
to be enabled in standard distros anyway. It is for debian, in any case.
I'll make an RFC patch and post it. See what people think.
Regards,
Hans
>>
>> I'll be following up with a v2 after I get a response from you. It will
>> address locking issues found with smatch: one introduced by my patch
>> and one that was there before.
>>
>
> Yep, ov2659 was b0rken already, thanks for fixing it while at there.
>
> Thanks
> j
>
>> Cheers,
>> Lubo
>>
>>>> extern const struct v4l2_file_operations v4l2_subdev_fops;
>>>> --
>>>> 2.19.1
>>>>
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() ??
2018-12-04 15:01 ` Lubomir Rintel
2018-12-06 8:30 ` [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() ?? jacopo mondi
@ 2018-12-09 21:17 ` sakari.ailus
1 sibling, 0 replies; 15+ messages in thread
From: sakari.ailus @ 2018-12-09 21:17 UTC (permalink / raw)
To: Lubomir Rintel
Cc: jacopo mondi, Lad, Prabhakar, Mauro Carvalho Chehab,
Rui Miguel Silva, Shunqian Zheng, Jonathan Corbet, Wenyou Yang,
linux-media, linux-kernel
Hi Lubomir, Jacopo,
On Tue, Dec 04, 2018 at 04:01:43PM +0100, Lubomir Rintel wrote:
> On Mon, 2018-12-03 at 14:48 +0100, jacopo mondi wrote:
> > Hi Lubomir,
> >
> > thanks for the patches
> >
> > On Wed, Nov 28, 2018 at 06:19:13PM +0100, Lubomir Rintel wrote:
> > > Provide a dummy implementation when configured without
> > > CONFIG_VIDEO_V4L2_SUBDEV_API to avoid ifdef dance in the drivers
> > > that can
> > > be built either with or without the option.
> > >
> > > Suggested-by: Jacopo Mondi <jacopo@jmondi.org>
> > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > > ---
> > > include/media/v4l2-subdev.h | 11 +++++++++++
> > > 1 file changed, 11 insertions(+)
> > >
> > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-
> > > subdev.h
> > > index 9102d6ca566e..906e28011bb4 100644
> > > --- a/include/media/v4l2-subdev.h
> > > +++ b/include/media/v4l2-subdev.h
> > > @@ -967,6 +967,17 @@ static inline struct v4l2_rect
> > > pad = 0;
> > > return &cfg[pad].try_compose;
> > > }
> > > +
> > > +#else /* !defined(CONFIG_VIDEO_V4L2_SUBDEV_API) */
> > > +
> > > +static inline struct v4l2_mbus_framefmt
> > > +*v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
> > > + struct v4l2_subdev_pad_config *cfg,
> > > + unsigned int pad)
> > > +{
> > > + return ERR_PTR(-ENOTTY);
> > > +}
> > > +
> > > #endif
> >
> > While at there, what about doing the same for get_try_crop and
> > get_try_compose? At lease provide stubs, I let you figure out if
> > you're willing to fix callers too, it seems there are quite a few of
> > them though
> >
> > $ git grep v4l2_subdev_get_try* drivers/media/ | grep -v '_format' |
> > wc -l
> > 44
>
> I'd be happy to do that. However, the drivers that use those seem to
> depend on CONFIG_VIDEO_V4L2_SUBDEV_API anyway. Should those
> dependencies be eventually done away with?
>
> Please pardon my ignorance -- I don't actually understand why would
> anyone disable CONFIG_VIDEO_V4L2_SUBDEV_API.
>
> I'll be following up with a v2 after I get a response from you. It will
> address locking issues found with smatch: one introduced by my patch
> and one that was there before.
Hans wrote a patch that enables VIDEO_V4L2_SUBDEV_API if
MEDIA_CAMERA_SUPPORT has been selected. So much of these #ifdefs will not
be needed going forward. It's an RFC for now, but that'd require a lot of
conditional compiling in drivers.
<URL:https://patchwork.linuxtv.org/patch/53370/>
The current use of the get_fmt callback serves the regular V4L2 API use, in
which case getting the try format is not meaningful.
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/6] media: ov7740: get rid of extra ifdefs
2018-11-28 17:19 [PATCH 0/6] media: don't ifdef v4l2_subdev_get_try_format() any more Lubomir Rintel
2018-11-28 17:19 ` [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() Lubomir Rintel
@ 2018-11-28 17:19 ` Lubomir Rintel
2019-01-15 9:25 ` Sakari Ailus
2018-11-28 17:19 ` [PATCH 3/6] media: ov2659: " Lubomir Rintel
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Lubomir Rintel @ 2018-11-28 17:19 UTC (permalink / raw)
To: Lad, Prabhakar, Mauro Carvalho Chehab, Rui Miguel Silva,
Shunqian Zheng, Jonathan Corbet, Wenyou Yang
Cc: Jacopo Mondi, linux-media, linux-kernel, Lubomir Rintel
Stubbed v4l2_subdev_get_try_format() will return a correct error when
configured without CONFIG_VIDEO_V4L2_SUBDEV_API.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
drivers/media/i2c/ov7740.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
index 6e9c233cfbe3..781ddcc743d4 100644
--- a/drivers/media/i2c/ov7740.c
+++ b/drivers/media/i2c/ov7740.c
@@ -780,9 +780,7 @@ static int ov7740_set_fmt(struct v4l2_subdev *sd,
struct ov7740 *ov7740 = container_of(sd, struct ov7740, subdev);
const struct ov7740_pixfmt *ovfmt;
const struct ov7740_framesize *fsize;
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
struct v4l2_mbus_framefmt *mbus_fmt;
-#endif
int ret;
mutex_lock(&ov7740->mutex);
@@ -795,16 +793,15 @@ static int ov7740_set_fmt(struct v4l2_subdev *sd,
ret = ov7740_try_fmt_internal(sd, &format->format, NULL, NULL);
if (ret)
goto error;
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+ if (IS_ERR(mbus_fmt)) {
+ ret = PTR_ERR(mbus_fmt);
+ goto error;
+ }
*mbus_fmt = format->format;
mutex_unlock(&ov7740->mutex);
return 0;
-#else
- ret = -ENOTTY;
- goto error;
-#endif
}
ret = ov7740_try_fmt_internal(sd, &format->format, &ovfmt, &fsize);
@@ -827,20 +824,18 @@ static int ov7740_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_format *format)
{
struct ov7740 *ov7740 = container_of(sd, struct ov7740, subdev);
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
struct v4l2_mbus_framefmt *mbus_fmt;
-#endif
int ret = 0;
mutex_lock(&ov7740->mutex);
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
- format->format = *mbus_fmt;
- ret = 0;
-#else
- ret = -ENOTTY;
-#endif
+ if (IS_ERR(mbus_fmt)) {
+ ret = PTR_ERR(mbus_fmt);
+ } else {
+ format->format = *mbus_fmt;
+ ret = 0;
+ }
} else {
format->format = ov7740->format;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] media: ov7740: get rid of extra ifdefs
2018-11-28 17:19 ` [PATCH 2/6] media: ov7740: get rid of extra ifdefs Lubomir Rintel
@ 2019-01-15 9:25 ` Sakari Ailus
0 siblings, 0 replies; 15+ messages in thread
From: Sakari Ailus @ 2019-01-15 9:25 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Lad, Prabhakar, Mauro Carvalho Chehab, Rui Miguel Silva,
Shunqian Zheng, Jonathan Corbet, Wenyou Yang, Jacopo Mondi,
linux-media, linux-kernel
On Wed, Nov 28, 2018 at 06:19:14PM +0100, Lubomir Rintel wrote:
> Stubbed v4l2_subdev_get_try_format() will return a correct error when
> configured without CONFIG_VIDEO_V4L2_SUBDEV_API.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
I'm marking these as rejected for now; let's see if we'll get rid of the
said Kconfig options.
Thanks.
--
Sakari Ailus
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/6] media: ov2659: get rid of extra ifdefs
2018-11-28 17:19 [PATCH 0/6] media: don't ifdef v4l2_subdev_get_try_format() any more Lubomir Rintel
2018-11-28 17:19 ` [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() Lubomir Rintel
2018-11-28 17:19 ` [PATCH 2/6] media: ov7740: get rid of extra ifdefs Lubomir Rintel
@ 2018-11-28 17:19 ` Lubomir Rintel
2018-12-03 10:42 ` Dan Carpenter
2018-11-28 17:19 ` [PATCH 4/6] media: ov2680: " Lubomir Rintel
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Lubomir Rintel @ 2018-11-28 17:19 UTC (permalink / raw)
To: Lad, Prabhakar, Mauro Carvalho Chehab, Rui Miguel Silva,
Shunqian Zheng, Jonathan Corbet, Wenyou Yang
Cc: Jacopo Mondi, linux-media, linux-kernel, Lubomir Rintel
Stubbed v4l2_subdev_get_try_format() will return a correct error when
configured without CONFIG_VIDEO_V4L2_SUBDEV_API.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
drivers/media/i2c/ov2659.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
index 799acce803fe..a66c12c8f278 100644
--- a/drivers/media/i2c/ov2659.c
+++ b/drivers/media/i2c/ov2659.c
@@ -1046,17 +1046,15 @@ static int ov2659_get_fmt(struct v4l2_subdev *sd,
dev_dbg(&client->dev, "ov2659_get_fmt\n");
if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
struct v4l2_mbus_framefmt *mf;
mf = v4l2_subdev_get_try_format(sd, cfg, 0);
+ if (IS_ERR(mf))
+ return PTR_ERR(mf);
mutex_lock(&ov2659->lock);
fmt->format = *mf;
mutex_unlock(&ov2659->lock);
return 0;
-#else
- return -ENOTTY;
-#endif
}
mutex_lock(&ov2659->lock);
@@ -1126,12 +1124,10 @@ static int ov2659_set_fmt(struct v4l2_subdev *sd,
mutex_lock(&ov2659->lock);
if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
mf = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
+ if (IS_ERR(mf))
+ return PTR_ERR(mf);
*mf = fmt->format;
-#else
- return -ENOTTY;
-#endif
} else {
s64 val;
--
2.19.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] media: ov2659: get rid of extra ifdefs
2018-11-28 17:19 ` [PATCH 3/6] media: ov2659: " Lubomir Rintel
@ 2018-12-03 10:42 ` Dan Carpenter
0 siblings, 0 replies; 15+ messages in thread
From: Dan Carpenter @ 2018-12-03 10:42 UTC (permalink / raw)
To: kbuild, Lubomir Rintel
Cc: kbuild-all, Lad, Prabhakar, Mauro Carvalho Chehab,
Rui Miguel Silva, Shunqian Zheng, Jonathan Corbet, Wenyou Yang,
Jacopo Mondi, linux-media, linux-kernel, Lubomir Rintel
Hi Lubomir,
url: https://github.com/0day-ci/linux/commits/Lubomir-Rintel/media-don-t-ifdef-v4l2_subdev_get_try_format-any-more/20181129-205631
base: git://linuxtv.org/media_tree.git master
smatch warnings:
drivers/media/i2c/ov2659.c:1157 ov2659_set_fmt() warn: inconsistent returns 'mutex:&ov2659->lock'.
Locked on: line 1129
Unlocked on: line 1119
# https://github.com/0day-ci/linux/commit/ceed6707bbb8d34fa04448a9eaf77a574dae59a8
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout ceed6707bbb8d34fa04448a9eaf77a574dae59a8
vim +1157 drivers/media/i2c/ov2659.c
c4c0283a Benoit Parrot 2015-03-20 1098
c4c0283a Benoit Parrot 2015-03-20 1099 static int ov2659_set_fmt(struct v4l2_subdev *sd,
c4c0283a Benoit Parrot 2015-03-20 1100 struct v4l2_subdev_pad_config *cfg,
c4c0283a Benoit Parrot 2015-03-20 1101 struct v4l2_subdev_format *fmt)
c4c0283a Benoit Parrot 2015-03-20 1102 {
c4c0283a Benoit Parrot 2015-03-20 1103 struct i2c_client *client = v4l2_get_subdevdata(sd);
5f5859d1 Dan Carpenter 2015-04-15 1104 int index = ARRAY_SIZE(ov2659_formats);
c4c0283a Benoit Parrot 2015-03-20 1105 struct v4l2_mbus_framefmt *mf = &fmt->format;
c4c0283a Benoit Parrot 2015-03-20 1106 const struct ov2659_framesize *size = NULL;
c4c0283a Benoit Parrot 2015-03-20 1107 struct ov2659 *ov2659 = to_ov2659(sd);
c4c0283a Benoit Parrot 2015-03-20 1108 int ret = 0;
c4c0283a Benoit Parrot 2015-03-20 1109
c4c0283a Benoit Parrot 2015-03-20 1110 dev_dbg(&client->dev, "ov2659_set_fmt\n");
c4c0283a Benoit Parrot 2015-03-20 1111
c4c0283a Benoit Parrot 2015-03-20 1112 __ov2659_try_frame_size(mf, &size);
c4c0283a Benoit Parrot 2015-03-20 1113
c4c0283a Benoit Parrot 2015-03-20 1114 while (--index >= 0)
c4c0283a Benoit Parrot 2015-03-20 1115 if (ov2659_formats[index].code == mf->code)
c4c0283a Benoit Parrot 2015-03-20 1116 break;
c4c0283a Benoit Parrot 2015-03-20 1117
c4c0283a Benoit Parrot 2015-03-20 1118 if (index < 0)
c4c0283a Benoit Parrot 2015-03-20 1119 return -EINVAL;
c4c0283a Benoit Parrot 2015-03-20 1120
c4c0283a Benoit Parrot 2015-03-20 1121 mf->colorspace = V4L2_COLORSPACE_SRGB;
c4c0283a Benoit Parrot 2015-03-20 1122 mf->field = V4L2_FIELD_NONE;
c4c0283a Benoit Parrot 2015-03-20 1123
c4c0283a Benoit Parrot 2015-03-20 1124 mutex_lock(&ov2659->lock);
c4c0283a Benoit Parrot 2015-03-20 1125
c4c0283a Benoit Parrot 2015-03-20 1126 if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
c4c0283a Benoit Parrot 2015-03-20 1127 mf = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
ceed6707 Lubomir Rintel 2018-11-28 1128 if (IS_ERR(mf))
ceed6707 Lubomir Rintel 2018-11-28 1129 return PTR_ERR(mf);
^^^^^^^^^^^^^^^^^^
goto unlock;
c4c0283a Benoit Parrot 2015-03-20 1130 *mf = fmt->format;
c4c0283a Benoit Parrot 2015-03-20 1131 } else {
c4c0283a Benoit Parrot 2015-03-20 1132 s64 val;
c4c0283a Benoit Parrot 2015-03-20 1133
c4c0283a Benoit Parrot 2015-03-20 1134 if (ov2659->streaming) {
c4c0283a Benoit Parrot 2015-03-20 1135 mutex_unlock(&ov2659->lock);
c4c0283a Benoit Parrot 2015-03-20 1136 return -EBUSY;
c4c0283a Benoit Parrot 2015-03-20 1137 }
c4c0283a Benoit Parrot 2015-03-20 1138
c4c0283a Benoit Parrot 2015-03-20 1139 ov2659->frame_size = size;
c4c0283a Benoit Parrot 2015-03-20 1140 ov2659->format = fmt->format;
c4c0283a Benoit Parrot 2015-03-20 1141 ov2659->format_ctrl_regs =
c4c0283a Benoit Parrot 2015-03-20 1142 ov2659_formats[index].format_ctrl_regs;
c4c0283a Benoit Parrot 2015-03-20 1143
c4c0283a Benoit Parrot 2015-03-20 1144 if (ov2659->format.code != MEDIA_BUS_FMT_SBGGR8_1X8)
c4c0283a Benoit Parrot 2015-03-20 1145 val = ov2659->pdata->link_frequency / 2;
c4c0283a Benoit Parrot 2015-03-20 1146 else
c4c0283a Benoit Parrot 2015-03-20 1147 val = ov2659->pdata->link_frequency;
c4c0283a Benoit Parrot 2015-03-20 1148
c4c0283a Benoit Parrot 2015-03-20 1149 ret = v4l2_ctrl_s_ctrl_int64(ov2659->link_frequency, val);
c4c0283a Benoit Parrot 2015-03-20 1150 if (ret < 0)
c4c0283a Benoit Parrot 2015-03-20 1151 dev_warn(&client->dev,
c4c0283a Benoit Parrot 2015-03-20 1152 "failed to set link_frequency rate (%d)\n",
c4c0283a Benoit Parrot 2015-03-20 1153 ret);
c4c0283a Benoit Parrot 2015-03-20 1154 }
c4c0283a Benoit Parrot 2015-03-20 1155
c4c0283a Benoit Parrot 2015-03-20 1156 mutex_unlock(&ov2659->lock);
c4c0283a Benoit Parrot 2015-03-20 @1157 return ret;
c4c0283a Benoit Parrot 2015-03-20 1158 }
c4c0283a Benoit Parrot 2015-03-20 1159
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/6] media: ov2680: get rid of extra ifdefs
2018-11-28 17:19 [PATCH 0/6] media: don't ifdef v4l2_subdev_get_try_format() any more Lubomir Rintel
` (2 preceding siblings ...)
2018-11-28 17:19 ` [PATCH 3/6] media: ov2659: " Lubomir Rintel
@ 2018-11-28 17:19 ` Lubomir Rintel
2018-12-03 10:43 ` Dan Carpenter
2018-11-28 17:19 ` [PATCH 5/6] media: ov5695: " Lubomir Rintel
2018-11-28 17:19 ` [PATCH 6/6] media: ov7670: " Lubomir Rintel
5 siblings, 1 reply; 15+ messages in thread
From: Lubomir Rintel @ 2018-11-28 17:19 UTC (permalink / raw)
To: Lad, Prabhakar, Mauro Carvalho Chehab, Rui Miguel Silva,
Shunqian Zheng, Jonathan Corbet, Wenyou Yang
Cc: Jacopo Mondi, linux-media, linux-kernel, Lubomir Rintel
Stubbed v4l2_subdev_get_try_format() will return a correct error when
configured without CONFIG_VIDEO_V4L2_SUBDEV_API.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
drivers/media/i2c/ov2680.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
index 0e34e15b67b3..f8b873aaacec 100644
--- a/drivers/media/i2c/ov2680.c
+++ b/drivers/media/i2c/ov2680.c
@@ -680,11 +680,9 @@ static int ov2680_get_fmt(struct v4l2_subdev *sd,
mutex_lock(&sensor->lock);
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
fmt = v4l2_subdev_get_try_format(&sensor->sd, cfg, format->pad);
-#else
- ret = -ENOTTY;
-#endif
+ if (IS_ERR(fmt))
+ return PTR_ERR(fmt);
} else {
fmt = &sensor->fmt;
}
@@ -703,9 +701,7 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd,
{
struct ov2680_dev *sensor = to_ov2680_dev(sd);
struct v4l2_mbus_framefmt *fmt = &format->format;
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
struct v4l2_mbus_framefmt *try_fmt;
-#endif
const struct ov2680_mode_info *mode;
int ret = 0;
@@ -728,12 +724,11 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd,
}
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
try_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
- format->format = *try_fmt;
-#else
- ret = -ENOTTY;
-#endif
+ if (IS_ERR(try_fmt))
+ ret = PTR_ERR(try_fmt);
+ else
+ format->format = *try_fmt;
goto unlock;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/6] media: ov2680: get rid of extra ifdefs
2018-11-28 17:19 ` [PATCH 4/6] media: ov2680: " Lubomir Rintel
@ 2018-12-03 10:43 ` Dan Carpenter
0 siblings, 0 replies; 15+ messages in thread
From: Dan Carpenter @ 2018-12-03 10:43 UTC (permalink / raw)
To: kbuild, Lubomir Rintel
Cc: kbuild-all, Lad, Prabhakar, Mauro Carvalho Chehab,
Rui Miguel Silva, Shunqian Zheng, Jonathan Corbet, Wenyou Yang,
Jacopo Mondi, linux-media, linux-kernel, Lubomir Rintel
Hi Lubomir,
url: https://github.com/0day-ci/linux/commits/Lubomir-Rintel/media-don-t-ifdef-v4l2_subdev_get_try_format-any-more/20181129-205631
base: git://linuxtv.org/media_tree.git master
smatch warnings:
drivers/media/i2c/ov2680.c:687 ov2680_get_fmt() warn: inconsistent returns 'mutex:&sensor->lock'.
Locked on: line 677
Unlocked on: line 670
# https://github.com/0day-ci/linux/commit/45699a2f04294ea9ca96a3d178232ecae7f607ed
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 45699a2f04294ea9ca96a3d178232ecae7f607ed
vim +687 drivers/media/i2c/ov2680.c
3ee47cad Rui Miguel Silva 2018-07-03 660
3ee47cad Rui Miguel Silva 2018-07-03 661 static int ov2680_get_fmt(struct v4l2_subdev *sd,
3ee47cad Rui Miguel Silva 2018-07-03 662 struct v4l2_subdev_pad_config *cfg,
3ee47cad Rui Miguel Silva 2018-07-03 663 struct v4l2_subdev_format *format)
3ee47cad Rui Miguel Silva 2018-07-03 664 {
3ee47cad Rui Miguel Silva 2018-07-03 665 struct ov2680_dev *sensor = to_ov2680_dev(sd);
3ee47cad Rui Miguel Silva 2018-07-03 666 struct v4l2_mbus_framefmt *fmt = NULL;
3ee47cad Rui Miguel Silva 2018-07-03 667 int ret = 0;
3ee47cad Rui Miguel Silva 2018-07-03 668
3ee47cad Rui Miguel Silva 2018-07-03 669 if (format->pad != 0)
3ee47cad Rui Miguel Silva 2018-07-03 670 return -EINVAL;
3ee47cad Rui Miguel Silva 2018-07-03 671
3ee47cad Rui Miguel Silva 2018-07-03 672 mutex_lock(&sensor->lock);
3ee47cad Rui Miguel Silva 2018-07-03 673
3ee47cad Rui Miguel Silva 2018-07-03 674 if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
3ee47cad Rui Miguel Silva 2018-07-03 675 fmt = v4l2_subdev_get_try_format(&sensor->sd, cfg, format->pad);
45699a2f Lubomir Rintel 2018-11-28 676 if (IS_ERR(fmt))
45699a2f Lubomir Rintel 2018-11-28 677 return PTR_ERR(fmt);
^^^^^^^^^^^^^^^^^^^
goto unlock;
3ee47cad Rui Miguel Silva 2018-07-03 678 } else {
3ee47cad Rui Miguel Silva 2018-07-03 679 fmt = &sensor->fmt;
3ee47cad Rui Miguel Silva 2018-07-03 680 }
3ee47cad Rui Miguel Silva 2018-07-03 681
3ee47cad Rui Miguel Silva 2018-07-03 682 if (fmt)
3ee47cad Rui Miguel Silva 2018-07-03 683 format->format = *fmt;
3ee47cad Rui Miguel Silva 2018-07-03 684
3ee47cad Rui Miguel Silva 2018-07-03 685 mutex_unlock(&sensor->lock);
3ee47cad Rui Miguel Silva 2018-07-03 686
3ee47cad Rui Miguel Silva 2018-07-03 @687 return ret;
3ee47cad Rui Miguel Silva 2018-07-03 688 }
3ee47cad Rui Miguel Silva 2018-07-03 689
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/6] media: ov5695: get rid of extra ifdefs
2018-11-28 17:19 [PATCH 0/6] media: don't ifdef v4l2_subdev_get_try_format() any more Lubomir Rintel
` (3 preceding siblings ...)
2018-11-28 17:19 ` [PATCH 4/6] media: ov2680: " Lubomir Rintel
@ 2018-11-28 17:19 ` Lubomir Rintel
2018-11-28 17:19 ` [PATCH 6/6] media: ov7670: " Lubomir Rintel
5 siblings, 0 replies; 15+ messages in thread
From: Lubomir Rintel @ 2018-11-28 17:19 UTC (permalink / raw)
To: Lad, Prabhakar, Mauro Carvalho Chehab, Rui Miguel Silva,
Shunqian Zheng, Jonathan Corbet, Wenyou Yang
Cc: Jacopo Mondi, linux-media, linux-kernel, Lubomir Rintel
Stubbed v4l2_subdev_get_try_format() will return a correct error when
configured without CONFIG_VIDEO_V4L2_SUBDEV_API.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
drivers/media/i2c/ov5695.c | 38 ++++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c
index 5d107c53364d..1469e8b90e1a 100644
--- a/drivers/media/i2c/ov5695.c
+++ b/drivers/media/i2c/ov5695.c
@@ -810,6 +810,7 @@ static int ov5695_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_format *fmt)
{
struct ov5695 *ov5695 = to_ov5695(sd);
+ struct v4l2_mbus_framefmt *try_fmt;
const struct ov5695_mode *mode;
s64 h_blank, vblank_def;
@@ -821,12 +822,12 @@ static int ov5695_set_fmt(struct v4l2_subdev *sd,
fmt->format.height = mode->height;
fmt->format.field = V4L2_FIELD_NONE;
if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
- *v4l2_subdev_get_try_format(sd, cfg, fmt->pad) = fmt->format;
-#else
- mutex_unlock(&ov5695->mutex);
- return -ENOTTY;
-#endif
+ try_fmt = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
+ if (IS_ERR(try_fmt)) {
+ mutex_unlock(&ov5695->mutex);
+ return PTR_ERR(try_fmt);
+ }
+ *try_fmt = fmt->format;
} else {
ov5695->cur_mode = mode;
h_blank = mode->hts_def - mode->width;
@@ -845,24 +846,25 @@ static int ov5695_set_fmt(struct v4l2_subdev *sd,
static int ov5695_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
- struct v4l2_subdev_format *fmt)
+ struct v4l2_subdev_format *format)
{
struct ov5695 *ov5695 = to_ov5695(sd);
+ struct v4l2_mbus_framefmt *fmt;
const struct ov5695_mode *mode = ov5695->cur_mode;
mutex_lock(&ov5695->mutex);
- if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
- fmt->format = *v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
-#else
- mutex_unlock(&ov5695->mutex);
- return -ENOTTY;
-#endif
+ if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+ fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+ if (IS_ERR(fmt)) {
+ mutex_unlock(&ov5695->mutex);
+ return PTR_ERR(fmt);
+ }
+ format->format = *fmt;
} else {
- fmt->format.width = mode->width;
- fmt->format.height = mode->height;
- fmt->format.code = MEDIA_BUS_FMT_SBGGR10_1X10;
- fmt->format.field = V4L2_FIELD_NONE;
+ format->format.width = mode->width;
+ format->format.height = mode->height;
+ format->format.code = MEDIA_BUS_FMT_SBGGR10_1X10;
+ format->format.field = V4L2_FIELD_NONE;
}
mutex_unlock(&ov5695->mutex);
--
2.19.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 6/6] media: ov7670: get rid of extra ifdefs
2018-11-28 17:19 [PATCH 0/6] media: don't ifdef v4l2_subdev_get_try_format() any more Lubomir Rintel
` (4 preceding siblings ...)
2018-11-28 17:19 ` [PATCH 5/6] media: ov5695: " Lubomir Rintel
@ 2018-11-28 17:19 ` Lubomir Rintel
5 siblings, 0 replies; 15+ messages in thread
From: Lubomir Rintel @ 2018-11-28 17:19 UTC (permalink / raw)
To: Lad, Prabhakar, Mauro Carvalho Chehab, Rui Miguel Silva,
Shunqian Zheng, Jonathan Corbet, Wenyou Yang
Cc: Jacopo Mondi, linux-media, linux-kernel, Lubomir Rintel
Stubbed v4l2_subdev_get_try_format() will return a correct error when
configured without CONFIG_VIDEO_V4L2_SUBDEV_API.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
drivers/media/i2c/ov7670.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index bc68a3a5b4ec..7d1fdf51bd91 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1013,9 +1013,7 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
struct ov7670_format_struct *ovfmt;
struct ov7670_win_size *wsize;
struct ov7670_info *info = to_state(sd);
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
struct v4l2_mbus_framefmt *mbus_fmt;
-#endif
unsigned char com7, com10 = 0;
int ret;
@@ -1026,13 +1024,11 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
ret = ov7670_try_fmt_internal(sd, &format->format, NULL, NULL);
if (ret)
return ret;
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+ if (IS_ERR(mbus_fmt))
+ return PTR_ERR(mbus_fmt);
*mbus_fmt = format->format;
return 0;
-#else
- return -ENOTTY;
-#endif
}
ret = ov7670_try_fmt_internal(sd, &format->format, &ovfmt, &wsize);
@@ -1105,18 +1101,14 @@ static int ov7670_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_format *format)
{
struct ov7670_info *info = to_state(sd);
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
struct v4l2_mbus_framefmt *mbus_fmt;
-#endif
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
+ if (IS_ERR(mbus_fmt))
+ return PTR_ERR(mbus_fmt);
format->format = *mbus_fmt;
return 0;
-#else
- return -ENOTTY;
-#endif
} else {
format->format = info->format;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 15+ messages in thread