[3/3] v4l2-fwnode: Document changes usage patterns of v4l2_fwnode_endpoint_parse
Commit Message
Document that it is possible to provide defaults for multiple bus types to
v4l2_fwnode_endpoint_parse and v4l2_fwnode_endpoint_alloc_parse. Also
underline the fact that detecting the bus type without bus-type property
is only for the old drivers.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
include/media/v4l2-fwnode.h | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
Comments
The subject should have been: "[PATCH 3/3] v4l2-fwnode: Document new usage
patterns of v4l2_fwnode_endpoint_parse".
Hi Sakari,
Thank you for the patch.
On Tue, Sep 08, 2020 at 11:51:21AM +0300, Sakari Ailus wrote:
> Document that it is possible to provide defaults for multiple bus types to
> v4l2_fwnode_endpoint_parse and v4l2_fwnode_endpoint_alloc_parse. Also
> underline the fact that detecting the bus type without bus-type property
> is only for the old drivers.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> include/media/v4l2-fwnode.h | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index d04f39b60096..ccaa5693df14 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -226,11 +226,10 @@ struct v4l2_fwnode_connector {
> * call this function once the correct type is found --- with a default
> * configuration valid for that type.
> *
> - * As a compatibility means guessing the bus type is also supported by setting
> - * @vep.bus_type to V4L2_MBUS_UNKNOWN. The caller may not provide a default
> - * configuration in this case as the defaults are specific to a given bus type.
> - * This functionality is deprecated and should not be used in new drivers and it
> - * is only supported for CSI-2 D-PHY, parallel and Bt.656 buses.
> + * It is also allowed to set @vep.bus_type to V4L2_MBUS_UNKNOWN. USING THIS
> + * FEATURE REQUIRES "bus-type" PROPERTY IN DT BINDINGS. For old drivers,
> + * guessing @vep.bus_type between CSI-2 D-PHY, parallel and Bt.656 busses is
While at it, s/Bt/BT/.
> + * supported. NEVER RELY ON GUESSING @vep.bus_type IN NEW DRIVERS!
That's fairly clear :-)
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
I wonder if, as a further improvement, we should turn the enum
v4l2_mbus_type into a bitfield, to let drivers tell which bus types they
support. The helpers could then return an error if the bus type reported
by the firmware doesn't match.
> *
> * The function does not change the V4L2 fwnode endpoint state if it fails.
> *
> @@ -269,11 +268,10 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);
> * call this function once the correct type is found --- with a default
> * configuration valid for that type.
> *
> - * As a compatibility means guessing the bus type is also supported by setting
> - * @vep.bus_type to V4L2_MBUS_UNKNOWN. The caller may not provide a default
> - * configuration in this case as the defaults are specific to a given bus type.
> - * This functionality is deprecated and should not be used in new drivers and it
> - * is only supported for CSI-2 D-PHY, parallel and Bt.656 buses.
> + * It is also allowed to set @vep.bus_type to V4L2_MBUS_UNKNOWN. USING THIS
> + * FEATURE REQUIRES "bus-type" PROPERTY IN DT BINDINGS. For old drivers,
> + * guessing @vep.bus_type between CSI-2 D-PHY, parallel and Bt.656 busses is
> + * supported. NEVER RELY ON GUESSING @vep.bus_type IN NEW DRIVERS!
> *
> * The function does not change the V4L2 fwnode endpoint state if it fails.
> *
Hi Laurent,
On Tue, Sep 08, 2020 at 03:51:07PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
Thank you for the review!
>
> On Tue, Sep 08, 2020 at 11:51:21AM +0300, Sakari Ailus wrote:
> > Document that it is possible to provide defaults for multiple bus types to
> > v4l2_fwnode_endpoint_parse and v4l2_fwnode_endpoint_alloc_parse. Also
> > underline the fact that detecting the bus type without bus-type property
> > is only for the old drivers.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> > include/media/v4l2-fwnode.h | 18 ++++++++----------
> > 1 file changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> > index d04f39b60096..ccaa5693df14 100644
> > --- a/include/media/v4l2-fwnode.h
> > +++ b/include/media/v4l2-fwnode.h
> > @@ -226,11 +226,10 @@ struct v4l2_fwnode_connector {
> > * call this function once the correct type is found --- with a default
> > * configuration valid for that type.
> > *
> > - * As a compatibility means guessing the bus type is also supported by setting
> > - * @vep.bus_type to V4L2_MBUS_UNKNOWN. The caller may not provide a default
> > - * configuration in this case as the defaults are specific to a given bus type.
> > - * This functionality is deprecated and should not be used in new drivers and it
> > - * is only supported for CSI-2 D-PHY, parallel and Bt.656 buses.
> > + * It is also allowed to set @vep.bus_type to V4L2_MBUS_UNKNOWN. USING THIS
> > + * FEATURE REQUIRES "bus-type" PROPERTY IN DT BINDINGS. For old drivers,
> > + * guessing @vep.bus_type between CSI-2 D-PHY, parallel and Bt.656 busses is
>
> While at it, s/Bt/BT/.
Yes.
>
> > + * supported. NEVER RELY ON GUESSING @vep.bus_type IN NEW DRIVERS!
>
> That's fairly clear :-)
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> I wonder if, as a further improvement, we should turn the enum
> v4l2_mbus_type into a bitfield, to let drivers tell which bus types they
> support. The helpers could then return an error if the bus type reported
> by the firmware doesn't match.
I agree. That'd be less work for the caller. That does require changing a
bunch of drivers though. Unless we add another field for that purpose, just
like the I²C framework did. I guess it's not necessary. Ideally it'd be
done so that trying to use it the old way would generate a compiler
warning.
Hi Sakari,
On Tue, Sep 08, 2020 at 11:51:21AM +0300, Sakari Ailus wrote:
> Document that it is possible to provide defaults for multiple bus types to
> v4l2_fwnode_endpoint_parse and v4l2_fwnode_endpoint_alloc_parse. Also
> underline the fact that detecting the bus type without bus-type property
> is only for the old drivers.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> include/media/v4l2-fwnode.h | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index d04f39b60096..ccaa5693df14 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -226,11 +226,10 @@ struct v4l2_fwnode_connector {
> * call this function once the correct type is found --- with a default
> * configuration valid for that type.
> *
> - * As a compatibility means guessing the bus type is also supported by setting
> - * @vep.bus_type to V4L2_MBUS_UNKNOWN. The caller may not provide a default
> - * configuration in this case as the defaults are specific to a given bus type.
> - * This functionality is deprecated and should not be used in new drivers and it
> - * is only supported for CSI-2 D-PHY, parallel and Bt.656 buses.
> + * It is also allowed to set @vep.bus_type to V4L2_MBUS_UNKNOWN. USING THIS
> + * FEATURE REQUIRES "bus-type" PROPERTY IN DT BINDINGS. For old drivers,
> + * guessing @vep.bus_type between CSI-2 D-PHY, parallel and Bt.656 busses is
> + * supported. NEVER RELY ON GUESSING @vep.bus_type IN NEW DRIVERS!
As per the discussion in:
[PATCH v4 1/3] media: i2c: ov772x: Parse endpoint properties
this leaves a bit of gray area on how to address cases where older dts
do not report any "bus-type" property, as in the case of PARALLEL vs
BT.656 Prabhakar has, it gets impossible to detect mis-configurations.
I have suggested him two different behaviours, depending if 'bus-type'
is present in the fwnode or not, but I'm not sure that's the best way
forward. What do you think ?
> *
> * The function does not change the V4L2 fwnode endpoint state if it fails.
> *
> @@ -269,11 +268,10 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);
> * call this function once the correct type is found --- with a default
> * configuration valid for that type.
> *
> - * As a compatibility means guessing the bus type is also supported by setting
> - * @vep.bus_type to V4L2_MBUS_UNKNOWN. The caller may not provide a default
> - * configuration in this case as the defaults are specific to a given bus type.
> - * This functionality is deprecated and should not be used in new drivers and it
> - * is only supported for CSI-2 D-PHY, parallel and Bt.656 buses.
> + * It is also allowed to set @vep.bus_type to V4L2_MBUS_UNKNOWN. USING THIS
> + * FEATURE REQUIRES "bus-type" PROPERTY IN DT BINDINGS. For old drivers,
> + * guessing @vep.bus_type between CSI-2 D-PHY, parallel and Bt.656 busses is
> + * supported. NEVER RELY ON GUESSING @vep.bus_type IN NEW DRIVERS!
> *
> * The function does not change the V4L2 fwnode endpoint state if it fails.
> *
> --
> 2.27.0
>
@@ -226,11 +226,10 @@ struct v4l2_fwnode_connector {
* call this function once the correct type is found --- with a default
* configuration valid for that type.
*
- * As a compatibility means guessing the bus type is also supported by setting
- * @vep.bus_type to V4L2_MBUS_UNKNOWN. The caller may not provide a default
- * configuration in this case as the defaults are specific to a given bus type.
- * This functionality is deprecated and should not be used in new drivers and it
- * is only supported for CSI-2 D-PHY, parallel and Bt.656 buses.
+ * It is also allowed to set @vep.bus_type to V4L2_MBUS_UNKNOWN. USING THIS
+ * FEATURE REQUIRES "bus-type" PROPERTY IN DT BINDINGS. For old drivers,
+ * guessing @vep.bus_type between CSI-2 D-PHY, parallel and Bt.656 busses is
+ * supported. NEVER RELY ON GUESSING @vep.bus_type IN NEW DRIVERS!
*
* The function does not change the V4L2 fwnode endpoint state if it fails.
*
@@ -269,11 +268,10 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);
* call this function once the correct type is found --- with a default
* configuration valid for that type.
*
- * As a compatibility means guessing the bus type is also supported by setting
- * @vep.bus_type to V4L2_MBUS_UNKNOWN. The caller may not provide a default
- * configuration in this case as the defaults are specific to a given bus type.
- * This functionality is deprecated and should not be used in new drivers and it
- * is only supported for CSI-2 D-PHY, parallel and Bt.656 buses.
+ * It is also allowed to set @vep.bus_type to V4L2_MBUS_UNKNOWN. USING THIS
+ * FEATURE REQUIRES "bus-type" PROPERTY IN DT BINDINGS. For old drivers,
+ * guessing @vep.bus_type between CSI-2 D-PHY, parallel and Bt.656 busses is
+ * supported. NEVER RELY ON GUESSING @vep.bus_type IN NEW DRIVERS!
*
* The function does not change the V4L2 fwnode endpoint state if it fails.
*