linux-kernel - Re: [PATCH v3 3/3] leds: add new LED driver for TI LP5812

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: <2507f003-c4f1-44be-93cd-176697f0bc8c@kernel.org>
Date: Tue, 18 Mar 2025 19:54:15 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Nam Tran <trannamatk@...il.com>, krzk+dt@...nel.org
Cc: pavel@...nel.org, lee@...nel.org, robh@...nel.org, conor+dt@...nel.org,
 devicetree@...r.kernel.org, linux-leds@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] leds: add new LED driver for TI LP5812

On 18/03/2025 14:35, Nam Tran wrote:
> From: Nam Tran <trannamatk@...il.com>
> To: Krzysztof Kozlowski <krzk+dt@...nel.org>
> Cc: Rob Herring <robh@...nel.org>, Conor Dooley <conor+dt@...nel.org> Pavel Machek <pavel@...nel.org>, Lee Jones <lee@...nel.org>, linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
> 
> I sincerely apologize for not addressing all of your previous comments earlier. That was not my intention, and I truly appreciate the time and effort you have put into reviewing my patch. Below, I would like to properly address your concerns.
> 
> On Fri, Mar 07, 2025 at 12:21:26AM +0700, Nam Tran wrote:
>> The chip can drive LED matrix 4x3.
>> This driver enables LED control via I2C.
> 
> You still did not respond to comments from v1. I don't see it being addressed.
> 
> Nam: I am sorry. This is my mistake. I think that I just need to update source code based on your comments and submit a new patch. This is the first time I try to update a new thing to the Linux Kernel. I will give answer inline your message for tracing easily.
> 
> In previous comment you have a question
>  "Why none of the 10 existing drivers fit for your device?"
> 
> Nam: I have carefully reviewed the existing LED drivers in the kernel, but none of them fully support the advanced capabilities of the LP5812. Unlike basic LED controllers, the LP5812 has difference features and register
> For more detail, please refer to this documentation https://www.ti.com/lit/ds/symlink/lp5812.pdf?ts=1741765622088&ref_url=https%253A%252F%252Fwww.ti.com%252Fproduct%252FLP5812

So you read my question and decided to not give an answer. Great.

The burden of proving this is on you. Do not expect me to read this and
10 other datasheets for existing LP devices. Maybe they fit, maybe they
don't but based on style of this submission and style of the code I feel
you just want to push your patches instead of integrate.

That's not how it works.



> 
>>
>> The driver is implemented in two parts:
>> - Core driver logic in leds-lp5812.c
>> - Common support functions in leds-lp5812-common.c
> 
> Why do you make two modules? This looks really unneccessary. Just compile them into one module. No, use proper kerneldoc Missing kerneldoc. Every exported symbol must have kerneldoc. Why this is not static?
> 
> Nam: I will merge source code into a file only. Therefore, I don’t need to export symbols here.

I cannot parse this. Use standard email quotes and replies. Like every
email client since 30 years.

Best regards,
Krzysztof