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

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

 



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.

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.

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.

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.

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.

Please kindly resend and include all necessary To/Cc entries.



...


> +
> +static struct i2c_driver ozl003_driver = {
> +	.driver = {
> +		.owner = THIS_MODULE,

Please do no send downstream, junk code. This was fixed years ago.

> +		.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.

Best regards,
Krzysztof





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux