kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Anders Blomdell]

Reverting commits
   366f6c955d4d1a5125ffcd6875ead26a3c7a2a1c
   9c5c8aaed50bf3478073ab51b8b1f3f5327d3cfa
fixes my problems.

Regards

Anders Blomdell
-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Greg Kroah-Hartman]

On Mon, Sep 12, 2022 at 09:59:08PM +0200, Anders Blomdell wrote:
> Reverting commits
>   366f6c955d4d1a5125ffcd6875ead26a3c7a2a1c
>   9c5c8aaed50bf3478073ab51b8b1f3f5327d3cfa
> fixes my problems.

What problems exactly?

And why not cc: the developers of those commits?

thanks,

greg k-h

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Anders Blomdell]

I get incorrect baudrates, my oscilloscope gives:

Programmed	Measured

   2400		  5208
   4800		 13150
   9600		 10410
  19200		 71420
  38400		142000
  57600		201600
115200		138800


On 2022-09-13 10:21, Greg Kroah-Hartman wrote:
> On Mon, Sep 12, 2022 at 09:59:08PM +0200, Anders Blomdell wrote:
>> Reverting commits
>>    366f6c955d4d1a5125ffcd6875ead26a3c7a2a1c
>>    9c5c8aaed50bf3478073ab51b8b1f3f5327d3cfa
>> fixes my problems.
> 
> What problems exactly?
> 
> And why not cc: the developers of those commits?
> 
> thanks,
> 
> greg k-h

-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Greg Kroah-Hartman]

On Tue, Sep 13, 2022 at 02:16:39PM +0200, Anders Blomdell wrote:
> I get incorrect baudrates, my oscilloscope gives:
> 
> Programmed	Measured
> 
>   2400		  5208
>   4800		 13150
>   9600		 10410
>  19200		 71420
>  38400		142000
>  57600		201600
> 115200		138800

I'm sorry, I have no context here at all, what does this pertain to?

confused,

greg k-h

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Anders Blomdell]

On 2022-09-13 14:30, Greg Kroah-Hartman wrote:
> On Tue, Sep 13, 2022 at 02:16:39PM +0200, Anders Blomdell wrote:
>> I get incorrect baudrates, my oscilloscope gives:
>>
>> Programmed	Measured
>>
>>    2400		  5208
>>    4800		 13150
>>    9600		 10410
>>   19200		 71420
>>   38400		142000
>>   57600		201600
>> 115200		138800
> 
> I'm sorry, I have no context here at all, what does this pertain to?
Programmed baudrate and the measured (actual) baudrate
> 
> confused,
> 
> greg k-h

/Anders
-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Greg Kroah-Hartman]

On Tue, Sep 13, 2022 at 02:43:03PM +0200, Anders Blomdell wrote:
> 
> 
> On 2022-09-13 14:30, Greg Kroah-Hartman wrote:
> > On Tue, Sep 13, 2022 at 02:16:39PM +0200, Anders Blomdell wrote:
> > > I get incorrect baudrates, my oscilloscope gives:
> > > 
> > > Programmed	Measured
> > > 
> > >    2400		  5208
> > >    4800		 13150
> > >    9600		 10410
> > >   19200		 71420
> > >   38400		142000
> > >   57600		201600
> > > 115200		138800
> > 
> > I'm sorry, I have no context here at all, what does this pertain to?
> Programmed baudrate and the measured (actual) baudrate

I really don't know what to do here, sorry.  Are you saying a specific
commit has broken this?  If so, did you test if you made a change it
fixed the issue?

What do you suggest happen here?

still confused,

greg k-h

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Anders Blomdell]

On 2022-09-13 16:01, Greg Kroah-Hartman wrote:
> On Tue, Sep 13, 2022 at 02:43:03PM +0200, Anders Blomdell wrote:
>>
>>
>> On 2022-09-13 14:30, Greg Kroah-Hartman wrote:
>>> On Tue, Sep 13, 2022 at 02:16:39PM +0200, Anders Blomdell wrote:
>>>> I get incorrect baudrates, my oscilloscope gives:
>>>>
>>>> Programmed	Measured
>>>>
>>>>     2400		  5208
>>>>     4800		 13150
>>>>     9600		 10410
>>>>    19200		 71420
>>>>    38400		142000
>>>>    57600		201600
>>>> 115200		138800
>>>
>>> I'm sorry, I have no context here at all, what does this pertain to?
>> Programmed baudrate and the measured (actual) baudrate
> 
> I really don't know what to do here, sorry.  Are you saying a specific
> commit has broken this?  If so, did you test if you made a change it
> fixed the issue?

Yes, commit 366f6c955d4d1a5125ffcd6875ead26a3c7a2a1c broke the one to one correspondence
between programmed and actual baudrate; reverting it (and 9c5c8aaed50bf3478073ab51b8b1f3f5327d3cfa
that builds on that patch) restores the expected functionality (i.e. you get the baudrate you ask for)
on 5.19.8.

> What do you suggest happen here?
Either there is a bug in the code, or the chipset on my card (a Delock 2xRS232 card) is not a true oxford
chipset (the package and PCI id's says that they are).

Since the chip seems to be discontinued since 2014 (see https://www.mouser.com/PCN/PLX_Technology_2013_8.pdf),
I think a revert would not be uncalled for.

> 
> still confused,
So am I

> 
> greg k-h

/Anders
-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Maciej W. Rozycki]

Hi Anders,

 Sorry to cause you trouble.

> > I really don't know what to do here, sorry.  Are you saying a specific
> > commit has broken this?  If so, did you test if you made a change it
> > fixed the issue?
> 
> Yes, commit 366f6c955d4d1a5125ffcd6875ead26a3c7a2a1c broke the one to one
> correspondence
> between programmed and actual baudrate; reverting it (and
> 9c5c8aaed50bf3478073ab51b8b1f3f5327d3cfa
> that builds on that patch) restores the expected functionality (i.e. you get
> the baudrate you ask for)
> on 5.19.8.

 I have implemented the calculation using parameters from original OxSemi 
datasheets and verified this code across three architectures available to 
me (POWER9, RISC-V, 32-bit x86) and a couple of cards made by different 
manufacturers connected to a non-OxSemi serial device each at the other 
end.  I have checked the usual baud rates of up to 460800bps.  Higher baud 
rates work too, though I could only try them between OxSemi devices, so 
actual rates were not verified for correctness.

 And offhand I can say it works just fine at 230400bps with 6.0.0-rc2 as 
at commit 10d4879f9ef0 and my RISC-V machine (using an EXSYS EX-44171 
1S+1P port PCIe option card, built around the OXPCIe952 too) talking to a 
remote console server in my lab.  I don't have an oscilloscope available 
to check actual waveforms produced.

> > What do you suggest happen here?
> Either there is a bug in the code, or the chipset on my card (a Delock 2xRS232
> card) is not a true oxford
> chipset (the package and PCI id's says that they are).

 A bug can never be ruled out.  I doubt that Delock would use a fake chip 
or indeed that anyone would choose to clone an OxSemi part, which seems 
fairly complex to me for a serial port.

> Since the chip seems to be discontinued since 2014 (see
> https://www.mouser.com/PCN/PLX_Technology_2013_8.pdf),
> I think a revert would not be uncalled for.

 The problem is the original calculation is inaccurate enough for the 
serial interface not to communicate correctly at higher baud rates.  I 
found setting two stop bits while talking to a remote end that has one 
stop bit set a possible workaround for some cases, but why not do the 
calculation correctly in the first place?

 If you're willing to debug it, then I'll be more than happy to supply you 
with diagnostic patches, some of which I made in the development of the 
fix.  Also what processor architecture do you use the interface with?

  Maciej

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Anders Blomdell]

On 2022-09-13 18:19, Maciej W. Rozycki wrote:
> Hi Anders,
> 
>   Sorry to cause you trouble.
> 
>>> I really don't know what to do here, sorry.  Are you saying a specific
>>> commit has broken this?  If so, did you test if you made a change it
>>> fixed the issue?
>>
>> Yes, commit 366f6c955d4d1a5125ffcd6875ead26a3c7a2a1c broke the one to one
>> correspondence
>> between programmed and actual baudrate; reverting it (and
>> 9c5c8aaed50bf3478073ab51b8b1f3f5327d3cfa
>> that builds on that patch) restores the expected functionality (i.e. you get
>> the baudrate you ask for)
>> on 5.19.8.
> 
>   I have implemented the calculation using parameters from original OxSemi
> datasheets and verified this code across three architectures available to
> me (POWER9, RISC-V, 32-bit x86) and a couple of cards made by different
> manufacturers connected to a non-OxSemi serial device each at the other
> end.  I have checked the usual baud rates of up to 460800bps.  Higher baud
> rates work too, though I could only try them between OxSemi devices, so
> actual rates were not verified for correctness.
> 
>   And offhand I can say it works just fine at 230400bps with 6.0.0-rc2 as
> at commit 10d4879f9ef0 and my RISC-V machine (using an EXSYS EX-44171
> 1S+1P port PCIe option card, built around the OXPCIe952 too) talking to a
> remote console server in my lab.  I don't have an oscilloscope available
> to check actual waveforms produced.
I have the problem with 5.19.8 on x86_64

> 
>>> What do you suggest happen here?
>> Either there is a bug in the code, or the chipset on my card (a Delock 2xRS232
>> card) is not a true oxford
>> chipset (the package and PCI id's says that they are).
> 
>   A bug can never be ruled out.  I doubt that Delock would use a fake chip
> or indeed that anyone would choose to clone an OxSemi part, which seems
> fairly complex to me for a serial port.
> 
>> Since the chip seems to be discontinued since 2014 (see
>> https://www.mouser.com/PCN/PLX_Technology_2013_8.pdf),
>> I think a revert would not be uncalled for.
> 
>   The problem is the original calculation is inaccurate enough for the
> serial interface not to communicate correctly at higher baud rates.  I
> found setting two stop bits while talking to a remote end that has one
> stop bit set a possible workaround for some cases, but why not do the
> calculation correctly in the first place?
> 
>   If you're willing to debug it, then I'll be more than happy to supply you
> with diagnostic patches, some of which I made in the development of the
> fix.  
Yes, please

> Also what processor architecture do you use the interface with?
The delock card is in an x86 machine, and it communicates with various RS232 devices
(hence speeds above 230400 usually works badly with a few meters of cabling)

The problem I have, is that the baudrate is very much off at the lower ranges I'm interested in
(2400-115200 mostly), even though the register values seems to make sense.

/Anders

-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Anders Blomdell]

I honestly don't understand how to calculate the  baudrate, the following hack:

         switch (baud) {
           case 9601: { tcr = 4;  cpr = 32; quot = 406; } break;
           case 9602: { tcr = 8;  cpr = 16; quot = 406; } break;
           case 9603: { tcr = 16; cpr = 8;  quot = 406; } break; /* tcr will be masked to 0 */
         }
	*frac = (cpr << 8) | (tcr & OXSEMI_TORNADO_TCR_MASK);
         
I believed that they should give the same baudrate, but I get these baudrates:

9600 -> 10700   tcr = 9;  cpr = 9;  quot = 643;    /* Use standard calculations, not hack */
9601 -> 38400	tcr = 4;  cpr = 32; quot = 406;
9602 -> 19200	tcr = 8;  cpr = 16; quot = 406
9603 ->  9600	tcr = 16; cpr = 8;  quot = 406;


On 2022-09-13 18:19, Maciej W. Rozycki wrote:
> Hi Anders,
> 
>   Sorry to cause you trouble.
> 
>>> I really don't know what to do here, sorry.  Are you saying a specific
>>> commit has broken this?  If so, did you test if you made a change it
>>> fixed the issue?
>>
>> Yes, commit 366f6c955d4d1a5125ffcd6875ead26a3c7a2a1c broke the one to one
>> correspondence
>> between programmed and actual baudrate; reverting it (and
>> 9c5c8aaed50bf3478073ab51b8b1f3f5327d3cfa
>> that builds on that patch) restores the expected functionality (i.e. you get
>> the baudrate you ask for)
>> on 5.19.8.
> 
>   I have implemented the calculation using parameters from original OxSemi
> datasheets and verified this code across three architectures available to
> me (POWER9, RISC-V, 32-bit x86) and a couple of cards made by different
> manufacturers connected to a non-OxSemi serial device each at the other
> end.  I have checked the usual baud rates of up to 460800bps.  Higher baud
> rates work too, though I could only try them between OxSemi devices, so
> actual rates were not verified for correctness.
> 
>   And offhand I can say it works just fine at 230400bps with 6.0.0-rc2 as
> at commit 10d4879f9ef0 and my RISC-V machine (using an EXSYS EX-44171
> 1S+1P port PCIe option card, built around the OXPCIe952 too) talking to a
> remote console server in my lab.  I don't have an oscilloscope available
> to check actual waveforms produced.
> 
>>> What do you suggest happen here?
>> Either there is a bug in the code, or the chipset on my card (a Delock 2xRS232
>> card) is not a true oxford
>> chipset (the package and PCI id's says that they are).
> 
>   A bug can never be ruled out.  I doubt that Delock would use a fake chip
> or indeed that anyone would choose to clone an OxSemi part, which seems
> fairly complex to me for a serial port.
> 
>> Since the chip seems to be discontinued since 2014 (see
>> https://www.mouser.com/PCN/PLX_Technology_2013_8.pdf),
>> I think a revert would not be uncalled for.
> 
>   The problem is the original calculation is inaccurate enough for the
> serial interface not to communicate correctly at higher baud rates.  I
> found setting two stop bits while talking to a remote end that has one
> stop bit set a possible workaround for some cases, but why not do the
> calculation correctly in the first place?
> 
>   If you're willing to debug it, then I'll be more than happy to supply you
> with diagnostic patches, some of which I made in the development of the
> fix.  Also what processor architecture do you use the interface with?
> 
>    Maciej

-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Anders Blomdell]

Seems like CPR/CPR2 does not affect baudrate, so maybe EFR bit 4 (enhance mode) is not set?
If CPR is stuck at 8 (1.0 scaling), it all makes sense, these corresponds to what the oscilloscope gives:

2400 ->  tcr: 9, cpr: 18, quot: 1286
          62500000/9/(8*.125)/1286 -> 5400
4800 -> tcr: 7, cpr: 23, quot: 647
	2500000/7/(8*.125)/647 -> 13799
9600 -> tcr: 9, cpr: 9, quot: 643
         62500000/9/(8*.125)/643 -> 10800.
19200 -> tcr: 8, cpr: 31, quot: 105
         62500000/7/(8*.125)/105 -> 85034
38400 -> 62500000/14/(8*.125)/30 -> 148809

/Anders

On 2022-09-13 18:19, Maciej W. Rozycki wrote:
> Hi Anders,
> 
>   Sorry to cause you trouble.
> 
>>> I really don't know what to do here, sorry.  Are you saying a specific
>>> commit has broken this?  If so, did you test if you made a change it
>>> fixed the issue?
>>
>> Yes, commit 366f6c955d4d1a5125ffcd6875ead26a3c7a2a1c broke the one to one
>> correspondence
>> between programmed and actual baudrate; reverting it (and
>> 9c5c8aaed50bf3478073ab51b8b1f3f5327d3cfa
>> that builds on that patch) restores the expected functionality (i.e. you get
>> the baudrate you ask for)
>> on 5.19.8.
> 
>   I have implemented the calculation using parameters from original OxSemi
> datasheets and verified this code across three architectures available to
> me (POWER9, RISC-V, 32-bit x86) and a couple of cards made by different
> manufacturers connected to a non-OxSemi serial device each at the other
> end.  I have checked the usual baud rates of up to 460800bps.  Higher baud
> rates work too, though I could only try them between OxSemi devices, so
> actual rates were not verified for correctness.
> 
>   And offhand I can say it works just fine at 230400bps with 6.0.0-rc2 as
> at commit 10d4879f9ef0 and my RISC-V machine (using an EXSYS EX-44171
> 1S+1P port PCIe option card, built around the OXPCIe952 too) talking to a
> remote console server in my lab.  I don't have an oscilloscope available
> to check actual waveforms produced.
> 
>>> What do you suggest happen here?
>> Either there is a bug in the code, or the chipset on my card (a Delock 2xRS232
>> card) is not a true oxford
>> chipset (the package and PCI id's says that they are).
> 
>   A bug can never be ruled out.  I doubt that Delock would use a fake chip
> or indeed that anyone would choose to clone an OxSemi part, which seems
> fairly complex to me for a serial port.
> 
>> Since the chip seems to be discontinued since 2014 (see
>> https://www.mouser.com/PCN/PLX_Technology_2013_8.pdf),
>> I think a revert would not be uncalled for.
> 
>   The problem is the original calculation is inaccurate enough for the
> serial interface not to communicate correctly at higher baud rates.  I
> found setting two stop bits while talking to a remote end that has one
> stop bit set a possible workaround for some cases, but why not do the
> calculation correctly in the first place?
> 
>   If you're willing to debug it, then I'll be more than happy to supply you
> with diagnostic patches, some of which I made in the development of the
> fix.  Also what processor architecture do you use the interface with?
> 
>    Maciej

-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Maciej W. Rozycki]

On Tue, 13 Sep 2022, Anders Blomdell wrote:

> Seems like CPR/CPR2 does not affect baudrate, so maybe EFR bit 4 (enhance
> mode) is not set?

 Thanks for assessing the problem.  I've thought already it could be 
misconfiguration.

> If CPR is stuck at 8 (1.0 scaling), it all makes sense, these corresponds to
> what the oscilloscope gives:
> 
> 2400 ->  tcr: 9, cpr: 18, quot: 1286
>          62500000/9/(8*.125)/1286 -> 5400
> 4800 -> tcr: 7, cpr: 23, quot: 647
> 	2500000/7/(8*.125)/647 -> 13799
> 9600 -> tcr: 9, cpr: 9, quot: 643
>         62500000/9/(8*.125)/643 -> 10800.
> 19200 -> tcr: 8, cpr: 31, quot: 105
>         62500000/7/(8*.125)/105 -> 85034
> 38400 -> 62500000/14/(8*.125)/30 -> 148809

 Agreed.

 As the first debug aid could you please enable DEBUG_AUTOCONF at the top 
of drivers/tty/serial/8250/8250_port.c and paste the relevant piece of 
8250 initialisation recorded in the kernel log?  This will confirm (or 
contradict) correct operation of the port configuration sequence.

 Also I have identified a code piece that handles the EFR in a destructive 
manner, which I must have previously missed, namely a conditional block 
guarded by UART_CAP_EFR in `serial8250_do_set_termios'.  It should likely 
be fixed, however it is supposed not to matter for OxSemi chips due to:

		/* UART_CAP_EFR breaks billionon CF bluetooth card. */
		.flags		= UART_CAP_FIFO | UART_CAP_SLEEP,

which then leads to:

serial 0000:07:00.3: detected caps 00000700 should be 00000500

and consequently UART_CAP_EFR gets cleared and this code block isn't 
supposed to be reached.  Can you confirm the presence of a similar message 
in your log?

 NB it seems to me too big a hammer to have a generic serial port feature 
globally disabled to work around an unidentified problem with an attached 
particular serial device.  Pavel, as the originator of commit d0694e2aeb81 
("serial: unbreak billionton CF card") can you please explain what the 
motivation was here?

 I could only track down two message threads related to the problem:

<https://lore.kernel.org/lkml/20110106134254.68fa27ac@lxorguk.ukuu.org.uk/>

and:

<https://lore.kernel.org/linux-serial/4D001AF1.80902@mainpine.com/>

but no attempt to actually narrow the issue down (also ISTM like a feature 
such as flow control ought to be controlled via a termios call rather than 
globally disabled).  Also could the corruption of the EFR in what is now 
`serial8250_do_set_termios' (and used to be `serial8250_set_termios' then) 
mentioned above be the culprit?

  Maciej

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Anders Blomdell]

On 2022-09-14 02:00, Maciej W. Rozycki wrote:
> On Tue, 13 Sep 2022, Anders Blomdell wrote:
> 
>> Seems like CPR/CPR2 does not affect baudrate, so maybe EFR bit 4 (enhance
>> mode) is not set?
> 
>   Thanks for assessing the problem.  I've thought already it could be
> misconfiguration.
> 
>> If CPR is stuck at 8 (1.0 scaling), it all makes sense, these corresponds to
>> what the oscilloscope gives:
>>
>> 2400 ->  tcr: 9, cpr: 18, quot: 1286
>>           62500000/9/(8*.125)/1286 -> 5400
>> 4800 -> tcr: 7, cpr: 23, quot: 647
>> 	2500000/7/(8*.125)/647 -> 13799
>> 9600 -> tcr: 9, cpr: 9, quot: 643
>>          62500000/9/(8*.125)/643 -> 10800.
>> 19200 -> tcr: 8, cpr: 31, quot: 105
>>          62500000/7/(8*.125)/105 -> 85034
>> 38400 -> 62500000/14/(8*.125)/30 -> 148809
> 
>   Agreed.
> 
>   As the first debug aid could you please enable DEBUG_AUTOCONF at the top
> of drivers/tty/serial/8250/8250_port.c and paste the relevant piece of
> 8250 initialisation recorded in the kernel log?  This will confirm (or
> contradict) correct operation of the port configuration sequence.

Modifications done:

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index c89cb881d9b0..f25e824d626e 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -115,11 +115,17 @@ struct serial8250_config {
  
  static inline int serial_in(struct uart_8250_port *up, int offset)
  {
-	return up->port.serial_in(&up->port, offset);
+       int result;
+	result =  up->port.serial_in(&up->port, offset);
+       printk(KERN_INFO "serial_in(%px, 0x%02x) -> 0x%02x\n",
+              up, offset, result);
+       return result;
  }
  
  static inline void serial_out(struct uart_8250_port *up, int offset, int value)
  {
+       printk(KERN_INFO "serial_out(%px, 0x%02x, 0x%02x)\n",
+              up, offset, value);
  	up->port.serial_out(&up->port, offset, value);
  }
  
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 2b86c55ed374..b5c1b1faff34 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -44,7 +44,7 @@
  /*
   * Debugging.
   */
-#if 0
+#if 1
  #define DEBUG_AUTOCONF(fmt...)	printk(fmt)
  #else
  #define DEBUG_AUTOCONF(fmt...)	do { } while (0)


Captured data:


[    3.830006] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[    3.830141] ttyS0: autoconf (0x03f8, 0x0000000000000000):
[    3.830146] serial_in(ffffffffb6b97520, 0x01) -> 0xff
[    3.830271] serial_out(ffffffffb6b97520, 0x01, 0x00)
[    3.830338] serial_in(ffffffffb6b97520, 0x01) -> 0xff
[    3.830400] serial_out(ffffffffb6b97520, 0x01, 0x0f)
[    3.830468] serial_in(ffffffffb6b97520, 0x01) -> 0xff
[    3.830530] serial_out(ffffffffb6b97520, 0x01, 0xff)
[    3.830594] IER test failed (0f, 0f)
[    3.830595] iir=255
[    3.830656] type=unknown
[    3.830888] ttyS1: autoconf (0x02f8, 0x0000000000000000):
[    3.830892] serial_in(ffffffffb6b97800, 0x01) -> 0xff
[    3.831016] serial_out(ffffffffb6b97800, 0x01, 0x00)
[    3.831080] serial_in(ffffffffb6b97800, 0x01) -> 0xff
[    3.831141] serial_out(ffffffffb6b97800, 0x01, 0x0f)
[    3.831208] serial_in(ffffffffb6b97800, 0x01) -> 0xff
[    3.831269] serial_out(ffffffffb6b97800, 0x01, 0xff)
[    3.831332] IER test failed (0f, 0f)
[    3.831333] iir=255
[    3.831393] type=unknown
[    3.831595] ttyS2: autoconf (0x03e8, 0x0000000000000000):
[    3.831598] serial_in(ffffffffb6b97ae0, 0x01) -> 0xff
[    3.831720] serial_out(ffffffffb6b97ae0, 0x01, 0x00)
[    3.831783] serial_in(ffffffffb6b97ae0, 0x01) -> 0xff
[    3.831844] serial_out(ffffffffb6b97ae0, 0x01, 0x0f)
[    3.831907] serial_in(ffffffffb6b97ae0, 0x01) -> 0xff
[    3.831969] serial_out(ffffffffb6b97ae0, 0x01, 0xff)
[    3.832040] IER test failed (0f, 0f)
[    3.832041] iir=255
[    3.832101] type=unknown
[    3.832305] ttyS3: autoconf (0x02e8, 0x0000000000000000):
[    3.832309] serial_in(ffffffffb6b97dc0, 0x01) -> 0xff
[    3.832431] serial_out(ffffffffb6b97dc0, 0x01, 0x00)
[    3.832497] serial_in(ffffffffb6b97dc0, 0x01) -> 0xff
[    3.832562] serial_out(ffffffffb6b97dc0, 0x01, 0x0f)
[    3.832627] serial_in(ffffffffb6b97dc0, 0x01) -> 0xff
[    3.832688] serial_out(ffffffffb6b97dc0, 0x01, 0xff)
[    3.832760] IER test failed (0f, 0f)
[    3.832761] iir=255
[    3.832820] type=unknown
[    3.835202] ttyS4: autoconf (0x0000, 0x(____ptrval____)):
[    3.835207] serial_in(ffffffffb6b980a0, 0x01) -> 0x00
[    3.836170] serial_out(ffffffffb6b980a0, 0x01, 0x00)
[    3.836170] serial_in(ffffffffb6b980a0, 0x01) -> 0x00
[    3.836170] serial_out(ffffffffb6b980a0, 0x01, 0x0f)
[    3.836170] serial_in(ffffffffb6b980a0, 0x01) -> 0x0f
[    3.836170] serial_out(ffffffffb6b980a0, 0x01, 0x00)
[    3.836170] serial_in(ffffffffb6b980a0, 0x04) -> 0x00
[    3.836170] serial_in(ffffffffb6b980a0, 0x03) -> 0x00
[    3.836170] serial_out(ffffffffb6b980a0, 0x03, 0xbf)
[    3.836170] serial_out(ffffffffb6b980a0, 0x02, 0x00)
[    3.836170] serial_out(ffffffffb6b980a0, 0x03, 0x00)
[    3.836170] serial_out(ffffffffb6b980a0, 0x02, 0x01)
[    3.836170] serial_in(ffffffffb6b980a0, 0x02) -> 0xc1
[    3.836170] serial_out(ffffffffb6b980a0, 0x03, 0x00)
[    3.836170] serial_out(ffffffffb6b980a0, 0x04, 0x00)
[    3.836170] serial_out(ffffffffb6b980a0, 0x02, 0x01)
[    3.836170] serial_out(ffffffffb6b980a0, 0x02, 0x07)
[    3.836170] serial_out(ffffffffb6b980a0, 0x02, 0x00)
[    3.836170] serial_in(ffffffffb6b980a0, 0x00) -> 0x78
[    3.836170] serial_out(ffffffffb6b980a0, 0x01, 0x00)
[    3.837488] iir=193
[    3.837490] type=16550A
[    3.837606] 0000:07:00.0: ttyS4 at MMIO 0xe3601000 (irq = 17, base_baud = 15625000) is a 16550A
[    3.837685] serial_out(ffffffffb6b980a0, 0x04, 0x80)
[    3.837914] ttyS5: autoconf (0x0000, 0x(____ptrval____)):
[    3.837918] serial_in(ffffffffb6b98380, 0x01) -> 0x00
[    3.838041] serial_out(ffffffffb6b98380, 0x01, 0x00)
[    3.838104] serial_in(ffffffffb6b98380, 0x01) -> 0x00
[    3.838165] serial_out(ffffffffb6b98380, 0x01, 0x0f)
[    3.838229] serial_in(ffffffffb6b98380, 0x01) -> 0x0f
[    3.838290] serial_out(ffffffffb6b98380, 0x01, 0x00)
[    3.838353] serial_in(ffffffffb6b98380, 0x04) -> 0x00
[    3.838417] serial_in(ffffffffb6b98380, 0x03) -> 0x00
[    3.838479] serial_out(ffffffffb6b98380, 0x03, 0xbf)
[    3.838541] serial_out(ffffffffb6b98380, 0x02, 0x00)
[    3.838602] serial_out(ffffffffb6b98380, 0x03, 0x00)
[    3.838667] serial_out(ffffffffb6b98380, 0x02, 0x01)
[    3.838731] serial_in(ffffffffb6b98380, 0x02) -> 0xc1
[    3.838791] serial_out(ffffffffb6b98380, 0x03, 0x00)
[    3.838853] serial_out(ffffffffb6b98380, 0x04, 0x00)
[    3.838891] serial_out(ffffffffb6b98380, 0x02, 0x01)
[    3.838891] serial_out(ffffffffb6b98380, 0x02, 0x07)
[    3.838891] serial_out(ffffffffb6b98380, 0x02, 0x00)
[    3.838891] serial_in(ffffffffb6b98380, 0x00) -> 0x1d
[    3.838891] serial_out(ffffffffb6b98380, 0x01, 0x00)
[    3.839232] iir=193
[    3.839233] type=16550A
[    3.839347] 0000:07:00.0: ttyS5 at MMIO 0xe3601200 (irq = 17, base_baud = 15625000) is a 16550A
[    3.839424] serial_out(ffffffffb6b98380, 0x04, 0x80)

Running this small test program:

#!/usr/bin/python3

import serial

p2 = serial.Serial('/dev/ttyS4', 19200)
p2.write(b'\x55')

Gives:

[  132.325041] serial_out(ffffffffb6b980a0, 0x02, 0x01)
[  132.325055] serial_out(ffffffffb6b980a0, 0x02, 0x07)
[  132.325060] serial_out(ffffffffb6b980a0, 0x02, 0x00)
[  132.325077] serial_in(ffffffffb6b980a0, 0x05) -> 0x60
[  132.325105] serial_out(ffffffffb6b980a0, 0x04, 0x88)
[  132.325119] serial_in(ffffffffb6b980a0, 0x05) -> 0x60
[  132.325134] serial_in(ffffffffb6b980a0, 0x06) -> 0xb0
[  132.325140] serial_out(ffffffffb6b980a0, 0x07, 0x02)
[  132.325144] serial_out(ffffffffb6b980a0, 0x05, 0x09)
[  132.325148] serial_out(ffffffffb6b980a0, 0x07, 0x01)
[  132.325151] serial_out(ffffffffb6b980a0, 0x05, 0x09)
[  132.325155] serial_out(ffffffffb6b980a0, 0x07, 0x03)
[  132.325158] serial_out(ffffffffb6b980a0, 0x05, 0x00)
[  132.325162] serial_out(ffffffffb6b980a0, 0x00, 0x83)
[  132.325165] serial_out(ffffffffb6b980a0, 0x01, 0x02)
[  132.325169] serial_out(ffffffffb6b980a0, 0x04, 0x88)
[  132.325175] serial_out(ffffffffb6b980a0, 0x04, 0x8b)
[  132.325245] serial_out(ffffffffb6b980a0, 0x07, 0x02)
[  132.325251] serial_out(ffffffffb6b980a0, 0x05, 0x08)
[  132.325254] serial_out(ffffffffb6b980a0, 0x07, 0x01)
[  132.325257] serial_out(ffffffffb6b980a0, 0x05, 0x1f)
[  132.325261] serial_out(ffffffffb6b980a0, 0x07, 0x03)
[  132.325264] serial_out(ffffffffb6b980a0, 0x05, 0x00)
[  132.325267] serial_out(ffffffffb6b980a0, 0x00, 0x69)
[  132.325271] serial_out(ffffffffb6b980a0, 0x01, 0x00)
[  132.325274] serial_out(ffffffffb6b980a0, 0x04, 0x8b)
[  132.325377] serial_out(ffffffffb6b980a0, 0x01, 0x07)
[  132.325385] serial_in(ffffffffb6b980a0, 0x05) -> 0x60
[  132.325389] serial_out(ffffffffb6b980a0, 0x00, 0x55)
[  132.325393] serial_out(ffffffffb6b980a0, 0x01, 0x05)
[  132.325623] serial_in(ffffffffb6b980a0, 0x05) -> 0xe9
[  132.325633] serial_in(ffffffffb6b980a0, 0x00) -> 0x78
[  132.325640] serial_in(ffffffffb6b980a0, 0x05) -> 0x60
[  132.325647] serial_in(ffffffffb6b980a0, 0x06) -> 0xb0
[  132.325758] serial_in(ffffffffb6b980a0, 0x05) -> 0xe9
[  132.325768] serial_in(ffffffffb6b980a0, 0x00) -> 0x1e
[  132.325773] serial_in(ffffffffb6b980a0, 0x05) -> 0x60
[  132.325780] serial_in(ffffffffb6b980a0, 0x06) -> 0xb0
[  132.325935] serial_in(ffffffffb6b980a0, 0x05) -> 0xe9
[  132.325943] serial_in(ffffffffb6b980a0, 0x00) -> 0x78
[  132.325948] serial_in(ffffffffb6b980a0, 0x05) -> 0x60
[  132.325955] serial_in(ffffffffb6b980a0, 0x06) -> 0xb0
[  132.326602] serial_in(ffffffffb6b980a0, 0x05) -> 0x61
[  132.326610] serial_in(ffffffffb6b980a0, 0x00) -> 0xfe
[  132.326615] serial_in(ffffffffb6b980a0, 0x05) -> 0x60
[  132.326622] serial_in(ffffffffb6b980a0, 0x06) -> 0xb0
[  132.473966] serial_in(ffffffffb6b980a0, 0x05) -> 0x60
[  132.473978] serial_out(ffffffffb6b980a0, 0x04, 0x88)
[  132.473990] serial_out(ffffffffb6b980a0, 0x04, 0x80)
[  132.473996] serial_out(ffffffffb6b980a0, 0x02, 0x01)
[  132.474000] serial_out(ffffffffb6b980a0, 0x02, 0x07)
[  132.474004] serial_out(ffffffffb6b980a0, 0x02, 0x00)





  
>   Also I have identified a code piece that handles the EFR in a destructive
> manner, which I must have previously missed, namely a conditional block
> guarded by UART_CAP_EFR in `serial8250_do_set_termios'.  It should likely
> be fixed, however it is supposed not to matter for OxSemi chips due to:
> 
> 		/* UART_CAP_EFR breaks billionon CF bluetooth card. */
> 		.flags		= UART_CAP_FIFO | UART_CAP_SLEEP,
> 
> which then leads to:
> 
> serial 0000:07:00.3: detected caps 00000700 should be 00000500
> 
> and consequently UART_CAP_EFR gets cleared and this code block isn't
> supposed to be reached.  Can you confirm the presence of a similar message
> in your log?
Can not see any such message in my dmsg

>   NB it seems to me too big a hammer to have a generic serial port feature
> globally disabled to work around an unidentified problem with an attached
> particular serial device.  Pavel, as the originator of commit d0694e2aeb81
> ("serial: unbreak billionton CF card") can you please explain what the
> motivation was here?
> 
>   I could only track down two message threads related to the problem:
> 
> <https://lore.kernel.org/lkml/20110106134254.68fa27ac@lxorguk.ukuu.org.uk/>
> 
> and:
> 
> <https://lore.kernel.org/linux-serial/4D001AF1.80902@mainpine.com/>
> 
> but no attempt to actually narrow the issue down (also ISTM like a feature
> such as flow control ought to be controlled via a termios call rather than
> globally disabled).  Also could the corruption of the EFR in what is now
> `serial8250_do_set_termios' (and used to be `serial8250_set_termios' then)
> mentioned above be the culprit?
> 

/Anders

-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Maciej W. Rozycki]

On Wed, 14 Sep 2022, Anders Blomdell wrote:

> [    3.837914] ttyS5: autoconf (0x0000, 0x(____ptrval____)):
> [    3.837918] serial_in(ffffffffb6b98380, 0x01) -> 0x00
> [    3.838041] serial_out(ffffffffb6b98380, 0x01, 0x00)
> [    3.838104] serial_in(ffffffffb6b98380, 0x01) -> 0x00
> [    3.838165] serial_out(ffffffffb6b98380, 0x01, 0x0f)
> [    3.838229] serial_in(ffffffffb6b98380, 0x01) -> 0x0f
> [    3.838290] serial_out(ffffffffb6b98380, 0x01, 0x00)
> [    3.838353] serial_in(ffffffffb6b98380, 0x04) -> 0x00
> [    3.838417] serial_in(ffffffffb6b98380, 0x03) -> 0x00
> [    3.838479] serial_out(ffffffffb6b98380, 0x03, 0xbf)
> [    3.838541] serial_out(ffffffffb6b98380, 0x02, 0x00)
> [    3.838602] serial_out(ffffffffb6b98380, 0x03, 0x00)
> [    3.838667] serial_out(ffffffffb6b98380, 0x02, 0x01)
> [    3.838731] serial_in(ffffffffb6b98380, 0x02) -> 0xc1
> [    3.838791] serial_out(ffffffffb6b98380, 0x03, 0x00)
> [    3.838853] serial_out(ffffffffb6b98380, 0x04, 0x00)
> [    3.838891] serial_out(ffffffffb6b98380, 0x02, 0x01)
> [    3.838891] serial_out(ffffffffb6b98380, 0x02, 0x07)
> [    3.838891] serial_out(ffffffffb6b98380, 0x02, 0x00)
> [    3.838891] serial_in(ffffffffb6b98380, 0x00) -> 0x1d
> [    3.838891] serial_out(ffffffffb6b98380, 0x01, 0x00)
> [    3.839232] iir=193
> [    3.839233] type=16550A
> [    3.839347] 0000:07:00.0: ttyS5 at MMIO 0xe3601200 (irq = 17, base_baud =
> 15625000) is a 16550A
> [    3.839424] serial_out(ffffffffb6b98380, 0x04, 0x80)

 Thank you.  I gather your OxSemi devices are ttyS4 and ttyS5, right?  So 
probing doesn't work for some reason and the port isn't even recognised as 
a 950 device, e.g. I have this for mine:

ttyS0: autoconf (0x0000, 0x(____ptrval____)):
EFRv2
950id=16:c9:50:0d
serial 0000:07:00.3: detected caps 00000700 should be 00000500
iir=193
type=16C950/954
0000:07:00.3: ttyS0 at MMIO 0x60301000 (irq = 26, base_baud = 15625000) is a 16C950/954

I'll examine your I/O conversation log in detail and will see if I can 
come up with a possible explanation.

 NB I'm at the GNU Tools Cauldron conference from tomorrow through this 
coming Monday, so I may not be able to get to the bottom of this issue 
right away.

  Maciej

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Maciej W. Rozycki]

On Wed, 14 Sep 2022, Maciej W. Rozycki wrote:

> I'll examine your I/O conversation log in detail and will see if I can 
> come up with a possible explanation.

 I think I know what is going on here.  Can you please confirm that you 
have the CONFIG_SERIAL_8250_16550A_VARIANTS option disabled (default to 
"off" for x86 only)?  That would explain things.

 Offhand I am not sure what to do here.  There are several options to 
choose from I can think of right now:

1. Disable new OxSemi Tornado clock code iff !SERIAL_8250_16550A_VARIANTS, 
   bringing back buggy calculation for rates above 115200bps and coarse 
   BOTHER granularity.

2. Same as above, but additionally limit the baud rates to 115200bps to 
   avoid buggy rates.

3. Force SERIAL_8250_16550A_VARIANTS to "y" if SERIAL_8250_PCI != "n". 

4. Remove SERIAL_8250_16550A_VARIANTS altogether and execute code it 
   guards unconditionally (does it still matter nowadays?).

5. Something else not yet determined.

 Greg, do you have any opinion?

  Maciej

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Anders Blomdell]

