diff mbox series

[v1,1/3] hwmon: (pwm-fan) Make use of device properties

Message ID 20220806152517.78159-1-andriy.shevchenko@linux.intel.com (mailing list archive)
State Changes Requested
Headers show
Series [v1,1/3] hwmon: (pwm-fan) Make use of device properties | expand

Commit Message

Andy Shevchenko Aug. 6, 2022, 3:25 p.m. UTC
Convert the module to be property provider agnostic and allow
it to be used on non-OF platforms.

Add mod_devicetable.h include.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/hwmon/Kconfig   |  2 +-
 drivers/hwmon/pwm-fan.c | 50 +++++++++++++++++++++--------------------
 2 files changed, 27 insertions(+), 25 deletions(-)

Comments

Uwe Kleine-König Aug. 7, 2022, 8:34 a.m. UTC | #1
Hello,

[dropped Bartlomiej Zolnierkiewicz from Cc:, the address bounced in the
past]

at a quick glance this looks nice. I wonder if it makes sense to split
the patch. For example the change

-	ctx->pwm = devm_of_pwm_get(dev, dev->of_node, NULL);
+	ctx->pwm = devm_pwm_get(dev, NULL);

could stand alone. Also I think this change is the relevant part in
patch #1 that makes patches #2 and #3 possible.

When this patch doesn't get split, the series needs some coordination,
as patch #1 is for hwmon and patches #2 and #3 are for pwm.

Splitting the series into:

	hwmon: (pwm-fan) Use of devm_pwm_get() instead of devm_of_pwm_get()
	pwm: core: Get rid of unused devm_of_pwm_get()
	pwm: core: Make of_pwm_get() static

for pwm and the remainder of this patch for hwmon might make application
of the changes here easier to coordinate.

But still: Thanks for your effort and
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe
Andy Shevchenko Aug. 7, 2022, 9:20 a.m. UTC | #2
On Sun, Aug 7, 2022 at 10:38 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

> at a quick glance this looks nice. I wonder if it makes sense to split
> the patch. For example the change
>
> -       ctx->pwm = devm_of_pwm_get(dev, dev->of_node, NULL);
> +       ctx->pwm = devm_pwm_get(dev, NULL);
>
> could stand alone. Also I think this change is the relevant part in
> patch #1 that makes patches #2 and #3 possible.

True.

> When this patch doesn't get split, the series needs some coordination,
> as patch #1 is for hwmon and patches #2 and #3 are for pwm.
>
> Splitting the series into:
>
>         hwmon: (pwm-fan) Use of devm_pwm_get() instead of devm_of_pwm_get()
>         pwm: core: Get rid of unused devm_of_pwm_get()
>         pwm: core: Make of_pwm_get() static
>
> for pwm and the remainder of this patch for hwmon might make application
> of the changes here easier to coordinate.

Either way it will need the hwmon maintainer ACKs or alike.
Since we have (plenty of) time I will wait a bit for hwmon maintainers
to react. Guenter, what would you prefer?

> But still: Thanks for your effort and
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks for looking into the series related to PWM core clean up!
Guenter Roeck Aug. 9, 2022, 7:43 p.m. UTC | #3
On 8/7/22 02:20, Andy Shevchenko wrote:
> On Sun, Aug 7, 2022 at 10:38 AM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> 
>> at a quick glance this looks nice. I wonder if it makes sense to split
>> the patch. For example the change
>>
>> -       ctx->pwm = devm_of_pwm_get(dev, dev->of_node, NULL);
>> +       ctx->pwm = devm_pwm_get(dev, NULL);
>>
>> could stand alone. Also I think this change is the relevant part in
>> patch #1 that makes patches #2 and #3 possible.
> 
> True.
> 
>> When this patch doesn't get split, the series needs some coordination,
>> as patch #1 is for hwmon and patches #2 and #3 are for pwm.
>>
>> Splitting the series into:
>>
>>          hwmon: (pwm-fan) Use of devm_pwm_get() instead of devm_of_pwm_get()
>>          pwm: core: Get rid of unused devm_of_pwm_get()
>>          pwm: core: Make of_pwm_get() static
>>
>> for pwm and the remainder of this patch for hwmon might make application
>> of the changes here easier to coordinate.
> 
> Either way it will need the hwmon maintainer ACKs or alike.
> Since we have (plenty of) time I will wait a bit for hwmon maintainers
> to react. Guenter, what would you prefer?
> 

