Re: [PATCH v5 12/14] serial: 8250: allow UART drivers to override rx_trig_bytes handling

On Sun, Aug 02, 2026 at 12:20:18PM +0300, Andy Shevchenko wrote:
> On Fri, Jul 31, 2026 at 10:49 AM Crescent Hsieh
> <[email protected]> wrote:
> >
> > The rx_trig_bytes sysfs attribute currently relies on 8250-internal
> > helper functions and assumes a fixed mapping between trigger levels and
> > FIFO behavior.
> >
> > Some UARTs provide hardware-specific RX trigger mechanisms that do not
> > fit this model. Add optional uart_port callbacks for setting and getting
> > the RX trigger level, and use them when provided, while preserving the
> > existing 8250 helpers as the default fallback.
> 
> ...
> 
> >         struct tty_port *port = dev_get_drvdata(dev);
> > +       struct uart_state *state = container_of(port, struct uart_state, port);
> > +       struct uart_port *uport = state->uart_port;
> >         int rxtrig_bytes;
> 
> Hmm... do_set_rxtrig() and do_get_rxtrig() do the same, but under the
> mutex lock. Hence there are questions:
> - is it correct to have without mutex?
> - if so, why not to propagate these (if you really need it), to the callees?
> - or should these callbacks be integrated in the lower level?
> 
> > -       rxtrig_bytes = do_serial8250_get_rxtrig(port);
> > +       if (uport->get_rxtrig)
> > +               rxtrig_bytes = uport->get_rxtrig(uport);
> > +       else
> > +               rxtrig_bytes = do_serial8250_get_rxtrig(port);

Yes, the callback path should be protected by the same mutex.

I agree that the callback dispatch should be moved into the lower-level
helpers rather than handled directly in rx_trig_bytes_show() and
rx_trig_bytes_store().

I also think the function names should be aligned with the naming
convention used by the other generic 8250 helpers for consistency.

So the resulting call  flow would be like:

		      rx_trig_bytes_store/show()
				  |
				  v
		     serial8250_set/get_rxtrig()
	                          |
	  +-----------------------+-----------------------+
	  |						  |
	  v						  v
uport->set/get_rxtrig()                  serial8250_do_set/get_rxtrig()

--
Thanks,
Crescent Hsieh