Hi, I forgot to address one of the comments. > From: Guenter Roeck [mailto:groeck7@xxxxxxxxx] On Behalf Of Guenter > Roeck > Sent: Wednesday, July 09, 2014 6:58 PM > To: Kamil Debski > Cc: devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; lm- > sensors@xxxxxxxxxxxxxx; t.figa@xxxxxxxxxxx; m.szyprowski@xxxxxxxxxxx > Subject: Re: [lm-sensors] [PATCH] hwmon: pwm-fan: Add pwm-fan driver > > On Wed, Jul 09, 2014 at 04:53:20PM +0200, Kamil Debski wrote: > > The pwm-fan driver enables control of fans connected to PWM lines. > > This driver uses the PWM framework, so it is compatible with all PWM > > devices that provide drivers through the PWM framework. > > > > Signed-off-by: Kamil Debski <k.debski@xxxxxxxxxxx> > > --- > > .../devicetree/bindings/hwmon/pwm-fan.txt | 12 ++ > > drivers/hwmon/Kconfig | 9 + > > drivers/hwmon/Makefile | 1 + > > drivers/hwmon/pwm-fan.c | 199 <snip> > > +static int pwm_fan_remove(struct platform_device *pdev) { > > + struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev); > > + > > + pwm_disable(ctx->pwm); > > Is this needed ? I don't see it used by the leds-pwm driver on cleanup. The pwm-samsung driver does not disable the PWM channel according to my understanding of the code. But for example the pwm-renesas-tpu does stop the PWM timer on free. What is the correct behavior? Is the channel should be disabled on free then I agree that pwm_disable is not necessary. > > + hwmon_device_unregister(ctx->hwmon); > > + > > + return 0; > > +} > > + <snip> Best wishes, -- Kamil Debski Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html