* [PATCH 0/1] sama5d3.dtsi pinctrl section fix
@ 2024-10-15 14:53 Alexey Tsirlin
2024-10-15 14:53 ` [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts Alexey Tsirlin
0 siblings, 1 reply; 11+ messages in thread
From: Alexey Tsirlin @ 2024-10-15 14:53 UTC (permalink / raw)
To: u-boot; +Cc: eugen.hristev, Alexey Tsirlin
At present, there are no PIO sections inside pinctrl DTS section in constradiction to the same file used in kernel.
This problem causes the pinctrl driver in u-boot to wrongly detect number of pio ports to be zero which in
turn causes the driver to deny any PIO configuration presented in any section of the DTS, for example Ethernet will
not work because of wrong PIO configuration.
Alexey Tsirlin (1):
Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel
dts
arch/arm/dts/sama5d3.dtsi | 111 +++++++++++++++++++-------------------
1 file changed, 56 insertions(+), 55 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts
2024-10-15 14:53 [PATCH 0/1] sama5d3.dtsi pinctrl section fix Alexey Tsirlin
@ 2024-10-15 14:53 ` Alexey Tsirlin
2024-10-17 8:51 ` Manikandan.M
0 siblings, 1 reply; 11+ messages in thread
From: Alexey Tsirlin @ 2024-10-15 14:53 UTC (permalink / raw)
To: u-boot; +Cc: eugen.hristev, Alexey Tsirlin
This allows setting the GPIO parameters from device tree, otherwise the
at91_pin_check_config will fail because the priv->nbanks equal to zero
Signed-off-by: Alexey Tsirlin <alexey@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@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;
- #interrupt-cells = <2>;
- clocks = <&pioA_clk>;
- bootph-all;
- };
+ pioA: gpio@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;
+ #interrupt-cells = <2>;
+ clocks = <&pioA_clk>;
+ bootph-all;
+ };
- pioB: gpio@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@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;
+ };
- pioC: gpio@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@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;
+ };
- pioD: gpio@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@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;
+ };
+
+ pioE: gpio@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;
+ };
- pioE: gpio@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@fffffc00 {
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts
2024-10-15 14:53 ` [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts Alexey Tsirlin
@ 2024-10-17 8:51 ` Manikandan.M
2024-10-18 7:15 ` Eugen Hristev
0 siblings, 1 reply; 11+ messages in thread
From: Manikandan.M @ 2024-10-17 8:51 UTC (permalink / raw)
To: alexey, u-boot; +Cc: Varshini.Rajendran, Hari.PrasathGE
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
>
> Signed-off-by: Alexey Tsirlin <alexey@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@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@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@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@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@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@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@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@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@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@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@fffffc00 {
--
Thanks and Regards,
Manikandan M.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts
2024-10-17 8:51 ` Manikandan.M
@ 2024-10-18 7:15 ` Eugen Hristev
2024-10-18 7:51 ` Tsirlin Alexey
2024-10-18 9:29 ` Manikandan.M
0 siblings, 2 replies; 11+ messages in thread
From: Eugen Hristev @ 2024-10-18 7:15 UTC (permalink / raw)
To: Manikandan.M, alexey, u-boot; +Cc: Varshini.Rajendran, Hari.PrasathGE
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@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 ?
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@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@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@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@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@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@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@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@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@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@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@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@fffffc00 {
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts
2024-10-18 7:15 ` Eugen Hristev
@ 2024-10-18 7:51 ` Tsirlin Alexey
2024-10-18 9:29 ` Manikandan.M
1 sibling, 0 replies; 11+ messages in thread
From: Tsirlin Alexey @ 2024-10-18 7:51 UTC (permalink / raw)
To: Eugen Hristev; +Cc: Manikandan.M, u-boot, Varshini.Rajendran, Hari.PrasathGE
Hi Eugen,
I will fix the subject as required. Just wanted to tell that I didn’t test the gpio commands in uboot. The part that interested me was the pinctl and that it is unable to set the correct peripheral function from DTS. The function that interested me was Ethernet. I will also check the gpio functionality than I am back to office on Sunday.
Thanks!
Alexey.
> On 18 Oct 2024, at 10:15, Eugen Hristev <eugen.hristev@linaro.org> wrote:
>
> 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@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 ?
>
> 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@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@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@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@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@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@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@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@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@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@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@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@fffffc00 {
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts
2024-10-18 7:15 ` Eugen Hristev
2024-10-18 7:51 ` Tsirlin Alexey
@ 2024-10-18 9:29 ` Manikandan.M
2024-10-20 5:14 ` Alexey Tsirlin
1 sibling, 1 reply; 11+ messages in thread
From: Manikandan.M @ 2024-10-18 9:29 UTC (permalink / raw)
To: eugen.hristev, alexey, u-boot; +Cc: Varshini.Rajendran, Hari.PrasathGE
Hi Eugen,
On 18/10/24 12:45 pm, Eugen Hristev wrote:
> [You don't often get email from eugen.hristev@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@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@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@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@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@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@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@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@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@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@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@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@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@fffffc00 {
>>
>
--
Thanks and Regards,
Manikandan M.
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts
2024-10-18 9:29 ` Manikandan.M
@ 2024-10-20 5:14 ` Alexey Tsirlin
2024-10-21 10:19 ` Manikandan.M
0 siblings, 1 reply; 11+ messages in thread
From: Alexey Tsirlin @ 2024-10-20 5:14 UTC (permalink / raw)
To: Manikandan.M, eugen.hristev, u-boot; +Cc: Varshini.Rajendran, Hari.PrasathGE
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@microchip.com <Manikandan.M@microchip.com>
Sent: Friday, 18 October 2024 12:30
To: eugen.hristev@linaro.org; alexey@all4bambi.com; u-boot@lists.denx.de
Cc: Varshini.Rajendran@microchip.com; Hari.PrasathGE@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@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@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@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@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@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@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@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@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@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@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@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@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@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@fffffc00 {
>>
>
--
Thanks and Regards,
Manikandan M.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts
2024-10-20 5:14 ` Alexey Tsirlin
@ 2024-10-21 10:19 ` Manikandan.M
2024-10-22 5:44 ` Alexey Tsirlin
0 siblings, 1 reply; 11+ messages in thread
From: Manikandan.M @ 2024-10-21 10:19 UTC (permalink / raw)
To: alexey, eugen.hristev, u-boot; +Cc: Varshini.Rajendran, Hari.PrasathGE
Hi Alexey,
Can you confirm if you are using the u-boot-mchp repo from GitHub [1] to
test the sama5d3_eds board where the driver changes to align U-Boot and
Kernel DT are already present.
The pinctrl driver probe fails with the proposed change and will cause
regression on other boards that uses the same pinctrl PIO3 driver in the
mainline repo.
[1] --> https://github.com/linux4microchip/u-boot-mchp
On 20/10/24 10:44 am, Alexey Tsirlin wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> 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@microchip.com <Manikandan.M@microchip.com>
> Sent: Friday, 18 October 2024 12:30
> To: eugen.hristev@linaro.org; alexey@all4bambi.com; u-boot@lists.denx.de
> Cc: Varshini.Rajendran@microchip.com; Hari.PrasathGE@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@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@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@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@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@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@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@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@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@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@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@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@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@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@fffffc00 {
>>>
>>
>
> --
> Thanks and Regards,
> Manikandan M.
--
Thanks and Regards,
Manikandan M.
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts
2024-10-21 10:19 ` Manikandan.M
@ 2024-10-22 5:44 ` Alexey Tsirlin
2024-10-22 8:46 ` Manikandan.M
0 siblings, 1 reply; 11+ messages in thread
From: Alexey Tsirlin @ 2024-10-22 5:44 UTC (permalink / raw)
To: Manikandan.M, eugen.hristev, u-boot; +Cc: Varshini.Rajendran, Hari.PrasathGE
Hi Manikandan,
I am using buildroot 2024.04 sama5d3_eds_headless_defconfig config setup
which leads to the following:
1. uboot from https://github.com/linux4microchip/u-boot-mchp.git
2. uboot version linux4microchip-2024.10-rc2
3. uboot config sama5d3_xplained_mmc
My board is sama5d3 EDS with DP83630 PHY connected to EMAC.
I confirm that using configuration above, I was not able to make Ethernet
(EMAC) in uboot work because it was failing to configure EMAC pins to the
required peripheral mode. The at91_pin_check_config function in pinctrl-at91
returned -EINVAL because of nbanks=0. After applying the patch, the Ethernet
is working fine.
I also confirm that after applying the patch, I was able to toggle IO pin
with gpio command (tried PIOC18 which is accessible through IO Connector
#1), although I cannot tell you if this functionality also worked before
applying the patch.
Regards,
Alexey.
-----Original Message-----
From: Manikandan.M@microchip.com <Manikandan.M@microchip.com>
Sent: Monday, 21 October 2024 13:19
To: alexey@all4bambi.com; eugen.hristev@linaro.org; u-boot@lists.denx.de
Cc: Varshini.Rajendran@microchip.com; Hari.PrasathGE@microchip.com
Subject: Re: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside
pinctrl as in kernel dts
Hi Alexey,
Can you confirm if you are using the u-boot-mchp repo from GitHub [1] to
test the sama5d3_eds board where the driver changes to align U-Boot and
Kernel DT are already present.
The pinctrl driver probe fails with the proposed change and will cause
regression on other boards that uses the same pinctrl PIO3 driver in the
mainline repo.
[1] --> https://github.com/linux4microchip/u-boot-mchp
On 20/10/24 10:44 am, Alexey Tsirlin wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> the content is safe
>
> 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@microchip.com <Manikandan.M@microchip.com>
> Sent: Friday, 18 October 2024 12:30
> To: eugen.hristev@linaro.org; alexey@all4bambi.com;
> u-boot@lists.denx.de
> Cc: Varshini.Rajendran@microchip.com; Hari.PrasathGE@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@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@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@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@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@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@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@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@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@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@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@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@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@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@fffffc00 {
>>>
>>
>
> --
> Thanks and Regards,
> Manikandan M.
--
Thanks and Regards,
Manikandan M.
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts
2024-10-22 8:46 ` Manikandan.M
@ 2024-10-22 8:45 ` Alexey Tsirlin
0 siblings, 0 replies; 11+ messages in thread
From: Alexey Tsirlin @ 2024-10-22 8:45 UTC (permalink / raw)
To: Manikandan.M, eugen.hristev, u-boot; +Cc: Varshini.Rajendran, Hari.PrasathGE
No problem!
Thanks!
Alexey.
-----Original Message-----
From: Manikandan.M@microchip.com <Manikandan.M@microchip.com>
Sent: Tuesday, 22 October 2024 11:47
To: alexey@all4bambi.com; eugen.hristev@linaro.org; u-boot@lists.denx.de
Cc: Varshini.Rajendran@microchip.com; Hari.PrasathGE@microchip.com
Subject: Re: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside
pinctrl as in kernel dts
Hi Alexey,
Thank you for confirming the u-boot tree used for testing your patch.
This suggested modification is a fix-up patch for the pinctrl driver changes
present in the uboot-mcho git tree. However, because those changes are not
yet present in the mainline U-Boot, this patch will cause regression on all
sama5d3 SoC boards.
I appreciate your work in finding and resolving the issue for sama5d3.Please
hold on to this patch; you can share a version 2 that resolves the comments
once the driver changes are in.
Also, fyi, this change is already present in the u-boot-mchp tree
linux4microchip-2024.10-rc2 version,
https://github.com/linux4microchip/u-boot-mchp/commit/a25b31c2558f9fe303a3b97dd660b87476446301
On 22/10/24 11:14 am, Alexey Tsirlin wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> the content is safe
>
> Hi Manikandan,
>
> I am using buildroot 2024.04 sama5d3_eds_headless_defconfig config
> setup which leads to the following:
> 1. uboot from https://github.com/linux4microchip/u-boot-mchp.git
> 2. uboot version linux4microchip-2024.10-rc2 3. uboot config
> sama5d3_xplained_mmc
>
> My board is sama5d3 EDS with DP83630 PHY connected to EMAC.
>
> I confirm that using configuration above, I was not able to make
> Ethernet
> (EMAC) in uboot work because it was failing to configure EMAC pins to
> the required peripheral mode. The at91_pin_check_config function in
> pinctrl-at91 returned -EINVAL because of nbanks=0. After applying the
> patch, the Ethernet is working fine.
>
> I also confirm that after applying the patch, I was able to toggle IO
> pin with gpio command (tried PIOC18 which is accessible through IO
> Connector #1), although I cannot tell you if this functionality also
> worked before applying the patch.
>
> Regards,
> Alexey.
>
> -----Original Message-----
> From: Manikandan.M@microchip.com <Manikandan.M@microchip.com>
> Sent: Monday, 21 October 2024 13:19
> To: alexey@all4bambi.com; eugen.hristev@linaro.org;
> u-boot@lists.denx.de
> Cc: Varshini.Rajendran@microchip.com; Hari.PrasathGE@microchip.com
> Subject: Re: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are
> inside pinctrl as in kernel dts
>
> Hi Alexey,
>
> Can you confirm if you are using the u-boot-mchp repo from GitHub [1]
> to test the sama5d3_eds board where the driver changes to align U-Boot
> and Kernel DT are already present.
> The pinctrl driver probe fails with the proposed change and will cause
> regression on other boards that uses the same pinctrl PIO3 driver in
> the mainline repo.
>
> [1] --> https://github.com/linux4microchip/u-boot-mchp
>
> On 20/10/24 10:44 am, Alexey Tsirlin wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>> know the content is safe
>>
>> 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@microchip.com <Manikandan.M@microchip.com>
>> Sent: Friday, 18 October 2024 12:30
>> To: eugen.hristev@linaro.org; alexey@all4bambi.com;
>> u-boot@lists.denx.de
>> Cc: Varshini.Rajendran@microchip.com; Hari.PrasathGE@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@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@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@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@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@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@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@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@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@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@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@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@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@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@fffffc00 {
>>>>
>>>
>>
>> --
>> Thanks and Regards,
>> Manikandan M.
>
> --
> Thanks and Regards,
> Manikandan M.
--
Thanks and Regards,
Manikandan M.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts
2024-10-22 5:44 ` Alexey Tsirlin
@ 2024-10-22 8:46 ` Manikandan.M
2024-10-22 8:45 ` Alexey Tsirlin
0 siblings, 1 reply; 11+ messages in thread
From: Manikandan.M @ 2024-10-22 8:46 UTC (permalink / raw)
To: alexey, eugen.hristev, u-boot; +Cc: Varshini.Rajendran, Hari.PrasathGE
Hi Alexey,
Thank you for confirming the u-boot tree used for testing your patch.
This suggested modification is a fix-up patch for the pinctrl driver
changes present in the uboot-mcho git tree. However, because those
changes are not yet present in the mainline U-Boot, this patch will
cause regression on all sama5d3 SoC boards.
I appreciate your work in finding and resolving the issue for
sama5d3.Please hold on to this patch; you can share a version 2 that
resolves the comments once the driver changes are in.
Also, fyi, this change is already present in the u-boot-mchp tree
linux4microchip-2024.10-rc2 version,
https://github.com/linux4microchip/u-boot-mchp/commit/a25b31c2558f9fe303a3b97dd660b87476446301
On 22/10/24 11:14 am, Alexey Tsirlin wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi Manikandan,
>
> I am using buildroot 2024.04 sama5d3_eds_headless_defconfig config setup
> which leads to the following:
> 1. uboot from https://github.com/linux4microchip/u-boot-mchp.git
> 2. uboot version linux4microchip-2024.10-rc2
> 3. uboot config sama5d3_xplained_mmc
>
> My board is sama5d3 EDS with DP83630 PHY connected to EMAC.
>
> I confirm that using configuration above, I was not able to make Ethernet
> (EMAC) in uboot work because it was failing to configure EMAC pins to the
> required peripheral mode. The at91_pin_check_config function in pinctrl-at91
> returned -EINVAL because of nbanks=0. After applying the patch, the Ethernet
> is working fine.
>
> I also confirm that after applying the patch, I was able to toggle IO pin
> with gpio command (tried PIOC18 which is accessible through IO Connector
> #1), although I cannot tell you if this functionality also worked before
> applying the patch.
>
> Regards,
> Alexey.
>
> -----Original Message-----
> From: Manikandan.M@microchip.com <Manikandan.M@microchip.com>
> Sent: Monday, 21 October 2024 13:19
> To: alexey@all4bambi.com; eugen.hristev@linaro.org; u-boot@lists.denx.de
> Cc: Varshini.Rajendran@microchip.com; Hari.PrasathGE@microchip.com
> Subject: Re: [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside
> pinctrl as in kernel dts
>
> Hi Alexey,
>
> Can you confirm if you are using the u-boot-mchp repo from GitHub [1] to
> test the sama5d3_eds board where the driver changes to align U-Boot and
> Kernel DT are already present.
> The pinctrl driver probe fails with the proposed change and will cause
> regression on other boards that uses the same pinctrl PIO3 driver in the
> mainline repo.
>
> [1] --> https://github.com/linux4microchip/u-boot-mchp
>
> On 20/10/24 10:44 am, Alexey Tsirlin wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> the content is safe
>>
>> 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@microchip.com <Manikandan.M@microchip.com>
>> Sent: Friday, 18 October 2024 12:30
>> To: eugen.hristev@linaro.org; alexey@all4bambi.com;
>> u-boot@lists.denx.de
>> Cc: Varshini.Rajendran@microchip.com; Hari.PrasathGE@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@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@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@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@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@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@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@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@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@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@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@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@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@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@fffffc00 {
>>>>
>>>
>>
>> --
>> Thanks and Regards,
>> Manikandan M.
>
> --
> Thanks and Regards,
> Manikandan M.
--
Thanks and Regards,
Manikandan M.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-10-22 13:18 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-15 14:53 [PATCH 0/1] sama5d3.dtsi pinctrl section fix Alexey Tsirlin
2024-10-15 14:53 ` [PATCH 1/1] Fixed sama5d3 dts file so PIO sections are inside pinctrl as in kernel dts Alexey Tsirlin
2024-10-17 8:51 ` Manikandan.M
2024-10-18 7:15 ` Eugen Hristev
2024-10-18 7:51 ` Tsirlin Alexey
2024-10-18 9:29 ` Manikandan.M
2024-10-20 5:14 ` Alexey Tsirlin
2024-10-21 10:19 ` Manikandan.M
2024-10-22 5:44 ` Alexey Tsirlin
2024-10-22 8:46 ` Manikandan.M
2024-10-22 8:45 ` Alexey Tsirlin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox