linux-kernel - Re: [RESEND PATCH v4 1/7] serial: Do not hold the port lock when setting rx-during-tx GPIO

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]

Date:   Wed, 22 Nov 2023 12:55:49 +0100
From:   Lino Sanfilippo <l.sanfilippo@...bus.com>
To:     Lino Sanfilippo <l.sanfilippo@...bus.com>,
        gregkh@...uxfoundation.org, jirislaby@...nel.org,
        ilpo.jarvinen@...ux.intel.com
Cc:     u.kleine-koenig@...gutronix.de, shawnguo@...nel.org,
        s.hauer@...gutronix.de, mcoquelin.stm32@...il.com,
        alexandre.torgue@...s.st.com, cniedermaier@...electronics.com,
        linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
        lukas@...ner.de, p.rosenberger@...bus.com, stable@...r.kernel.org,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Hugo Villeneuve <hugo@...ovil.com>
Subject: Re: [RESEND PATCH v4 1/7] serial: Do not hold the port lock when
 setting rx-during-tx GPIO



On 19.11.23 12:28, Lino Sanfilippo wrote:
> Both the imx and stm32 driver set the rx-during-tx GPIO in rs485_config().
> Since this function is called with the port lock held, this can be an
> problem in case that setting the GPIO line can sleep (e.g. if a GPIO
> expander is used which is connected via SPI or I2C).
> 
> Avoid this issue by moving the GPIO setting outside of the port lock into
> the serial core and thus making it a generic feature.
> 
> Since both setting the term and the rx-during-tx GPIO is done at the same
> code place, move it into a common function.

> -				       const struct serial_rs485 *rs485)
> +/*
> + * Set optional RS485 GPIOs for bus termination and data reception during
> + * transmission. These GPIOs are controlled by the serial core independently
> + * from the UART driver.
> + */
> +static void uart_set_rs485_gpios(struct uart_port *port,
> +				 const struct serial_rs485 *rs485)
>  {
>  	if (!(rs485->flags & SER_RS485_ENABLED))
>  		return;
>  
>  	gpiod_set_value_cansleep(port->rs485_term_gpio,
>  				 !!(rs485->flags & SER_RS485_TERMINATE_BUS));
> +	gpiod_set_value_cansleep(port->rs485_rx_during_tx_gpio,
> +				 !!(rs485->flags & SER_RS485_RX_DURING_TX));
>  }
>  

Rasmus is about to add support for another RS485 related GPIO (see 
https://lore.kernel.org/all/20231120151056.148450-3-linux@rasmusvillemoes.dk/ )
which has to be configured both before and after port->rs485_config(). This
does not fit very well with the idea of handling all these GPIOs in
one function. 

So I would like to revise this patch and send an updated version in a v5
of this series in which the suggestion from Hugo
(see https://lore.kernel.org/all/20231011183656.5111ba32ec0c9d43171662a1@hugovil.com/)
is implemented to use separate functions for the GPIO configurations.

Regards,
Lino