Hi Shimoda-san,

On Thu, May 30, 2019 at 12:21 PM Yoshihiro Shimoda
<[email protected]> wrote:
> According to the Documentation/pwm.txt, all PWM consumers should have
> power management. Since this sysfs interface is one of consumers so that
> this patch adds suspend/resume support.
>
> Signed-off-by: Yoshihiro Shimoda <[email protected]>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <[email protected]>

> --- a/drivers/pwm/sysfs.c
> +++ b/drivers/pwm/sysfs.c

> @@ -372,10 +373,109 @@ static struct attribute *pwm_chip_attrs[] = {
>  };
>  ATTRIBUTE_GROUPS(pwm_chip);
>

You may want to add a comment "takes export->lock on success" here...

> +static struct pwm_export *pwm_class_get_state(struct device *parent,
> +                                             struct pwm_device *pwm,
> +                                             struct pwm_state *state)
> +{
> +       struct device *child;
> +       struct pwm_export *export;
> +
> +       if (!test_bit(PWMF_EXPORTED, &pwm->flags))
> +               return NULL;
> +
> +       child = device_find_child(parent, pwm, pwm_unexport_match);
> +       if (!child)
> +               return NULL;
> +
> +       export = child_to_pwm_export(child);
> +       put_device(child);      /* for device_find_child() */
> +
> +       mutex_lock(&export->lock);
> +       pwm_get_state(pwm, state);
> +
> +       return export;
> +}
> +
> +static int pwm_class_apply_state(struct pwm_export *export,
> +                                struct pwm_device *pwm,
> +                                struct pwm_state *state)
> +{
> +       int ret = pwm_apply_state(pwm, state);
> +

... and "release lock taken in pwm_class_get_state()" here.

> +       mutex_unlock(&export->lock);
> +
> +       return ret;
> +}

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Reply via email to