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

[Krzysztof Kozlowski <krzk@kernel.org>]

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@kernel.org>
> Sent: Wednesday, November 27, 2024 11:54 PM
> To: Bhavin Sharma <bhavin.sharma@siliconsignals.io>; sre@kernel.org <sre@kernel.org>; krzk+dt@kernel.org <krzk+dt@kernel.org>; robh@kernel.org <robh@kernel.org>; conor+dt@kernel.org <conor+dt@kernel.org>
> Cc: Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>; linux-pm@vger.kernel.org <linux-pm@vger.kernel.org>; devicetree@vger.kernel.org <devicetree@vger.kernel.org>; linux-


All this must be removed.


Best regards,
Krzysztof