On Wed, Feb 08, 2023 at 01:27:37PM +0200, Andy Shevchenko wrote: > On Wed, Feb 8, 2023 at 11:52 AM Laurent Pinchart wrote: > > On Mon, Jan 23, 2023 at 01:51:36PM +0100, Hans de Goede wrote: > > ... > > > > Add a new ovxxxx_16bit_addr_reg_helpers.h header file with static inline > > > versions of these register access helpers, so that this code duplication > > > can be removed. > > > > Any reason to hand-roll those instead of using regmap ? Also, may I > > suggest to have a look at drivers/media/i2c/imx290.c > > While this is a bit error prone example, a patch is on its way, ... The two cleanups are nice, but they're cleanup, not error fixes :-) > > for an example of > > how registers of different sizes can be handled in a less error-prone > > way, using single read/write functions that adapt to the size > > automatically ? > > ...with regmap fields I believe you can avoid even that and use proper > regmap accessors directly. I haven't looked at regmap fields so I don't know if they're the right tool for the job. If we can use the regmap API as-is without any wrapper, even better. Otherwise, new regmap helpers and/or I2C helpers may also be an option. This is a very common use case, not limited to OV camera sensors, or even camera sensors in general. -- Regards, Laurent Pinchart