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

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

 



Hi Hans,

On Fri, Feb 10, 2023 at 01:47:49PM +0100, Hans de Goede wrote:
> > And if someone dislikes having to pass NULL for the last argument, we
> > could use some macro magic to accept both the 3 arguments and 4
> > arguments variants.
> > 
> > int __cci_write3(struct cci *cci, u32 reg, u32 val);
> > int __cci_write4(struct cci *cci, u32 reg, u32 val, int *err);
> > 
> > #define __cci_write(_1, _2, _3, _4, NAME, ...) NAME
> > #define cci_write(...) __cci_write(__VA_ARGS__, __cci_write4, __cci_write3)(__VA_ARGS__)
> 
> TBH this just feels like code obfuscation to me and it is also going
> to write havoc with various smarted code-editors / IDEs which give
> proptype info to the user while typing the function name.
> 
> Having the extra ", NULL" there in calls which don't use / need
> the *err thingie really is not a big deal IMHO.

It's still an eyesore if the driver doesn't use that pattern of register
access error handling. I also prioritise source code itself rather than try
to make it fit for a particular editor (which is neither Emacs nor Vim I
suppose?).

My preference is to provide an interface that best suits a driver's needs,
whatever that is. There are just a couple of common patterns and the one above
is one of the rare ones.

-- 
Regards,

Sakari Ailus




[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