linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Michal Vokáč" <michal.vokac@ysoft.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: "Thierry Reding" <thierry.reding@gmail.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-pwm@vger.kernel.org" <linux-pwm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Lukasz Majewski" <l.majewski@majess.pl>,
	"Fabio Estevam" <fabio.estevam@nxp.com>,
	"Lothar Waßmann" <LW@karo-electronics.de>,
	"Linus Walleij" <linus.walleij@linaro.org>
Subject: Re: [RFC PATCH v3 2/2] pwm: imx: Configure output to GPIO in disabled state
Date: Wed, 30 Jan 2019 15:42:29 +0100	[thread overview]
Message-ID: <72bad040-05f4-607f-f210-468e46ea3228@ysoft.com> (raw)
In-Reply-To: <20190124104435.e6paqwcuz3hizwnv@pengutronix.de>

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


  reply	other threads:[~2019-01-30 14:42 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-06 13:41 [RFC PATCH v3 0/2] pwm: imx: Configure output to GPIO in disabled state Vokáč Michal
2018-12-06 13:41 ` [RFC PATCH v3 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO Vokáč Michal
2018-12-10 23:16   ` Rob Herring
2018-12-14 13:45   ` Linus Walleij
2018-12-06 13:41 ` [RFC PATCH v3 2/2] pwm: imx: Configure output to GPIO in disabled state Vokáč Michal
2018-12-06 13:59   ` Uwe Kleine-König
2018-12-06 15:37     ` Vokáč Michal
2018-12-06 16:16       ` Uwe Kleine-König
2018-12-10 11:15         ` Vokáč Michal
2018-12-10 11:17           ` Uwe Kleine-König
2018-12-10 11:38             ` Vokáč Michal
2018-12-12  8:01   ` Uwe Kleine-König
2018-12-12 11:42     ` Vokáč Michal
2018-12-12 12:12       ` Uwe Kleine-König
2019-01-24  8:59         ` Michal Vokáč
2019-01-24  9:22           ` Uwe Kleine-König
2019-01-24 10:12             ` Michal Vokáč
2019-01-24 10:44               ` Uwe Kleine-König
2019-01-30 14:42                 ` Michal Vokáč [this message]
2019-01-30 15:39                   ` Uwe Kleine-König
2019-02-01 15:50                     ` Michal Vokáč

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=72bad040-05f4-607f-f210-468e46ea3228@ysoft.com \
    --to=michal.vokac@ysoft.com \
    --cc=LW@karo-electronics.de \
    --cc=devicetree@vger.kernel.org \
    --cc=fabio.estevam@nxp.com \
    --cc=l.majewski@majess.pl \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).