From: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Conor Dooley <conor@kernel.org>
Cc: Durai Manickam KR <durai.manickamkr@microchip.com>,
Hari.PrasathGE@microchip.com,
balamanikandan.gunasundar@microchip.com,
manikandan.m@microchip.com, varshini.rajendran@microchip.com,
dharma.b@microchip.com, nayabbasha.sayed@microchip.com,
balakrishnan.s@microchip.com, cristian.birsan@microchip.com,
nicolas.ferre@microchip.com, krzysztof.kozlowski@linaro.org,
davem@davemloft.net, arnd@arndb.de, olof@lixom.net,
soc@kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Kavyasree.Kotagiri@microchip.com,
Horatiu.Vultur@microchip.com, robh+dt@kernel.org, andrew@lunn.ch,
michael@walle.cc, jerry.ray@microchip.com, conor+dt@kernel.org,
jesper.nilsson@axis.com, sergiu.moga@microchip.com,
andre.przywara@arm.com, ada@thorsis.com
Subject: Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
Date: Wed, 12 Jul 2023 22:04:47 +0200 [thread overview]
Message-ID: <20230712200447118d91bc@mail.local> (raw)
In-Reply-To: <20230712-revolving-tactful-67cdb02c664b@spud>
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@microchip.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@f0000000 {
> > > > 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@f0004000 {
> > > > 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@f0020000 {
> > > > 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@f0024000 {
> > > > 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@f8010000 {
> > > > 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@f8014000 {
> > > > 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@f8018000 {
> > > > 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@f801c000 {
> > > > 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@f8020000 {
> > > > 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@f8024000 {
> > > > 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@f8028000 {
> > > > 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@f8040000 {
> > > > 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@f8044000 {
> > > > 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
next prev parent reply other threads:[~2023-07-12 20:04 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-12 10:00 [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection Durai Manickam KR
[not found] ` <20230712-dweeb-suspend-27d0f7292a2d@spud>
2023-07-12 19:30 ` Alexandre Belloni
[not found] ` <20230712-revolving-tactful-67cdb02c664b@spud>
2023-07-12 20:04 ` Alexandre Belloni [this message]
2023-07-13 8:29 ` Durai.ManickamKR
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230712200447118d91bc@mail.local \
--to=alexandre.belloni@bootlin.com \
--cc=Hari.PrasathGE@microchip.com \
--cc=Horatiu.Vultur@microchip.com \
--cc=Kavyasree.Kotagiri@microchip.com \
--cc=ada@thorsis.com \
--cc=andre.przywara@arm.com \
--cc=andrew@lunn.ch \
--cc=arnd@arndb.de \
--cc=balakrishnan.s@microchip.com \
--cc=balamanikandan.gunasundar@microchip.com \
--cc=conor+dt@kernel.org \
--cc=conor@kernel.org \
--cc=cristian.birsan@microchip.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=dharma.b@microchip.com \
--cc=durai.manickamkr@microchip.com \
--cc=jerry.ray@microchip.com \
--cc=jesper.nilsson@axis.com \
--cc=krzysztof.kozlowski@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=manikandan.m@microchip.com \
--cc=michael@walle.cc \
--cc=nayabbasha.sayed@microchip.com \
--cc=nicolas.ferre@microchip.com \
--cc=olof@lixom.net \
--cc=robh+dt@kernel.org \
--cc=sergiu.moga@microchip.com \
--cc=soc@kernel.org \
--cc=varshini.rajendran@microchip.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).