lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2e0aefc90a80bdf13df0e59857c52ca7@walle.cc>
Date:   Fri, 04 Mar 2022 12:01:09 +0100
From:   Michael Walle <michael@...le.cc>
To:     Claudiu.Beznea@...rochip.com
Cc:     Kavyasree.Kotagiri@...rochip.com, Nicolas.Ferre@...rochip.com,
        arnd@...db.de, olof@...om.net, soc@...nel.org,
        linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, robh+dt@...nel.org,
        krzysztof.kozlowski@...onical.com, alexandre.belloni@...tlin.com
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 Claudiu.Beznea@...rochip.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@...le.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@...40000 {
>>                         #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/20220210123704.477826-1-michael@walle.cc/

>> +                               dma-names = "tx", "rx";
>> +                               clocks = <&nic_clk>;
>> +                               clock-names = "usart";
>> +                               atmel,fifo-size = <32>;
>> +                               status = "disabled";
>> +                       };
>>                 };
>> 
>>                 flx1: flexcom@...44000 {
>> @@ -102,6 +115,19 @@ flx1: flexcom@...44000 {
>>                         #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@...48000 {
>> @@ -129,6 +155,19 @@ flx2: flexcom@...60000 {
>>                         #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@...64000 {
>> @@ -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@...70000 {
>>                         #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@...8c000 {
>> --
>> 2.30.2
>> 

-- 
-michael

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