2018-05-26 19:36:07

by Matt Turner

[permalink] [raw]
Subject: [PATCH] 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]>
Signed-off-by: Matt Turner <[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(+)

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



2018-06-06 06:11:19

by Matt Turner

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

On Sat, May 26, 2018 at 12:35 PM, Matt Turner <[email protected]> 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 <[email protected]>
> Signed-off-by: Matt Turner <[email protected]>
> ---

Hi,

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

Matt

2018-06-06 17:07:11

by Andy Shevchenko

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

On Wed, Jun 6, 2018 at 9:07 AM, Matt Turner <[email protected]> wrote:
> On Sat, May 26, 2018 at 12:35 PM, Matt Turner <[email protected]> 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 <[email protected]>
>> Signed-off-by: Matt Turner <[email protected]>
>> ---
>
> Hi,
>
> Can I expect this to be picked up for the 4.18 merge window?
>
> Matt



--
With Best Regards,
Andy Shevchenko

2018-06-06 17:08:19

by Andy Shevchenko

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

On Wed, Jun 6, 2018 at 8:04 PM, Andy Shevchenko
<[email protected]> wrote:
> On Wed, Jun 6, 2018 at 9:07 AM, Matt Turner <[email protected]> wrote:
>> On Sat, May 26, 2018 at 12:35 PM, Matt Turner <[email protected]> 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

2018-06-06 19:08:52

by Andy Shevchenko

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

On Sat, May 26, 2018 at 10:35 PM, Matt Turner <[email protected]> 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

2018-07-05 22:18:58

by Sudip Mukherjee

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

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 <[email protected]>
> Signed-off-by: Matt Turner <[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?
>

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

2018-07-06 15:36:13

by Matt Turner

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

On Thu, Jul 5, 2018 at 3:17 PM Sudip Mukherjee
<[email protected]> 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 <[email protected]>
> > Signed-off-by: Matt Turner <[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?
> >
>
> 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