Oxford Semiconductor PCIe (Tornado) 950 serial port devices need to 
operate in the enhanced mode via the EFR register for the Divide-by-M 
N/8 baud rate generator prescaler to be used in their native UART mode.  
Otherwise the prescaler is fixed at 1 causing grossly incorrect baud 
rates to be programmed.

Accessing the EFR register requires 16550A features to have been probed 
for, so request this to happen regardless of SERIAL_8250_16550A_VARIANTS 
by setting UPF_FULL_PROBE in port flags.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Reported-by: Anders Blomdell <anders.blomdell@control.lth.se>
Fixes: 366f6c955d4d ("serial: 8250: Add proper clock handling for OxSemi PCIe devices")
Cc: stable@vger.kernel.org # v5.19+
---
 drivers/tty/serial/8250/8250_pci.c |    5 +++++
 1 file changed, 5 insertions(+)

linux-serial-8250-oxsemi-efr.diff
Index: linux-macro/drivers/tty/serial/8250/8250_pci.c
===================================================================
--- linux-macro.orig/drivers/tty/serial/8250/8250_pci.c
+++ linux-macro/drivers/tty/serial/8250/8250_pci.c
@@ -1232,6 +1232,10 @@ static void pci_oxsemi_tornado_set_mctrl
 	serial8250_do_set_mctrl(port, mctrl);
 }
 
+/*
+ * We require EFR features for clock programming, so set UPF_FULL_PROBE
+ * for full probing regardless of CONFIG_SERIAL_8250_16550A_VARIANTS setting.
+ */
 static int pci_oxsemi_tornado_setup(struct serial_private *priv,
 				    const struct pciserial_board *board,
 				    struct uart_8250_port *up, int idx)
@@ -1239,6 +1243,7 @@ static int pci_oxsemi_tornado_setup(stru
 	struct pci_dev *dev = priv->dev;
 
 	if (pci_oxsemi_tornado_p(dev)) {
+		up->port.flags |= UPF_FULL_PROBE;
 		up->port.get_divisor = pci_oxsemi_tornado_get_divisor;
 		up->port.set_divisor = pci_oxsemi_tornado_set_divisor;
 		up->port.set_mctrl = pci_oxsemi_tornado_set_mctrl;

On 17. 09. 22, 12:07, Maciej W. Rozycki wrote:
> Oxford Semiconductor PCIe (Tornado) 950 serial port devices need to
> operate in the enhanced mode via the EFR register for the Divide-by-M
> N/8 baud rate generator prescaler to be used in their native UART mode.
> Otherwise the prescaler is fixed at 1 causing grossly incorrect baud
> rates to be programmed.
> 
> Accessing the EFR register requires 16550A features to have been probed
> for, so request this to happen regardless of SERIAL_8250_16550A_VARIANTS
> by setting UPF_FULL_PROBE in port flags.
> 
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Reported-by: Anders Blomdell <anders.blomdell@control.lth.se>
> Fixes: 366f6c955d4d ("serial: 8250: Add proper clock handling for OxSemi PCIe devices")
> Cc: stable@vger.kernel.org # v5.19+
> ---
>   drivers/tty/serial/8250/8250_pci.c |    5 +++++
>   1 file changed, 5 insertions(+)
> 
> linux-serial-8250-oxsemi-efr.diff
> Index: linux-macro/drivers/tty/serial/8250/8250_pci.c
> ===================================================================
> --- linux-macro.orig/drivers/tty/serial/8250/8250_pci.c
> +++ linux-macro/drivers/tty/serial/8250/8250_pci.c
> @@ -1232,6 +1232,10 @@ static void pci_oxsemi_tornado_set_mctrl
>   	serial8250_do_set_mctrl(port, mctrl);
>   }
>   
> +/*
> + * We require EFR features for clock programming, so set UPF_FULL_PROBE
> + * for full probing regardless of CONFIG_SERIAL_8250_16550A_VARIANTS setting.
> + */

It'd make more sense to me to move this comment right before the line 
you add below.

>   static int pci_oxsemi_tornado_setup(struct serial_private *priv,
>   				    const struct pciserial_board *board,
>   				    struct uart_8250_port *up, int idx)
> @@ -1239,6 +1243,7 @@ static int pci_oxsemi_tornado_setup(stru
>   	struct pci_dev *dev = priv->dev;
>   
>   	if (pci_oxsemi_tornado_p(dev)) {
> +		up->port.flags |= UPF_FULL_PROBE;
>   		up->port.get_divisor = pci_oxsemi_tornado_get_divisor;
>   		up->port.set_divisor = pci_oxsemi_tornado_set_divisor;
>   		up->port.set_mctrl = pci_oxsemi_tornado_set_mctrl;

thanks,
-- 
js
suse labs

On Mon, 19 Sep 2022, Jiri Slaby wrote:

> > linux-serial-8250-oxsemi-efr.diff
> > Index: linux-macro/drivers/tty/serial/8250/8250_pci.c
> > ===================================================================
> > --- linux-macro.orig/drivers/tty/serial/8250/8250_pci.c
> > +++ linux-macro/drivers/tty/serial/8250/8250_pci.c
> > @@ -1232,6 +1232,10 @@ static void pci_oxsemi_tornado_set_mctrl
> >   	serial8250_do_set_mctrl(port, mctrl);
> >   }
> >   +/*
> > + * We require EFR features for clock programming, so set UPF_FULL_PROBE
> > + * for full probing regardless of CONFIG_SERIAL_8250_16550A_VARIANTS
> > setting.
> > + */
> 
> It'd make more sense to me to move this comment right before the line you add
> below.

 I favour the style where what a function does is documented above it, but 
I won't insist on it if having a comment within is what we prefer here.

  Maciej

On Mon, 19 Sep 2022, Maciej W. Rozycki wrote:

> > > linux-serial-8250-oxsemi-efr.diff
> > > Index: linux-macro/drivers/tty/serial/8250/8250_pci.c
> > > ===================================================================
> > > --- linux-macro.orig/drivers/tty/serial/8250/8250_pci.c
> > > +++ linux-macro/drivers/tty/serial/8250/8250_pci.c
> > > @@ -1232,6 +1232,10 @@ static void pci_oxsemi_tornado_set_mctrl
> > >   	serial8250_do_set_mctrl(port, mctrl);
> > >   }
> > >   +/*
> > > + * We require EFR features for clock programming, so set UPF_FULL_PROBE
> > > + * for full probing regardless of CONFIG_SERIAL_8250_16550A_VARIANTS
> > > setting.
> > > + */
> > 
> > It'd make more sense to me to move this comment right before the line you add
> > below.
> 
>  I favour the style where what a function does is documented above it, but 
> I won't insist on it if having a comment within is what we prefer here.

 Having looked at it again I changed my mind and decided it'll be more 
consistent with the rest of the code if this comment remains above the 
function after all.

 My rationale is it is the only function for OxSemi Tornado devices still 
without an introductory comment, the other functions have their internals 
documented solely within their leading comments, and last but not least it 
is obvious what the comment refers to, especially as the function is so 
small (as to fit even in an 80x24 character glass TTY device).

 I have posted v2 with your other suggestions applied.  Thank you for your 
review.

  Maciej