Re: Have my PA8800 back online... (serial port missing on v4.14)

Re: Have my PA8800 back online... (serial port missing on v4.14)

[Helge Deller]

Adding the linux-serial mailing list:

* Frank Scheiner <frank.scheiner@web.de>:
> On 12/04/2017 03:54 PM, Helge Deller wrote:
> > Anyway, the *only* problem we have right now is, that the Linux kernel 4.14 doesn't detect all serial ports which were detected in earlier kernels.
> > Thus the kernel will talk to the non-existant serial port at 0xfffffffff4050010 instead of 0xfffffffff4050000.
> > 
> > 4.13:
> > [   28.882849] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
> > [   28.898720] 0000:e0:01.0: ttyS0 at MMIO 0xfffffffff4051000 (irq = 73, base_baud = 115200) is a 16450
> > [   28.934669] 0000:e0:01.1: ttyS1 at MMIO 0xfffffffff4050000 (irq = 73, base_baud = 115200) is a 16550A
> > [   28.963031] 0000:e0:01.1: ttyS2 at MMIO 0xfffffffff4050010 (irq = 73, base_baud = 115200) is a 16550A
> > [   28.984946] 0000:e0:01.1: ttyS3 at MMIO 0xfffffffff4050038 (irq = 73, base_baud = 115200) is a 16550A
> 
> > 
> > ...but for v4.14.x only the following serial ports are detected:
> > [   28.671984] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
> > [   28.708902] 0000:e0:01.1: ttyS0 at MMIO 0xfffffffff4050000 (irq = 73, base_baud = 115200) is a 16550A
> > [   28.731145] 0000:e0:01.1: ttyS1 at MMIO 0xfffffffff4050010 (irq = 73, base_baud = 115200) is a 16550A
> > 
> > 
> > Maybe reverting this commit brings back the old behavior:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7d8905d064058f4b65057e0101588f362f288bc0
> 
> I'm unsure about this commit, it speaks more of avoiding duplicate messages
> for device enabling.

Reverting this commit:

commit 7d8905d064058f4b65057e0101588f362f288bc0
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date:   Mon Jul 24 20:28:32 2017 +0300

    serial: 8250_pci: Enable device after we check black list

indeed fixes the problem.

After reverting, the serial port from the Diva card shows up as ttyS0 (as before).
With that patch applied, the serial port from the Diva card gets
ignored and the previous ttyS1 port becomes ttyS0 which then breaks
booting the parisc machine because the kernel expects the serial port on
ttyS1.


I'm not sure what the best way forward is.
Fact is, that the patch above changes the behaviour and serial ports
which were existant before suddenly vanish with kernel 4.14.

This following patch does work, and adds back the Diva serial port on parisc.
Not sure if it's acceptable though.

Helge

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 0c101a7470b0..61319e968e8c 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -3393,6 +3393,10 @@ static int serial_pci_is_class_communication(struct pci_dev *dev)
 	 * (Should we try to make guesses for multiport serial devices
 	 * later?)
 	 */
+	if (IS_ENABLED(CONFIG_PARISC) &&
+	    (dev->class >> 8) == PCI_CLASS_COMMUNICATION_OTHER)
+		return 0;
+
 	if ((((dev->class >> 8) != PCI_CLASS_COMMUNICATION_SERIAL) &&
 	     ((dev->class >> 8) != PCI_CLASS_COMMUNICATION_MODEM)) ||
 	    (dev->class & 0xff) > 6)

Re: Have my PA8800 back online...

[Helge Deller]

