Re: [PATCH 28/57] media: Add ovxxxx_16bit_addr_reg_helpers.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux