From: Chen-Yu Tsai <wens@csie.org>
To: Olliver Schinagl <o.schinagl@ultimaker.com>
Cc: Olliver Schinagl <oliver@schinagl.nl>,
Thierry Reding <thierry.reding@gmail.com>,
Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
Mark Rutland <mark.rutland@arm.com>,
Ian Campbell <ijc+devicetree@hellion.org.uk>,
Kumar Gala <galak@codeaurora.org>,
Joachim Eastwood <manabian@gmail.com>,
Maxime Ripard <maxime.ripard@free-electrons.com>,
Alexandre Belloni <alexandre.belloni@free-electrons.com>,
Olliver Schinagl <oliver+list@schinagl.nl>,
linux-pwm@vger.kernel.org,
devicetree <devicetree@vger.kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 03/10] pwm: sunxi: Yield some time to the pwm-block to become ready
Date: Sat, 7 Nov 2015 00:34:29 +0800 [thread overview]
Message-ID: <CAGb2v67tfOH-+6=Q=s44K8iGwKomQ3i86bVRwWFqJRKDp-JZzg@mail.gmail.com> (raw)
In-Reply-To: <1445895161-2317-4-git-send-email-o.schinagl@ultimaker.com>
On Tue, Oct 27, 2015 at 5:32 AM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> The pwm-block of some of the sunxi chips feature a 'ready' flag to
> indicate the software that it is ready for new commands.
>
> Right now, when we call pwm_config and set the period, we write the
> values to the registers, and turn off the clock to the IP. Because of
> this, the hardware does not have time to configure the hardware and set
> the 'ready' flag.
>
> By running the clock just before making new changes and before checking
> if the hardware is ready, the hardware has time to reconfigure itself
> and set the clear the flag appropriately.
>
> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
> ---
> drivers/pwm/pwm-sun4i.c | 43 +++++++++++++++++++++++++------------------
> 1 file changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index 58ff424..4d84d9d 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -104,6 +104,22 @@ static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> u64 clk_rate, div = 0;
> unsigned int prescaler = 0;
> int err;
> + int ret = 0;
> +
> + /* Let the PWM hardware run before making any changes. We do this to
> + * allow the hardware to have some time to clear the 'ready' flag.
> + */
> + err = clk_prepare_enable(sun4i_pwm->clk);
> + if (err) {
> + dev_err(chip->dev, "failed to enable PWM clock\n");
> + return err;
> + }
> + spin_lock(&sun4i_pwm->ctrl_lock);
> + val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> + clk_gate = val & BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
> + val |= BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
> + sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
> + spin_unlock(&sun4i_pwm->ctrl_lock);
>
> clk_rate = clk_get_rate(sun4i_pwm->clk);
>
> @@ -136,7 +152,9 @@ static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>
> if (div - 1 > PWM_PRD_MASK) {
> dev_err(chip->dev, "period exceeds the maximum value\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + spin_lock(&sun4i_pwm->ctrl_lock);
> + goto out;
> }
> }
>
> @@ -145,26 +163,14 @@ static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> do_div(div, period_ns);
> dty = div;
>
> - err = clk_prepare_enable(sun4i_pwm->clk);
> - if (err) {
> - dev_err(chip->dev, "failed to enable PWM clock\n");
> - return err;
> - }
> -
> spin_lock(&sun4i_pwm->ctrl_lock);
> val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> -
> if (sun4i_pwm->data->has_rdy && (val & PWM_RDY(pwm->hwpwm))) {
Instead of moving the code around to try to give the hardware some unspecified
time to run, could we use a tight busy loop with a timeout to read the register
and check if it's been cleared? I think that works better with cpufreq as well.
Thanks.
ChenYu
> - spin_unlock(&sun4i_pwm->ctrl_lock);
> - clk_disable_unprepare(sun4i_pwm->clk);
> - return -EBUSY;
> - }
> -
> - clk_gate = val & BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
> - if (clk_gate) {
> - val &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
> - sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
> + ret = -EBUSY;
> + goto out;
> }
> + val &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
> + sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
>
> val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> val &= ~BIT_CH(PWM_PRESCAL_MASK, pwm->hwpwm);
> @@ -174,6 +180,7 @@ static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> val = (dty & PWM_DTY_MASK) | PWM_PRD(prd);
> sun4i_pwm_writel(sun4i_pwm, val, PWM_CH_PRD(pwm->hwpwm));
>
> +out:
> if (clk_gate) {
> val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> val |= clk_gate;
> @@ -183,7 +190,7 @@ static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> spin_unlock(&sun4i_pwm->ctrl_lock);
> clk_disable_unprepare(sun4i_pwm->clk);
>
> - return 0;
> + return ret;
> }
>
> static int sun4i_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
> --
> 2.6.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2015-11-06 16:34 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-26 21:32 [PATCH 00/10] Olliver Schinagl
2015-10-26 21:32 ` [PATCH 01/10] pwm: lpc18xx_pwm: use pwm_set_chip_data Olliver Schinagl
2015-10-26 21:58 ` Ezequiel Garcia
2015-10-27 7:22 ` Olliver Schinagl
2015-10-26 21:32 ` [PATCH 02/10] pwm: sunxi: fix whitespace issue Olliver Schinagl
2015-11-06 16:08 ` Thierry Reding
2015-10-26 21:32 ` [PATCH 03/10] pwm: sunxi: Yield some time to the pwm-block to become ready Olliver Schinagl
2015-11-06 16:12 ` Thierry Reding
2015-11-06 16:34 ` Chen-Yu Tsai [this message]
2015-10-26 21:32 ` [PATCH 04/10] pwm: core: use bitops Olliver Schinagl
2015-11-06 14:46 ` Thierry Reding
2015-11-06 14:49 ` Olliver Schinagl
2015-11-06 21:36 ` Andy Shevchenko
2015-10-26 21:32 ` [PATCH 05/10] pwm: sysfs: do not unnecessarily store result in var Olliver Schinagl
2015-11-06 14:51 ` Thierry Reding
2015-10-26 21:32 ` [PATCH 06/10] pwm: sysfs: make use of the DEVICE_ATTR_[RW][WO] macro's Olliver Schinagl
2015-11-06 14:52 ` Thierry Reding
2015-10-26 21:32 ` [PATCH 07/10] pwm: gpio: Add a generic gpio based PWM driver Olliver Schinagl
2015-10-27 7:42 ` Rob Herring
2015-10-27 8:50 ` Olliver Schinagl
2015-11-06 15:57 ` Thierry Reding
2015-10-26 21:32 ` [PATCH 08/10] pwm: core: add pulse feature to the PWM framework Olliver Schinagl
2015-10-26 21:59 ` kbuild test robot
2015-10-26 22:09 ` kbuild test robot
2015-10-26 22:10 ` kbuild test robot
2015-10-26 22:11 ` kbuild test robot
2015-10-26 23:06 ` kbuild test robot
2015-11-06 15:18 ` Thierry Reding
2015-11-06 15:46 ` Olliver Schinagl
2015-11-06 16:05 ` Thierry Reding
2015-11-06 16:18 ` Olliver Schinagl
2015-10-26 21:32 ` [PATCH 09/10] pwm: pwm_gpio: add pulse option Olliver Schinagl
2015-10-26 21:32 ` [PATCH 10/10] pwm: sunxi: Add possibility to pulse the sunxi pwm output Olliver Schinagl
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='CAGb2v67tfOH-+6=Q=s44K8iGwKomQ3i86bVRwWFqJRKDp-JZzg@mail.gmail.com' \
--to=wens@csie.org \
--cc=alexandre.belloni@free-electrons.com \
--cc=devicetree@vger.kernel.org \
--cc=galak@codeaurora.org \
--cc=ijc+devicetree@hellion.org.uk \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=manabian@gmail.com \
--cc=mark.rutland@arm.com \
--cc=maxime.ripard@free-electrons.com \
--cc=o.schinagl@ultimaker.com \
--cc=oliver+list@schinagl.nl \
--cc=oliver@schinagl.nl \
--cc=pawel.moll@arm.com \
--cc=robh+dt@kernel.org \
--cc=thierry.reding@gmail.com \
/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).