From: Jacopo Mondi <jacopo@jmondi.org> To: Marco Felsch <m.felsch@pengutronix.de>, Hans Verkuil <hverkuil@xs4all.nl>, Sakari Ailus <sakari.ailus@linux.intel.com> Cc: mchehab@kernel.org, robh+dt@kernel.org, mark.rutland@arm.com, devicetree@vger.kernel.org, p.zabel@pengutronix.de, javierm@redhat.com, afshin.nasser@gmail.com, laurent.pinchart@ideasonboard.com, linux-media@vger.kernel.org, kernel@pengutronix.de Subject: Re: [PATCH v4 4/7] media: v4l2-subdev: add stubs for v4l2_subdev_get_try_* Date: Thu, 21 Mar 2019 11:01:47 +0100 [thread overview] Message-ID: <20190321100147.eqfnbw6jjatqvfvw@uno.localdomain> (raw) In-Reply-To: <20190129160757.2314-5-m.felsch@pengutronix.de> [-- Attachment #1: Type: text/plain, Size: 2875 bytes --] Hi Marco, FYI we've been there already: https://patchwork.kernel.org/patch/10703029/ and that ended with Hans' patch: https://patchwork.linuxtv.org/patch/53370/ which didn't get far unfortunately. On Tue, Jan 29, 2019 at 05:07:54PM +0100, Marco Felsch wrote: > In case of missing CONFIG_VIDEO_V4L2_SUBDEV_API those helpers aren't > available. So each driver have to add ifdefs around those helpers or > add the CONFIG_VIDEO_V4L2_SUBDEV_API as dependcy. > > Make these helpers available in case of CONFIG_VIDEO_V4L2_SUBDEV_API > isn't set to avoid ifdefs. This approach is less error prone too. > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > --- > include/media/v4l2-subdev.h | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 47af609dc8f1..90c9a301d72a 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -916,8 +916,6 @@ struct v4l2_subdev_fh { > #define to_v4l2_subdev_fh(fh) \ > container_of(fh, struct v4l2_subdev_fh, vfh) > > -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > - > /** > * v4l2_subdev_get_try_format - ancillary routine to call > * &struct v4l2_subdev_pad_config->try_fmt > @@ -931,9 +929,13 @@ static inline struct v4l2_mbus_framefmt > struct v4l2_subdev_pad_config *cfg, > unsigned int pad) > { > +#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > if (WARN_ON(pad >= sd->entity.num_pads)) > pad = 0; > return &cfg[pad].try_fmt; > +#else > + return NULL; Since Hans' attempt didn't succeed, maybe we want to reconsider this approach? I liked Lubomir's version better, but in any case, small details... Shouldn't you return ERR_PTR(-ENOTTY) here instead of NULL ? + Sakari, Hans: Alternatively, what if we add CONFIG_VIDEO_V4L2_SUBDEV_API as a dependency of all sensor drivers that still use the #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API v4l2_subdev_get_try_format(sd, cfg, format->pad); #else -ENOTTY #endif pattern so we could remove that block #ifdef blocks and do not touch the v4l2-subdev.h header? Should I send a patch? Thanks j > +#endif > } > > /** > @@ -949,9 +951,13 @@ static inline struct v4l2_rect > struct v4l2_subdev_pad_config *cfg, > unsigned int pad) > { > +#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > if (WARN_ON(pad >= sd->entity.num_pads)) > pad = 0; > return &cfg[pad].try_crop; > +#else > + return NULL; > +#endif > } > > /** > @@ -967,11 +973,14 @@ static inline struct v4l2_rect > struct v4l2_subdev_pad_config *cfg, > unsigned int pad) > { > +#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > if (WARN_ON(pad >= sd->entity.num_pads)) > pad = 0; > return &cfg[pad].try_compose; > -} > +#else > + return NULL; > #endif > +} > > extern const struct v4l2_file_operations v4l2_subdev_fops; > > -- > 2.20.1 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Jacopo Mondi <jacopo@jmondi.org> To: Marco Felsch <m.felsch@pengutronix.de>, Hans Verkuil <hverkuil@xs4all.nl>, Sakari Ailus <sakari.ailus@linux.intel.com> Cc: mchehab@kernel.org, robh+dt@kernel.org, mark.rutland@arm.com, sakari.ailus@linux.intel.com, devicetree@vger.kernel.org, p.zabel@pengutronix.de, javierm@redhat.com, afshin.nasser@gmail.com, laurent.pinchart@ideasonboard.com, linux-media@vger.kernel.org, kernel@pengutronix.de Subject: Re: [PATCH v4 4/7] media: v4l2-subdev: add stubs for v4l2_subdev_get_try_* Date: Thu, 21 Mar 2019 11:01:47 +0100 [thread overview] Message-ID: <20190321100147.eqfnbw6jjatqvfvw@uno.localdomain> (raw) In-Reply-To: <20190129160757.2314-5-m.felsch@pengutronix.de> [-- Attachment #1: Type: text/plain, Size: 2875 bytes --] Hi Marco, FYI we've been there already: https://patchwork.kernel.org/patch/10703029/ and that ended with Hans' patch: https://patchwork.linuxtv.org/patch/53370/ which didn't get far unfortunately. On Tue, Jan 29, 2019 at 05:07:54PM +0100, Marco Felsch wrote: > In case of missing CONFIG_VIDEO_V4L2_SUBDEV_API those helpers aren't > available. So each driver have to add ifdefs around those helpers or > add the CONFIG_VIDEO_V4L2_SUBDEV_API as dependcy. > > Make these helpers available in case of CONFIG_VIDEO_V4L2_SUBDEV_API > isn't set to avoid ifdefs. This approach is less error prone too. > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > --- > include/media/v4l2-subdev.h | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 47af609dc8f1..90c9a301d72a 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -916,8 +916,6 @@ struct v4l2_subdev_fh { > #define to_v4l2_subdev_fh(fh) \ > container_of(fh, struct v4l2_subdev_fh, vfh) > > -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > - > /** > * v4l2_subdev_get_try_format - ancillary routine to call > * &struct v4l2_subdev_pad_config->try_fmt > @@ -931,9 +929,13 @@ static inline struct v4l2_mbus_framefmt > struct v4l2_subdev_pad_config *cfg, > unsigned int pad) > { > +#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > if (WARN_ON(pad >= sd->entity.num_pads)) > pad = 0; > return &cfg[pad].try_fmt; > +#else > + return NULL; Since Hans' attempt didn't succeed, maybe we want to reconsider this approach? I liked Lubomir's version better, but in any case, small details... Shouldn't you return ERR_PTR(-ENOTTY) here instead of NULL ? + Sakari, Hans: Alternatively, what if we add CONFIG_VIDEO_V4L2_SUBDEV_API as a dependency of all sensor drivers that still use the #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API v4l2_subdev_get_try_format(sd, cfg, format->pad); #else -ENOTTY #endif pattern so we could remove that block #ifdef blocks and do not touch the v4l2-subdev.h header? Should I send a patch? Thanks j > +#endif > } > > /** > @@ -949,9 +951,13 @@ static inline struct v4l2_rect > struct v4l2_subdev_pad_config *cfg, > unsigned int pad) > { > +#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > if (WARN_ON(pad >= sd->entity.num_pads)) > pad = 0; > return &cfg[pad].try_crop; > +#else > + return NULL; > +#endif > } > > /** > @@ -967,11 +973,14 @@ static inline struct v4l2_rect > struct v4l2_subdev_pad_config *cfg, > unsigned int pad) > { > +#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > if (WARN_ON(pad >= sd->entity.num_pads)) > pad = 0; > return &cfg[pad].try_compose; > -} > +#else > + return NULL; > #endif > +} > > extern const struct v4l2_file_operations v4l2_subdev_fops; > > -- > 2.20.1 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2019-03-21 10:01 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-01-29 16:07 [PATCH v4 0/7] TVP5150 new features Marco Felsch 2019-01-29 16:07 ` [PATCH v4 1/7] partial revert of "[media] tvp5150: add HW input connectors support" Marco Felsch 2019-01-29 16:07 ` [PATCH v4 2/7] media: tvp5150: add input source selection of_graph support Marco Felsch 2019-03-21 13:03 ` Jacopo Mondi 2019-03-27 16:22 ` Marco Felsch 2019-01-29 16:07 ` [PATCH v4 3/7] media: dt-bindings: tvp5150: Add input port connectors DT bindings Marco Felsch 2019-01-29 16:07 ` [PATCH v4 4/7] media: v4l2-subdev: add stubs for v4l2_subdev_get_try_* Marco Felsch 2019-03-21 10:01 ` Jacopo Mondi [this message] 2019-03-21 10:01 ` Jacopo Mondi 2019-03-21 10:59 ` Marco Felsch 2019-01-29 16:07 ` [PATCH v4 5/7] media: tvp5150: add FORMAT_TRY support for get/set selection handlers Marco Felsch 2019-01-29 16:07 ` [PATCH v4 6/7] media: tvp5150: initialize subdev before parsing device tree Marco Felsch 2019-01-29 16:07 ` [PATCH v4 7/7] media: tvp5150: add s_power callback Marco Felsch 2019-02-12 16:09 ` [PATCH v4 0/7] TVP5150 new features Marco Felsch 2019-03-05 9:46 ` Marco Felsch
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190321100147.eqfnbw6jjatqvfvw@uno.localdomain \ --to=jacopo@jmondi.org \ --cc=afshin.nasser@gmail.com \ --cc=devicetree@vger.kernel.org \ --cc=hverkuil@xs4all.nl \ --cc=javierm@redhat.com \ --cc=kernel@pengutronix.de \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-media@vger.kernel.org \ --cc=m.felsch@pengutronix.de \ --cc=mark.rutland@arm.com \ --cc=mchehab@kernel.org \ --cc=p.zabel@pengutronix.de \ --cc=robh+dt@kernel.org \ --cc=sakari.ailus@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.