[PATCH v2 2/4] hwmon: (cros_ec) Add support for fan target speed

Thomas Weißschuh posted 4 patches 2 weeks, 5 days ago
[PATCH v2 2/4] hwmon: (cros_ec) Add support for fan target speed
Posted by Thomas Weißschuh 2 weeks, 5 days ago
Use EC_CMD_PWM_GET_FAN_TARGET_RPM to retrieve the target fan speed.
The EC only supports this for the first fan.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 Documentation/hwmon/cros_ec_hwmon.rst |  3 +++
 drivers/hwmon/cros_ec_hwmon.c         | 26 +++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/Documentation/hwmon/cros_ec_hwmon.rst b/Documentation/hwmon/cros_ec_hwmon.rst
index b7dc88d22fda..ebc8da48fa8a 100644
--- a/Documentation/hwmon/cros_ec_hwmon.rst
+++ b/Documentation/hwmon/cros_ec_hwmon.rst
@@ -29,6 +29,9 @@ Supported features
 Fan readings
     Always supported.
 
+Fan target speed
+    If supported by the EC.
+
 Temperature readings
     Always supported.
 
diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
index 48331703f2f5..53abd55cba05 100644
--- a/drivers/hwmon/cros_ec_hwmon.c
+++ b/drivers/hwmon/cros_ec_hwmon.c
@@ -86,6 +86,20 @@ static int cros_ec_hwmon_read_pwm_enable(struct cros_ec_device *cros_ec, u8 inde
 	return 0;
 }
 
+static int cros_ec_hwmon_read_fan_target(struct cros_ec_device *cros_ec, u16 *speed)
+{
+	struct ec_response_pwm_get_fan_rpm resp;
+	int ret;
+
+	ret = cros_ec_cmd(cros_ec, 0, EC_CMD_PWM_GET_FAN_TARGET_RPM,
+			  NULL, 0, &resp, sizeof(resp));
+	if (ret < 0)
+		return ret;
+
+	*speed = resp.rpm;
+	return 0;
+}
+
 static int cros_ec_hwmon_read_temp(struct cros_ec_device *cros_ec, u8 index, u8 *temp)
 {
 	unsigned int offset;
@@ -143,6 +157,11 @@ static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 			ret = cros_ec_hwmon_read_fan_speed(priv->cros_ec, channel, &speed);
 			if (ret == 0)
 				*val = cros_ec_hwmon_is_error_fan(speed);
+
+		} else if (attr == hwmon_fan_target) {
+			ret = cros_ec_hwmon_read_fan_target(priv->cros_ec, &speed);
+			if (ret == 0)
+				*val = speed;
 		}
 	} else if (type == hwmon_pwm) {
 		if (attr == hwmon_pwm_enable) {
@@ -259,8 +278,13 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
 					u32 attr, int channel)
 {
 	const struct cros_ec_hwmon_priv *priv = data;
+	u16 speed;
 
 	if (type == hwmon_fan) {
+		if (attr == hwmon_fan_target &&
+		    cros_ec_hwmon_read_fan_target(priv->cros_ec, &speed) == -EOPNOTSUPP)
+			return 0;
+
 		if (priv->usable_fans & BIT(channel))
 			return 0444;
 	} else if (type == hwmon_pwm) {
@@ -277,7 +301,7 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
 static const struct hwmon_channel_info * const cros_ec_hwmon_info[] = {
 	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
 	HWMON_CHANNEL_INFO(fan,
-			   HWMON_F_INPUT | HWMON_F_FAULT,
+			   HWMON_F_INPUT | HWMON_F_FAULT | HWMON_F_TARGET,
 			   HWMON_F_INPUT | HWMON_F_FAULT,
 			   HWMON_F_INPUT | HWMON_F_FAULT,
 			   HWMON_F_INPUT | HWMON_F_FAULT),

-- 
2.52.0

Re: [PATCH v2 2/4] hwmon: (cros_ec) Add support for fan target speed
Posted by Guenter Roeck 1 week, 4 days ago
On Sun, Jan 18, 2026 at 10:45:56AM +0100, Thomas Weißschuh wrote:
> Use EC_CMD_PWM_GET_FAN_TARGET_RPM to retrieve the target fan speed.
> The EC only supports this for the first fan.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Applied.

Thanks,
Guenter
Re: [PATCH v2 2/4] hwmon: (cros_ec) Add support for fan target speed
Posted by Tzung-Bi Shih 2 weeks, 2 days ago
On Sun, Jan 18, 2026 at 10:45:56AM +0100, Thomas Weißschuh wrote:
> @@ -259,8 +278,13 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
>  					u32 attr, int channel)
>  {
>  	const struct cros_ec_hwmon_priv *priv = data;
> +	u16 speed;
>  
>  	if (type == hwmon_fan) {
> +		if (attr == hwmon_fan_target &&
> +		    cros_ec_hwmon_read_fan_target(priv->cros_ec, &speed) == -EOPNOTSUPP)

[v2 4/4] patch uses is_cros_ec_cmd_available() for the purpose.  Can't it
also use here?
Re: [PATCH v2 2/4] hwmon: (cros_ec) Add support for fan target speed
Posted by Thomas Weißschuh 2 weeks, 2 days ago
On 2026-01-21 09:08:26+0000, Tzung-Bi Shih wrote:
> On Sun, Jan 18, 2026 at 10:45:56AM +0100, Thomas Weißschuh wrote:
> > @@ -259,8 +278,13 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
> >  					u32 attr, int channel)
> >  {
> >  	const struct cros_ec_hwmon_priv *priv = data;
> > +	u16 speed;
> >  
> >  	if (type == hwmon_fan) {
> > +		if (attr == hwmon_fan_target &&
> > +		    cros_ec_hwmon_read_fan_target(priv->cros_ec, &speed) == -EOPNOTSUPP)
> 
> [v2 4/4] patch uses is_cros_ec_cmd_available() for the purpose.  Can't it
> also use here?

That is somewhat intentional. The code in patch 4 is executed many
times, so caching the result is faster. The code here is only executed
once. Calling the accessor instead of using  is_cros_ec_cmd_available()
makes sure that we test for the correct command and version, without
needing another, long CROS_EC_HWMON_*_VERSION define.


Thomas
Re: [PATCH v2 2/4] hwmon: (cros_ec) Add support for fan target speed
Posted by Tzung-Bi Shih 2 weeks, 2 days ago
On Wed, Jan 21, 2026 at 09:55:38PM +0100, Thomas Weißschuh wrote:
> On 2026-01-21 09:08:26+0000, Tzung-Bi Shih wrote:
> > On Sun, Jan 18, 2026 at 10:45:56AM +0100, Thomas Weißschuh wrote:
> > > @@ -259,8 +278,13 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
> > >  					u32 attr, int channel)
> > >  {
> > >  	const struct cros_ec_hwmon_priv *priv = data;
> > > +	u16 speed;
> > >  
> > >  	if (type == hwmon_fan) {
> > > +		if (attr == hwmon_fan_target &&
> > > +		    cros_ec_hwmon_read_fan_target(priv->cros_ec, &speed) == -EOPNOTSUPP)
> > 
> > [v2 4/4] patch uses is_cros_ec_cmd_available() for the purpose.  Can't it
> > also use here?
> 
> That is somewhat intentional. The code in patch 4 is executed many
> times, so caching the result is faster. The code here is only executed
> once. Calling the accessor instead of using  is_cros_ec_cmd_available()
> makes sure that we test for the correct command and version, without
> needing another, long CROS_EC_HWMON_*_VERSION define.

I have no strong preference.  For the patch,
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>