Re: [PATCH v2 2/2] hwmon: ltc4282: add support for the LTC4282 chip

[Krzysztof Kozlowski <krzk@kernel.org>]

On 29/11/2023 09:35, Nuno Sá wrote:
> On Tue, 2023-11-28 at 10:03 -0800, Guenter Roeck wrote:
>> On 11/28/23 08:50, Krzysztof Kozlowski wrote:
>>> On 27/11/2023 17:03, Andy Shevchenko wrote:
>>>> On Mon, Nov 27, 2023 at 09:12:14AM +0100, Krzysztof Kozlowski wrote:
>>>>> On 27/11/2023 09:10, Krzysztof Kozlowski wrote:
>>>>
>>>> ...
>>>>
>>>>> Wait, this was not even unusual test, just standard compile, which means
>>>>> you did not do basic tests on your end. You must build your new driver
>>>>> with W=1, smatch, sparse and coccinelle before sending upstream.
>>>>
>>>> Well, sparse is lagging in development, for the last year it's at least two
>>>> times it broke kernel builds because of being not ready for the new stuff used
>>>> in the kernel. Do we have anybody to sync this? I don't think so, hence
>>>> requiring this from developer is doubtful. Otherwise I agree, that basic
>>>> compilation with GCC/LLVM must be done.
>>>
>>> Sparse still detects several issues and handles lock annotations, so it
>>> is useful. But if you disagree with that part, I still insist on Smatch
>>> (which is actively developed and works great) and Coccinelle (also
>>> actively developed).
>>>
>>
>> Quite frankly, for my part I would be more than happy if people would read
>> and follow Documentation/hwmon/submitting-patches.rst. Most submitters don't
>> bother. That doesn't even mention building with W=1 (the much more optimistic
>> me who wrote that document several years ago thought that would be obvious),
>> much less running any source code analysis tools . Feel free to submit a patch
>> to strengthen the wording there. If you do that, it would have to be more explicit
>> then "run smatch" or "run coccinelle" because hardly anyone would know how
>> to do that.
>>
> 
> IMO, submitting patches to linux is already not the most straightforward thing in the

True...

> world. If we are now going to ask to run smatch, cocci, sparse and so on, we will
> scare even more developers from the community... I mean, the bots are also in place

This is not related to Linux at all. When you develop any C or C++ code,
you run these tools. Upstream or downstream, does not matter. Why would
you not use automated, free and easy tools to detect errors in your
code? It's just a matter of professional approach to your code.

> to help with these kind of more advanced analysis, right?

They do not come for free (someone is paying for them even if they are
for free to you) and they have delays in responses.

> 
>> Until then, there isn't really anything to insist on. I even had submitters
>> react angry when I asked them to follow the guidance in that document,
>> so I won't insist on anything that isn't documented as requirement.
>> Quite frankly, most of the time I'd probably fix up issues such as missing
>> "static" myself because I am tired having to argue about it with submitters
>> who don't care.
>>
> 
> For the record, I do care about the code I submit and missing 'static' is quite
> embarrassing, yes. The only reason why I still dind't send the v3 fixing that is
> because I'm giving more time to see if you can review some of the other changes. I'm
> pretty sure I'll be asked to change more things in v2 (as stated in the cover, there
> are still some shady things in the driver).

Sure, mistakes happen. I did my share as well. I only brought the point
that contributor should use automated tools available for free to them
before using community resources. Community resources, including the
most expensive one - review by maintainers, are scarce.

Best regards,
Krzysztof