Hi > +static int tpu_pwm_suspend(struct device *dev) > +{ > + struct tpu_device *tpu = dev_get_drvdata(dev); > + struct pwm_chip *chip = &tpu->chip; > + struct pwm_device *pwm; > + int i; > + > + for (i = 0; i <= 3; i++) { > + if ((pwm_get_chip_data(&chip->pwms[i])) != NULL) { > + pwm = &chip->pwms[i]; > + if (!test_bit(PWMF_REQUESTED, &pwm->flags)) > + return 0; > + } > + } why 3 ? About code, how about this ? pwm = &chip->pwms[i]; if (pwm_get_chip_data(pwm)) { ... Can we use chip->pwms at driver ? I'm not sure but pwm.h say struct pwm_chip { ... => /* only used internally by the PWM framework */ struct list_head list; struct pwm_device *pwms; }; > + > + pm_runtime_put(dev); > + > + return 0; > +} Do we need to call pm_runtime_xxx here ? > +static int tpu_pwm_resume(struct device *dev) > +{ > + struct tpu_device *tpu = dev_get_drvdata(dev); > + struct pwm_chip *chip = &tpu->chip; > + struct pwm_device *pwm; > + int i; > + > + pm_runtime_get_sync(dev); > + > + for (i = 0; i <= 3; i++) { > + if ((pwm_get_chip_data(&chip->pwms[i])) != NULL) { > + pwm = &chip->pwms[i]; > + tpu_pwm_restart_timer(pwm); > + } > + } ditto