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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <59692110-eca9-4f0f-b6f9-e23bf835733c@amd.com>
Date: Wed, 20 Aug 2025 08:58:13 +0200
From: Michal Simek <michal.simek@....com>
To: Harshit Shah <hshah@...ado.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Jiri Slaby <jirislaby@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] serial: xilinx_uartps: read reg size from DTS



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@...ado.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@....com>

Thanks,
Michal