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

[Akashdeep Kaur <a-kaur@ti.com>]

Hi Udit,

On 04/09/25 14:27, Kumar, Udit wrote:
> 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
...
>>   #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

This information will be updated in TRM in coming cycles.
> 
> 
>> +#define DS_ISO_OVERRIDE_SHIFT   (22)
>> +#define DS_ISO_BYPASS_EN_SHIFT  (23)
> 
> Please follow same convention as for rest of bit fields

Updated.

> 
> 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,
This is added for consistency across the entire file.
> 
> 
>> +
>> +#define DS_PULL_UP              (1 << DS_PULLTYPE_SEL_SHIFT | 
...
>> +#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

Moved these to appropriate location.

> 
>> +#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
Combined the macros to have default wakeup on edge
> 
> 
>>   /* Default mux configuration for gpio-ranges to use with pinctrl */
>>   #define PIN_GPIO_RANGE_IOPAD    (PIN_INPUT | 7)

Regards,
Akashdeep Kaur