Hi Eugen,
On 31/01/25 12:10 am, Eugen Hristev wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On 1/30/25 05:51, Manikandan Muralidharan wrote:
>> From: Balamanikandan Gunasundar <balamanikandan.gunasun...@microchip.com>
>>
>> Define the pinctrl nodes with its label to align with the Linux DT.
>> Without this change the pinmux nodes are grouped under an additional
>> 'pinctrl' child node which is not identified by the pinctrl driver
>> when the GPIO banks are made as child nodes of pinctrl node.
>
> This DT commit breaks the boards, and you are fixing the driver after ?
This patch is a potential fix for an issue in u-boot where pin-mux of
nand, qspi etc are not grouped or identified by the pinctrl-at91.c
driver because in board DTS they are defined under a plain 'pinctrl'
node entity without the pinctrl DTSI-label reference or full path
representation, hence in the final dtb there are two different pinctrl
node entities (1) pinctrl@fffff40 (2) pinctrl.
The patch has no dependency with the driver changes.
>
> Does changing the driver mean that this old DT stops working ? Or it
> still works?
In patch 6/7, the fallback to current code flow to capture the reg
address of GPIO banks are still available along with the new
implementation but since the 5/7 binds the GPIO driver with pinctrl, I
doubt in the old DTs with pioX GPIO banks defined outside the pinctrl
node may not work.>
> Eugen
>>
>> Signed-off-by: Balamanikandan Gunasundar
>> <balamanikandan.gunasun...@microchip.com>
>> Signed-off-by: Manikandan Muralidharan <manikanda...@microchip.com>
>> ---
>> arch/arm/dts/sam9x60ek.dts | 142 +++++++++++++++++++------------------
>> 1 file changed, 73 insertions(+), 69 deletions(-)
>>
>> diff --git a/arch/arm/dts/sam9x60ek.dts b/arch/arm/dts/sam9x60ek.dts
>> index 74016f5e288..3bf45c14e42 100644
>> --- a/arch/arm/dts/sam9x60ek.dts
>> +++ b/arch/arm/dts/sam9x60ek.dts
>> @@ -78,75 +78,6 @@
>> };
>> };
>> };
>> -
>> - pinctrl {
>> - nand {
>> - pinctrl_nand_oe_we: nand-oe-we-0 {
>> - atmel,pins =
>> - <AT91_PIOD 0
>> AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
>> - AT91_PIOD 1
>> AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)>;
>> - };
>> -
>> - pinctrl_nand_rb: nand-rb-0 {
>> - atmel,pins =
>> - <AT91_PIOD 5
>> AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP>;
>> - };
>> -
>> - pinctrl_nand_cs: nand-cs-0 {
>> - atmel,pins =
>> - <AT91_PIOD 4
>> AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP>;
>> - };
>> - };
>> -
>> - ebi {
>> - pinctrl_ebi_data_0_7: ebi-data-lsb-0 {
>> - atmel,pins =
>> - <AT91_PIOD 6
>> AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
>> - AT91_PIOD 7
>> AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
>> - AT91_PIOD 8
>> AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
>> - AT91_PIOD 9
>> AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
>> - AT91_PIOD 10
>> AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
>> - AT91_PIOD 11
>> AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
>> - AT91_PIOD 12
>> AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
>> - AT91_PIOD 13
>> AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)>;
>> - };
>> -
>> - pinctrl_ebi_addr_nand: ebi-addr-0 {
>> - atmel,pins =
>> - <AT91_PIOD 2
>> AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
>> - AT91_PIOD 3
>> AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)>;
>> - };
>> - };
>> -
>> - pinctrl_qspi: qspi {
>> - atmel,pins =
>> - <AT91_PIOB 19 AT91_PERIPH_A
>> AT91_PINCTRL_NONE
>> - AT91_PIOB 20 AT91_PERIPH_A
>> AT91_PINCTRL_NONE
>> - AT91_PIOB 21 AT91_PERIPH_A
>> AT91_PINCTRL_PULL_UP
>> - AT91_PIOB 22 AT91_PERIPH_A
>> AT91_PINCTRL_PULL_UP
>> - AT91_PIOB 23 AT91_PERIPH_A
>> AT91_PINCTRL_PULL_UP
>> - AT91_PIOB 24 AT91_PERIPH_A
>> AT91_PINCTRL_PULL_UP>;
>> - };
>> -
>> - pinctrl_flx0: flx0_default {
>> - atmel,pins =
>> - <AT91_PIOA 0 AT91_PERIPH_A
>> AT91_PINCTRL_NONE
>> - AT91_PIOA 1 AT91_PERIPH_A
>> AT91_PINCTRL_NONE>;
>> - };
>> -
>> - pinctrl_onewire_tm_default: onewire_tm_default
>> {
>> - atmel,pins =
>> - <AT91_PIOD 14 AT91_PERIPH_GPIO
>> AT91_PINCTRL_PULL_UP>;
>> - };
>> -
>> - usb1 {
>> - pinctrl_usb_default: usb_default {
>> - atmel,pins = <AT91_PIOD 15
>> AT91_PERIPH_GPIO AT91_PINCTRL_NONE
>> - AT91_PIOD 16
>> AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
>> - };
>> - };
>> -
>> - };
>> };
>> };
>> };
>> @@ -221,6 +152,79 @@
>> status = "okay";
>> };
>>
>> +&pinctrl {
>> + /* shared pinctrl settings */
>> + qspi {
>> + pinctrl_qspi: qspi {
>> + atmel,pins =
>> + <AT91_PIOB 19 AT91_PERIPH_A AT91_PINCTRL_NONE
>> + AT91_PIOB 20 AT91_PERIPH_A AT91_PINCTRL_NONE
>> + AT91_PIOB 21 AT91_PERIPH_A
>> AT91_PINCTRL_PULL_UP
>> + AT91_PIOB 22 AT91_PERIPH_A
>> AT91_PINCTRL_PULL_UP
>> + AT91_PIOB 23 AT91_PERIPH_A
>> AT91_PINCTRL_PULL_UP
>> + AT91_PIOB 24 AT91_PERIPH_A
>> AT91_PINCTRL_PULL_UP>;
>> + };
>> + };
>> +
>> + nand {
>> + pinctrl_nand_oe_we: nand-oe-we-0 {
>> + atmel,pins =
>> + <AT91_PIOD 0 AT91_PERIPH_A (AT91_PINCTRL_NONE
>> | AT91_PINCTRL_SLEWRATE_DIS)
>> + AT91_PIOD 1 AT91_PERIPH_A (AT91_PINCTRL_NONE
>> | AT91_PINCTRL_SLEWRATE_DIS)>;
>> + };
>> +
>> + pinctrl_nand_rb: nand-rb-0 {
>> + atmel,pins =
>> + <AT91_PIOD 5 AT91_PERIPH_GPIO
>> AT91_PINCTRL_PULL_UP>;
>> + };
>> +
>> + pinctrl_nand_cs: nand-cs-0 {
>> + atmel,pins =
>> + <AT91_PIOD 4 AT91_PERIPH_GPIO
>> AT91_PINCTRL_PULL_UP>;
>> + };
>> + };
>> +
>> + ebi {
>> + pinctrl_ebi_data_0_7: ebi-data-lsb-0 {
>> + atmel,pins =
>> + <AT91_PIOD 6 AT91_PERIPH_A (AT91_PINCTRL_NONE
>> | AT91_PINCTRL_SLEWRATE_DIS)
>> + AT91_PIOD 7 AT91_PERIPH_A (AT91_PINCTRL_NONE
>> | AT91_PINCTRL_SLEWRATE_DIS)
>> + AT91_PIOD 8 AT91_PERIPH_A (AT91_PINCTRL_NONE
>> | AT91_PINCTRL_SLEWRATE_DIS)
>> + AT91_PIOD 9 AT91_PERIPH_A (AT91_PINCTRL_NONE
>> | AT91_PINCTRL_SLEWRATE_DIS)
>> + AT91_PIOD 10 AT91_PERIPH_A (AT91_PINCTRL_NONE
>> | AT91_PINCTRL_SLEWRATE_DIS)
>> + AT91_PIOD 11 AT91_PERIPH_A (AT91_PINCTRL_NONE
>> | AT91_PINCTRL_SLEWRATE_DIS)
>> + AT91_PIOD 12 AT91_PERIPH_A (AT91_PINCTRL_NONE
>> | AT91_PINCTRL_SLEWRATE_DIS)
>> + AT91_PIOD 13 AT91_PERIPH_A (AT91_PINCTRL_NONE
>> | AT91_PINCTRL_SLEWRATE_DIS)>;
>> + };
>> +
>> + pinctrl_ebi_addr_nand: ebi-addr-0 {
>> + atmel,pins =
>> + <AT91_PIOD 2 AT91_PERIPH_A (AT91_PINCTRL_NONE
>> | AT91_PINCTRL_SLEWRATE_DIS)
>> + AT91_PIOD 3 AT91_PERIPH_A (AT91_PINCTRL_NONE
>> | AT91_PINCTRL_SLEWRATE_DIS)>;
>> + };
>> + };
>> +
>> + flexcom {
>> + pinctrl_flx0: flx0_default {
>> + atmel,pins =
>> + <AT91_PIOA 0 AT91_PERIPH_A AT91_PINCTRL_NONE
>> + AT91_PIOA 1 AT91_PERIPH_A AT91_PINCTRL_NONE>;
>> + };
>> + };
>> +
>> + pinctrl_onewire_tm_default: onewire_tm_default {
>> + atmel,pins =
>> + <AT91_PIOD 14 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP>;
>> + };
>> +
>> + usb1 {
>> + pinctrl_usb_default: usb_default {
>> + atmel,pins = <AT91_PIOD 15 AT91_PERIPH_GPIO
>> AT91_PINCTRL_NONE
>> + AT91_PIOD 16 AT91_PERIPH_GPIO
>> AT91_PINCTRL_NONE>;
>> + };
>> + };
>> +};
>> +
>> &usb1 {
>> num-ports = <3>;
>> atmel,vbus-gpio = <0
>
--
Thanks and Regards,
Manikandan M.