I have a substantial number of patches pending for the pwm-fan driver.
Some of those will conflict with this patch. I'll have to spend more time
to be able to understand the implications.

Guenter
Guenter Roeck Aug. 18, 2022, 11:20 p.m. UTC | #4
On Sat, Aug 06, 2022 at 06:25:15PM +0300, Andy Shevchenko wrote:
> Convert the module to be property provider agnostic and allow
> it to be used on non-OF platforms.
> 
> Add mod_devicetable.h include.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

I had another look at this patch. A substantial part of the changes
is because device properties don't support of_property_read_u32_index(),
reworking the code to use device_property_read_u32_array() instead.
Sorry, I don't like it, it results in a substantial number of unnecessary
changes. Device properties should support the equivalent of
of_property_read_u32_index() instead to simplify conversions.

Guenter

> ---
>  drivers/hwmon/Kconfig   |  2 +-
>  drivers/hwmon/pwm-fan.c | 50 +++++++++++++++++++++--------------------
>  2 files changed, 27 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index e70d9614bec2..58912a5c5de8 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1613,7 +1613,7 @@ source "drivers/hwmon/pmbus/Kconfig"
>  
>  config SENSORS_PWM_FAN
>  	tristate "PWM fan"
> -	depends on (PWM && OF) || COMPILE_TEST
> +	depends on PWM || COMPILE_TEST
>  	depends on THERMAL || THERMAL=n
>  	help
>  	  If you say yes here you get support for fans connected to PWM lines.
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 6c08551d8d14..9ce9f2543861 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -9,10 +9,11 @@
>  
>  #include <linux/hwmon.h>
>  #include <linux/interrupt.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> -#include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/property.h>
>  #include <linux/pwm.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/sysfs.h>
> @@ -25,7 +26,6 @@ struct pwm_fan_tach {
>  	int irq;
>  	atomic_t pulses;
>  	unsigned int rpm;
> -	u8 pulses_per_revolution;
>  };
>  
>  struct pwm_fan_ctx {
> @@ -36,6 +36,7 @@ struct pwm_fan_ctx {
>  
>  	int tach_count;
>  	struct pwm_fan_tach *tachs;
> +	u32 *pulses_per_revolution;
>  	ktime_t sample_start;
>  	struct timer_list rpm_timer;
>  
> @@ -73,7 +74,7 @@ static void sample_timer(struct timer_list *t)
>  			pulses = atomic_read(&tach->pulses);
>  			atomic_sub(pulses, &tach->pulses);
>  			tach->rpm = (unsigned int)(pulses * 1000 * 60) /
> -				(tach->pulses_per_revolution * delta);
> +				(ctx->pulses_per_revolution[i] * delta);
>  		}
>  
>  		ctx->sample_start = ktime_get();
> @@ -229,16 +230,14 @@ static const struct thermal_cooling_device_ops pwm_fan_cooling_ops = {
>  	.set_cur_state = pwm_fan_set_cur_state,
>  };
>  
> -static int pwm_fan_of_get_cooling_data(struct device *dev,
> -				       struct pwm_fan_ctx *ctx)
> +static int pwm_fan_get_cooling_data(struct device *dev, struct pwm_fan_ctx *ctx)
>  {
> -	struct device_node *np = dev->of_node;
>  	int num, i, ret;
>  
> -	if (!of_find_property(np, "cooling-levels", NULL))
> +	if (!device_property_present(dev, "cooling-levels"))
>  		return 0;
>  
> -	ret = of_property_count_u32_elems(np, "cooling-levels");
> +	ret = device_property_count_u32(dev, "cooling-levels");
>  	if (ret <= 0) {
>  		dev_err(dev, "Wrong data!\n");
>  		return ret ? : -EINVAL;
> @@ -250,8 +249,8 @@ static int pwm_fan_of_get_cooling_data(struct device *dev,
>  	if (!ctx->pwm_fan_cooling_levels)
>  		return -ENOMEM;
>  
> -	ret = of_property_read_u32_array(np, "cooling-levels",
> -					 ctx->pwm_fan_cooling_levels, num);
> +	ret = device_property_read_u32_array(dev, "cooling-levels",
> +					     ctx->pwm_fan_cooling_levels, num);
>  	if (ret) {
>  		dev_err(dev, "Property 'cooling-levels' cannot be read!\n");
>  		return ret;
> @@ -302,7 +301,7 @@ static int pwm_fan_probe(struct platform_device *pdev)
>  
>  	mutex_init(&ctx->lock);
>  
> -	ctx->pwm = devm_of_pwm_get(dev, dev->of_node, NULL);
> +	ctx->pwm = devm_pwm_get(dev, NULL);
>  	if (IS_ERR(ctx->pwm))
>  		return dev_err_probe(dev, PTR_ERR(ctx->pwm), "Could not get PWM\n");
>  
> @@ -370,6 +369,20 @@ static int pwm_fan_probe(struct platform_device *pdev)
>  		if (!fan_channel_config)
>  			return -ENOMEM;
>  		ctx->fan_channel.config = fan_channel_config;
> +
> +		ctx->pulses_per_revolution = devm_kmalloc_array(dev,
> +								ctx->tach_count,
> +								sizeof(*ctx->pulses_per_revolution),
> +								GFP_KERNEL);
> +		if (!ctx->pulses_per_revolution)
> +			return -ENOMEM;
> +
> +		/* Setup default pulses per revolution */
> +		memset32(ctx->pulses_per_revolution, 2, ctx->tach_count);
> +
> +		device_property_read_u32_array(dev, "pulses-per-revolution",
> +					       ctx->pulses_per_revolution,
> +					       ctx->tach_count);
>  	}
>  
>  	channels = devm_kcalloc(dev, channel_count + 1,
> @@ -381,7 +394,6 @@ static int pwm_fan_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < ctx->tach_count; i++) {
>  		struct pwm_fan_tach *tach = &ctx->tachs[i];
> -		u32 ppr = 2;
>  
>  		tach->irq = platform_get_irq(pdev, i);
>  		if (tach->irq == -EPROBE_DEFER)
> @@ -397,20 +409,10 @@ static int pwm_fan_probe(struct platform_device *pdev)
>  			}
>  		}
>  
> -		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;
> -		}
> -
>  		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);
> +			i, tach->irq, ctx->pulses_per_revolution[i]);
>  	}
>  
>  	if (ctx->tach_count > 0) {
> @@ -430,7 +432,7 @@ static int pwm_fan_probe(struct platform_device *pdev)
>  		return PTR_ERR(hwmon);
>  	}
>  
> -	ret = pwm_fan_of_get_cooling_data(dev, ctx);
> +	ret = pwm_fan_get_cooling_data(dev, ctx);
>  	if (ret)
>  		return ret;
>
Andy Shevchenko Aug. 19, 2022, 9:56 a.m. UTC | #5
On Fri, Aug 19, 2022 at 2:41 AM Guenter Roeck <linux@roeck-us.net> wrote:
> On Sat, Aug 06, 2022 at 06:25:15PM +0300, Andy Shevchenko wrote:
> > Convert the module to be property provider agnostic and allow
> > it to be used on non-OF platforms.
> >
> > Add mod_devicetable.h include.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>
> I had another look at this patch. A substantial part of the changes
> is because device properties don't support of_property_read_u32_index(),
> reworking the code to use device_property_read_u32_array() instead.
> Sorry, I don't like it, it results in a substantial number of unnecessary
> changes. Device properties should support the equivalent of
> of_property_read_u32_index() instead to simplify conversions.

