| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAD7vxx+t=t84hGqVHz3Y3y+eKii5E3J2=oxMaDgtQgbD=WU+ow@mail.gmail.com>
Date: Sat, 30 May 2015 09:41:31 -0700
From: Tim Kryger <tim.kryger@...il.com>
To: Jonathan Richardson <jonathar@...adcom.com>
Cc: Dmitry Torokhov <dtor@...gle.com>,
Anatol Pomazau <anatol@...gle.com>,
Arun Ramamurthy <arun.ramamurthy@...adcom.com>,
Thierry Reding <thierry.reding@...il.com>,
Scott Branden <sbranden@...adcom.com>,
bcm-kernel-feedback-list <bcm-kernel-feedback-list@...adcom.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux PWM <linux-pwm@...r.kernel.org>
Subject: Re: [PATCH v8 3/5] pwm: kona: Fix incorrect config, disable, and
polarity procedures
On Tue, May 26, 2015 at 1:08 PM, Jonathan Richardson
<jonathar@...adcom.com> wrote:
> The config procedure didn't follow the spec which periodically resulted
> in failing to enable the output signal. This happened one in ten or
> twenty attempts. Following the spec and adding a 400ns delay in the
> appropriate locations resolves this problem.
>
> The disable and polarity procedures now also follow the spec. The old
> procedures would result in no change in signal when called.
I think you may want to adjust your commit title and message to more
clearly describe what this change is doing. Perhaps something like:
pwm: kona: Modify settings application sequence
Update the driver so that settings are applied in accordance with the
most recent version of the hardware spec. The revised sequence clears
the trigger bit, waits 400ns, writes settings, sets the trigger bit,
and waits another 400ns. This corrects an issue where occasionally a
requested change was not properly reflected in the PWM output.
Otherwise, this patch looks reasonable so
Reviewed-by: Tim Kryger <tim.kryger@...il.com>
>
> Reviewed-by: Arun Ramamurthy <arunrama@...adcom.com>
> Reviewed-by: Scott Branden <sbranden@...adcom.com>
> Tested-by: Scott Branden <sbranden@...adcom.com>
> Signed-off-by: Jonathan Richardson <jonathar@...adcom.com>
> ---
> drivers/pwm/pwm-bcm-kona.c | 47 +++++++++++++++++++++++++++++++++++---------
> 1 file changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pwm/pwm-bcm-kona.c b/drivers/pwm/pwm-bcm-kona.c
> index 32b3ec6..c87621f 100644
> --- a/drivers/pwm/pwm-bcm-kona.c
> +++ b/drivers/pwm/pwm-bcm-kona.c
> @@ -76,19 +76,36 @@ static inline struct kona_pwmc *to_kona_pwmc(struct pwm_chip *_chip)
> return container_of(_chip, struct kona_pwmc, chip);
> }
>
> -static void kona_pwmc_apply_settings(struct kona_pwmc *kp, unsigned int chan)
> +/*
> + * Clear trigger bit but set smooth bit to maintain old output.
> + */
> +static void kona_pwmc_prepare_for_settings(struct kona_pwmc *kp,
> + unsigned int chan)
> {
> unsigned int value = readl(kp->base + PWM_CONTROL_OFFSET);
>
> - /* Clear trigger bit but set smooth bit to maintain old output */
> value |= 1 << PWM_CONTROL_SMOOTH_SHIFT(chan);
> value &= ~(1 << PWM_CONTROL_TRIGGER_SHIFT(chan));
> writel(value, kp->base + PWM_CONTROL_OFFSET);
>
> + /*
> + * There must be a min 400ns delay between clearing enable and setting
> + * it. Failing to do this may result in no PWM signal.
> + */
> + ndelay(400);
> +}
Since it doesn't function as an enable, please call it the trigger bit.
> +
> +static void kona_pwmc_apply_settings(struct kona_pwmc *kp, unsigned int chan)
> +{
> + unsigned int value = readl(kp->base + PWM_CONTROL_OFFSET);
> +
> /* Set trigger bit and clear smooth bit to apply new settings */
> value &= ~(1 << PWM_CONTROL_SMOOTH_SHIFT(chan));
> value |= 1 << PWM_CONTROL_TRIGGER_SHIFT(chan);
> writel(value, kp->base + PWM_CONTROL_OFFSET);
> +
> + /* PWMOUT_ENABLE must be held high for at least 400 ns. */
> + ndelay(400);
> }
Same here.
>
> static int kona_pwmc_config(struct pwm_chip *chip, struct pwm_device *pwm,
> @@ -133,8 +150,14 @@ static int kona_pwmc_config(struct pwm_chip *chip, struct pwm_device *pwm,
> return -EINVAL;
> }
>
> - /* If the PWM channel is enabled, write the settings to the HW */
> + /*
> + * Don't apply settings if disabled. The period and duty cycle are
> + * always calculated above to ensure the new values are
> + * validated immediately instead of on enable.
> + */
> if (test_bit(PWMF_ENABLED, &pwm->flags)) {
> + kona_pwmc_prepare_for_settings(kp, chan);
> +
> value = readl(kp->base + PRESCALE_OFFSET);
> value &= ~PRESCALE_MASK(chan);
> value |= prescale << PRESCALE_SHIFT(chan);
> @@ -164,6 +187,8 @@ static int kona_pwmc_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
> return ret;
> }
>
> + kona_pwmc_prepare_for_settings(kp, chan);
> +
> value = readl(kp->base + PWM_CONTROL_OFFSET);
>
> if (polarity == PWM_POLARITY_NORMAL)
> @@ -175,9 +200,6 @@ static int kona_pwmc_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
>
> kona_pwmc_apply_settings(kp, chan);
>
> - /* Wait for waveform to settle before gating off the clock */
> - ndelay(400);
> -
> clk_disable_unprepare(kp->clk);
>
> return 0;
> @@ -207,13 +229,20 @@ static void kona_pwmc_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> {
> struct kona_pwmc *kp = to_kona_pwmc(chip);
> unsigned int chan = pwm->hwpwm;
> + unsigned int value;
> +
> + kona_pwmc_prepare_for_settings(kp, chan);
>
> /* Simulate a disable by configuring for zero duty */
> writel(0, kp->base + DUTY_CYCLE_HIGH_OFFSET(chan));
> - kona_pwmc_apply_settings(kp, chan);
> + writel(0, kp->base + PERIOD_COUNT_OFFSET(chan));
>
> - /* Wait for waveform to settle before gating off the clock */
> - ndelay(400);
> + /* Set prescale to 0 for this channel */
> + value = readl(kp->base + PRESCALE_OFFSET);
> + value &= ~PRESCALE_MASK(chan);
> + writel(value, kp->base + PRESCALE_OFFSET);
> +
> + kona_pwmc_apply_settings(kp, chan);
>
> clk_disable_unprepare(kp->clk);
> }
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists