Hi,

 A recent change has added a SERIAL_8250_16550A_VARIANTS option, which 
lets one request the 8250 driver not to probe for 16550A device features 
so as to reduce the driver's device startup time in virtual machines.  
This has turned out problematic to a more recent update for the OxSemi 
Tornado series PCIe devices, whose new baud rate generator handling code 
actually requires switching hardware into the enhanced mode for correct 
operation, which actually requires 16550A device features to have been 
probed for.

 This small patch series fixes the issue by letting individual device 
subdrivers to request full 16550A device feature probing by means of a 
flag regardless of the SERIAL_8250_16550A_VARIANTS setting chosen.

 The changes have been verified with an OXPCIe952 device, in the native 
UART mode and a 64-bit RISC-V system as well as in the legacy UART mode 
and a 32-bit x86 system.

 Credit to Anders for reporting this issue and then working through the 
resolution.

  Maciej

On September 17, 2022 11:07:18 AM GMT+01:00, "Maciej W. Rozycki" <macro@orcam.me.uk> wrote:
>Hi,
>
> A recent change has added a SERIAL_8250_16550A_VARIANTS option, which 
>lets one request the 8250 driver not to probe for 16550A device features 
>so as to reduce the driver's device startup time in virtual machines.  
>This has turned out problematic to a more recent update for the OxSemi 
>Tornado series PCIe devices, whose new baud rate generator handling code 
>actually requires switching hardware into the enhanced mode for correct 
>operation, which actually requires 16550A device features to have been 
>probed for.
>
> This small patch series fixes the issue by letting individual device 
>subdrivers to request full 16550A device feature probing by means of a 
>flag regardless of the SERIAL_8250_16550A_VARIANTS setting chosen.
>
> The changes have been verified with an OXPCIe952 device, in the native 
>UART mode and a 64-bit RISC-V system as well as in the legacy UART mode 
>and a 32-bit x86 system.

Seems reasonable to me, as long as the flag is only set by drivers that know they've found their hardware.

On Sat, 17 Sep 2022, Josh Triplett wrote:

> > This small patch series fixes the issue by letting individual device 
> >subdrivers to request full 16550A device feature probing by means of a 
> >flag regardless of the SERIAL_8250_16550A_VARIANTS setting chosen.
> >
> > The changes have been verified with an OXPCIe952 device, in the native 
> >UART mode and a 64-bit RISC-V system as well as in the legacy UART mode 
> >and a 32-bit x86 system.
> 
> Seems reasonable to me, as long as the flag is only set by drivers that 
> know they've found their hardware.

 That has been my intent or otherwise the change would make no sense as 
far as I am concerned.

 In principle for most if not all PCI/e devices we could suppress UART 
probing altogether and still support device's features as we could infer 
the features from the vendor:device ID pair via a table of per-device 
flags.  This might even have worked if we started making one right from 
the beginning as individual devices were added to our 8250/PCI driver.

 Though I can imagine that for some devices no documentation was available 
to the contributor and it could have been hard to determine whether a 
feature actually discovered is really guaranteed for a given vendor:device 
ID or whether there are additional constraints, such as a device revision.  

 I imagine especially early PCI serial port devices may have used discrete 
UART chips behind a piece of PCI glue (just as we now see numerous PCIe 
devices with the actual device placed behind a PCIe-to-PCI bridge onboard) 
and then the set of features could have depended on the specific UART 
chips chosen which may have changed in the course of the life of product.

 At this point however I suspect it would be hard to (re)construct such a 
table and in any case it could have been a maintenance burden, so I guess 
we need to live with what we have.

 Thank you for your input.

  Maciej