Re: [RFC PATCH v3 2/2] pwm: imx: Configure output to GPIO in disabled state

[Michal VokÃÄ]

On 24.1.2019 11:44, Uwe Kleine-KÃnig wrote:
> On Thu, Jan 24, 2019 at 11:12:12AM +0100, Michal VokÃÄ wrote:
> > On 24.1.2019 10:22, Uwe Kleine-KÃnig wrote:
> > > I think it might be beneficial to allow (or require) that disable acts
> > > immediately. But this is not how it used to be and in my discussion with
> > > Thierry (IIRC) he required to complete the currently running period.
> >
> > I am confused here. IFAIK it always used to be that:
> >
> > 	pwm_apply_state(pwm, { .enabled = 0 });
> >
> > immediately stops the PWM with:
> >
> > 	writel(0, imx->mmio_base + MX3_PWMCR);
> >
> > regardless of the period (for pwm-imx).
>
> Then is is a bug since forever (well, or a fact that resulted from the
> intended semantic not being documented and the driver author not caring
> or knowing better).

Hi Uwe, I skimmed all the PWM drivers trying to find out how others are
waiting for the end of a period before disabling PWM.

There is 50 PWM drivers in total and I could find only two drivers that
do something that resembles waiting for an end of a period:

 - pwm-atmel.c
   https://elixir.bootlin.com/linux/v5.0-rc3/source/drivers/pwm/pwm-atmel.c#L176
	
 - pwm-sun4i.c
   https://elixir.bootlin.com/linux/v5.0-rc3/source/drivers/pwm/pwm-sun4i.c#L284

I did not delve into that too much but I believe there are some HW
requirements on those platforms to stop the PWM that way. Others
simply stop the PWM straight away. So I am confused even more
and wonder where your requirements came from?

Anyway, as I could not find any real example IÂtried to solve it
according to your earlier suggestion. That is configure duty_cycle=0
and some time later disable PWM.

It generates additional problems. The PWM block has a FIFO with four
slots. Before adding the sample with duty cycle=0 into the FIFO it
is wise to wait for a free slot in the FIFO. Then when the sample is
added it may actually happen that the sample is the fourth in the
FIFO. So it may take up to four periods until we can disable the PWM.
These four periods can take up to 16s. Hmmm :(

Reconfigure the period is not an option. According to the TRM, once you
write new value into the period register, the counter is cleared and
new period is started. That means you can produce the spikes this way
as well..

I agree there are bugs in the driver and it is far from providing
complete support of the i.MX PWM HW. Still, I believe those are totally
independent problems from the pinctrl stuff and so should not block
the discussion/inclusion of this series.

I am sorry if this is getting on your nerves Uwe. I am doing my
best to test all your suggestions and possible solutions to find
a good compromise. At least I learned a lot from doing all that stuff.

Thank you for your time,
Michal