Re: [PATCH RESEND v8 17/21] media: i2c: maxim-serdes: add MAX9296A driver

On Mon, Jan 26, 2026 at 11:55:47AM +0200, Ceclan Dumitru wrote:

On 1/20/26 3:34 PM, Sakari Ailus wrote:

quoted

Hi Dumitru,

On Mon, Dec 08, 2025 at 04:13:09PM +0200, Dumitru Ceclan via B4 Relay wrote:

quoted

+	*ops = max9296a_common_ops;
+
+	ops->versions = priv->info->ops->versions;
+	ops->modes = priv->info->ops->modes;
+	ops->needs_single_link_version = priv->info->ops->needs_single_link_version;
+	ops->needs_unique_stream_id = priv->info->ops->needs_unique_stream_id;
+	ops->fix_tx_ids = priv->info->ops->fix_tx_ids;
+	ops->num_phys = priv->info->ops->num_phys;
+	ops->num_pipes = priv->info->ops->num_pipes;
+	ops->num_links = priv->info->ops->num_links;
+	ops->phys_configs = priv->info->ops->phys_configs;
+	ops->set_pipe_enable = priv->info->ops->set_pipe_enable;
+	ops->set_pipe_stream_id = priv->info->ops->set_pipe_stream_id;
+	ops->set_pipe_tunnel_phy = priv->info->ops->set_pipe_tunnel_phy;
+	ops->set_pipe_tunnel_enable = priv->info->ops->set_pipe_tunnel_enable;
+	ops->use_atr = priv->info->ops->use_atr;
+	ops->tpg_mode = priv->info->ops->tpg_mode;

What's the reason for doing these assignments and a copy of the memory? Why
not to just keep a pointer to the struct memory instead? I think there's
another case of the same.

Would this be alright:
#define MAX9296A_COMMON_OPS					\

	.num_remaps_per_pipe = 16,				\

	.tpg_entries = { ... },					\

	.init = max9296a_init,					\

	.set_enable = max9296a_set_enable,			\


static const struct max_des_ops max9296a_ops = {

	MAX9296A_COMMON_OPS,

	.versions = BIT(MAX_SERDES_GMSL_2_3GBPS) |

		    BIT(MAX_SERDES_GMSL_2_6GBPS),
	.modes = BIT(MAX_SERDES_GMSL_PIXEL_MODE),
	/* ... */

	};



static int max9296a_probe(struct i2c_client *client)

{

	/* ... */

	priv->des.ops = priv->info->ops;

	/* ... */

}

That's still a copy. Why is a copy needed, why can't you write

 	priv->des.ops = &priv->info->ops;

or event replace priv->des.ops with priv->info->ops through the code ?
Is there anything in the ops structure that needs to be modified at
runtime ?

-- 
Regards,

Laurent Pinchart