Re: [PATCH] Fixes: a23870110a38 ("asus-wmi: add support for showing middle fan RPM")

From: Hans de Goede
Date: Mon Aug 21 2023 - 09:32:23 EST

Next message: Naresh Kamboju: "clang-17: selftests: vdso_standalone_test_x86.c:(.text+0x1e6): undefined reference to `memcpy'"
Previous message: haowenchao (C): "Re: [PATCH 00/13] scsi: Support LUN/target based error handle"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Luke,

The Fixes: tag goes into the Signed-off-by block, not in the Subject.

I have fixed this up while merging this:

Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans

On 8/15/23 03:42, Luke D. Jones wrote:
> After the addition of the mid fan custom curve functionality various
> incorrect behaviour was uncovered. This commit fixes these areas.
>
> - Ensure mid fan attributes actually use the correct fan ID
> - Correction to a bit mask for selecting the correct fan data
> - Refactor the curve show/store functions to be cleaner and and
> match each others layout
>
> Signed-off-by: Luke D. Jones <luke@xxxxxxxxxx>
> ---
> drivers/platform/x86/asus-wmi.c | 78 ++++++++++++++++-----------------
> 1 file changed, 38 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index d14d0ea9d65f..14ee43c61eb2 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -2902,9 +2902,8 @@ static int fan_curve_get_factory_default(struct asus_wmi *asus, u32 fan_dev)
> {
> struct fan_curve_data *curves;
> u8 buf[FAN_CURVE_BUF_LEN];
> - int fan_idx = 0;
> + int err, fan_idx;
> u8 mode = 0;
> - int err;
>
> if (asus->throttle_thermal_policy_available)
> mode = asus->throttle_thermal_policy_mode;
> @@ -2914,13 +2913,6 @@ static int fan_curve_get_factory_default(struct asus_wmi *asus, u32 fan_dev)
> else if (mode == 1)
> mode = 2;
>
> - if (fan_dev == ASUS_WMI_DEVID_GPU_FAN_CURVE)
> - fan_idx = FAN_CURVE_DEV_GPU;
> -
> - if (fan_dev == ASUS_WMI_DEVID_MID_FAN_CURVE)
> - fan_idx = FAN_CURVE_DEV_MID;
> -
> - curves = &asus->custom_fan_curves[fan_idx];
> err = asus_wmi_evaluate_method_buf(asus->dsts_id, fan_dev, mode, buf,
> FAN_CURVE_BUF_LEN);
> if (err) {
> @@ -2928,9 +2920,17 @@ static int fan_curve_get_factory_default(struct asus_wmi *asus, u32 fan_dev)
> return err;
> }
>
> - fan_curve_copy_from_buf(curves, buf);
> + fan_idx = FAN_CURVE_DEV_CPU;
> + if (fan_dev == ASUS_WMI_DEVID_GPU_FAN_CURVE)
> + fan_idx = FAN_CURVE_DEV_GPU;
> +
> + if (fan_dev == ASUS_WMI_DEVID_MID_FAN_CURVE)
> + fan_idx = FAN_CURVE_DEV_MID;
> +
> + curves = &asus->custom_fan_curves[fan_idx];
> curves->device_id = fan_dev;
>
> + fan_curve_copy_from_buf(curves, buf);
> return 0;
> }
>
> @@ -2960,7 +2960,7 @@ static struct fan_curve_data *fan_curve_attr_select(struct asus_wmi *asus,
> {
> int index = to_sensor_dev_attr(attr)->index;
>
> - return &asus->custom_fan_curves[index & FAN_CURVE_DEV_GPU];
> + return &asus->custom_fan_curves[index];
> }
>
> /* Determine which fan the attribute is for if SENSOR_ATTR_2 */
> @@ -2969,7 +2969,7 @@ static struct fan_curve_data *fan_curve_attr_2_select(struct asus_wmi *asus,
> {
> int nr = to_sensor_dev_attr_2(attr)->nr;
>
> - return &asus->custom_fan_curves[nr & FAN_CURVE_DEV_GPU];
> + return &asus->custom_fan_curves[nr & ~FAN_CURVE_PWM_MASK];
> }
>
> static ssize_t fan_curve_show(struct device *dev,
> @@ -2978,13 +2978,13 @@ static ssize_t fan_curve_show(struct device *dev,
> struct sensor_device_attribute_2 *dev_attr = to_sensor_dev_attr_2(attr);
> struct asus_wmi *asus = dev_get_drvdata(dev);
> struct fan_curve_data *data;
> - int value, index, nr;
> + int value, pwm, index;
>
> data = fan_curve_attr_2_select(asus, attr);
> + pwm = dev_attr->nr & FAN_CURVE_PWM_MASK;
> index = dev_attr->index;
> - nr = dev_attr->nr;
>
> - if (nr & FAN_CURVE_PWM_MASK)
> + if (pwm)
> value = data->percents[index];
> else
> value = data->temps[index];
> @@ -3027,23 +3027,21 @@ static ssize_t fan_curve_store(struct device *dev,
> struct sensor_device_attribute_2 *dev_attr = to_sensor_dev_attr_2(attr);
> struct asus_wmi *asus = dev_get_drvdata(dev);
> struct fan_curve_data *data;
> + int err, pwm, index;
> u8 value;
> - int err;
> -
> - int pwm = dev_attr->nr & FAN_CURVE_PWM_MASK;
> - int index = dev_attr->index;
>
> data = fan_curve_attr_2_select(asus, attr);
> + pwm = dev_attr->nr & FAN_CURVE_PWM_MASK;
> + index = dev_attr->index;
>
> err = kstrtou8(buf, 10, &value);
> if (err < 0)
> return err;
>
> - if (pwm) {
> + if (pwm)
> data->percents[index] = value;
> - } else {
> + else
> data->temps[index] = value;
> - }
>
> /*
> * Mark as disabled so the user has to explicitly enable to apply a
> @@ -3156,7 +3154,7 @@ static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point8_temp, fan_curve,
> FAN_CURVE_DEV_CPU, 7);
>
> static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point1_pwm, fan_curve,
> - FAN_CURVE_DEV_CPU | FAN_CURVE_PWM_MASK, 0);
> + FAN_CURVE_DEV_CPU | FAN_CURVE_PWM_MASK, 0);
> static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point2_pwm, fan_curve,
> FAN_CURVE_DEV_CPU | FAN_CURVE_PWM_MASK, 1);
> static SENSOR_DEVICE_ATTR_2_RW(pwm1_auto_point3_pwm, fan_curve,
> @@ -3209,40 +3207,40 @@ static SENSOR_DEVICE_ATTR_2_RW(pwm2_auto_point8_pwm, fan_curve,
> FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 7);
>
> /* MID */
> -static SENSOR_DEVICE_ATTR_RW(pwm3_enable, fan_curve_enable, FAN_CURVE_DEV_GPU);
> +static SENSOR_DEVICE_ATTR_RW(pwm3_enable, fan_curve_enable, FAN_CURVE_DEV_MID);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point1_temp, fan_curve,
> - FAN_CURVE_DEV_GPU, 0);
> + FAN_CURVE_DEV_MID, 0);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point2_temp, fan_curve,
> - FAN_CURVE_DEV_GPU, 1);
> + FAN_CURVE_DEV_MID, 1);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point3_temp, fan_curve,
> - FAN_CURVE_DEV_GPU, 2);
> + FAN_CURVE_DEV_MID, 2);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point4_temp, fan_curve,
> - FAN_CURVE_DEV_GPU, 3);
> + FAN_CURVE_DEV_MID, 3);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point5_temp, fan_curve,
> - FAN_CURVE_DEV_GPU, 4);
> + FAN_CURVE_DEV_MID, 4);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point6_temp, fan_curve,
> - FAN_CURVE_DEV_GPU, 5);
> + FAN_CURVE_DEV_MID, 5);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point7_temp, fan_curve,
> - FAN_CURVE_DEV_GPU, 6);
> + FAN_CURVE_DEV_MID, 6);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point8_temp, fan_curve,
> - FAN_CURVE_DEV_GPU, 7);
> + FAN_CURVE_DEV_MID, 7);
>
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point1_pwm, fan_curve,
> - FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 0);
> + FAN_CURVE_DEV_MID | FAN_CURVE_PWM_MASK, 0);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point2_pwm, fan_curve,
> - FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 1);
> + FAN_CURVE_DEV_MID | FAN_CURVE_PWM_MASK, 1);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point3_pwm, fan_curve,
> - FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 2);
> + FAN_CURVE_DEV_MID | FAN_CURVE_PWM_MASK, 2);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point4_pwm, fan_curve,
> - FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 3);
> + FAN_CURVE_DEV_MID | FAN_CURVE_PWM_MASK, 3);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point5_pwm, fan_curve,
> - FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 4);
> + FAN_CURVE_DEV_MID | FAN_CURVE_PWM_MASK, 4);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point6_pwm, fan_curve,
> - FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 5);
> + FAN_CURVE_DEV_MID | FAN_CURVE_PWM_MASK, 5);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point7_pwm, fan_curve,
> - FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 6);
> + FAN_CURVE_DEV_MID | FAN_CURVE_PWM_MASK, 6);
> static SENSOR_DEVICE_ATTR_2_RW(pwm3_auto_point8_pwm, fan_curve,
> - FAN_CURVE_DEV_GPU | FAN_CURVE_PWM_MASK, 7);
> + FAN_CURVE_DEV_MID | FAN_CURVE_PWM_MASK, 7);
>
> static struct attribute *asus_fan_curve_attr[] = {
> /* CPU */

Next message: Naresh Kamboju: "clang-17: selftests: vdso_standalone_test_x86.c:(.text+0x1e6): undefined reference to `memcpy'"
Previous message: haowenchao (C): "Re: [PATCH 00/13] scsi: Support LUN/target based error handle"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]