linux-kernel - Re: [PATCH v2 1/5] hwmon: pmbus: mpq8785: Prepare driver for multiple device support

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: <e002674f-f180-425a-9f60-ce8f9126b058@kernel.org>
Date: Fri, 9 May 2025 11:22:25 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Paweł Dembicki <paweldembicki@...il.com>
Cc: linux-hwmon@...r.kernel.org, Jean Delvare <jdelvare@...e.com>,
 Guenter Roeck <linux@...ck-us.net>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Jonathan Corbet <corbet@....net>,
 Noah Wang <noahwang.wang@...look.com>,
 Naresh Solanki <naresh.solanki@...ements.com>,
 Fabio Estevam <festevam@...il.com>, Michal Simek <michal.simek@....com>,
 Grant Peltier <grantpeltier93@...il.com>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Shen Lichuan <shenlichuan@...o.com>, Peter Zijlstra <peterz@...radead.org>,
 Greg KH <gregkh@...uxfoundation.org>, Charles Hsu <ythsu0511@...il.com>,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 1/5] hwmon: pmbus: mpq8785: Prepare driver for multiple
 device support

On 09/05/2025 09:41, Paweł Dembicki wrote:
> pt., 9 maj 2025 o 09:03 Krzysztof Kozlowski <krzk@...nel.org> napisał(a):
>>
>> On 09/05/2025 08:51, Pawel Dembicki wrote:
>>> Refactor the driver to support multiple Monolithic Power Systems devices.
>>> Introduce chip ID handling based on device tree matching.
>>>
>>> No functional changes intended.
>>>
>>> Signed-off-by: Pawel Dembicki <paweldembicki@...il.com>
>>>
>>> ---
>>> v2:
>>>  - no changes done
>>> ---
>>>  drivers/hwmon/pmbus/mpq8785.c | 38 +++++++++++++++++++++++++++--------
>>>  1 file changed, 30 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
>>> index 331c274ca892..00ec21b081cb 100644
>>> --- a/drivers/hwmon/pmbus/mpq8785.c
>>> +++ b/drivers/hwmon/pmbus/mpq8785.c
>>> @@ -8,6 +8,8 @@
>>>  #include <linux/of_device.h>
>>>  #include "pmbus.h"
>>>
>>> +enum chips { mpq8785 };
>>
>> Use Linux coding style, so:
>> 1. missing wrapping after/before each {}
>> 2. missing descriptive name for the type (mpq8785_chips)
>> 3. CAPITALICS see Linux coding style - there is a chapter exactly about
>> this.
>>
>>
> 
> Sorry, I was thinking that it is a local pmbus tradition.
> Many drivers have the same enum without capitalics :
> 
> grep -r "enum chips {" .
> ./isl68137.c:enum chips {
> ./bel-pfe.c:enum chips {pfe1100, pfe3000};
> ./mp2975.c:enum chips {
> ./ucd9200.c:enum chips { ucd9200, ucd9220, ucd9222, ucd9224, ucd9240,
> ucd9244, ucd9246,
> ./zl6100.c:enum chips { zl2004, zl2005, zl2006, zl2008, zl2105,
> zl2106, zl6100, zl6105,
> ./ucd9000.c:enum chips { ucd9000, ucd90120, ucd90124, ucd90160,
> ucd90320, ucd9090,
> ./max16601.c:enum chips { max16508, max16600, max16601, max16602 };
> ./q54sj108a2.c:enum chips {
> ./bpa-rs600.c:enum chips { bpa_rs600, bpd_rs600 };
> ./adm1275.c:enum chips { adm1075, adm1272, adm1273, adm1275, adm1276,
> adm1278, adm1281, adm1293, adm1294 };
> ./max20730.c:enum chips {
> ./mp2856.c:enum chips { mp2856, mp2857 };
> ./tps53679.c:enum chips {
> ./ltc2978.c:enum chips {
> ./max34440.c:enum chips {
> ./pim4328.c:enum chips { pim4006, pim4328, pim4820 };
> ./fsp-3y.c:enum chips {
> ./lm25066.c:enum chips { lm25056, lm25066, lm5064, lm5066, lm5066i };

If that's standard for this subsystem, then it's fine, although to me it
feels odd to see all over the code lower case constant.


Best regards,
Krzysztof