diff mbox series

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

Message ID 20241112115650.988943-2-s-vadapalli@ti.com (mailing list archive)
State New
Headers show
Series Add Deep Sleep pinmux macros for TI's K3 SoCs | expand

Commit Message

Siddharth Vadapalli Nov. 12, 2024, 11:56 a.m. UTC
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@ti.com>
---
 arch/arm64/boot/dts/ti/k3-pinctrl.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Rob Herring Nov. 15, 2024, 3:48 p.m. UTC | #1
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@ti.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.

Rob
Siddharth Vadapalli Nov. 18, 2024, 5:19 a.m. UTC | #2
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@ti.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.
diff mbox series

Patch

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)
+
 /* Default mux configuration for gpio-ranges to use with pinctrl */
 #define PIN_GPIO_RANGE_IOPAD	(PIN_INPUT | 7)