Hi Eugen,
On 18/10/24 12:45 pm, Eugen Hristev wrote:
> [You don't often get email from eugen.hris...@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, manikanda...@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 <ale...@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.
