RE: [lm-sensors] [PATCH] hwmon: pwm-fan: Add pwm-fan driver

[Kamil Debski <k.debski@samsung.com>]

Hi,

I forgot to address one of the comments.

> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter
> Roeck
> Sent: Wednesday, July 09, 2014 6:58 PM
> To: Kamil Debski
> Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; lm-
> sensors@lm-sensors.org; t.figa@samsung.com; m.szyprowski@samsung.com
> 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@samsung.com>
> > ---
> >  .../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