[PATCH 0/5] V4L2 fwnode fixes, improvements and cleanups

[PATCH 0/5] V4L2 fwnode fixes, improvements and cleanups

[Sakari Ailus]

Hi all,

This small set improves V4L2 fwnode framework in small but in an important
ways. In particular:

- Default parameters are no longer zeroed for V4L2_MBUS_UNKNOWN --- this
  makes defaults for multiple bus types actually working, so it's a bugfix

- Don't tell to use v4l2_fwnode_endpoint_alloc_parse in all drivers;
  v4l2_fwnode_endpoint_parse is just as good if the device doesn't need
  link-frequencies.

- Rewrite v4l2_fwnode_endpoint(_alloc)_parse documentation

Laurent: I believe this addresses your request to parse multiple bus types
with a single call to v4l2_fwnode_endpoint_(alloc_)parse --- please see
the 4th patch. You don't really need to tell the default, but the caller
will need to check the result is one of the known types. But as the caller
very probably needs to do some work based on the bus type right afterwards
this is hardly an actual burden for drivers.

Sakari Ailus (5):
  adv748x: Zero entire struct v4l2_fwnode_endpoint
  v4l2-fwnode: v4l2_fwnode_endpoint_parse caller must init vep argument
  v4l2-fwnode: Don't zero parts of struct v4l2_fwnode_endpoint anymore
  v4l2-fwnode: Rework v4l2_fwnode_endpoint_parse documentation
  v4l2-fwnode: Say it's fine to use v4l2_fwnode_endpoint_parse

 drivers/media/i2c/adv748x/adv748x-core.c |  3 +-
 drivers/media/v4l2-core/v4l2-fwnode.c    | 12 ----
 include/media/v4l2-fwnode.h              | 70 ++++++++++++++++--------
 3 files changed, 47 insertions(+), 38 deletions(-)

-- 
2.27.0

[PATCH 1/5] adv748x: Zero entire struct v4l2_fwnode_endpoint

[Sakari Ailus]

The v4l2_fwnode_parse_endpoint() function can make use of defaults in
multiple bus types. To use this feature, all callers must zero the rest of
the fields of this struct, too. All other drivers appear to do that
already apart from this one.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/adv748x/adv748x-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index 1fe7f97c6d52..ae8b7ebf3830 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -589,14 +589,13 @@ static int adv748x_parse_csi2_lanes(struct adv748x_state *state,
 				    unsigned int port,
 				    struct device_node *ep)
 {
-	struct v4l2_fwnode_endpoint vep;
+	struct v4l2_fwnode_endpoint vep = { .bus_type = V4L2_MBUS_CSI2_DPHY };
 	unsigned int num_lanes;
 	int ret;
 
 	if (port != ADV748X_PORT_TXA && port != ADV748X_PORT_TXB)
 		return 0;
 
-	vep.bus_type = V4L2_MBUS_CSI2_DPHY;
 	ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &vep);
 	if (ret)
 		return ret;
-- 
2.27.0

[PATCH 2/5] v4l2-fwnode: v4l2_fwnode_endpoint_parse caller must init vep argument

[Sakari Ailus]

Document that the caller of v4l2_fwnode_endpoint_parse() must init the
fields of struct v4l2_fwnode_endpoint (vep argument) fields.

It used to be that the fields were zeroed by v4l2_fwnode_endpoint_parse
when bus type was set to V4L2_MBUS_UNKNOWN but with commit bb4bba9232fc
that no longer makes sense.

Fixes: bb4bba9232fc ("media: v4l2-fwnode: Make bus configuration a struct")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 include/media/v4l2-fwnode.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index c09074276543..0f9a768b1a8d 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -231,6 +231,8 @@ struct v4l2_fwnode_connector {
  * 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 caller is required to initialise all fields of @vep.
+ *
  * The function does not change the V4L2 fwnode endpoint state if it fails.
  *
  * NOTE: This function does not parse properties the size of which is variable
@@ -273,6 +275,8 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);
  * 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 caller is required to initialise all fields of @vep.
+ *
  * The function does not change the V4L2 fwnode endpoint state if it fails.
  *
  * v4l2_fwnode_endpoint_alloc_parse() has two important differences to
-- 
2.27.0

[PATCH 3/5] v4l2-fwnode: Don't zero parts of struct v4l2_fwnode_endpoint anymore

[Sakari Ailus]

Don't zero parts of the vep argument to v4l2_fwnode_endpoint_parse()
anymore as this can no longer be done while still supporting defaults on
multiple bus types.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index dfc53d11053f..44dd04b05e29 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -416,20 +416,8 @@ static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
 	enum v4l2_mbus_type mbus_type;
 	int rval;
 
