linux-kernel - Re: [PATCH v4 2/2] power: supply: Add STC3117 fuel gauge unit driver

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: <057577e0-a0e6-453c-8e14-13fc078d99a6@kernel.org>
Date: Thu, 28 Nov 2024 09:55:57 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Bhavin Sharma <bhavin.sharma@...iconsignals.io>,
 "sre@...nel.org" <sre@...nel.org>, "krzk+dt@...nel.org"
 <krzk+dt@...nel.org>, "robh@...nel.org" <robh@...nel.org>,
 "conor+dt@...nel.org" <conor+dt@...nel.org>
Cc: Hardevsinh Palaniya <hardevsinh.palaniya@...iconsignals.io>,
 "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 2/2] power: supply: Add STC3117 fuel gauge unit driver

On 28/11/2024 09:44, Bhavin Sharma wrote:
> Hi Krzysztof,
>  
> Thank you for your review and feedback.
>  
>>> +struct stc3117_data {
>>> +     struct i2c_client       *client;
>>> +     struct regmap           *regmap;
>>> +     struct delayed_work update_work;
>>> +     struct power_supply     *battery;
>>> +
>>> +     u8 SOCValue[16];
>>> +     int CC_cnf;
>>> +     int VM_cnf;
>>> +     int CC_adj;
>>> +     int VM_adj;
>>> +     int AvgCurrent;
>>> +     int AvgVoltage;
>>> +     int Current;
>>> +     int Voltage;
>>> +     int Temp;
>>> +     int SOC;
>>> +     int OCV;
>>> +     int HRSOC;
>>> +     int Presence;
>>> +     int Battery_state;
>>
>> That's some Windows coding style... You need to clean up everything here
>> to match Linux Coding style.
>  
> Could you clarify what specific changes are required here to align with the Linux
> coding style?


Entire. Go one by one: "Breaking long lines and strings" - not
implemented. "Naming" - not implemented. Then go with every point. You
are making here some sort of shortcut - ignoring coding style, not
reading it and insisting on me to provide you exact things to change.
No, that's way too many things. You are supposed to read the coding style.

>  
> I am not sure what exactly needs to be changed here.
>  
>>> +     data->battery = devm_power_supply_register(&client->dev,
>>> +                                                &stc3117_battery_desc, &psy_cfg);
>>> +     if (IS_ERR(data->battery))
>>> +             dev_err_probe(&client->dev, PTR_ERR(data->battery), "failed to register battery\n");
>>> +
>> You ignored (again!) received comments. In multiple places. Go back to
>> previous email and carefully read commetns.
>  
> Sebastian suggested using dev_err_probe, while you mentioned using dev_err.
> so what should i follow ?


No. That's not true. Read comments again. I am not happy that after
pointing out you still insist and force me to re-iterate the same.
That's my last reply in this matter:

comment was:
"return dev_err_probe(dev, PTR_ERR(stc_sply), "failed to register
battery\n");"

Where do you have "return" statement?

What about all my other comments? You are supposed to reply inline and
acknowledge each of such comment. That's the only way I believe you will
really do what we ask you to do.

> 
>> One more thing:
>>  
>> Please wrap code according to coding style (checkpatch is not a coding
>> style description, but only a tool).
>  
> Could you recommend an example driver from the kernel source tree that 
> follows the expected coding style? This would help me ensure compliance.


`git log -- path` will tell give you the latest drivers..


>  
> Best Regards,
> Bhavin
>  


Trim your replies and do not top-post. All this copied stuff below is
making things just difficult to read.

> 
> 
> 
> 
> 
> 
> 
> ________________________________________
> From: Krzysztof Kozlowski <krzk@...nel.org>
> Sent: Wednesday, November 27, 2024 11:54 PM
> To: Bhavin Sharma <bhavin.sharma@...iconsignals.io>; sre@...nel.org <sre@...nel.org>; krzk+dt@...nel.org <krzk+dt@...nel.org>; robh@...nel.org <robh@...nel.org>; conor+dt@...nel.org <conor+dt@...nel.org>
> Cc: Hardevsinh Palaniya <hardevsinh.palaniya@...iconsignals.io>; linux-pm@...r.kernel.org <linux-pm@...r.kernel.org>; devicetree@...r.kernel.org <devicetree@...r.kernel.org>; linux-


All this must be removed.


Best regards,
Krzysztof