linux-kernel - Re: [PATCH v3 2/3] hwmon: (cros

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: <6cd4eb0f-4b4b-4540-877b-6531023b4a5d@roeck-us.net>
Date: Wed, 18 Jun 2025 05:53:29 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Sung-Chi Li <lschyi@...omium.org>, Thomas Weißschuh
 <linux@...ssschuh.net>
Cc: Benson Leung <bleung@...omium.org>, Guenter Roeck <groeck@...omium.org>,
 Jean Delvare <jdelvare@...e.com>, Jonathan Corbet <corbet@....net>,
 chrome-platform@...ts.linux.dev, linux-kernel@...r.kernel.org,
 linux-hwmon@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH v3 2/3] hwmon: (cros_ec) add PWM control over fans

On 6/18/25 00:26, Sung-Chi Li wrote:
> On Mon, May 12, 2025 at 09:30:39AM +0200, Thomas Weißschuh wrote:
> 
> Sorry for the late reply, I missed mails for this series.
> 
>> On 2025-05-12 15:11:56+0800, Sung-Chi Li via B4 Relay wrote:
>>> From: Sung-Chi Li <lschyi@...omium.org>
>>>   static int cros_ec_hwmon_read_temp(struct cros_ec_device *cros_ec, u8 index, u8 *temp)
>>>   {
>>>   	unsigned int offset;
>>> @@ -73,7 +117,9 @@ static long cros_ec_hwmon_temp_to_millicelsius(u8 temp)
>>>   static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>>>   			      u32 attr, int channel, long *val)
>>>   {
>>> +	u8 control_method;
>>>   	struct cros_ec_hwmon_priv *priv = dev_get_drvdata(dev);
>>> +	u8 pwm_value;
>>>   	int ret = -EOPNOTSUPP;
>>>   	u16 speed;
>>>   	u8 temp;
>>
>> Ordering again.
>>
>> This should be:
>>
>> struct cros_ec_hwmon_priv *priv = dev_get_drvdata(dev);
>> int ret = -EOPNOTSUPP;
>> u8 control_method;
>> u8 pwm_value;
>> u16 speed;
>> u8 temp;
>>
>> or:
>>
>> struct cros_ec_hwmon_priv *priv = dev_get_drvdata(dev);
>> u8 control_method, pwm_value, temp;
>> int ret = -EOPNOTSUPP;
>> u16 speed;
>>
>> <snip>
>>
> 
> Would you mind to share the sorting logic, so I do not bother you with checking
> these styling issue? Initially, I thought the sorting is based on the variable
> name, but after seeing your example (which I am appreciated), I am not sure how
> the sorting works. Is it sorted along with the variable types (
> "u8 control_method", "u16 speed", etc)? If that is the case, why "u16 speed" is
> in the middle of the u8 group variables?
> 


It is called "reverse christmas tree". Longer lines first.

Guenter