linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] pwm-fan: Support multiple tachometer inputs
@ 2020-12-12 19:50 Paul Barker
  2020-12-12 19:50 ` [PATCH v3 1/2] hwmon: pwm-fan: Store tach data separately Paul Barker
  2020-12-12 19:50 ` [PATCH v3 2/2] hwmon: pwm-fan: Support multiple fan tachometers Paul Barker
  0 siblings, 2 replies; 5+ messages in thread
From: Paul Barker @ 2020-12-12 19:50 UTC (permalink / raw)
  To: Kamil Debski, Bartlomiej Zolnierkiewicz, Jean Delvare, Guenter Roeck
  Cc: Paul Barker, linux-hwmon

These changes were made to support a custom board where one PWM output
is routed to two fans, each of which has a tachometer signal routed to a
GPIO input on the SoC.

As the custom board doesn't currently support the latest mainline kernel
I've tested these changes on a SanCloud BeagleBone Enhanced using an
oscilloscope to check the PWM output and a signal generator to simulate
the fan tachometer signals. I've tested variants of the device tree with
0, 1 and 2 fan tachometer inputs configured to ensure the logic in the
probe function is correct.

The device tree bindings changes have been submitted in a separate
series:
https://lore.kernel.org/linux-devicetree/20200920180943.352526-1-pbarker@konsulko.com/

These changes can also be pulled from:

  https://gitlab.com/pbarker.dev/staging/linux.git
  tag: for-hwmon/pwm-fan-tachometers-v3_2020-12-12

Changes since v2:

  * Split refactoring and API conversion into a separate patch series
    which has now been accepted. These changes now use the recommended
    hwmon API and dynamically allocate struct hwmon_channel_info
    objects as needed.

  * Rebased changes on the current HEAD of hwmon-next.

Changes since v1:

  * Split RPM calculation fix into a separate patch which has now been
    accepted.

  * Break the changes down into smaller patches so they're easier to
    review.

  * Rebased changes on hwmon-next.

Paul Barker (2):
  hwmon: pwm-fan: Store tach data separately
  hwmon: pwm-fan: Support multiple fan tachometers

 drivers/hwmon/pwm-fan.c | 111 +++++++++++++++++++++++++---------------
 1 file changed, 69 insertions(+), 42 deletions(-)


base-commit: 1a033769a4fe9a86ee791fd553b6a996dd76e026
-- 
2.26.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/2] hwmon: pwm-fan: Store tach data separately
  2020-12-12 19:50 [PATCH v3 0/2] pwm-fan: Support multiple tachometer inputs Paul Barker
