[PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts
Alexey Tsirlin
alexey at all4bambi.com
Sun Oct 20 07:14:45 CEST 2024
Hi Manikandan,
I have tested gpio cmd option on SAMA5D3 EDS board (BTW it wasn't enabled by
default in sama5d3_xplained_mmc_defconfig which is used by this board) in
u-boot with the DT change as I proposed and it seems to work fine, at least
it detects all the 5 GPIO ports (A-E). pinmux cmd does not work too much,
but this is because pinctrl-at91 does not implement get_pins_count function.
Without the proposed change I was not able to make the Ethernet (EMAC)
detect the PHY because MDIO interface was not working - the correct
peripheral mode for the EMAC pins was not set as defined in DT.
Regards,
Alexey.
-----Original Message-----
From: Manikandan.M at microchip.com <Manikandan.M at microchip.com>
Sent: Friday, 18 October 2024 12:30
To: eugen.hristev at linaro.org; alexey at all4bambi.com; u-boot at lists.denx.de
Cc: Varshini.Rajendran at microchip.com; Hari.PrasathGE at microchip.com
Subject: Re: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside
pinctrl as in kernel dts
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