Re: [PATCH 5/5] media/i2c: max96717: allow user to override operation mode from DT

[Julien Massot <julien.massot@collabora.com>]

Hi Laurentiu,

On Fri, 2025-02-07 at 13:29 +0200, Laurentiu Palcu wrote:
> There are situations when the CFG pins set the chip up for a certain
> mode of operation (ie: pixel mode or tunneling mode), because the HW
> designers decided this way, and we, the users, want to change that. For
> that, add an optional DT property that would allow toggling the
> operation mode from the configured one to the other one.
> 
> The driver still only supports tunneling mode, that didn't change.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> ---
>  drivers/media/i2c/max96717.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/i2c/max96717.c b/drivers/media/i2c/max96717.c
> index 47a3be195a971..a591ca5d5f44f 100644
> --- a/drivers/media/i2c/max96717.c
> +++ b/drivers/media/i2c/max96717.c


enum gmsl2_mode {
  GMSL2_PIXEL_MODE,
  GMSL2_MODE_TUNNEL,
};

> @@ -161,6 +161,7 @@ struct max96717_priv {
>  	struct clk_hw                     clk_hw;
>  	struct gpio_chip                  gpio_chip;
>  	enum max96717_vpg_mode            pattern;
> +	bool				  mode_override;
	enum gmsl2_mode                   mode;
I would prefer to set the mode in an explicit way instead of toggling
the bit in the register.

>  	struct max96717_fsync_desc	  fsync;
>  };
>  
> @@ -1066,6 +1067,14 @@ static int max96717_hw_init(struct max96717_priv *priv)
>  		return dev_err_probe(dev, ret,
>  				     "Fail to read mipi rx extension");
>  
> +	if (priv->mode_override) {
        if (priv->mode_override && priv->mode == GMSL2_MODE_TUNNEL) {
> +		
> +
> +		ret = cci_write(priv->regmap, MAX96717_MIPI_RX_EXT11, val, NULL);
		ret = cci_update_bits(priv->regmap, MAX96717_MIPI_RX_EXT11, MAX96717_TUN_MODE,
					MAX96717_TUN_MODE, NULL);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Unable to update operation mode\n");
> +	}
> +
In case we are overwriting the mode to tunnel mode then no need to read the EXT11 register.

>  	if (!(val & MAX96717_TUN_MODE))
>  		return dev_err_probe(dev, -EOPNOTSUPP,
>  				     "Only supporting tunnel mode");

In fact the driver can works in pixel mode, but since we don't set the "stream id" the
deserializer have to configured with the right one :)

> @@ -1101,6 +1110,9 @@ static int max96717_parse_dt(struct max96717_priv *priv)
>  
>  	priv->mipi_csi2 = vep.bus.mipi_csi2;
>  
> +	if (fwnode_property_present(dev_fwnode(dev), "maxim,cfg-mode-override"))
> +		priv->mode_override = true;
> +
	source_fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
							MAX96717_PAD_SOURCE, 0, 0);
	if (fwnode_property_present(source_fwnode, "maxim,tunnel-mode")) {
		priv->mode_override = true;
		priv->mode = GMSL2_MODE_TUNNEL;
	}
So we can parse the tunnel property from the GMSL port.

>  	priv->fsync.pin = -1;
>  	count = fwnode_property_present(dev_fwnode(dev), "maxim,fsync-config");
>  	if (count > 0) {

Best Regards,

-- 
Julien