[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a89f9e4f-5306-4943-a70f-f6a60a4537d3@ti.com>
Date: Mon, 8 Sep 2025 18:34:19 -0500
From: Kendall Willis <k-willis@...com>
To: Akashdeep Kaur <a-kaur@...com>, <praneeth@...com>, <nm@...com>,
<afd@...com>, <vigneshr@...com>, <d-gole@...com>, <u-kumar1@...com>,
<sebin.francis@...com>, <kristo@...nel.org>, <robh@...nel.org>,
<krzk+dt@...nel.org>, <conor+dt@...nel.org>,
<linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
CC: <vishalm@...com>
Subject: Re: [PATCH v5 3/4] arm64: dts: ti: k3-pinctrl: Add the remaining
macros
On 9/5/25 00:14, Akashdeep Kaur wrote:
> Add the drive strength, 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"
The Table number should be 14-8769 for the AM62P TRM.
> in AM625 TRM[0].
You should reference both the AM62X and AM62P TRMs because in the AM62P
TRM the ISO_OVERRIDE_EN bit is reserved, whereas in the AM62X TRM it is
defined.
> 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/pdf/SPRUJ83
>
> Signed-off-by: Akashdeep Kaur <a-kaur@...com>
> ---
> arch/arm64/boot/dts/ti/k3-pinctrl.h | 47 ++++++++++++++++++++++++++++-
> 1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-pinctrl.h b/arch/arm64/boot/dts/ti/k3-pinctrl.h
> index c0f09be8d3f9..8ce37ace94c9 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)
> +#define ISO_OVERRIDE_EN_SHIFT (22)
> +#define ISO_BYPASS_EN_SHIFT (23)
> #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,29 @@
> #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)
nit: IMO either keep the format of these macros to ENABLE or EN unless
there is a good reason to change the format.
> +
> +#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_STATE_EN (1 << DS_EN_SHIFT)
> +#define DS_STATE_DISABLE (0 << DS_EN_SHIFT)
nit: Is there anyway this could be more descriptive? Like
DS_IO_OVERRIDE_EN or DS_OVERRIDE_CTRL. It is hard to tell what these
bits do unless you look at the TRM, whereas the other macros are easier
to deduce their function.
> +
> +#define DS_INPUT_EN (1 << DS_OUT_DIS_SHIFT | DS_STATE_EN)
> +#define DS_INPUT_DISABLE (0 << DS_OUT_DIS_SHIFT | DS_STATE_EN)
By looking at the TRM it looks like this disables or enables output, not
input. Shifting a 1 to DS_OUT_DIS_SHIFT should disable output.
> +
> +#define DS_OUT_VALUE_ZERO (0 << DS_OUT_VAL_SHIFT)
> +#define DS_OUT_VALUE_ONE (1 << DS_OUT_VAL_SHIFT)
> +
> +/* Configuration to enable wake-up on pin activity */
> +#define WKUP_ENABLE (1 << WKUP_EN_SHIFT)
> +#define WKUP_DISABLE (0 << 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)
> +
> /* 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,6 +82,10 @@
> #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)
DRV_STR value of 1 is reserved in both AM62X and AM62P TRMs
> +#define PIN_DRIVE_STRENGTH_FAST (2 << DRV_STR_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)
> @@ -65,6 +98,18 @@
> #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 << ISO_BYPASS_EN_SHIFT)
> +#define PIN_DS_ISO_BYPASS_DISABLE (0 << ISO_BYPASS_EN_SHIFT)
> +
> +#define PIN_DS_OUTPUT_LOW (DS_INPUT_DISABLE | DS_OUT_VALUE_ZERO)
> +#define PIN_DS_OUTPUT_HIGH (DS_INPUT_DISABLE | DS_OUT_VALUE_ONE)
> +#define PIN_DS_INPUT (DS_INPUT_EN | DS_PULL_DISABLE)
> +#define PIN_DS_INPUT_PULLUP (DS_INPUT_EN | DS_PULL_UP)
> +#define PIN_DS_INPUT_PULLDOWN (DS_INPUT_EN | DS_PULL_DOWN)
> +
> +#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_ENABLE | WKUP_ON_EDGE)
>
> /* Default mux configuration for gpio-ranges to use with pinctrl */
> #define PIN_GPIO_RANGE_IOPAD (PIN_INPUT | 7)
Best,
Kendall
Powered by blists - more mailing lists