Re: [PATCH v1 0/2] serial: 8250_pci: Split Pericom driver

From: Jay Dolan
Date: Tue Nov 23 2021 - 23:29:37 EST

Next message: kernel test robot: "[mcgrof-next:20211123-sysctl-cleanups 20/41] include/linux/sysctl.h:230:13: error: expected ';' after return statement"
Previous message: Finn Thain: "[PATCH] pata_falcon: Add missing __iomem annotations"
In reply to: Andy Shevchenko: "Re: [PATCH v1 0/2] serial: 8250_pci: Split Pericom driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 11/23/21 7:38 AM, Andy Shevchenko wrote:

On Tue, Nov 23, 2021 at 07:02:44AM -0800, Jay Dolan wrote:

On 11/23/21 1:25 AM, Andy Shevchenko wrote:

On Mon, Nov 22, 2021 at 09:19:09PM -0800, Jay Dolan wrote:

On 11/22/21 4:48 AM, Andy Shevchenko wrote:

On Mon, Nov 22, 2021 at 02:47:20PM +0200, Andy Shevchenko wrote:

On Thu, Nov 18, 2021 at 10:32:51PM -0800, Jay Dolan wrote:

On 11/17/21 6:57 AM, Andy Shevchenko wrote:

Split Pericom driver to a separate module.
While at it, re-enable high baud rates.

Jay, can you, please, test this on as many hardware as you have?

...

* Add in pericom_do_startup() because the UPF_MAGIC_MULTIPLIER doesn't
stick.

Can't find an evidence that this is the case. Can you recheck this (reading
flags back via sysfs or so)? So, for v2 I'll leave my approach.

Otherwise how the other drivers which are using that flag survive? If it's
indeed an issue, it should be fixed on generic level.

I modified pericom_do_startup to log when the UPF_MAGIC_MULTIPLIER flag was
present. Then tried to set the port to 3000000 a few times. The port
stayed at 9600. It looks like pericom_do_startup() is getting called twice
per port on boot, and the flag is gone with the second one.

