Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Guenter Roeck <groeck7@gmail.com>
Date:   Sat, 23 Jul 2022 07:14:39 -0700
From:   Guenter Roeck <linux@roeck-us.net>
To:     Armin Wolf <W_Armin@gmx.de>
Cc:     jdelvare@suse.com, linux-hwmon@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] hwmon: (dell-smm) Add support for pwmX_fan_channel
 attribute
Message-ID: <20220723141439.GB2979894@roeck-us.net>
References: <20220723033820.22612-1-W_Armin@gmx.de>
 <20220723033820.22612-3-W_Armin@gmx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220723033820.22612-3-W_Armin@gmx.de>
Precedence: bulk

On Sat, Jul 23, 2022 at 05:38:20AM +0200, Armin Wolf wrote:
> The SMM interface does not differentiate between fan channels
> and pwm channels, so each pwmX is associated with fanX_*.
> 
> Inform userspace programs of this with the pwmX_fan_channel
> attribute.
> 
> Tested on a Dell Inspiron 3505.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  Documentation/hwmon/dell-smm-hwmon.rst |  1 +
>  drivers/hwmon/dell-smm-hwmon.c         | 15 ++++++++++++---
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/hwmon/dell-smm-hwmon.rst b/Documentation/hwmon/dell-smm-hwmon.rst
> index e5d85e40972c..2b0651c3f7c5 100644
> --- a/Documentation/hwmon/dell-smm-hwmon.rst
> +++ b/Documentation/hwmon/dell-smm-hwmon.rst
> @@ -38,6 +38,7 @@ fan[1-3]_min                    RO      Minimal Fan speed in RPM
>  fan[1-3]_max                    RO      Maximal Fan speed in RPM
>  fan[1-3]_target                 RO      Expected Fan speed in RPM
>  pwm[1-3]                        RW      Control the fan PWM duty-cycle.
> +pwm[1-3]_fan_channel            RO      Hints which PWM is connected to which fan
>  pwm1_enable                     WO      Enable or disable automatic BIOS fan
>                                          control (not supported on all laptops,
>                                          see below for details).
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 7f8d95dd2717..e6bf65d2ffc6 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -713,6 +713,11 @@ static umode_t dell_smm_is_visible(const void *drvdata, enum hwmon_sensor_types
>  			if (data->fan[channel])
>  				return 0644;
> 
> +			break;
> +		case hwmon_pwm_fan_channel:
> +			if (data->fan[channel])
> +				return 0444;
> +
>  			break;
>  		case hwmon_pwm_enable:
>  			if (data->auto_fan)
> @@ -800,6 +805,10 @@ static int dell_smm_read(struct device *dev, enum hwmon_sensor_types type, u32 a
> 
>  			*val = clamp_val(ret * data->i8k_pwm_mult, 0, 255);
> 
> +			return 0;
> +		case hwmon_pwm_fan_channel:
> +			*val = channel + 1;
> +

This effectively just returns the channel number and
thus adds adds zero value.

Guenter

>  			return 0;
>  		default:
>  			break;
> @@ -943,9 +952,9 @@ static const struct hwmon_channel_info *dell_smm_info[] = {
>  			   HWMON_F_TARGET
>  			   ),
>  	HWMON_CHANNEL_INFO(pwm,
> -			   HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
> -			   HWMON_PWM_INPUT,
> -			   HWMON_PWM_INPUT
> +			   HWMON_PWM_INPUT | HWMON_PWM_FAN_CHANNEL | HWMON_PWM_ENABLE,
> +			   HWMON_PWM_INPUT | HWMON_PWM_FAN_CHANNEL,
> +			   HWMON_PWM_INPUT | HWMON_PWM_FAN_CHANNEL
>  			   ),
>  	NULL
>  };
> --
> 2.30.2
>