On 2022-09-14 16:15, Maciej W. Rozycki wrote:
> On Wed, 14 Sep 2022, Maciej W. Rozycki wrote:
> 
>> I'll examine your I/O conversation log in detail and will see if I can
>> come up with a possible explanation.
> 
>   I think I know what is going on here.  Can you please confirm that you
> have the CONFIG_SERIAL_8250_16550A_VARIANTS option disabled (default to
> "off" for x86 only)?  That would explain things.
Just found that myself while going through the initializaton sequence (while now knowing what to look for)

Thats what you get for choosing Fedora ;-)

> 
>   Offhand I am not sure what to do here.  There are several options to
> choose from I can think of right now:
> 
> 1. Disable new OxSemi Tornado clock code iff !SERIAL_8250_16550A_VARIANTS,
>     bringing back buggy calculation for rates above 115200bps and coarse
>     BOTHER granularity.
> 
> 2. Same as above, but additionally limit the baud rates to 115200bps to
>     avoid buggy rates.
> 
> 3. Force SERIAL_8250_16550A_VARIANTS to "y" if SERIAL_8250_PCI != "n".
> 
> 4. Remove SERIAL_8250_16550A_VARIANTS altogether and execute code it
>     guards unconditionally (does it still matter nowadays?).
> 
> 5. Something else not yet determined.
> 
>   Greg, do you have any opinion?
> 
>    Maciej

