Re: [PATCH v5 2/8] serial: 8250: share the RX trigger conversion helper

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3285 bytes --]

On Thu, 29 Oct 2026, James Hilliard wrote:

> Extract the receive-trigger threshold search into serial8250_bytes_to_fcr()
> so drivers with per-port threshold tables can use the same conversion as
> the generic 8250 implementation. Keep the implementation in 8250_port.c and
> export it for subdrivers.

To explain why we're doing this, the text could first mention that 
inability to define these thresholds per driver have been one of the main 
reasons why there has been the temptation to add new port types into 
uart_config[] (that would be better left as a legacy way of defining 
things).

> Include the unsupported-table check in the helper and preserve the existing
> nearest-lower selection and highest-encoding fallback.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>  drivers/tty/serial/8250/8250.h      |  2 ++
>  drivers/tty/serial/8250/8250_port.c | 21 +++++++++++++--------
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> index 21ed9c41b751..1600f2e45d76 100644
> --- a/drivers/tty/serial/8250/8250.h
> +++ b/drivers/tty/serial/8250/8250.h
> @@ -73,6 +73,8 @@ struct serial8250_config {
>  	unsigned int	flags;
>  };
>  
> +int serial8250_bytes_to_fcr(const unsigned char *rxtrig_bytes, unsigned char bytes);
> +
>  enum uart_port_type {
>  	/* Internal 8250 only */
>  	UART_PORT_AIROHA		= 124,
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 36b2b0a2c05c..fc3987b5a70c 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -3015,22 +3015,27 @@ static int fcr_get_rxtrig_bytes(struct uart_8250_port *up)
>  	return bytes ? bytes : -EOPNOTSUPP;
>  }
>  
> -static int bytes_to_fcr_rxtrig(struct uart_8250_port *up, unsigned char bytes)
> +int serial8250_bytes_to_fcr(const unsigned char *rxtrig_bytes, unsigned char bytes)
>  {
> -	const struct serial8250_config *conf_type = &uart_config[up->port.type];
> -	int i;
> -
> -	if (!conf_type->rxtrig_bytes[UART_FCR_R_TRIG_BITS(UART_FCR_R_TRIG_00)])
> +	if (!rxtrig_bytes[UART_FCR_R_TRIG_BITS(UART_FCR_R_TRIG_00)])
>  		return -EOPNOTSUPP;
>  
> -	for (i = 1; i < UART_FCR_R_TRIG_MAX_STATE; i++) {
> -		if (bytes < conf_type->rxtrig_bytes[i])
> +	for (unsigned int i = 1; i < UART_FCR_R_TRIG_MAX_STATE; i++) {
> +		if (bytes < rxtrig_bytes[i])
>  			/* Use the nearest lower value */
> -			return (--i) << UART_FCR_R_TRIG_SHIFT;
> +			return (i - 1) << UART_FCR_R_TRIG_SHIFT;
>  	}
>  
>  	return UART_FCR_R_TRIG_11;
>  }
> +EXPORT_SYMBOL_GPL(serial8250_bytes_to_fcr);

Put the export to namespace.

With that fixed,

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

> +static int bytes_to_fcr_rxtrig(struct uart_8250_port *up, unsigned char bytes)
> +{
> +	const struct serial8250_config *conf_type = &uart_config[up->port.type];
> +
> +	return serial8250_bytes_to_fcr(conf_type->rxtrig_bytes, bytes);
> +}
>  
>  static int do_get_rxtrig(struct tty_port *port)
>  {
> 
> 

-- 
 i.