-	if (vep->bus_type == V4L2_MBUS_UNKNOWN) {
-		/* Zero fields from bus union to until the end */
-		memset(&vep->bus, 0,
-		       sizeof(*vep) - offsetof(typeof(*vep), bus));
-	}
-
 	pr_debug("===== begin parsing endpoint %pfw\n", fwnode);
 
-	/*
-	 * Zero the fwnode graph endpoint memory in case we don't end up parsing
-	 * the endpoint.
-	 */
-	memset(&vep->base, 0, sizeof(vep->base));
-
 	fwnode_property_read_u32(fwnode, "bus-type", &bus_type);
 	pr_debug("fwnode video bus type %s (%u), mbus type %s (%u)\n",
 		 v4l2_fwnode_bus_type_to_string(bus_type), bus_type,
-- 
2.27.0

[PATCH 4/5] v4l2-fwnode: Rework v4l2_fwnode_endpoint_parse documentation

[Sakari Ailus]

Rework the documentation of v4l2_fwnode_endpoint_parse for better
readability, usefulness and correctness.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 include/media/v4l2-fwnode.h | 62 ++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index 0f9a768b1a8d..0c28dc11e829 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -219,17 +219,26 @@ struct v4l2_fwnode_connector {
  * @vep: pointer to the V4L2 fwnode data structure
  *
  * This function parses the V4L2 fwnode endpoint specific parameters from the
- * firmware. The caller is responsible for assigning @vep.bus_type to a valid
- * media bus type. The caller may also set the default configuration for the
- * endpoint --- a configuration that shall be in line with the DT binding
- * documentation. Should a device support multiple bus types, the caller may
- * call this function once the correct type is found --- with a default
- * configuration valid for that type.
- *
- * 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!
+ * firmware. There are two ways to use this function, either by letting it
+ * obtain the type of the bus (by setting the @vep.bus_type field to
+ * V4L2_MBUS_UNKNOWN) or specifying the bus type explicitly to one of the &enum
+ * v4l2_mbus_type types.
+ *
+ * When @vep.bus_type is V4L2_MBUS_UNKNOWN, the function will use the "bus-type"
+ * property to determine the type when it is available. The caller is
+ * responsible for validating the contents of @vep.bus_type field after the call
+ * returns.
+ *
+ * As a deprecated functionality to support older DT bindings without "bus-type"
+ * property for devices that support multiple types, if the "bus-type" property
+ * does not exist, the function will attempt to guess the type based on the
+ * endpoint properties available. NEVER RELY ON GUESSING THE BUS TYPE IN NEW
+ * DRIVERS OR BINDINGS.
+ *
+ * It is also possible to set @vep.bus_type corresponding to an actual bus. In
+ * this case the function will only attempt to parse properties related to this
+ * bus, and it will return an error if the value of the "bus-type" property
+ * corresponds to a different bus.
  *
  * The caller is required to initialise all fields of @vep.
  *
@@ -263,17 +272,26 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);
  * @vep: pointer to the V4L2 fwnode data structure
  *
  * This function parses the V4L2 fwnode endpoint specific parameters from the
- * firmware. The caller is responsible for assigning @vep.bus_type to a valid
- * media bus type. The caller may also set the default configuration for the
- * endpoint --- a configuration that shall be in line with the DT binding
- * documentation. Should a device support multiple bus types, the caller may
- * call this function once the correct type is found --- with a default
- * configuration valid for that type.
- *
- * 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!
+ * firmware. There are two ways to use this function, either by letting it
+ * obtain the type of the bus (by setting the @vep.bus_type field to
+ * V4L2_MBUS_UNKNOWN) or specifying the bus type explicitly to one of the &enum
+ * v4l2_mbus_type types.
+ *
+ * When @vep.bus_type is V4L2_MBUS_UNKNOWN, the function will use the "bus-type"
+ * property to determine the type when it is available. The caller is
+ * responsible for validating the contents of @vep.bus_type field after the call
+ * returns.
+ *
+ * As a deprecated functionality to support older DT bindings without "bus-type"
+ * property for devices that support multiple types, if the "bus-type" property
+ * does not exist, the function will attempt to guess the type based on the
+ * endpoint properties available. NEVER RELY ON GUESSING THE BUS TYPE IN NEW
+ * DRIVERS OR BINDINGS.
+ *
+ * It is also possible to set @vep.bus_type corresponding to an actual bus. In
+ * this case the function will only attempt to parse properties related to this
+ * bus, and it will return an error if the value of the "bus-type" property
+ * corresponds to a different bus.
  *
  * The caller is required to initialise all fields of @vep.
  *
-- 
2.27.0

[PATCH 5/5] v4l2-fwnode: Say it's fine to use v4l2_fwnode_endpoint_parse

[Sakari Ailus]

Earlier it was expected that there would be more variable size endpoint
properties and that most if not all drivers would need them. For that
reason it was expected also that v4l2_fwnode_endpoint_parse would no
longer be needed.

What actually happened that not all drivers require "link-frequencies",
the only variable size media endpoint property without a small upper
limit. Therefore drivers that do not need that information are fine using
v4l2_fwnode_endpoint_parse. So don't tell drivers to use
v4l2_fwnode_endpoint_alloc_parse in all cases.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 include/media/v4l2-fwnode.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index 0c28dc11e829..cbd872e59f8e 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -245,8 +245,8 @@ struct v4l2_fwnode_connector {
  * The function does not change the V4L2 fwnode endpoint state if it fails.
  *
  * NOTE: This function does not parse properties the size of which is variable
- * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() in
- * new drivers instead.
+ * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() if
+ * you need properties of variable size.
  *
  * Return: %0 on success or a negative error code on failure:
  *	   %-ENOMEM on memory allocation failure
-- 
2.27.0

Re: [PATCH 1/5] adv748x: Zero entire struct v4l2_fwnode_endpoint

[Niklas Söderlund]

Hi Sakari,

Thanks for your patch.

On 2020-09-30 17:48:07 +0300, Sakari Ailus wrote:
> The v4l2_fwnode_parse_endpoint() function can make use of defaults in
> multiple bus types. To use this feature, all callers must zero the rest of
> the fields of this struct, too. All other drivers appear to do that
> already apart from this one.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/media/i2c/adv748x/adv748x-core.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
> index 1fe7f97c6d52..ae8b7ebf3830 100644
> --- a/drivers/media/i2c/adv748x/adv748x-core.c
> +++ b/drivers/media/i2c/adv748x/adv748x-core.c
> @@ -589,14 +589,13 @@ static int adv748x_parse_csi2_lanes(struct adv748x_state *state,
>  				    unsigned int port,
>  				    struct device_node *ep)
>  {
> -	struct v4l2_fwnode_endpoint vep;
> +	struct v4l2_fwnode_endpoint vep = { .bus_type = V4L2_MBUS_CSI2_DPHY };
>  	unsigned int num_lanes;
>  	int ret;
>  
>  	if (port != ADV748X_PORT_TXA && port != ADV748X_PORT_TXB)
>  		return 0;
>  
> -	vep.bus_type = V4L2_MBUS_CSI2_DPHY;
>  	ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &vep);
>  	if (ret)
>  		return ret;
> -- 
> 2.27.0
> 
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/5] v4l2-fwnode: v4l2_fwnode_endpoint_parse caller must init vep argument

[Niklas Söderlund]

Hi Sakari,

Thanks for your work.

On 2020-09-30 17:48:08 +0300, Sakari Ailus wrote:
> Document that the caller of v4l2_fwnode_endpoint_parse() must init the
> fields of struct v4l2_fwnode_endpoint (vep argument) fields.
> 
> It used to be that the fields were zeroed by v4l2_fwnode_endpoint_parse
> when bus type was set to V4L2_MBUS_UNKNOWN but with commit bb4bba9232fc
> that no longer makes sense.
> 
> Fixes: bb4bba9232fc ("media: v4l2-fwnode: Make bus configuration a struct")
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  include/media/v4l2-fwnode.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index c09074276543..0f9a768b1a8d 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -231,6 +231,8 @@ struct v4l2_fwnode_connector {
>   * 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 caller is required to initialise all fields of @vep.
> + *
>   * The function does not change the V4L2 fwnode endpoint state if it fails.
>   *
>   * NOTE: This function does not parse properties the size of which is variable
> @@ -273,6 +275,8 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);
>   * 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 caller is required to initialise all fields of @vep.
> + *
>   * The function does not change the V4L2 fwnode endpoint state if it fails.
>   *
>   * v4l2_fwnode_endpoint_alloc_parse() has two important differences to
> -- 
> 2.27.0
> 
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 4/5] v4l2-fwnode: Rework v4l2_fwnode_endpoint_parse documentation

