Re: [PATCH v2 1/4] pwm: kona: Remove setting default smooth type and polarity for all channels

[Tim Kryger <tim.kryger@gmail.com>]

On Thu, Dec 4, 2014 at 12:22 PM, Jonathan Richardson
<jonathar@broadcom.com> wrote:
> Hi Tim,
>
> I'm going to take over this submission because I made the changes to the
> driver. Arun was filling in for me while I was on leave. Now I'm back
> and I think I can help clarify why these changes were made. A summary
> for all the changes should help.
>
> There were two problems. First was a problem caused by setting the
> polarity in probe. It caused the speaker to click when the polarity was
> set so we took that out as it didn't seem to serve any useful purpose
> that I could see. The polarity changes should be made in the polarity
> callback.

Please provide more details about your configuration.

Are you using the pwm-beeper driver with a piezo?

After a reset, all PWM output will be low until the PWM clock is
enabled at which point it will be constant high.  Are you confident
that this transition is not responsible for the click you are hearing?

> The second and bigger problem was the smooth type sequence. If it isn't
> done according to the spec then one in ten or twenty times you won't
> even get a signal when it's enabled. Following the correct sequence with
> the 400 ns delays solves this problem.

Would the following minor change be sufficient to fix the issue?

diff --git a/drivers/pwm/pwm-bcm-kona.c b/drivers/pwm/pwm-bcm-kona.c
index 02bc048..c537efd 100644
--- a/drivers/pwm/pwm-bcm-kona.c
+++ b/drivers/pwm/pwm-bcm-kona.c
@@ -85,6 +85,9 @@ static void kona_pwmc_apply_settings(struct kona_pwmc *kp, uns
        value &= ~(1 << PWM_CONTROL_TRIGGER_SHIFT(chan));
        writel(value, kp->base + PWM_CONTROL_OFFSET);

+       ndelay(400);
+
        /* 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);

>
> Additionally, by not following the sequence you won't get a smooth
> transition. You'll get a change in the settings (duty cycle, period) but
> may get a non smooth transition. So it's important to follow the spec
> here. We don't want non-smooth transitions.

Please provide your rationale for requiring smooth transitions.

Thanks,
Tim Kryger