[PATCH v2 3/6] hwmon: sparx5: add LAN969x fan monitoring support

[PATCH v2 3/6] hwmon: sparx5: add LAN969x fan monitoring support

[Robert Marko]

LAN969x has a fan controller with a tachometer in a register block
separate from its Sparx5-compatible temperature sensor. Match the fan
node independently and expose fan1_input on its own hwmon device.

Read the optional pulses-per-revolution property from the fan child.
Preserve the temperature-only behavior of existing device trees.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v2:
- Register fan and temperature monitoring as separate hwmon devices.
- Reject a present but malformed pulses-per-revolution property.

 Documentation/hwmon/sparx5-temp.rst |   9 ++
 drivers/hwmon/Kconfig               |   7 +-
 drivers/hwmon/sparx5-temp.c         | 146 ++++++++++++++++++++++++----
 3 files changed, 139 insertions(+), 23 deletions(-)

diff --git a/Documentation/hwmon/sparx5-temp.rst b/Documentation/hwmon/sparx5-temp.rst
index 0140809089542..a053d12fbdbbb 100644
--- a/Documentation/hwmon/sparx5-temp.rst
+++ b/Documentation/hwmon/sparx5-temp.rst
@@ -15,6 +15,10 @@ Supported chips:
 
 Author: Lars Povlsen <lars.povlsen@microchip.com>
 
+The driver also supports the Microchip LAN969x temperature sensor and fan
+controller as separate devices. The fan controller uses the
+microchip,lan9691-fan compatible.
+
 Description
 -----------
 
@@ -30,4 +34,9 @@ The following attributes are supported.
 
 ======================= ========================================================
 temp1_input		Die temperature (in millidegree Celsius.)
+fan1_input              Fan speed in RPM (LAN969x only).
 ======================= ========================================================
+
+The tachometer counts pulses over one-second intervals, independently of the
+PWM output. The fan child's pulses-per-revolution property specifies how many
+pulses represent one revolution; it defaults to two.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 3ae676a56d95c..deb9eb82b8501 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -669,11 +669,12 @@ config SENSORS_I5K_AMB
 	  will be called i5k_amb.
 
 config SENSORS_SPARX5
-	tristate "Sparx5 SoC temperature sensor"
+	tristate "Sparx5 and LAN969x SoC hardware monitoring"
 	depends on ARCH_SPARX5 || ARCH_LAN969X || COMPILE_TEST
 	help
-	  If you say yes here you get support for temperature monitoring
-	  with the Microchip Sparx5 SoC.
+	  If you say yes here you get support for temperature monitoring on
+	  Microchip Sparx5 and LAN969x SoCs, as well as fan monitoring on
+	  LAN969x SoCs.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called sparx5-temp.
diff --git a/drivers/hwmon/sparx5-temp.c b/drivers/hwmon/sparx5-temp.c
index 0fd5c147f8167..1f197004780cc 100644
--- a/drivers/hwmon/sparx5-temp.c
+++ b/drivers/hwmon/sparx5-temp.c
@@ -9,8 +9,11 @@
 #include <linux/hwmon.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 
 #define TEMP_CTRL		0
 #define TEMP_CFG		4
@@ -20,9 +23,26 @@
 #define  TEMP_STAT_VALID	BIT(12)
 #define  TEMP_STAT_TEMP		GENMASK(11, 0)
 
+#define FAN_CFG			0
+#define  FAN_CFG_GATE_ENA	BIT(2)
+#define  FAN_CFG_STAT_MODE	BIT(0)
+#define FAN_PWM_FREQ		4
+#define  FAN_PWM_FREQ_CYCLES	GENMASK(27, 16)
+#define FAN_CNT			8
+#define  FAN_CNT_DATA		GENMASK(15, 0)
+
+#define FAN_TACH_INTERVAL_HZ	100000
+
+struct s5_match_data {
+	bool is_fan;
+};
+
 struct s5_hwmon {
 	void __iomem *base;
+	void __iomem *fan;
 	struct clk *clk;
+	unsigned long clk_rate;
+	u32 pulses_per_revolution;
 };
 
 static void s5_temp_enable(struct s5_hwmon *hwmon)
@@ -37,6 +57,24 @@ static void s5_temp_enable(struct s5_hwmon *hwmon)
 	writel(val, hwmon->base + TEMP_CFG);
 }
 
+static void s5_fan_enable(struct s5_hwmon *hwmon)
+{
+	u32 cycles, val;
+
+	cycles = DIV_ROUND_CLOSEST(hwmon->clk_rate, FAN_TACH_INTERVAL_HZ);
+	cycles = clamp_val(cycles, 1, FIELD_MAX(FAN_PWM_FREQ_CYCLES));
+
+	val = readl(hwmon->fan + FAN_PWM_FREQ);
+	val &= ~FAN_PWM_FREQ_CYCLES;
+	val |= FIELD_PREP(FAN_PWM_FREQ_CYCLES, cycles);
+	writel(val, hwmon->fan + FAN_PWM_FREQ);
+
+	/* Count all tachometer pulses over one-second intervals. */
+	val = readl(hwmon->fan + FAN_CFG);
+	val &= ~(FAN_CFG_STAT_MODE | FAN_CFG_GATE_ENA);
+	writel(val, hwmon->fan + FAN_CFG);
+}
+
 static int s5_read_temp(struct s5_hwmon *hwmon, long *temp)
 {
 	int value;
@@ -62,6 +100,17 @@ static int s5_read_temp(struct s5_hwmon *hwmon, long *temp)
 	return 0;
 }
 
+static int s5_read_fan(struct s5_hwmon *hwmon, long *val)
+{
+	u32 count;
+
+	count = readl_relaxed(hwmon->fan + FAN_CNT);
+	count = FIELD_GET(FAN_CNT_DATA, count);
+	*val = count * 60 / hwmon->pulses_per_revolution;
+
+	return 0;
+}
+
 static int s5_read(struct device *dev, enum hwmon_sensor_types type,
 		   u32 attr, int channel, long *val)
 {
@@ -72,6 +121,10 @@ static int s5_read(struct device *dev, enum hwmon_sensor_types type,
 		if (attr == hwmon_temp_input)
 			return s5_read_temp(hwmon, val);
 		break;
+	case hwmon_fan:
+		if (attr == hwmon_fan_input && hwmon->fan)
+			return s5_read_fan(hwmon, val);
+		break;
 	default:
 		break;
 	}
@@ -82,62 +135,115 @@ static int s5_read(struct device *dev, enum hwmon_sensor_types type,
 static umode_t s5_is_visible(const void *_data, enum hwmon_sensor_types type,
 			     u32 attr, int channel)
 {
-	if (type != hwmon_temp)
-		return 0;
+	const struct s5_hwmon *hwmon = _data;
 
-	switch (attr) {
-	case hwmon_temp_input:
-		return 0444;
+	switch (type) {
+	case hwmon_temp:
+		return attr == hwmon_temp_input ? 0444 : 0;
+	case hwmon_fan:
+		if (hwmon->fan && attr == hwmon_fan_input)
+			return 0444;
+		return 0;
 	default:
 		return 0;
 	}
 }
 
-static const struct hwmon_channel_info * const s5_info[] = {
+static const struct hwmon_channel_info * const s5_temp_info[] = {
 	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
 	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
 	NULL
 };
 
+static const struct hwmon_channel_info * const s5_fan_info[] = {
+	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
+	NULL
+};
+
 static const struct hwmon_ops s5_hwmon_ops = {
 	.is_visible = s5_is_visible,
 	.read = s5_read,
 };
 
-static const struct hwmon_chip_info s5_chip_info = {
+static const struct hwmon_chip_info s5_temp_chip_info = {
 	.ops = &s5_hwmon_ops,
-	.info = s5_info,
+	.info = s5_temp_info,
+};
+
+static const struct hwmon_chip_info s5_fan_chip_info = {
+	.ops = &s5_hwmon_ops,
+	.info = s5_fan_info,
 };
 
 static int s5_temp_probe(struct platform_device *pdev)
 {
+	const struct s5_match_data *match_data;
+	struct device *dev = &pdev->dev;
 	struct device *hwmon_dev;
 	struct s5_hwmon *hwmon;
+	struct device_node *fan_np __free(device_node) = NULL;
+	void __iomem *base;
+	bool is_fan;
+	int ret;
 
-	hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon), GFP_KERNEL);
+	match_data = device_get_match_data(dev);
+	is_fan = match_data && match_data->is_fan;
+
+	hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL);
 	if (!hwmon)
 		return -ENOMEM;
 
-	hwmon->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(hwmon->base))
-		return PTR_ERR(hwmon->base);
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	if (is_fan) {
+		hwmon->fan = base;
+		hwmon->pulses_per_revolution = 2;
+		fan_np = of_get_available_child_by_name(dev->of_node, "fan");
+		if (fan_np &&
+		    of_property_present(fan_np, "pulses-per-revolution")) {
+			ret = of_property_read_u32(fan_np, "pulses-per-revolution",
+						   &hwmon->pulses_per_revolution);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "invalid pulses-per-revolution\n");
+		}
+		if (!hwmon->pulses_per_revolution ||
+		    hwmon->pulses_per_revolution > 4)
+			return dev_err_probe(dev, -EINVAL,
+					     "invalid pulses-per-revolution\n");
+	} else {
+		hwmon->base = base;
+	}
 
-	hwmon->clk = devm_clk_get_enabled(&pdev->dev, NULL);
+	hwmon->clk = devm_clk_get_enabled(dev, NULL);
 	if (IS_ERR(hwmon->clk))
 		return PTR_ERR(hwmon->clk);
+	hwmon->clk_rate = clk_get_rate(hwmon->clk);
 
-	s5_temp_enable(hwmon);
+	if (is_fan)
+		s5_fan_enable(hwmon);
+	else
+		s5_temp_enable(hwmon);
 
-	hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
-							 "s5_temp",
-							 hwmon,
-							 &s5_chip_info,
-							 NULL);
+	hwmon_dev = devm_hwmon_device_register_with_info(dev,
+					is_fan ? "lan969x_fan" : "s5_temp", hwmon,
+					is_fan ? &s5_fan_chip_info :
+						 &s5_temp_chip_info, NULL);
 
 	return PTR_ERR_OR_ZERO(hwmon_dev);
 }
 
+static const struct s5_match_data lan969x_fan_match_data = {
+	.is_fan = true,
+};
+
 static const struct of_device_id s5_temp_match[] = {
+	{
+		.compatible = "microchip,lan9691-fan",
+		.data = &lan969x_fan_match_data,
+	},
 	{ .compatible = "microchip,sparx5-temp" },
 	{},
 };
@@ -154,5 +260,5 @@ static struct platform_driver s5_temp_driver = {
 module_platform_driver(s5_temp_driver);
 
 MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>");
-MODULE_DESCRIPTION("Sparx5 SoC temperature sensor driver");
+MODULE_DESCRIPTION("Sparx5 SoC hardware monitoring driver");
 MODULE_LICENSE("GPL");
-- 
2.56.0

Re: [PATCH v2 3/6] hwmon: sparx5: add LAN969x fan monitoring support

[Guenter Roeck]

On Mon, Nov 09, 2026 at 02:35:07PM +0100, Robert Marko wrote:
> LAN969x has a fan controller with a tachometer in a register block
> separate from its Sparx5-compatible temperature sensor. Match the fan
> node independently and expose fan1_input on its own hwmon device.
> 
> Read the optional pulses-per-revolution property from the fan child.
> Preserve the temperature-only behavior of existing device trees.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>

I don't see the point of keeping this as single driver with two completely
different sets of functionality. If you want to have two instances, please
make it separate drivers.

Thanks,
Guenter

> ---
> Changes in v2:
> - Register fan and temperature monitoring as separate hwmon devices.
> - Reject a present but malformed pulses-per-revolution property.
> 
>  Documentation/hwmon/sparx5-temp.rst |   9 ++
>  drivers/hwmon/Kconfig               |   7 +-
>  drivers/hwmon/sparx5-temp.c         | 146 ++++++++++++++++++++++++----
>  3 files changed, 139 insertions(+), 23 deletions(-)
> 
> diff --git a/Documentation/hwmon/sparx5-temp.rst b/Documentation/hwmon/sparx5-temp.rst
> index 0140809089542..a053d12fbdbbb 100644
> --- a/Documentation/hwmon/sparx5-temp.rst
> +++ b/Documentation/hwmon/sparx5-temp.rst
> @@ -15,6 +15,10 @@ Supported chips:
>  
>  Author: Lars Povlsen <lars.povlsen@microchip.com>
>  
> +The driver also supports the Microchip LAN969x temperature sensor and fan
> +controller as separate devices. The fan controller uses the
> +microchip,lan9691-fan compatible.
> +
>  Description
>  -----------
>  
> @@ -30,4 +34,9 @@ The following attributes are supported.
>  
>  ======================= ========================================================
>  temp1_input		Die temperature (in millidegree Celsius.)
> +fan1_input              Fan speed in RPM (LAN969x only).

FWIW, this is misleading. One instance will support the temperature, a
separate hwmon device will support the fan. Really, if you want to have
two separate devices, make it two separate drivers.

>  ======================= ========================================================
> +
> +The tachometer counts pulses over one-second intervals, independently of the
> +PWM output. The fan child's pulses-per-revolution property specifies how many
> +pulses represent one revolution; it defaults to two.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 3ae676a56d95c..deb9eb82b8501 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -669,11 +669,12 @@ config SENSORS_I5K_AMB
>  	  will be called i5k_amb.
>  
>  config SENSORS_SPARX5
> -	tristate "Sparx5 SoC temperature sensor"
> +	tristate "Sparx5 and LAN969x SoC hardware monitoring"
>  	depends on ARCH_SPARX5 || ARCH_LAN969X || COMPILE_TEST
>  	help
> -	  If you say yes here you get support for temperature monitoring
> -	  with the Microchip Sparx5 SoC.
> +	  If you say yes here you get support for temperature monitoring on
> +	  Microchip Sparx5 and LAN969x SoCs, as well as fan monitoring on
> +	  LAN969x SoCs.

Same here. Based on the description and the documentation, a user would not
expect to get two separate hardware monitoring devices.

>  
>  	  This driver can also be built as a module. If so, the module
>  	  will be called sparx5-temp.
> diff --git a/drivers/hwmon/sparx5-temp.c b/drivers/hwmon/sparx5-temp.c
> index 0fd5c147f8167..1f197004780cc 100644
> --- a/drivers/hwmon/sparx5-temp.c
> +++ b/drivers/hwmon/sparx5-temp.c
> @@ -9,8 +9,11 @@
>  #include <linux/hwmon.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
> +#include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/property.h>
>  
>  #define TEMP_CTRL		0
>  #define TEMP_CFG		4
> @@ -20,9 +23,26 @@
>  #define  TEMP_STAT_VALID	BIT(12)
>  #define  TEMP_STAT_TEMP		GENMASK(11, 0)
>  
> +#define FAN_CFG			0
> +#define  FAN_CFG_GATE_ENA	BIT(2)
> +#define  FAN_CFG_STAT_MODE	BIT(0)
> +#define FAN_PWM_FREQ		4
> +#define  FAN_PWM_FREQ_CYCLES	GENMASK(27, 16)
> +#define FAN_CNT			8
> +#define  FAN_CNT_DATA		GENMASK(15, 0)
> +
> +#define FAN_TACH_INTERVAL_HZ	100000
> +
> +struct s5_match_data {
> +	bool is_fan;
> +};
> +
>  struct s5_hwmon {
>  	void __iomem *base;
> +	void __iomem *fan;
>  	struct clk *clk;
> +	unsigned long clk_rate;
> +	u32 pulses_per_revolution;
>  };
>  
>  static void s5_temp_enable(struct s5_hwmon *hwmon)
> @@ -37,6 +57,24 @@ static void s5_temp_enable(struct s5_hwmon *hwmon)
>  	writel(val, hwmon->base + TEMP_CFG);
>  }
>  
> +static void s5_fan_enable(struct s5_hwmon *hwmon)
> +{
> +	u32 cycles, val;
> +
> +	cycles = DIV_ROUND_CLOSEST(hwmon->clk_rate, FAN_TACH_INTERVAL_HZ);
> +	cycles = clamp_val(cycles, 1, FIELD_MAX(FAN_PWM_FREQ_CYCLES));
> +
> +	val = readl(hwmon->fan + FAN_PWM_FREQ);
> +	val &= ~FAN_PWM_FREQ_CYCLES;
> +	val |= FIELD_PREP(FAN_PWM_FREQ_CYCLES, cycles);
> +	writel(val, hwmon->fan + FAN_PWM_FREQ);
> +
> +	/* Count all tachometer pulses over one-second intervals. */
> +	val = readl(hwmon->fan + FAN_CFG);
> +	val &= ~(FAN_CFG_STAT_MODE | FAN_CFG_GATE_ENA);
> +	writel(val, hwmon->fan + FAN_CFG);
> +}
> +
>  static int s5_read_temp(struct s5_hwmon *hwmon, long *temp)
>  {
>  	int value;
> @@ -62,6 +100,17 @@ static int s5_read_temp(struct s5_hwmon *hwmon, long *temp)
>  	return 0;
>  }
>  
> +static int s5_read_fan(struct s5_hwmon *hwmon, long *val)
> +{
> +	u32 count;
> +
> +	count = readl_relaxed(hwmon->fan + FAN_CNT);
> +	count = FIELD_GET(FAN_CNT_DATA, count);
> +	*val = count * 60 / hwmon->pulses_per_revolution;
> +
> +	return 0;
> +}
> +
>  static int s5_read(struct device *dev, enum hwmon_sensor_types type,
>  		   u32 attr, int channel, long *val)
>  {
> @@ -72,6 +121,10 @@ static int s5_read(struct device *dev, enum hwmon_sensor_types type,
>  		if (attr == hwmon_temp_input)
>  			return s5_read_temp(hwmon, val);
>  		break;
> +	case hwmon_fan:
> +		if (attr == hwmon_fan_input && hwmon->fan)
> +			return s5_read_fan(hwmon, val);
> +		break;
>  	default:
>  		break;
>  	}
> @@ -82,62 +135,115 @@ static int s5_read(struct device *dev, enum hwmon_sensor_types type,
>  static umode_t s5_is_visible(const void *_data, enum hwmon_sensor_types type,
>  			     u32 attr, int channel)
>  {
> -	if (type != hwmon_temp)
> -		return 0;
> +	const struct s5_hwmon *hwmon = _data;
>  
> -	switch (attr) {
> -	case hwmon_temp_input:
> -		return 0444;
> +	switch (type) {
> +	case hwmon_temp:
> +		return attr == hwmon_temp_input ? 0444 : 0;
> +	case hwmon_fan:
> +		if (hwmon->fan && attr == hwmon_fan_input)
> +			return 0444;
> +		return 0;
>  	default:
>  		return 0;
>  	}
>  }
>  
> -static const struct hwmon_channel_info * const s5_info[] = {
> +static const struct hwmon_channel_info * const s5_temp_info[] = {
>  	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
>  	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
>  	NULL
>  };
>  
> +static const struct hwmon_channel_info * const s5_fan_info[] = {
> +	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
> +	NULL
> +};
> +
>  static const struct hwmon_ops s5_hwmon_ops = {
>  	.is_visible = s5_is_visible,
>  	.read = s5_read,
>  };
>  
> -static const struct hwmon_chip_info s5_chip_info = {
> +static const struct hwmon_chip_info s5_temp_chip_info = {
>  	.ops = &s5_hwmon_ops,
> -	.info = s5_info,
> +	.info = s5_temp_info,
> +};
> +
> +static const struct hwmon_chip_info s5_fan_chip_info = {
> +	.ops = &s5_hwmon_ops,
> +	.info = s5_fan_info,
>  };
>  
>  static int s5_temp_probe(struct platform_device *pdev)
>  {
> +	const struct s5_match_data *match_data;
> +	struct device *dev = &pdev->dev;
>  	struct device *hwmon_dev;
>  	struct s5_hwmon *hwmon;
> +	struct device_node *fan_np __free(device_node) = NULL;
> +	void __iomem *base;
> +	bool is_fan;
> +	int ret;
>  
> -	hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon), GFP_KERNEL);
> +	match_data = device_get_match_data(dev);
> +	is_fan = match_data && match_data->is_fan;
> +
> +	hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL);
>  	if (!hwmon)
>  		return -ENOMEM;
>  
> -	hwmon->base = devm_platform_ioremap_resource(pdev, 0);
> -	if (IS_ERR(hwmon->base))
> -		return PTR_ERR(hwmon->base);
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	if (is_fan) {
> +		hwmon->fan = base;
> +		hwmon->pulses_per_revolution = 2;
> +		fan_np = of_get_available_child_by_name(dev->of_node, "fan");
> +		if (fan_np &&
> +		    of_property_present(fan_np, "pulses-per-revolution")) {
> +			ret = of_property_read_u32(fan_np, "pulses-per-revolution",
> +						   &hwmon->pulses_per_revolution);
> +			if (ret)
> +				return dev_err_probe(dev, ret,
> +						     "invalid pulses-per-revolution\n");
> +		}
> +		if (!hwmon->pulses_per_revolution ||
> +		    hwmon->pulses_per_revolution > 4)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "invalid pulses-per-revolution\n");
> +	} else {
> +		hwmon->base = base;
> +	}
>  
> -	hwmon->clk = devm_clk_get_enabled(&pdev->dev, NULL);
> +	hwmon->clk = devm_clk_get_enabled(dev, NULL);
>  	if (IS_ERR(hwmon->clk))
>  		return PTR_ERR(hwmon->clk);
> +	hwmon->clk_rate = clk_get_rate(hwmon->clk);
>  
> -	s5_temp_enable(hwmon);
> +	if (is_fan)
> +		s5_fan_enable(hwmon);
> +	else
> +		s5_temp_enable(hwmon);
>  
> -	hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
> -							 "s5_temp",
> -							 hwmon,
> -							 &s5_chip_info,
> -							 NULL);
> +	hwmon_dev = devm_hwmon_device_register_with_info(dev,
> +					is_fan ? "lan969x_fan" : "s5_temp", hwmon,
> +					is_fan ? &s5_fan_chip_info :
> +						 &s5_temp_chip_info, NULL);
>  
>  	return PTR_ERR_OR_ZERO(hwmon_dev);
>  }
>  
> +static const struct s5_match_data lan969x_fan_match_data = {
> +	.is_fan = true,
> +};
> +
>  static const struct of_device_id s5_temp_match[] = {
> +	{
> +		.compatible = "microchip,lan9691-fan",
> +		.data = &lan969x_fan_match_data,
> +	},
>  	{ .compatible = "microchip,sparx5-temp" },
>  	{},
>  };
> @@ -154,5 +260,5 @@ static struct platform_driver s5_temp_driver = {
>  module_platform_driver(s5_temp_driver);
>  
>  MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>");
> -MODULE_DESCRIPTION("Sparx5 SoC temperature sensor driver");
> +MODULE_DESCRIPTION("Sparx5 SoC hardware monitoring driver");
>  MODULE_LICENSE("GPL");

Re: [PATCH v2 3/6] hwmon: sparx5: add LAN969x fan monitoring support

[Robert Marko]

On Tue, Nov 10, 2026 at 12:51 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Mon, Nov 09, 2026 at 02:35:07PM +0100, Robert Marko wrote:
> > LAN969x has a fan controller with a tachometer in a register block
> > separate from its Sparx5-compatible temperature sensor. Match the fan
> > node independently and expose fan1_input on its own hwmon device.
> >
> > Read the optional pulses-per-revolution property from the fan child.
> > Preserve the temperature-only behavior of existing device trees.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
>
> I don't see the point of keeping this as single driver with two completely
> different sets of functionality. If you want to have two instances, please
> make it separate drivers.

In v1 it was the same instance but then DT maintainers pointed out
that it should be different nodes at least.

I will split it into a separate driver in v3.

Regards,
Robert

>
> Thanks,
> Guenter
>
> > ---
> > Changes in v2:
> > - Register fan and temperature monitoring as separate hwmon devices.
> > - Reject a present but malformed pulses-per-revolution property.
> >
> >  Documentation/hwmon/sparx5-temp.rst |   9 ++
> >  drivers/hwmon/Kconfig               |   7 +-
> >  drivers/hwmon/sparx5-temp.c         | 146 ++++++++++++++++++++++++----
> >  3 files changed, 139 insertions(+), 23 deletions(-)
> >
> > diff --git a/Documentation/hwmon/sparx5-temp.rst b/Documentation/hwmon/sparx5-temp.rst
> > index 0140809089542..a053d12fbdbbb 100644
> > --- a/Documentation/hwmon/sparx5-temp.rst
> > +++ b/Documentation/hwmon/sparx5-temp.rst
> > @@ -15,6 +15,10 @@ Supported chips:
> >
> >  Author: Lars Povlsen <lars.povlsen@microchip.com>
> >
> > +The driver also supports the Microchip LAN969x temperature sensor and fan
> > +controller as separate devices. The fan controller uses the
> > +microchip,lan9691-fan compatible.
> > +
> >  Description
> >  -----------
> >
> > @@ -30,4 +34,9 @@ The following attributes are supported.
> >
> >  ======================= ========================================================
> >  temp1_input          Die temperature (in millidegree Celsius.)
> > +fan1_input              Fan speed in RPM (LAN969x only).
>
> FWIW, this is misleading. One instance will support the temperature, a
> separate hwmon device will support the fan. Really, if you want to have
> two separate devices, make it two separate drivers.
>
> >  ======================= ========================================================
> > +
> > +The tachometer counts pulses over one-second intervals, independently of the
> > +PWM output. The fan child's pulses-per-revolution property specifies how many
> > +pulses represent one revolution; it defaults to two.
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 3ae676a56d95c..deb9eb82b8501 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -669,11 +669,12 @@ config SENSORS_I5K_AMB
> >         will be called i5k_amb.
> >
> >  config SENSORS_SPARX5
> > -     tristate "Sparx5 SoC temperature sensor"
> > +     tristate "Sparx5 and LAN969x SoC hardware monitoring"
> >       depends on ARCH_SPARX5 || ARCH_LAN969X || COMPILE_TEST
> >       help
> > -       If you say yes here you get support for temperature monitoring
> > -       with the Microchip Sparx5 SoC.
> > +       If you say yes here you get support for temperature monitoring on
> > +       Microchip Sparx5 and LAN969x SoCs, as well as fan monitoring on
> > +       LAN969x SoCs.
>
> Same here. Based on the description and the documentation, a user would not
> expect to get two separate hardware monitoring devices.
>
> >
> >         This driver can also be built as a module. If so, the module
> >         will be called sparx5-temp.
> > diff --git a/drivers/hwmon/sparx5-temp.c b/drivers/hwmon/sparx5-temp.c
> > index 0fd5c147f8167..1f197004780cc 100644
> > --- a/drivers/hwmon/sparx5-temp.c
> > +++ b/drivers/hwmon/sparx5-temp.c
> > @@ -9,8 +9,11 @@
> >  #include <linux/hwmon.h>
> >  #include <linux/init.h>
> >  #include <linux/io.h>
> > +#include <linux/kernel.h>
> >  #include <linux/module.h>
> > +#include <linux/of.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/property.h>
> >
> >  #define TEMP_CTRL            0
> >  #define TEMP_CFG             4
> > @@ -20,9 +23,26 @@
> >  #define  TEMP_STAT_VALID     BIT(12)
> >  #define  TEMP_STAT_TEMP              GENMASK(11, 0)
> >
> > +#define FAN_CFG                      0
> > +#define  FAN_CFG_GATE_ENA    BIT(2)
> > +#define  FAN_CFG_STAT_MODE   BIT(0)
> > +#define FAN_PWM_FREQ         4
> > +#define  FAN_PWM_FREQ_CYCLES GENMASK(27, 16)
> > +#define FAN_CNT                      8
> > +#define  FAN_CNT_DATA                GENMASK(15, 0)
> > +
> > +#define FAN_TACH_INTERVAL_HZ 100000
> > +
> > +struct s5_match_data {
> > +     bool is_fan;
> > +};
> > +
> >  struct s5_hwmon {
> >       void __iomem *base;
> > +     void __iomem *fan;
> >       struct clk *clk;
> > +     unsigned long clk_rate;
> > +     u32 pulses_per_revolution;
> >  };
> >
> >  static void s5_temp_enable(struct s5_hwmon *hwmon)
> > @@ -37,6 +57,24 @@ static void s5_temp_enable(struct s5_hwmon *hwmon)
> >       writel(val, hwmon->base + TEMP_CFG);
> >  }
> >
> > +static void s5_fan_enable(struct s5_hwmon *hwmon)
> > +{
> > +     u32 cycles, val;
> > +
> > +     cycles = DIV_ROUND_CLOSEST(hwmon->clk_rate, FAN_TACH_INTERVAL_HZ);
> > +     cycles = clamp_val(cycles, 1, FIELD_MAX(FAN_PWM_FREQ_CYCLES));
> > +
> > +     val = readl(hwmon->fan + FAN_PWM_FREQ);
> > +     val &= ~FAN_PWM_FREQ_CYCLES;
> > +     val |= FIELD_PREP(FAN_PWM_FREQ_CYCLES, cycles);
> > +     writel(val, hwmon->fan + FAN_PWM_FREQ);
> > +
> > +     /* Count all tachometer pulses over one-second intervals. */
> > +     val = readl(hwmon->fan + FAN_CFG);
> > +     val &= ~(FAN_CFG_STAT_MODE | FAN_CFG_GATE_ENA);
> > +     writel(val, hwmon->fan + FAN_CFG);
> > +}
> > +
> >  static int s5_read_temp(struct s5_hwmon *hwmon, long *temp)
> >  {
> >       int value;
> > @@ -62,6 +100,17 @@ static int s5_read_temp(struct s5_hwmon *hwmon, long *temp)
> >       return 0;
> >  }
> >
> > +static int s5_read_fan(struct s5_hwmon *hwmon, long *val)
> > +{
> > +     u32 count;
> > +
> > +     count = readl_relaxed(hwmon->fan + FAN_CNT);
> > +     count = FIELD_GET(FAN_CNT_DATA, count);
> > +     *val = count * 60 / hwmon->pulses_per_revolution;
> > +
> > +     return 0;
> > +}
> > +
> >  static int s5_read(struct device *dev, enum hwmon_sensor_types type,
> >                  u32 attr, int channel, long *val)
> >  {
> > @@ -72,6 +121,10 @@ static int s5_read(struct device *dev, enum hwmon_sensor_types type,
> >               if (attr == hwmon_temp_input)
> >                       return s5_read_temp(hwmon, val);
> >               break;
> > +     case hwmon_fan:
> > +             if (attr == hwmon_fan_input && hwmon->fan)
> > +                     return s5_read_fan(hwmon, val);
> > +             break;
> >       default:
> >               break;
> >       }
> > @@ -82,62 +135,115 @@ static int s5_read(struct device *dev, enum hwmon_sensor_types type,
> >  static umode_t s5_is_visible(const void *_data, enum hwmon_sensor_types type,
> >                            u32 attr, int channel)
> >  {
> > -     if (type != hwmon_temp)
> > -             return 0;
> > +     const struct s5_hwmon *hwmon = _data;
> >
> > -     switch (attr) {
> > -     case hwmon_temp_input:
> > -             return 0444;
> > +     switch (type) {
> > +     case hwmon_temp:
> > +             return attr == hwmon_temp_input ? 0444 : 0;
> > +     case hwmon_fan:
> > +             if (hwmon->fan && attr == hwmon_fan_input)
> > +                     return 0444;
> > +             return 0;
> >       default:
> >               return 0;
> >       }
> >  }
> >
> > -static const struct hwmon_channel_info * const s5_info[] = {
> > +static const struct hwmon_channel_info * const s5_temp_info[] = {
> >       HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
> >       HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
> >       NULL
> >  };
> >
> > +static const struct hwmon_channel_info * const s5_fan_info[] = {
> > +     HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
> > +     NULL
> > +};
> > +
> >  static const struct hwmon_ops s5_hwmon_ops = {
> >       .is_visible = s5_is_visible,
> >       .read = s5_read,
> >  };
> >
> > -static const struct hwmon_chip_info s5_chip_info = {
> > +static const struct hwmon_chip_info s5_temp_chip_info = {
> >       .ops = &s5_hwmon_ops,
> > -     .info = s5_info,
> > +     .info = s5_temp_info,
> > +};
> > +
> > +static const struct hwmon_chip_info s5_fan_chip_info = {
> > +     .ops = &s5_hwmon_ops,
> > +     .info = s5_fan_info,
> >  };
> >
> >  static int s5_temp_probe(struct platform_device *pdev)
> >  {
> > +     const struct s5_match_data *match_data;
> > +     struct device *dev = &pdev->dev;
> >       struct device *hwmon_dev;
> >       struct s5_hwmon *hwmon;
> > +     struct device_node *fan_np __free(device_node) = NULL;
> > +     void __iomem *base;
> > +     bool is_fan;
> > +     int ret;
> >
> > -     hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon), GFP_KERNEL);
> > +     match_data = device_get_match_data(dev);
> > +     is_fan = match_data && match_data->is_fan;
> > +
> > +     hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL);
> >       if (!hwmon)
> >               return -ENOMEM;
> >
> > -     hwmon->base = devm_platform_ioremap_resource(pdev, 0);
> > -     if (IS_ERR(hwmon->base))
> > -             return PTR_ERR(hwmon->base);
> > +     base = devm_platform_ioremap_resource(pdev, 0);
> > +     if (IS_ERR(base))
> > +             return PTR_ERR(base);
> > +
> > +     if (is_fan) {
> > +             hwmon->fan = base;
> > +             hwmon->pulses_per_revolution = 2;
> > +             fan_np = of_get_available_child_by_name(dev->of_node, "fan");
> > +             if (fan_np &&
> > +                 of_property_present(fan_np, "pulses-per-revolution")) {
> > +                     ret = of_property_read_u32(fan_np, "pulses-per-revolution",
> > +                                                &hwmon->pulses_per_revolution);
> > +                     if (ret)
> > +                             return dev_err_probe(dev, ret,
> > +                                                  "invalid pulses-per-revolution\n");
> > +             }
> > +             if (!hwmon->pulses_per_revolution ||
> > +                 hwmon->pulses_per_revolution > 4)
> > +                     return dev_err_probe(dev, -EINVAL,
> > +                                          "invalid pulses-per-revolution\n");
> > +     } else {
> > +             hwmon->base = base;
> > +     }
> >
> > -     hwmon->clk = devm_clk_get_enabled(&pdev->dev, NULL);
> > +     hwmon->clk = devm_clk_get_enabled(dev, NULL);
> >       if (IS_ERR(hwmon->clk))
> >               return PTR_ERR(hwmon->clk);
> > +     hwmon->clk_rate = clk_get_rate(hwmon->clk);
> >
> > -     s5_temp_enable(hwmon);
> > +     if (is_fan)
> > +             s5_fan_enable(hwmon);
> > +     else
> > +             s5_temp_enable(hwmon);
> >
> > -     hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
> > -                                                      "s5_temp",
> > -                                                      hwmon,
> > -                                                      &s5_chip_info,
> > -                                                      NULL);
> > +     hwmon_dev = devm_hwmon_device_register_with_info(dev,
> > +                                     is_fan ? "lan969x_fan" : "s5_temp", hwmon,
> > +                                     is_fan ? &s5_fan_chip_info :
> > +                                              &s5_temp_chip_info, NULL);
> >
> >       return PTR_ERR_OR_ZERO(hwmon_dev);
> >  }
> >
> > +static const struct s5_match_data lan969x_fan_match_data = {
> > +     .is_fan = true,
> > +};
> > +
> >  static const struct of_device_id s5_temp_match[] = {
> > +     {
> > +             .compatible = "microchip,lan9691-fan",
> > +             .data = &lan969x_fan_match_data,
> > +     },
> >       { .compatible = "microchip,sparx5-temp" },
> >       {},
> >  };
> > @@ -154,5 +260,5 @@ static struct platform_driver s5_temp_driver = {
> >  module_platform_driver(s5_temp_driver);
> >
> >  MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>");
> > -MODULE_DESCRIPTION("Sparx5 SoC temperature sensor driver");
> > +MODULE_DESCRIPTION("Sparx5 SoC hardware monitoring driver");
> >  MODULE_LICENSE("GPL");



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura d.d.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

[PATCH v2 4/6] hwmon: sparx5: add LAN969x PWM support

[Robert Marko]

The LAN969x fan controller provides an 8-bit PWM output with a
programmable frequency and polarity.

Expose pwm1 and pwm1_freq on the fan hwmon device. Parse the fan child
node's standard PWM specifier and use its period and polarity flags to
configure the hardware.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v2:
- Drop the private mutex; hwmon serializes sysfs accesses.
- Preserve firmware PWM polarity when no fan child is present.
- Convert the DT period directly to a divider and reject invalid values
  before changing fan registers.

 Documentation/hwmon/sparx5-temp.rst |   9 ++
 drivers/hwmon/Kconfig               |   2 +-
 drivers/hwmon/sparx5-temp.c         | 198 ++++++++++++++++++++++++++--
 3 files changed, 196 insertions(+), 13 deletions(-)

diff --git a/Documentation/hwmon/sparx5-temp.rst b/Documentation/hwmon/sparx5-temp.rst
index a053d12fbdbbb..edb12f890172c 100644
--- a/Documentation/hwmon/sparx5-temp.rst
+++ b/Documentation/hwmon/sparx5-temp.rst
@@ -35,8 +35,17 @@ The following attributes are supported.
 ======================= ========================================================
 temp1_input		Die temperature (in millidegree Celsius.)
 fan1_input              Fan speed in RPM (LAN969x only).
+pwm1                    PWM duty cycle, from 0 to 255 (LAN969x only).
+pwm1_freq               PWM frequency in Hz (LAN969x only).
 ======================= ========================================================
 
 The tachometer counts pulses over one-second intervals, independently of the
 PWM output. The fan child's pulses-per-revolution property specifies how many
 pulses represent one revolution; it defaults to two.
+
+On LAN969x, the fan child's pwms property selects the initial PWM period and
+polarity. The output is dedicated to this fan and is not available to external
+PWM consumers. The pwm1 value describes the logical duty cycle independently
+of the output polarity. Frequency requests are rounded to the nearest
+hardware divider value; requests outside the divider range are rejected.
+pwm1_freq reports the resulting frequency.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index deb9eb82b8501..b77112518ebe6 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -673,7 +673,7 @@ config SENSORS_SPARX5
 	depends on ARCH_SPARX5 || ARCH_LAN969X || COMPILE_TEST
 	help
 	  If you say yes here you get support for temperature monitoring on
-	  Microchip Sparx5 and LAN969x SoCs, as well as fan monitoring on
+	  Microchip Sparx5 and LAN969x SoCs, as well as the fan controller on
 	  LAN969x SoCs.
 
 	  This driver can also be built as a module. If so, the module
diff --git a/drivers/hwmon/sparx5-temp.c b/drivers/hwmon/sparx5-temp.c
index 1f197004780cc..bdf08931f5b93 100644
--- a/drivers/hwmon/sparx5-temp.c
+++ b/drivers/hwmon/sparx5-temp.c
@@ -15,6 +15,8 @@
 #include <linux/platform_device.h>
 #include <linux/property.h>
 
+#include <dt-bindings/pwm/pwm.h>
+
 #define TEMP_CTRL		0
 #define TEMP_CFG		4
 #define  TEMP_CFG_CYCLES	GENMASK(24, 15)
@@ -24,14 +26,18 @@
 #define  TEMP_STAT_TEMP		GENMASK(11, 0)
 
 #define FAN_CFG			0
+#define  FAN_CFG_DUTY_CYCLE	GENMASK(23, 16)
+#define  FAN_CFG_INV_POL	BIT(3)
 #define  FAN_CFG_GATE_ENA	BIT(2)
 #define  FAN_CFG_STAT_MODE	BIT(0)
 #define FAN_PWM_FREQ		4
 #define  FAN_PWM_FREQ_CYCLES	GENMASK(27, 16)
+#define  FAN_PWM_FREQ_DIV	GENMASK(15, 0)
 #define FAN_CNT			8
 #define  FAN_CNT_DATA		GENMASK(15, 0)
 
 #define FAN_TACH_INTERVAL_HZ	100000
+#define FAN_PWM_FREQ_SCALE	256
 
 struct s5_match_data {
 	bool is_fan;
@@ -43,6 +49,8 @@ struct s5_hwmon {
 	struct clk *clk;
 	unsigned long clk_rate;
 	u32 pulses_per_revolution;
+	u32 pwm_period;
+	bool pwm_inverted;
 };
 
 static void s5_temp_enable(struct s5_hwmon *hwmon)
@@ -72,6 +80,11 @@ static void s5_fan_enable(struct s5_hwmon *hwmon)
 	/* Count all tachometer pulses over one-second intervals. */
 	val = readl(hwmon->fan + FAN_CFG);
 	val &= ~(FAN_CFG_STAT_MODE | FAN_CFG_GATE_ENA);
+	if (hwmon->pwm_period) {
+		val &= ~FAN_CFG_INV_POL;
+		if (hwmon->pwm_inverted)
+			val |= FAN_CFG_INV_POL;
+	}
 	writel(val, hwmon->fan + FAN_CFG);
 }
 
@@ -111,6 +124,32 @@ static int s5_read_fan(struct s5_hwmon *hwmon, long *val)
 	return 0;
 }
 
+static int s5_read_pwm(struct s5_hwmon *hwmon, long *val)
+{
+	u32 data;
+
+	data = readl_relaxed(hwmon->fan + FAN_CFG);
+	*val = FIELD_GET(FAN_CFG_DUTY_CYCLE, data);
+
+	return 0;
+}
+
+static int s5_read_pwm_freq(struct s5_hwmon *hwmon, long *val)
+{
+	unsigned long rate;
+	u32 divider;
+
+	divider = readl_relaxed(hwmon->fan + FAN_PWM_FREQ);
+	divider = FIELD_GET(FAN_PWM_FREQ_DIV, divider);
+	if (!divider)
+		return -ENODATA;
+
+	rate = DIV_ROUND_CLOSEST(hwmon->clk_rate, FAN_PWM_FREQ_SCALE);
+	*val = DIV_ROUND_CLOSEST(rate, divider);
+
+	return 0;
+}
+
 static int s5_read(struct device *dev, enum hwmon_sensor_types type,
 		   u32 attr, int channel, long *val)
 {
@@ -125,6 +164,18 @@ static int s5_read(struct device *dev, enum hwmon_sensor_types type,
 		if (attr == hwmon_fan_input && hwmon->fan)
 			return s5_read_fan(hwmon, val);
 		break;
+	case hwmon_pwm:
+		if (!hwmon->fan)
+			break;
+		switch (attr) {
+		case hwmon_pwm_input:
+			return s5_read_pwm(hwmon, val);
+		case hwmon_pwm_freq:
+			return s5_read_pwm_freq(hwmon, val);
+		default:
+			break;
+		}
+		break;
 	default:
 		break;
 	}
@@ -132,6 +183,85 @@ static int s5_read(struct device *dev, enum hwmon_sensor_types type,
 	return -EOPNOTSUPP;
 }
 
+static int s5_write_pwm(struct s5_hwmon *hwmon, long val)
+{
+	u32 data;
+
+	if (val < 0 || val > 255)
+		return -EINVAL;
+
+	data = readl_relaxed(hwmon->fan + FAN_CFG);
+	data &= ~FAN_CFG_DUTY_CYCLE;
+	data |= FIELD_PREP(FAN_CFG_DUTY_CYCLE, val);
+	writel_relaxed(data, hwmon->fan + FAN_CFG);
+
+	return 0;
+}
+
+static void s5_write_pwm_divider(struct s5_hwmon *hwmon, u32 divider)
+{
+	u32 data;
+
+	data = readl_relaxed(hwmon->fan + FAN_PWM_FREQ);
+	data &= ~FAN_PWM_FREQ_DIV;
+	data |= FIELD_PREP(FAN_PWM_FREQ_DIV, divider);
+	writel_relaxed(data, hwmon->fan + FAN_PWM_FREQ);
+}
+
+static int s5_write_pwm_period(struct s5_hwmon *hwmon)
+{
+	unsigned long rate;
+	u64 divider;
+
+	rate = DIV_ROUND_CLOSEST(hwmon->clk_rate, FAN_PWM_FREQ_SCALE);
+	divider = DIV_ROUND_CLOSEST_ULL((u64)rate * hwmon->pwm_period,
+					NSEC_PER_SEC);
+	if (!divider || divider > FIELD_MAX(FAN_PWM_FREQ_DIV))
+		return -ERANGE;
+
+	s5_write_pwm_divider(hwmon, divider);
+
+	return 0;
+}
+
+static int s5_write_pwm_freq(struct s5_hwmon *hwmon, long val)
+{
+	unsigned long rate, divider;
+
+	if (val <= 0)
+		return -EINVAL;
+
+	rate = DIV_ROUND_CLOSEST(hwmon->clk_rate, FAN_PWM_FREQ_SCALE);
+	if (val < DIV_ROUND_CLOSEST(rate, FIELD_MAX(FAN_PWM_FREQ_DIV)) ||
+	    val > rate)
+		return -ERANGE;
+
+	divider = DIV_ROUND_CLOSEST(rate, val);
+	divider = min_t(unsigned long, divider, FIELD_MAX(FAN_PWM_FREQ_DIV));
+
+	s5_write_pwm_divider(hwmon, divider);
+
+	return 0;
+}
+
+static int s5_write(struct device *dev, enum hwmon_sensor_types type,
+		    u32 attr, int channel, long val)
+{
+	struct s5_hwmon *hwmon = dev_get_drvdata(dev);
+
+	if (type != hwmon_pwm || !hwmon->fan)
+		return -EOPNOTSUPP;
+
+	switch (attr) {
+	case hwmon_pwm_input:
+		return s5_write_pwm(hwmon, val);
+	case hwmon_pwm_freq:
+		return s5_write_pwm_freq(hwmon, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static umode_t s5_is_visible(const void *_data, enum hwmon_sensor_types type,
 			     u32 attr, int channel)
 {
@@ -144,6 +274,12 @@ static umode_t s5_is_visible(const void *_data, enum hwmon_sensor_types type,
 		if (hwmon->fan && attr == hwmon_fan_input)
 			return 0444;
 		return 0;
+	case hwmon_pwm:
+		if (!hwmon->fan)
+			return 0;
+		if (attr == hwmon_pwm_input || attr == hwmon_pwm_freq)
+			return 0644;
+		return 0;
 	default:
 		return 0;
 	}
@@ -157,12 +293,14 @@ static const struct hwmon_channel_info * const s5_temp_info[] = {
 
 static const struct hwmon_channel_info * const s5_fan_info[] = {
 	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
+	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_INPUT | HWMON_PWM_FREQ),
 	NULL
 };
 
 static const struct hwmon_ops s5_hwmon_ops = {
 	.is_visible = s5_is_visible,
 	.read = s5_read,
+	.write = s5_write,
 };
 
 static const struct hwmon_chip_info s5_temp_chip_info = {
@@ -175,6 +313,42 @@ static const struct hwmon_chip_info s5_fan_chip_info = {
 	.info = s5_fan_info,
 };
 
+static int s5_get_fan_data(struct device *dev, struct device_node *fan_np,
+			   struct s5_hwmon *hwmon)
+{
+	struct of_phandle_args args;
+	int ret;
+
+	ret = of_parse_phandle_with_args(fan_np, "pwms", "#pwm-cells", 0,
+					 &args);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to parse fan PWM\n");
+
+	if (args.np != dev->of_node || args.args_count != 2 || !args.args[0] ||
+	    args.args[1] & ~PWM_POLARITY_INVERTED) {
+		of_node_put(args.np);
+		return dev_err_probe(dev, -EINVAL, "invalid fan PWM specifier\n");
+	}
+
+	hwmon->pwm_period = args.args[0];
+	hwmon->pwm_inverted = args.args[1] & PWM_POLARITY_INVERTED;
+	of_node_put(args.np);
+
+	if (of_property_present(fan_np, "pulses-per-revolution")) {
+		ret = of_property_read_u32(fan_np, "pulses-per-revolution",
+					   &hwmon->pulses_per_revolution);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "invalid pulses-per-revolution\n");
+	}
+	if (!hwmon->pulses_per_revolution ||
+	    hwmon->pulses_per_revolution > 4)
+		return dev_err_probe(dev, -EINVAL,
+				     "invalid pulses-per-revolution\n");
+
+	return 0;
+}
+
 static int s5_temp_probe(struct platform_device *pdev)
 {
 	const struct s5_match_data *match_data;
@@ -201,18 +375,11 @@ static int s5_temp_probe(struct platform_device *pdev)
 		hwmon->fan = base;
 		hwmon->pulses_per_revolution = 2;
 		fan_np = of_get_available_child_by_name(dev->of_node, "fan");
-		if (fan_np &&
-		    of_property_present(fan_np, "pulses-per-revolution")) {
-			ret = of_property_read_u32(fan_np, "pulses-per-revolution",
-						   &hwmon->pulses_per_revolution);
+		if (fan_np) {
+			ret = s5_get_fan_data(dev, fan_np, hwmon);
 			if (ret)
-				return dev_err_probe(dev, ret,
-						     "invalid pulses-per-revolution\n");
+				return ret;
 		}
-		if (!hwmon->pulses_per_revolution ||
-		    hwmon->pulses_per_revolution > 4)
-			return dev_err_probe(dev, -EINVAL,
-					     "invalid pulses-per-revolution\n");
 	} else {
 		hwmon->base = base;
 	}
@@ -222,10 +389,17 @@ static int s5_temp_probe(struct platform_device *pdev)
 		return PTR_ERR(hwmon->clk);
 	hwmon->clk_rate = clk_get_rate(hwmon->clk);
 
-	if (is_fan)
+	if (is_fan) {
+		if (hwmon->pwm_period) {
+			ret = s5_write_pwm_period(hwmon);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "invalid fan PWM period\n");
+		}
 		s5_fan_enable(hwmon);
-	else
+	} else {
 		s5_temp_enable(hwmon);
+	}
 
 	hwmon_dev = devm_hwmon_device_register_with_info(dev,
 					is_fan ? "lan969x_fan" : "s5_temp", hwmon,
-- 
2.56.0

[PATCH v2 5/6] hwmon: sparx5: register LAN969x fan as a cooling device

[Robert Marko]

Allow the LAN969x hardware fan controller to be used by thermal-zone
cooling maps when cooling-levels are provided on the fan child node.

Translate cooling states to PWM duty-cycle values and keep the cooling
state synchronized with direct pwm1 writes. Register the cooling device
against the fan child so thermal references resolve to the correct node.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v2:
- Use hwmon_lock() for cooling callbacks instead of a private mutex.

 Documentation/hwmon/sparx5-temp.rst |  14 +++
 drivers/hwmon/sparx5-temp.c         | 145 ++++++++++++++++++++++++++--
 2 files changed, 150 insertions(+), 9 deletions(-)

diff --git a/Documentation/hwmon/sparx5-temp.rst b/Documentation/hwmon/sparx5-temp.rst
index edb12f890172c..522707ed5a5f5 100644
--- a/Documentation/hwmon/sparx5-temp.rst
+++ b/Documentation/hwmon/sparx5-temp.rst
@@ -49,3 +49,17 @@ PWM consumers. The pwm1 value describes the logical duty cycle independently
 of the output polarity. Frequency requests are rounded to the nearest
 hardware divider value; requests outside the divider range are rejected.
 pwm1_freq reports the resulting frequency.
+
+Thermal cooling
+---------------
+
+With device-tree thermal support enabled, the LAN969x fan can also act as a
+thermal cooling device. The fan child's cooling-levels property maps cooling
+states to PWM values from 0 to 255, in ascending order. When cooling levels
+are provided, the driver initially selects the highest level.
+
+Manual pwm1 writes remain available and update the reported cooling state
+to the highest state whose PWM value does not exceed the requested value,
+or state zero if the value is below the first level. Subsequent thermal
+cooling requests can override manual PWM settings. Each cooling request
+programs the exact PWM value associated with the requested state.
diff --git a/drivers/hwmon/sparx5-temp.c b/drivers/hwmon/sparx5-temp.c
index bdf08931f5b93..caa3fad367684 100644
--- a/drivers/hwmon/sparx5-temp.c
+++ b/drivers/hwmon/sparx5-temp.c
@@ -14,6 +14,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
+#include <linux/thermal.h>
 
 #include <dt-bindings/pwm/pwm.h>
 
@@ -47,10 +48,14 @@ struct s5_hwmon {
 	void __iomem *base;
 	void __iomem *fan;
 	struct clk *clk;
+	struct device *hwmon_dev;
 	unsigned long clk_rate;
 	u32 pulses_per_revolution;
 	u32 pwm_period;
 	bool pwm_inverted;
+	u32 *cooling_levels;
+	unsigned long cooling_state;
+	unsigned long max_cooling_state;
 };
 
 static void s5_temp_enable(struct s5_hwmon *hwmon)
@@ -124,6 +129,20 @@ static int s5_read_fan(struct s5_hwmon *hwmon, long *val)
 	return 0;
 }
 
+static void s5_update_cooling_state(struct s5_hwmon *hwmon, u32 pwm)
+{
+	unsigned long state;
+
+	if (!hwmon->cooling_levels)
+		return;
+
+	for (state = 0; state < hwmon->max_cooling_state; state++)
+		if (pwm < hwmon->cooling_levels[state + 1])
+			break;
+
+	hwmon->cooling_state = state;
+}
+
 static int s5_read_pwm(struct s5_hwmon *hwmon, long *val)
 {
 	u32 data;
@@ -183,17 +202,23 @@ static int s5_read(struct device *dev, enum hwmon_sensor_types type,
 	return -EOPNOTSUPP;
 }
 
-static int s5_write_pwm(struct s5_hwmon *hwmon, long val)
+static void s5_set_pwm(struct s5_hwmon *hwmon, u32 pwm)
 {
 	u32 data;
 
-	if (val < 0 || val > 255)
-		return -EINVAL;
-
 	data = readl_relaxed(hwmon->fan + FAN_CFG);
 	data &= ~FAN_CFG_DUTY_CYCLE;
-	data |= FIELD_PREP(FAN_CFG_DUTY_CYCLE, val);
+	data |= FIELD_PREP(FAN_CFG_DUTY_CYCLE, pwm);
 	writel_relaxed(data, hwmon->fan + FAN_CFG);
+}
+
+static int s5_write_pwm(struct s5_hwmon *hwmon, long val)
+{
+	if (val < 0 || val > 255)
+		return -EINVAL;
+
+	s5_set_pwm(hwmon, val);
+	s5_update_cooling_state(hwmon, val);
 
 	return 0;
 }
@@ -313,6 +338,89 @@ static const struct hwmon_chip_info s5_fan_chip_info = {
 	.info = s5_fan_info,
 };
 
+static int s5_cooling_get_max_state(struct thermal_cooling_device *cdev,
+				    unsigned long *state)
+{
+	struct s5_hwmon *hwmon = cdev->devdata;
+
+	*state = hwmon->max_cooling_state;
+
+	return 0;
+}
+
+static int s5_cooling_get_cur_state(struct thermal_cooling_device *cdev,
+				    unsigned long *state)
+{
+	struct s5_hwmon *hwmon = cdev->devdata;
+
+	hwmon_lock(hwmon->hwmon_dev);
+	*state = hwmon->cooling_state;
+	hwmon_unlock(hwmon->hwmon_dev);
+
+	return 0;
+}
+
+static int s5_cooling_set_cur_state(struct thermal_cooling_device *cdev,
+				    unsigned long state)
+{
+	struct s5_hwmon *hwmon = cdev->devdata;
+
+	if (state > hwmon->max_cooling_state)
+		return -EINVAL;
+
+	hwmon_lock(hwmon->hwmon_dev);
+	s5_set_pwm(hwmon, hwmon->cooling_levels[state]);
+	hwmon->cooling_state = state;
+	hwmon_unlock(hwmon->hwmon_dev);
+
+	return 0;
+}
+
+static const struct thermal_cooling_device_ops s5_cooling_ops = {
+	.get_max_state = s5_cooling_get_max_state,
+	.get_cur_state = s5_cooling_get_cur_state,
+	.set_cur_state = s5_cooling_set_cur_state,
+};
+
+static int s5_get_cooling_data(struct device *dev, struct device_node *fan_np,
+			       struct s5_hwmon *hwmon)
+{
+	u32 *levels;
+	int count, i, ret;
+
+	if (!of_property_present(fan_np, "cooling-levels"))
+		return 0;
+
+	count = of_property_count_u32_elems(fan_np, "cooling-levels");
+	if (count <= 0)
+		return dev_err_probe(dev, count ? : -EINVAL,
+				     "invalid cooling-levels property\n");
+
+	levels = devm_kcalloc(dev, count, sizeof(*levels), GFP_KERNEL);
+	if (!levels)
+		return -ENOMEM;
+	hwmon->cooling_levels = levels;
+
+	ret = of_property_read_u32_array(fan_np, "cooling-levels",
+					 hwmon->cooling_levels, count);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to read cooling-levels\n");
+
+	for (i = 0; i < count; i++) {
+		if (hwmon->cooling_levels[i] > 255)
+			return dev_err_probe(dev, -EINVAL,
+					     "cooling level %d exceeds 255\n", i);
+		if (i && hwmon->cooling_levels[i] < hwmon->cooling_levels[i - 1])
+			return dev_err_probe(dev, -EINVAL,
+					     "cooling levels must be ascending\n");
+	}
+
+	hwmon->max_cooling_state = count - 1;
+
+	return 0;
+}
+
 static int s5_get_fan_data(struct device *dev, struct device_node *fan_np,
 			   struct s5_hwmon *hwmon)
 {
@@ -346,18 +454,19 @@ static int s5_get_fan_data(struct device *dev, struct device_node *fan_np,
 		return dev_err_probe(dev, -EINVAL,
 				     "invalid pulses-per-revolution\n");
 
-	return 0;
+	return s5_get_cooling_data(dev, fan_np, hwmon);
 }
 
 static int s5_temp_probe(struct platform_device *pdev)
 {
 	const struct s5_match_data *match_data;
+	struct thermal_cooling_device *cdev;
 	struct device *dev = &pdev->dev;
-	struct device *hwmon_dev;
 	struct s5_hwmon *hwmon;
 	struct device_node *fan_np __free(device_node) = NULL;
 	void __iomem *base;
 	bool is_fan;
+	u32 initial_pwm;
 	int ret;
 
 	match_data = device_get_match_data(dev);
@@ -397,16 +506,34 @@ static int s5_temp_probe(struct platform_device *pdev)
 						     "invalid fan PWM period\n");
 		}
 		s5_fan_enable(hwmon);
+
+		if (hwmon->cooling_levels) {
+			initial_pwm = hwmon->cooling_levels[hwmon->max_cooling_state];
+			ret = s5_write_pwm(hwmon, initial_pwm);
+			if (ret)
+				return ret;
+		}
 	} else {
 		s5_temp_enable(hwmon);
 	}
 
-	hwmon_dev = devm_hwmon_device_register_with_info(dev,
+	hwmon->hwmon_dev = devm_hwmon_device_register_with_info(dev,
 					is_fan ? "lan969x_fan" : "s5_temp", hwmon,
 					is_fan ? &s5_fan_chip_info :
 						 &s5_temp_chip_info, NULL);
+	if (IS_ERR(hwmon->hwmon_dev))
+		return PTR_ERR(hwmon->hwmon_dev);
+
+	if (IS_ENABLED(CONFIG_THERMAL_OF) && hwmon->cooling_levels) {
+		cdev = devm_thermal_of_child_cooling_device_register(dev, fan_np,
+								     "lan969x-fan", hwmon,
+								     &s5_cooling_ops);
+		if (IS_ERR(cdev))
+			return dev_err_probe(dev, PTR_ERR(cdev),
+					     "failed to register cooling device\n");
+	}
 
-	return PTR_ERR_OR_ZERO(hwmon_dev);
+	return 0;
 }
 
 static const struct s5_match_data lan969x_fan_match_data = {
-- 
2.56.0

[PATCH v2 6/6] arm64: dts: microchip: lan969x: add fan controller

[Robert Marko]

LAN969x provides a PWM fan output and tachometer input in a register
block separate from the temperature sensor. Describe it as its own
fan-controller node and keep the temperature sensor compatible.

Connect the EV23X71A fan to the controller and route its pins. Use a
25 kHz normal-polarity PWM for the board low-side FET. Set cooling
levels so the fan starts at full speed.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v2:
- Split the fan controller from the temperature node.
- Add the EV23X71A fan child with board-specific PWM and cooling levels.

 arch/arm64/boot/dts/microchip/lan9691.dtsi         | 7 +++++++
 arch/arm64/boot/dts/microchip/lan9696-ev23x71a.dts | 8 +++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/microchip/lan9691.dtsi b/arch/arm64/boot/dts/microchip/lan9691.dtsi
index 52c22dd1a15d5..039c2014aa341 100644
--- a/arch/arm64/boot/dts/microchip/lan9691.dtsi
+++ b/arch/arm64/boot/dts/microchip/lan9691.dtsi
@@ -522,6 +522,13 @@ sgpio_out: gpio@1 {
 			};
 		};
 
+		fanctrl: fan-controller@e2010348 {
+			compatible = "microchip,lan9691-fan";
+			reg = <0xe2010348 0xc>;
+			clocks = <&fx100_clk>;
+			#pwm-cells = <2>;
+		};
+
 		tmon: hwmon@e2020100 {
 			compatible = "microchip,lan9691-temp", "microchip,sparx5-temp";
 			reg = <0xe2020100 0xc>;
diff --git a/arch/arm64/boot/dts/microchip/lan9696-ev23x71a.dts b/arch/arm64/boot/dts/microchip/lan9696-ev23x71a.dts
index abb21a8bace9e..3cbe5beebce1d 100644
--- a/arch/arm64/boot/dts/microchip/lan9696-ev23x71a.dts
+++ b/arch/arm64/boot/dts/microchip/lan9696-ev23x71a.dts
@@ -775,9 +775,15 @@ port29: port@29 {
 	};
 };
 
-&tmon {
+&fanctrl {
 	pinctrl-0 = <&fan_pins>;
 	pinctrl-names = "default";
+
+	fan {
+		pwms = <&fanctrl 40000 0>;
+		cooling-levels = <0 128 192 255>;
+		#cooling-cells = <2>;
+	};
 };
 
 &usart0 {
-- 
2.56.0

Re: [PATCH v2 1/6] dt-bindings: hwmon: add Microchip LAN969x fan controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4403 bytes --]

On Mon, Nov 09, 2026 at 02:35:05PM +0100, Robert Marko wrote:
> The LAN969x fan controller has a PWM output and tachometer input in a
> register block separate from the Sparx5-compatible temperature sensor.
> Describe it with its own compatible and binding.
> 
> The optional fan child specifies PWM period and polarity, tachometer
> pulses per revolution, and thermal cooling levels.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> ---
> Changes in v2:
> - Describe the fan controller in a separate binding and compatible.
> - Keep the existing temperature-sensor binding unchanged.

So, I think that this device has been modelled incorrectly from the
outset. "devcpu_gcb" should probably be a syscon/simplemfd, as least
going off of what I see in
https://microchip-ung.github.io/lan969x-automotive_reginfo/reginfo_LAN969x-Automotive.html?select=devcpu_gcb
and the fact that we see silly offsets in the dts:
		reset: reset-controller@e201000c {
			reg = <0xe201000c 0x4>;
		};

		gpio: pinctrl@e20100d4 {
			reg = <0xe20100d4 0xd4>,
			      <0xe2010370 0xa8>;
		};

		mdio0: mdio@e20101a8 {
			reg = <0xe20101a8 0x24>;
		};

		mdio1: mdio@e20101cc {
			reg = <0xe20101cc 0x24>;
		};

		sgpio: gpio@e2010230 {
			reg = <0xe2010230 0x118>;
		};
That ship has sailed I think though, so I'm not going to ask you to
rework this whole device.

> 
>  .../bindings/hwmon/microchip,lan9691-fan.yaml | 83 +++++++++++++++++++
>  1 file changed, 83 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml b/Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml
> new file mode 100644
> index 0000000000000..0b7d38a6d20ae
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/microchip,lan9691-fan.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip LAN969x Fan Controller
> +
> +maintainers:
> +  - Robert Marko <robert.marko@sartura.hr>
> +
> +description: |
> +  LAN969x fan controller with a PWM output and tachometer input. The fan
> +  child's pwms property references this node to specify the PWM period and
> +  polarity.
> +
> +properties:
> +  compatible:
> +    const: microchip,lan9691-fan
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: System reference clock
> +
> +  '#pwm-cells':
> +    const: 2
> +    description: |
> +      Number of cells in a PWM specifier.
> +      - cell 0: PWM period in nanoseconds
> +      - cell 1: PWM polarity: 0 or PWM_POLARITY_INVERTED
> +
> +  fan:
> +    $ref: fan-common.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      cooling-levels:
> +        description:
> +          PWM duty-cycle values corresponding to thermal cooling states.
> +        items:
> +          maximum: 255
> +
> +      pulses-per-revolution:
> +        minimum: 1
> +        maximum: 4
> +        default: 2
> +
> +    required:
> +      - pwms
> +
> +    dependencies:
> +      '#cooling-cells':
> +        - cooling-levels
> +      cooling-levels:
> +        - '#cooling-cells'
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - '#pwm-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/pwm/pwm.h>
> +
> +    fanctrl: fan-controller@e2010348 {
> +        compatible = "microchip,lan9691-fan";
> +        reg = <0xe2010348 0xc>;

btw, how do I translate the addresses you have here to those in
https://microchip-ung.github.io/lan969x-automotive_reginfo/reginfo_LAN969x-Automotive.html?select=devcpu_gcb,fan_ctrl
? Or are you looking at a register map in a different form?

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

Cheers,
Conor.

> +        clocks = <&sys_clk>;
> +        #pwm-cells = <2>;
> +
> +        fan {
> +            pwms = <&fanctrl 40000 PWM_POLARITY_INVERTED>;
> +            cooling-levels = <0 128 192 255>;
> +            #cooling-cells = <2>;
> +        };
> +    };
> -- 
> 2.56.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/6] dt-bindings: hwmon: add Microchip LAN969x fan controller

[Robert Marko]

On Mon, Nov 9, 2026 at 8:14 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Mon, Nov 09, 2026 at 02:35:05PM +0100, Robert Marko wrote:
> > The LAN969x fan controller has a PWM output and tachometer input in a
> > register block separate from the Sparx5-compatible temperature sensor.
> > Describe it with its own compatible and binding.
> >
> > The optional fan child specifies PWM period and polarity, tachometer
> > pulses per revolution, and thermal cooling levels.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > ---
> > Changes in v2:
> > - Describe the fan controller in a separate binding and compatible.
> > - Keep the existing temperature-sensor binding unchanged.
>
> So, I think that this device has been modelled incorrectly from the
> outset. "devcpu_gcb" should probably be a syscon/simplemfd, as least
> going off of what I see in
> https://microchip-ung.github.io/lan969x-automotive_reginfo/reginfo_LAN969x-Automotive.html?select=devcpu_gcb
> and the fact that we see silly offsets in the dts:
>                 reset: reset-controller@e201000c {
>                         reg = <0xe201000c 0x4>;
>                 };
>
>                 gpio: pinctrl@e20100d4 {
>                         reg = <0xe20100d4 0xd4>,
>                               <0xe2010370 0xa8>;
>                 };
>
>                 mdio0: mdio@e20101a8 {
>                         reg = <0xe20101a8 0x24>;
>                 };
>
>                 mdio1: mdio@e20101cc {
>                         reg = <0xe20101cc 0x24>;
>                 };
>
>                 sgpio: gpio@e2010230 {
>                         reg = <0xe2010230 0x118>;
>                 };
> That ship has sailed I think though, so I'm not going to ask you to
> rework this whole device.
>
> >
> >  .../bindings/hwmon/microchip,lan9691-fan.yaml | 83 +++++++++++++++++++
> >  1 file changed, 83 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml b/Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml
> > new file mode 100644
> > index 0000000000000..0b7d38a6d20ae
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml
> > @@ -0,0 +1,83 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hwmon/microchip,lan9691-fan.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Microchip LAN969x Fan Controller
> > +
> > +maintainers:
> > +  - Robert Marko <robert.marko@sartura.hr>
> > +
> > +description: |
> > +  LAN969x fan controller with a PWM output and tachometer input. The fan
> > +  child's pwms property references this node to specify the PWM period and
> > +  polarity.
> > +
> > +properties:
> > +  compatible:
> > +    const: microchip,lan9691-fan
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: System reference clock
> > +
> > +  '#pwm-cells':
> > +    const: 2
> > +    description: |
> > +      Number of cells in a PWM specifier.
> > +      - cell 0: PWM period in nanoseconds
> > +      - cell 1: PWM polarity: 0 or PWM_POLARITY_INVERTED
> > +
> > +  fan:
> > +    $ref: fan-common.yaml#
> > +    unevaluatedProperties: false
> > +
> > +    properties:
> > +      cooling-levels:
> > +        description:
> > +          PWM duty-cycle values corresponding to thermal cooling states.
> > +        items:
> > +          maximum: 255
> > +
> > +      pulses-per-revolution:
> > +        minimum: 1
> > +        maximum: 4
> > +        default: 2
> > +
> > +    required:
> > +      - pwms
> > +
> > +    dependencies:
> > +      '#cooling-cells':
> > +        - cooling-levels
> > +      cooling-levels:
> > +        - '#cooling-cells'
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - '#pwm-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/pwm/pwm.h>
> > +
> > +    fanctrl: fan-controller@e2010348 {
> > +        compatible = "microchip,lan9691-fan";
> > +        reg = <0xe2010348 0xc>;
>
> btw, how do I translate the addresses you have here to those in
> https://microchip-ung.github.io/lan969x-automotive_reginfo/reginfo_LAN969x-Automotive.html?select=devcpu_gcb,fan_ctrl
> ? Or are you looking at a register map in a different form?

Hi Conor,
Microchip has cml-utils, which allows exporting the registers and
masks automatically, its part of the public SDK.

Or you can get the address from the register map you linked, its just
posted as word addresses, so multiply by 4.
So for DEVCPU_GCB:FAN_CTRL
- default offset: 0x38800000
- DEVCPU_GCB target offset: 0x4000
- FAN_CTRL base = 0xd2

And you get:
(0x38800000 + 0x4000 + 0xd2) × 4 = 0xe2010348

Or you can just task your LLM agent to do the math since its quite annoying.

Regards,
Robert

>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> pw-bot: not-applicable
>
> Cheers,
> Conor.
>
> > +        clocks = <&sys_clk>;
> > +        #pwm-cells = <2>;
> > +
> > +        fan {
> > +            pwms = <&fanctrl 40000 PWM_POLARITY_INVERTED>;
> > +            cooling-levels = <0 128 192 255>;
> > +            #cooling-cells = <2>;
> > +        };
> > +    };
> > --
> > 2.56.0
> >



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura d.d.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH v2 1/6] dt-bindings: hwmon: add Microchip LAN969x fan controller

[Rob Herring]

On Mon, Nov 09, 2026 at 02:35:05PM +0100, Robert Marko wrote:
> The LAN969x fan controller has a PWM output and tachometer input in a
> register block separate from the Sparx5-compatible temperature sensor.
> Describe it with its own compatible and binding.
> 
> The optional fan child specifies PWM period and polarity, tachometer
> pulses per revolution, and thermal cooling levels.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> ---
> Changes in v2:
> - Describe the fan controller in a separate binding and compatible.
> - Keep the existing temperature-sensor binding unchanged.
> 
>  .../bindings/hwmon/microchip,lan9691-fan.yaml | 83 +++++++++++++++++++
>  1 file changed, 83 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml b/Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml
> new file mode 100644
> index 0000000000000..0b7d38a6d20ae
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/microchip,lan9691-fan.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip LAN969x Fan Controller
> +
> +maintainers:
> +  - Robert Marko <robert.marko@sartura.hr>
> +
> +description: |

Don't need '|'

> +  LAN969x fan controller with a PWM output and tachometer input. The fan
> +  child's pwms property references this node to specify the PWM period and
> +  polarity.
> +
> +properties:
> +  compatible:
> +    const: microchip,lan9691-fan
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: System reference clock
> +
> +  '#pwm-cells':
> +    const: 2
> +    description: |
> +      Number of cells in a PWM specifier.
> +      - cell 0: PWM period in nanoseconds
> +      - cell 1: PWM polarity: 0 or PWM_POLARITY_INVERTED
> +
> +  fan:
> +    $ref: fan-common.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      cooling-levels:
> +        description:
> +          PWM duty-cycle values corresponding to thermal cooling states.
> +        items:
> +          maximum: 255
> +
> +      pulses-per-revolution:
> +        minimum: 1
> +        maximum: 4

Drop. That's already the max in fan-common.yaml. minimum should be added 
there too as 0 would never make sense.

> +        default: 2
> +
> +    required:
> +      - pwms
> +
> +    dependencies:
> +      '#cooling-cells':
> +        - cooling-levels
> +      cooling-levels:
> +        - '#cooling-cells'

The dependencies should also go in fan-common.yaml. Perhaps 
cooling-levels without #cooling-cells might make sense sometimes?

Rob

Re: [PATCH v2 1/6] dt-bindings: hwmon: add Microchip LAN969x fan controller

[Robert Marko]

On Tue, Nov 10, 2026 at 3:54 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Nov 09, 2026 at 02:35:05PM +0100, Robert Marko wrote:
> > The LAN969x fan controller has a PWM output and tachometer input in a
> > register block separate from the Sparx5-compatible temperature sensor.
> > Describe it with its own compatible and binding.
> >
> > The optional fan child specifies PWM period and polarity, tachometer
> > pulses per revolution, and thermal cooling levels.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > ---
> > Changes in v2:
> > - Describe the fan controller in a separate binding and compatible.
> > - Keep the existing temperature-sensor binding unchanged.
> >
> >  .../bindings/hwmon/microchip,lan9691-fan.yaml | 83 +++++++++++++++++++
> >  1 file changed, 83 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml b/Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml
> > new file mode 100644
> > index 0000000000000..0b7d38a6d20ae
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/microchip,lan9691-fan.yaml
> > @@ -0,0 +1,83 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hwmon/microchip,lan9691-fan.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Microchip LAN969x Fan Controller
> > +
> > +maintainers:
> > +  - Robert Marko <robert.marko@sartura.hr>
> > +
> > +description: |
>
> Don't need '|'

Hi Rob,

Got it, will drop in v3.

>
> > +  LAN969x fan controller with a PWM output and tachometer input. The fan
> > +  child's pwms property references this node to specify the PWM period and
> > +  polarity.
> > +
> > +properties:
> > +  compatible:
> > +    const: microchip,lan9691-fan
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: System reference clock
> > +
> > +  '#pwm-cells':
> > +    const: 2
> > +    description: |
> > +      Number of cells in a PWM specifier.
> > +      - cell 0: PWM period in nanoseconds
> > +      - cell 1: PWM polarity: 0 or PWM_POLARITY_INVERTED
> > +
> > +  fan:
> > +    $ref: fan-common.yaml#
> > +    unevaluatedProperties: false
> > +
> > +    properties:
> > +      cooling-levels:
> > +        description:
> > +          PWM duty-cycle values corresponding to thermal cooling states.
> > +        items:
> > +          maximum: 255
> > +
> > +      pulses-per-revolution:
> > +        minimum: 1
> > +        maximum: 4
>
> Drop. That's already the max in fan-common.yaml. minimum should be added
> there too as 0 would never make sense.

Ok, will add 1 as minimum in fan-common.yaml as a separate commit in v3.

>
> > +        default: 2
> > +
> > +    required:
> > +      - pwms
> > +
> > +    dependencies:
> > +      '#cooling-cells':
> > +        - cooling-levels
> > +      cooling-levels:
> > +        - '#cooling-cells'
>
> The dependencies should also go in fan-common.yaml. Perhaps
> cooling-levels without #cooling-cells might make sense sometimes?

I removed cooling-levels requiring #cooling-cells, as it can
technically work without it, its not ideal, but it works.

I am not sure if its feasible for fan-common.yaml though as we already
have schemas for EMC2305, and more importantly
DTS-es for freescale/imx95-tqma9596la-mba95xxca.dts and
rockchip/rk3568-radxa-cm3j-rpi-cm4.dts
that have #cooling-cells without cooling-levels.
EMC2305 driver even calculates cooling states without a levels table.

I would appreciate guidance on how to proceed with this.

Regards,
Robert

>
> Rob



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura d.d.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH v2 1/6] dt-bindings: hwmon: add Microchip LAN969x fan controller

[Rob Herring]

On Wed, Nov 11, 2026 at 01:21:56PM +0100, Robert Marko wrote:
> On Tue, Nov 10, 2026 at 3:54 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, Nov 09, 2026 at 02:35:05PM +0100, Robert Marko wrote:
> > > The LAN969x fan controller has a PWM output and tachometer input in a
> > > register block separate from the Sparx5-compatible temperature sensor.
> > > Describe it with its own compatible and binding.
> > >
> > > The optional fan child specifies PWM period and polarity, tachometer
> > > pulses per revolution, and thermal cooling levels.
> > >
> > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > > ---
> > > Changes in v2:
> > > - Describe the fan controller in a separate binding and compatible.
> > > - Keep the existing temperature-sensor binding unchanged.

[...]

> > > +    dependencies:
> > > +      '#cooling-cells':
> > > +        - cooling-levels
> > > +      cooling-levels:
> > > +        - '#cooling-cells'
> >
> > The dependencies should also go in fan-common.yaml. Perhaps
> > cooling-levels without #cooling-cells might make sense sometimes?
> 
> I removed cooling-levels requiring #cooling-cells, as it can
> technically work without it, its not ideal, but it works.
> 
> I am not sure if its feasible for fan-common.yaml though as we already
> have schemas for EMC2305, and more importantly
> DTS-es for freescale/imx95-tqma9596la-mba95xxca.dts and
> rockchip/rk3568-radxa-cm3j-rpi-cm4.dts
> that have #cooling-cells without cooling-levels.
> EMC2305 driver even calculates cooling states without a levels table.
> 
> I would appreciate guidance on how to proceed with this.

Okay. Leave it as-is.

Rob