[PATCH hwmon-next v2 0/3] hwmon: (mlxreg-fan) Add support for multiply PWM and extend number of tachometers

[PATCH hwmon-next v2 0/3] hwmon: (mlxreg-fan) Add support for multiply PWM and extend number of tachometers

[Vadim Pasternak]

Extend the number of supported tachometers.
Add support for additional PWM and cooling devices.

Patch set contains:
Patch #1 - extends number of tachometers.
Patch #2 - extends number of PWM devices.
Patch #3 - extend number of cooling devices for thermal zone bindings.

Vadim Pasternak (3):
  hwmon: (mlxreg-fan) Extend the maximum number of tachometers
  hwmon: (mlxreg-fan) Extend driver to support multiply PWM
  hwmon: (mlxreg-fan) Extend driver to support multiply cooling devices

 drivers/hwmon/mlxreg-fan.c | 129 ++++++++++++++++++++++++++-----------
 1 file changed, 93 insertions(+), 36 deletions(-)

-- 
2.20.1

[PATCH hwmon-next v2 1/3] hwmon: (mlxreg-fan) Extend the maximum number of tachometers

[Vadim Pasternak]

Extend support of maximum tachometers from 12 to 14 in order to support
new systems, equipped with more fans.

Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
---
 drivers/hwmon/mlxreg-fan.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
index 89fe7b9fe26b..0f5b109817a7 100644
--- a/drivers/hwmon/mlxreg-fan.c
+++ b/drivers/hwmon/mlxreg-fan.c
@@ -12,7 +12,7 @@
 #include <linux/regmap.h>
 #include <linux/thermal.h>
 
-#define MLXREG_FAN_MAX_TACHO		12
+#define MLXREG_FAN_MAX_TACHO		14
 #define MLXREG_FAN_MAX_STATE		10
 #define MLXREG_FAN_MIN_DUTY		51	/* 20% */
 #define MLXREG_FAN_MAX_DUTY		255	/* 100% */
@@ -266,6 +266,8 @@ static const struct hwmon_channel_info *mlxreg_fan_hwmon_info[] = {
 			   HWMON_F_INPUT | HWMON_F_FAULT,
 			   HWMON_F_INPUT | HWMON_F_FAULT,
 			   HWMON_F_INPUT | HWMON_F_FAULT,
+			   HWMON_F_INPUT | HWMON_F_FAULT,
+			   HWMON_F_INPUT | HWMON_F_FAULT,
 			   HWMON_F_INPUT | HWMON_F_FAULT),
 	HWMON_CHANNEL_INFO(pwm,
 			   HWMON_PWM_INPUT),
-- 
2.20.1

Re: [PATCH hwmon-next v2 1/3] hwmon: (mlxreg-fan) Extend the maximum number of tachometers

[Guenter Roeck]

On Thu, Sep 16, 2021 at 10:47:17PM +0300, Vadim Pasternak wrote:
> Extend support of maximum tachometers from 12 to 14 in order to support
> new systems, equipped with more fans.
> 
> Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>

Applied, no need to resend.

> ---
>  drivers/hwmon/mlxreg-fan.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
> index 89fe7b9fe26b..0f5b109817a7 100644
> --- a/drivers/hwmon/mlxreg-fan.c
> +++ b/drivers/hwmon/mlxreg-fan.c
> @@ -12,7 +12,7 @@
>  #include <linux/regmap.h>
>  #include <linux/thermal.h>
>  
> -#define MLXREG_FAN_MAX_TACHO		12
> +#define MLXREG_FAN_MAX_TACHO		14
>  #define MLXREG_FAN_MAX_STATE		10
>  #define MLXREG_FAN_MIN_DUTY		51	/* 20% */
>  #define MLXREG_FAN_MAX_DUTY		255	/* 100% */
> @@ -266,6 +266,8 @@ static const struct hwmon_channel_info *mlxreg_fan_hwmon_info[] = {
>  			   HWMON_F_INPUT | HWMON_F_FAULT,
>  			   HWMON_F_INPUT | HWMON_F_FAULT,
>  			   HWMON_F_INPUT | HWMON_F_FAULT,
> +			   HWMON_F_INPUT | HWMON_F_FAULT,
> +			   HWMON_F_INPUT | HWMON_F_FAULT,
>  			   HWMON_F_INPUT | HWMON_F_FAULT),
>  	HWMON_CHANNEL_INFO(pwm,
>  			   HWMON_PWM_INPUT),
> -- 
> 2.20.1
> 

[PATCH hwmon-next v2 2/3] hwmon: (mlxreg-fan) Extend driver to support multiply PWM

[Vadim Pasternak]