[-- Attachment #1: Type: text/plain, Size: 2249 bytes --]

On 08.12.2017 23:14, John David Anglin wrote:
> On 2017-12-08 4:29 PM, Helge Deller wrote:
>>> root@rp3440:~# lspci -nn
>>> [...]
>>> e0:01.0 Communication controller [0780]: Hewlett-Packard Company Auxiliary Diva Serial Port [103c:1290] (rev 01)
>>> e0:01.1 Serial controller [0700]: Hewlett-Packard Company Diva Serial [GSP] Multiport UART [103c:1048] (rev 03)
>>>
>>> Interesting - actually the current situation looks like what [your patch from May 29th] should have accomplished. But as you dropped it in favor of a patch to palo, it seems to be unrelated.
>>>
>>> [your patch from May 29th]:https://patchwork.kernel.org/patch/9753613/
>> The patch had some other issues which is why I dropped it.
>> I like the testing of the class id much more, as it's done by the current upstream code.
>> E.g. this diva port reports PCI_CLASS_COMMUNICATION_OTHER as PCI CLASS ID, which differs to what the serial port driver should handle.
>>
> Yes, you previously noted that this port has no external connector or access via Ethernet.  I suspect it talks to some internal module.
> I don't know if there's any reason to access the device it's connected to (if any).  It probably should be "ttyO0".
> 
> Although the renumbering of the serial ports is somewhat annoying

Yes, it's really annoying.
Especially right now, where all kernels >= 4.14 need ttyS0, while older onese require ttyS1 as boot console. 
It's confusing.

> maybe palo change should be reverted so default is always ttyS0

Yes, agreed.
With the current situation we are not better with preselecting ttyS1 on rp34x0 machines.

> I would like the enumeration of the RV100 port skipped as the implementation on the rp34xx machines is broken and can't be used.

The RV100 port refers here to the built-in ATI Radeon 7000 graphics card which is part of Diva management card.
It's not working on rp34x0 machines. 

> It's enumeration means one has to explicitly specify the console argument in the boot command line as there seems to no way to
> prefer the serial console port.

Can you please try attached patch which disables the serial MUX and ATI card?
If it works for you and if we backport it to all kernels and if we revert palo to use ttyS0 for all machines we might be good.

Helge

[-- Attachment #2: p1 --]
[-- Type: text/plain, Size: 1537 bytes --]

diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index a25fed52f7e9..8e42827d3c27 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -1692,3 +1692,39 @@ void lba_set_iregs(struct parisc_device *lba, u32 ibase, u32 imask)
 	iounmap(base_addr);
 }
 
+
+/*
+ * The design of the Diva management card in rp34x0 machines (rp3410, rp3440)
+ * seems rushed, so that many built-in components simply don't work.
+ * The following quirks disable the serial AUX port and the built-in ATI RV100
+ * Radeon 7000 graphics card which both don't have any external connectors and
+ * thus are useless, and even worse, e.g. the AUX ports occupies ttyS0 and
+ * as such makes those machines the only PARISC machines on which we can't
+ * use ttyS0 as boot console.
+ */
+static void quirk_diva_ati_card(struct pci_dev *dev)
+{
+	/* subsystem IDs are from Diva */
+	if (dev->subsystem_vendor != PCI_VENDOR_ID_HP ||
+	    dev->subsystem_device != 0x1292)
+		return;
+
+	dev_info(dev, "Hiding Diva built-in ATI card.");
+	dev->device = 0;
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_RADEON_QY,
+	quirk_diva_ati_card);
+
+static void quirk_diva_aux_disable(struct pci_dev *dev)
+{
+	/* subsystem IDs are from Diva */
+	if (dev->subsystem_vendor != PCI_VENDOR_ID_HP ||
+	    dev->subsystem_device != 0x1291)
+		return;
+
+	dev_info(dev, "Hiding Diva built-in AUX serial device");
+	dev->device = 0;
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_DIVA_AUX,
+	quirk_diva_aux_disable);
+

Re: Have my PA8800 back online... (serial port missing on v4.14)

[Andy Shevchenko]

On Fri, 2017-12-08 at 20:06 +0100, Helge Deller wrote:
> Adding the linux-serial mailing list:

Thanks for pointing to this out.
Details are below.

> > > Anyway, the *only* problem we have right now is, that the Linux
> > > kernel 4.14 doesn't detect all serial ports which were detected in
> > > earlier kernels.

> > > Thus the kernel will talk to the non-existant serial port at
> > > 0xfffffffff4050010 instead of 0xfffffffff4050000.

Wait, from this sentence you actually confirm that patch removes *non-
existant* ports.

Can you elaborate what you imply here?

> > > 4.13:
> > > [   28.882849] Serial: 8250/16550 driver, 4 ports, IRQ sharing
> > > enabled
> > > [   28.898720] 0000:e0:01.0: ttyS0 at MMIO 0xfffffffff4051000 (irq
> > > = 73, base_baud = 115200) is a 16450
> > > [   28.934669] 0000:e0:01.1: ttyS1 at MMIO 0xfffffffff4050000 (irq
> > > = 73, base_baud = 115200) is a 16550A
> > > [   28.963031] 0000:e0:01.1: ttyS2 at MMIO 0xfffffffff4050010 (irq
> > > = 73, base_baud = 115200) is a 16550A
> > > [   28.984946] 0000:e0:01.1: ttyS3 at MMIO 0xfffffffff4050038 (irq
> > > = 73, base_baud = 115200) is a 16550A

>From here it looks like multi-function PCI device with two functions
with 1 + 3 serial ports each.

> > > ...but for v4.14.x only the following serial ports are detected:
> > > [   28.671984] Serial: 8250/16550 driver, 4 ports, IRQ sharing
> > > enabled
> > > [   28.708902] 0000:e0:01.1: ttyS0 at MMIO 0xfffffffff4050000 (irq
> > > = 73, base_baud = 115200) is a 16550A
> > > [   28.731145] 0000:e0:01.1: ttyS1 at MMIO 0xfffffffff4050010 (irq
> > > = 73, base_baud = 115200) is a 16550A

I'm quite curious how ttyS0 and ttyS3 in previous run (old kernel)
appear.

> > > 
> > > 
> > > Maybe reverting this commit brings back the old behavior:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > > /commit/?id=7d8905d064058f4b65057e0101588f362f288bc0
> > 
> > I'm unsure about this commit, it speaks more of avoiding duplicate
> > messages
> > for device enabling.

No, it's about trying IRQs twice, though it might be not fully clear
from commit message: the example there shows that IRQs are probed twice
and on some platforms it may be a problem.

> 
> Reverting this commit:
> 
> commit 7d8905d064058f4b65057e0101588f362f288bc0
> Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date:   Mon Jul 24 20:28:32 2017 +0300
> 
>     serial: 8250_pci: Enable device after we check black list
> 
> indeed fixes the problem.
> 
> After reverting, the serial port from the Diva card shows up as ttyS0
> (as before).

> With that patch applied, the serial port from the Diva card gets
> ignored and the previous ttyS1 port becomes ttyS0 which then breaks
> booting the parisc machine because the kernel expects the serial port
> on
> ttyS1.

> I'm not sure what the best way forward is.
> Fact is, that the patch above changes the behaviour and serial ports
> which were existant before suddenly vanish with kernel 4.14.

As stated in the commit message there "We can do this since PCI
specification requires class, device and vendor ID registers to be
always present in the configuration space."

So, my understanding that the patch reveals the issue with these ports.

(Of course, I agree this is regression and needs to be fixed ASAP)

> This following patch does work, and adds back the Diva serial port on
> parisc.

> Not sure if it's acceptable though.

For me it looks like the best quick solution right now.

The proper one sounds like a specific initialization routine for these
ports.

Send it as a formal patch and you may add

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

P.S. Sorry, we have no parisc hardware around to test.

> 
> Helge
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c
> b/drivers/tty/serial/8250/8250_pci.c
> index 0c101a7470b0..61319e968e8c 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -3393,6 +3393,10 @@ static int
> serial_pci_is_class_communication(struct pci_dev *dev)
>  	 * (Should we try to make guesses for multiport serial
> devices
>  	 * later?)
>  	 */
> +	if (IS_ENABLED(CONFIG_PARISC) &&
> +	    (dev->class >> 8) == PCI_CLASS_COMMUNICATION_OTHER)
> +		return 0;
> +
>  	if ((((dev->class >> 8) != PCI_CLASS_COMMUNICATION_SERIAL) &&
>  	     ((dev->class >> 8) != PCI_CLASS_COMMUNICATION_MODEM)) ||
>  	    (dev->class & 0xff) > 6)

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: Have my PA8800 back online...

[Helge Deller]

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]

On 10.12.2017 00:49, John David Anglin wrote:
> On 2017-12-09, at 4:03 PM, Helge Deller wrote:
> 
>> Can you please try attached patch which disables the serial MUX and ATI card?
>> If it works for you and if we backport it to all kernels and if we revert palo to use ttyS0 for all machines we might be good.
> 
> I hacked on the change but I couldn't get it to work.  As far as I can tell, the quirks aren't being called.
> Tried EARLY, HEADER and FINAL.  I think the ids are correct.

Strange. The attached patch does work for me on panama up until boot.
Haven't tested what lspci reports afterwards...

[    1.832294] LBA 0:7: PCI host bridge to bus 0000:e0
[    1.832497] pci_bus 0000:e0: root bus resource [io  0x60000-0x6ffff] (bus address [0x0000-0xffff])
[    1.833005] pci_bus 0000:e0: root bus resource [mem 0xfffffffff0000000-0xfffffffffe77ffff] (bus address [0xf0000000-0xfe77ffff])
[    1.840028] pci_bus 0000:e0: root bus resource [bus e0-e7]
     1.844276] subsystem_vendor = 0x103c, subsystem_device =0x1291
[    1.848022] pci 0000:e0:01.0: Hiding Diva built-in AUX serial device
     1.849136] subsystem_vendor = 0x103c, subsystem_device =0x1292