Not all (device property) providers can have such API available. Are
you suggesting to
 a) alloc memory for entire array;
 b) cache one for a given index;
 c) free a memory;
 d) loop as many times as index op is called.

Sorry, this is way too far and non-optimal in comparison to the
substantial number of unnecessary changes (two or three small
refactorings?).

Another way is to provide a pwm-fan-acpi, which will be the copy of
the driver after this patch is applied. I don't think it's a very
bright idea either.
Andy Shevchenko Aug. 19, 2022, 9:58 a.m. UTC | #6
On Fri, Aug 19, 2022 at 12:56 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Aug 19, 2022 at 2:41 AM Guenter Roeck <linux@roeck-us.net> wrote:
> > On Sat, Aug 06, 2022 at 06:25:15PM +0300, Andy Shevchenko wrote:
> > > Convert the module to be property provider agnostic and allow
> > > it to be used on non-OF platforms.
> > >
> > > Add mod_devicetable.h include.
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >
> > I had another look at this patch. A substantial part of the changes
> > is because device properties don't support of_property_read_u32_index(),
> > reworking the code to use device_property_read_u32_array() instead.
> > Sorry, I don't like it, it results in a substantial number of unnecessary
> > changes. Device properties should support the equivalent of
> > of_property_read_u32_index() instead to simplify conversions.
>
> Not all (device property) providers can have such API available. Are
> you suggesting to
>  a) alloc memory for entire array;
>  b) cache one for a given index;
>  c) free a memory;
>  d) loop as many times as index op is called.
>
> Sorry, this is way too far and non-optimal in comparison to the
> substantial number of unnecessary changes (two or three small
> refactorings?).
>
> Another way is to provide a pwm-fan-acpi, which will be the copy of
> the driver after this patch is applied. I don't think it's a very
> bright idea either.