-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Greg Kroah-Hartman]

On Wed, Sep 14, 2022 at 03:15:52PM +0100, Maciej W. Rozycki wrote:
> On Wed, 14 Sep 2022, Maciej W. Rozycki wrote:
> 
> > I'll examine your I/O conversation log in detail and will see if I can 
> > come up with a possible explanation.
> 
>  I think I know what is going on here.  Can you please confirm that you 
> have the CONFIG_SERIAL_8250_16550A_VARIANTS option disabled (default to 
> "off" for x86 only)?  That would explain things.
> 
>  Offhand I am not sure what to do here.  There are several options to 
> choose from I can think of right now:
> 
> 1. Disable new OxSemi Tornado clock code iff !SERIAL_8250_16550A_VARIANTS, 
>    bringing back buggy calculation for rates above 115200bps and coarse 
>    BOTHER granularity.
> 
> 2. Same as above, but additionally limit the baud rates to 115200bps to 
>    avoid buggy rates.

Maybe this one?  That feels odd that we do different things for this old
config option, that's not good.  So making this "just work" should be
the best idea if at all possible.

> 
> 3. Force SERIAL_8250_16550A_VARIANTS to "y" if SERIAL_8250_PCI != "n". 
> 
> 4. Remove SERIAL_8250_16550A_VARIANTS altogether and execute code it 
>    guards unconditionally (does it still matter nowadays?).
> 
> 5. Something else not yet determined.