[    1.852023] pci 0000:e0:02.0: Hiding Diva built-in ATI card.
....

Helge

[-- Attachment #2: p1 --]
[-- Type: text/plain, Size: 1777 bytes --]

diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index a25fed52f7e9..dbb4158cf098 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -1692,3 +1692,45 @@ void lba_set_iregs(struct parisc_device *lba, u32 ibase, u32 imask)
 	iounmap(base_addr);
 }
 
+
+/*
+ * The design of the Diva management card in rp34x0 machines (rp3410, rp3440)
+ * seems rushed, so that many built-in components simply don't work.
+ * The following quirks disable the serial AUX port and the built-in ATI RV100
+ * Radeon 7000 graphics card which both don't have any external connectors and
+ * thus are useless, and even worse, e.g. the AUX ports occupies ttyS0 and
+ * as such makes those machines the only PARISC machines on which we can't
+ * use ttyS0 as boot console.
+ */
+static void quirk_diva_ati_card(struct pci_dev *dev)
+{
+	printk("subsystem_vendor = 0x%x, subsystem_device =0x%x\n",
+		dev->subsystem_vendor, dev->subsystem_device);
+
+	/* subsystem IDs are from Diva */
+	if (dev->subsystem_vendor != PCI_VENDOR_ID_HP ||
+	    dev->subsystem_device != 0x1292)
+		return;
+
+	dev_info(&dev->dev, "Hiding Diva built-in ATI card.");
+	dev->device = 0;
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_RADEON_QY,
+	quirk_diva_ati_card);
+
+static void quirk_diva_aux_disable(struct pci_dev *dev)
+{
+	printk("subsystem_vendor = 0x%x, subsystem_device =0x%x\n",
+		dev->subsystem_vendor, dev->subsystem_device);
+
+	/* subsystem IDs are from Diva */
+	if (dev->subsystem_vendor != PCI_VENDOR_ID_HP ||
+	    dev->subsystem_device != 0x1291)
+		return;
+
+	dev_info(&dev->dev, "Hiding Diva built-in AUX serial device");
+	dev->device = 0;
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_DIVA_AUX,
+	quirk_diva_aux_disable);
+

Re: Have my PA8800 back online...

[John David Anglin]

