Re: [PATCH v2 1/4] media: i2c: et8ek8: et8ek8_driver: add support for crc configuration via device tree

[Krzysztof Kozlowski <krzk@kernel.org>]

On Sun, Dec 14, 2025 at 11:58:32PM -0800, Alex Tran wrote:
> Retrieve the configuration for CRC from the device tree instead
> using the hard coded USE_CRC macro. If there is an issue
> retrieving the endpoint node or the CRC property then the default
> of 1 is used and the driver does not fail to maintain backward
> compatibility.
> 
> Signed-off-by: Alex Tran <alex.t.tran@gmail.com>
> ---
>  drivers/media/i2c/et8ek8/et8ek8_driver.c | 49 +++++++++++++++++++-----
>  1 file changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> index 2cb7b7187..4ef92359c 100644
> --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
> +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> @@ -29,6 +29,7 @@
>  #include <media/media-entity.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
>  #include <media/v4l2-subdev.h>
>  
>  #include "et8ek8_reg.h"
> @@ -45,6 +46,7 @@ struct et8ek8_sensor {
>  	struct regulator *vana;
>  	struct clk *ext_clk;
>  	u32 xclk_freq;
> +	u32 use_crc;
>  
>  	u16 version;
>  
> @@ -130,8 +132,6 @@ static struct et8ek8_gain {
>  
>  #define ET8EK8_I2C_DELAY	3	/* msec delay b/w accesses */
>  
> -#define USE_CRC			1
> -
>  /*
>   * Register access helpers
>   *
> @@ -844,20 +844,16 @@ static int et8ek8_power_on(struct et8ek8_sensor *sensor)
>  	if (rval)
>  		goto out;
>  
> -#ifdef USE_CRC
>  	rval = et8ek8_i2c_read_reg(client, ET8EK8_REG_8BIT, 0x1263, &val);
>  	if (rval)
>  		goto out;
> -#if USE_CRC /* TODO get crc setting from DT */
> -	val |= BIT(4);
> -#else
> -	val &= ~BIT(4);
> -#endif
> +	if (sensor->use_crc)
> +		val |= BIT(4);
> +	else
> +		val &= ~BIT(4);
>  	rval = et8ek8_i2c_write_reg(client, ET8EK8_REG_8BIT, 0x1263, val);
>  	if (rval)
>  		goto out;
> -#endif
> -
>  out:
>  	if (rval)
>  		et8ek8_power_off(sensor);
> @@ -1396,6 +1392,34 @@ static int __maybe_unused et8ek8_resume(struct device *dev)
>  	return __et8ek8_set_power(sensor, true);
>  }
>  
> +static int et8ek8_parse_fwnode(struct device *dev, struct et8ek8_sensor *sensor)
> +{
> +	struct v4l2_fwnode_endpoint bus_cfg = {
> +		.bus_type = V4L2_MBUS_CCP2,
> +	};
> +	struct fwnode_handle *ep;
> +	int ret;
> +
> +	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0,
> +					     FWNODE_GRAPH_ENDPOINT_NEXT);
> +	if (!ep) {
> +		dev_warn(dev, "could not get endpoint node\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
> +	if (ret) {
> +		dev_warn(dev, "parsing endpoint node failed\n");
> +		goto done;
> +	}
> +
> +	fwnode_property_read_u32(ep, "crc", &sensor->use_crc);

Undocumented ABI. In case you document it in other cases, it is really
unclear, so be sure you read submitting patches in DT.

Best regards,
Krzysztof