That said, I will split a change for PWM cleaning up series and leave
the rest on the hwmon maintainers to reconsider.
Guenter Roeck Aug. 19, 2022, 1:09 p.m. UTC | #7
On Fri, Aug 19, 2022 at 12:56:42PM +0300, Andy Shevchenko wrote:
> On Fri, Aug 19, 2022 at 2:41 AM Guenter Roeck <linux@roeck-us.net> wrote:
> > On Sat, Aug 06, 2022 at 06:25:15PM +0300, Andy Shevchenko wrote:
> > > Convert the module to be property provider agnostic and allow
> > > it to be used on non-OF platforms.
> > >
> > > Add mod_devicetable.h include.
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >
> > I had another look at this patch. A substantial part of the changes
> > is because device properties don't support of_property_read_u32_index(),
> > reworking the code to use device_property_read_u32_array() instead.
> > Sorry, I don't like it, it results in a substantial number of unnecessary
> > changes. Device properties should support the equivalent of
> > of_property_read_u32_index() instead to simplify conversions.
> 
> Not all (device property) providers can have such API available. Are
> you suggesting to
>  a) alloc memory for entire array;
>  b) cache one for a given index;
>  c) free a memory;
>  d) loop as many times as index op is called.
> 
> Sorry, this is way too far and non-optimal in comparison to the
> substantial number of unnecessary changes (two or three small
> refactorings?).
> 
> Another way is to provide a pwm-fan-acpi, which will be the copy of
> the driver after this patch is applied. I don't think it's a very
> bright idea either.
> 
An alternative might be to split the patch in two parts, one replacing
of_property_read_u32_index() with of_property_read_u32_array() as
preparation, with the above rationale and a note that this is to
prepare for the switch to device properties, and then the actual device
property switch. Some context showing how other conversions handled this
problem would also be nice, though not necessary.