On 2017-12-11 9:46 AM, Helge Deller wrote:
> On 10.12.2017 00:49, John David Anglin wrote:
>> On 2017-12-09, at 4:03 PM, Helge Deller wrote:
>>
>>> Can you please try attached patch which disables the serial MUX and ATI card?
>>> If it works for you and if we backport it to all kernels and if we revert palo to use ttyS0 for all machines we might be good.
>> I hacked on the change but I couldn't get it to work.  As far as I can tell, the quirks aren't being called.
>> Tried EARLY, HEADER and FINAL.  I think the ids are correct.
> Strange. The attached patch does work for me on panama up until boot.
> Haven't tested what lspci reports afterwards...
>
> [    1.832294] LBA 0:7: PCI host bridge to bus 0000:e0
> [    1.832497] pci_bus 0000:e0: root bus resource [io  0x60000-0x6ffff] (bus address [0x0000-0xffff])
> [    1.833005] pci_bus 0000:e0: root bus resource [mem 0xfffffffff0000000-0xfffffffffe77ffff] (bus address [0xf0000000-0xfe77ffff])
> [    1.840028] pci_bus 0000:e0: root bus resource [bus e0-e7]
>       1.844276] subsystem_vendor = 0x103c, subsystem_device =0x1291
> [    1.848022] pci 0000:e0:01.0: Hiding Diva built-in AUX serial device
>       1.849136] subsystem_vendor = 0x103c, subsystem_device =0x1292
> [    1.852023] pci 0000:e0:02.0: Hiding Diva built-in ATI card.
> ....

Maybe I messed up build in some way.  I didn't clean things between 
builds.  I never got the output shown above.
We have period at end of the second message.

Dave

-- 
John David Anglin  dave.anglin@bell.net

Re: Have my PA8800 back online...

[John David Anglin]

On 2017-12-11 9:46 AM, Helge Deller wrote:
> Strange. The attached patch does work for me on panama up until boot.
> Haven't tested what lspci reports afterwards...
Yes, it also works for me applied to v4.13.16.  ttyS1 is now ttyS0. 
Haven't tried boot
without console argument yet.

lspci still sees the hidden devices which I think is good.

I removed period from one of the dev_info strings.  We probably don't 
need printk's
that I added.

Dave

-- 
John David Anglin  dave.anglin@bell.net

Re: Have my PA8800 back online... (serial port missing on v4.14)

[Helge Deller]

On 11.12.2017 09:26, Andy Shevchenko wrote:
> On Fri, 2017-12-08 at 20:06 +0100, Helge Deller wrote:
>>>> Anyway, the *only* problem we have right now is, that the Linux
>>>> kernel 4.14 doesn't detect all serial ports which were detected in
>>>> earlier kernels.
> 
>>>> Thus the kernel will talk to the non-existant serial port at
>>>> 0xfffffffff4050010 instead of 0xfffffffff4050000.
> 
> Wait, from this sentence you actually confirm that patch removes *non-
> existant* ports.
> 
> Can you elaborate what you imply here?

The PCI card is a HP "Diva" card, which is basically a server 
remote management board (like HP iLO). 

With "non-existant serial port" I was referring to those few ports
on the Diva card with which one can communicate directly via a proprietary
protocol with the management card.

In older kernels (before your patch) those ports showed up as serial
ports (although they were of no use for the Linux serial driver).
With v4.14 those ports don't show up as ttyS device any longer.  
 
