diff mbox series

media: v4l2-fwnode: Return -EINVAL for invalid bus-type

Message ID 20200915155544.826-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive)
State Superseded, archived
Delegated to: Sakari Ailus
Headers
Series media: v4l2-fwnode: Return -EINVAL for invalid bus-type |

Commit Message

Prabhakar Mahadev Lad Sept. 15, 2020, 3:55 p.m. UTC 
  With the current implementation if invalid bus-type is passed via DT
v4l2_fwnode_endpoint_parse() defaulted the mus-type to V4L2_MBUS_PARALLEL
instead of returning error.

This Patch adds V4L2_MBUS_INVALID entry to v4l2_mbus_type enum and when
invalid bus-type is detected in v4l2_fwnode_endpoint_parse() it returns
-EINVAL to the caller.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 6 +++++-
 include/media/v4l2-mediabus.h         | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

Comments

Biju Das Sept. 15, 2020, 4 p.m. UTC | #1 
Hi Prabhakar,

Thanks for the patch.

> Subject: [PATCH] media: v4l2-fwnode: Return -EINVAL for invalid bus-type
>
> With the current implementation if invalid bus-type is passed via DT
> v4l2_fwnode_endpoint_parse() defaulted the mus-type to

Typo mus-type??

> V4L2_MBUS_PARALLEL instead of returning error.
>
> This Patch adds V4L2_MBUS_INVALID entry to v4l2_mbus_type enum and

This patch ??