We can't just remove it, as for x86 the default is disabled due to this
only being relevant for very old hardware.

thanks,

greg k-h

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Anders Blomdell]

On 2022-09-14 16:57, Greg Kroah-Hartman wrote:
> On Wed, Sep 14, 2022 at 03:15:52PM +0100, Maciej W. Rozycki wrote:
>> On Wed, 14 Sep 2022, Maciej W. Rozycki wrote:
>>
>>> I'll examine your I/O conversation log in detail and will see if I can
>>> come up with a possible explanation.
>>
>>   I think I know what is going on here.  Can you please confirm that you
>> have the CONFIG_SERIAL_8250_16550A_VARIANTS option disabled (default to
>> "off" for x86 only)?  That would explain things.
>>
>>   Offhand I am not sure what to do here.  There are several options to
>> choose from I can think of right now:
>>
>> 1. Disable new OxSemi Tornado clock code iff !SERIAL_8250_16550A_VARIANTS,
>>     bringing back buggy calculation for rates above 115200bps and coarse
>>     BOTHER granularity.
>>
>> 2. Same as above, but additionally limit the baud rates to 115200bps to
>>     avoid buggy rates.
> 
> Maybe this one?  That feels odd that we do different things for this old
> config option, that's not good.  So making this "just work" should be
> the best idea if at all possible.
> 
>>
>> 3. Force SERIAL_8250_16550A_VARIANTS to "y" if SERIAL_8250_PCI != "n".
>>
>> 4. Remove SERIAL_8250_16550A_VARIANTS altogether and execute code it
>>     guards unconditionally (does it still matter nowadays?).
>>
>> 5. Something else not yet determined.
We could force an EFR probe for this specific driver only.

