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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 9 Oct 2021 23:34:37 +0300
From:   Sakari Ailus <sakari.ailus@....fi>
To:     Randy Dunlap <rdunlap@...radead.org>
Cc:     Jacopo Mondi <jacopo@...ndi.org>,
        Krzysztof Hałasa <khalasa@...p.pl>,
        "joe@...ches.com" <joe@...ches.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH v5] Driver for ON Semi AR0521 camera sensor

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