All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
@ 2023-07-12 10:00 ` Durai Manickam KR
  0 siblings, 0 replies; 13+ messages in thread
From: Durai Manickam KR @ 2023-07-12 10:00 UTC (permalink / raw)
  To: Hari.PrasathGE, balamanikandan.gunasundar, manikandan.m,
	varshini.rajendran, dharma.b, nayabbasha.sayed, balakrishnan.s,
	cristian.birsan, nicolas.ferre, krzysztof.kozlowski,
	alexandre.belloni, davem, arnd, olof, soc, devicetree,
	linux-arm-kernel, linux-kernel, Kavyasree.Kotagiri,
	Horatiu.Vultur, robh+dt, andrew, michael, jerry.ray, conor+dt,
	jesper.nilsson, sergiu.moga, andre.przywara, ada
  Cc: Durai Manickam KR

Remove the dbgu compatible strings in the UART submodule of the
flexcom for the proper SOC detection.

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
@ 2023-07-12 10:00 ` Durai Manickam KR
  0 siblings, 0 replies; 13+ messages in thread
From: Durai Manickam KR @ 2023-07-12 10:00 UTC (permalink / raw)
  To: Hari.PrasathGE, balamanikandan.gunasundar, manikandan.m,
	varshini.rajendran, dharma.b, nayabbasha.sayed, balakrishnan.s,
	cristian.birsan, nicolas.ferre, krzysztof.kozlowski,
	alexandre.belloni, davem, arnd, olof, soc, devicetree,
	linux-arm-kernel, linux-kernel, Kavyasree.Kotagiri,
	Horatiu.Vultur, robh+dt, andrew, michael, jerry.ray, conor+dt,
	jesper.nilsson, sergiu.moga, andre.przywara, ada
  Cc: Durai Manickam KR

Remove the dbgu compatible strings in the UART submodule of the
flexcom for the proper SOC detection.

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
  2023-07-12 10:00 ` Durai Manickam KR
@ 2023-07-12 18:43   ` Conor Dooley
  -1 siblings, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2023-07-12 18:43 UTC (permalink / raw)
  To: Durai Manickam KR
  Cc: Hari.PrasathGE, balamanikandan.gunasundar, manikandan.m,
	varshini.rajendran, dharma.b, nayabbasha.sayed, balakrishnan.s,
	cristian.birsan, nicolas.ferre, krzysztof.kozlowski,
	alexandre.belloni, davem, arnd, olof, soc, devicetree,
	linux-arm-kernel, linux-kernel, Kavyasree.Kotagiri,
	Horatiu.Vultur, robh+dt, andrew, michael, jerry.ray, conor+dt,
	jesper.nilsson, sergiu.moga, andre.przywara, ada

[-- Attachment #1: Type: text/plain, Size: 7762 bytes --]

Hey,

On Wed, Jul 12, 2023 at 03:30:42PM +0530, Durai Manickam KR wrote:
> Remove the dbgu compatible strings in the UART submodule of the
> flexcom for the proper SOC detection.

I am afraid that I do not understand what this means. The first
compatible, before and after your patch, is a sam9x60 one, so I don't
see how the SoC comes into it. Grepping to try and understand:
rg microchip,sam9x60-usart drivers/

rg microchip,sam9x60-dbgu drivers/

rg atmel,at91sam9260-dbgu drivers/
drivers/soc/atmel/soc.c
259:					     "atmel,at91sam9260-dbgu");