Add additional PWM attributes in order to support the systems, which
can be equipped with up-to four PWM controllers. System capability of
additional PWM support is validated through the reading of relevant
registers.

Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
---
v0->v2:
 Comments pointed out by Guenter:
 - Fix handling of PWM counter, increment 'pwm_num', drop 'pwm_avail'.
---
 drivers/hwmon/mlxreg-fan.c | 55 +++++++++++++++++++++++++++++---------
 1 file changed, 43 insertions(+), 12 deletions(-)

diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
index 0f5b109817a7..1a146cc4b0fd 100644
--- a/drivers/hwmon/mlxreg-fan.c
+++ b/drivers/hwmon/mlxreg-fan.c
@@ -13,6 +13,8 @@
 #include <linux/thermal.h>
 
 #define MLXREG_FAN_MAX_TACHO		14
+#define MLXREG_FAN_MAX_PWM		4
+#define MLXREG_FAN_PWM_NOT_CONNECTED	0xff
 #define MLXREG_FAN_MAX_STATE		10
 #define MLXREG_FAN_MIN_DUTY		51	/* 20% */
 #define MLXREG_FAN_MAX_DUTY		255	/* 100% */
@@ -105,7 +107,7 @@ struct mlxreg_fan {
 	void *regmap;
 	struct mlxreg_core_platform_data *pdata;
 	struct mlxreg_fan_tacho tacho[MLXREG_FAN_MAX_TACHO];
-	struct mlxreg_fan_pwm pwm;
+	struct mlxreg_fan_pwm pwm[MLXREG_FAN_MAX_PWM];
 	int tachos_per_drwr;
 	int samples;
 	int divider;
@@ -119,6 +121,7 @@ mlxreg_fan_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 {
 	struct mlxreg_fan *fan = dev_get_drvdata(dev);
 	struct mlxreg_fan_tacho *tacho;
+	struct mlxreg_fan_pwm *pwm;
 	u32 regval;
 	int err;
 
@@ -169,9 +172,10 @@ mlxreg_fan_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 		break;
 
 	case hwmon_pwm:
+		pwm = &fan->pwm[channel];
 		switch (attr) {
 		case hwmon_pwm_input:
-			err = regmap_read(fan->regmap, fan->pwm.reg, &regval);
+			err = regmap_read(fan->regmap, pwm->reg, &regval);
 			if (err)
 				return err;
 
@@ -195,6 +199,7 @@ mlxreg_fan_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 		 int channel, long val)
 {
 	struct mlxreg_fan *fan = dev_get_drvdata(dev);
+	struct mlxreg_fan_pwm *pwm;
 
 	switch (type) {
 	case hwmon_pwm:
@@ -203,7 +208,8 @@ mlxreg_fan_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 			if (val < MLXREG_FAN_MIN_DUTY ||
 			    val > MLXREG_FAN_MAX_DUTY)
 				return -EINVAL;
-			return regmap_write(fan->regmap, fan->pwm.reg, val);
+			pwm = &fan->pwm[channel];
+			return regmap_write(fan->regmap, pwm->reg, val);
 		default:
 			return -EOPNOTSUPP;
 		}
@@ -235,7 +241,7 @@ mlxreg_fan_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
 		break;
 
 	case hwmon_pwm:
-		if (!(((struct mlxreg_fan *)data)->pwm.connected))
+		if (!(((struct mlxreg_fan *)data)->pwm[channel].connected))
 			return 0;
 
 		switch (attr) {
@@ -270,6 +276,9 @@ static const struct hwmon_channel_info *mlxreg_fan_hwmon_info[] = {
 			   HWMON_F_INPUT | HWMON_F_FAULT,
 			   HWMON_F_INPUT | HWMON_F_FAULT),
 	HWMON_CHANNEL_INFO(pwm,
+			   HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT,
 			   HWMON_PWM_INPUT),
 	NULL
 };
@@ -300,7 +309,7 @@ static int mlxreg_fan_get_cur_state(struct thermal_cooling_device *cdev,
 	u32 regval;
 	int err;
 
-	err = regmap_read(fan->regmap, fan->pwm.reg, &regval);
+	err = regmap_read(fan->regmap, fan->pwm[0].reg, &regval);
 	if (err) {
 		dev_err(fan->dev, "Failed to query PWM duty\n");
 		return err;
@@ -343,7 +352,7 @@ static int mlxreg_fan_set_cur_state(struct thermal_cooling_device *cdev,
 		for (i = state; i <= MLXREG_FAN_MAX_STATE; i++)
 			fan->cooling_levels[i] = i;
 
-		err = regmap_read(fan->regmap, fan->pwm.reg, &regval);
+		err = regmap_read(fan->regmap, fan->pwm[0].reg, &regval);
 		if (err) {
 			dev_err(fan->dev, "Failed to query PWM duty\n");
 			return err;
@@ -361,7 +370,7 @@ static int mlxreg_fan_set_cur_state(struct thermal_cooling_device *cdev,
 
 	/* Normalize the state to the valid speed range. */
 	state = fan->cooling_levels[state];
-	err = regmap_write(fan->regmap, fan->pwm.reg,
+	err = regmap_write(fan->regmap, fan->pwm[0].reg,
 			   MLXREG_FAN_PWM_STATE2DUTY(state));
 	if (err) {
 		dev_err(fan->dev, "Failed to write PWM duty\n");
@@ -392,6 +401,22 @@ static int mlxreg_fan_connect_verify(struct mlxreg_fan *fan,
 	return !!(regval & data->bit);
 }
 
+static int mlxreg_pwm_connect_verify(struct mlxreg_fan *fan,
+				     struct mlxreg_core_data *data)
+{
+	u32 regval;
+	int err;
+
+	err = regmap_read(fan->regmap, data->reg, &regval);
+	if (err) {
+		dev_err(fan->dev, "Failed to query pwm register 0x%08x\n",
+			data->reg);
+		return err;
+	}
+
+	return regval != MLXREG_FAN_PWM_NOT_CONNECTED;
+}
+
 static int mlxreg_fan_speed_divider_get(struct mlxreg_fan *fan,
 					struct mlxreg_core_data *data)
 {
@@ -420,8 +445,8 @@ static int mlxreg_fan_speed_divider_get(struct mlxreg_fan *fan,
 static int mlxreg_fan_config(struct mlxreg_fan *fan,
 			     struct mlxreg_core_platform_data *pdata)
 {
+	int tacho_num = 0, tacho_avail = 0, pwm_num = 0, i;
 	struct mlxreg_core_data *data = pdata->data;
-	int tacho_num = 0, tacho_avail = 0, i;
 	bool configured = false;
 	int err;
 
@@ -451,13 +476,19 @@ static int mlxreg_fan_config(struct mlxreg_fan *fan,
 			fan->tacho[tacho_num++].connected = true;
 			tacho_avail++;
 		} else if (strnstr(data->label, "pwm", sizeof(data->label))) {
-			if (fan->pwm.connected) {
-				dev_err(fan->dev, "duplicate pwm entry: %s\n",
+			if (pwm_num == MLXREG_FAN_MAX_TACHO) {
+				dev_err(fan->dev, "too many pwm entries: %s\n",
 					data->label);
 				return -EINVAL;
 			}
-			fan->pwm.reg = data->reg;
-			fan->pwm.connected = true;
+
+			err = mlxreg_pwm_connect_verify(fan, data);
+			if (err)
+				return err;
+
+			fan->pwm[pwm_num].reg = data->reg;
+			fan->pwm[pwm_num].connected = true;
+			pwm_num++;
 		} else if (strnstr(data->label, "conf", sizeof(data->label))) {
 			if (configured) {
 				dev_err(fan->dev, "duplicate conf entry: %s\n",
-- 
2.20.1

Re: [PATCH hwmon-next v2 2/3] hwmon: (mlxreg-fan) Extend driver to support multiply PWM

[Guenter Roeck]

On Thu, Sep 16, 2021 at 10:47:18PM +0300, Vadim Pasternak wrote:
> Add additional PWM attributes in order to support the systems, which
> can be equipped with up-to four PWM controllers. System capability of
> additional PWM support is validated through the reading of relevant
> registers.
> 
> Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>

Applied, no need to resend.

Guenter

> ---
> v0->v2:
>  Comments pointed out by Guenter:
>  - Fix handling of PWM counter, increment 'pwm_num', drop 'pwm_avail'.
> ---
>  drivers/hwmon/mlxreg-fan.c | 55 +++++++++++++++++++++++++++++---------
>  1 file changed, 43 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
> index 0f5b109817a7..1a146cc4b0fd 100644
> --- a/drivers/hwmon/mlxreg-fan.c
> +++ b/drivers/hwmon/mlxreg-fan.c
> @@ -13,6 +13,8 @@
>  #include <linux/thermal.h>
>  
>  #define MLXREG_FAN_MAX_TACHO		14
> +#define MLXREG_FAN_MAX_PWM		4
> +#define MLXREG_FAN_PWM_NOT_CONNECTED	0xff
>  #define MLXREG_FAN_MAX_STATE		10
>  #define MLXREG_FAN_MIN_DUTY		51	/* 20% */
>  #define MLXREG_FAN_MAX_DUTY		255	/* 100% */
> @@ -105,7 +107,7 @@ struct mlxreg_fan {
>  	void *regmap;
>  	struct mlxreg_core_platform_data *pdata;
>  	struct mlxreg_fan_tacho tacho[MLXREG_FAN_MAX_TACHO];
> -	struct mlxreg_fan_pwm pwm;
> +	struct mlxreg_fan_pwm pwm[MLXREG_FAN_MAX_PWM];
>  	int tachos_per_drwr;
>  	int samples;
>  	int divider;
> @@ -119,6 +121,7 @@ mlxreg_fan_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  {
>  	struct mlxreg_fan *fan = dev_get_drvdata(dev);
>  	struct mlxreg_fan_tacho *tacho;
> +	struct mlxreg_fan_pwm *pwm;
>  	u32 regval;
>  	int err;
>  
> @@ -169,9 +172,10 @@ mlxreg_fan_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  		break;
>  
>  	case hwmon_pwm:
> +		pwm = &fan->pwm[channel];
>  		switch (attr) {
>  		case hwmon_pwm_input:
> -			err = regmap_read(fan->regmap, fan->pwm.reg, &regval);
> +			err = regmap_read(fan->regmap, pwm->reg, &regval);
>  			if (err)
>  				return err;
>  
> @@ -195,6 +199,7 @@ mlxreg_fan_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  		 int channel, long val)
>  {
>  	struct mlxreg_fan *fan = dev_get_drvdata(dev);
> +	struct mlxreg_fan_pwm *pwm;
>  
>  	switch (type) {
>  	case hwmon_pwm:
> @@ -203,7 +208,8 @@ mlxreg_fan_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  			if (val < MLXREG_FAN_MIN_DUTY ||
>  			    val > MLXREG_FAN_MAX_DUTY)
>  				return -EINVAL;
> -			return regmap_write(fan->regmap, fan->pwm.reg, val);
> +			pwm = &fan->pwm[channel];
> +			return regmap_write(fan->regmap, pwm->reg, val);
>  		default:
>  			return -EOPNOTSUPP;
>  		}
> @@ -235,7 +241,7 @@ mlxreg_fan_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
>  		break;
>  
>  	case hwmon_pwm:
> -		if (!(((struct mlxreg_fan *)data)->pwm.connected))
> +		if (!(((struct mlxreg_fan *)data)->pwm[channel].connected))
>  			return 0;
>  
>  		switch (attr) {
> @@ -270,6 +276,9 @@ static const struct hwmon_channel_info *mlxreg_fan_hwmon_info[] = {
>  			   HWMON_F_INPUT | HWMON_F_FAULT,
>  			   HWMON_F_INPUT | HWMON_F_FAULT),
>  	HWMON_CHANNEL_INFO(pwm,
> +			   HWMON_PWM_INPUT,
> +			   HWMON_PWM_INPUT,
> +			   HWMON_PWM_INPUT,
>  			   HWMON_PWM_INPUT),
>  	NULL
>  };
> @@ -300,7 +309,7 @@ static int mlxreg_fan_get_cur_state(struct thermal_cooling_device *cdev,
>  	u32 regval;
>  	int err;
>  
> -	err = regmap_read(fan->regmap, fan->pwm.reg, &regval);
> +	err = regmap_read(fan->regmap, fan->pwm[0].reg, &regval);
>  	if (err) {
>  		dev_err(fan->dev, "Failed to query PWM duty\n");
>  		return err;
> @@ -343,7 +352,7 @@ static int mlxreg_fan_set_cur_state(struct thermal_cooling_device *cdev,
>  		for (i = state; i <= MLXREG_FAN_MAX_STATE; i++)
>  			fan->cooling_levels[i] = i;
>  
> -		err = regmap_read(fan->regmap, fan->pwm.reg, &regval);
> +		err = regmap_read(fan->regmap, fan->pwm[0].reg, &regval);
>  		if (err) {
>  			dev_err(fan->dev, "Failed to query PWM duty\n");
>  			return err;
> @@ -361,7 +370,7 @@ static int mlxreg_fan_set_cur_state(struct thermal_cooling_device *cdev,
>  
>  	/* Normalize the state to the valid speed range. */
>  	state = fan->cooling_levels[state];
> -	err = regmap_write(fan->regmap, fan->pwm.reg,
> +	err = regmap_write(fan->regmap, fan->pwm[0].reg,
>  			   MLXREG_FAN_PWM_STATE2DUTY(state));
>  	if (err) {
>  		dev_err(fan->dev, "Failed to write PWM duty\n");
> @@ -392,6 +401,22 @@ static int mlxreg_fan_connect_verify(struct mlxreg_fan *fan,
>  	return !!(regval & data->bit);
>  }
>  
> +static int mlxreg_pwm_connect_verify(struct mlxreg_fan *fan,
> +				     struct mlxreg_core_data *data)
> +{
> +	u32 regval;
> +	int err;
> +
> +	err = regmap_read(fan->regmap, data->reg, &regval);
> +	if (err) {
> +		dev_err(fan->dev, "Failed to query pwm register 0x%08x\n",
> +			data->reg);
> +		return err;
> +	}
> +
> +	return regval != MLXREG_FAN_PWM_NOT_CONNECTED;
> +}
> +
>  static int mlxreg_fan_speed_divider_get(struct mlxreg_fan *fan,
>  					struct mlxreg_core_data *data)
>  {
> @@ -420,8 +445,8 @@ static int mlxreg_fan_speed_divider_get(struct mlxreg_fan *fan,
>  static int mlxreg_fan_config(struct mlxreg_fan *fan,
>  			     struct mlxreg_core_platform_data *pdata)
>  {
> +	int tacho_num = 0, tacho_avail = 0, pwm_num = 0, i;
>  	struct mlxreg_core_data *data = pdata->data;
> -	int tacho_num = 0, tacho_avail = 0, i;
>  	bool configured = false;
>  	int err;
>  
> @@ -451,13 +476,19 @@ static int mlxreg_fan_config(struct mlxreg_fan *fan,
>  			fan->tacho[tacho_num++].connected = true;
>  			tacho_avail++;
>  		} else if (strnstr(data->label, "pwm", sizeof(data->label))) {
> -			if (fan->pwm.connected) {
> -				dev_err(fan->dev, "duplicate pwm entry: %s\n",
> +			if (pwm_num == MLXREG_FAN_MAX_TACHO) {
> +				dev_err(fan->dev, "too many pwm entries: %s\n",
>  					data->label);
>  				return -EINVAL;
>  			}
> -			fan->pwm.reg = data->reg;
> -			fan->pwm.connected = true;
> +
> +			err = mlxreg_pwm_connect_verify(fan, data);
> +			if (err)
> +				return err;
> +
> +			fan->pwm[pwm_num].reg = data->reg;
> +			fan->pwm[pwm_num].connected = true;
> +			pwm_num++;
>  		} else if (strnstr(data->label, "conf", sizeof(data->label))) {
>  			if (configured) {
>  				dev_err(fan->dev, "duplicate conf entry: %s\n",
> -- 
> 2.20.1
> 

[PATCH hwmon-next v2 3/3] hwmon: (mlxreg-fan) Extend driver to support multiply cooling devices

[Vadim Pasternak]

Add support for additional cooling devices in order to support the
systems, which can be equipped with up-to four PWM controllers.

Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
---
v0->v2:
 Comments pointed out by Guenter:
 - Drop call to thermal_cooling_device_unregister() in error flow,
   devices registered by devm_thermal_of_cooling_device_register()
   should be cleaned automatically.
---
 drivers/hwmon/mlxreg-fan.c | 76 +++++++++++++++++++++++++-------------
 1 file changed, 50 insertions(+), 26 deletions(-)

diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
index 1a146cc4b0fd..566bee333c3c 100644
--- a/drivers/hwmon/mlxreg-fan.c
+++ b/drivers/hwmon/mlxreg-fan.c
@@ -63,6 +63,8 @@
 					 MLXREG_FAN_MAX_DUTY,		\
 					 MLXREG_FAN_MAX_STATE))
 
+struct mlxreg_fan;
+
 /*
  * struct mlxreg_fan_tacho - tachometer data (internal use):
  *
@@ -81,12 +83,18 @@ struct mlxreg_fan_tacho {
 /*
  * struct mlxreg_fan_pwm - PWM data (internal use):
  *
+ * @fan: private data;
  * @connected: indicates if PWM is connected;
  * @reg: register offset;
+ * @cooling: cooling device levels;
+ * @cdev: cooling device;
  */
 struct mlxreg_fan_pwm {
+	struct mlxreg_fan *fan;
 	bool connected;
 	u32 reg;
+	u8 cooling_levels[MLXREG_FAN_MAX_STATE + 1];
+	struct thermal_cooling_device *cdev;
 };
 
 /*
@@ -99,8 +107,6 @@ struct mlxreg_fan_pwm {
  * @tachos_per_drwr - number of tachometers per drawer;
  * @samples: minimum allowed samples per pulse;
  * @divider: divider value for tachometer RPM calculation;
- * @cooling: cooling device levels;
- * @cdev: cooling device;
  */
 struct mlxreg_fan {
 	struct device *dev;
@@ -111,8 +117,6 @@ struct mlxreg_fan {
 	int tachos_per_drwr;
 	int samples;
 	int divider;
-	u8 cooling_levels[MLXREG_FAN_MAX_STATE + 1];
-	struct thermal_cooling_device *cdev;
 };
 
 static int
@@ -305,11 +309,12 @@ static int mlxreg_fan_get_cur_state(struct thermal_cooling_device *cdev,
 				    unsigned long *state)
 
 {
-	struct mlxreg_fan *fan = cdev->devdata;
+	struct mlxreg_fan_pwm *pwm = cdev->devdata;
+	struct mlxreg_fan *fan = pwm->fan;
 	u32 regval;
 	int err;
 
-	err = regmap_read(fan->regmap, fan->pwm[0].reg, &regval);
+	err = regmap_read(fan->regmap, pwm->reg, &regval);
 	if (err) {
 		dev_err(fan->dev, "Failed to query PWM duty\n");
 		return err;
@@ -324,7 +329,8 @@ static int mlxreg_fan_set_cur_state(struct thermal_cooling_device *cdev,
 				    unsigned long state)
 
 {
-	struct mlxreg_fan *fan = cdev->devdata;
+	struct mlxreg_fan_pwm *pwm = cdev->devdata;
+	struct mlxreg_fan *fan = pwm->fan;
 	unsigned long cur_state;
 	int i, config = 0;
 	u32 regval;
@@ -348,11 +354,11 @@ static int mlxreg_fan_set_cur_state(struct thermal_cooling_device *cdev,
 		config = 1;
 		state -= MLXREG_FAN_MAX_STATE;
 		for (i = 0; i < state; i++)
-			fan->cooling_levels[i] = state;
+			pwm->cooling_levels[i] = state;
 		for (i = state; i <= MLXREG_FAN_MAX_STATE; i++)
-			fan->cooling_levels[i] = i;
+			pwm->cooling_levels[i] = i;
 
-		err = regmap_read(fan->regmap, fan->pwm[0].reg, &regval);
+		err = regmap_read(fan->regmap, pwm->reg, &regval);
 		if (err) {
 			dev_err(fan->dev, "Failed to query PWM duty\n");
 			return err;
@@ -369,8 +375,8 @@ static int mlxreg_fan_set_cur_state(struct thermal_cooling_device *cdev,
 		return -EINVAL;
 
 	/* Normalize the state to the valid speed range. */
-	state = fan->cooling_levels[state];
-	err = regmap_write(fan->regmap, fan->pwm[0].reg,
+	state = pwm->cooling_levels[state];
+	err = regmap_write(fan->regmap, pwm->reg,
 			   MLXREG_FAN_PWM_STATE2DUTY(state));
 	if (err) {
 		dev_err(fan->dev, "Failed to write PWM duty\n");
@@ -541,13 +547,37 @@ static int mlxreg_fan_config(struct mlxreg_fan *fan,
 		fan->tachos_per_drwr = tacho_avail / drwr_avail;
 	}
 
-	/* Init cooling levels per PWM state. */
-	for (i = 0; i < MLXREG_FAN_SPEED_MIN_LEVEL; i++)
-		fan->cooling_levels[i] = MLXREG_FAN_SPEED_MIN_LEVEL;
-	for (i = MLXREG_FAN_SPEED_MIN_LEVEL; i <= MLXREG_FAN_MAX_STATE; i++)
-		fan->cooling_levels[i] = i;
+	return 0;
+}
+
+static int mlxreg_fan_cooling_config(struct device *dev, struct mlxreg_fan *fan)
+{
+	int i, j, err;
+
+	for (i = 0; i <= MLXREG_FAN_MAX_PWM; i++) {
+		struct mlxreg_fan_pwm *pwm = &fan->pwm[i];
+
+		if (!pwm->connected)
+			continue;
+		pwm->fan = fan;
+		pwm->cdev = devm_thermal_of_cooling_device_register(dev, NULL, "mlxreg_fan", pwm,
+								    &mlxreg_fan_cooling_ops);
+		if (IS_ERR(pwm->cdev)) {
+			dev_err(dev, "Failed to register cooling device\n");
+			err = PTR_ERR(pwm->cdev);
+			goto devm_thermal_of_cooling_device_register_fail;
+		}
+
+		/* Init cooling levels per PWM state. */
+		for (j = 0; j < MLXREG_FAN_SPEED_MIN_LEVEL; j++)
+			pwm->cooling_levels[j] = MLXREG_FAN_SPEED_MIN_LEVEL;
+		for (j = MLXREG_FAN_SPEED_MIN_LEVEL; j <= MLXREG_FAN_MAX_STATE; j++)
+			pwm->cooling_levels[j] = j;
+	}
 
 	return 0;
+devm_thermal_of_cooling_device_register_fail:
+	return err;
 }
 
 static int mlxreg_fan_probe(struct platform_device *pdev)
@@ -584,16 +614,10 @@ static int mlxreg_fan_probe(struct platform_device *pdev)
 		return PTR_ERR(hwm);
 	}
 
-	if (IS_REACHABLE(CONFIG_THERMAL)) {
-		fan->cdev = devm_thermal_of_cooling_device_register(dev,
-			NULL, "mlxreg_fan", fan, &mlxreg_fan_cooling_ops);
-		if (IS_ERR(fan->cdev)) {
-			dev_err(dev, "Failed to register cooling device\n");
-			return PTR_ERR(fan->cdev);
-		}
-	}
+	if (IS_REACHABLE(CONFIG_THERMAL))
+		err = mlxreg_fan_cooling_config(dev, fan);
 
-	return 0;
+	return err;
 }
 
 static struct platform_driver mlxreg_fan_driver = {
-- 
2.20.1

Re: [PATCH hwmon-next v2 3/3] hwmon: (mlxreg-fan) Extend driver to support multiply cooling devices

[Guenter Roeck]

On Thu, Sep 16, 2021 at 10:47:19PM +0300, Vadim Pasternak wrote:
> Add support for additional cooling devices in order to support the
> systems, which can be equipped with up-to four PWM controllers.
> 
> Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
> ---
> v0->v2:
>  Comments pointed out by Guenter:
>  - Drop call to thermal_cooling_device_unregister() in error flow,
>    devices registered by devm_thermal_of_cooling_device_register()
>    should be cleaned automatically.
> ---
>  drivers/hwmon/mlxreg-fan.c | 76 +++++++++++++++++++++++++-------------
>  1 file changed, 50 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
> index 1a146cc4b0fd..566bee333c3c 100644
> --- a/drivers/hwmon/mlxreg-fan.c
> +++ b/drivers/hwmon/mlxreg-fan.c
> @@ -63,6 +63,8 @@
>  					 MLXREG_FAN_MAX_DUTY,		\
>  					 MLXREG_FAN_MAX_STATE))
>  
> +struct mlxreg_fan;
> +
>  /*
>   * struct mlxreg_fan_tacho - tachometer data (internal use):
>   *
> @@ -81,12 +83,18 @@ struct mlxreg_fan_tacho {
>  /*
>   * struct mlxreg_fan_pwm - PWM data (internal use):
>   *
> + * @fan: private data;
>   * @connected: indicates if PWM is connected;
>   * @reg: register offset;
> + * @cooling: cooling device levels;
> + * @cdev: cooling device;
>   */
>  struct mlxreg_fan_pwm {
> +	struct mlxreg_fan *fan;
>  	bool connected;
>  	u32 reg;
> +	u8 cooling_levels[MLXREG_FAN_MAX_STATE + 1];
> +	struct thermal_cooling_device *cdev;
>  };
>  
>  /*
> @@ -99,8 +107,6 @@ struct mlxreg_fan_pwm {
>   * @tachos_per_drwr - number of tachometers per drawer;
>   * @samples: minimum allowed samples per pulse;
>   * @divider: divider value for tachometer RPM calculation;
> - * @cooling: cooling device levels;
> - * @cdev: cooling device;
>   */
>  struct mlxreg_fan {
>  	struct device *dev;
> @@ -111,8 +117,6 @@ struct mlxreg_fan {
>  	int tachos_per_drwr;
>  	int samples;
>  	int divider;
> -	u8 cooling_levels[MLXREG_FAN_MAX_STATE + 1];
> -	struct thermal_cooling_device *cdev;
>  };
>  
>  static int
> @@ -305,11 +309,12 @@ static int mlxreg_fan_get_cur_state(struct thermal_cooling_device *cdev,
>  				    unsigned long *state)
>  
>  {
> -	struct mlxreg_fan *fan = cdev->devdata;
> +	struct mlxreg_fan_pwm *pwm = cdev->devdata;
> +	struct mlxreg_fan *fan = pwm->fan;
>  	u32 regval;
>  	int err;
>  
> -	err = regmap_read(fan->regmap, fan->pwm[0].reg, &regval);
> +	err = regmap_read(fan->regmap, pwm->reg, &regval);
>  	if (err) {
>  		dev_err(fan->dev, "Failed to query PWM duty\n");
>  		return err;
> @@ -324,7 +329,8 @@ static int mlxreg_fan_set_cur_state(struct thermal_cooling_device *cdev,
>  				    unsigned long state)
>  
>  {
> -	struct mlxreg_fan *fan = cdev->devdata;
> +	struct mlxreg_fan_pwm *pwm = cdev->devdata;
> +	struct mlxreg_fan *fan = pwm->fan;
>  	unsigned long cur_state;
>  	int i, config = 0;
>  	u32 regval;
> @@ -348,11 +354,11 @@ static int mlxreg_fan_set_cur_state(struct thermal_cooling_device *cdev,
>  		config = 1;
>  		state -= MLXREG_FAN_MAX_STATE;
>  		for (i = 0; i < state; i++)
> -			fan->cooling_levels[i] = state;
> +			pwm->cooling_levels[i] = state;
>  		for (i = state; i <= MLXREG_FAN_MAX_STATE; i++)
> -			fan->cooling_levels[i] = i;
> +			pwm->cooling_levels[i] = i;
>  
> -		err = regmap_read(fan->regmap, fan->pwm[0].reg, &regval);
> +		err = regmap_read(fan->regmap, pwm->reg, &regval);
>  		if (err) {
>  			dev_err(fan->dev, "Failed to query PWM duty\n");
>  			return err;
> @@ -369,8 +375,8 @@ static int mlxreg_fan_set_cur_state(struct thermal_cooling_device *cdev,
>  		return -EINVAL;
>  
>  	/* Normalize the state to the valid speed range. */
> -	state = fan->cooling_levels[state];
> -	err = regmap_write(fan->regmap, fan->pwm[0].reg,
> +	state = pwm->cooling_levels[state];
> +	err = regmap_write(fan->regmap, pwm->reg,
>  			   MLXREG_FAN_PWM_STATE2DUTY(state));
>  	if (err) {
>  		dev_err(fan->dev, "Failed to write PWM duty\n");
> @@ -541,13 +547,37 @@ static int mlxreg_fan_config(struct mlxreg_fan *fan,
>  		fan->tachos_per_drwr = tacho_avail / drwr_avail;
>  	}
>  
> -	/* Init cooling levels per PWM state. */
> -	for (i = 0; i < MLXREG_FAN_SPEED_MIN_LEVEL; i++)
> -		fan->cooling_levels[i] = MLXREG_FAN_SPEED_MIN_LEVEL;
> -	for (i = MLXREG_FAN_SPEED_MIN_LEVEL; i <= MLXREG_FAN_MAX_STATE; i++)
> -		fan->cooling_levels[i] = i;
> +	return 0;
> +}
> +
> +static int mlxreg_fan_cooling_config(struct device *dev, struct mlxreg_fan *fan)
> +{
> +	int i, j, err;
> +
> +	for (i = 0; i <= MLXREG_FAN_MAX_PWM; i++) {
> +		struct mlxreg_fan_pwm *pwm = &fan->pwm[i];
> +
> +		if (!pwm->connected)
> +			continue;
> +		pwm->fan = fan;
> +		pwm->cdev = devm_thermal_of_cooling_device_register(dev, NULL, "mlxreg_fan", pwm,
> +								    &mlxreg_fan_cooling_ops);
> +		if (IS_ERR(pwm->cdev)) {
> +			dev_err(dev, "Failed to register cooling device\n");
> +			err = PTR_ERR(pwm->cdev);
> +			goto devm_thermal_of_cooling_device_register_fail;

Unnecessary goto.

			return PTR_ERR(pwm->cdev);

> +		}
> +
> +		/* Init cooling levels per PWM state. */
> +		for (j = 0; j < MLXREG_FAN_SPEED_MIN_LEVEL; j++)
> +			pwm->cooling_levels[j] = MLXREG_FAN_SPEED_MIN_LEVEL;
> +		for (j = MLXREG_FAN_SPEED_MIN_LEVEL; j <= MLXREG_FAN_MAX_STATE; j++)
> +			pwm->cooling_levels[j] = j;
> +	}
>  
>  	return 0;
> +devm_thermal_of_cooling_device_register_fail:
> +	return err;
>  }
>  
>  static int mlxreg_fan_probe(struct platform_device *pdev)
> @@ -584,16 +614,10 @@ static int mlxreg_fan_probe(struct platform_device *pdev)
>  		return PTR_ERR(hwm);
>  	}
>  
> -	if (IS_REACHABLE(CONFIG_THERMAL)) {
> -		fan->cdev = devm_thermal_of_cooling_device_register(dev,
> -			NULL, "mlxreg_fan", fan, &mlxreg_fan_cooling_ops);
> -		if (IS_ERR(fan->cdev)) {
> -			dev_err(dev, "Failed to register cooling device\n");
> -			return PTR_ERR(fan->cdev);
> -		}
> -	}
> +	if (IS_REACHABLE(CONFIG_THERMAL))
> +		err = mlxreg_fan_cooling_config(dev, fan);
>  
> -	return 0;
> +	return err;
>  }
>  
>  static struct platform_driver mlxreg_fan_driver = {
> -- 
> 2.20.1
>