[Niklas Söderlund]

Hi Sakari,

Thanks for your patch.

On 2020-09-30 17:48:10 +0300, Sakari Ailus wrote:
> Rework the documentation of v4l2_fwnode_endpoint_parse for better
> readability, usefulness and correctness.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  include/media/v4l2-fwnode.h | 62 ++++++++++++++++++++++++-------------
>  1 file changed, 40 insertions(+), 22 deletions(-)
> 
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index 0f9a768b1a8d..0c28dc11e829 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -219,17 +219,26 @@ struct v4l2_fwnode_connector {
>   * @vep: pointer to the V4L2 fwnode data structure
>   *
>   * This function parses the V4L2 fwnode endpoint specific parameters from the
> - * firmware. The caller is responsible for assigning @vep.bus_type to a valid
> - * media bus type. The caller may also set the default configuration for the
> - * endpoint --- a configuration that shall be in line with the DT binding
> - * documentation. Should a device support multiple bus types, the caller may
> - * call this function once the correct type is found --- with a default
> - * configuration valid for that type.
> - *
> - * 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!
> + * firmware. There are two ways to use this function, either by letting it
> + * obtain the type of the bus (by setting the @vep.bus_type field to
> + * V4L2_MBUS_UNKNOWN) or specifying the bus type explicitly to one of the &enum
> + * v4l2_mbus_type types.
> + *
> + * When @vep.bus_type is V4L2_MBUS_UNKNOWN, the function will use the "bus-type"
> + * property to determine the type when it is available. The caller is
> + * responsible for validating the contents of @vep.bus_type field after the call
> + * returns.
> + *
> + * As a deprecated functionality to support older DT bindings without "bus-type"
> + * property for devices that support multiple types, if the "bus-type" property
> + * does not exist, the function will attempt to guess the type based on the
> + * endpoint properties available. NEVER RELY ON GUESSING THE BUS TYPE IN NEW
> + * DRIVERS OR BINDINGS.
> + *
> + * It is also possible to set @vep.bus_type corresponding to an actual bus. In
> + * this case the function will only attempt to parse properties related to this
> + * bus, and it will return an error if the value of the "bus-type" property
> + * corresponds to a different bus.
>   *
>   * The caller is required to initialise all fields of @vep.
>   *
> @@ -263,17 +272,26 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);
>   * @vep: pointer to the V4L2 fwnode data structure
>   *
>   * This function parses the V4L2 fwnode endpoint specific parameters from the
> - * firmware. The caller is responsible for assigning @vep.bus_type to a valid
> - * media bus type. The caller may also set the default configuration for the
> - * endpoint --- a configuration that shall be in line with the DT binding
> - * documentation. Should a device support multiple bus types, the caller may
> - * call this function once the correct type is found --- with a default
> - * configuration valid for that type.
> - *
> - * 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!
> + * firmware. There are two ways to use this function, either by letting it
> + * obtain the type of the bus (by setting the @vep.bus_type field to
> + * V4L2_MBUS_UNKNOWN) or specifying the bus type explicitly to one of the &enum
> + * v4l2_mbus_type types.
> + *
> + * When @vep.bus_type is V4L2_MBUS_UNKNOWN, the function will use the "bus-type"
> + * property to determine the type when it is available. The caller is
> + * responsible for validating the contents of @vep.bus_type field after the call
> + * returns.
> + *
> + * As a deprecated functionality to support older DT bindings without "bus-type"
> + * property for devices that support multiple types, if the "bus-type" property
> + * does not exist, the function will attempt to guess the type based on the
> + * endpoint properties available. NEVER RELY ON GUESSING THE BUS TYPE IN NEW
> + * DRIVERS OR BINDINGS.
> + *
> + * It is also possible to set @vep.bus_type corresponding to an actual bus. In
> + * this case the function will only attempt to parse properties related to this
> + * bus, and it will return an error if the value of the "bus-type" property
> + * corresponds to a different bus.
>   *
>   * The caller is required to initialise all fields of @vep.
>   *
> -- 
> 2.27.0
> 
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 5/5] v4l2-fwnode: Say it's fine to use v4l2_fwnode_endpoint_parse