rg atmel,at91sam9260-usart drivers/
drivers/tty/serial/atmel_serial.c
2723:OF_EARLYCON_DECLARE(atmel_serial, "atmel,at91sam9260-usart",

drivers/mfd/at91-usart.c
50:	{ .compatible = "atmel,at91sam9260-usart" },

It looks like you are trying to avoid the atmel soc driver from
detecting the atmel,at91sam9260-dbgu compatible?
Is this not actually compatible with the sam9260's debug uart?

Please try to explain things in more detail so that people unfamiliar
with at91 hardware are not confused. As an outsider, removing
compatibles to fix detection seems like it would not work - the sam9x60
compatibles are first in the list. To me it seems like this removes,
rather than fixes detection, and to fix detection you would need to
make some changes to the driver?

Also, the binding says:
      - items:
          - const: microchip,sam9x60-dbgu
          - const: microchip,sam9x60-usart
          - const: atmel,at91sam9260-dbgu
          - const: atmel,at91sam9260-usart

Your patch here makes the dts non-compliant with the bindings, as far as
I can tell, so you will need to go and modify the bindings to do this
either way.

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
@ 2023-07-12 18:43   ` Conor Dooley
  0 siblings, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2023-07-12 18:43 UTC (permalink / raw)
  To: Durai Manickam KR
  Cc: andrew, alexandre.belloni, devicetree, jerry.ray, Hari.PrasathGE,
	varshini.rajendran, Horatiu.Vultur, manikandan.m, jesper.nilsson,
	balamanikandan.gunasundar, michael, dharma.b, cristian.birsan,
	nayabbasha.sayed, conor+dt, arnd, andre.przywara, soc, robh+dt,
	sergiu.moga, linux-arm-kernel, balakrishnan.s, ada, linux-kernel,
	krzysztof.kozlowski, Kavyasree.Kotagiri, olof, davem


[-- Attachment #1.1: Type: text/plain, Size: 7762 bytes --]

Hey,

On Wed, Jul 12, 2023 at 03:30:42PM +0530, Durai Manickam KR wrote:
> Remove the dbgu compatible strings in the UART submodule of the
> flexcom for the proper SOC detection.

I am afraid that I do not understand what this means. The first
compatible, before and after your patch, is a sam9x60 one, so I don't
see how the SoC comes into it. Grepping to try and understand:
rg microchip,sam9x60-usart drivers/

rg microchip,sam9x60-dbgu drivers/

rg atmel,at91sam9260-dbgu drivers/
drivers/soc/atmel/soc.c
259:					     "atmel,at91sam9260-dbgu");

rg atmel,at91sam9260-usart drivers/
drivers/tty/serial/atmel_serial.c
2723:OF_EARLYCON_DECLARE(atmel_serial, "atmel,at91sam9260-usart",

drivers/mfd/at91-usart.c
50:	{ .compatible = "atmel,at91sam9260-usart" },

It looks like you are trying to avoid the atmel soc driver from
detecting the atmel,at91sam9260-dbgu compatible?
Is this not actually compatible with the sam9260's debug uart?

Please try to explain things in more detail so that people unfamiliar
with at91 hardware are not confused. As an outsider, removing
compatibles to fix detection seems like it would not work - the sam9x60
compatibles are first in the list. To me it seems like this removes,
rather than fixes detection, and to fix detection you would need to
make some changes to the driver?

Also, the binding says:
      - items:
          - const: microchip,sam9x60-dbgu
          - const: microchip,sam9x60-usart
          - const: atmel,at91sam9260-dbgu
          - const: atmel,at91sam9260-usart

Your patch here makes the dts non-compliant with the bindings, as far as
I can tell, so you will need to go and modify the bindings to do this
either way.

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
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
  2023-07-12 18:43   ` Conor Dooley
@ 2023-07-12 19:30     ` Alexandre Belloni
  -1 siblings, 0 replies; 13+ messages in thread
From: Alexandre Belloni @ 2023-07-12 19:30 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Durai Manickam KR, Hari.PrasathGE, balamanikandan.gunasundar,
	manikandan.m, varshini.rajendran, dharma.b, nayabbasha.sayed,
	balakrishnan.s, cristian.birsan, nicolas.ferre,
	krzysztof.kozlowski, davem, arnd, olof, soc, devicetree,
	linux-arm-kernel, linux-kernel, Kavyasree.Kotagiri,
	Horatiu.Vultur, robh+dt, andrew, michael, jerry.ray, conor+dt,
	jesper.nilsson, sergiu.moga, andre.przywara, ada

On 12/07/2023 19:43:51+0100, Conor Dooley wrote:
> Hey,
> 
> On Wed, Jul 12, 2023 at 03:30:42PM +0530, Durai Manickam KR wrote:
> > Remove the dbgu compatible strings in the UART submodule of the
> > flexcom for the proper SOC detection.
> 
> I am afraid that I do not understand what this means. The first
> compatible, before and after your patch, is a sam9x60 one, so I don't
> see how the SoC comes into it. Grepping to try and understand:
> rg microchip,sam9x60-usart drivers/
> 
> rg microchip,sam9x60-dbgu drivers/
> 
> rg atmel,at91sam9260-dbgu drivers/
> drivers/soc/atmel/soc.c
> 259:					     "atmel,at91sam9260-dbgu");
> 
> rg atmel,at91sam9260-usart drivers/
> drivers/tty/serial/atmel_serial.c
> 2723:OF_EARLYCON_DECLARE(atmel_serial, "atmel,at91sam9260-usart",
> 
> drivers/mfd/at91-usart.c
> 50:	{ .compatible = "atmel,at91sam9260-usart" },
> 
> It looks like you are trying to avoid the atmel soc driver from
> detecting the atmel,at91sam9260-dbgu compatible?
> Is this not actually compatible with the sam9260's debug uart?
> 
> Please try to explain things in more detail so that people unfamiliar
> with at91 hardware are not confused. As an outsider, removing
> compatibles to fix detection seems like it would not work - the sam9x60
> compatibles are first in the list. To me it seems like this removes,
> rather than fixes detection, and to fix detection you would need to
> make some changes to the driver?
> 
> Also, the binding says:
>       - items:
>           - const: microchip,sam9x60-dbgu
>           - const: microchip,sam9x60-usart
>           - const: atmel,at91sam9260-dbgu
>           - const: atmel,at91sam9260-usart
> 
> Your patch here makes the dts non-compliant with the bindings, as far as
> I can tell, so you will need to go and modify the bindings to do this
> either way.

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.

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.

> 
> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
@ 2023-07-12 19:30     ` Alexandre Belloni
  0 siblings, 0 replies; 13+ messages in thread
From: Alexandre Belloni @ 2023-07-12 19:30 UTC (permalink / raw)
  To: Conor Dooley
  Cc: andrew, jerry.ray, devicetree, Hari.PrasathGE,
	varshini.rajendran, Horatiu.Vultur, manikandan.m, jesper.nilsson,
	balamanikandan.gunasundar, michael, dharma.b, cristian.birsan,
	nayabbasha.sayed, conor+dt, arnd, andre.przywara, soc, robh+dt,
	Durai Manickam KR, sergiu.moga, linux-arm-kernel, balakrishnan.s,
	ada, linux-kernel, krzysztof.kozlowski, Kavyasree.Kotagiri, olof,
	davem

On 12/07/2023 19:43:51+0100, Conor Dooley wrote:
> Hey,
> 
> On Wed, Jul 12, 2023 at 03:30:42PM +0530, Durai Manickam KR wrote:
> > Remove the dbgu compatible strings in the UART submodule of the
> > flexcom for the proper SOC detection.
> 
> I am afraid that I do not understand what this means. The first
> compatible, before and after your patch, is a sam9x60 one, so I don't
> see how the SoC comes into it. Grepping to try and understand:
> rg microchip,sam9x60-usart drivers/
> 
> rg microchip,sam9x60-dbgu drivers/
> 
> rg atmel,at91sam9260-dbgu drivers/
> drivers/soc/atmel/soc.c
> 259:					     "atmel,at91sam9260-dbgu");
> 
> rg atmel,at91sam9260-usart drivers/
> drivers/tty/serial/atmel_serial.c
> 2723:OF_EARLYCON_DECLARE(atmel_serial, "atmel,at91sam9260-usart",
> 
> drivers/mfd/at91-usart.c
> 50:	{ .compatible = "atmel,at91sam9260-usart" },
> 
> It looks like you are trying to avoid the atmel soc driver from
> detecting the atmel,at91sam9260-dbgu compatible?
> Is this not actually compatible with the sam9260's debug uart?
> 
> Please try to explain things in more detail so that people unfamiliar
> with at91 hardware are not confused. As an outsider, removing
> compatibles to fix detection seems like it would not work - the sam9x60
> compatibles are first in the list. To me it seems like this removes,
> rather than fixes detection, and to fix detection you would need to
> make some changes to the driver?
> 
> Also, the binding says:
>       - items:
>           - const: microchip,sam9x60-dbgu
>           - const: microchip,sam9x60-usart
>           - const: atmel,at91sam9260-dbgu
>           - const: atmel,at91sam9260-usart
> 
> Your patch here makes the dts non-compliant with the bindings, as far as
> I can tell, so you will need to go and modify the bindings to do this
> either way.

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.

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.

> 
> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
  2023-07-12 19:30     ` Alexandre Belloni
@ 2023-07-12 19:42       ` Conor Dooley
  -1 siblings, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2023-07-12 19:42 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Durai Manickam KR, Hari.PrasathGE, balamanikandan.gunasundar,
	manikandan.m, varshini.rajendran, dharma.b, nayabbasha.sayed,
	balakrishnan.s, cristian.birsan, nicolas.ferre,
	krzysztof.kozlowski, davem, arnd, olof, soc, devicetree,
	linux-arm-kernel, linux-kernel, Kavyasree.Kotagiri,
	Horatiu.Vultur, robh+dt, andrew, michael, jerry.ray, conor+dt,
	jesper.nilsson, sergiu.moga, andre.przywara, ada

[-- Attachment #1: Type: text/plain, Size: 9765 bytes --]

On Wed, Jul 12, 2023 at 09:30:33PM +0200, Alexandre Belloni wrote:
> On 12/07/2023 19:43:51+0100, Conor Dooley wrote:
> > On Wed, Jul 12, 2023 at 03:30:42PM +0530, Durai Manickam KR wrote:
> > > Remove the dbgu compatible strings in the UART submodule of the
> > > flexcom for the proper SOC detection.
> > 
> > I am afraid that I do not understand what this means. The first
> > compatible, before and after your patch, is a sam9x60 one, so I don't
> > see how the SoC comes into it. Grepping to try and understand:
> > rg microchip,sam9x60-usart drivers/
> > 
> > rg microchip,sam9x60-dbgu drivers/
> > 
> > rg atmel,at91sam9260-dbgu drivers/
> > drivers/soc/atmel/soc.c
> > 259:					     "atmel,at91sam9260-dbgu");
> > 
> > rg atmel,at91sam9260-usart drivers/
> > drivers/tty/serial/atmel_serial.c
> > 2723:OF_EARLYCON_DECLARE(atmel_serial, "atmel,at91sam9260-usart",
> > 
> > drivers/mfd/at91-usart.c
> > 50:	{ .compatible = "atmel,at91sam9260-usart" },
> > 
> > It looks like you are trying to avoid the atmel soc driver from
> > detecting the atmel,at91sam9260-dbgu compatible?
> > Is this not actually compatible with the sam9260's debug uart?
> > 
> > Please try to explain things in more detail so that people unfamiliar
> > with at91 hardware are not confused. As an outsider, removing
> > compatibles to fix detection seems like it would not work - the sam9x60
> > compatibles are first in the list. To me it seems like this removes,
> > rather than fixes detection, and to fix detection you would need to
> > make some changes to the driver?
> > 
> > Also, the binding says:
> >       - items:
> >           - const: microchip,sam9x60-dbgu
> >           - const: microchip,sam9x60-usart
> >           - const: atmel,at91sam9260-dbgu
> >           - const: atmel,at91sam9260-usart
> > 
> > Your patch here makes the dts non-compliant with the bindings, as far as
> > I can tell, so you will need to go and modify the bindings to do this
> > either way.
> 
> 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 ;)
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...

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
@ 2023-07-12 19:42       ` Conor Dooley
  0 siblings, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2023-07-12 19:42 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: andrew, jerry.ray, devicetree, Hari.PrasathGE,
	varshini.rajendran, Horatiu.Vultur, manikandan.m, jesper.nilsson,
	balamanikandan.gunasundar, michael, dharma.b, cristian.birsan,
	nayabbasha.sayed, conor+dt, arnd, andre.przywara, soc, robh+dt,
	Durai Manickam KR, sergiu.moga, linux-arm-kernel, balakrishnan.s,
	ada, linux-kernel, krzysztof.kozlowski, Kavyasree.Kotagiri, olof,
	davem


[-- Attachment #1.1: Type: text/plain, Size: 9765 bytes --]

On Wed, Jul 12, 2023 at 09:30:33PM +0200, Alexandre Belloni wrote:
> On 12/07/2023 19:43:51+0100, Conor Dooley wrote:
> > On Wed, Jul 12, 2023 at 03:30:42PM +0530, Durai Manickam KR wrote:
> > > Remove the dbgu compatible strings in the UART submodule of the
> > > flexcom for the proper SOC detection.
> > 
> > I am afraid that I do not understand what this means. The first
> > compatible, before and after your patch, is a sam9x60 one, so I don't
> > see how the SoC comes into it. Grepping to try and understand:
> > rg microchip,sam9x60-usart drivers/
> > 
> > rg microchip,sam9x60-dbgu drivers/
> > 
> > rg atmel,at91sam9260-dbgu drivers/
> > drivers/soc/atmel/soc.c
> > 259:					     "atmel,at91sam9260-dbgu");
> > 
> > rg atmel,at91sam9260-usart drivers/
> > drivers/tty/serial/atmel_serial.c
> > 2723:OF_EARLYCON_DECLARE(atmel_serial, "atmel,at91sam9260-usart",
> > 
> > drivers/mfd/at91-usart.c
> > 50:	{ .compatible = "atmel,at91sam9260-usart" },
> > 
> > It looks like you are trying to avoid the atmel soc driver from
> > detecting the atmel,at91sam9260-dbgu compatible?
> > Is this not actually compatible with the sam9260's debug uart?
> > 
> > Please try to explain things in more detail so that people unfamiliar
> > with at91 hardware are not confused. As an outsider, removing
> > compatibles to fix detection seems like it would not work - the sam9x60
> > compatibles are first in the list. To me it seems like this removes,
> > rather than fixes detection, and to fix detection you would need to
> > make some changes to the driver?
> > 
> > Also, the binding says:
> >       - items:
> >           - const: microchip,sam9x60-dbgu
> >           - const: microchip,sam9x60-usart
> >           - const: atmel,at91sam9260-dbgu
> >           - const: atmel,at91sam9260-usart
> > 
> > Your patch here makes the dts non-compliant with the bindings, as far as
> > I can tell, so you will need to go and modify the bindings to do this
> > either way.
> 
> 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 ;)
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...

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
  2023-07-12 19:42       ` Conor Dooley
@ 2023-07-12 20:04         ` Alexandre Belloni
  -1 siblings, 0 replies; 13+ messages in thread
From: Alexandre Belloni @ 2023-07-12 20:04 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Durai Manickam KR, Hari.PrasathGE, balamanikandan.gunasundar,
	manikandan.m, varshini.rajendran, dharma.b, nayabbasha.sayed,
	balakrishnan.s, cristian.birsan, nicolas.ferre,
	krzysztof.kozlowski, davem, arnd, olof, soc, devicetree,
	linux-arm-kernel, linux-kernel, Kavyasree.Kotagiri,
	Horatiu.Vultur, robh+dt, andrew, michael, jerry.ray, conor+dt,
	jesper.nilsson, sergiu.moga, andre.przywara, ada

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
@ 2023-07-12 20:04         ` Alexandre Belloni
  0 siblings, 0 replies; 13+ messages in thread
From: Alexandre Belloni @ 2023-07-12 20:04 UTC (permalink / raw)
  To: Conor Dooley
  Cc: andrew, jerry.ray, devicetree, Hari.PrasathGE,
	varshini.rajendran, Horatiu.Vultur, manikandan.m, jesper.nilsson,
	balamanikandan.gunasundar, michael, dharma.b, cristian.birsan,
	nayabbasha.sayed, conor+dt, arnd, andre.przywara, soc, robh+dt,
	Durai Manickam KR, sergiu.moga, linux-arm-kernel, balakrishnan.s,
	ada, linux-kernel, krzysztof.kozlowski, Kavyasree.Kotagiri, olof,
	davem

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
  2023-07-12 20:04         ` Alexandre Belloni
@ 2023-07-13  8:29           ` Durai.ManickamKR
  -1 siblings, 0 replies; 13+ messages in thread
From: Durai.ManickamKR @ 2023-07-13  8:29 UTC (permalink / raw)
  To: alexandre.belloni, conor
  Cc: Hari.PrasathGE, Balamanikandan.Gunasundar, Manikandan.M,
	Varshini.Rajendran, Dharma.B, Nayabbasha.Sayed, Balakrishnan.S,
	Cristian.Birsan, Nicolas.Ferre, krzysztof.kozlowski, davem, arnd,
	olof, soc, devicetree, linux-arm-kernel, linux-kernel,
	Kavyasree.Kotagiri, Horatiu.Vultur, robh+dt, andrew, michael,
	Jerry.Ray, conor+dt, jesper.nilsson, sergiu.moga, andre.przywara,
	ada


On 13/07/23 01:34, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> 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.
Noted, I hope updating the yaml would be proper approach.
>>> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
@ 2023-07-13  8:29           ` Durai.ManickamKR
  0 siblings, 0 replies; 13+ messages in thread
From: Durai.ManickamKR @ 2023-07-13  8:29 UTC (permalink / raw)
  To: alexandre.belloni, conor
  Cc: andrew, Jerry.Ray, devicetree, Hari.PrasathGE,
	Varshini.Rajendran, Horatiu.Vultur, Manikandan.M, jesper.nilsson,
	Balamanikandan.Gunasundar, krzysztof.kozlowski, Dharma.B,
	Cristian.Birsan, Nayabbasha.Sayed, conor+dt, arnd,
	andre.przywara, soc, robh+dt, sergiu.moga, linux-arm-kernel,
	Balakrishnan.S, ada, linux-kernel, michael, Kavyasree.Kotagiri,
	olof, davem


On 13/07/23 01:34, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> 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.
Noted, I hope updating the yaml would be proper approach.
>>> 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection
  2023-07-12 10:00 ` Durai Manickam KR
  (?)
  (?)
@ 2023-07-24 14:05 ` patchwork-bot+linux-soc
  -1 siblings, 0 replies; 13+ messages in thread
From: patchwork-bot+linux-soc @ 2023-07-24 14:05 UTC (permalink / raw)
  To: Durai Manickam KR; +Cc: soc

Hello:

This patch was applied to soc/soc.git (arm/fixes)
by Arnd Bergmann <arnd@arndb.de>:

On Wed, 12 Jul 2023 15:30:42 +0530 you wrote:
> Remove the dbgu compatible strings in the UART submodule of the
> flexcom for the proper SOC detection.
> 
> 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(-)

Here is the summary with links:
  - ARM: dts: at91: sam9x60: fix the SOC detection
    https://git.kernel.org/soc/soc/c/f6ad3c13f1b8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-07-24 14:05 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-12 10:00 [PATCH] ARM: dts: at91: sam9x60: fix the SOC detection Durai Manickam KR
2023-07-12 10:00 ` Durai Manickam KR
2023-07-12 18:43 ` Conor Dooley
2023-07-12 18:43   ` Conor Dooley
2023-07-12 19:30   ` Alexandre Belloni
2023-07-12 19:30     ` Alexandre Belloni
2023-07-12 19:42     ` Conor Dooley
2023-07-12 19:42       ` Conor Dooley
2023-07-12 20:04       ` Alexandre Belloni
2023-07-12 20:04         ` Alexandre Belloni
2023-07-13  8:29         ` Durai.ManickamKR
2023-07-13  8:29           ` Durai.ManickamKR
2023-07-24 14:05 ` patchwork-bot+linux-soc

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.