Messages in this thread | | | From | <> | Subject | Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes | Date | Mon, 7 Mar 2022 11:53:16 +0000 |
| |
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 Claudiu.Beznea@microchip.com: >> 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 <michael@walle.cc> >>> --- >>> 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/20220210123704.477826-1-michael@walle.cc/ > >>> + 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
| |