linux-kernel - Re: [PATCH] leds: driver for O2 Micro LED IC

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CABCoZhCxHLbi4KcZUZPTHyeKxen2erB4kHGxNn073P2xq3z1Fw@mail.gmail.com>
Date: Wed, 6 Nov 2024 07:48:42 -0800
From: anish kumar <yesanishhere@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: anishkmr@...zon.com, pavel@....cz, dmurphy@...com, 
	linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Karthik Poduval <kpoduval@...126.com>, Yue Hu <yhuamzn@...zon.com>
Subject: Re: [PATCH] leds: driver for O2 Micro LED IC

On Wed, Nov 6, 2024 at 2:46 AM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On 06/11/2024 02:54, anishkmr@...zon.com wrote:
> > From: Anish Kumar <anishkmr@...zon.com>
> >
> > LED Driver for O2 Micro LED IC
> >
> > reviewed-by: Anish Kumar <yesanishhere@...il.com>
> > Signed-off-by: Karthik Poduval <kpoduval@...126.com>
> > Signed-off-by: Yue Hu <yhuamzn@...zon.com>
> > ---
> >  .../devicetree/bindings/leds/leds-ozl003.txt  |  23 ++
>
> 1. Please run scripts/checkpatch.pl and fix reported warnings. Then
> please run `scripts/checkpatch.pl --strict` and (probably) fix more
> warnings. Some warnings can be ignored, especially from --strict run,
> but the code here looks like it needs a fix. Feel free to get in touch
> if the warning is not clear.

It was run on the code but not on this text file. Missed that.
>
> 2. No bindings in TXT. They must com ine DT schema. It is no 2017
> anymore. Please reach to your colleagues in Amazon for some internal
> guidance on upstreaming. Such big companies should perform basic
> internal review instead of asking community to explain that basic stuff.

Will convert this to YAML and send.
>
> 3. Please use scripts/get_maintainers.pl to get a list of necessary
> people and lists to CC. It might happen, that command when run on an
> older kernel, gives you outdated entries. Therefore please be sure you
> base your patches on recent Linux kernel.

We did but our mistake is we cloned the wrong git, we cloned
git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git

>
> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
> people, so fix your workflow. Tools might also fail if you work on some
> ancient tree (don't, instead use mainline) or work on fork of kernel
> (don't, instead use mainline). Just use b4 and everything should be
> fine, although remember about `b4 prep --auto-to-cc` if you added new
> patches to the patchset.

./get_maintainers returned wrong values because of wrong tree.

>
> You missed at least devicetree list (maybe more), so this won't be
> tested by automated tooling. Performing review on untested code might be
> a waste of time.

will add that.
>
> Please kindly resend and include all necessary To/Cc entries.

Will do. Thanks for review.
>
>
>
> ...
>
>
> > +
> > +static struct i2c_driver ozl003_driver = {
> > +     .driver = {
> > +             .owner = THIS_MODULE,
>
> Please do no send downstream, junk code. This was fixed years ago.

ok.
>
> > +             .name = "ozl003",
> > +             .of_match_table = ozl003_match_table,
> > +     },
> > +     .id_table = ozl003_id,
> > +     .probe = ozl003_probe,
> > +     .remove = ozl003_remove,
> > +};
> > +
>
> ...
>
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:ozl003-led");
>
> Drop as well. Useless or incorrect.

ok.
>
> Best regards,
> Krzysztof
>