Re: [PATCH 5/5] hwmon: (nct6683) Enable pwm control on ASRock B850 Steel Legend WiFi

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The sysfs show/store callbacks for 'pwm' and 'pwm_enable' attributes incorrectly cast the incoming attribute pointer to `struct sensor_device_attribute_2`, causing broken behavior on Big Endian systems.
--

--- Patch [5]: [PATCH 5/5] hwmon: (nct6683) Enable pwm control on ASRock B850 Steel Legend WiFi ---
commit 098ac72f5515df05870f944950a3f04a964252c2
Author: Johan Dahlin <jdahlin@gmail.com>

hwmon: (nct6683) Enable pwm control on ASRock B850 Steel Legend WiFi

Fan control has been measured on this board: with a fan in manual mode the
applied pwm value tracks what is written, and the fan responds
proportionally from 337 RPM at pwm 60 to 933 RPM at pwm 255.

Allow pwm writes on this board.

> diff --git a/drivers/hwmon/nct6683.c b/drivers/hwmon/nct6683.c
> index 6825e6c3af34f..df28055b1a1c4 100644
> --- a/drivers/hwmon/nct6683.c
> +++ b/drivers/hwmon/nct6683.c
> @@ -972,7 +972,8 @@ SENSOR_TEMPLATE(pwm, "pwm%d", S_IRUGO, show_pwm, store_pwm, 0);

[Severity: High]
This is a pre-existing issue, but since this patch enables PWM writes for
a new board, is there a type punning bug in the PWM sysfs callbacks?

SENSOR_TEMPLATE defines these attributes as struct sensor_device_attribute,
which has an 'int index' field (4 bytes). However, the callbacks like
show_pwm() and store_pwm() cast this attribute to
struct sensor_device_attribute_2:

drivers/hwmon/nct6683.c:show_pwm() {
    struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
    int index = sattr->index;
    ...
}

struct sensor_device_attribute_2 uses a 'u8 index' (1 byte). On Big Endian
systems, reading the 1-byte u8 from a 4-byte int will yield 0 instead of
the true index, so all PWM attribute accesses will incorrectly target
channel 0.

Does this need to be fixed to use to_sensor_dev_attr(attr) instead?

>   */
>  static bool nct6683_has_fan_control(struct nct6683_data *data)
>  {
> -	return data->customer_id == NCT6683_CUSTOMER_ID_MITAC;
> +	return data->customer_id == NCT6683_CUSTOMER_ID_MITAC ||
> +	       data->customer_id == NCT6683_CUSTOMER_ID_ASROCK8;
>  }
>  
>  /*

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825092546.669450-1-jdahlin@gmail.com?part=5