[PATCH] serial: xilinx_uartps: read reg size from DTS

[PATCH] serial: xilinx_uartps: read reg size from DTS

[Harshit Shah]

Current implementation uses `CDNS_UART_REGISTER_SPACE(0x1000)`
for request_mem_region() and ioremap() in cdns_uart_request_port() API.

The cadence/xilinx IP has register space defined from offset 0x0 to 0x48.
It also mentions that the register map is defined as [6:0]. So, the upper
region may/maynot be used based on the IP integration.

In Axiado AX3000 SoC two UART instances are defined
0x100 apart. That is creating issue in some other instance due to overlap
with addresses.

Since, this address space is already being defined in the
devicetree, use the same when requesting the register space.

Signed-off-by: Harshit Shah <hshah@axiado.com>
---
 drivers/tty/serial/xilinx_uartps.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index fe457bf1e15bb4fc77a5c7de2aea8bfbdbaa643a..a66b44d21fba2558d0b2a62864d86d3b73152e26 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -33,7 +33,6 @@
 #define CDNS_UART_MINOR		0	/* works best with devtmpfs */
 #define CDNS_UART_NR_PORTS	16
 #define CDNS_UART_FIFO_SIZE	64	/* FIFO size */
-#define CDNS_UART_REGISTER_SPACE	0x1000
 #define TX_TIMEOUT		500000
 
 /* Rx Trigger level */