[Niklas Söderlund]

Hi Sakari,

Thanks for your work.

On 2020-09-30 17:48:11 +0300, Sakari Ailus wrote:
> Earlier it was expected that there would be more variable size endpoint
> properties and that most if not all drivers would need them. For that
> reason it was expected also that v4l2_fwnode_endpoint_parse would no
> longer be needed.
> 
> What actually happened that not all drivers require "link-frequencies",
> the only variable size media endpoint property without a small upper
> limit. Therefore drivers that do not need that information are fine using
> v4l2_fwnode_endpoint_parse. So don't tell drivers to use
> v4l2_fwnode_endpoint_alloc_parse in all cases.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  include/media/v4l2-fwnode.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index 0c28dc11e829..cbd872e59f8e 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -245,8 +245,8 @@ struct v4l2_fwnode_connector {
>   * The function does not change the V4L2 fwnode endpoint state if it fails.
>   *
>   * NOTE: This function does not parse properties the size of which is variable
> - * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() in
> - * new drivers instead.
> + * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() if
> + * you need properties of variable size.
>   *
>   * Return: %0 on success or a negative error code on failure:
>   *	   %-ENOMEM on memory allocation failure
> -- 
> 2.27.0
> 
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 3/5] v4l2-fwnode: Don't zero parts of struct v4l2_fwnode_endpoint anymore

