Re: [PATCH v5] Driver for ON Semi AR0521 camera sensor

[Sakari Ailus]

Hi Randy, others,

On Sat, Oct 09, 2021 at 01:18:50PM -0700, Randy Dunlap wrote:
> On 10/9/21 2:07 AM, Jacopo Mondi wrote:
> > Hi Krzysztof,
> > 
> >     I've been testing this driver in the last few days, thanks for your
> > effort in upstreaming it!
> > 
> > I'll separately comment on what I had to change to have it working for
> > my use case, but let me continue the discussion from where it was left
> > pending here to add my 2 cents.
> > 
> > On Thu, Oct 07, 2021 at 11:11:09AM +0200, Krzysztof Hałasa wrote:
> > > Hi Sakari,
> > > 
> > > Thanks for your input.
> > > 
> > > > Where's the corresponding DT binding patch? Ideally it would be part of the
> > > > same set.
> > > 
> > > Well I've sent it a moment before this one. Will make them a set next
> > > time.
> > > 
> > > > > +#define AR0521_WIDTH_BLANKING_MIN     572u
> > > > > +#define AR0521_HEIGHT_BLANKING_MIN     28u // must be even
> > > > 
> > > > Please use /* */ for comments. The SPDX tag is an exception.
> > > 
> > > As far as I know, this is no longer the case, the C99 comments are now
> > > permitted and maybe even encouraged. Or was I dreaming?
> > > 
> > > checkpatch doesn't protest either.
> > 
> > To my understanding the C99 standard added support for the //
> > commenting style and tollerate them, but they're still from C++ and I
> > see very few places where they're used in the kernel, and per as far I
> > know they're still not allowed by the coding style
> > https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting
> 
> http://lkml.iu.edu/hypermail/linux/kernel/1607.1/00627.html
> 
> Maybe we should update coding-style then.

That's not really a statement for C++ (//) comments as it's against other
/* ... */ multi-line comment styles some of which are used by the
networking stack.

I haven't read all the discussion but I see coding-style.rst still
documents a different multi-line comment style for the network stack.

What comes to // comments, almost all the cases that exist in the kernel
currently outside SPDX tags are either adjacent copyright notices or the
AMD DRM driver. Based on this, I'd rather not adopt that style in a sensor
driver.

-- 
Regards,

Sakari Ailus