On Wed, 13 Apr 2016, Thierry Reding wrote: > On Wed, Apr 13, 2016 at 11:25:54AM +0100, Lee Jones wrote: > > On Tue, 12 Apr 2016, Thierry Reding wrote: > > > > > On Wed, Mar 02, 2016 at 03:32:07PM +0000, Lee Jones wrote: > > > > Once a PWM Capture has been initiated, the capture call > > > > enables a rising edge detection IRQ, then waits. Once each > > > > of the 3 phase changes have been recorded the thread then > > > > wakes. The remaining part of the call carries out the > > > > relevant calculations and passes back a formatted string to > > > > the caller. > > > > > > > > Signed-off-by: Lee Jones <lee.jo...@linaro.org> > > > > --- > > > > drivers/pwm/pwm-sti.c | 72 > > > > +++++++++++++++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 72 insertions(+) > > > > > > > > diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c > > > > index 82a69e4..8de9b4a 100644 > > > > --- a/drivers/pwm/pwm-sti.c > > > > +++ b/drivers/pwm/pwm-sti.c
[...] > > > > + /* Prepare capture measurement */ > > > > + d->index = 0; > > > > + regmap_write(pc->regmap, PWM_CPT_EDGE(channel), > > > > CPT_EDGE_RISING); > > > > + regmap_field_write(pc->pwm_cpt_int_en, BIT(channel)); > > > > + ret = wait_event_interruptible_timeout(d->wait, d->index > 1, > > > > HZ); > > > > > > The timeout here should make sure callers don't hang forever. But maybe > > > you can still make sure that when the PWM gets disabled the wait queue > > > is woken and perhaps return an appropriate error code to let users know > > > that the operation was interrupted. > > > > Sure. I'll look into that. > > > > > Also, how about letting callers choose the value of the timeout? In some > > > cases they may be interested in long-running signals. In other cases the > > > whole second timeout may be much too long. > > > > I'm not opposed to it. How do you suggest we do that? > > The easiest would probably be to add an unsigned long timeout parameter > to the pwm_capture() function and ->capture() callbacks. > > But thinking about this further I'm wondering if it might not be easier > and more flexible to move the timeout completely outside of this code > and into callers. I suspect that the most simple way to do that would be > to add a completion to struct pwm_capture that callers can use to wait > for completion of a capture. This would make the whole process > asynchronous and allow interesting things like making the sysfs capture > file pollable, for example. Okay, so how do you propose we handle this with sysfs? Perhaps another RW file to set it? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
