Hi Andy, On 6/14/23 22:13, Andy Shevchenko wrote: > On Wed, Jun 14, 2023 at 10:24 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >> >> Use the new comon CCI register access helpers to replace the private >> register access helpers in the ov5693 driver. > > ... > >> struct ov5693_device { >> struct i2c_client *client; >> struct device *dev; >> + struct regmap *regmap; > > I forgot what the conclusion was regarding this. Now we have a client > (which embeds the struct device), struct device pointer (is it the > same device?), and now regmap, associated with (yet another?) struct > device. All 3 device pointers will point to the same device. I think that having both a struct device *dev and a struct regmap *regmap is fine even though the dev can also be gotten from the regmap with regmap_get_device() note that regmap_get_device() is not an inline, so it comes with some execution cost. And when used to set a global dev helper var in functions the compiler must always call it even if the helper var is only used to log errors. But I agree that the pre-existing situation of having both a client and a dev pointer is unnecessary. And I just checked and after these changes the client pointer is unused, so I'll drop that for the next version. Regards, Hans > >> } ctrls; >> }; > > > > -- > With Best Regards, > Andy Shevchenko >