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

[Vignesh Raghavendra <vigneshr@ti.com>]

On 31/07/25 17:26, Akashdeep Kaur wrote:
> Add the drive strength, schmitt trigger enable macros to pinctrl file.
> Add the missing macros for deep sleep configuration control.
> Reword the existing deep sleep macros to provide combinations that can
> directly be used in device tree files.
> 
> Signed-off-by: Akashdeep Kaur <a-kaur@ti.com>
> ---
>  arch/arm64/boot/dts/ti/k3-pinctrl.h | 66 +++++++++++++++++++++++------
>  1 file changed, 54 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-pinctrl.h b/arch/arm64/boot/dts/ti/k3-pinctrl.h
> index c0f09be8d3f9..f26f1fcf6f74 100644
> --- a/arch/arm64/boot/dts/ti/k3-pinctrl.h
> +++ b/arch/arm64/boot/dts/ti/k3-pinctrl.h
> @@ -3,7 +3,7 @@
>   * 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
> @@ -19,6 +19,13 @@
>  #define DS_OUT_VAL_SHIFT	(26)
>  #define DS_PULLUD_EN_SHIFT	(27)
>  #define DS_PULLTYPE_SEL_SHIFT	(28)


> +#define WKUP_EN_SHIFT		    (29)
> +#define WKUP_LVL_EN_SHIFT	    (7)
> +#define WKUP_LVL_POL_SHIFT	    (8)
> +#define ST_EN_SHIFT		        (14)
> +#define DRV_STR_SHIFT		    (19)
> +#define DS_ISO_OVERRIDE_EN_SHIFT (22)
> +#define DS_ISO_BYPASS_EN_SHIFT  (23)

These *_SHIFT macros are mostly arranged in ascending order of bit
position, please follow the same.

Also, ST_EN_SHIFT is already defined ?

>  
>  /* Schmitt trigger configuration */
>  #define ST_DISABLE		(0 << ST_EN_SHIFT)
> @@ -33,6 +40,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)
> +
> +#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 +80,33 @@
>  #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_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)

No we cannot delete these. There maybe downstream dts files already
using these macros and thus can cause incompatibilities.

> +#define PIN_DS_FORCE_ENABLE		    (1 << FORCE_DS_EN_SHIFT)

> +#define PIN_DS_ISO_OVERRIDE_DISABLE	(0 << DS_ISO_OVERRIDE_EN_SHIFT)
> +#define PIN_DS_ISO_OVERRIDE	        (1 << DS_ISO_OVERRIDE_EN_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)
> +
> +#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 WKUP_EN_EDGE		(WKUP_ENABLE | WKUP_ON_EDGE)
> +#define WKUP_EN_LEVEL_LOW	(WKUP_ENABLE | WKUP_ON_LEVEL | WKUP_LEVEL_LOW)
> +#define WKUP_EN_LEVEL_HIGH	(WKUP_ENABLE | WKUP_ON_LEVEL | WKUP_LEVEL_HIGH)
> +#define WKUP_EN		        WKUP_EN_EDGE

All existing macros in this block have PIN_* suffix. Please align with
the same.

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


For future versions, please split the changes to 2 patches: one for
white-space changes only and one for addition of new macros.

-- 
Regards
Vignesh
https://ti.com/opensource