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

Michal Simek
2025-08-20T06:58:18Z

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