> when invalid bus-type is detected in v4l2_fwnode_endpoint_parse() it
> returns -EINVAL to the caller.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 6 +++++-
>  include/media/v4l2-mediabus.h         | 2 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-
> core/v4l2-fwnode.c
> index a4c3c77c1894..a6f3549eadd3 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -93,7 +93,7 @@ v4l2_fwnode_bus_type_to_mbus(enum
> v4l2_fwnode_bus_type type)
>  const struct v4l2_fwnode_bus_conv *conv =
>  get_v4l2_fwnode_bus_conv_by_fwnode_bus(type);
>
> -return conv ? conv->mbus_type : V4L2_MBUS_UNKNOWN;
> +return conv ? conv->mbus_type : V4L2_MBUS_INVALID;
>  }
>
>  static const char *
> @@ -436,6 +436,10 @@ static int __v4l2_fwnode_endpoint_parse(struct
> fwnode_handle *fwnode,
>   v4l2_fwnode_mbus_type_to_string(vep->bus_type),
>   vep->bus_type);
>  mbus_type = v4l2_fwnode_bus_type_to_mbus(bus_type);
> +if (mbus_type == V4L2_MBUS_INVALID) {
> +pr_debug("unsupported bus type %u\n", bus_type);
> +return -EINVAL;
> +}
>
>  if (vep->bus_type != V4L2_MBUS_UNKNOWN) {
>  if (mbus_type != V4L2_MBUS_UNKNOWN && diff --git
> a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index
> 45f88f0248c4..b4f630783cb7 100644
> --- a/include/media/v4l2-mediabus.h
> +++ b/include/media/v4l2-mediabus.h
> @@ -78,6 +78,7 @@
>   * @V4L2_MBUS_CCP2:CCP2 (Compact Camera Port 2)
>   * @V4L2_MBUS_CSI2_DPHY: MIPI CSI-2 serial interface, with D-PHY
>   * @V4L2_MBUS_CSI2_CPHY: MIPI CSI-2 serial interface, with C-PHY
> + * @V4L2_MBUS_INVALID:invalid bus type (keep it last for sanity)
>   */
>  enum v4l2_mbus_type {
>  V4L2_MBUS_UNKNOWN,
> @@ -87,6 +88,7 @@ enum v4l2_mbus_type {
>  V4L2_MBUS_CCP2,
>  V4L2_MBUS_CSI2_DPHY,
>  V4L2_MBUS_CSI2_CPHY,
> +V4L2_MBUS_INVALID,
>  };
>
>  /**
> --
> 2.17.1



Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
Lad, Prabhakar Sept. 15, 2020, 5:35 p.m. UTC | #2 
Hi Biju,

Thank you for the review.

On Tue, Sep 15, 2020 at 5:01 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Prabhakar,
>
> Thanks for the patch.
>
> > Subject: [PATCH] media: v4l2-fwnode: Return -EINVAL for invalid bus-type
> >
> > With the current implementation if invalid bus-type is passed via DT
> > v4l2_fwnode_endpoint_parse() defaulted the mus-type to
>
> Typo mus-type??
>
> > V4L2_MBUS_PARALLEL instead of returning error.
> >
> > This Patch adds V4L2_MBUS_INVALID entry to v4l2_mbus_type enum and
>
> This patch ??
>
Argh my bad, will fix that and post a v2.

Cheers,
Prabhakar

> > when invalid bus-type is detected in v4l2_fwnode_endpoint_parse() it
> > returns -EINVAL to the caller.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-fwnode.c | 6 +++++-
> >  include/media/v4l2-mediabus.h         | 2 ++
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-
> > core/v4l2-fwnode.c
> > index a4c3c77c1894..a6f3549eadd3 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -93,7 +93,7 @@ v4l2_fwnode_bus_type_to_mbus(enum
> > v4l2_fwnode_bus_type type)
> >  const struct v4l2_fwnode_bus_conv *conv =
> >  get_v4l2_fwnode_bus_conv_by_fwnode_bus(type);
> >
> > -return conv ? conv->mbus_type : V4L2_MBUS_UNKNOWN;
> > +return conv ? conv->mbus_type : V4L2_MBUS_INVALID;
> >  }
> >
> >  static const char *
> > @@ -436,6 +436,10 @@ static int __v4l2_fwnode_endpoint_parse(struct
> > fwnode_handle *fwnode,
> >   v4l2_fwnode_mbus_type_to_string(vep->bus_type),
> >   vep->bus_type);
> >  mbus_type = v4l2_fwnode_bus_type_to_mbus(bus_type);
> > +if (mbus_type == V4L2_MBUS_INVALID) {
> > +pr_debug("unsupported bus type %u\n", bus_type);
> > +return -EINVAL;
> > +}
> >
> >  if (vep->bus_type != V4L2_MBUS_UNKNOWN) {
> >  if (mbus_type != V4L2_MBUS_UNKNOWN && diff --git
> > a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index
> > 45f88f0248c4..b4f630783cb7 100644
> > --- a/include/media/v4l2-mediabus.h
> > +++ b/include/media/v4l2-mediabus.h
> > @@ -78,6 +78,7 @@
> >   * @V4L2_MBUS_CCP2:CCP2 (Compact Camera Port 2)
> >   * @V4L2_MBUS_CSI2_DPHY: MIPI CSI-2 serial interface, with D-PHY
> >   * @V4L2_MBUS_CSI2_CPHY: MIPI CSI-2 serial interface, with C-PHY
> > + * @V4L2_MBUS_INVALID:invalid bus type (keep it last for sanity)
> >   */
> >  enum v4l2_mbus_type {
> >  V4L2_MBUS_UNKNOWN,
> > @@ -87,6 +88,7 @@ enum v4l2_mbus_type {
> >  V4L2_MBUS_CCP2,
> >  V4L2_MBUS_CSI2_DPHY,
> >  V4L2_MBUS_CSI2_CPHY,
> > +V4L2_MBUS_INVALID,
> >  };
> >
> >  /**
> > --
> > 2.17.1
>
>
>
> Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
Sakari Ailus Sept. 15, 2020, 7:48 p.m. UTC | #3 
Hi Prabhakar,

On Tue, Sep 15, 2020 at 04:55:44PM +0100, Lad Prabhakar wrote:
> With the current implementation if invalid bus-type is passed via DT
> v4l2_fwnode_endpoint_parse() defaulted the mus-type to V4L2_MBUS_PARALLEL
> instead of returning error.

The default is dug from the rest of the properties, it could be different
from parallel. You could simply not mention the actual result, just that it
should have been an error.

> 
> This Patch adds V4L2_MBUS_INVALID entry to v4l2_mbus_type enum and when
> invalid bus-type is detected in v4l2_fwnode_endpoint_parse() it returns
> -EINVAL to the caller.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 6 +++++-
>  include/media/v4l2-mediabus.h         | 2 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index a4c3c77c1894..a6f3549eadd3 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -93,7 +93,7 @@ v4l2_fwnode_bus_type_to_mbus(enum v4l2_fwnode_bus_type type)
>  	const struct v4l2_fwnode_bus_conv *conv =
>  		get_v4l2_fwnode_bus_conv_by_fwnode_bus(type);
>  
> -	return conv ? conv->mbus_type : V4L2_MBUS_UNKNOWN;
> +	return conv ? conv->mbus_type : V4L2_MBUS_INVALID;
>  }
>  
>  static const char *
> @@ -436,6 +436,10 @@ static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
>  		 v4l2_fwnode_mbus_type_to_string(vep->bus_type),
>  		 vep->bus_type);
>  	mbus_type = v4l2_fwnode_bus_type_to_mbus(bus_type);
> +	if (mbus_type == V4L2_MBUS_INVALID) {
> +		pr_debug("unsupported bus type %u\n", bus_type);
> +		return -EINVAL;
> +	}
>  
>  	if (vep->bus_type != V4L2_MBUS_UNKNOWN) {
>  		if (mbus_type != V4L2_MBUS_UNKNOWN &&
> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> index 45f88f0248c4..b4f630783cb7 100644
> --- a/include/media/v4l2-mediabus.h
> +++ b/include/media/v4l2-mediabus.h
> @@ -78,6 +78,7 @@
>   * @V4L2_MBUS_CCP2:	CCP2 (Compact Camera Port 2)
>   * @V4L2_MBUS_CSI2_DPHY: MIPI CSI-2 serial interface, with D-PHY
>   * @V4L2_MBUS_CSI2_CPHY: MIPI CSI-2 serial interface, with C-PHY
> + * @V4L2_MBUS_INVALID:	invalid bus type (keep it last for sanity)

s/it last for sanity/as last/

>   */
>  enum v4l2_mbus_type {
>  	V4L2_MBUS_UNKNOWN,
> @@ -87,6 +88,7 @@ enum v4l2_mbus_type {
>  	V4L2_MBUS_CCP2,
>  	V4L2_MBUS_CSI2_DPHY,
>  	V4L2_MBUS_CSI2_CPHY,
> +	V4L2_MBUS_INVALID,
>  };
>  
>  /**
diff mbox series

Patch

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index a4c3c77c1894..a6f3549eadd3 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -93,7 +93,7 @@  v4l2_fwnode_bus_type_to_mbus(enum v4l2_fwnode_bus_type type)
 	const struct v4l2_fwnode_bus_conv *conv =
 		get_v4l2_fwnode_bus_conv_by_fwnode_bus(type);
 
-	return conv ? conv->mbus_type : V4L2_MBUS_UNKNOWN;
+	return conv ? conv->mbus_type : V4L2_MBUS_INVALID;
 }
 
 static const char *
@@ -436,6 +436,10 @@  static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
 		 v4l2_fwnode_mbus_type_to_string(vep->bus_type),
 		 vep->bus_type);
 	mbus_type = v4l2_fwnode_bus_type_to_mbus(bus_type);
+	if (mbus_type == V4L2_MBUS_INVALID) {
+		pr_debug("unsupported bus type %u\n", bus_type);
+		return -EINVAL;
+	}
 
 	if (vep->bus_type != V4L2_MBUS_UNKNOWN) {
 		if (mbus_type != V4L2_MBUS_UNKNOWN &&
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 45f88f0248c4..b4f630783cb7 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -78,6 +78,7 @@ 
  * @V4L2_MBUS_CCP2:	CCP2 (Compact Camera Port 2)
  * @V4L2_MBUS_CSI2_DPHY: MIPI CSI-2 serial interface, with D-PHY
  * @V4L2_MBUS_CSI2_CPHY: MIPI CSI-2 serial interface, with C-PHY
+ * @V4L2_MBUS_INVALID:	invalid bus type (keep it last for sanity)
  */
 enum v4l2_mbus_type {
 	V4L2_MBUS_UNKNOWN,
@@ -87,6 +88,7 @@  enum v4l2_mbus_type {
 	V4L2_MBUS_CCP2,
 	V4L2_MBUS_CSI2_DPHY,
 	V4L2_MBUS_CSI2_CPHY,
+	V4L2_MBUS_INVALID,
 };
 
 /**