Message-ID: <e9e9fe33-5341-f366-1b15-816892485fe1@accesio.com>
Date:   Tue, 23 Nov 2021 20:29:30 -0800
From:   Jay Dolan <jay.dolan@...esio.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
        Jiri Slaby <jirislaby@...nel.org>
Subject: Re: [PATCH v1 0/2] serial: 8250_pci: Split Pericom driver



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.

Powered by blists - more mailing lists