Pros: driver behaves the same regardless of CONFIG_SERIAL_8250_16550A_VARIANTS
       other chips/drivers will not get probed
Cons: autoconfig code will be somewhat bigger since code after the test will be reachable
       change in unrelated part (8250_core.c) to propagate .probe flags

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 82726cda6066..b9f28f95cfd5 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1011,6 +1011,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
  		uart->rs485_start_tx	= up->rs485_start_tx;
  		uart->rs485_stop_tx	= up->rs485_stop_tx;
  		uart->dma		= up->dma;
+		uart->probe		= up->probe;
  
  		/* Take tx_loadsz from fifosize if it wasn't set separately */
  		if (uart->port.fifosize && !uart->tx_loadsz)
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index f6732c1ed238..b0b21e49ec6c 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1242,6 +1242,7 @@ static int pci_oxsemi_tornado_setup(struct serial_private *priv,
  		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;
+		up->probe |= UART_PROBE_EFR;
  	}
  
  	return pci_default_setup(priv, board, up, idx);
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 2b86c55ed374..b207d9982936 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1029,9 +1029,9 @@ static void autoconfig_16550a(struct uart_8250_port *up)
  	up->port.type = PORT_16550A;
  	up->capabilities |= UART_CAP_FIFO;
  
-	if (!IS_ENABLED(CONFIG_SERIAL_8250_16550A_VARIANTS))
+	if (!IS_ENABLED(CONFIG_SERIAL_8250_16550A_VARIANTS) &&
+	    !(up->probe & UART_PROBE_EFR))
  		return;
