diff mbox series

[v2,4/7,media] v4l2-subdev: fix v4l2_subdev_get_try_* dependency

Message ID 20180813092508.1334-5-m.felsch@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series TVP5150 fixes and new features | expand

Commit Message

Marco Felsch Aug. 13, 2018, 9:25 a.m. UTC 
These helpers make us of the media-controller entity which is only
available if the CONFIG_MEDIA_CONTROLLER is enabled.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 include/media/v4l2-subdev.h | 100 ++++++++++++++++++------------------
 1 file changed, 50 insertions(+), 50 deletions(-)

Comments

Sakari Ailus Sept. 14, 2018, 1:25 p.m. UTC | #1 
Hi Marco,

On Mon, Aug 13, 2018 at 11:25:05AM +0200, Marco Felsch wrote:
> These helpers make us of the media-controller entity which is only
> available if the CONFIG_MEDIA_CONTROLLER is enabled.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  include/media/v4l2-subdev.h | 100 ++++++++++++++++++------------------
>  1 file changed, 50 insertions(+), 50 deletions(-)
> 
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index ce48f1fcf295..79c066934ad2 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -912,6 +912,56 @@ struct v4l2_subdev_fh {
>  #define to_v4l2_subdev_fh(fh)	\
>  	container_of(fh, struct v4l2_subdev_fh, vfh)
>  
> +extern const struct v4l2_file_operations v4l2_subdev_fops;
> +
> +/**
> + * v4l2_set_subdevdata - Sets V4L2 dev private device data
> + *
> + * @sd: pointer to &struct v4l2_subdev
> + * @p: pointer to the private device data to be stored.
> + */
> +static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
> +{
> +	sd->dev_priv = p;
> +}
> +
> +/**
> + * v4l2_get_subdevdata - Gets V4L2 dev private device data
> + *
> + * @sd: pointer to &struct v4l2_subdev
> + *
> + * Returns the pointer to the private device data to be stored.
> + */
> +static inline void *v4l2_get_subdevdata(const struct v4l2_subdev *sd)
> +{
> +	return sd->dev_priv;
> +}
> +
> +/**
> + * v4l2_set_subdev_hostdata - Sets V4L2 dev private host data
> + *
> + * @sd: pointer to &struct v4l2_subdev
> + * @p: pointer to the private data to be stored.
> + */
> +static inline void v4l2_set_subdev_hostdata(struct v4l2_subdev *sd, void *p)
> +{
> +	sd->host_priv = p;
> +}
> +
> +/**
> + * v4l2_get_subdev_hostdata - Gets V4L2 dev private data
> + *
> + * @sd: pointer to &struct v4l2_subdev
> + *
> + * Returns the pointer to the private host data to be stored.
> + */
> +static inline void *v4l2_get_subdev_hostdata(const struct v4l2_subdev *sd)
> +{
> +	return sd->host_priv;
> +}

Could you leave the functions dealing with host_priv where they are? I'd
like to avoid expanding their use; rather reduce it. The field is
problematic in some cases and generally not really needed either.

> +
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +
>  /**
>   * v4l2_subdev_get_try_format - ancillary routine to call
>   *	&struct v4l2_subdev_pad_config->try_fmt
> @@ -978,56 +1028,6 @@ static inline struct v4l2_rect
>  #endif
>  }
>  
> -extern const struct v4l2_file_operations v4l2_subdev_fops;
> -
> -/**
> - * v4l2_set_subdevdata - Sets V4L2 dev private device data
> - *
> - * @sd: pointer to &struct v4l2_subdev
> - * @p: pointer to the private device data to be stored.
> - */
> -static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
> -{
> -	sd->dev_priv = p;
> -}
> -
> -/**
> - * v4l2_get_subdevdata - Gets V4L2 dev private device data
> - *
> - * @sd: pointer to &struct v4l2_subdev
> - *
> - * Returns the pointer to the private device data to be stored.
> - */
> -static inline void *v4l2_get_subdevdata(const struct v4l2_subdev *sd)
> -{
> -	return sd->dev_priv;
> -}
> -
> -/**
> - * v4l2_set_subdev_hostdata - Sets V4L2 dev private host data
> - *
> - * @sd: pointer to &struct v4l2_subdev
> - * @p: pointer to the private data to be stored.
> - */
> -static inline void v4l2_set_subdev_hostdata(struct v4l2_subdev *sd, void *p)
> -{
> -	sd->host_priv = p;
> -}
> -
> -/**
> - * v4l2_get_subdev_hostdata - Gets V4L2 dev private data
> - *
> - * @sd: pointer to &struct v4l2_subdev
> - *
> - * Returns the pointer to the private host data to be stored.
> - */
> -static inline void *v4l2_get_subdev_hostdata(const struct v4l2_subdev *sd)
> -{
> -	return sd->host_priv;
> -}
> -
> -#ifdef CONFIG_MEDIA_CONTROLLER
> -
>  /**
>   * v4l2_subdev_link_validate_default - validates a media link
>   *
Marco Felsch Sept. 14, 2018, 6:10 p.m. UTC | #2 
Hi Sakari,

On 18-09-14 16:25, Sakari Ailus wrote:
> Hi Marco,
> 
> On Mon, Aug 13, 2018 at 11:25:05AM +0200, Marco Felsch wrote:
> > These helpers make us of the media-controller entity which is only
> > available if the CONFIG_MEDIA_CONTROLLER is enabled.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  include/media/v4l2-subdev.h | 100 ++++++++++++++++++------------------
> >  1 file changed, 50 insertions(+), 50 deletions(-)
> > 
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index ce48f1fcf295..79c066934ad2 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -912,6 +912,56 @@ struct v4l2_subdev_fh {
> >  #define to_v4l2_subdev_fh(fh)	\
> >  	container_of(fh, struct v4l2_subdev_fh, vfh)
> >  
> > +extern const struct v4l2_file_operations v4l2_subdev_fops;
> > +
> > +/**
> > + * v4l2_set_subdevdata - Sets V4L2 dev private device data
> > + *
> > + * @sd: pointer to &struct v4l2_subdev
> > + * @p: pointer to the private device data to be stored.
> > + */
> > +static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
> > +{
> > +	sd->dev_priv = p;
> > +}
> > +
> > +/**
> > + * v4l2_get_subdevdata - Gets V4L2 dev private device data
> > + *
> > + * @sd: pointer to &struct v4l2_subdev
> > + *
> > + * Returns the pointer to the private device data to be stored.
> > + */
> > +static inline void *v4l2_get_subdevdata(const struct v4l2_subdev *sd)
> > +{
> > +	return sd->dev_priv;
> > +}
> > +
> > +/**
> > + * v4l2_set_subdev_hostdata - Sets V4L2 dev private host data
> > + *
> > + * @sd: pointer to &struct v4l2_subdev
> > + * @p: pointer to the private data to be stored.
> > + */
> > +static inline void v4l2_set_subdev_hostdata(struct v4l2_subdev *sd, void *p)
> > +{
> > +	sd->host_priv = p;
> > +}
> > +
> > +/**
> > + * v4l2_get_subdev_hostdata - Gets V4L2 dev private data
> > + *
> > + * @sd: pointer to &struct v4l2_subdev
> > + *
> > + * Returns the pointer to the private host data to be stored.
> > + */
> > +static inline void *v4l2_get_subdev_hostdata(const struct v4l2_subdev *sd)
> > +{
> > +	return sd->host_priv;
> > +}
> 
> Could you leave the functions dealing with host_priv where they are? I'd
> like to avoid expanding their use; rather reduce it. The field is
> problematic in some cases and generally not really needed either.

Sure, I just moved the v4l2_subdev_get_try_* formats to the
CONFIG_MEDIA_CONTROLLER ifdef block to avoid a second ifdef block (see
below). Git made it that way.. and I'm with you, the patch looks not good.
Should I open a 2nd ifdef CONFIG_MEDIA_CONTROLLER block instead?

Regards,
Marco

> 
> > +
> > +#ifdef CONFIG_MEDIA_CONTROLLER
> > +
> >  /**
> >   * v4l2_subdev_get_try_format - ancillary routine to call
> >   *	&struct v4l2_subdev_pad_config->try_fmt
> > @@ -978,56 +1028,6 @@ static inline struct v4l2_rect
> >  #endif
> >  }
> >  
> > -extern const struct v4l2_file_operations v4l2_subdev_fops;
> > -
> > -/**
> > - * v4l2_set_subdevdata - Sets V4L2 dev private device data
> > - *
> > - * @sd: pointer to &struct v4l2_subdev
> > - * @p: pointer to the private device data to be stored.
> > - */
> > -static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
> > -{
> > -	sd->dev_priv = p;
> > -}
> > -
> > -/**
> > - * v4l2_get_subdevdata - Gets V4L2 dev private device data
> > - *
> > - * @sd: pointer to &struct v4l2_subdev
> > - *
> > - * Returns the pointer to the private device data to be stored.
> > - */
> > -static inline void *v4l2_get_subdevdata(const struct v4l2_subdev *sd)
> > -{
> > -	return sd->dev_priv;
> > -}
> > -
> > -/**
> > - * v4l2_set_subdev_hostdata - Sets V4L2 dev private host data
> > - *
> > - * @sd: pointer to &struct v4l2_subdev
> > - * @p: pointer to the private data to be stored.
> > - */
> > -static inline void v4l2_set_subdev_hostdata(struct v4l2_subdev *sd, void *p)
> > -{
> > -	sd->host_priv = p;
> > -}
> > -
> > -/**
> > - * v4l2_get_subdev_hostdata - Gets V4L2 dev private data
> > - *
> > - * @sd: pointer to &struct v4l2_subdev
> > - *
> > - * Returns the pointer to the private host data to be stored.
> > - */
> > -static inline void *v4l2_get_subdev_hostdata(const struct v4l2_subdev *sd)
> > -{
> > -	return sd->host_priv;
> > -}
> > -
> > -#ifdef CONFIG_MEDIA_CONTROLLER
> > -
> >  /**
> >   * v4l2_subdev_link_validate_default - validates a media link
> >   *
> 
> -- 
> Kind regards,
> 
> Sakari Ailus
> sakari.ailus@linux.intel.com
>
diff mbox series

Patch

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index ce48f1fcf295..79c066934ad2 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -912,6 +912,56 @@  struct v4l2_subdev_fh {
 #define to_v4l2_subdev_fh(fh)	\
 	container_of(fh, struct v4l2_subdev_fh, vfh)
 
+extern const struct v4l2_file_operations v4l2_subdev_fops;
+
+/**
+ * v4l2_set_subdevdata - Sets V4L2 dev private device data
+ *
+ * @sd: pointer to &struct v4l2_subdev
+ * @p: pointer to the private device data to be stored.
+ */
+static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
+{
+	sd->dev_priv = p;
+}
+
+/**
+ * v4l2_get_subdevdata - Gets V4L2 dev private device data
+ *
+ * @sd: pointer to &struct v4l2_subdev
+ *
+ * Returns the pointer to the private device data to be stored.
+ */
+static inline void *v4l2_get_subdevdata(const struct v4l2_subdev *sd)
+{
+	return sd->dev_priv;
+}
+
+/**
+ * v4l2_set_subdev_hostdata - Sets V4L2 dev private host data
+ *
+ * @sd: pointer to &struct v4l2_subdev
+ * @p: pointer to the private data to be stored.
+ */
+static inline void v4l2_set_subdev_hostdata(struct v4l2_subdev *sd, void *p)
+{
+	sd->host_priv = p;
+}
+
+/**
+ * v4l2_get_subdev_hostdata - Gets V4L2 dev private data
+ *
+ * @sd: pointer to &struct v4l2_subdev
+ *
+ * Returns the pointer to the private host data to be stored.
+ */
+static inline void *v4l2_get_subdev_hostdata(const struct v4l2_subdev *sd)
+{
+	return sd->host_priv;
+}
+
+#ifdef CONFIG_MEDIA_CONTROLLER
+
 /**
  * v4l2_subdev_get_try_format - ancillary routine to call
  *	&struct v4l2_subdev_pad_config->try_fmt
@@ -978,56 +1028,6 @@  static inline struct v4l2_rect
 #endif
 }
 
-extern const struct v4l2_file_operations v4l2_subdev_fops;
-
-/**
- * v4l2_set_subdevdata - Sets V4L2 dev private device data
- *
- * @sd: pointer to &struct v4l2_subdev
- * @p: pointer to the private device data to be stored.
- */
-static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
-{
-	sd->dev_priv = p;
-}
-
-/**
- * v4l2_get_subdevdata - Gets V4L2 dev private device data
- *
- * @sd: pointer to &struct v4l2_subdev
- *
- * Returns the pointer to the private device data to be stored.
- */
-static inline void *v4l2_get_subdevdata(const struct v4l2_subdev *sd)
-{
-	return sd->dev_priv;
-}
-
-/**
- * v4l2_set_subdev_hostdata - Sets V4L2 dev private host data
- *
- * @sd: pointer to &struct v4l2_subdev
- * @p: pointer to the private data to be stored.
- */
-static inline void v4l2_set_subdev_hostdata(struct v4l2_subdev *sd, void *p)
-{
-	sd->host_priv = p;
-}
-
-/**
- * v4l2_get_subdev_hostdata - Gets V4L2 dev private data
- *
- * @sd: pointer to &struct v4l2_subdev
- *
- * Returns the pointer to the private host data to be stored.
- */
-static inline void *v4l2_get_subdev_hostdata(const struct v4l2_subdev *sd)
-{
-	return sd->host_priv;
-}
-
-#ifdef CONFIG_MEDIA_CONTROLLER
-
 /**
  * v4l2_subdev_link_validate_default - validates a media link
  *