linux-kernel - Re: [PATCH 1/2] arm64: dts: ti: k3-pinctrl: Introduce deep sleep macros

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] [day] [month] [year] [list]

Message-ID: <5b35c736-74d6-4fe9-ae82-272dc2e98b82@ti.com>
Date: Mon, 18 Nov 2024 10:49:26 +0530
From: Siddharth Vadapalli <s-vadapalli@...com>
To: Rob Herring <robh@...nel.org>
CC: Siddharth Vadapalli <s-vadapalli@...com>, <nm@...com>, <vigneshr@...com>,
        <kristo@...nel.org>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <srk@...com>
Subject: Re: [PATCH 1/2] arm64: dts: ti: k3-pinctrl: Introduce deep sleep
 macros

On Fri, Nov 15, 2024 at 09:48:22AM -0600, Rob Herring wrote:

Hello Rob,

> On Tue, Nov 12, 2024 at 05:26:49PM +0530, Siddharth Vadapalli wrote:
> > The behavior of pins in deep sleep mode can be configured by programming
> > the corresponding bits in the respective Pad Configuration register. Add
> > macros to support this.
> > 
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@...com>
> > ---
> >  arch/arm64/boot/dts/ti/k3-pinctrl.h | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/ti/k3-pinctrl.h b/arch/arm64/boot/dts/ti/k3-pinctrl.h
> > index 22b8d73cfd32..cac7cccc1112 100644
> > --- a/arch/arm64/boot/dts/ti/k3-pinctrl.h
> > +++ b/arch/arm64/boot/dts/ti/k3-pinctrl.h
> > @@ -12,6 +12,12 @@
> >  #define PULLTYPESEL_SHIFT	(17)
> >  #define RXACTIVE_SHIFT		(18)
> >  #define DEBOUNCE_SHIFT		(11)
> > +#define FORCE_DS_EN_SHIFT	(15)
> > +#define DS_EN_SHIFT		(24)
> > +#define DS_OUT_DIS_SHIFT	(25)
> > +#define DS_OUT_VAL_SHIFT	(26)
> > +#define DS_PULLUD_EN_SHIFT	(27)
> > +#define DS_PULLTYPE_SEL_SHIFT	(28)
> >  
> >  #define PULL_DISABLE		(1 << PULLUDEN_SHIFT)
> >  #define PULL_ENABLE		(0 << PULLUDEN_SHIFT)
> > @@ -38,6 +44,19 @@
> >  #define PIN_DEBOUNCE_CONF5	(5 << DEBOUNCE_SHIFT)
> >  #define PIN_DEBOUNCE_CONF6	(6 << DEBOUNCE_SHIFT)
> >  
> > +#define PIN_DS_FORCE_DISABLE		(0 << FORCE_DS_EN_SHIFT)
> > +#define PIN_DS_FORCE_ENABLE		(1 << FORCE_DS_EN_SHIFT)
> > +#define PIN_DS_IO_OVERRIDE_DISABLE	(0 << DS_IO_OVERRIDE_EN_SHIFT)
> > +#define PIN_DS_IO_OVERRIDE_ENABLE	(1 << DS_IO_OVERRIDE_EN_SHIFT)
> > +#define PIN_DS_OUT_ENABLE		(0 << DS_OUT_DIS_SHIFT)
> > +#define PIN_DS_OUT_DISABLE		(1 << DS_OUT_DIS_SHIFT)
> > +#define PIN_DS_OUT_VALUE_ZERO		(0 << DS_OUT_VAL_SHIFT)
> > +#define PIN_DS_OUT_VALUE_ONE		(1 << DS_OUT_VAL_SHIFT)
> > +#define PIN_DS_PULLUD_ENABLE		(0 << DS_PULLUD_EN_SHIFT)
> > +#define PIN_DS_PULLUD_DISABLE		(1 << DS_PULLUD_EN_SHIFT)
> > +#define PIN_DS_PULL_DOWN		(0 << DS_PULLTYPE_SEL_SHIFT)
> > +#define PIN_DS_PULL_UP			(1 << DS_PULLTYPE_SEL_SHIFT)
> 
> Are you going to go add the 0 defines to all the existing cases? If you 
> do, it's a lot of pointless churn. If you don't, then it is inconsistent 
> when they do get used. I would drop them all.

The "0 defines" are present for the existing cases as well, namely:
PULL_ENABLE, PULL_DOWN and INPUT_DISABLE are all "0 defines".

Other existing macros are defined in terms of the above, due to which it
might have appeared to be the case that only some of the "0 defines" are
present. For example, the following macros make use of the "0 defines":
PIN_OUTPUT, PIN_OUTPUT_PULLUP, PIN_OUTPUT_PULLDOWN and PIN_INPUT_PULLDOWN

So the current patch is consistent with the existing convention followed
in the k3-pinctrl.h file. Please let me know if I should still drop the
"0 defines" in this patch.

Regards,
Siddharth.