@ 2020-12-12 19:50 ` Paul Barker
  2020-12-30 16:18   ` Guenter Roeck
  2020-12-12 19:50 ` [PATCH v3 2/2] hwmon: pwm-fan: Support multiple fan tachometers Paul Barker
  1 sibling, 1 reply; 5+ messages in thread
From: Paul Barker @ 2020-12-12 19:50 UTC (permalink / raw)
  To: Kamil Debski, Bartlomiej Zolnierkiewicz, Jean Delvare, Guenter Roeck
  Cc: Paul Barker, linux-hwmon

The data for the (optional) fan tachometer input is moved to a separate
structure which is only allocated if an input is actually configured.

After this change the pulse IRQ handler takes a pointer to the
tachometer data structure instead of the whole device context.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 drivers/hwmon/pwm-fan.c | 52 +++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 777439f48c14..4a7f0e87b08f 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -21,15 +21,19 @@
 
 #define MAX_PWM 255
 
+struct pwm_fan_tach {
+	int irq;
+	atomic_t pulses;
+	unsigned int rpm;
+	u8 pulses_per_revolution;
+};
+
 struct pwm_fan_ctx {
 	struct mutex lock;
 	struct pwm_device *pwm;
 	struct regulator *reg_en;
 
-	int irq;
-	atomic_t pulses;
-	unsigned int rpm;
-	u8 pulses_per_revolution;
+	struct pwm_fan_tach *tach;
 	ktime_t sample_start;
 	struct timer_list rpm_timer;
 
@@ -65,9 +69,9 @@ static const struct hwmon_channel_info pwm_fan_channel_fan = {
 /* This handler assumes self resetting edge triggered interrupt. */
 static irqreturn_t pulse_handler(int irq, void *dev_id)
 {
-	struct pwm_fan_ctx *ctx = dev_id;
+	struct pwm_fan_tach *tach = dev_id;
 
-	atomic_inc(&ctx->pulses);
+	atomic_inc(&tach->pulses);
 
 	return IRQ_HANDLED;
 }
@@ -75,14 +79,15 @@ static irqreturn_t pulse_handler(int irq, void *dev_id)
 static void sample_timer(struct timer_list *t)
 {
 	struct pwm_fan_ctx *ctx = from_timer(ctx, t, rpm_timer);
+	struct pwm_fan_tach *tach = ctx->tach;
 	unsigned int delta = ktime_ms_delta(ktime_get(), ctx->sample_start);
 	int pulses;
 
 	if (delta) {
-		pulses = atomic_read(&ctx->pulses);
-		atomic_sub(pulses, &ctx->pulses);
-		ctx->rpm = (unsigned int)(pulses * 1000 * 60) /
-			(ctx->pulses_per_revolution * delta);
+		pulses = atomic_read(&tach->pulses);
+		atomic_sub(pulses, &tach->pulses);
+		tach->rpm = (unsigned int)(pulses * 1000 * 60) /
+			(tach->pulses_per_revolution * delta);
 
 		ctx->sample_start = ktime_get();
 	}
@@ -152,7 +157,7 @@ static int pwm_fan_read(struct device *dev, enum hwmon_sensor_types type,
 		return 0;
 
 	case hwmon_fan:
-		*val = ctx->rpm;
+		*val = ctx->tach->rpm;
 		return 0;
 
 	default:
@@ -362,14 +367,21 @@ static int pwm_fan_probe(struct platform_device *pdev)
 	channels[0] = &pwm_fan_channel_pwm;
 
 	if (tach_count > 0) {
+		struct pwm_fan_tach *tach;
 		u32 ppr = 2;
 
-		ctx->irq = platform_get_irq(pdev, 0);
-		if (ctx->irq == -EPROBE_DEFER)
-			return ctx->irq;
-		if (ctx->irq > 0) {
-			ret = devm_request_irq(dev, ctx->irq, pulse_handler, 0,
-					       pdev->name, ctx);
+		tach = devm_kzalloc(dev, sizeof(struct pwm_fan_tach),
+					 GFP_KERNEL);
+		if (!tach)
+			return -ENOMEM;
+		ctx->tach = tach;
+
+		tach->irq = platform_get_irq(pdev, 0);
+		if (tach->irq == -EPROBE_DEFER)
+			return tach->irq;
+		if (tach->irq > 0) {
+			ret = devm_request_irq(dev, tach->irq, pulse_handler, 0,
+					       pdev->name, tach);
 			if (ret) {
 				dev_err(dev,
 					"Failed to request interrupt: %d\n",
@@ -381,14 +393,14 @@ static int pwm_fan_probe(struct platform_device *pdev)
 		of_property_read_u32(dev->of_node,
 				     "pulses-per-revolution",
 				     &ppr);
-		ctx->pulses_per_revolution = ppr;
-		if (!ctx->pulses_per_revolution) {
+		tach->pulses_per_revolution = ppr;
+		if (!tach->pulses_per_revolution) {
 			dev_err(dev, "pulses-per-revolution can't be zero.\n");
 			return -EINVAL;
 		}
 
 		dev_dbg(dev, "tach: irq=%d, pulses_per_revolution=%d\n",
-			ctx->irq, ctx->pulses_per_revolution);
+			tach->irq, tach->pulses_per_revolution);
 
 		ctx->sample_start = ktime_get();
 		mod_timer(&ctx->rpm_timer, jiffies + HZ);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 2/2] hwmon: pwm-fan: Support multiple fan tachometers
  2020-12-12 19:50 [PATCH v3 0/2] pwm-fan: Support multiple tachometer inputs Paul Barker
  2020-12-12 19:50 ` [PATCH v3 1/2] hwmon: pwm-fan: Store tach data separately Paul Barker
