Hi Andy, On 6/14/23 22:15, 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 ov2680 driver. >> >> While at it also switch to using the same register address defines >> as the standard drivers/media/i2c/ov2680.c driver to make merging >> the 2 drivers simpler. > > ... > >> @@ -121,7 +59,8 @@ struct ov2680_dev { >> struct media_pad pad; >> /* Protect against concurrent changes to controls */ >> struct mutex lock; >> - struct i2c_client *client; >> + struct device *dev; >> + struct regmap *regmap; > > Similar question as per patch 2. Do we need both of them? You are right that having both is not strictly necessary, but the entire atomisp-ov2680.c file is going away as soon as my main ov2680.c driver changes series is merged. The only reason to upstream this patch is because much of the work landing in the main ov2680.c is copy -pasted from the state of atomisp-ov2680.c *after this patch* , so having this in git history before deleting atomisp-ov2680.c is helpful in case someone ever finds the need to compare the code. Since the next patch for atomisp-ov2680.c after this one is going to be deleting the entire file I really don't feel like spending time on fixing this minor review remark, I hope you understand. Regards, Hans >> struct gpio_desc *powerdown; >> struct fwnode_handle *ep_fwnode; >> bool is_streaming; > >> } ctrls; >> }; >