@@ -1098,15 +1097,15 @@ static int cdns_uart_verify_port(struct uart_port *port,
  */
 static int cdns_uart_request_port(struct uart_port *port)
 {
-	if (!request_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE,
+	if (!request_mem_region(port->mapbase, port->mapsize,
 					 CDNS_UART_NAME)) {
 		return -ENOMEM;
 	}
 
-	port->membase = ioremap(port->mapbase, CDNS_UART_REGISTER_SPACE);
+	port->membase = ioremap(port->mapbase, port->mapsize);
 	if (!port->membase) {
 		dev_err(port->dev, "Unable to map registers\n");
-		release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+		release_mem_region(port->mapbase, port->mapsize);
 		return -ENOMEM;
 	}
 	return 0;
@@ -1121,7 +1120,7 @@ static int cdns_uart_request_port(struct uart_port *port)
  */
 static void cdns_uart_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+	release_mem_region(port->mapbase, port->mapsize);
 	iounmap(port->membase);
 	port->membase = NULL;
 }
@@ -1780,6 +1779,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	 * and triggers invocation of the config_port() entry point.
 	 */
 	port->mapbase = res->start;
+	port->mapsize = resource_size(res);
 	port->irq = irq;
 	port->dev = &pdev->dev;
 	port->uartclk = clk_get_rate(cdns_uart_data->uartclk);

---
base-commit: 8742b2d8935f476449ef37e263bc4da3295c7b58
change-id: 20250813-xilinx-uartps-reg-size-c3be67d88b7c

Best regards,
-- 
Harshit Shah <hshah@axiado.com>

Re: [PATCH] serial: xilinx_uartps: read reg size from DTS

[Michal Simek]

On 8/19/25 22:53, Harshit Shah wrote:
> Current implementation uses `CDNS_UART_REGISTER_SPACE(0x1000)`
> for request_mem_region() and ioremap() in cdns_uart_request_port() API.
> 
> The cadence/xilinx IP has register space defined from offset 0x0 to 0x48.
> It also mentions that the register map is defined as [6:0]. So, the upper
> region may/maynot be used based on the IP integration.
> 
> In Axiado AX3000 SoC two UART instances are defined
> 0x100 apart. That is creating issue in some other instance due to overlap
> with addresses.
> 
> Since, this address space is already being defined in the
> devicetree, use the same when requesting the register space.
> 
> Signed-off-by: Harshit Shah <hshah@axiado.com>
> ---
>   drivers/tty/serial/xilinx_uartps.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index fe457bf1e15bb4fc77a5c7de2aea8bfbdbaa643a..a66b44d21fba2558d0b2a62864d86d3b73152e26 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -33,7 +33,6 @@
>   #define CDNS_UART_MINOR		0	/* works best with devtmpfs */
>   #define CDNS_UART_NR_PORTS	16
>   #define CDNS_UART_FIFO_SIZE	64	/* FIFO size */
> -#define CDNS_UART_REGISTER_SPACE	0x1000
>   #define TX_TIMEOUT		500000
>   
>   /* Rx Trigger level */
> @@ -1098,15 +1097,15 @@ static int cdns_uart_verify_port(struct uart_port *port,
>    */
>   static int cdns_uart_request_port(struct uart_port *port)
>   {
> -	if (!request_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE,
> +	if (!request_mem_region(port->mapbase, port->mapsize,
>   					 CDNS_UART_NAME)) {
>   		return -ENOMEM;
>   	}
>   
> -	port->membase = ioremap(port->mapbase, CDNS_UART_REGISTER_SPACE);
> +	port->membase = ioremap(port->mapbase, port->mapsize);
>   	if (!port->membase) {
>   		dev_err(port->dev, "Unable to map registers\n");
> -		release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
> +		release_mem_region(port->mapbase, port->mapsize);
>   		return -ENOMEM;
>   	}
>   	return 0;
> @@ -1121,7 +1120,7 @@ static int cdns_uart_request_port(struct uart_port *port)
>    */
>   static void cdns_uart_release_port(struct uart_port *port)
>   {
> -	release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
> +	release_mem_region(port->mapbase, port->mapsize);
>   	iounmap(port->membase);
>   	port->membase = NULL;
>   }
> @@ -1780,6 +1779,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
>   	 * and triggers invocation of the config_port() entry point.
>   	 */
>   	port->mapbase = res->start;
> +	port->mapsize = resource_size(res);
>   	port->irq = irq;
>   	port->dev = &pdev->dev;
>   	port->uartclk = clk_get_rate(cdns_uart_data->uartclk);
> 
> ---
> base-commit: 8742b2d8935f476449ef37e263bc4da3295c7b58
> change-id: 20250813-xilinx-uartps-reg-size-c3be67d88b7c
> 
> Best regards,

yes. There is no reason to hardcode it to 0x1000. Only 0x48 is used on our 
silicon. Information about size can be taken from DT.

Acked-by: Michal Simek <michal.simek@amd.com>

Thanks,
Michal

[PATCH] serial: xilinx_uartps: read reg size from DTS

[Harshit Shah]

Current implementation uses `CDNS_UART_REGISTER_SPACE(0x1000)`
for request_mem_region() and ioremap() in cdns_uart_request_port() API.

The cadence/xilinx IP has register space defined from offset 0x0 to 0x48.
It also mentions that the register map is defined as [6:0]. So, the upper
region may/maynot be used based on the IP integration.

Fixes: 1f7055779001 ("arm64: dts: axiado: Add initial support for AX3000 SoC and eval board")
In Axiado AX3000 SoC two UART instances are defined
0x100 apart. That is creating issue in some other instance due to overlap
with addresses.

Since, this address space is already being defined in the
devicetree, use the same when requesting the register space.

Acked-by: Michal Simek <michal.simek@amd.com>
Signed-off-by: Harshit Shah <hshah@axiado.com>
---
- Add fixes tag in commit msg
- Link to v1: https://lore.kernel.org/r/20250819-xilinx-uartps-reg-size-v1-1-0fb7341023fb@axiado.com
---
 drivers/tty/serial/xilinx_uartps.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index fe457bf1e15bb4fc77a5c7de2aea8bfbdbaa643a..a66b44d21fba2558d0b2a62864d86d3b73152e26 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -33,7 +33,6 @@
 #define CDNS_UART_MINOR		0	/* works best with devtmpfs */
 #define CDNS_UART_NR_PORTS	16
 #define CDNS_UART_FIFO_SIZE	64	/* FIFO size */
-#define CDNS_UART_REGISTER_SPACE	0x1000
 #define TX_TIMEOUT		500000
 
 /* Rx Trigger level */
@@ -1098,15 +1097,15 @@ static int cdns_uart_verify_port(struct uart_port *port,
  */
 static int cdns_uart_request_port(struct uart_port *port)
 {
-	if (!request_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE,
+	if (!request_mem_region(port->mapbase, port->mapsize,
 					 CDNS_UART_NAME)) {
 		return -ENOMEM;
 	}
 
-	port->membase = ioremap(port->mapbase, CDNS_UART_REGISTER_SPACE);
+	port->membase = ioremap(port->mapbase, port->mapsize);
 	if (!port->membase) {
 		dev_err(port->dev, "Unable to map registers\n");
-		release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+		release_mem_region(port->mapbase, port->mapsize);
 		return -ENOMEM;
 	}
 	return 0;
@@ -1121,7 +1120,7 @@ static int cdns_uart_request_port(struct uart_port *port)
  */
 static void cdns_uart_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+	release_mem_region(port->mapbase, port->mapsize);
 	iounmap(port->membase);
 	port->membase = NULL;
 }
@@ -1780,6 +1779,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	 * and triggers invocation of the config_port() entry point.
 	 */
 	port->mapbase = res->start;
+	port->mapsize = resource_size(res);
 	port->irq = irq;
 	port->dev = &pdev->dev;
 	port->uartclk = clk_get_rate(cdns_uart_data->uartclk);

---
base-commit: 8742b2d8935f476449ef37e263bc4da3295c7b58
change-id: 20250813-xilinx-uartps-reg-size-c3be67d88b7c

Best regards,
-- 
Harshit Shah <hshah@axiado.com>

Re: [PATCH] serial: xilinx_uartps: read reg size from DTS

[Greg Kroah-Hartman]

On Fri, Aug 22, 2025 at 11:58:14AM -0700, Harshit Shah wrote:
> Current implementation uses `CDNS_UART_REGISTER_SPACE(0x1000)`
> for request_mem_region() and ioremap() in cdns_uart_request_port() API.
> 
> The cadence/xilinx IP has register space defined from offset 0x0 to 0x48.
> It also mentions that the register map is defined as [6:0]. So, the upper
> region may/maynot be used based on the IP integration.
> 
> Fixes: 1f7055779001 ("arm64: dts: axiado: Add initial support for AX3000 SoC and eval board")
> In Axiado AX3000 SoC two UART instances are defined
> 0x100 apart. That is creating issue in some other instance due to overlap
> with addresses.
> 
> Since, this address space is already being defined in the
> devicetree, use the same when requesting the register space.
> 
> Acked-by: Michal Simek <michal.simek@amd.com>
> Signed-off-by: Harshit Shah <hshah@axiado.com>
> ---
> - Add fixes tag in commit msg

That fixes tag needs to go where the signed-off-by area is.  See the
many examples on the lists and in the tree itself for specifics.

> - Link to v1: https://lore.kernel.org/r/20250819-xilinx-uartps-reg-size-v1-1-0fb7341023fb@axiado.com

Then why is this one not marked "v2"?

Can you fix this all up and send a v3?

thanks,

greg k-h

Re: [PATCH] serial: xilinx_uartps: read reg size from DTS

[Harshit Shah]

On 8/22/2025 11:36 PM, Greg Kroah-Hartman wrote:
> Can you fix this all up and send a v3?
>
Apologies for the previous email. I accidentally sent with the HTML.

Thank you Greg for the review. I have fixed the tag and sent v3.

Regards,

Harshit.