Hi Andy, On Wed, Feb 08, 2023 at 07:31:43PM +0200, Andy Shevchenko wrote: > On Wed, Feb 8, 2023 at 6:04 PM Laurent Pinchart wrote: > > On Wed, Feb 08, 2023 at 05:50:26PM +0200, Andy Shevchenko wrote: > > > On Wed, Feb 8, 2023 at 5:41 PM Laurent Pinchart 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. > > > > Did I miss something ? Your two patches replace a tiny amount of code > > with helper functions that don't change any behaviour. It's nicer with > > those helpers, no question about that, but "not written well" is a bit > > of a stretch and feels quite insulting. > > Sorry for your feelings, what I meant is the pure educational purposes > of the example. When one takes the mentioned driver as an example and > uses the code in a slightly different environment the endianess issue > may become a real one. That's why we have helpers in kernel to improve > robustness against blind copy'n'paste approach. It does not mean your > code is broken per se. Reference code should be pristine as much as possible, no disagreement there. That's why I think your two patches are good :-) > > Feel free to submit patches that > > add new "well-written" helpers. -- Regards, Laurent Pinchart