>>>> 4.13:
>>>> [   28.882849] Serial: 8250/16550 driver, 4 ports, IRQ sharing
>>>> enabled
>>>> [   28.898720] 0000:e0:01.0: ttyS0 at MMIO 0xfffffffff4051000 (irq
>>>> = 73, base_baud = 115200) is a 16450
>>>> [   28.934669] 0000:e0:01.1: ttyS1 at MMIO 0xfffffffff4050000 (irq
>>>> = 73, base_baud = 115200) is a 16550A
>>>> [   28.963031] 0000:e0:01.1: ttyS2 at MMIO 0xfffffffff4050010 (irq
>>>> = 73, base_baud = 115200) is a 16550A
>>>> [   28.984946] 0000:e0:01.1: ttyS3 at MMIO 0xfffffffff4050038 (irq
>>>> = 73, base_baud = 115200) is a 16550A
> 
> From here it looks like multi-function PCI device with two functions
> with 1 + 3 serial ports each.
> 
>>>> ...but for v4.14.x only the following serial ports are detected:
>>>> [   28.671984] Serial: 8250/16550 driver, 4 ports, IRQ sharing
>>>> enabled
>>>> [   28.708902] 0000:e0:01.1: ttyS0 at MMIO 0xfffffffff4050000 (irq
>>>> = 73, base_baud = 115200) is a 16550A
>>>> [   28.731145] 0000:e0:01.1: ttyS1 at MMIO 0xfffffffff4050010 (irq
>>>> = 73, base_baud = 115200) is a 16550A
> 
> I'm quite curious how ttyS0 and ttyS3 in previous run (old kernel)
> appear.
> 
>>>>
>>>>
>>>> Maybe reverting this commit brings back the old behavior:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>>>> /commit/?id=7d8905d064058f4b65057e0101588f362f288bc0
>>>
>>> I'm unsure about this commit, it speaks more of avoiding duplicate
>>> messages
>>> for device enabling.
> 
> No, it's about trying IRQs twice, though it might be not fully clear
> from commit message: the example there shows that IRQs are probed twice
> and on some platforms it may be a problem.
> 
>>
>> Reverting this commit:
>>
>> commit 7d8905d064058f4b65057e0101588f362f288bc0
>> Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Date:   Mon Jul 24 20:28:32 2017 +0300
>>
>>     serial: 8250_pci: Enable device after we check black list
>>
>> indeed fixes the problem.
>>
>> After reverting, the serial port from the Diva card shows up as ttyS0
>> (as before).
> 
>> With that patch applied, the serial port from the Diva card gets
>> ignored and the previous ttyS1 port becomes ttyS0 which then breaks
>> booting the parisc machine because the kernel expects the serial port
>> on
>> ttyS1.
> 
>> I'm not sure what the best way forward is.
>> Fact is, that the patch above changes the behaviour and serial ports
>> which were existant before suddenly vanish with kernel 4.14.
> 
> As stated in the commit message there "We can do this since PCI
> specification requires class, device and vendor ID registers to be
> always present in the configuration space."

That's OK.

> So, my understanding that the patch reveals the issue with these ports.

Your patch indirectly changes the behavior.

You check with serial_pci_is_class_communication(dev) if this serial device
is of class SERIAL or MODEM.
If it isn't you exit pciserial_init_one() and don't register the device.

Before your patch this check was inside the function serial_pci_guess_board()
and if (ent->driver_data != pbn_default) the pci serial port got registered 
and initialized *even* if it's *not* of class SERIAL or MODEM.

> (Of course, I agree this is regression and needs to be fixed ASAP)

I don't know if it's easy to fix without reverting your patch.
On the other side, your patch is correct in the sense that it avoids
registering serial ports which shouldn't be registered.
It's just that it now behaves differently and that it breaks booting
Linux on some parisc machines.
 
>> This following patch does work, and adds back the Diva serial port on
>> parisc.
> 
>> Not sure if it's acceptable though.
> 
> For me it looks like the best quick solution right now.
> 
> The proper one sounds like a specific initialization routine for these
> ports.
> 
> Send it as a formal patch and you may add
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks for the offer to accept this patch, but maybe we are able
to come up with another patch which simply hides those unsupported
devices (serial port and ATI graphics card device on the Diva card).
I posted a proposed patch here:
http://www.spinics.net/lists/linux-parisc/msg08187.html

But I wonder if we are the only platform which notice this different
behavior now. I assume others will notice it soon too.

> P.S. Sorry, we have no parisc hardware around to test.

