linux-kernel - Re: [PATCH v1 1/2] media: i2c: et8ek8: et8ek8_driver: add support for crc configuration via device tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aRGe1NkDHYtDOfnw@kekkonen.localdomain>
Date: Mon, 10 Nov 2025 10:14:12 +0200
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Alex Tran <alex.t.tran@...il.com>
Cc: Pavel Machek <pavel@...nel.org>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/2] media: i2c: et8ek8: et8ek8_driver: add support
 for crc configuration via device tree

Hi Alex,

Thanks for the patch.

On Sat, Nov 08, 2025 at 03:29:23PM -0800, Alex Tran wrote:
> Retrieve the configuration for CRC from the device tree instead
> using the hard coded USE_CRC directive. 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.

Is there a need for making this configurable? I have to say I can't recall
the specifics of CCP2 but presumably the receiver side would need to be
aligned with this as well, requiring such configuration on both sides.

If you want to pursue this further, please also cc me to the DT binding
patch.

> 
> Signed-off-by: Alex Tran <alex.t.tran@...il.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);
> +done:
> +	v4l2_fwnode_endpoint_free(&bus_cfg);
> +	fwnode_handle_put(ep);
> +	return ret;
> +}
> +
>  static int et8ek8_probe(struct i2c_client *client)
>  {
>  	struct et8ek8_sensor *sensor;
> @@ -1406,6 +1430,11 @@ static int et8ek8_probe(struct i2c_client *client)
>  	if (!sensor)
>  		return -ENOMEM;
>  
> +	sensor->use_crc = 1;
> +	ret = et8ek8_parse_fwnode(dev, sensor);
> +	if (ret)
> +		dev_warn(dev, "parsing endpoint failed\n");
> +
>  	sensor->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
>  	if (IS_ERR(sensor->reset)) {
>  		dev_dbg(&client->dev, "could not request reset gpio\n");

-- 
Kind regards,

Sakari Ailus