@ 2020-12-12 19:50 ` Paul Barker
  2020-12-30 16:18   ` Guenter Roeck
  1 sibling, 1 reply; 5+ messages in thread
From: Paul Barker @ 2020-12-12 19:50 UTC (permalink / raw)
  To: Kamil Debski, Bartlomiej Zolnierkiewicz, Jean Delvare, Guenter Roeck
  Cc: Paul Barker, linux-hwmon

The pwm-fan driver is extended to support multiple fan tachometer
signals connected to GPIO inputs. This is intended to support the case
where a single PWM output signal is routed to multiple fans, each of
which have a tachometer output connected back to a GPIO pin.

The number of fan tachometer inputs is determined by the number of
interrupt sources configured for the pwm-fan device. The number of
pulses-per-revolution entries should match the number of interrupt
sources so that each input has a value assigned.

The fan tachometer measurements are exposed as sysfs files fan1_input,
fan2_input, etc up to the number of configured inputs.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 drivers/hwmon/pwm-fan.c | 91 ++++++++++++++++++++++++-----------------
 1 file changed, 53 insertions(+), 38 deletions(-)

diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 4a7f0e87b08f..cc64bae0ffa8 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -33,7 +33,8 @@ struct pwm_fan_ctx {
 	struct pwm_device *pwm;
 	struct regulator *reg_en;
 
-	struct pwm_fan_tach *tach;
+	int tach_count;
+	struct pwm_fan_tach *tachs;
 	ktime_t sample_start;
 	struct timer_list rpm_timer;
 
@@ -44,6 +45,7 @@ struct pwm_fan_ctx {
 	struct thermal_cooling_device *cdev;
 
 	struct hwmon_chip_info info;
+	struct hwmon_channel_info fan_channel;
 };
 
 static const u32 pwm_fan_channel_config_pwm[] = {
@@ -56,16 +58,6 @@ static const struct hwmon_channel_info pwm_fan_channel_pwm = {
 	.config = pwm_fan_channel_config_pwm,
 };
 
-static const u32 pwm_fan_channel_config_fan[] = {
-	HWMON_F_INPUT,
-	0
-};
-
-static const struct hwmon_channel_info pwm_fan_channel_fan = {
-	.type = hwmon_fan,
-	.config = pwm_fan_channel_config_fan,
-};
-
 /* This handler assumes self resetting edge triggered interrupt. */
 static irqreturn_t pulse_handler(int irq, void *dev_id)
 {
@@ -79,15 +71,19 @@ static irqreturn_t pulse_handler(int irq, void *dev_id)
 static void sample_timer(struct timer_list *t)
 {
 	struct pwm_fan_ctx *ctx = from_timer(ctx, t, rpm_timer);
-	struct pwm_fan_tach *tach = ctx->tach;
 	unsigned int delta = ktime_ms_delta(ktime_get(), ctx->sample_start);
-	int pulses;
+	int i;
 
 	if (delta) {
-		pulses = atomic_read(&tach->pulses);
-		atomic_sub(pulses, &tach->pulses);
-		tach->rpm = (unsigned int)(pulses * 1000 * 60) /
-			(tach->pulses_per_revolution * delta);
+		for (i = 0; i < ctx->tach_count; i++) {
+			struct pwm_fan_tach *tach = &ctx->tachs[i];
+			int pulses;
+
+			pulses = atomic_read(&tach->pulses);
+			atomic_sub(pulses, &tach->pulses);
+			tach->rpm = (unsigned int)(pulses * 1000 * 60) /
+				(tach->pulses_per_revolution * delta);
+		}
 
 		ctx->sample_start = ktime_get();
 	}
@@ -157,7 +153,7 @@ static int pwm_fan_read(struct device *dev, enum hwmon_sensor_types type,
 		return 0;
 
 	case hwmon_fan:
-		*val = ctx->tach->rpm;
+		*val = ctx->tachs[channel].rpm;
 		return 0;
 
 	default:
@@ -304,8 +300,10 @@ static int pwm_fan_probe(struct platform_device *pdev)
 	struct device *hwmon;
 	int ret;
 	struct pwm_state state = { };
-	int tach_count;
 	const struct hwmon_channel_info **channels;
+	u32 *fan_channel_config;
+	int channel_count = 1;	/* We always have a PWM channel. */
+	int i;
 
 	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
@@ -354,29 +352,41 @@ static int pwm_fan_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	tach_count = platform_irq_count(pdev);
-	if (tach_count < 0)
-		return dev_err_probe(dev, tach_count,
+	ctx->tach_count = platform_irq_count(pdev);
+	if (ctx->tach_count < 0)
+		return dev_err_probe(dev, ctx->tach_count,
 				     "Could not get number of fan tachometer inputs\n");
+	dev_dbg(dev, "%d fan tachometer inputs\n", ctx->tach_count);
 
-	channels = devm_kcalloc(dev, tach_count + 2,
+	if (ctx->tach_count) {
+		channel_count++;	/* We also have a FAN channel. */
+
+		ctx->tachs = devm_kcalloc(dev, ctx->tach_count,
+					  sizeof(struct pwm_fan_tach),
+					  GFP_KERNEL);
+		if (!ctx->tachs)
+			return -ENOMEM;
+
+		ctx->fan_channel.type = hwmon_fan;
+		fan_channel_config = devm_kcalloc(dev, ctx->tach_count + 1,
+						  sizeof(u32), GFP_KERNEL);
+		if (!fan_channel_config)
+			return -ENOMEM;
+		ctx->fan_channel.config = fan_channel_config;
+	}
+
+	channels = devm_kcalloc(dev, channel_count + 1,
 				sizeof(struct hwmon_channel_info *), GFP_KERNEL);
 	if (!channels)
 		return -ENOMEM;
 
 	channels[0] = &pwm_fan_channel_pwm;
 
-	if (tach_count > 0) {
-		struct pwm_fan_tach *tach;
+	for (i = 0; i < ctx->tach_count; i++) {
+		struct pwm_fan_tach *tach = &ctx->tachs[i];
 		u32 ppr = 2;
 
-		tach = devm_kzalloc(dev, sizeof(struct pwm_fan_tach),
-					 GFP_KERNEL);
-		if (!tach)
-			return -ENOMEM;
-		ctx->tach = tach;
-
-		tach->irq = platform_get_irq(pdev, 0);
+		tach->irq = platform_get_irq(pdev, i);
 		if (tach->irq == -EPROBE_DEFER)
 			return tach->irq;
 		if (tach->irq > 0) {
@@ -390,22 +400,27 @@ static int pwm_fan_probe(struct platform_device *pdev)
 			}
 		}
 
-		of_property_read_u32(dev->of_node,
-				     "pulses-per-revolution",
-				     &ppr);
+		of_property_read_u32_index(dev->of_node,
+					   "pulses-per-revolution",
+					   i,
+					   &ppr);
 		tach->pulses_per_revolution = ppr;
 		if (!tach->pulses_per_revolution) {
 			dev_err(dev, "pulses-per-revolution can't be zero.\n");
 			return -EINVAL;
 		}
 
-		dev_dbg(dev, "tach: irq=%d, pulses_per_revolution=%d\n",
-			tach->irq, tach->pulses_per_revolution);
+		fan_channel_config[i] = HWMON_F_INPUT;
+
+		dev_dbg(dev, "tach%d: irq=%d, pulses_per_revolution=%d\n",
+			i, tach->irq, tach->pulses_per_revolution);
+	}
 
+	if (ctx->tach_count > 0) {
 		ctx->sample_start = ktime_get();
 		mod_timer(&ctx->rpm_timer, jiffies + HZ);
 
-		channels[1] = &pwm_fan_channel_fan;
+		channels[1] = &ctx->fan_channel;
 	}
 
 	ctx->info.ops = &pwm_fan_hwmon_ops;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/2] hwmon: pwm-fan: Store tach data separately
  2020-12-12 19:50 ` [PATCH v3 1/2] hwmon: pwm-fan: Store tach data separately Paul Barker
@ 2020-12-30 16:18   ` Guenter Roeck
  0 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2020-12-30 16:18 UTC (permalink / raw)
  To: Paul Barker
  Cc: Kamil Debski, Bartlomiej Zolnierkiewicz, Jean Delvare, linux-hwmon

On Sat, Dec 12, 2020 at 07:50:07PM +0000, Paul Barker wrote:
> The data for the (optional) fan tachometer input is moved to a separate
> structure which is only allocated if an input is actually configured.
> 
> After this change the pulse IRQ handler takes a pointer to the
> tachometer data structure instead of the whole device context.
> 
> Signed-off-by: Paul Barker <pbarker@konsulko.com>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/pwm-fan.c | 52 +++++++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 777439f48c14..4a7f0e87b08f 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -21,15 +21,19 @@
>  
>  #define MAX_PWM 255
>  
> +struct pwm_fan_tach {
> +	int irq;
> +	atomic_t pulses;
> +	unsigned int rpm;
> +	u8 pulses_per_revolution;
> +};
> +
>  struct pwm_fan_ctx {
>  	struct mutex lock;
>  	struct pwm_device *pwm;
>  	struct regulator *reg_en;
>  
> -	int irq;
> -	atomic_t pulses;
> -	unsigned int rpm;
> -	u8 pulses_per_revolution;
> +	struct pwm_fan_tach *tach;
>  	ktime_t sample_start;
>  	struct timer_list rpm_timer;
>  
> @@ -65,9 +69,9 @@ static const struct hwmon_channel_info pwm_fan_channel_fan = {
>  /* This handler assumes self resetting edge triggered interrupt. */
>  static irqreturn_t pulse_handler(int irq, void *dev_id)
>  {
> -	struct pwm_fan_ctx *ctx = dev_id;
> +	struct pwm_fan_tach *tach = dev_id;
>  
> -	atomic_inc(&ctx->pulses);
> +	atomic_inc(&tach->pulses);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -75,14 +79,15 @@ static irqreturn_t pulse_handler(int irq, void *dev_id)
>  static void sample_timer(struct timer_list *t)
>  {
>  	struct pwm_fan_ctx *ctx = from_timer(ctx, t, rpm_timer);
> +	struct pwm_fan_tach *tach = ctx->tach;
>  	unsigned int delta = ktime_ms_delta(ktime_get(), ctx->sample_start);
>  	int pulses;
>  
>  	if (delta) {
> -		pulses = atomic_read(&ctx->pulses);
> -		atomic_sub(pulses, &ctx->pulses);
> -		ctx->rpm = (unsigned int)(pulses * 1000 * 60) /
> -			(ctx->pulses_per_revolution * delta);
> +		pulses = atomic_read(&tach->pulses);
> +		atomic_sub(pulses, &tach->pulses);
> +		tach->rpm = (unsigned int)(pulses * 1000 * 60) /
> +			(tach->pulses_per_revolution * delta);
>  
>  		ctx->sample_start = ktime_get();
>  	}
> @@ -152,7 +157,7 @@ static int pwm_fan_read(struct device *dev, enum hwmon_sensor_types type,
>  		return 0;
>  
>  	case hwmon_fan:
> -		*val = ctx->rpm;
> +		*val = ctx->tach->rpm;
>  		return 0;
>  
>  	default:
> @@ -362,14 +367,21 @@ static int pwm_fan_probe(struct platform_device *pdev)
>  	channels[0] = &pwm_fan_channel_pwm;
>  
>  	if (tach_count > 0) {
> +		struct pwm_fan_tach *tach;
>  		u32 ppr = 2;
>  
> -		ctx->irq = platform_get_irq(pdev, 0);
> -		if (ctx->irq == -EPROBE_DEFER)
> -			return ctx->irq;
> -		if (ctx->irq > 0) {
> -			ret = devm_request_irq(dev, ctx->irq, pulse_handler, 0,
> -					       pdev->name, ctx);
> +		tach = devm_kzalloc(dev, sizeof(struct pwm_fan_tach),
> +					 GFP_KERNEL);
> +		if (!tach)
> +			return -ENOMEM;
> +		ctx->tach = tach;
> +
> +		tach->irq = platform_get_irq(pdev, 0);
> +		if (tach->irq == -EPROBE_DEFER)
> +			return tach->irq;
> +		if (tach->irq > 0) {
> +			ret = devm_request_irq(dev, tach->irq, pulse_handler, 0,
> +					       pdev->name, tach);
>  			if (ret) {
>  				dev_err(dev,
>  					"Failed to request interrupt: %d\n",
> @@ -381,14 +393,14 @@ static int pwm_fan_probe(struct platform_device *pdev)
>  		of_property_read_u32(dev->of_node,
>  				     "pulses-per-revolution",
>  				     &ppr);
> -		ctx->pulses_per_revolution = ppr;
> -		if (!ctx->pulses_per_revolution) {
> +		tach->pulses_per_revolution = ppr;
> +		if (!tach->pulses_per_revolution) {
>  			dev_err(dev, "pulses-per-revolution can't be zero.\n");
>  			return -EINVAL;
>  		}
>  
>  		dev_dbg(dev, "tach: irq=%d, pulses_per_revolution=%d\n",
> -			ctx->irq, ctx->pulses_per_revolution);
> +			tach->irq, tach->pulses_per_revolution);
>  
>  		ctx->sample_start = ktime_get();
>  		mod_timer(&ctx->rpm_timer, jiffies + HZ);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 2/2] hwmon: pwm-fan: Support multiple fan tachometers
  2020-12-12 19:50 ` [PATCH v3 2/2] hwmon: pwm-fan: Support multiple fan tachometers Paul Barker
@ 2020-12-30 16:18   ` Guenter Roeck
  0 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2020-12-30 16:18 UTC (permalink / raw)
  To: Paul Barker
  Cc: Kamil Debski, Bartlomiej Zolnierkiewicz, Jean Delvare, linux-hwmon

On Sat, Dec 12, 2020 at 07:50:08PM +0000, Paul Barker wrote:
> The pwm-fan driver is extended to support multiple fan tachometer
> signals connected to GPIO inputs. This is intended to support the case
> where a single PWM output signal is routed to multiple fans, each of
> which have a tachometer output connected back to a GPIO pin.
> 
> The number of fan tachometer inputs is determined by the number of
> interrupt sources configured for the pwm-fan device. The number of
> pulses-per-revolution entries should match the number of interrupt
> sources so that each input has a value assigned.
> 
> The fan tachometer measurements are exposed as sysfs files fan1_input,
> fan2_input, etc up to the number of configured inputs.
> 
> Signed-off-by: Paul Barker <pbarker@konsulko.com>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/pwm-fan.c | 91 ++++++++++++++++++++++++-----------------
>  1 file changed, 53 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 4a7f0e87b08f..cc64bae0ffa8 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -33,7 +33,8 @@ struct pwm_fan_ctx {
>  	struct pwm_device *pwm;
>  	struct regulator *reg_en;
>  
> -	struct pwm_fan_tach *tach;
> +	int tach_count;
> +	struct pwm_fan_tach *tachs;
>  	ktime_t sample_start;
>  	struct timer_list rpm_timer;
>  
> @@ -44,6 +45,7 @@ struct pwm_fan_ctx {
>  	struct thermal_cooling_device *cdev;
>  
>  	struct hwmon_chip_info info;
> +	struct hwmon_channel_info fan_channel;
>  };
>  
>  static const u32 pwm_fan_channel_config_pwm[] = {
> @@ -56,16 +58,6 @@ static const struct hwmon_channel_info pwm_fan_channel_pwm = {
>  	.config = pwm_fan_channel_config_pwm,
>  };
>  
> -static const u32 pwm_fan_channel_config_fan[] = {
> -	HWMON_F_INPUT,
> -	0
> -};
> -
> -static const struct hwmon_channel_info pwm_fan_channel_fan = {
> -	.type = hwmon_fan,
> -	.config = pwm_fan_channel_config_fan,
> -};
> -
>  /* This handler assumes self resetting edge triggered interrupt. */
>  static irqreturn_t pulse_handler(int irq, void *dev_id)
>  {
> @@ -79,15 +71,19 @@ static irqreturn_t pulse_handler(int irq, void *dev_id)
>  static void sample_timer(struct timer_list *t)
>  {
>  	struct pwm_fan_ctx *ctx = from_timer(ctx, t, rpm_timer);
> -	struct pwm_fan_tach *tach = ctx->tach;
>  	unsigned int delta = ktime_ms_delta(ktime_get(), ctx->sample_start);
> -	int pulses;
> +	int i;
>  
>  	if (delta) {
> -		pulses = atomic_read(&tach->pulses);
> -		atomic_sub(pulses, &tach->pulses);
> -		tach->rpm = (unsigned int)(pulses * 1000 * 60) /
> -			(tach->pulses_per_revolution * delta);
> +		for (i = 0; i < ctx->tach_count; i++) {
> +			struct pwm_fan_tach *tach = &ctx->tachs[i];
> +			int pulses;
> +
> +			pulses = atomic_read(&tach->pulses);
> +			atomic_sub(pulses, &tach->pulses);
> +			tach->rpm = (unsigned int)(pulses * 1000 * 60) /
> +				(tach->pulses_per_revolution * delta);
> +		}
>  
>  		ctx->sample_start = ktime_get();
>  	}
> @@ -157,7 +153,7 @@ static int pwm_fan_read(struct device *dev, enum hwmon_sensor_types type,
>  		return 0;
>  
>  	case hwmon_fan:
> -		*val = ctx->tach->rpm;
> +		*val = ctx->tachs[channel].rpm;
>  		return 0;
>  
>  	default:
> @@ -304,8 +300,10 @@ static int pwm_fan_probe(struct platform_device *pdev)
>  	struct device *hwmon;
>  	int ret;
>  	struct pwm_state state = { };
> -	int tach_count;
>  	const struct hwmon_channel_info **channels;
> +	u32 *fan_channel_config;
> +	int channel_count = 1;	/* We always have a PWM channel. */
> +	int i;
>  
>  	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>  	if (!ctx)
> @@ -354,29 +352,41 @@ static int pwm_fan_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	tach_count = platform_irq_count(pdev);
> -	if (tach_count < 0)
> -		return dev_err_probe(dev, tach_count,
> +	ctx->tach_count = platform_irq_count(pdev);
> +	if (ctx->tach_count < 0)
> +		return dev_err_probe(dev, ctx->tach_count,
>  				     "Could not get number of fan tachometer inputs\n");
> +	dev_dbg(dev, "%d fan tachometer inputs\n", ctx->tach_count);
>  
> -	channels = devm_kcalloc(dev, tach_count + 2,
> +	if (ctx->tach_count) {
> +		channel_count++;	/* We also have a FAN channel. */
> +
> +		ctx->tachs = devm_kcalloc(dev, ctx->tach_count,
> +					  sizeof(struct pwm_fan_tach),
> +					  GFP_KERNEL);
> +		if (!ctx->tachs)
> +			return -ENOMEM;
> +
> +		ctx->fan_channel.type = hwmon_fan;
> +		fan_channel_config = devm_kcalloc(dev, ctx->tach_count + 1,
> +						  sizeof(u32), GFP_KERNEL);
> +		if (!fan_channel_config)
> +			return -ENOMEM;
> +		ctx->fan_channel.config = fan_channel_config;
> +	}
> +
> +	channels = devm_kcalloc(dev, channel_count + 1,
>  				sizeof(struct hwmon_channel_info *), GFP_KERNEL);
>  	if (!channels)
>  		return -ENOMEM;
>  
>  	channels[0] = &pwm_fan_channel_pwm;
>  
> -	if (tach_count > 0) {
> -		struct pwm_fan_tach *tach;
> +	for (i = 0; i < ctx->tach_count; i++) {
> +		struct pwm_fan_tach *tach = &ctx->tachs[i];
>  		u32 ppr = 2;
>  
> -		tach = devm_kzalloc(dev, sizeof(struct pwm_fan_tach),
> -					 GFP_KERNEL);
> -		if (!tach)
> -			return -ENOMEM;
> -		ctx->tach = tach;
> -
> -		tach->irq = platform_get_irq(pdev, 0);
> +		tach->irq = platform_get_irq(pdev, i);
>  		if (tach->irq == -EPROBE_DEFER)
>  			return tach->irq;
>  		if (tach->irq > 0) {
> @@ -390,22 +400,27 @@ static int pwm_fan_probe(struct platform_device *pdev)
>  			}
>  		}
>  
> -		of_property_read_u32(dev->of_node,
> -				     "pulses-per-revolution",
> -				     &ppr);
> +		of_property_read_u32_index(dev->of_node,
> +					   "pulses-per-revolution",
> +					   i,
> +					   &ppr);
>  		tach->pulses_per_revolution = ppr;
>  		if (!tach->pulses_per_revolution) {
>  			dev_err(dev, "pulses-per-revolution can't be zero.\n");
>  			return -EINVAL;
>  		}
>  
> -		dev_dbg(dev, "tach: irq=%d, pulses_per_revolution=%d\n",
> -			tach->irq, tach->pulses_per_revolution);
> +		fan_channel_config[i] = HWMON_F_INPUT;
> +
> +		dev_dbg(dev, "tach%d: irq=%d, pulses_per_revolution=%d\n",
> +			i, tach->irq, tach->pulses_per_revolution);
> +	}
>  
> +	if (ctx->tach_count > 0) {
>  		ctx->sample_start = ktime_get();
>  		mod_timer(&ctx->rpm_timer, jiffies + HZ);
>  
> -		channels[1] = &pwm_fan_channel_fan;
> +		channels[1] = &ctx->fan_channel;
>  	}
>  
>  	ctx->info.ops = &pwm_fan_hwmon_ops;

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-12-30 16:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-12 19:50 [PATCH v3 0/2] pwm-fan: Support multiple tachometer inputs Paul Barker
2020-12-12 19:50 ` [PATCH v3 1/2] hwmon: pwm-fan: Store tach data separately Paul Barker
2020-12-30 16:18   ` Guenter Roeck
2020-12-12 19:50 ` [PATCH v3 2/2] hwmon: pwm-fan: Support multiple fan tachometers Paul Barker
2020-12-30 16:18   ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).