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 <[email protected]> > --- > 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 <[email protected]>
> + 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
>
signature.asc
Description: Digital signature