[Niklas Söderlund]

Hi Sakari,

Thanks for your work.

On 2020-09-30 17:48:09 +0300, Sakari Ailus wrote:
> Don't zero parts of the vep argument to v4l2_fwnode_endpoint_parse()
> anymore as this can no longer be done while still supporting defaults on
> multiple bus types.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index dfc53d11053f..44dd04b05e29 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -416,20 +416,8 @@ static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
>  	enum v4l2_mbus_type mbus_type;
>  	int rval;
>  
> -	if (vep->bus_type == V4L2_MBUS_UNKNOWN) {
> -		/* Zero fields from bus union to until the end */
> -		memset(&vep->bus, 0,
> -		       sizeof(*vep) - offsetof(typeof(*vep), bus));
> -	}
> -
>  	pr_debug("===== begin parsing endpoint %pfw\n", fwnode);
>  
> -	/*
> -	 * Zero the fwnode graph endpoint memory in case we don't end up parsing
> -	 * the endpoint.
> -	 */
> -	memset(&vep->base, 0, sizeof(vep->base));
> -
>  	fwnode_property_read_u32(fwnode, "bus-type", &bus_type);
>  	pr_debug("fwnode video bus type %s (%u), mbus type %s (%u)\n",
>  		 v4l2_fwnode_bus_type_to_string(bus_type), bus_type,
> -- 
> 2.27.0
> 
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/5] v4l2-fwnode: v4l2_fwnode_endpoint_parse caller must init vep argument

[Sakari Ailus]

On Fri, Oct 02, 2020 at 05:53:41PM +0200, Niklas Söderlund wrote:
> Hi Sakari,
> 
> Thanks for your work.
> 
> On 2020-09-30 17:48:08 +0300, Sakari Ailus wrote:
> > Document that the caller of v4l2_fwnode_endpoint_parse() must init the
> > fields of struct v4l2_fwnode_endpoint (vep argument) fields.
> > 
> > It used to be that the fields were zeroed by v4l2_fwnode_endpoint_parse
> > when bus type was set to V4L2_MBUS_UNKNOWN but with commit bb4bba9232fc
> > that no longer makes sense.
> > 
> > Fixes: bb4bba9232fc ("media: v4l2-fwnode: Make bus configuration a struct")
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Thanks for the review!

-- 
Sakari Ailus

