linux-kernel - Re: [PATCH v2 1/2] pwm: kona: Fix incorrect enable, config, and disable procedures

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD7vxxJ2++PNteSZ+F5epvPW_nrrSZT9pja325pmLVhq_MHL0w@mail.gmail.com>
Date:	Sun, 14 Dec 2014 23:18:35 -0800
From:	Tim Kryger <tim.kryger@...il.com>
To:	Jonathan Richardson <jonathar@...adcom.com>
Cc:	Scott Branden <sbranden@...adcom.com>,
	Arun Ramamurthy <arun.ramamurthy@...adcom.com>,
	Thierry Reding <thierry.reding@...il.com>,
	Ray Jui <rjui@...adcom.com>,
	bcm-kernel-feedback-list@...adcom.com,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-pwm@...r.kernel.org
Subject: Re: [PATCH v2 1/2] pwm: kona: Fix incorrect enable, config, and
 disable procedures

On Wed, Dec 10, 2014 at 5:07 PM, Jonathan Richardson
<jonathar@...adcom.com> wrote:

>     If config is called when the pwm is disabled and there is nothing to do,
>     the while loop to calculate duty cycle and period doesn't need to be
>     done. The function now just returns if the pwm state is disabled.

It doesn't take long to figure out whether the duty and period are achievable.

If the caller specifies bad settings, why not return an error immediately?

> @@ -207,13 +240,23 @@ 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 = readl(kp->base + PWM_CONTROL_OFFSET);
> +
> +       /* Set smooth type to 0 and disable */
> +       value &= ~(1 << PWM_CONTROL_SMOOTH_SHIFT(chan));
> +       value &= ~(1 << PWM_CONTROL_TRIGGER_SHIFT(chan));
> +       writel(value, kp->base + PWM_CONTROL_OFFSET);
>
>         /* 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);
>  }

I've mentioned this before but I will say it again, when the smooth
and trigger bit are both low, the output is constant high.

If you look at the PWM output on a scope you will see it go high for
400 ns during your disable even if the duty prior to the disable was
zero.

How are you testing your proposed changes?

Thanks,
Tim Kryger
--
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/