Skip to contentSkip to conversation

parport: Add support for the WCH384 4S multi-IO card

PATCH In review Matt Turner 3 participants 7 messages May 26, 2018 – Jul 6, 2018 Parallel port

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 parport: Add support for the WCH384 4S multi-IO card

This Multi-IO card has one serial 16550-like serial connectors. Here's the lspci output, after this commit is applied:

01:00.0 Serial controller [0700]: Device [1c00:3470] (rev 10) (prog-if 05 [16850])

        Subsystem: Device [1c00:3470]
        Flags: fast devsel, IRQ 16
        I/O ports at e000 [size=256]
        Memory at f0100000 (32-bit, prefetchable) [size=32K]
        I/O ports at e100 [size=4]
        Expansion ROM at f7d00000 [disabled] [size=32K]
        Capabilities: [60] Power Management version 3
        Capabilities: [68] MSI: Enable- Count=1/32 Maskable+ 64bit+
        Capabilities: [80] Express Legacy Endpoint, MSI 00
        Capabilities: [100] Advanced Error Reporting
        Kernel driver in use: parport_serial

This card was already added to the blacklist of 8250_pci.c in commit 72a3c0e4e662 ("tty: Add support for the WCH384 4S multi-IO card").

Cc Sergej Pupykin <[email protected]>

---
It looks like CH355_4S is similarly missing, but I don't have hardware to test.

This commit makes me wonder if I'm missing something -- how could anything have worked after commit 72a3c0e4e662 without support in parport_serial?

 drivers/parport/parport_serial.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/parport/parport_serial.c +10 -0
index e15b4845f7c6..2c166d5a0d91 100644
@@ -65,6 +65,7 @@ enum parport_pc_pci_cards {
 	wch_ch353_1s1p,
 	wch_ch353_2s1p,
 	wch_ch382_2s1p,
+	wch_ch384_4,
 	sunix_2s1p,
 };
 
@@ -153,6 +154,7 @@ static struct parport_pc_pci cards[] = {
 	/* wch_ch353_1s1p*/             { 1, { { 1, -1}, } },
 	/* wch_ch353_2s1p*/             { 1, { { 2, -1}, } },
 	/* wch_ch382_2s1p*/             { 1, { { 2, -1}, } },
+	/* wch_ch384_4 */               { 1, { { 4, -1}, } },
 	/* sunix_2s1p */                { 1, { { 3, -1 }, } },
 };
 
