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:17:34 -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 1/2] hwmon: Add pwmX_fan_channel attribute
Message-ID: <20220723141734.GC2979894@roeck-us.net>
References: <20220723033820.22612-1-W_Armin@gmx.de>
 <20220723033820.22612-2-W_Armin@gmx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220723033820.22612-2-W_Armin@gmx.de>
Precedence: bulk

On Sat, Jul 23, 2022 at 05:38:19AM +0200, Armin Wolf wrote:
> Until now, userspace software needs to guess which
> PWM channel is associated with which fan channel by
> probing each PWM output and watch for fan speed changes.
> This proccess is error-prone and unreliable.
> 
> Some hwmon chips, especially firmware-based ones, already
> know which PWM output is associated with which fan channel.
> 
> Allow such chips to export this knowledge to userspace.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

All of the chips I am aware of have a fixed association from pwm channel
output to fan input. None I am aware of make this association configurable.
I do not see the value of this attribute.

Guenter

> ---
>  Documentation/ABI/testing/sysfs-class-hwmon | 8 ++++++++
>  Documentation/hwmon/sysfs-interface.rst     | 3 +++
>  drivers/hwmon/hwmon.c                       | 1 +
>  include/linux/hwmon.h                       | 2 ++
>  4 files changed, 14 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-hwmon b/Documentation/ABI/testing/sysfs-class-hwmon
> index 7271781a23b2..f3d653bcf736 100644
> --- a/Documentation/ABI/testing/sysfs-class-hwmon
> +++ b/Documentation/ABI/testing/sysfs-class-hwmon
> @@ -315,6 +315,14 @@ Description:
> 
>  		RW
> 
> +What:		/sys/class/hwmon/hwmonX/pwmY_fan_channel
> +Description:
> +		Select which fan channel is controlled by this PWM output.
> +
> +		Valid fan channel/PWM output combinations are chip-dependent.
> +
> +		RW
> +
>  What:		/sys/class/hwmon/hwmonX/pwmY_auto_channels_temp
>  Description:
>  		Select which temperature channels affect this PWM output in
> diff --git a/Documentation/hwmon/sysfs-interface.rst b/Documentation/hwmon/sysfs-interface.rst
> index 209626fb2405..17fcec03d3c5 100644
> --- a/Documentation/hwmon/sysfs-interface.rst
> +++ b/Documentation/hwmon/sysfs-interface.rst
> @@ -209,6 +209,9 @@ PWM
>  `pwm[1-*]_freq`
>  		Base PWM frequency in Hz.
> 
> +`pwm[1-*]_fan_channel`
> +                Select which fan channel is controlled by this PWM output.
> +
>  `pwm[1-*]_auto_channels_temp`
>  		Select which temperature channels affect this PWM output in
>  		auto mode.
> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index 2e2cd79d89eb..8c2d7574c461 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
> @@ -604,6 +604,7 @@ static const char * const hwmon_pwm_attr_templates[] = {
>  	[hwmon_pwm_enable] = "pwm%d_enable",
>  	[hwmon_pwm_mode] = "pwm%d_mode",
>  	[hwmon_pwm_freq] = "pwm%d_freq",
> +	[hwmon_pwm_fan_channel] = "pwm%d_fan_channel",
>  	[hwmon_pwm_auto_channels_temp] = "pwm%d_auto_channels_temp",
>  };
> 
> diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
> index 14325f93c6b2..9d40cc1e520f 100644
> --- a/include/linux/hwmon.h
> +++ b/include/linux/hwmon.h
> @@ -332,6 +332,7 @@ enum hwmon_pwm_attributes {
>  	hwmon_pwm_enable,
>  	hwmon_pwm_mode,
>  	hwmon_pwm_freq,
> +	hwmon_pwm_fan_channel,
>  	hwmon_pwm_auto_channels_temp,
>  };
> 
> @@ -339,6 +340,7 @@ enum hwmon_pwm_attributes {
>  #define HWMON_PWM_ENABLE		BIT(hwmon_pwm_enable)
>  #define HWMON_PWM_MODE			BIT(hwmon_pwm_mode)
>  #define HWMON_PWM_FREQ			BIT(hwmon_pwm_freq)
> +#define HWMON_PWM_FAN_CHANNEL		BIT(hwmon_pwm_fan_channel)
>  #define HWMON_PWM_AUTO_CHANNELS_TEMP	BIT(hwmon_pwm_auto_channels_temp)
> 
>  enum hwmon_intrusion_attributes {
> --
> 2.30.2
>