No problem.

Thanks!
Helge
 
>>
>> Helge
>>
>> diff --git a/drivers/tty/serial/8250/8250_pci.c
>> b/drivers/tty/serial/8250/8250_pci.c
>> index 0c101a7470b0..61319e968e8c 100644
>> --- a/drivers/tty/serial/8250/8250_pci.c
>> +++ b/drivers/tty/serial/8250/8250_pci.c
>> @@ -3393,6 +3393,10 @@ static int
>> serial_pci_is_class_communication(struct pci_dev *dev)
>>  	 * (Should we try to make guesses for multiport serial
>> devices
>>  	 * later?)
>>  	 */
>> +	if (IS_ENABLED(CONFIG_PARISC) &&
>> +	    (dev->class >> 8) == PCI_CLASS_COMMUNICATION_OTHER)
>> +		return 0;
>> +
>>  	if ((((dev->class >> 8) != PCI_CLASS_COMMUNICATION_SERIAL) &&
>>  	     ((dev->class >> 8) != PCI_CLASS_COMMUNICATION_MODEM)) ||
>>  	    (dev->class & 0xff) > 6)
> 

Re: Have my PA8800 back online...

[Helge Deller]

* John David Anglin <dave.anglin@bell.net>:
> On 2017-12-11 9:46 AM, Helge Deller wrote:
> > Strange. The attached patch does work for me on panama up until boot.
> > Haven't tested what lspci reports afterwards...

> Yes, it also works for me applied to v4.13.16.  ttyS1 is now ttyS0.

Good.

> Haven't tried boot without console argument yet.
> lspci still sees the hidden devices which I think is good.

Agreed.

> I removed period from one of the dev_info strings.  We probably don't need
> printk's that I added.

Yes, I dropped them.
Here is an updated patch for patchwork:

_________

Hide serial AUX and ATI functions on Diva GSP card

Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index a25fed52f7e9..55fb30057d5e 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -1692,3 +1692,37 @@ void lba_set_iregs(struct parisc_device *lba, u32 ibase, u32 imask)
 	iounmap(base_addr);
 }
 
+
+/*
+ * The design of the Diva management card in rp34x0 machines (rp3410, rp3440)
+ * seems rushed, so that many built-in components simply don't work.
+ * The following quirks disable the serial AUX port and the built-in ATI RV100
+ * Radeon 7000 graphics card which both don't have any external connectors and
+ * thus are useless, and even worse, e.g. the AUX port occupies ttyS0 and as
+ * such makes those machines the only PARISC machines on which we can't use
+ * ttyS0 as boot console.
+ */
+static void quirk_diva_ati_card(struct pci_dev *dev)
+{
+	if (dev->subsystem_vendor != PCI_VENDOR_ID_HP ||
+	    dev->subsystem_device != 0x1292)
+		return;
+
+	dev_info(&dev->dev, "Hiding Diva built-in ATI card");
+	dev->device = 0;
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_RADEON_QY,
+	quirk_diva_ati_card);
+
+static void quirk_diva_aux_disable(struct pci_dev *dev)
+{
+	if (dev->subsystem_vendor != PCI_VENDOR_ID_HP ||
+	    dev->subsystem_device != 0x1291)
+		return;
+
+	dev_info(&dev->dev, "Hiding Diva built-in AUX serial device");
+	dev->device = 0;
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_DIVA_AUX,
+	quirk_diva_aux_disable);
+

Re: Have my PA8800 back online... (serial port missing on v4.14)

[Andy Shevchenko]

On Tue, 2017-12-12 at 21:11 +0100, Helge Deller wrote:
> On 11.12.2017 09:26, Andy Shevchenko wrote:
> > On Fri, 2017-12-08 at 20:06 +0100, Helge Deller wrote:

> Before your patch this check was inside the function
> serial_pci_guess_board()
> and if (ent->driver_data != pbn_default) the pci serial port got
> registered 
> and initialized *even* if it's *not* of class SERIAL or MODEM.