Thanks,
Guenter
Andy Shevchenko Aug. 19, 2022, 11:32 p.m. UTC | #8
On Fri, Aug 19, 2022 at 4:09 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Fri, Aug 19, 2022 at 12:56:42PM +0300, Andy Shevchenko wrote:
> > On Fri, Aug 19, 2022 at 2:41 AM Guenter Roeck <linux@roeck-us.net> wrote:
> > > On Sat, Aug 06, 2022 at 06:25:15PM +0300, Andy Shevchenko wrote:
> > > > Convert the module to be property provider agnostic and allow
> > > > it to be used on non-OF platforms.
> > > >
> > > > Add mod_devicetable.h include.
> > > >
> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > >
> > > I had another look at this patch. A substantial part of the changes
> > > is because device properties don't support of_property_read_u32_index(),
> > > reworking the code to use device_property_read_u32_array() instead.
> > > Sorry, I don't like it, it results in a substantial number of unnecessary
> > > changes. Device properties should support the equivalent of
> > > of_property_read_u32_index() instead to simplify conversions.
> >
> > Not all (device property) providers can have such API available. Are
> > you suggesting to
> >  a) alloc memory for entire array;
> >  b) cache one for a given index;
> >  c) free a memory;
> >  d) loop as many times as index op is called.
> >
> > Sorry, this is way too far and non-optimal in comparison to the
> > substantial number of unnecessary changes (two or three small
> > refactorings?).
> >
> > Another way is to provide a pwm-fan-acpi, which will be the copy of
> > the driver after this patch is applied. I don't think it's a very
> > bright idea either.
> >
> An alternative might be to split the patch in two parts, one replacing
> of_property_read_u32_index() with of_property_read_u32_array() as
> preparation, with the above rationale and a note that this is to
> prepare for the switch to device properties, and then the actual device
> property switch. Some context showing how other conversions handled this
> problem would also be nice, though not necessary.

Thanks for the idea, I like it and it would indeed simplify the
understanding of the changes made.
diff mbox series

Patch

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index e70d9614bec2..58912a5c5de8 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1613,7 +1613,7 @@  source "drivers/hwmon/pmbus/Kconfig"
 
 config SENSORS_PWM_FAN
 	tristate "PWM fan"
-	depends on (PWM && OF) || COMPILE_TEST
+	depends on PWM || COMPILE_TEST
 	depends on THERMAL || THERMAL=n
 	help
 	  If you say yes here you get support for fans connected to PWM lines.
diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 6c08551d8d14..9ce9f2543861 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -9,10 +9,11 @@ 
 
 #include <linux/hwmon.h>
 #include <linux/interrupt.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
-#include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/pwm.h>
 #include <linux/regulator/consumer.h>
 #include <linux/sysfs.h>
