Re: [PATCH v3 3/3] arm64: dts: ti: k3-pinctrl: Add the remaining macros

["Kumar, Udit" <u-kumar1@ti.com>]

Hello Akashdeep,

On 9/2/2025 12:49 PM, Akashdeep Kaur wrote:
> Add the drive stregth, schmitt trigger enable macros to pinctrl file.
> Add the missing macros for DeepSleep configuration control referenced
> from "Table 14-6172. Description Of The Pad Configuration Register Bits"
> in AM625 TRM[0].
> Add some DeepSleep macros to provide combinations that can be used
> directly in device tree files example PIN_DS_OUTPUT_LOW that
> configures pin to be output and also sets its value to 0.
>
> [0] https://www.ti.com/lit/ug/spruiv7b/spruiv7b.pdf
>
> Signed-off-by: Akashdeep Kaur <a-kaur@ti.com>
> ---
>   arch/arm64/boot/dts/ti/k3-pinctrl.h | 55 +++++++++++++++++++++++++++--
>   1 file changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-pinctrl.h b/arch/arm64/boot/dts/ti/k3-pinctrl.h
> index c0f09be8d3f9..39aad59075d1 100644
> --- a/arch/arm64/boot/dts/ti/k3-pinctrl.h
> +++ b/arch/arm64/boot/dts/ti/k3-pinctrl.h
> @@ -3,15 +3,20 @@
>    * This header provides constants for pinctrl bindings for TI's K3 SoC
>    * family.
>    *
> - * Copyright (C) 2018-2024 Texas Instruments Incorporated - https://www.ti.com/
> + * Copyright (C) 2018-2025 Texas Instruments Incorporated - https://www.ti.com/
>    */
>   #ifndef DTS_ARM64_TI_K3_PINCTRL_H
>   #define DTS_ARM64_TI_K3_PINCTRL_H
>   
> +#define WKUP_LVL_EN_SHIFT       (7)
> +#define WKUP_LVL_POL_SHIFT      (8)
>   #define ST_EN_SHIFT		(14)
>   #define PULLUDEN_SHIFT		(16)
>   #define PULLTYPESEL_SHIFT	(17)
>   #define RXACTIVE_SHIFT		(18)
> +#define DRV_STR_SHIFT           (19)

referring to above TRM mentioned in commit message

Bit 20-19 are for DRV_STR, and description says

0 - Default
1 - Reserved
2 - Reserved
3 - Reserved

Not sure, is there some additional document to be referred for 
PIN_DRIVE_STRENGTH


> +#define DS_ISO_OVERRIDE_SHIFT   (22)
> +#define DS_ISO_BYPASS_EN_SHIFT  (23)

Please follow same convention as for rest of bit fields

DS_ISO_OVERRIDE_SHIFT  to ISO_OVR_SHIFT and
DS_ISO_BYPASS_EN_SHIFT to ISO_BYP_SHIFT



>   #define DEBOUNCE_SHIFT		(11)
>   #define FORCE_DS_EN_SHIFT	(15)
>   #define DS_EN_SHIFT		(24)
> @@ -19,6 +24,7 @@
>   #define DS_OUT_VAL_SHIFT	(26)
>   #define DS_PULLUD_EN_SHIFT	(27)
>   #define DS_PULLTYPE_SEL_SHIFT	(28)
> +#define WKUP_EN_SHIFT           (29)
>   
>   /* Schmitt trigger configuration */
>   #define ST_DISABLE		(0 << ST_EN_SHIFT)
> @@ -33,6 +39,26 @@
>   #define INPUT_EN		(1 << RXACTIVE_SHIFT)
>   #define INPUT_DISABLE		(0 << RXACTIVE_SHIFT)
>   
> +#define DS_PULL_DISABLE         (1 << DS_PULLUD_EN_SHIFT)
> +#define DS_PULL_ENABLE          (0 << DS_PULLUD_EN_SHIFT)

what is purpose of shifting zero,


