| Message ID | 20200915155544.826-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: v4l2-fwnode: Return -EINVAL for invalid bus-type | expand |
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
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
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 --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, }; /**
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(-)