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

Andy Shevchenko
2023-02-08T15:51:09Z

On Wed, Feb 8, 2023 at 5:41 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> 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 :-)

It depends on which side you look at it. I admit I haven't dug into
the code to see if endianess can be an issue there, but the code
itself is not written well, esp. when one offers it as an example. So
definitely there is a fix on the upper layer.

--
With Best Regards,
Andy Shevchenko