[ 4.925577] [J4D] flag present
[ 4.926121] [J4D[ flag not present
[ 4.926843] [J4D] flag present
[ 4.927415] [J4D[ flag not present
[ 4.928106] [J4D] flag present
[ 4.928673] [J4D[ flag not present
[ 4.929419] [J4D] flag present
[ 4.930447] [J4D[ flag not present

[ 49.528504] [J4D[ flag not present
[ 51.675240] [J4D[ flag not present
[ 59.617954] [J4D[ flag not present

Then I modified it to log when it was adding the flag in. The port was set
to 3000000. Also the flag only needed to be added in once. It sticks after
the first time.

[ 4.647546] [J4D] flag present
[ 4.648119] [J4D] flag not present(adding)
[ 4.648778] [J4D] flag present
[ 4.649330] [J4D] flag not present(adding)
[ 4.650001] [J4D] flag present
[ 4.650537] [J4D] flag not present(adding)
[ 4.651192] [J4D] flag present
[ 4.651718] [J4D] flag not present(adding)

[ 96.025668] [J4D] flag present
[ 100.130626] [J4D] flag present
[ 116.435436] [J4D] flag present

I mostly just guessed at do_startup() being the place to set the magic
multiplier flag after it didn't stick in quirk in 8250_pci.c.

Can you share `dmesg` and output of `lspci -nk -vv` on the machine with the
kernel with patches applied and running?

Provided below.

Thanks!

Also, I am going to lose the place for my test station to
the family Christmas tree after tomorrow. I'm not sure when or where I'm
going to get it set back up.

Understood, in case you still have time to test one idea below. As far as I
understand current state of affairs the problematic part is the magic
multiplier.

01:00.0 0700: 494f:10dc (prog-if 02 [16550])
Subsystem: 0001:0001
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 16
Region 0: I/O ports at e000 [size=64]
Region 1: Memory at fe400000 (32-bit, non-prefetchable) [size=4K]

Okay, we are interested so far in IO bar (which is BAR0).
Do you know what is the BAR1 for?

I found a copy of the chip spec here. https://www.diodes.com/assets/Datasheets/PI7C9X7958.pdf BARs are described in chapter seven.

The other BAR is MMIO access to the UART registers. Pericom has an out of tree driver for this. It doesn't build against current kernels, and I have never gotten it stable.

Capabilities: [80] Power Management version 3
Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA
PME(D0-,D1-,D2-,D3hot+,D3cold-)
Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [8c] MSI: Enable- Count=1/1 Maskable- 64bit+
Address: 0000000000000000 Data: 0000
Capabilities: [9c] Vital Product Data
Not readable
Capabilities: [a4] Vendor Specific Information: Len=28 <?>
Capabilities: [e0] Express (v1) Legacy Endpoint, MSI 00
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <64ns, L1 <1us
ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 128 bytes
DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s
<512ns, L1 <1us
ClockPM- Surprise- LLActRep- BwNot- ASPMOptComp-
LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk-
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk- DLActive- BWMgmt-
ABWMgmt-
Capabilities: [100 v1] Advanced Error Reporting
UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP-
ECRC- UnsupReq- ACSViol-
UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP-
ECRC- UnsupReq- ACSViol-
UESvrt: DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+
ECRC- UnsupReq- ACSViol-
CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
AERCap: First Error Pointer: 00, GenCap- CGenEn- ChkCap- ChkEn-
Kernel driver in use: 8250_pericom
Kernel modules: 8250_pci, 8250_pericom

Yep, two drivers matches and only 8250_pericom is in use. All good here.

[ 0.062531] pci 0000:01:00.0: [494f:10dc] type 00 class 0x070002
[ 0.062551] pci 0000:01:00.0: reg 0x10: [io 0xe000-0xe03f]
[ 0.062562] pci 0000:01:00.0: reg 0x14: [mem 0xfe400000-0xfe400fff]
[ 0.062670] pci 0000:01:00.0: supports D1 D2
[ 0.062673] pci 0000:01:00.0: PME# supported from D3hot

Yeah, this interesting layout with 64 bytes and gap in UARTs 3-6.

[ 2.927496] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[ 2.927703] 00:04: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a
16550A
[ 2.979481] 0000:01:00.0: ttyS4 at I/O 0xe000 (irq = 16, base_baud =
921600) is a 16550A
[ 2.979487] 0000:01:00.0: ttyS4 extra baud rates supported: 1843200,
3686400
[ 2.979822] 0000:01:00.0: ttyS5 at I/O 0xe008 (irq = 16, base_baud =
921600) is a 16550A
[ 2.979825] 0000:01:00.0: ttyS5 extra baud rates supported: 1843200,
3686400
[ 2.982020] 0000:01:00.0: ttyS6 at I/O 0xe010 (irq = 16, base_baud =
921600) is a 16550A
[ 2.982025] 0000:01:00.0: ttyS6 extra baud rates supported: 1843200,
3686400
[ 2.982256] 0000:01:00.0: ttyS7 at I/O 0xe038 (irq = 16, base_baud =
921600) is a 16550A
[ 2.982259] 0000:01:00.0: ttyS7 extra baud rates supported: 1843200,
3686400

This is most important part. The code autodetects 16550A. Now I'm wondering
if the following change will keep MAGIC multiplier untouched (instead of
current patch):

- uart.port.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF | UPF_SHARE_IRQ;
+ uart.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SHARE_IRQ | UPF_MAGIC_MULTIPLIER;
+ uart.port.type = PORT_16550A;

No luck. The behavior of the multiplier flag is the same as before.

[ 42.730050] 8250_pericom 0000:01:00.0: VPD access failed. This is likely
a firmware bug on this device. Contact the card vendor for a firmware
update

Not sure what is this and how it may affect anything.

This is produced when running lspci.

Next message: kernel test robot: "[mcgrof-next:20211123-sysctl-cleanups 20/41] include/linux/sysctl.h:230:13: error: expected ';' after return statement"
Previous message: Finn Thain: "[PATCH] pata_falcon: Add missing __iomem annotations"
In reply to: Andy Shevchenko: "Re: [PATCH v1 0/2] serial: 8250_pci: Split Pericom driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]