linux-kernel - Re: [PATCH v5 6/7] serial: omap: do not override settings for RS485 support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <e1e8d86e-2cb-db8d-77a5-dcb5cd3fbb22@linux.intel.com>
Date:   Wed, 13 Dec 2023 12:26:16 +0200 (EET)
From:   Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To:     Lino Sanfilippo <l.sanfilippo@...bus.com>
cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        u.kleine-koenig@...gutronix.de, shawnguo@...nel.org,
        s.hauer@...gutronix.de, mcoquelin.stm32@...il.com,
        alexandre.torgue@...s.st.com, cniedermaier@...electronics.com,
        hugo@...ovil.com, LKML <linux-kernel@...r.kernel.org>,
        linux-serial <linux-serial@...r.kernel.org>,
        LinoSanfilippo@....de, Lukas Wunner <lukas@...ner.de>,
        p.rosenberger@...bus.com, stable@...r.kernel.org
Subject: Re: [PATCH v5 6/7] serial: omap: do not override settings for RS485
 support

On Sat, 9 Dec 2023, Lino Sanfilippo wrote:

> In serial_omap_rs485() RS485 support may be deactivated due to a missing

There's no serial_omap_rs485() function. I assume/know you meant 
serial_omap_probe_rs485() but please correct.

> RTS GPIO. This is done by nullifying the ports rs485_supported struct.
> After that however the serial_omap_rs485_supported struct is assigned to
> the same structure unconditionally, which results in an unintended
> reactivation of RS485 support.
>
> Fix this by callling serial_omap_rs485() after the assignment of

callling -> calling.

Again, the function name is incorrect.

> rs485_supported.

Wouldn't it be better if all rs485 init/setups would occur in the same 
place rather than being spread around? That is, move the rs485_config and 
rs485_supported setup into serial_omap_probe_rs485()?

-- 
 i.

> Fixes: e2752ae3cfc9 ("serial: omap: Disallow RS-485 if rts-gpio is not specified")
> Cc: stable@...r.kernel.org
> Signed-off-by: Lino Sanfilippo <l.sanfilippo@...bus.com>
> ---
>  drivers/tty/serial/omap-serial.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index ad4c1c5d0a7f..d9b2936308c4 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -1604,10 +1604,6 @@ static int serial_omap_probe(struct platform_device *pdev)
>  		dev_info(up->port.dev, "no wakeirq for uart%d\n",
>  			 up->port.line);
>  
> -	ret = serial_omap_probe_rs485(up, &pdev->dev);
> -	if (ret < 0)
> -		goto err_rs485;
> -
>  	sprintf(up->name, "OMAP UART%d", up->port.line);
>  	up->port.mapbase = mem->start;
>  	up->port.membase = base;
> @@ -1622,6 +1618,10 @@ static int serial_omap_probe(struct platform_device *pdev)
>  			 DEFAULT_CLK_SPEED);
>  	}
>  
> +	ret = serial_omap_probe_rs485(up, &pdev->dev);
> +	if (ret < 0)
> +		goto err_rs485;
> +
>  	up->latency = PM_QOS_CPU_LATENCY_DEFAULT_VALUE;
>  	up->calc_latency = PM_QOS_CPU_LATENCY_DEFAULT_VALUE;
>  	cpu_latency_qos_add_request(&up->pm_qos_request, up->latency);
> -- 
> 2.42.0
> 
>