> +
> +#define DS_PULL_UP              (1 << DS_PULLTYPE_SEL_SHIFT | DS_PULL_ENABLE)
> +#define DS_PULL_DOWN            (0 << DS_PULLTYPE_SEL_SHIFT | DS_PULL_ENABLE)
> +
> +#define DS_INPUT_EN             (1 << DS_OUT_DIS_SHIFT)
> +#define DS_INPUT_DISABLE        (0 << DS_OUT_DIS_SHIFT)
> +
> +#define DS_OUT_VALUE_ZERO       (0 << DS_OUT_VAL_SHIFT)
> +#define DS_OUT_VALUE_ONE        (1 << DS_OUT_VAL_SHIFT)
> +
> +#define WKUP_ENABLE             (1 << WKUP_EN_SHIFT)
> +#define WKUP_ON_LEVEL           (1 << WKUP_LVL_EN_SHIFT)
> +#define WKUP_ON_EDGE            (0 << WKUP_LVL_EN_SHIFT)
> +#define WKUP_LEVEL_LOW          (0 << WKUP_LVL_POL_SHIFT)
> +#define WKUP_LEVEL_HIGH         (1 << WKUP_LVL_POL_SHIFT)
> +
> +#define WKUP_DISABLE            (0 << WKUP_EN_SHIFT)
> +
>   /* Only these macros are expected be used directly in device tree files */
>   #define PIN_OUTPUT		(INPUT_DISABLE | PULL_DISABLE)
>   #define PIN_OUTPUT_PULLUP	(INPUT_DISABLE | PULL_UP)
> @@ -53,18 +79,41 @@
>   #define PIN_DEBOUNCE_CONF5	(5 << DEBOUNCE_SHIFT)
>   #define PIN_DEBOUNCE_CONF6	(6 << DEBOUNCE_SHIFT)
>   
> +#define PIN_DRIVE_STRENGTH_NOMINAL      (0 << DRV_STR_SHIFT)
> +#define PIN_DRIVE_STRENGTH_SLOW         (1 << DRV_STR_SHIFT)
> +#define PIN_DRIVE_STRENGTH_FAST         (2 << DRV_STR_SHIFT)
> +
> +#define PIN_SCHMITT_TRIGGER_DISABLE	(0 << ST_EN_SHIFT)
> +#define PIN_SCHMITT_TRIGGER_ENABLE	(1 << ST_EN_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_ENABLE		DS_INPUT_DISABLE
> +#define PIN_DS_OUT_DISABLE		DS_INPUT_EN
>   #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)
> +#define PIN_DS_ISO_BYPASS               (1 << DS_ISO_BYPASS_EN_SHIFT)
> +#define PIN_DS_ISO_BYPASS_DISABLE       (0 << DS_ISO_BYPASS_EN_SHIFT)
> +
> +#define DS_STATE_VAL                    (1 << DS_EN_SHIFT)
> +#define ACTIVE_STATE_VAL                (0 << DS_EN_SHIFT)
> +

Please do not mix PIN_x #define with other internal defines


> +#define PIN_DS_OUTPUT_LOW               (DS_STATE_VAL | DS_INPUT_DISABLE | DS_OUT_VALUE_ZERO)
> +#define PIN_DS_OUTPUT_HIGH              (DS_STATE_VAL | DS_INPUT_DISABLE | DS_OUT_VALUE_ONE)
> +#define PIN_DS_INPUT                    (DS_STATE_VAL | DS_INPUT_EN | DS_PULL_DISABLE)
> +#define PIN_DS_INPUT_PULLUP             (DS_STATE_VAL | DS_INPUT_EN | DS_PULL_UP)
> +#define PIN_DS_INPUT_PULLDOWN           (DS_STATE_VAL | DS_INPUT_EN | DS_PULL_DOWN)
> +
> +#define PIN_WKUP_EN_EDGE                (WKUP_ENABLE | WKUP_ON_EDGE)
> +#define PIN_WKUP_EN_LEVEL_LOW           (WKUP_ENABLE | WKUP_ON_LEVEL | WKUP_LEVEL_LOW)
> +#define PIN_WKUP_EN_LEVEL_HIGH          (WKUP_ENABLE | WKUP_ON_LEVEL | WKUP_LEVEL_HIGH)
> +#define PIN_WKUP_EN                     WKUP_EN_EDGE

what is difference between PIN_WKUP_EN_EDGE and PIN_WKUP_EN


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