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

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

[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 <ml@sergej.pp.ru>
Signed-off-by: Matt Turner <mattst88@gmail.com>
---
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(+)

diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c
index e15b4845f7c6..2c166d5a0d91 100644
--- a/drivers/parport/parport_serial.c
+++ b/drivers/parport/parport_serial.c
@@ -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

Re: [PATCH] parport: Add support for the WCH384 4S multi-IO card

[Matt Turner]

On Sat, May 26, 2018 at 12:35 PM, Matt Turner <mattst88@gmail.com> wrote:
> 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 <ml@sergej.pp.ru>
> Signed-off-by: Matt Turner <mattst88@gmail.com>
> ---

Hi,

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

Matt

Re: [PATCH] parport: Add support for the WCH384 4S multi-IO card

[Andy Shevchenko]

On Wed, Jun 6, 2018 at 9:07 AM, Matt Turner <mattst88@gmail.com> wrote:
> On Sat, May 26, 2018 at 12:35 PM, Matt Turner <mattst88@gmail.com> wrote:
>> 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").

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.

>>
>> Cc: Sergej Pupykin <ml@sergej.pp.ru>
>> Signed-off-by: Matt Turner <mattst88@gmail.com>
>> ---
>
> Hi,
>
> Can I expect this to be picked up for the 4.18 merge window?
>
> Matt



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] parport: Add support for the WCH384 4S multi-IO card

[Andy Shevchenko]

On Wed, Jun 6, 2018 at 8:04 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Jun 6, 2018 at 9:07 AM, Matt Turner <mattst88@gmail.com> wrote:
>> On Sat, May 26, 2018 at 12:35 PM, Matt Turner <mattst88@gmail.com> wrote:
>>> 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").
>
> 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.

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?


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] parport: Add support for the WCH384 4S multi-IO card

[Andy Shevchenko]

On Sat, May 26, 2018 at 10:35 PM, Matt Turner <mattst88@gmail.com> wrote:

> 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.

>
>  drivers/parport/parport_serial.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c
> index e15b4845f7c6..2c166d5a0d91 100644
> --- a/drivers/parport/parport_serial.c
> +++ b/drivers/parport/parport_serial.c
> @@ -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
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] parport: Add support for the WCH384 4S multi-IO card

[Sudip Mukherjee]

Hi Matt,

On Sat, May 26, 2018 at 12:35:23PM -0700, Matt Turner wrote:
> 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 <ml@sergej.pp.ru>
> Signed-off-by: Matt Turner <mattst88@gmail.com>
> ---
> 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

Re: [PATCH] parport: Add support for the WCH384 4S multi-IO card

[Matt Turner]

On Thu, Jul 5, 2018 at 3:17 PM Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
>
> Hi Matt,
>
> On Sat, May 26, 2018 at 12:35:23PM -0700, Matt Turner wrote:
> > 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 <ml@sergej.pp.ru>
> > Signed-off-by: Matt Turner <mattst88@gmail.com>
> > ---
> > 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.

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

Thanks!
Matt