Re: [PATCH 1/5] adv748x: Zero entire struct v4l2_fwnode_endpoint

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Wed, Sep 30, 2020 at 05:48:07PM +0300, Sakari Ailus wrote:
> The v4l2_fwnode_parse_endpoint() function can make use of defaults in
> multiple bus types. To use this feature, all callers must zero the rest of
> the fields of this struct, too. All other drivers appear to do that
> already apart from this one.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/i2c/adv748x/adv748x-core.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
> index 1fe7f97c6d52..ae8b7ebf3830 100644
> --- a/drivers/media/i2c/adv748x/adv748x-core.c
> +++ b/drivers/media/i2c/adv748x/adv748x-core.c
> @@ -589,14 +589,13 @@ static int adv748x_parse_csi2_lanes(struct adv748x_state *state,
>  				    unsigned int port,
>  				    struct device_node *ep)
>  {
> -	struct v4l2_fwnode_endpoint vep;
> +	struct v4l2_fwnode_endpoint vep = { .bus_type = V4L2_MBUS_CSI2_DPHY };
>  	unsigned int num_lanes;
>  	int ret;
>  
>  	if (port != ADV748X_PORT_TXA && port != ADV748X_PORT_TXB)
>  		return 0;
>  
> -	vep.bus_type = V4L2_MBUS_CSI2_DPHY;
>  	ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &vep);
>  	if (ret)
>  		return ret;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/5] v4l2-fwnode: v4l2_fwnode_endpoint_parse caller must init vep argument

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Wed, Sep 30, 2020 at 05:48:08PM +0300, Sakari Ailus wrote:
> Document that the caller of v4l2_fwnode_endpoint_parse() must init the
> fields of struct v4l2_fwnode_endpoint (vep argument) fields.
> 
> It used to be that the fields were zeroed by v4l2_fwnode_endpoint_parse
> when bus type was set to V4L2_MBUS_UNKNOWN but with commit bb4bba9232fc
> that no longer makes sense.
> 
> Fixes: bb4bba9232fc ("media: v4l2-fwnode: Make bus configuration a struct")
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  include/media/v4l2-fwnode.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index c09074276543..0f9a768b1a8d 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -231,6 +231,8 @@ struct v4l2_fwnode_connector {
>   * 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 caller is required to initialise all fields of @vep.

Would it make sense to explicitly state that fields should be zeroed if
no specific value is desired ? Maybe

 * The caller is required to initialise all fields of @vep, either with
 * explicitly values, or by zeroing them.

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> + *
>   * The function does not change the V4L2 fwnode endpoint state if it fails.
>   *
>   * NOTE: This function does not parse properties the size of which is variable
> @@ -273,6 +275,8 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);
>   * 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 caller is required to initialise all fields of @vep.
> + *
>   * The function does not change the V4L2 fwnode endpoint state if it fails.
>   *
>   * v4l2_fwnode_endpoint_alloc_parse() has two important differences to

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/5] v4l2-fwnode: Don't zero parts of struct v4l2_fwnode_endpoint anymore

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Wed, Sep 30, 2020 at 05:48:09PM +0300, Sakari Ailus wrote:
> Don't zero parts of the vep argument to v4l2_fwnode_endpoint_parse()
> anymore as this can no longer be done while still supporting defaults on
> multiple bus types.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index dfc53d11053f..44dd04b05e29 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -416,20 +416,8 @@ static int __v4l2_fwnode_endpoint_parse(struct fwnode_handle *fwnode,
>  	enum v4l2_mbus_type mbus_type;
>  	int rval;
>  
> -	if (vep->bus_type == V4L2_MBUS_UNKNOWN) {
> -		/* Zero fields from bus union to until the end */
> -		memset(&vep->bus, 0,
> -		       sizeof(*vep) - offsetof(typeof(*vep), bus));
> -	}
> -
>  	pr_debug("===== begin parsing endpoint %pfw\n", fwnode);
>  
> -	/*
> -	 * Zero the fwnode graph endpoint memory in case we don't end up parsing
> -	 * the endpoint.
> -	 */
> -	memset(&vep->base, 0, sizeof(vep->base));
> -
>  	fwnode_property_read_u32(fwnode, "bus-type", &bus_type);
>  	pr_debug("fwnode video bus type %s (%u), mbus type %s (%u)\n",
>  		 v4l2_fwnode_bus_type_to_string(bus_type), bus_type,

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/5] v4l2-fwnode: Rework v4l2_fwnode_endpoint_parse documentation

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Wed, Sep 30, 2020 at 05:48:10PM +0300, Sakari Ailus wrote:
> Rework the documentation of v4l2_fwnode_endpoint_parse for better
> readability, usefulness and correctness.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  include/media/v4l2-fwnode.h | 62 ++++++++++++++++++++++++-------------
>  1 file changed, 40 insertions(+), 22 deletions(-)
> 
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index 0f9a768b1a8d..0c28dc11e829 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -219,17 +219,26 @@ struct v4l2_fwnode_connector {
>   * @vep: pointer to the V4L2 fwnode data structure
>   *
>   * This function parses the V4L2 fwnode endpoint specific parameters from the
> - * firmware. The caller is responsible for assigning @vep.bus_type to a valid
> - * media bus type. The caller may also set the default configuration for the
> - * endpoint --- a configuration that shall be in line with the DT binding
> - * documentation. Should a device support multiple bus types, the caller may
> - * call this function once the correct type is found --- with a default
> - * configuration valid for that type.
> - *
> - * 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!
> + * firmware. There are two ways to use this function, either by letting it
> + * obtain the type of the bus (by setting the @vep.bus_type field to
> + * V4L2_MBUS_UNKNOWN) or specifying the bus type explicitly to one of the &enum
> + * v4l2_mbus_type types.
> + *
> + * When @vep.bus_type is V4L2_MBUS_UNKNOWN, the function will use the "bus-type"
> + * property to determine the type when it is available. The caller is
> + * responsible for validating the contents of @vep.bus_type field after the call
> + * returns.

As an additional improvement, I wonder if it would make sense to turn
the bus types into a bit flag, to let the caller tell which bus types it
supports. Or maybe that would be overkill ?

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> + *
> + * As a deprecated functionality to support older DT bindings without "bus-type"
> + * property for devices that support multiple types, if the "bus-type" property
> + * does not exist, the function will attempt to guess the type based on the
> + * endpoint properties available. NEVER RELY ON GUESSING THE BUS TYPE IN NEW
> + * DRIVERS OR BINDINGS.
> + *
> + * It is also possible to set @vep.bus_type corresponding to an actual bus. In
> + * this case the function will only attempt to parse properties related to this
> + * bus, and it will return an error if the value of the "bus-type" property
> + * corresponds to a different bus.
>   *
>   * The caller is required to initialise all fields of @vep.
>   *
> @@ -263,17 +272,26 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);
>   * @vep: pointer to the V4L2 fwnode data structure
>   *
>   * This function parses the V4L2 fwnode endpoint specific parameters from the
> - * firmware. The caller is responsible for assigning @vep.bus_type to a valid
> - * media bus type. The caller may also set the default configuration for the
> - * endpoint --- a configuration that shall be in line with the DT binding
> - * documentation. Should a device support multiple bus types, the caller may
> - * call this function once the correct type is found --- with a default
> - * configuration valid for that type.
> - *
> - * 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!
> + * firmware. There are two ways to use this function, either by letting it
> + * obtain the type of the bus (by setting the @vep.bus_type field to
> + * V4L2_MBUS_UNKNOWN) or specifying the bus type explicitly to one of the &enum
> + * v4l2_mbus_type types.
> + *
> + * When @vep.bus_type is V4L2_MBUS_UNKNOWN, the function will use the "bus-type"
> + * property to determine the type when it is available. The caller is
> + * responsible for validating the contents of @vep.bus_type field after the call
> + * returns.
> + *
> + * As a deprecated functionality to support older DT bindings without "bus-type"
> + * property for devices that support multiple types, if the "bus-type" property
> + * does not exist, the function will attempt to guess the type based on the
> + * endpoint properties available. NEVER RELY ON GUESSING THE BUS TYPE IN NEW
> + * DRIVERS OR BINDINGS.
> + *
> + * It is also possible to set @vep.bus_type corresponding to an actual bus. In
> + * this case the function will only attempt to parse properties related to this
> + * bus, and it will return an error if the value of the "bus-type" property
> + * corresponds to a different bus.
>   *
>   * The caller is required to initialise all fields of @vep.
>   *

-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/5] v4l2-fwnode: Say it's fine to use v4l2_fwnode_endpoint_parse

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Wed, Sep 30, 2020 at 05:48:11PM +0300, Sakari Ailus wrote:
> Earlier it was expected that there would be more variable size endpoint
> properties and that most if not all drivers would need them. For that
> reason it was expected also that v4l2_fwnode_endpoint_parse would no
> longer be needed.
> 
> What actually happened that not all drivers require "link-frequencies",
> the only variable size media endpoint property without a small upper
> limit. Therefore drivers that do not need that information are fine using
> v4l2_fwnode_endpoint_parse. So don't tell drivers to use
> v4l2_fwnode_endpoint_alloc_parse in all cases.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  include/media/v4l2-fwnode.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index 0c28dc11e829..cbd872e59f8e 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -245,8 +245,8 @@ struct v4l2_fwnode_connector {
>   * The function does not change the V4L2 fwnode endpoint state if it fails.
>   *
>   * NOTE: This function does not parse properties the size of which is variable
> - * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() in
> - * new drivers instead.
> + * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() if
> + * you need properties of variable size.

Shouldn't we explicitly mention link-frequencies ? "properties the size
of which is variable without a low fixed limit" is not very clear for a
casual reader.

>   *
>   * Return: %0 on success or a negative error code on failure:
>   *	   %-ENOMEM on memory allocation failure

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/5] v4l2-fwnode: Rework v4l2_fwnode_endpoint_parse documentation