@@ -25,7 +26,6 @@  struct pwm_fan_tach {
 	int irq;
 	atomic_t pulses;
 	unsigned int rpm;
-	u8 pulses_per_revolution;
 };
 
 struct pwm_fan_ctx {
@@ -36,6 +36,7 @@  struct pwm_fan_ctx {
 
 	int tach_count;
 	struct pwm_fan_tach *tachs;
+	u32 *pulses_per_revolution;
 	ktime_t sample_start;
 	struct timer_list rpm_timer;
 
@@ -73,7 +74,7 @@  static void sample_timer(struct timer_list *t)
 			pulses = atomic_read(&tach->pulses);
 			atomic_sub(pulses, &tach->pulses);
 			tach->rpm = (unsigned int)(pulses * 1000 * 60) /
-				(tach->pulses_per_revolution * delta);
+				(ctx->pulses_per_revolution[i] * delta);
 		}
 
 		ctx->sample_start = ktime_get();
@@ -229,16 +230,14 @@  static const struct thermal_cooling_device_ops pwm_fan_cooling_ops = {
 	.set_cur_state = pwm_fan_set_cur_state,
 };
 
-static int pwm_fan_of_get_cooling_data(struct device *dev,
-				       struct pwm_fan_ctx *ctx)
+static int pwm_fan_get_cooling_data(struct device *dev, struct pwm_fan_ctx *ctx)
 {
-	struct device_node *np = dev->of_node;
 	int num, i, ret;
 
-	if (!of_find_property(np, "cooling-levels", NULL))
+	if (!device_property_present(dev, "cooling-levels"))
 		return 0;
 
-	ret = of_property_count_u32_elems(np, "cooling-levels");
+	ret = device_property_count_u32(dev, "cooling-levels");
 	if (ret <= 0) {
 		dev_err(dev, "Wrong data!\n");
 		return ret ? : -EINVAL;
@@ -250,8 +249,8 @@  static int pwm_fan_of_get_cooling_data(struct device *dev,
 	if (!ctx->pwm_fan_cooling_levels)
 		return -ENOMEM;
 
-	ret = of_property_read_u32_array(np, "cooling-levels",
-					 ctx->pwm_fan_cooling_levels, num);
+	ret = device_property_read_u32_array(dev, "cooling-levels",
+					     ctx->pwm_fan_cooling_levels, num);
 	if (ret) {
 		dev_err(dev, "Property 'cooling-levels' cannot be read!\n");
 		return ret;
@@ -302,7 +301,7 @@  static int pwm_fan_probe(struct platform_device *pdev)
 
 	mutex_init(&ctx->lock);
 
-	ctx->pwm = devm_of_pwm_get(dev, dev->of_node, NULL);
+	ctx->pwm = devm_pwm_get(dev, NULL);
 	if (IS_ERR(ctx->pwm))
 		return dev_err_probe(dev, PTR_ERR(ctx->pwm), "Could not get PWM\n");
 
@@ -370,6 +369,20 @@  static int pwm_fan_probe(struct platform_device *pdev)
 		if (!fan_channel_config)
 			return -ENOMEM;
 		ctx->fan_channel.config = fan_channel_config;
+
+		ctx->pulses_per_revolution = devm_kmalloc_array(dev,
+								ctx->tach_count,
+								sizeof(*ctx->pulses_per_revolution),
+								GFP_KERNEL);
+		if (!ctx->pulses_per_revolution)
+			return -ENOMEM;
+
+		/* Setup default pulses per revolution */
+		memset32(ctx->pulses_per_revolution, 2, ctx->tach_count);
+
+		device_property_read_u32_array(dev, "pulses-per-revolution",
+					       ctx->pulses_per_revolution,
+					       ctx->tach_count);
 	}
 
 	channels = devm_kcalloc(dev, channel_count + 1,
@@ -381,7 +394,6 @@  static int pwm_fan_probe(struct platform_device *pdev)
 
 	for (i = 0; i < ctx->tach_count; i++) {
 		struct pwm_fan_tach *tach = &ctx->tachs[i];
-		u32 ppr = 2;
 
 		tach->irq = platform_get_irq(pdev, i);
 		if (tach->irq == -EPROBE_DEFER)
@@ -397,20 +409,10 @@  static int pwm_fan_probe(struct platform_device *pdev)
 			}
 		}
 
-		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;
-		}
-
 		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);
+			i, tach->irq, ctx->pulses_per_revolution[i]);
 	}
 
 	if (ctx->tach_count > 0) {
@@ -430,7 +432,7 @@  static int pwm_fan_probe(struct platform_device *pdev)
 		return PTR_ERR(hwmon);
 	}
 
-	ret = pwm_fan_of_get_cooling_data(dev, ctx);
+	ret = pwm_fan_get_cooling_data(dev, ctx);
 	if (ret)
 		return ret;