| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150225182449.GA2306@developer.amazonguestwifi.org>
Date: Wed, 25 Feb 2015 14:24:51 -0400
From: Eduardo Valentin <edubezval@...il.com>
To: Lukasz Majewski <l.majewski@...sung.com>
Cc: Kamil Debski <k.debski@...sung.com>,
Jean Delvare <jdelvare@...e.de>,
Guenter Roeck <linux@...ck-us.net>,
Kukjin Kim <kgene@...nel.org>, lm-sensors@...sensors.org,
Linux PM list <linux-pm@...r.kernel.org>,
"linux-samsung-soc@...r.kernel.org"
<linux-samsung-soc@...r.kernel.org>, devicetree@...r.kernel.org,
Lukasz Majewski <l.majewski@...ess.pl>,
Kukjin Kim <kgene.kim@...sung.com>,
linux-kernel@...r.kernel.org,
Sjoerd Simons <sjoerd.simons@...labora.co.uk>,
Abhilash Kesavan <kesavan.abhilash@...il.com>,
Abhilash Kesavan <a.kesavan@...sung.com>
Subject: Re: [PATCH v5 6/6] hwmon: pwm-fan: Code for using PWM FAN as a
cooling device
Hey Lukasz,
On Wed, Feb 25, 2015 at 04:34:22PM +0100, Lukasz Majewski wrote:
> The PWM FAN device can now be used as a thermal cooling device. Necessary
> infrastructure has been added in this commit.
>
> Signed-off-by: Lukasz Majewski <l.majewski@...sung.com>
> ---
> Changes for v2:
> - Replace pwm_fan_cooling_states with pwm_fan_cooling_levels
> - Update ctx->pwm_fan_state when correct data from device tree is available
> - Using therma_cdev_update() when thermal is ready for controlling the fan
> Changes for v3:
> - Rename patch heading
> - pwm_fan_of_get_cooling_data() now returns -EINVAL when no "cooling-levels"
> property defined
> - register of cooling device only when proper cooling data is present
> Changes for v4:
> - None
> Changes for v5:
> - Check for IS_ENABLED(CONFIG_THERMAL) has been added to prevent from
> executing thermal_* specific functions
> ---
> drivers/hwmon/pwm-fan.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 82 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index e6ed353..2c1a8f0 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -24,6 +24,7 @@
> #include <linux/platform_device.h>
> #include <linux/pwm.h>
> #include <linux/sysfs.h>
> +#include <linux/thermal.h>
>
> #define MAX_PWM 255
>
> @@ -68,6 +69,17 @@ exit_set_pwm_err:
> return ret;
> }
>
> +static void pwm_fan_update_state(struct pwm_fan_ctx *ctx, unsigned long pwm)
> +{
> + int i;
> +
> + for (i = 0; i < ctx->pwm_fan_max_state; ++i)
> + if (pwm < ctx->pwm_fan_cooling_levels[i + 1])
> + break;
> +
> + ctx->pwm_fan_state = i;
> +}
> +
> static ssize_t set_pwm(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> @@ -82,6 +94,7 @@ static ssize_t set_pwm(struct device *dev, struct device_attribute *attr,
> if (ret)
> return ret;
>
> + pwm_fan_update_state(ctx, pwm);
> return count;
> }
>
> @@ -103,6 +116,59 @@ static struct attribute *pwm_fan_attrs[] = {
>
> ATTRIBUTE_GROUPS(pwm_fan);
>
> +/* thermal cooling device callbacks */
> +static int pwm_fan_get_max_state(struct thermal_cooling_device *cdev,
> + unsigned long *state)
> +{
> + struct pwm_fan_ctx *ctx = cdev->devdata;
> +
Why this call back is the only one you didn't add a check for ctx ==
NULL?
> + *state = ctx->pwm_fan_max_state;
> +
> + return 0;
> +}
> +
> +static int pwm_fan_get_cur_state(struct thermal_cooling_device *cdev,
> + unsigned long *state)
> +{
> + struct pwm_fan_ctx *ctx = cdev->devdata;
> +
> + if (!ctx)
> + return -EINVAL;
> +
> + *state = ctx->pwm_fan_state;
> +
> + return 0;
> +}
> +
> +static int
> +pwm_fan_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state)
> +{
> + struct pwm_fan_ctx *ctx = cdev->devdata;
> + int ret;
> +
> + if (!ctx || (state > ctx->pwm_fan_max_state))
> + return -EINVAL;
> +
> + if (state == ctx->pwm_fan_state)
> + return 0;
> +
> + ret = __set_pwm(ctx, ctx->pwm_fan_cooling_levels[state]);
> + if (ret) {
> + dev_err(&cdev->device, "Cannot set pwm!\n");
> + return ret;
> + }
> +
> + ctx->pwm_fan_state = state;
> +
> + return ret;
> +}
> +
> +static const struct thermal_cooling_device_ops pwm_fan_cooling_ops = {
> + .get_max_state = pwm_fan_get_max_state,
> + .get_cur_state = pwm_fan_get_cur_state,
> + .set_cur_state = pwm_fan_set_cur_state,
> +};
> +
> int pwm_fan_of_get_cooling_data(struct device *dev, struct pwm_fan_ctx *ctx)
> {
> struct device_node *np = dev->of_node;
> @@ -145,8 +211,9 @@ int pwm_fan_of_get_cooling_data(struct device *dev, struct pwm_fan_ctx *ctx)
>
> static int pwm_fan_probe(struct platform_device *pdev)
> {
> - struct device *hwmon;
> + struct thermal_cooling_device *cdev;
> struct pwm_fan_ctx *ctx;
> + struct device *hwmon;
> int duty_cycle;
> int ret;
>
> @@ -193,6 +260,20 @@ static int pwm_fan_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + ctx->pwm_fan_state = ctx->pwm_fan_max_state;
> + if (IS_ENABLED(CONFIG_THERMAL)) {
> + cdev = thermal_of_cooling_device_register(pdev->dev.of_node,
> + "pwm-fan", ctx,
> + &pwm_fan_cooling_ops);
I know this is a PITA, but thermal subsystem still does not use devm
based helpers. That means, you need to be old school and unregister the
device yourself.
Please add the cdev to ctx structure:
+ ctx->cdev = cdev;
and add a call for:
thermal_cooling_device_unregister(ctx->cdev);
in static int pwm_fan_remove(struct platform_device *pdev)
despite the above two minor issues, you may add my
Acked-by: Eduardo Valentin <edubezval@...il.com>
> + if (IS_ERR(cdev)) {
> + dev_err(&pdev->dev,
> + "Failed to register pwm-fan as cooling device");
> + pwm_disable(ctx->pwm);
> + return PTR_ERR(cdev);
> + }
> + thermal_cdev_update(cdev);
> + }
> +
> return 0;
> }
>
> --
> 2.0.0.rc2
>
Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)
Powered by blists - more mailing lists