linux-kernel - Re: [PATCH 3/4] pwm: tegra: Add DT binding details to configure pin in suspends/resume

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20170406130315.GB7784@ulmo.ba.sec>
Date:   Thu, 6 Apr 2017 15:03:15 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Jon Hunter <jonathanh@...dia.com>
Cc:     Laxman Dewangan <ldewangan@...dia.com>, robh+dt@...nel.org,
        mark.rutland@....com, linux-pwm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] pwm: tegra: Add DT binding details to configure pin
 in suspends/resume

On Thu, Apr 06, 2017 at 09:57:09AM +0100, Jon Hunter wrote:
> 
> On 05/04/17 15:13, Laxman Dewangan wrote:
> > In some of NVIDIA Tegra's platform, PWM controller is used to
> > control the PWM controlled regulators. PWM signal is connected to
> > the VID pin of the regulator where duty cycle of PWM signal decide
> > the voltage level of the regulator output.
> > 
> > The tristate (high impedance of PWM pin form Tegra) also define
> > one of the state of PWM regulator which needs to be configure in
> > suspend state of system.
> > 
> > Add DT binding details to provide the pin configuration state
> > from PWM and pinctrl DT node in suspend and active state of
> > the system.
> > 
> > Signed-off-by: Laxman Dewangan <ldewangan@...dia.com>
> > ---
> >  .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 43 ++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> > index b4e7377..145c323 100644
> > --- a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> > +++ b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> > @@ -19,6 +19,19 @@ Required properties:
> >  - reset-names: Must include the following entries:
> >    - pwm
> >  
> > +Optional properties:
> > +============================
> > +In some of the interface like PWM based regualator device, it is required
> > +to configure the pins diffrently in different states, specially in suspend
> 
> s/diffrently/differently
> s/specially/especially
> 
> > +state of the system. The configuration of pin is provided via the pinctrl
> > +DT node as detailed in the pinctrl DT binding document
> > +	Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > +
> > +The PWM node will have following optional properties.
> > +pinctrl-names:	Pin state names. Must be "suspend" and "resume".
> 
> Why not just use the pre-defined names here? There is a pre-defined name
> for "default", "idle" and "sleep" and then you can use the following
> APIs and avoid the lookup of the state ...
> 
> pinctrl_pm_select_default_state()
> pinctrl_pm_select_idle_state()
> pinctrl_pm_select_sleep_state()
> 
> Note for i2c [0][1], I used "default" as the active/on state (which I
> know is not that descriptive) and then used 'idle' as the suspended
> state. This way we don't need any custom names.

Agreed, I think that's how these states are meant to be used.

Thierry

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