Messages in this thread | | | From | Sandipan Patra <> | Subject | RE: [PATCH 1/2] hwmon: pwm-fan: Add profile support and add remove module support | Date | Tue, 26 May 2020 12:08:14 +0000 |
| |
Hi,
> -----Original Message----- > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck > Sent: Tuesday, May 26, 2020 5:12 PM > To: Sandipan Patra <spatra@nvidia.com>; Thierry Reding > <treding@nvidia.com>; Jonathan Hunter <jonathanh@nvidia.com>; u.kleine- > koenig@pengutronix.de; kamil@wypas.org; jdelvare@suse.com; > robh+dt@kernel.org > Cc: Bibek Basu <bbasu@nvidia.com>; Bitan Biswas <bbiswas@nvidia.com>; > linux-pwm@vger.kernel.org; linux-hwmon@vger.kernel.org; > devicetree@vger.kernel.org; linux-tegra@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH 1/2] hwmon: pwm-fan: Add profile support and add remove > module support > > External email: Use caution opening links or attachments > > > On 5/25/20 10:06 PM, Sandipan Patra wrote: > > This change has 2 parts: > > 1. Add support for profiles mode settings. > > This allows different fan settings for trip point temp/hyst/pwm. > > T194 has multiple fan-profiles support. > > > > 2. Add pwm-fan remove support. This is essential since the config is > > tristate capable. > > > > Signed-off-by: Sandipan Patra <spatra@nvidia.com> > > --- > > drivers/hwmon/pwm-fan.c | 112 > > ++++++++++++++++++++++++++++++++++++++++++------ > > 1 file changed, 100 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c index > > 30b7b3e..26db589 100644 > > --- a/drivers/hwmon/pwm-fan.c > > +++ b/drivers/hwmon/pwm-fan.c > > [ ... ] > > > > > +static int pwm_fan_remove(struct platform_device *pdev) { > > + struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev); > > + struct pwm_args args; > > + > > + if (!ctx) > > + return -EINVAL; > > + > > + if (IS_ENABLED(CONFIG_THERMAL)) > > + thermal_cooling_device_unregister(ctx->cdev); > > + > > + pwm_get_args(ctx->pwm, &args); > > + pwm_config(ctx->pwm, 0, args.period); > > + pwm_disable(ctx->pwm); > > + > > + return 0; > > +} > > + > > I don't think you actually tested this. I would suggest to make yourself familiar > with 'devm' functions and their use, and then resubmit. >
Thanks Guenter. I missed to mention about devm while unregistering the cooling device. That would definitely cause a mistake in code. I am noting it for further patch.
For a better clarity, I will push next version of this patch to handle only multiple profiles support. "remove fan module" will be supported by a separate patch altogether.
Thanks & Regards, Sandipan
> Thanks, > Guenter
| |