Hi Geert-san, > From: Geert Uytterhoeven, Sent: Friday, May 31, 2019 4:34 PM > > Hi Shimoda-san, > > On Thu, May 30, 2019 at 12:21 PM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@xxxxxxxxxxx> 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 <yoshihiro.shimoda.uh@xxxxxxxxxxx> > > Thanks for your patch! > > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> Thank you for your review! I'll add comments you described on v3 patch. Best regards, Yoshihiro Shimoda > > --- 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 -- geert@xxxxxxxxxxxxxx > > 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