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-next>] [day] [month] [year] [list]
Message-ID: <20230712200447118d91bc@mail.local>
Date:   Wed, 12 Jul 2023 22:04:47 +0200
From:   Alexandre Belloni <alexandre.belloni@...tlin.com>
To:     Conor Dooley <conor@...nel.org>
Cc:     Durai Manickam KR <durai.manickamkr@...rochip.com>,
        Hari.PrasathGE@...rochip.com,
        balamanikandan.gunasundar@...rochip.com,
        manikandan.m@...rochip.com, varshini.rajendran@...rochip.com,
        dharma.b@...rochip.com, nayabbasha.sayed@...rochip.com,
        balakrishnan.s@...rochip.com, cristian.birsan@...rochip.com,
        nicolas.ferre@...rochip.com, krzysztof.kozlowski@...aro.org,
        davem@...emloft.net, arnd@...db.de, olof@...om.net, soc@...nel.org,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, Kavyasree.Kotagiri@...rochip.com,
        Horatiu.Vultur@...rochip.com, robh+dt@...nel.org, andrew@...n.ch,
        michael@...le.cc, jerry.ray@...rochip.com, conor+dt@...nel.org,
        jesper.nilsson@...s.com, sergiu.moga@...rochip.com,
        andre.przywara@....com, ada@...rsis.com
Subject: Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection

On 12/07/2023 20:42:00+0100, Conor Dooley wrote:
> > There can be only one dbgu on a SoC and it has the chipid register. It
> > has always been wrong to have a dbgu compatible on the flexcom uart as
> > they are not dbgu.
> 
> Ahh, thanks for the explanation. Sounds like stuff that might be obvious
> to those working on arm stuff all the time, but not all of those CCed on
> the patch ;)

Well, this is not really arm specific, rather microchip specific ;)

> The correct thing to do would seem to be updating the dt-binding to
> something that permits what is listed below, or just removing the second
> compatible, to leave "microchip,sam9x60-usart" in isolation, since that
> is permitted also.
> 
> > Anyway, my advice has always been that you must not do chipid detection
> > on at91 because there is no point in doing it because you need to have
> > a correct dts to be able to find the dbgu chipid register so you either
> > you already know what you are running on or you are going to read bogus
> > registers anyway.
> 
> I won't claim to have an opinion on any of that, but sounds like you'd
> be better off just reading the board/SoC level compatibles to figure out
> what you are running on...
> 

Yeah, exactly my point!

> Thanks,
> Conor.
> 
> > > > Fixes: 99c808335877 (ARM: dts: at91: sam9x60: Add missing flexcom definitions)
> > > > Signed-off-by: Durai Manickam KR <durai.manickamkr@...rochip.com>
> > > > ---
> > > >  arch/arm/boot/dts/microchip/sam9x60.dtsi | 26 ++++++++++++------------
> > > >  1 file changed, 13 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi
> > > > index 8b53997675e7..73d570a17269 100644
> > > > --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi
> > > > +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi
> > > > @@ -172,7 +172,7 @@ flx4: flexcom@...00000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart4: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					interrupts = <13 IRQ_TYPE_LEVEL_HIGH 7>;
> > > >  					dmas = <&dma0
> > > > @@ -240,7 +240,7 @@ flx5: flexcom@...04000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart5: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					atmel,usart-mode = <AT91_USART_MODE_SERIAL>;
> > > >  					interrupts = <14 IRQ_TYPE_LEVEL_HIGH 7>;
> > > > @@ -370,7 +370,7 @@ flx11: flexcom@...20000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart11: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					interrupts = <32 IRQ_TYPE_LEVEL_HIGH 7>;
> > > >  					dmas = <&dma0
> > > > @@ -419,7 +419,7 @@ flx12: flexcom@...24000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart12: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					interrupts = <33 IRQ_TYPE_LEVEL_HIGH 7>;
> > > >  					dmas = <&dma0
> > > > @@ -576,7 +576,7 @@ flx6: flexcom@...10000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart6: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					interrupts = <9 IRQ_TYPE_LEVEL_HIGH 7>;
> > > >  					dmas = <&dma0
> > > > @@ -625,7 +625,7 @@ flx7: flexcom@...14000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart7: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					interrupts = <10 IRQ_TYPE_LEVEL_HIGH 7>;
> > > >  					dmas = <&dma0
> > > > @@ -674,7 +674,7 @@ flx8: flexcom@...18000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart8: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					interrupts = <11 IRQ_TYPE_LEVEL_HIGH 7>;
> > > >  					dmas = <&dma0
> > > > @@ -723,7 +723,7 @@ flx0: flexcom@...1c000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart0: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					interrupts = <5 IRQ_TYPE_LEVEL_HIGH 7>;
> > > >  					dmas = <&dma0
> > > > @@ -791,7 +791,7 @@ flx1: flexcom@...20000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart1: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					interrupts = <6 IRQ_TYPE_LEVEL_HIGH 7>;
> > > >  					dmas = <&dma0
> > > > @@ -859,7 +859,7 @@ flx2: flexcom@...24000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart2: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					interrupts = <7 IRQ_TYPE_LEVEL_HIGH 7>;
> > > >  					dmas = <&dma0
> > > > @@ -927,7 +927,7 @@ flx3: flexcom@...28000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart3: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					interrupts = <8 IRQ_TYPE_LEVEL_HIGH 7>;
> > > >  					dmas = <&dma0
> > > > @@ -1050,7 +1050,7 @@ flx9: flexcom@...40000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart9: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					interrupts = <15 IRQ_TYPE_LEVEL_HIGH 7>;
> > > >  					dmas = <&dma0
> > > > @@ -1099,7 +1099,7 @@ flx10: flexcom@...44000 {
> > > >  				status = "disabled";
> > > >  
> > > >  				uart10: serial@200 {
> > > > -					compatible = "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart";
> > > > +					compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
> > > >  					reg = <0x200 0x200>;
> > > >  					interrupts = <16 IRQ_TYPE_LEVEL_HIGH 7>;
> > > >  					dmas = <&dma0
> > > > -- 
> > > > 2.25.1
> > > > 
> > 
> > 
> > 
> > -- 
> > Alexandre Belloni, co-owner and COO, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com



-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