Ah, okay, it explains indeed.
Though PCI devices with wrong class should have their own quirks for my
p.o.v.

> > (Of course, I agree this is regression and needs to be fixed ASAP)
> 
> I don't know if it's easy to fix without reverting your patch.

As I explained earlier it's about pci_enable_device() called twice for
the same device which basically calls pcibios_enable_irq() twice which
might be a problem on some platforms. (At least I have such use case).
Perhaps it's possible to workaround the issue on those platforms, though
I didn't come up with the better solution that time.

> Thanks for the offer to accept this patch, but maybe we are able
> to come up with another patch which simply hides those unsupported
> devices (serial port and ATI graphics card device on the Diva card).
> I posted a proposed patch here:
> http://www.spinics.net/lists/linux-parisc/msg08187.html

Reading briefly that one I guess it's even better (now I realized you
even do not have connectors of those devices outside).

> But I wonder if we are the only platform which notice this different
> behavior now. I assume others will notice it soon too.

Why so? Most of the 8250 PCI devices are enumerated by class. Others
have no such misclassification.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: Have my PA8800 back online... (serial port missing on v4.14)

[Helge Deller]

Hi Andy,

On 13.12.2017 16:16, Andy Shevchenko wrote:
> On Tue, 2017-12-12 at 21:11 +0100, Helge Deller wrote:
>> On 11.12.2017 09:26, Andy Shevchenko wrote:
>>> On Fri, 2017-12-08 at 20:06 +0100, Helge Deller wrote:
> 
>> Before your patch this check was inside the function
>> serial_pci_guess_board()
>> and if (ent->driver_data != pbn_default) the pci serial port got
>> registered 
>> and initialized *even* if it's *not* of class SERIAL or MODEM.
> 
> Ah, okay, it explains indeed.
> Though PCI devices with wrong class should have their own quirks for my
> p.o.v.
> 
>>> (Of course, I agree this is regression and needs to be fixed ASAP)
>>
>> I don't know if it's easy to fix without reverting your patch.
> 
> As I explained earlier it's about pci_enable_device() called twice for
> the same device which basically calls pcibios_enable_irq() twice which
> might be a problem on some platforms. (At least I have such use case).
> Perhaps it's possible to workaround the issue on those platforms, though
> I didn't come up with the better solution that time.
> 
>> Thanks for the offer to accept this patch, but maybe we are able
>> to come up with another patch which simply hides those unsupported
>> devices (serial port and ATI graphics card device on the Diva card).

>> I posted a proposed patch here:
>> http://www.spinics.net/lists/linux-parisc/msg08187.html
> 
> Reading briefly that one I guess it's even better (now I realized you
> even do not have connectors of those devices outside).

It's now fixed for parisc by new PCI quirks which
disable the parisc serial AUX port: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bcf3f1752a622f1372d3252d0fea8855d89812e7

Thanks,
Helge

Re: Have my PA8800 back online... (serial port missing on v4.14)

[Andy Shevchenko]

On Mon, 2017-12-18 at 21:07 +0100, Helge Deller wrote:
> Hi Andy,
> 
> On 13.12.2017 16:16, Andy Shevchenko wrote:
> > On Tue, 2017-12-12 at 21:11 +0100, Helge Deller wrote:
> > > On 11.12.2017 09:26, Andy Shevchenko wrote:
> > > > 
> > > Thanks for the offer to accept this patch, but maybe we are able
> > > to come up with another patch which simply hides those unsupported
> > > devices (serial port and ATI graphics card device on the Diva
> > > card).
> > > I posted a proposed patch here:
> > > http://www.spinics.net/lists/linux-parisc/msg08187.html
> > 
> > Reading briefly that one I guess it's even better (now I realized
> > you
> > even do not have connectors of those devices outside).
> 
> It's now fixed for parisc by new PCI quirks which
> disable the parisc serial AUX port: 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/com
> mit/?id=bcf3f1752a622f1372d3252d0fea8855d89812e7

Thank you for taking care of this.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy