2022-03-03 17:33:02

by Michael Walle

[permalink] [raw]
Subject: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes

Add all the usart nodes for the flexcom block. There was already
an usart node for the flexcom3 block. But it missed the DMA
channels. Although the DMA channels are specified, DMA is not
enabled by default because break detection doesn't work with DMA.

Keep the nodes disabled by default.

Signed-off-by: Michael Walle <[email protected]>
---
arch/arm/boot/dts/lan966x.dtsi | 55 ++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)

diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
index a7d46a2ca058..bea69b6d2749 100644
--- a/arch/arm/boot/dts/lan966x.dtsi
+++ b/arch/arm/boot/dts/lan966x.dtsi
@@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
#size-cells = <1>;
ranges = <0x0 0xe0040000 0x800>;
status = "disabled";
+
+ usart0: serial@200 {
+ compatible = "atmel,at91sam9260-usart";
+ reg = <0x200 0x200>;
+ interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
+ dmas = <&dma0 AT91_XDMAC_DT_PERID(3)>,
+ <&dma0 AT91_XDMAC_DT_PERID(2)>;
+ dma-names = "tx", "rx";
+ clocks = <&nic_clk>;
+ clock-names = "usart";
+ atmel,fifo-size = <32>;
+ status = "disabled";
+ };
};

flx1: flexcom@e0044000 {
@@ -102,6 +115,19 @@ flx1: flexcom@e0044000 {
#size-cells = <1>;
ranges = <0x0 0xe0044000 0x800>;
status = "disabled";
+
+ usart1: serial@200 {
+ compatible = "atmel,at91sam9260-usart";
+ reg = <0x200 0x200>;
+ interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+ dmas = <&dma0 AT91_XDMAC_DT_PERID(5)>,
+ <&dma0 AT91_XDMAC_DT_PERID(4)>;
+ dma-names = "tx", "rx";
+ clocks = <&nic_clk>;
+ clock-names = "usart";
+ atmel,fifo-size = <32>;
+ status = "disabled";
+ };
};

trng: rng@e0048000 {
@@ -129,6 +155,19 @@ flx2: flexcom@e0060000 {
#size-cells = <1>;
ranges = <0x0 0xe0060000 0x800>;
status = "disabled";
+
+ usart2: serial@200 {
+ compatible = "atmel,at91sam9260-usart";
+ reg = <0x200 0x200>;
+ interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
+ dmas = <&dma0 AT91_XDMAC_DT_PERID(7)>,
+ <&dma0 AT91_XDMAC_DT_PERID(6)>;
+ dma-names = "tx", "rx";
+ clocks = <&nic_clk>;
+ clock-names = "usart";
+ atmel,fifo-size = <32>;
+ status = "disabled";
+ };
};

flx3: flexcom@e0064000 {
@@ -144,6 +183,9 @@ usart3: serial@200 {
compatible = "atmel,at91sam9260-usart";
reg = <0x200 0x200>;
interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
+ dmas = <&dma0 AT91_XDMAC_DT_PERID(9)>,
+ <&dma0 AT91_XDMAC_DT_PERID(8)>;
+ dma-names = "tx", "rx";
clocks = <&nic_clk>;
clock-names = "usart";
atmel,fifo-size = <32>;
@@ -178,6 +220,19 @@ flx4: flexcom@e0070000 {
#size-cells = <1>;
ranges = <0x0 0xe0070000 0x800>;
status = "disabled";
+
+ usart4: serial@200 {
+ compatible = "atmel,at91sam9260-usart";
+ reg = <0x200 0x200>;
+ interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+ dmas = <&dma0 AT91_XDMAC_DT_PERID(11)>,
+ <&dma0 AT91_XDMAC_DT_PERID(10)>;
+ dma-names = "tx", "rx";
+ clocks = <&nic_clk>;
+ clock-names = "usart";
+ atmel,fifo-size = <32>;
+ status = "disabled";
+ };
};

timer0: timer@e008c000 {
--
2.30.2


2022-03-04 11:30:44

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes

Hi,

thanks for the quick review.

Am 2022-03-04 09:30, schrieb [email protected]:
> On 03.03.2022 18:03, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> the content is safe
>>
>> Add all the usart nodes for the flexcom block. There was already
>> an usart node for the flexcom3 block. But it missed the DMA
>> channels.
>
> And it would be good to go though a different patch.

sure

>> Although the DMA channels are specified, DMA is not
>> enabled by default because break detection doesn't work with DMA.
>>
>> Keep the nodes disabled by default.
>>
>> Signed-off-by: Michael Walle <[email protected]>
>> ---
>> arch/arm/boot/dts/lan966x.dtsi | 55
>> ++++++++++++++++++++++++++++++++++
>> 1 file changed, 55 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/lan966x.dtsi
>> b/arch/arm/boot/dts/lan966x.dtsi
>> index a7d46a2ca058..bea69b6d2749 100644
>> --- a/arch/arm/boot/dts/lan966x.dtsi
>> +++ b/arch/arm/boot/dts/lan966x.dtsi
>> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>> #size-cells = <1>;
>> ranges = <0x0 0xe0040000 0x800>;
>> status = "disabled";
>> +
>> + usart0: serial@200 {
>> + compatible =
>> "atmel,at91sam9260-usart";
>
> Are the usart blocks in lan966x 1:1 compatible with what is is sam9260?
> In
> case not it may worth to have a new compatible here, for lan966x, such
> that
> when new features will be implemented in usart driver for lan966x the
> old
> DT (this one) will work with the new kernel implementation.

During my review of the inital dtsi patch, I've asked the same question
[1]
and I was told they are the same.

At least this exact usart compatible is already in this file. I was
under
the impression, that was the least controversial compatible :)

But you'll need to tell me if they are the same or not, I don't have
any clue what microchip has reused. The only thing I can add is that
there is a version register within the IP blocks which is already used
in some drivers.

> Same for the rest of the nodes added in this series.
>
>> + reg = <0x200 0x200>;
>> + interrupts = <GIC_SPI 48
>> IRQ_TYPE_LEVEL_HIGH>;
>> + dmas = <&dma0 AT91_XDMAC_DT_PERID(3)>,
>> + <&dma0
>> AT91_XDMAC_DT_PERID(2)>;
>
> Keep dma entries aligned.

Oh shoot. How embarrassing they are alligned when the tab width is 4 *g*
Thanks.

-michael

[1]
https://lore.kernel.org/lkml/[email protected]/

>> + dma-names = "tx", "rx";
>> + clocks = <&nic_clk>;
>> + clock-names = "usart";
>> + atmel,fifo-size = <32>;
>> + status = "disabled";
>> + };
>> };
>>
>> flx1: flexcom@e0044000 {
>> @@ -102,6 +115,19 @@ flx1: flexcom@e0044000 {
>> #size-cells = <1>;
>> ranges = <0x0 0xe0044000 0x800>;
>> status = "disabled";
>> +
>> + usart1: serial@200 {
>> + compatible =
>> "atmel,at91sam9260-usart";
>> + reg = <0x200 0x200>;
>> + interrupts = <GIC_SPI 49
>> IRQ_TYPE_LEVEL_HIGH>;
>> + dmas = <&dma0 AT91_XDMAC_DT_PERID(5)>,
>> + <&dma0
>> AT91_XDMAC_DT_PERID(4)>;
>> + dma-names = "tx", "rx";
>> + clocks = <&nic_clk>;
>> + clock-names = "usart";
>> + atmel,fifo-size = <32>;
>> + status = "disabled";
>> + };
>> };
>>
>> trng: rng@e0048000 {
>> @@ -129,6 +155,19 @@ flx2: flexcom@e0060000 {
>> #size-cells = <1>;
>> ranges = <0x0 0xe0060000 0x800>;
>> status = "disabled";
>> +
>> + usart2: serial@200 {
>> + compatible =
>> "atmel,at91sam9260-usart";
>> + reg = <0x200 0x200>;
>> + interrupts = <GIC_SPI 50
>> IRQ_TYPE_LEVEL_HIGH>;
>> + dmas = <&dma0 AT91_XDMAC_DT_PERID(7)>,
>> + <&dma0
>> AT91_XDMAC_DT_PERID(6)>;
>> + dma-names = "tx", "rx";
>> + clocks = <&nic_clk>;
>> + clock-names = "usart";
>> + atmel,fifo-size = <32>;
>> + status = "disabled";
>> + };
>> };
>>
>> flx3: flexcom@e0064000 {
>> @@ -144,6 +183,9 @@ usart3: serial@200 {
>> compatible =
>> "atmel,at91sam9260-usart";
>> reg = <0x200 0x200>;
>> interrupts = <GIC_SPI 51
>> IRQ_TYPE_LEVEL_HIGH>;
>> + dmas = <&dma0 AT91_XDMAC_DT_PERID(9)>,
>> + <&dma0
>> AT91_XDMAC_DT_PERID(8)>;
>> + dma-names = "tx", "rx";
>> clocks = <&nic_clk>;
>> clock-names = "usart";
>> atmel,fifo-size = <32>;
>> @@ -178,6 +220,19 @@ flx4: flexcom@e0070000 {
>> #size-cells = <1>;
>> ranges = <0x0 0xe0070000 0x800>;
>> status = "disabled";
>> +
>> + usart4: serial@200 {
>> + compatible =
>> "atmel,at91sam9260-usart";
>> + reg = <0x200 0x200>;
>> + interrupts = <GIC_SPI 52
>> IRQ_TYPE_LEVEL_HIGH>;
>> + dmas = <&dma0
>> AT91_XDMAC_DT_PERID(11)>,
>> + <&dma0
>> AT91_XDMAC_DT_PERID(10)>;
>> + dma-names = "tx", "rx";
>> + clocks = <&nic_clk>;
>> + clock-names = "usart";
>> + atmel,fifo-size = <32>;
>> + status = "disabled";
>> + };
>> };
>>
>> timer0: timer@e008c000 {
>> --
>> 2.30.2
>>

--
-michael

2022-03-04 18:12:40

by Claudiu Beznea

[permalink] [raw]
Subject: Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes

On 03.03.2022 18:03, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Add all the usart nodes for the flexcom block. There was already
> an usart node for the flexcom3 block. But it missed the DMA
> channels.

And it would be good to go though a different patch.

> Although the DMA channels are specified, DMA is not
> enabled by default because break detection doesn't work with DMA.
>
> Keep the nodes disabled by default.
>
> Signed-off-by: Michael Walle <[email protected]>
> ---
> arch/arm/boot/dts/lan966x.dtsi | 55 ++++++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
> index a7d46a2ca058..bea69b6d2749 100644
> --- a/arch/arm/boot/dts/lan966x.dtsi
> +++ b/arch/arm/boot/dts/lan966x.dtsi
> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
> #size-cells = <1>;
> ranges = <0x0 0xe0040000 0x800>;
> status = "disabled";
> +
> + usart0: serial@200 {
> + compatible = "atmel,at91sam9260-usart";

Are the usart blocks in lan966x 1:1 compatible with what is is sam9260? In
case not it may worth to have a new compatible here, for lan966x, such that
when new features will be implemented in usart driver for lan966x the old
DT (this one) will work with the new kernel implementation. Same for the
rest of the nodes added in this series.

> + reg = <0x200 0x200>;
> + interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
> + dmas = <&dma0 AT91_XDMAC_DT_PERID(3)>,
> + <&dma0 AT91_XDMAC_DT_PERID(2)>;

Keep dma entries aligned.

> + dma-names = "tx", "rx";
> + clocks = <&nic_clk>;
> + clock-names = "usart";
> + atmel,fifo-size = <32>;
> + status = "disabled";
> + };
> };
>
> flx1: flexcom@e0044000 {
> @@ -102,6 +115,19 @@ flx1: flexcom@e0044000 {
> #size-cells = <1>;
> ranges = <0x0 0xe0044000 0x800>;
> status = "disabled";
> +
> + usart1: serial@200 {
> + compatible = "atmel,at91sam9260-usart";
> + reg = <0x200 0x200>;
> + interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> + dmas = <&dma0 AT91_XDMAC_DT_PERID(5)>,
> + <&dma0 AT91_XDMAC_DT_PERID(4)>;
> + dma-names = "tx", "rx";
> + clocks = <&nic_clk>;
> + clock-names = "usart";
> + atmel,fifo-size = <32>;
> + status = "disabled";
> + };
> };
>
> trng: rng@e0048000 {
> @@ -129,6 +155,19 @@ flx2: flexcom@e0060000 {
> #size-cells = <1>;
> ranges = <0x0 0xe0060000 0x800>;
> status = "disabled";
> +
> + usart2: serial@200 {
> + compatible = "atmel,at91sam9260-usart";
> + reg = <0x200 0x200>;
> + interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
> + dmas = <&dma0 AT91_XDMAC_DT_PERID(7)>,
> + <&dma0 AT91_XDMAC_DT_PERID(6)>;
> + dma-names = "tx", "rx";
> + clocks = <&nic_clk>;
> + clock-names = "usart";
> + atmel,fifo-size = <32>;
> + status = "disabled";
> + };
> };
>
> flx3: flexcom@e0064000 {
> @@ -144,6 +183,9 @@ usart3: serial@200 {
> compatible = "atmel,at91sam9260-usart";
> reg = <0x200 0x200>;
> interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
> + dmas = <&dma0 AT91_XDMAC_DT_PERID(9)>,
> + <&dma0 AT91_XDMAC_DT_PERID(8)>;
> + dma-names = "tx", "rx";
> clocks = <&nic_clk>;
> clock-names = "usart";
> atmel,fifo-size = <32>;
> @@ -178,6 +220,19 @@ flx4: flexcom@e0070000 {
> #size-cells = <1>;
> ranges = <0x0 0xe0070000 0x800>;
> status = "disabled";
> +
> + usart4: serial@200 {
> + compatible = "atmel,at91sam9260-usart";
> + reg = <0x200 0x200>;
> + interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> + dmas = <&dma0 AT91_XDMAC_DT_PERID(11)>,
> + <&dma0 AT91_XDMAC_DT_PERID(10)>;
> + dma-names = "tx", "rx";
> + clocks = <&nic_clk>;
> + clock-names = "usart";
> + atmel,fifo-size = <32>;
> + status = "disabled";
> + };
> };
>
> timer0: timer@e008c000 {
> --
> 2.30.2
>

2022-03-07 13:21:51

by Claudiu Beznea

[permalink] [raw]
Subject: Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes

On 04.03.2022 13:01, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> Hi,
>
> thanks for the quick review.
>
> Am 2022-03-04 09:30, schrieb [email protected]:
>> On 03.03.2022 18:03, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> Add all the usart nodes for the flexcom block. There was already
>>> an usart node for the flexcom3 block. But it missed the DMA
>>> channels.
>>
>> And it would be good to go though a different patch.
>
> sure
>
>>> Although the DMA channels are specified, DMA is not
>>> enabled by default because break detection doesn't work with DMA.
>>>
>>> Keep the nodes disabled by default.
>>>
>>> Signed-off-by: Michael Walle <[email protected]>
>>> ---
>>>  arch/arm/boot/dts/lan966x.dtsi | 55
>>> ++++++++++++++++++++++++++++++++++
>>>  1 file changed, 55 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/lan966x.dtsi
>>> b/arch/arm/boot/dts/lan966x.dtsi
>>> index a7d46a2ca058..bea69b6d2749 100644
>>> --- a/arch/arm/boot/dts/lan966x.dtsi
>>> +++ b/arch/arm/boot/dts/lan966x.dtsi
>>> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>>>                         #size-cells = <1>;
>>>                         ranges = <0x0 0xe0040000 0x800>;
>>>                         status = "disabled";
>>> +
>>> +                       usart0: serial@200 {
>>> +                               compatible =
>>> "atmel,at91sam9260-usart";
>>
>> Are the usart blocks in lan966x 1:1 compatible with what is is sam9260?
>> In
>> case not it may worth to have a new compatible here, for lan966x, such
>> that
>> when new features will be implemented in usart driver for lan966x the
>> old
>> DT (this one) will work with the new kernel implementation.
>
> During my review of the inital dtsi patch, I've asked the same question
> [1]
> and I was told they are the same.
>
> At least this exact usart compatible is already in this file. I was
> under
> the impression, that was the least controversial compatible :)

OK.

>
> But you'll need to tell me if they are the same or not, I don't have
> any clue what microchip has reused.

From software point of view comparing registers should be good, as far as I
can tell. All AT91 datasheet should be available. I though you have checked
one against LAN966. At the moment I don't have a DS for LAN966. I'll find
one and have a look.

> The only thing I can add is that
> there is a version register within the IP blocks which is already used
> in some drivers.
>
>> Same for the rest of the nodes added in this series.
>>
>>> +                               reg = <0x200 0x200>;
>>> +                               interrupts = <GIC_SPI 48
>>> IRQ_TYPE_LEVEL_HIGH>;
>>> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(3)>,
>>> +                                          <&dma0
>>> AT91_XDMAC_DT_PERID(2)>;
>>
>> Keep dma entries aligned.
>
> Oh shoot. How embarrassing they are alligned when the tab width is 4 *g*
> Thanks.
>
> -michael
>
> [1]
> https://lore.kernel.org/lkml/[email protected]/
>
>>> +                               dma-names = "tx", "rx";
>>> +                               clocks = <&nic_clk>;
>>> +                               clock-names = "usart";
>>> +                               atmel,fifo-size = <32>;
>>> +                               status = "disabled";
>>> +                       };
>>>                 };
>>>
>>>                 flx1: flexcom@e0044000 {
>>> @@ -102,6 +115,19 @@ flx1: flexcom@e0044000 {
>>>                         #size-cells = <1>;
>>>                         ranges = <0x0 0xe0044000 0x800>;
>>>                         status = "disabled";
>>> +
>>> +                       usart1: serial@200 {
>>> +                               compatible =
>>> "atmel,at91sam9260-usart";
>>> +                               reg = <0x200 0x200>;
>>> +                               interrupts = <GIC_SPI 49
>>> IRQ_TYPE_LEVEL_HIGH>;
>>> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(5)>,
>>> +                                          <&dma0
>>> AT91_XDMAC_DT_PERID(4)>;
>>> +                               dma-names = "tx", "rx";
>>> +                               clocks = <&nic_clk>;
>>> +                               clock-names = "usart";
>>> +                               atmel,fifo-size = <32>;
>>> +                               status = "disabled";
>>> +                       };
>>>                 };
>>>
>>>                 trng: rng@e0048000 {
>>> @@ -129,6 +155,19 @@ flx2: flexcom@e0060000 {
>>>                         #size-cells = <1>;
>>>                         ranges = <0x0 0xe0060000 0x800>;
>>>                         status = "disabled";
>>> +
>>> +                       usart2: serial@200 {
>>> +                               compatible =
>>> "atmel,at91sam9260-usart";
>>> +                               reg = <0x200 0x200>;
>>> +                               interrupts = <GIC_SPI 50
>>> IRQ_TYPE_LEVEL_HIGH>;
>>> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(7)>,
>>> +                                          <&dma0
>>> AT91_XDMAC_DT_PERID(6)>;
>>> +                               dma-names = "tx", "rx";
>>> +                               clocks = <&nic_clk>;
>>> +                               clock-names = "usart";
>>> +                               atmel,fifo-size = <32>;
>>> +                               status = "disabled";
>>> +                       };
>>>                 };
>>>
>>>                 flx3: flexcom@e0064000 {
>>> @@ -144,6 +183,9 @@ usart3: serial@200 {
>>>                                 compatible =
>>> "atmel,at91sam9260-usart";
>>>                                 reg = <0x200 0x200>;
>>>                                 interrupts = <GIC_SPI 51
>>> IRQ_TYPE_LEVEL_HIGH>;
>>> +                               dmas = <&dma0 AT91_XDMAC_DT_PERID(9)>,
>>> +                                          <&dma0
>>> AT91_XDMAC_DT_PERID(8)>;
>>> +                               dma-names = "tx", "rx";
>>>                                 clocks = <&nic_clk>;
>>>                                 clock-names = "usart";
>>>                                 atmel,fifo-size = <32>;
>>> @@ -178,6 +220,19 @@ flx4: flexcom@e0070000 {
>>>                         #size-cells = <1>;
>>>                         ranges = <0x0 0xe0070000 0x800>;
>>>                         status = "disabled";
>>> +
>>> +                       usart4: serial@200 {
>>> +                               compatible =
>>> "atmel,at91sam9260-usart";
>>> +                               reg = <0x200 0x200>;
>>> +                               interrupts = <GIC_SPI 52
>>> IRQ_TYPE_LEVEL_HIGH>;
>>> +                               dmas = <&dma0
>>> AT91_XDMAC_DT_PERID(11)>,
>>> +                                          <&dma0
>>> AT91_XDMAC_DT_PERID(10)>;
>>> +                               dma-names = "tx", "rx";
>>> +                               clocks = <&nic_clk>;
>>> +                               clock-names = "usart";
>>> +                               atmel,fifo-size = <32>;
>>> +                               status = "disabled";
>>> +                       };
>>>                 };
>>>
>>>                 timer0: timer@e008c000 {
>>> --
>>> 2.30.2
>>>
>
> --
> -michael

2022-03-07 18:57:59

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes

Am 2022-03-07 12:53, schrieb [email protected]:
> On 04.03.2022 13:01, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> the
>> content is safe
>>
>> Hi,
>>
>> thanks for the quick review.
>>
>> Am 2022-03-04 09:30, schrieb [email protected]:
>>> On 03.03.2022 18:03, Michael Walle wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>> know
>>>> the content is safe
>>>>
>>>> Add all the usart nodes for the flexcom block. There was already
>>>> an usart node for the flexcom3 block. But it missed the DMA
>>>> channels.
>>>
>>> And it would be good to go though a different patch.
>>
>> sure
>>
>>>> Although the DMA channels are specified, DMA is not
>>>> enabled by default because break detection doesn't work with DMA.
>>>>
>>>> Keep the nodes disabled by default.
>>>>
>>>> Signed-off-by: Michael Walle <[email protected]>
>>>> ---
>>>>  arch/arm/boot/dts/lan966x.dtsi | 55
>>>> ++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 55 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/lan966x.dtsi
>>>> b/arch/arm/boot/dts/lan966x.dtsi
>>>> index a7d46a2ca058..bea69b6d2749 100644
>>>> --- a/arch/arm/boot/dts/lan966x.dtsi
>>>> +++ b/arch/arm/boot/dts/lan966x.dtsi
>>>> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>>>>                         #size-cells = <1>;
>>>>                         ranges = <0x0 0xe0040000 0x800>;
>>>>                         status = "disabled";
>>>> +
>>>> +                       usart0: serial@200 {
>>>> +                               compatible =
>>>> "atmel,at91sam9260-usart";
>>>
>>> Are the usart blocks in lan966x 1:1 compatible with what is is
>>> sam9260?
>>> In
>>> case not it may worth to have a new compatible here, for lan966x,
>>> such
>>> that
>>> when new features will be implemented in usart driver for lan966x the
>>> old
>>> DT (this one) will work with the new kernel implementation.
>>
>> During my review of the inital dtsi patch, I've asked the same
>> question
>> [1]
>> and I was told they are the same.
>>
>> At least this exact usart compatible is already in this file. I was
>> under
>> the impression, that was the least controversial compatible :)
>
> OK.
>
>>
>> But you'll need to tell me if they are the same or not, I don't have
>> any clue what microchip has reused.
>
> From software point of view comparing registers should be good, as far
> as I
> can tell. All AT91 datasheet should be available. I though you have
> checked
> one against LAN966. At the moment I don't have a DS for LAN966. I'll
> find
> one and have a look.

So my train of thought was like: even if the registers are the same I
cannot be sure that it is the exact same IP and will behave the same.
Therefore, it is something only microchip can answer.

You can find the registers at
https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html

I'm not aware of any "classic" datasheet.

-michael

2022-03-18 15:45:49

by Claudiu Beznea

[permalink] [raw]
Subject: Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes

On 07.03.2022 14:04, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> Am 2022-03-07 12:53, schrieb [email protected]:
>> On 04.03.2022 13:01, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the
>>> content is safe
>>>
>>> Hi,
>>>
>>> thanks for the quick review.
>>>
>>> Am 2022-03-04 09:30, schrieb [email protected]:
>>>> On 03.03.2022 18:03, Michael Walle wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>> know
>>>>> the content is safe
>>>>>
>>>>> Add all the usart nodes for the flexcom block. There was already
>>>>> an usart node for the flexcom3 block. But it missed the DMA
>>>>> channels.
>>>>
>>>> And it would be good to go though a different patch.
>>>
>>> sure
>>>
>>>>> Although the DMA channels are specified, DMA is not
>>>>> enabled by default because break detection doesn't work with DMA.
>>>>>
>>>>> Keep the nodes disabled by default.
>>>>>
>>>>> Signed-off-by: Michael Walle <[email protected]>
>>>>> ---
>>>>>  arch/arm/boot/dts/lan966x.dtsi | 55
>>>>> ++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 55 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/lan966x.dtsi
>>>>> b/arch/arm/boot/dts/lan966x.dtsi
>>>>> index a7d46a2ca058..bea69b6d2749 100644
>>>>> --- a/arch/arm/boot/dts/lan966x.dtsi
>>>>> +++ b/arch/arm/boot/dts/lan966x.dtsi
>>>>> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>>>>>                         #size-cells = <1>;
>>>>>                         ranges = <0x0 0xe0040000 0x800>;
>>>>>                         status = "disabled";
>>>>> +
>>>>> +                       usart0: serial@200 {
>>>>> +                               compatible =
>>>>> "atmel,at91sam9260-usart";
>>>>
>>>> Are the usart blocks in lan966x 1:1 compatible with what is is
>>>> sam9260?
>>>> In
>>>> case not it may worth to have a new compatible here, for lan966x,
>>>> such
>>>> that
>>>> when new features will be implemented in usart driver for lan966x the
>>>> old
>>>> DT (this one) will work with the new kernel implementation.
>>>
>>> During my review of the inital dtsi patch, I've asked the same
>>> question
>>> [1]
>>> and I was told they are the same.
>>>
>>> At least this exact usart compatible is already in this file. I was
>>> under
>>> the impression, that was the least controversial compatible :)
>>
>> OK.
>>
>>>
>>> But you'll need to tell me if they are the same or not, I don't have
>>> any clue what microchip has reused.
>>
>> From software point of view comparing registers should be good, as far
>> as I
>> can tell. All AT91 datasheet should be available. I though you have
>> checked
>> one against LAN966. At the moment I don't have a DS for LAN966. I'll
>> find
>> one and have a look.
>
> So my train of thought was like: even if the registers are the same I
> cannot be sure that it is the exact same IP and will behave the same.
> Therefore, it is something only microchip can answer.
>
> You can find the registers at
> https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
>
> I'm not aware of any "classic" datasheet.

You can find all AT91 datasheet on Microchip web site [1].

Simple register comparison b/w register mapping at [2] and SAMA5D2
datasheet [3] (which uses the same compatible), SAM9X60 datasheet [3] and
SAMA7G5 datasheet (not public at the moment) brings up a difference at
register FLEX_US_CR (bits 16, 17) which are not available on SAMA5D2,
SAM9X60 or SAMA7G5. Unless this is a mistake on documentation at [2] I say
it needs a new compatible.

Kavya, could you confirm this?

Thank you,
Claudiu Beznea

[1] https://www.microchip.com/
[2] https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
[3] http://ww1.microchip.com/downloads/en/devicedoc/ds60001476b.pdf#G22.2193277
[4]
https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/DataSheets/SAM9X60-Data-Sheet-DS60001579E.pdf

>
> -michael

2022-03-23 03:16:19

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes

Am 2022-03-18 13:17, schrieb [email protected]:
> On 07.03.2022 14:04, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> the
>> content is safe
>>
>> Am 2022-03-07 12:53, schrieb [email protected]:
>>> On 04.03.2022 13:01, Michael Walle wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>> know
>>>> the
>>>> content is safe
>>>>
>>>> Hi,
>>>>
>>>> thanks for the quick review.
>>>>
>>>> Am 2022-03-04 09:30, schrieb [email protected]:
>>>>> On 03.03.2022 18:03, Michael Walle wrote:
>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>>> know
>>>>>> the content is safe
>>>>>>
>>>>>> Add all the usart nodes for the flexcom block. There was already
>>>>>> an usart node for the flexcom3 block. But it missed the DMA
>>>>>> channels.
>>>>>
>>>>> And it would be good to go though a different patch.
>>>>
>>>> sure
>>>>
>>>>>> Although the DMA channels are specified, DMA is not
>>>>>> enabled by default because break detection doesn't work with DMA.
>>>>>>
>>>>>> Keep the nodes disabled by default.
>>>>>>
>>>>>> Signed-off-by: Michael Walle <[email protected]>
>>>>>> ---
>>>>>>  arch/arm/boot/dts/lan966x.dtsi | 55
>>>>>> ++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 55 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/lan966x.dtsi
>>>>>> b/arch/arm/boot/dts/lan966x.dtsi
>>>>>> index a7d46a2ca058..bea69b6d2749 100644
>>>>>> --- a/arch/arm/boot/dts/lan966x.dtsi
>>>>>> +++ b/arch/arm/boot/dts/lan966x.dtsi
>>>>>> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>>>>>>                         #size-cells = <1>;
>>>>>>                         ranges = <0x0 0xe0040000 0x800>;
>>>>>>                         status = "disabled";
>>>>>> +
>>>>>> +                       usart0: serial@200 {
>>>>>> +                               compatible =
>>>>>> "atmel,at91sam9260-usart";
>>>>>
>>>>> Are the usart blocks in lan966x 1:1 compatible with what is is
>>>>> sam9260?
>>>>> In
>>>>> case not it may worth to have a new compatible here, for lan966x,
>>>>> such
>>>>> that
>>>>> when new features will be implemented in usart driver for lan966x
>>>>> the
>>>>> old
>>>>> DT (this one) will work with the new kernel implementation.
>>>>
>>>> During my review of the inital dtsi patch, I've asked the same
>>>> question
>>>> [1]
>>>> and I was told they are the same.
>>>>
>>>> At least this exact usart compatible is already in this file. I was
>>>> under
>>>> the impression, that was the least controversial compatible :)
>>>
>>> OK.
>>>
>>>>
>>>> But you'll need to tell me if they are the same or not, I don't have
>>>> any clue what microchip has reused.
>>>
>>> From software point of view comparing registers should be good, as
>>> far
>>> as I
>>> can tell. All AT91 datasheet should be available. I though you have
>>> checked
>>> one against LAN966. At the moment I don't have a DS for LAN966. I'll
>>> find
>>> one and have a look.
>>
>> So my train of thought was like: even if the registers are the same I
>> cannot be sure that it is the exact same IP and will behave the same.
>> Therefore, it is something only microchip can answer.
>>
>> You can find the registers at
>> https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
>>
>> I'm not aware of any "classic" datasheet.
>
> You can find all AT91 datasheet on Microchip web site [1].
>
> Simple register comparison b/w register mapping at [2] and SAMA5D2
> datasheet [3] (which uses the same compatible), SAM9X60 datasheet [3]
> and
> SAMA7G5 datasheet (not public at the moment) brings up a difference at
> register FLEX_US_CR (bits 16, 17) which are not available on SAMA5D2,
> SAM9X60 or SAMA7G5. Unless this is a mistake on documentation at [2] I
> say
> it needs a new compatible.

I can't follow you here. These bits are already used in the current UART
driver and are supported on the LAN966X. So if anything, SAMA5D2,
SAM9X60
and SAMA7G5 need a new compatible, no?

-michael

> Kavya, could you confirm this?
>
> Thank you,
> Claudiu Beznea
>
> [1] https://www.microchip.com/
> [2]
> https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
> [3]
> http://ww1.microchip.com/downloads/en/devicedoc/ds60001476b.pdf#G22.2193277
> [4]
> https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/DataSheets/SAM9X60-Data-Sheet-DS60001579E.pdf
>
>>
>> -michael

2022-03-25 19:06:50

by Claudiu Beznea

[permalink] [raw]
Subject: Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes

On 22.03.2022 23:39, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> Am 2022-03-18 13:17, schrieb [email protected]:
>> On 07.03.2022 14:04, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the
>>> content is safe
>>>
>>> Am 2022-03-07 12:53, schrieb [email protected]:
>>>> On 04.03.2022 13:01, Michael Walle wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>> know
>>>>> the
>>>>> content is safe
>>>>>
>>>>> Hi,
>>>>>
>>>>> thanks for the quick review.
>>>>>
>>>>> Am 2022-03-04 09:30, schrieb [email protected]:
>>>>>> On 03.03.2022 18:03, Michael Walle wrote:
>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>>>>>> know
>>>>>>> the content is safe
>>>>>>>
>>>>>>> Add all the usart nodes for the flexcom block. There was already
>>>>>>> an usart node for the flexcom3 block. But it missed the DMA
>>>>>>> channels.
>>>>>>
>>>>>> And it would be good to go though a different patch.
>>>>>
>>>>> sure
>>>>>
>>>>>>> Although the DMA channels are specified, DMA is not
>>>>>>> enabled by default because break detection doesn't work with DMA.
>>>>>>>
>>>>>>> Keep the nodes disabled by default.
>>>>>>>
>>>>>>> Signed-off-by: Michael Walle <[email protected]>
>>>>>>> ---
>>>>>>>  arch/arm/boot/dts/lan966x.dtsi | 55
>>>>>>> ++++++++++++++++++++++++++++++++++
>>>>>>>  1 file changed, 55 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/lan966x.dtsi
>>>>>>> b/arch/arm/boot/dts/lan966x.dtsi
>>>>>>> index a7d46a2ca058..bea69b6d2749 100644
>>>>>>> --- a/arch/arm/boot/dts/lan966x.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/lan966x.dtsi
>>>>>>> @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
>>>>>>>                         #size-cells = <1>;
>>>>>>>                         ranges = <0x0 0xe0040000 0x800>;
>>>>>>>                         status = "disabled";
>>>>>>> +
>>>>>>> +                       usart0: serial@200 {
>>>>>>> +                               compatible =
>>>>>>> "atmel,at91sam9260-usart";
>>>>>>
>>>>>> Are the usart blocks in lan966x 1:1 compatible with what is is
>>>>>> sam9260?
>>>>>> In
>>>>>> case not it may worth to have a new compatible here, for lan966x,
>>>>>> such
>>>>>> that
>>>>>> when new features will be implemented in usart driver for lan966x
>>>>>> the
>>>>>> old
>>>>>> DT (this one) will work with the new kernel implementation.
>>>>>
>>>>> During my review of the inital dtsi patch, I've asked the same
>>>>> question
>>>>> [1]
>>>>> and I was told they are the same.
>>>>>
>>>>> At least this exact usart compatible is already in this file. I was
>>>>> under
>>>>> the impression, that was the least controversial compatible :)
>>>>
>>>> OK.
>>>>
>>>>>
>>>>> But you'll need to tell me if they are the same or not, I don't have
>>>>> any clue what microchip has reused.
>>>>
>>>> From software point of view comparing registers should be good, as
>>>> far
>>>> as I
>>>> can tell. All AT91 datasheet should be available. I though you have
>>>> checked
>>>> one against LAN966. At the moment I don't have a DS for LAN966. I'll
>>>> find
>>>> one and have a look.
>>>
>>> So my train of thought was like: even if the registers are the same I
>>> cannot be sure that it is the exact same IP and will behave the same.
>>> Therefore, it is something only microchip can answer.
>>>
>>> You can find the registers at
>>> https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
>>>
>>> I'm not aware of any "classic" datasheet.
>>
>> You can find all AT91 datasheet on Microchip web site [1].
>>
>> Simple register comparison b/w register mapping at [2] and SAMA5D2
>> datasheet [3] (which uses the same compatible),  SAM9X60 datasheet [3]
>> and
>> SAMA7G5 datasheet (not public at the moment) brings up a difference at
>> register FLEX_US_CR (bits 16, 17) which are not available on SAMA5D2,
>> SAM9X60 or SAMA7G5. Unless this is a mistake on documentation at [2] I
>> say
>> it needs a new compatible.
>
> I can't follow you here. These bits are already used in the current UART
> driver

You're right, I haven't checked the driver.

> and are supported on the LAN966X. So if anything, SAMA5D2,
> SAM9X60
> and SAMA7G5 need a new compatible, no?

It seems that's true unless some errors in datasheet. I'll double check on
my side.

Thank you,
Claudiu Beznea

>
> -michael
>
>> Kavya, could you confirm this?
>>
>> Thank you,
>> Claudiu Beznea
>>
>> [1] https://www.microchip.com/
>> [2]
>> https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
>> [3]
>> http://ww1.microchip.com/downloads/en/devicedoc/ds60001476b.pdf#G22.2193277
>> [4]
>> https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/DataSheets/SAM9X60-Data-Sheet-DS60001579E.pdf
>>
>>
>>>
>>> -michael