@@ -260,6 +262,7 @@ static struct pci_device_id parport_serial_pci_tbl[] = {
 	{ 0x4348, 0x5053, PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch353_1s1p},
 	{ 0x4348, 0x7053, 0x4348, 0x3253, 0, 0, wch_ch353_2s1p},
 	{ 0x1c00, 0x3250, 0x1c00, 0x3250, 0, 0, wch_ch382_2s1p},
+	{ 0x1c00, 0x3470, PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch384_4},
 
 	/*
 	 * More SUNIX variations. At least one of these has part number
@@ -504,6 +507,13 @@ static struct pciserial_board pci_parport_serial_boards[] = {
 		.uart_offset    = 8,
 		.first_offset   = 0xC0,
 	},
+	[wch_ch384_4] = {
+		.flags          = FL_BASE0,
+		.num_ports      = 4,
+		.base_baud      = 115200,
+		.uart_offset    = 8,
+		.first_offset   = 0xC0,
+	},
 	[sunix_2s1p] = {
 		.flags		= FL_BASE0|FL_BASE_BARS,
 		.num_ports	= 2,
Signature · 1 line
2.16.1
#1 lore raw
Quoted 16 lines · Matt Turner

This Multi-IO card has one serial 16550-like serial connectors. Here's the lspci output, after this commit is applied:

01:00.0 Serial controller [0700]: Device [1c00:3470] (rev 10) (prog-if 05 [16850])

        Subsystem: Device [1c00:3470]
        Flags: fast devsel, IRQ 16
        I/O ports at e000 [size=256]
        Memory at f0100000 (32-bit, prefetchable) [size=32K]
        I/O ports at e100 [size=4]
        Expansion ROM at f7d00000 [disabled] [size=32K]
        Capabilities: [60] Power Management version 3
        Capabilities: [68] MSI: Enable- Count=1/32 Maskable+ 64bit+
        Capabilities: [80] Express Legacy Endpoint, MSI 00
        Capabilities: [100] Advanced Error Reporting
        Kernel driver in use: parport_serial
Quoted 6 lines

This card was already added to the blacklist of 8250_pci.c in commit 72a3c0e4e662 ("tty: Add support for the WCH384 4S multi-IO card").

Cc Sergej Pupykin <[email protected]>

---

Hi,

Can I expect this to be picked up for the 4.18 merge window?

Matt

#2 lore raw
Quoted 17 lines · Matt Turner
Quoted 16 lines · Matt Turner

This Multi-IO card has one serial 16550-like serial connectors. Here's the lspci output, after this commit is applied:

01:00.0 Serial controller [0700]: Device [1c00:3470] (rev 10) (prog-if 05 [16850])

        Subsystem: Device [1c00:3470]
        Flags: fast devsel, IRQ 16
        I/O ports at e000 [size=256]
        Memory at f0100000 (32-bit, prefetchable) [size=32K]
        I/O ports at e100 [size=4]
        Expansion ROM at f7d00000 [disabled] [size=32K]
        Capabilities: [60] Power Management version 3
        Capabilities: [68] MSI: Enable- Count=1/32 Maskable+ 64bit+
        Capabilities: [80] Express Legacy Endpoint, MSI 00
        Capabilities: [100] Advanced Error Reporting
        Kernel driver in use: parport_serial
Quoted 2 lines
Quoted 2 lines

This card was already added to the blacklist of 8250_pci.c in commit 72a3c0e4e662 ("tty: Add support for the WCH384 4S multi-IO card").

First of all, this patch is in conflict with tty-next AFAICS.

Second, what card w/o parport should do in parport_serial?

Third, this patch brings more mess: either you should remove the code from 8250_pci, or do not add it here. See below.

it looks like the mentioned patch mistakenly added it to blacklist by unknown reason.

Feels to me like NAK to this one.

And btw, you missed s letter in the model name across the code and I don't see any of cards there listed with numports == 0.

Quoted 10 lines
Quoted 4 lines
Cc Sergej Pupykin <[email protected]>

---

Hi,

Can I expect this to be picked up for the 4.18 merge window?

Matt

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#3 lore raw
Quoted 28 lines · Andy Shevchenko
Quoted 17 lines · Matt Turner
Quoted 16 lines · Matt Turner

This Multi-IO card has one serial 16550-like serial connectors. Here's the lspci output, after this commit is applied:

01:00.0 Serial controller [0700]: Device [1c00:3470] (rev 10) (prog-if 05 [16850])

        Subsystem: Device [1c00:3470]
        Flags: fast devsel, IRQ 16
        I/O ports at e000 [size=256]
        Memory at f0100000 (32-bit, prefetchable) [size=32K]
        I/O ports at e100 [size=4]
        Expansion ROM at f7d00000 [disabled] [size=32K]
        Capabilities: [60] Power Management version 3
        Capabilities: [68] MSI: Enable- Count=1/32 Maskable+ 64bit+
        Capabilities: [80] Express Legacy Endpoint, MSI 00
        Capabilities: [100] Advanced Error Reporting
        Kernel driver in use: parport_serial
Quoted 2 lines
Quoted 2 lines

This card was already added to the blacklist of 8250_pci.c in commit 72a3c0e4e662 ("tty: Add support for the WCH384 4S multi-IO card").

First of all, this patch is in conflict with tty-next AFAICS.

Second, what card w/o parport should do in parport_serial?

Third, this patch brings more mess: either you should remove the code from 8250_pci, or do not add it here. See below.

Quoted 7 lines

it looks like the mentioned patch mistakenly added it to blacklist by unknown reason.

Feels to me like NAK to this one.

And btw, you missed s letter in the model name across the code and I don't see any of cards there listed with numports == 0.

Ah, it seems that you have two identical VID:DID devices with either numports == 0 or 1.

First one should be served by 8250_pci. The second one indeed is good to have here.

Though, how to distinguish them? By PCI class? by sVID:sDID?

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#4 lore raw
In reply to Matt Turner
Quoted 6 lines · Matt Turner

It looks like CH355_4S is similarly missing, but I don't have hardware to test.

This commit makes me wonder if I'm missing something -- how could anything have worked after commit 72a3c0e4e662 without support in parport_serial?

I guess that commit actually worked and the culprit would be the following one which changes logic of black list application.

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

I will send a patch to remove stalled entries from blacklist.

Quoted 49 lines
 drivers/parport/parport_serial.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/parport/parport_serial.c +10 -1
index e15b4845f7c6..2c166d5a0d91 100644
@@ -65,6 +65,7 @@ enum parport_pc_pci_cards {
        wch_ch353_1s1p,
        wch_ch353_2s1p,
        wch_ch382_2s1p,
+       wch_ch384_4,
        sunix_2s1p,
 };
 
@@ -153,6 +154,7 @@ static struct parport_pc_pci cards[] = {
        /* wch_ch353_1s1p*/             { 1, { { 1, -1}, } },
        /* wch_ch353_2s1p*/             { 1, { { 2, -1}, } },
        /* wch_ch382_2s1p*/             { 1, { { 2, -1}, } },
+       /* wch_ch384_4 */               { 1, { { 4, -1}, } },
        /* sunix_2s1p */                { 1, { { 3, -1 }, } },
 };
 
