[PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts

Manikandan.M at microchip.com Manikandan.M at microchip.com
Fri Oct 18 11:29:47 CEST 2024


Hi Eugen,

On 18/10/24 12:45 pm, Eugen Hristev wrote:
> [You don't often get email from eugen.hristev at linaro.org. Learn why this 
> is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> the content is safe
> 
> Hello Alexey,
> 
> Please fix the subject to adhere to the rules ARM: dts: .... etc,
> if you are unsure, please follow previous commits that touched this file.
> 
> On 10/17/24 11:51, Manikandan.M at microchip.com wrote:
>> Hi Alexey,
>>
>> On 15/10/24 8:23 pm, Alexey Tsirlin wrote:
>>> This allows setting the GPIO parameters from device tree, otherwise the
>>> at91_pin_check_config will fail because the priv->nbanks equal to zero
> 
> I remember these pin banks are outside of the pinctrl because the driver
> fails to probe them if they are inside.
> Is this no longer true ?
Indeed, you are correct.With the current code base the pinctrl fails to 
probe if they are defined inside.
I started to review this code with an intention that my changes for 
pinctrl driver and DT to align U-Boot pinctrl DT node with the kernel 
had already been made upstream, however that is not the case.
This patch is valid and necessary only after when my changes are 
up-streamed otherwise driver probe will fail

Since I don't own a board with this SoC, Alexey, could you kindly check 
the GPIO functions and determine whether this patch is actually 
necessary for the problem you're experiencing?If not, after 
incorporating the driver changes, you can send this as part of the DT 
alignment
> 
> Manikandan, is it possible to test this on the board? and use the gpio
> command in U-boot to toggle the pins , like e.g. for the LEDs and see if
> there is no regression ?
> 
> Thanks,
> Eugen
>>>
>>> Signed-off-by: Alexey Tsirlin <alexey at all4bambi.com>
>>> ---
>>>
>>>    arch/arm/dts/sama5d3.dtsi | 111 
>>> +++++++++++++++++++-------------------
>>>    1 file changed, 56 insertions(+), 55 deletions(-)
>>>
>>> diff --git a/arch/arm/dts/sama5d3.dtsi b/arch/arm/dts/sama5d3.dtsi
>>> index 4c03a302ec..c671ea42f2 100644
>>> --- a/arch/arm/dts/sama5d3.dtsi
>>> +++ b/arch/arm/dts/sama5d3.dtsi
>>> @@ -873,66 +873,67 @@
>>>                                                       AT91_PIOE 17 
>>> AT91_PERIPH_B AT91_PINCTRL_NONE>; /* PE17 periph B, conflicts with 
>>> A17 */
>>>                                      };
>>>                              };
>>> -                    };
>>>
>>> -                    pioA: gpio at fffff200 {
>>> -                            compatible = "atmel,at91sam9x5-gpio", 
>>> "atmel,at91rm9200-gpio";
>>> -                            reg = <0xfffff200 0x100>;
>>> -                            interrupts = <6 IRQ_TYPE_LEVEL_HIGH 1>;
>>> -                            #gpio-cells = <2>;
>>> -                            gpio-controller;
>>> -                            interrupt-controller;har
>>> -                            #interrupt-cells = <2>;
>>> -                            clocks = <&pioA_clk>;
>>> -                            bootph-all;
>>> -                    };
>>> +                            pioA: gpio at fffff200 {
>>> +                                  compatible = 
>>> "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
>> Spaces instead of tab before 'compatible', should be consistent with the
>> remaining properties of pioA node.
>>> +                                    reg = <0xfffff200 0x100>;
>>> +                                    interrupts = <6 
>>> IRQ_TYPE_LEVEL_HIGH 1>;
>>> +                                    #gpio-cells = <2>;
>>> +                                    gpio-controller;
>>> +                                    interrupt-controller;
>>> +                                    #interrupt-cells = <2>;
>>> +                                    clocks = <&pioA_clk>;
>>> +                                    bootph-all;
>>> +                            };
>>>
>>> -                    pioB: gpio at fffff400 {
>>> -                            compatible = "atmel,at91sam9x5-gpio", 
>>> "atmel,at91rm9200-gpio";
>>> -                            reg = <0xfffff400 0x100>;
>>> -                            interrupts = <7 IRQ_TYPE_LEVEL_HIGH 1>;
>>> -                            #gpio-cells = <2>;
>>> -                            gpio-controller;
>>> -                            interrupt-controller;
>>> -                            #interrupt-cells = <2>;
>>> -                            clocks = <&pioB_clk>;
>>> -                            bootph-all;
>>> -                    };
>>> +                            pioB: gpio at fffff400 {
>>> +                                  compatible = 
>>> "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
>> Ditto
>>> +                                    reg = <0xfffff400 0x100>;
>>> +                                    interrupts = <7 
>>> IRQ_TYPE_LEVEL_HIGH 1>;
>>> +                                    #gpio-cells = <2>;
>>> +                                    gpio-controller;
>>> +                                    interrupt-controller;
>>> +                                    #interrupt-cells = <2>;
>>> +                                    clocks = <&pioB_clk>;
>>> +                                    bootph-all;
>>> +                            };
>>>
>>> -                    pioC: gpio at fffff600 {
>>> -                            compatible = "atmel,at91sam9x5-gpio", 
>>> "atmel,at91rm9200-gpio";
>>> -                            reg = <0xfffff600 0x100>;
>>> -                            interrupts = <8 IRQ_TYPE_LEVEL_HIGH 1>;
>>> -                            #gpio-cells = <2>;
>>> -                            gpio-controller;
>>> -                            interrupt-controller;
>>> -                            #interrupt-cells = <2>;
>>> -                            clocks = <&pioC_clk>;
>>> -                            bootph-all;
>>> -                    };
>>> +                            pioC: gpio at fffff600 {
>>> +                                  compatible = 
>>> "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
>> Ditto
>>> +                                    reg = <0xfffff600 0x100>;
>>> +                                    interrupts = <8 
>>> IRQ_TYPE_LEVEL_HIGH 1>;
>>> +                                    #gpio-cells = <2>;
>>> +                                    gpio-controller;
>>> +                                    interrupt-controller;
>>> +                                    #interrupt-cells = <2>;
>>> +                                    clocks = <&pioC_clk>;
>>> +                                    bootph-all;
>>> +                            };
>>>
>>> -                    pioD: gpio at fffff800 {
>>> -                            compatible = "atmel,at91sam9x5-gpio", 
>>> "atmel,at91rm9200-gpio";
>>> -                            reg = <0xfffff800 0x100>;
>>> -                            interrupts = <9 IRQ_TYPE_LEVEL_HIGH 1>;
>>> -                            #gpio-cells = <2>;
>>> -                            gpio-controller;
>>> -                            interrupt-controller;
>>> -                            #interrupt-cells = <2>;
>>> -                            clocks = <&pioD_clk>;
>>> -                            bootph-all;
>>> -                    };
>>> +                            pioD: gpio at fffff800 {
>>> +                                  compatible = 
>>> "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
>> Ditto
>>> +                                    reg = <0xfffff800 0x100>;
>>> +                                    interrupts = <9 
>>> IRQ_TYPE_LEVEL_HIGH 1>;
>>> +                                    #gpio-cells = <2>;
>>> +                                    gpio-controller;
>>> +                                    interrupt-controller;
>>> +                                    #interrupt-cells = <2>;
>>> +                                    clocks = <&pioD_clk>;
>>> +                                    bootph-all;
>>> +                            };
>>> +
>>> +                            pioE: gpio at fffffa00 {
>>> +                                  compatible = 
>>> "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
>> Ditto
>>> +                                    reg = <0xfffffa00 0x100>;
>>> +                                    interrupts = <10 
>>> IRQ_TYPE_LEVEL_HIGH 1>;
>>> +                                    #gpio-cells = <2>;
>>> +                                    gpio-controller;
>>> +                                    interrupt-controller;
>>> +                                    #interrupt-cells = <2>;
>>> +                                    clocks = <&pioE_clk>;
>>> +                                    bootph-all;
>>> +                            };
>>>
>> Extra line
>>> -                    pioE: gpio at fffffa00 {
>>> -                            compatible = "atmel,at91sam9x5-gpio", 
>>> "atmel,at91rm9200-gpio";
>>> -                            reg = <0xfffffa00 0x100>;
>>> -                            interrupts = <10 IRQ_TYPE_LEVEL_HIGH 1>;
>>> -                            #gpio-cells = <2>;
>>> -                            gpio-controller;
>>> -                            interrupt-controller;
>>> -                            #interrupt-cells = <2>;
>>> -                            clocks = <&pioE_clk>;
>>> -                            bootph-all;
>>>                      };
>>>
>>>                      pmc: pmc at fffffc00 {
>>
> 

-- 
Thanks and Regards,
Manikandan M.



More information about the U-Boot mailing list