On Wed, Nov 6, 2024 at 2:46 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote:
>
> On 06/11/2024 02:54, anishkmr@xxxxxxxxxx wrote:
> > From: Anish Kumar <anishkmr@xxxxxxxxxx>
> >
> > LED Driver for O2 Micro LED IC
> >
> > reviewed-by: Anish Kumar <yesanishhere@xxxxxxxxx>
> > Signed-off-by: Karthik Poduval <kpoduval@xxxxxxxxxx>
> > Signed-off-by: Yue Hu <yhuamzn@xxxxxxxxxx>
> > ---
> > .../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
>