@@ -260,6 +262,7 @@ static struct pci_device_id parport_serial_pci_tbl[] = {
        { 0x4348, 0x5053, PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch353_1s1p},
        { 0x4348, 0x7053, 0x4348, 0x3253, 0, 0, wch_ch353_2s1p},
        { 0x1c00, 0x3250, 0x1c00, 0x3250, 0, 0, wch_ch382_2s1p},
+       { 0x1c00, 0x3470, PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch384_4},
 
        /*
         * More SUNIX variations. At least one of these has part number
@@ -504,6 +507,13 @@ static struct pciserial_board pci_parport_serial_boards[] = {
                .uart_offset    = 8,
                .first_offset   = 0xC0,
        },
+       [wch_ch384_4] = {
+               .flags          = FL_BASE0,
+               .num_ports      = 4,
+               .base_baud      = 115200,
+               .uart_offset    = 8,
+               .first_offset   = 0xC0,
+       },
        [sunix_2s1p] = {
                .flags          = FL_BASE0|FL_BASE_BARS,
                .num_ports      = 2,
--

2.16.1

Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#5 lore raw
In reply to Matt Turner

Hi Matt,

Quoted 21 lines · Matt Turner

This Multi-IO card has one serial 16550-like serial connectors. Here's the lspci output, after this commit is applied:

01:00.0 Serial controller [0700]: Device [1c00:3470] (rev 10) (prog-if 05 [16850])

        Subsystem: Device [1c00:3470]
        Flags: fast devsel, IRQ 16
        I/O ports at e000 [size=256]
        Memory at f0100000 (32-bit, prefetchable) [size=32K]
        I/O ports at e100 [size=4]
        Expansion ROM at f7d00000 [disabled] [size=32K]
        Capabilities: [60] Power Management version 3
        Capabilities: [68] MSI: Enable- Count=1/32 Maskable+ 64bit+
        Capabilities: [80] Express Legacy Endpoint, MSI 00
        Capabilities: [100] Advanced Error Reporting
        Kernel driver in use: parport_serial

This card was already added to the blacklist of 8250_pci.c in commit 72a3c0e4e662 ("tty: Add support for the WCH384 4S multi-IO card").

Cc Sergej Pupykin <[email protected]>
Quoted 8 lines

---
It looks like CH355_4S is similarly missing, but I don't have hardware to test.

This commit makes me wonder if I'm missing something -- how could anything have worked after commit 72a3c0e4e662 without support in parport_serial?

Is the patch still needed to be applied? After Andy's patch to tty tree, the device (0x1c00, 0x3470) will be probed by 8250_pci.c.

--
Regards
Sudip

#6 lore raw
Quoted 29 lines · Sudip Mukherjee

Hi Matt,

Quoted 25 lines · Matt Turner

This Multi-IO card has one serial 16550-like serial connectors. Here's the lspci output, after this commit is applied:

01:00.0 Serial controller [0700]: Device [1c00:3470] (rev 10) (prog-if 05 [16850])

        Subsystem: Device [1c00:3470]
        Flags: fast devsel, IRQ 16
        I/O ports at e000 [size=256]
        Memory at f0100000 (32-bit, prefetchable) [size=32K]
        I/O ports at e100 [size=4]
        Expansion ROM at f7d00000 [disabled] [size=32K]
        Capabilities: [60] Power Management version 3
        Capabilities: [68] MSI: Enable- Count=1/32 Maskable+ 64bit+
        Capabilities: [80] Express Legacy Endpoint, MSI 00
        Capabilities: [100] Advanced Error Reporting
        Kernel driver in use: parport_serial

This card was already added to the blacklist of 8250_pci.c in commit 72a3c0e4e662 ("tty: Add support for the WCH384 4S multi-IO card").

Cc Sergej Pupykin <[email protected]>

---
It looks like CH355_4S is similarly missing, but I don't have hardware to test.

Quoted 7 lines
Quoted 4 lines

This commit makes me wonder if I'm missing something -- how could anything have worked after commit 72a3c0e4e662 without support in parport_serial?

Is the patch still needed to be applied? After Andy's patch to tty tree, the device (0x1c00, 0x3470) will be probed by 8250_pci.c.

Yes, Andy's patch replaces this one. It can be discarded.

Thanks!
Matt