Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1477112

Re: [PATCH 1/2] pwm: sunxi: allow the pwm to finish its pulse before disable

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

Hi Maxime!,

On za, 2016-08-27 at 00:19 +0200, Maxime Ripard wrote:
> On Thu, Aug 25, 2016 at 07:50:10PM +0200, Olliver Schinagl wrote:
> > 
> > When we inform the PWM block to stop toggeling the output, we may
> > end up
> > in a state where the output is not what we would expect (e.g. not
> > the
> > low-pulse) but whatever the output was at when the clock got
> > disabled.
> > 
> > To counter this we have to wait for maximally the time of one whole
> > period to ensure the pwm hardware was able to finish. Since we
> > already
> > told the PWM hardware to disable it self, it will not continue
> > toggling
> > but merly finish its current pulse.
> > 
> > If a whole period is considered to much, it may be contemplated to
> > use a
> > half period + a little bit to ensure we get passed the transition.
> > 
> > Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> > ---
> >  drivers/pwm/pwm-sun4i.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > index 03a99a5..5e97c8a 100644
> > --- a/drivers/pwm/pwm-sun4i.c
> > +++ b/drivers/pwm/pwm-sun4i.c
> > @@ -8,6 +8,7 @@
> >  
> >  #include <linux/bitops.h>
> >  #include <linux/clk.h>
> > +#include <linux/delay.h>
> >  #include <linux/err.h>
> >  #include <linux/io.h>
> >  #include <linux/module.h>
> > @@ -245,6 +246,16 @@ static void sun4i_pwm_disable(struct pwm_chip
> > *chip, struct pwm_device *pwm)
> >  	spin_lock(&sun4i_pwm->ctrl_lock);
> >  	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> >  	val &= ~BIT_CH(PWM_EN, pwm->hwpwm);
> > +	sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
> > +	spin_unlock(&sun4i_pwm->ctrl_lock);
> > +
> > +	/* Allow for the PWM hardware to finish its last toggle.
> > The pulse
> > +	 * may have just started and thus we should wait a full
> > period.
> > +	 */
> > +	ndelay(pwm_get_period(pwm));
> 
> Can't that use the ready bit as well?
It depends whatever is cheaper. If we disable the pwm, we have to
commit that request to hardware first. Then we have to read back the
has ready and in the strange situation it is not, wait for it to become
ready?

Also, that would mean we would loop in a spin lock, or keep
setting/clearing an additional spinlock to read the ready bit.

If that is cheaper then an ndelay, I can rewrite it of course, and
assuming the 'ready' bit gets set from disabeling the PWM. It needs to
be investigated if disabeling the PWM, the ready bit is used.q

> 
> Maxime
> 

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCHv2 0/2] pwm: sunxi: give the pwm IP block more time Olliver Schinagl <oliver@schinagl.nl> - 2016-08-25 20:00 +0200
  [PATCH 1/2] pwm: sunxi: allow the pwm to finish its pulse before disable Olliver Schinagl <oliver@schinagl.nl> - 2016-08-25 20:00 +0200
    Re: [PATCH 1/2] pwm: sunxi: allow the pwm to finish its pulse before  disable Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-08-27 00:20 +0200
      Re: [PATCH 1/2] pwm: sunxi: allow the pwm to finish its pulse  before disable Olliver Schinagl <oliver@schinagl.nl> - 2016-09-06 09:20 +0200
        Re: [PATCH 1/2] pwm: sunxi: allow the pwm to finish its pulse before  disable Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-06 22:00 +0200
          Re: [PATCH 1/2] pwm: sunxi: allow the pwm to finish its pulse  before disable Olliver Schinagl <oliver@schinagl.nl> - 2016-09-09 11:10 +0200
  [PATCH 2/2] pwm: sunxi: Yield some time to the pwm-block to become ready Olliver Schinagl <oliver@schinagl.nl> - 2016-08-25 20:00 +0200
    Re: [PATCH 2/2] pwm: sunxi: Yield some time to the pwm-block to  become ready Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-08-27 00:30 +0200

csiph-web