[Sakari Ailus]

Hi Laurent,

On Sun, Oct 04, 2020 at 11:40:13AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Wed, Sep 30, 2020 at 05:48:10PM +0300, Sakari Ailus wrote:
> > Rework the documentation of v4l2_fwnode_endpoint_parse for better
> > readability, usefulness and correctness.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  include/media/v4l2-fwnode.h | 62 ++++++++++++++++++++++++-------------
> >  1 file changed, 40 insertions(+), 22 deletions(-)
> > 
> > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> > index 0f9a768b1a8d..0c28dc11e829 100644
> > --- a/include/media/v4l2-fwnode.h
> > +++ b/include/media/v4l2-fwnode.h
> > @@ -219,17 +219,26 @@ struct v4l2_fwnode_connector {
> >   * @vep: pointer to the V4L2 fwnode data structure
> >   *
> >   * This function parses the V4L2 fwnode endpoint specific parameters from the
> > - * firmware. The caller is responsible for assigning @vep.bus_type to a valid
> > - * media bus type. The caller may also set the default configuration for the
> > - * endpoint --- a configuration that shall be in line with the DT binding
> > - * documentation. Should a device support multiple bus types, the caller may
> > - * call this function once the correct type is found --- with a default
> > - * configuration valid for that type.
> > - *
> > - * 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!
> > + * firmware. There are two ways to use this function, either by letting it
> > + * obtain the type of the bus (by setting the @vep.bus_type field to
> > + * V4L2_MBUS_UNKNOWN) or specifying the bus type explicitly to one of the &enum
> > + * v4l2_mbus_type types.
> > + *
> > + * When @vep.bus_type is V4L2_MBUS_UNKNOWN, the function will use the "bus-type"
> > + * property to determine the type when it is available. The caller is
> > + * responsible for validating the contents of @vep.bus_type field after the call
> > + * returns.
> 
> As an additional improvement, I wonder if it would make sense to turn
> the bus types into a bit flag, to let the caller tell which bus types it
> supports. Or maybe that would be overkill ?
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks for the review!

I wonder how useful that would be as drivers typically do bus type specific
checks right after this function returns. Although I wouldn't object a
patch doing that if it's seen useful somewhere.

-- 
Sakari Ailus

Re: [PATCH 5/5] v4l2-fwnode: Say it's fine to use v4l2_fwnode_endpoint_parse

[Sakari Ailus]

On Sun, Oct 04, 2020 at 11:41:48AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Wed, Sep 30, 2020 at 05:48:11PM +0300, Sakari Ailus wrote:
> > Earlier it was expected that there would be more variable size endpoint
> > properties and that most if not all drivers would need them. For that
> > reason it was expected also that v4l2_fwnode_endpoint_parse would no
> > longer be needed.
> > 
> > What actually happened that not all drivers require "link-frequencies",
> > the only variable size media endpoint property without a small upper
> > limit. Therefore drivers that do not need that information are fine using
> > v4l2_fwnode_endpoint_parse. So don't tell drivers to use
> > v4l2_fwnode_endpoint_alloc_parse in all cases.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  include/media/v4l2-fwnode.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> > index 0c28dc11e829..cbd872e59f8e 100644
> > --- a/include/media/v4l2-fwnode.h
> > +++ b/include/media/v4l2-fwnode.h
> > @@ -245,8 +245,8 @@ struct v4l2_fwnode_connector {
> >   * The function does not change the V4L2 fwnode endpoint state if it fails.
> >   *
> >   * NOTE: This function does not parse properties the size of which is variable
> > - * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() in
> > - * new drivers instead.
> > + * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() if
> > + * you need properties of variable size.
> 
> Shouldn't we explicitly mention link-frequencies ? "properties the size
> of which is variable without a low fixed limit" is not very clear for a
> casual reader.

I can add that, sure.

-- 
Sakari Ailus

Re: [PATCH 5/5] v4l2-fwnode: Say it's fine to use v4l2_fwnode_endpoint_parse

[Sakari Ailus]

On Sun, Oct 04, 2020 at 11:41:48AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Wed, Sep 30, 2020 at 05:48:11PM +0300, Sakari Ailus wrote:
> > Earlier it was expected that there would be more variable size endpoint
> > properties and that most if not all drivers would need them. For that
> > reason it was expected also that v4l2_fwnode_endpoint_parse would no
> > longer be needed.
> > 
> > What actually happened that not all drivers require "link-frequencies",
> > the only variable size media endpoint property without a small upper
> > limit. Therefore drivers that do not need that information are fine using
> > v4l2_fwnode_endpoint_parse. So don't tell drivers to use
> > v4l2_fwnode_endpoint_alloc_parse in all cases.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  include/media/v4l2-fwnode.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> > index 0c28dc11e829..cbd872e59f8e 100644
> > --- a/include/media/v4l2-fwnode.h
> > +++ b/include/media/v4l2-fwnode.h
> > @@ -245,8 +245,8 @@ struct v4l2_fwnode_connector {
> >   * The function does not change the V4L2 fwnode endpoint state if it fails.
> >   *
> >   * NOTE: This function does not parse properties the size of which is variable
> > - * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() in
> > - * new drivers instead.
> > + * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() if
> > + * you need properties of variable size.
> 
> Shouldn't we explicitly mention link-frequencies ? "properties the size
> of which is variable without a low fixed limit" is not very clear for a
> casual reader.

I'll use this in v2:

+ * NOTE: This function does not parse "link-frequencies" property as its size is
+ * not known in advance. Please use v4l2_fwnode_endpoint_alloc_parse() if you
+ * need properties of variable size.

-- 
Sakari Ailus