Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <a2b54b76-01ce-4aa2-a00b-e65e123ba7e9@kunbus.com>
Date:   Wed, 22 Nov 2023 12:55:49 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [RESEND PATCH v4 1/7] serial: Do not hold the port lock when
 setting rx-during-tx GPIO
Content-Language: en-US
To:     Lino Sanfilippo <l.sanfilippo@kunbus.com>,
        gregkh@linuxfoundation.org, jirislaby@kernel.org,
        ilpo.jarvinen@linux.intel.com
Cc:     u.kleine-koenig@pengutronix.de, shawnguo@kernel.org,
        s.hauer@pengutronix.de, mcoquelin.stm32@gmail.com,
        alexandre.torgue@foss.st.com, cniedermaier@dh-electronics.com,
        linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
        lukas@wunner.de, p.rosenberger@kunbus.com, stable@vger.kernel.org,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Hugo Villeneuve <hugo@hugovil.com>
References: <20231119112856.11587-1-l.sanfilippo@kunbus.com>
 <20231119112856.11587-2-l.sanfilippo@kunbus.com>
From:   Lino Sanfilippo <l.sanfilippo@kunbus.com>
In-Reply-To: <20231119112856.11587-2-l.sanfilippo@kunbus.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bmVUUi84ajRnSWNIWktQSEo3MUNBOWd4M1l1eEtlRkVQRDA1L3JIN1dLQy9B?=
 =?utf-8?B?WTY3WGI1K3Y4T2ZhTWJpZWRERi92QXAxZEtTWmN4SkNQSy81VEpMMERWazRm?=
 =?utf-8?B?K0RlbkhNZEMyTzR5ZUtIakM2d0hlWUYvUFZ2eHNWOTluN0JZelNmSXdQSnJZ?=
 =?utf-8?B?WHg3c2JIbXVQdUdOc2dEeEN6WE1zcnRvME9BY2NEQWhaUmp0SWNGb0prWS9i?=
 =?utf-8?B?b0JCK0RQU1I2T2Yvb1VXdjJJbHFQa05yNm80b2J6NldoVk0reXJXK1Mwbjhv?=
 =?utf-8?B?akxncFZmRDlqbEJ1bVlzMWYzbFhMZ1FTYmhXRS90dmg2ZzN5K21BYUZFbFBK?=
 =?utf-8?B?dXp4eTNxQkhQM040NzVXK0ZCN1dVY2graTU0LzA2clpwQjFvTWkyR1VQNDd4?=
 =?utf-8?B?RGFiYU5ld0NSK0VVdjYwd2JTUlloNlIvSHNhdFR3RFUzajBEb2I4Wmx3dVpJ?=
 =?utf-8?B?RjI3WmFuWjhjREdjTG1ERW5yaThQWkM3Ky91NE5WYURhNUJBWTdmOGJyYXBF?=
 =?utf-8?B?ZDA3WGliOU1RS1hhQTFOUEVzSEJTZEFOSnZnd2VsQmNtYktxV2NmbHdvK3hC?=
 =?utf-8?B?aXM4U3JQRythcjZvZVNEazI1ZlNQOENVMHJyRUU0TFI0ZFB0bElYaVRVdzl6?=
 =?utf-8?B?WlAwOXYwWXk3N1lsOHM4MS9FeWJsMGR6STNtc1hoTXY0RUd4MVVmTlRaS09i?=
 =?utf-8?B?Q01PRmtCY2xWZnp4dzhpV2R4emlOMUx0RSt5eUhOY0g1eWgvNk9Jck1vd1Jn?=
 =?utf-8?B?T2JDUG5jWkR1SEtuNjlBcnRoRmtsaHNPWW9RSjlSZWpDNHJLYWVVV0FSM1da?=
 =?utf-8?B?TURtZVpiTDlVNFg4TFJkdGFtNk11Y3FCMDBNYkw5TVUyRlhuY2d6Ti9iM1pv?=
 =?utf-8?B?V002elczSzVvOUZXSWYrMnBFam0wYlE3MHVWUjdzUGw1NTd6MFExWDMrM1gz?=
 =?utf-8?B?aXhNWW1YM29ndjlnckRUTVYxWFA4RUpFWFVpcm56K1QwcjNtUXUwdkhnNWF6?=
 =?utf-8?B?VHZMVXZhbVRUVUpYaWp4OWNNWmx0Nk56OGI2T0JUek5vbkQwMmJCNG9JZ3pT?=
 =?utf-8?B?OVJ4cFFoRXdXYWxLdWNmNjk4R0ZxYjBrUWtYUHYyYjhRc1RIZyt5NDkwMEcv?=
 =?utf-8?B?dXUrd0hQMzZnRmJ5ZHI5ekFySkNFNFhOYTFmcDZOVWxqcDB0NG43cEM2L1Jw?=
 =?utf-8?B?NkNYKzdlK1doeDJiM3pTWGhBeXdWeC9qeENlY2ViOWJwVmFiN1lXMTZ2ZWFI?=
 =?utf-8?B?NzhEaEtTNkpKaksvWmZYZXNKOFZGczJNdjgyRk90U3N2RzFkM0dNWTgzQ1V4?=
 =?utf-8?B?SElZbjRUR3Y4c25NL3VFbjZ6NE1jSS9BdHRMbjREdSsycUN0cjdZVlEwNHMr?=
 =?utf-8?B?ejlldlVPZVl0anBqKzdnb3psYUxveUMwckJBZjJTZGp3S0xYN3huMSt0N2xq?=
 =?utf-8?B?YlQ0OFhGSlFNRy9BYUZTanA4bjRONGlMcmZRUUtmWXNYazN1ditlbWVLWTc4?=
 =?utf-8?B?a2dUWmEzeGZlOVhSQkw2cmg5M1dHRUpneDVHOWt6NXMra3ExNkNVd1NmcE5F?=
 =?utf-8?B?Q1BGU1BGTDhYWHdXN2NLL0NBb1hqczQ0TUo2QTg0cVhaa0xhL3Y2SFh5aE1G?=
 =?utf-8?B?WXNmTkN5TkdPUjBSRGRMV05lY3FkTkg3N1VRTUEzK1VZZU9MNzFnZHZpTHMw?=
 =?utf-8?B?dUMrNmNUVmd6YzN6MTJnQi9mSWxqbkR1engzd2FKRUhrdDlxVGRyUDlpK01L?=
 =?utf-8?B?dlpqVGRYMkdERHFMSnBQTy9UaGR3dzFYa0VackM2UWdXYWRmR2g5OER6Vytn?=
 =?utf-8?B?ZXVFeEF4QjA1YlNacFRYaFBOZjd3OU1kVWpiMFZuS3Z2ckJ3TlNPcHlnek1M?=
 =?utf-8?B?NmtJa1BXU21TTjAyUUdLTlRiRFY5ckU0TldkUUp6TXhLc24xYk9NeXZQMHdK?=
 =?utf-8?B?M1RQbmY5Q0lWN2VHVSsvNGNSRWpqLzB6VWw2VWlwRUM1ZTY0SUZmMVRuT2xS?=
 =?utf-8?B?ZWpnTnM5aEJVRjloamFKRWRvN0hyZHcyZnVnSG8wamNGTXU2RDAwdHRwOVZk?=
 =?utf-8?B?LzJYTlpGcW1WbGprTjdqNHQ2YTBGaGtpaDZFZDhzbWcwNkFUbmV6dVRNbUhU?=
 =?utf-8?B?THMyL3hYSVNnbFVXdHBOckJYQmZRaWVwMUxrczlsYkwyTmRULzdkdVVORW4v?=
 =?utf-8?Q?nElebTolCb05y027JlSNR+2PlLt+s475tRaZNxx5FVyc?=
X-OriginatorOrg: kunbus.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3198fe32-ef41-45c1-0acf-08dbeb51f9d6
X-MS-Exchange-CrossTenant-AuthSource: VI1P193MB0413.EURP193.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Nov 2023 11:55:52.1313
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: aaa4d814-e659-4b0a-9698-1c671f11520b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: yFKH903Erm6MHz98StTZEyK1yCGHIaGpsfKunYuhlZ0EI/boCpX9/UCJ/2feEHJscf8lz1a9Sbt5rX6taa8vMQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXP193MB1904
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Wed, 22 Nov 2023 03:56:48 -0800 (PST)


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