On Fri, Jan 17, 2020 at 02:35:57AM +0000, Jeff LaBundy wrote: > +static void iqs620_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, > + struct pwm_state *state) > +{ > + struct iqs620_pwm_private *iqs620_pwm; > + > + iqs620_pwm = container_of(chip, struct iqs620_pwm_private, chip); > + > + mutex_lock(&iqs620_pwm->lock); > + > + /* > + * Since the device cannot generate a 0% duty cycle, requests to do so > + * cause subsequent calls to iqs620_pwm_get_state to report the output > + * as disabled with duty cycle equal to that which was in use prior to > + * the request. This is not ideal, but is the best compromise based on > + * the capabilities of the device. > + */ > + state->enabled = iqs620_pwm->out_en; Hmm, when .get_state is called first (before the first invokation of .apply) .out_en doesn't represent the hardware's state but is false unconditionally. This makes it hard to take over a running PWM setup by the bootloader. Best regards Uwe > + state->duty_cycle = DIV_ROUND_UP((iqs620_pwm->duty_val + 1) * > + IQS620_PWM_PERIOD_NS, 256); > + > + mutex_unlock(&iqs620_pwm->lock); > + > + state->period = IQS620_PWM_PERIOD_NS; > +} -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |