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

[Kamil Debski]

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 linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/