-
  	/*
  	 * Check for presence of the EFR when DLAB is set.
  	 * Only ST16C650V1 UARTs pass this test.
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index ff84a3ed10ea..0855316468e2 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -112,6 +112,7 @@ struct uart_8250_port {
  	unsigned char		probe;
  	struct mctrl_gpios	*gpios;
  #define UART_PROBE_RSA	(1 << 0)
+#define UART_PROBE_EFR	(1 << 1)
  
  	/*
  	 * Some bits in registers are cleared on a read, so they must

> 
> We can't just remove it, as for x86 the default is disabled due to this
> only being relevant for very old hardware.
> 
> thanks,
> 
> greg k-h

-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Maciej W. Rozycki]

On Thu, 15 Sep 2022, Anders Blomdell wrote:

> > > 1. Disable new OxSemi Tornado clock code iff !SERIAL_8250_16550A_VARIANTS,
> > >     bringing back buggy calculation for rates above 115200bps and coarse
> > >     BOTHER granularity.
> > > 
> > > 2. Same as above, but additionally limit the baud rates to 115200bps to
> > >     avoid buggy rates.
> > 
> > Maybe this one?  That feels odd that we do different things for this old
> > config option, that's not good.  So making this "just work" should be
> > the best idea if at all possible.

 Though quite an invasive one too IMO to deal with a corner case, plus 
distributions would likely get it wrong, just as this case indicates, and 
would ship a crippled (though at least working) driver.

> > > 3. Force SERIAL_8250_16550A_VARIANTS to "y" if SERIAL_8250_PCI != "n".
> > > 
> > > 4. Remove SERIAL_8250_16550A_VARIANTS altogether and execute code it
> > >     guards unconditionally (does it still matter nowadays?).
> > > 
> > > 5. Something else not yet determined.
> We could force an EFR probe for this specific driver only.

 Having investigated the history of SERIAL_8250_16550A_VARIANTS and the 
possible options I came to a similar conclusion.  I wasn't aware the 
option has about only just been introduced and must have confused it with 
one of the older ones such as SERIAL_8250_EXTENDED, which has been there 
since forever.

> Pros: driver behaves the same regardless of CONFIG_SERIAL_8250_16550A_VARIANTS
>       other chips/drivers will not get probed
> Cons: autoconfig code will be somewhat bigger since code after the test will
> be reachable
>       change in unrelated part (8250_core.c) to propagate .probe flags

 But SERIAL_8250_16550A_VARIANTS is quite a recent addition and the 
motivation wasn't code size, so I wouldn't be concerned here.

 Another pro is the change is small and sweet, so no large chunks of code 
to maintain.  And it is expandable easily to other subdrivers should they 
need a similar arrangement.

> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index ff84a3ed10ea..0855316468e2 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -112,6 +112,7 @@ struct uart_8250_port {
>  	unsigned char		probe;
>  	struct mctrl_gpios	*gpios;
>  #define UART_PROBE_RSA	(1 << 0)
> +#define UART_PROBE_EFR	(1 << 1)
>   	/*
>  	 * Some bits in registers are cleared on a read, so they must

 I chose to do it differently though.  And I got stumped, because when I 
got to verifying it I decided to double-check my code with a 32-bit system 
too, that is my x86 box.  And surprisingly existing code worked correctly 
despite having SERIAL_8250_16550A_VARIANTS unset as if the fix wasn't 
needed in the first place.

 After a lot of fiddling I realised there is a feature of the OxSemi ASIC 
that is not documented in the datasheet, the existence of which however 
you can read about between the lines:

"Extending CPR for Legacy UART

"When operating in Legacy UART mode, the system drivers will assume the 
industry standard 1.8432MHz reference clock.  As the Reference clock for 
the UARTs in the OXPCIe952 is 62.5MHz, all DLL/DLM values will be 
incorrect if no pre-scaling is done by the UART.  In order to correct this 
effect the CPR register must divide the reference clock by 33.90842 which 
is approximated to 33.875 by default after a reset.  As such the CPR has 
been extended to support a larger 6 bit integer range by using bit-0 of 
CPR2 to represent bit-6 of the integer portion of the clock pre-scalar"

What it doesn't say is that in the legacy UART mode the Divide-by-M N/8 
prescaler is implicitly enabled despite that the MCR[7] bit is forced 0 in 
the non-enhanced mode.  This is opposite to how things work in the native 
UART mode where the MCR[7] bit is respected regardless.  It is only when 
the enhanced mode is enabled via EFR[4] that the MCR[7] bit is respected 
both in the legacy and the native UART mode.

 It makes sense, because legacy 8250 UART drivers will expect to work with 
`base_baud' set to 115200 while using the MCR the way it has been with the 
original 16450 chip.  Conversely legacy 16450 UART drivers do not support 
the native UART mode anyway, so it doesn't have to implement this special 
case.

 And I have the option card jumpered to work in the legacy UART mode in my 
x86 system (just so that I can verify both modes of operation at any 
time):

0000:04:00.0: ttyS2 at I/O 0x1000 (irq = 10, base_baud = 15625000) is a 16C950/954
0000:04:00.1: ttyS3 at I/O 0x1008 (irq = 5, base_baud = 15625000) is a 16C950/954

04:00.0 Serial controller [0700]: Oxford Semiconductor Ltd OXPCIe952 Legacy 950 UART #1 [1415:c144]
04:00.1 Serial controller [0700]: Oxford Semiconductor Ltd OXPCIe952 Legacy 950 UART #2 [1415:c145]

vs the native UART mode (here in POWER9):

0001:01:00.0: ttyS0 at MMIO 0x600c080401000 (irq = 40, base_baud = 15625000) is a 16C950/954
0001:01:00.0: ttyS1 at MMIO 0x600c080401200 (irq = 40, base_baud = 15625000) is a 16C950/954

0001:01:00.0 Serial controller [0700]: Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART [1415:c158]

Oh well!

 With that phenomenon sorted I'll be posting the final fix shortly, as 
soon I have made a suitable change description referring the relevant 
sources.

 I take it you don't mind being mentioned by means of `Reported-by'?

  Maciej

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Maciej W. Rozycki]

On Wed, 14 Sep 2022, Anders Blomdell wrote:

> >   I think I know what is going on here.  Can you please confirm that you
> > have the CONFIG_SERIAL_8250_16550A_VARIANTS option disabled (default to
> > "off" for x86 only)?  That would explain things.
> Just found that myself while going through the initializaton sequence (while
> now knowing what to look for)
> 
> Thats what you get for choosing Fedora ;-)

 May I suggest that as a person directly affected you file a defect with 
the distribution?  I think a distribution kernel should in principle have 
all the reasonable features enabled for the hardware handled.

  Maciej

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Maciej W. Rozycki]

On Tue, 13 Sep 2022, Anders Blomdell wrote:

> The delock card is in an x86 machine, and it communicates with various RS232
> devices
> (hence speeds above 230400 usually works badly with a few meters of cabling)

 FWIW I was able to get reliable communication between a pair of OXPCIe952 
devices over a ~1.5m connection (a flat Cisco console cable) using rates 
of up to 3500000bps despite the cards having line transceivers specified 
for up to 1MHz operation only (cards marketed for 921600bps operation).  
The only issue with rates above 576000bps were input overruns, asking for 
DMA operation really (which the OXPCIe952 does support; only we don't!).

 Cf.: 
<https://lore.kernel.org/linux-serial/alpine.DEB.2.21.2106071700090.1601@angie.orcam.me.uk/>.

  Maciej

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Anders Blomdell]

On 2022-09-16 23:03, Maciej W. Rozycki wrote:
> On Wed, 14 Sep 2022, Anders Blomdell wrote:
> 
>>>    I think I know what is going on here.  Can you please confirm that you
>>> have the CONFIG_SERIAL_8250_16550A_VARIANTS option disabled (default to
>>> "off" for x86 only)?  That would explain things.
>> Just found that myself while going through the initializaton sequence (while
>> now knowing what to look for)
>>
>> Thats what you get for choosing Fedora ;-)
> 
>   May I suggest that as a person directly affected you file a defect with
> the distribution?  I think a distribution kernel should in principle have
> all the reasonable features enabled for the hardware handled.
First thing I did :-)

https://bugzilla.redhat.com/show_bug.cgi?id=2126201

> 
>    Maciej

-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Grant Edwards]

On 2022-09-13, Anders Blomdell <anders.blomdell@control.lth.se> wrote:
> I get incorrect baudrates, my oscilloscope gives:
>
> Programmed	Measured
>
>    2400		  5208
>    4800		 13150
>    9600		 10410
>   19200		 71420
>   38400		142000
>   57600		201600
> 115200		138800

I just ran into what I think is the same problem when upgrading from
5.10.76 to 5.15.68 (sorry I don't have any intermediate kernel
versions to test with). This is an oxford quad 950 board that has
worked flawlessly for many years. Now the baud rates are all wrong.

$ lspci | grep OX
03:00.0 Serial controller: Oxford Semiconductor Ltd OXPCIe954 Quad Native 950 UART

$ dmesg | grep ttyS
[    0.265026] 0000:03:00.0: ttyS0 at MMIO 0xf7801000 (irq = 19, base_baud = 15625000) is a 16550A
[    0.265130] 0000:03:00.0: ttyS1 at MMIO 0xf7801200 (irq = 19, base_baud = 15625000) is a 16550A
[    0.265231] 0000:03:00.0: ttyS2 at MMIO 0xf7801400 (irq = 19, base_baud = 15625000) is a 16550A
[    0.265358] 0000:03:00.0: ttyS3 at MMIO 0xf7801600 (irq = 19, base_baud = 15625000) is a 16550A

The only change I could see in dmesg/setserial output is that the
baud base changed from 4000000 to 15625000. However, changing the baud
base back to 4000000 does not make the ports work again

With the default baud base of 15625000, baud rates look like this:

Programmed  Measured
    2400       5398
    4800      13812
    9600      10796 
   19200      74418
   
The curious thing is that the buad rate errors are non-linear, so you
can't just adjust the base baud value to get correct baud rates. The
algorithm used to calculate the baud divisors seems to be broken.

I've read through the rest of this thread a couple times, but was
unable to figure out what to do to fix this problem or if it got fixed
in more recent kernel versions.

Did this problem get fixed in more recent kernels?

Is there something I can do with a 6.15 kernel to get it this board to
work again?

--
Grant

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Grant Edwards]

On 2022-10-04, Grant Edwards <grant.b.edwards@gmail.com> wrote:

> I just ran into what I think is the same problem when upgrading from
> 5.10.76 to 5.15.68 (sorry I don't have any intermediate kernel
> versions to test with). This is an oxford quad 950 board that has
> worked flawlessly for many years. Now the baud rates are all wrong.
>
> [...]

After reading through the thread a third time, I tried enabling
CONFIG_SERIAL_8250_16550A_VARIANTS in my 6.15.69 kernel, and my	quad
Oxford board works again.

The first two times I read through the thread I misunderstood the statement

    Can you please confirm that you have the
    CONFIG_SERIAL_8250_16550A_VARIANTS option disabled (default to
    "off" for x86 only)?

as meaning that you should disable that option as a prerequisite to
making it work. So I checked to make sure it was disabled (it
was). Yes, it's obvious now what was meant was that having it disabled
explains the previous observations.

--
Grant

Re: kernel 5.19.8: "Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART" gets wrong baudrate (PCI ID 1415:c158)

[Ilpo Järvinen]

On Tue, 4 Oct 2022, Grant Edwards wrote:

> On 2022-09-13, Anders Blomdell <anders.blomdell@control.lth.se> wrote:
> > I get incorrect baudrates, my oscilloscope gives:
> >
> > Programmed	Measured
> >
> >    2400		  5208
> >    4800		 13150
> >    9600		 10410
> >   19200		 71420
> >   38400		142000
> >   57600		201600
> > 115200		138800
> 
> I just ran into what I think is the same problem when upgrading from
> 5.10.76 to 5.15.68 (sorry I don't have any intermediate kernel
> versions to test with). This is an oxford quad 950 board that has
> worked flawlessly for many years. Now the baud rates are all wrong.
> 
> $ lspci | grep OX
> 03:00.0 Serial controller: Oxford Semiconductor Ltd OXPCIe954 Quad Native 950 UART
> 
> $ dmesg | grep ttyS
> [    0.265026] 0000:03:00.0: ttyS0 at MMIO 0xf7801000 (irq = 19, base_baud = 15625000) is a 16550A
> [    0.265130] 0000:03:00.0: ttyS1 at MMIO 0xf7801200 (irq = 19, base_baud = 15625000) is a 16550A
> [    0.265231] 0000:03:00.0: ttyS2 at MMIO 0xf7801400 (irq = 19, base_baud = 15625000) is a 16550A
> [    0.265358] 0000:03:00.0: ttyS3 at MMIO 0xf7801600 (irq = 19, base_baud = 15625000) is a 16550A
> 
> The only change I could see in dmesg/setserial output is that the
> baud base changed from 4000000 to 15625000. However, changing the baud
> base back to 4000000 does not make the ports work again
> 
> With the default baud base of 15625000, baud rates look like this:
> 
> Programmed  Measured
>     2400       5398
>     4800      13812
>     9600      10796 
>    19200      74418
>    
> The curious thing is that the buad rate errors are non-linear, so you
> can't just adjust the base baud value to get correct baud rates. The
> algorithm used to calculate the baud divisors seems to be broken.
> 
> I've read through the rest of this thread a couple times, but was
> unable to figure out what to do to fix this problem or if it got fixed
> in more recent kernel versions.
> 
> Did this problem get fixed in more recent kernels?

This series was used to fix the problem:

https://lore.kernel.org/linux-serial/alpine.DEB.2.21.2209201659350.41633@angie.orcam.me.uk/T/#t

Patch 2/3 went only for v5.19+.

-- 
 i.


> Is there something I can do with a 6.15 kernel to get it this board to
> work again?
> 
> --
> Grant
>