linux-kernel - Re: [RCF PATCH,v2,2/2] pwm: imx: Configure output to GPIO in disabled state

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]

Date:   Wed, 14 Nov 2018 12:14:15 +0100
From:   Thierry Reding <thierry.reding@...il.com>
To:     Vokáč Michal <Michal.Vokac@...ft.com>
Cc:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>, Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Lukasz Majewski <l.majewski@...ess.pl>,
        Fabio Estevam <fabio.estevam@....com>,
        Lothar Waßmann <LW@...o-electronics.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>
Subject: Re: [RCF PATCH,v2,2/2] pwm: imx:
 Configure output to GPIO in disabled state

On Thu, Nov 08, 2018 at 03:21:44PM +0000, Vokáč Michal wrote:
> On 7.11.2018 16:01, Uwe Kleine-König wrote:
[...]
> > For both the solution is to let the bootloader enable the pwm with
> > the right output level. Am I missing something?
> 
> Bootloader is only a small part of the whole solution I think. And I
> suppose you meant: "enable the *GPIO* with the right output level".
> 
>   - Even if you use GPIO in bootloader to set the required level the
>     time frame from imx_pwm_probe to imx_pwm_apply is not covered.
> 
>   - Currently there is no support in Linux pwm-imx driver to detect
>     the PWM chip is already enabled at probe time. I actually send
>     patches for this a month ago [1]. No response yet.
> 
>   - Inverted PWM does not work in U-Boot (on imx at least). And it
>     does not seam like it can be fixed easily. I do not know what is
>     the situation in other bootloaders.
> 
> So my current bootloader solution is one of:
>   - Set the pin to the appropriate (HIGH) level using GPIO.
>   - Do not touch the pin at all, it has 100k pull-up by default.

First of all, I don't think we should rely on any bootloader setting up
things properly. I already said elsewhere that the reset defaults will
likely already be such that the PWM outputs zero power (i.e. high for
inversed PWM). Michal's comments above seem to suggest that this is
indeed the case. If the pin is 100k pull-up by default (I assume that
means on reset), then that's exactly what I would expect for this kind
of pin.

And that's also an excellent pin state for the kernel to use when it no
longer needs to actively drive the PWM. That means on pwm_disable() we
can simply revert to the 100k pull-up default and let the PWM pin be in
the very same state that it is on reset. Can't get any better than that.

So really, I think the only proper solution to this problem is to get
pinctrl involved and configure the pin as PWM when it is actively used,
and change it back to 100k pull-up otherwise.

And, yes, I understand that this slightly complicates the driver, but
it's really the right thing to do and it fixes all known issues, and to
me that's clear evidence that it is the right solution and therefore
definitely worth the added complexity.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)