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] [day] [month] [year] [list]

Message-ID: <9bcdd707-4920-4ed8-a808-22988ef0b2da@roeck-us.net>
Date: Fri, 9 May 2025 07:15:26 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Krzysztof Kozlowski <krzk@...nel.org>,
 Paweł Dembicki <paweldembicki@...il.com>
Cc: linux-hwmon@...r.kernel.org, Jean Delvare <jdelvare@...e.com>,
 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 5/9/25 02:22, Krzysztof Kozlowski wrote:
> 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 :
>>

hwmon, really, not just pmbus.

>> 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.
> 

hwmon traditionally (as in: from the very beginning) uses lowercase enums
for chip types. It would be even more odd to change that now.

Thanks,
Guenter