unrecognized multi io pci card

PATCH In review gianluca 3 participants 7 messages Aug 28, 2012 – Sep 6, 2012

Subsystems (2)

TTY layer and serial drivers Parallel port

lore

Share & tools

lore

gianluca

author

2012-08-28 12:30 UTC #0 lore raw

Hello,

I recently bought a pci card with two serial ports and one parallel port since my machine didn't have any, but the card isn't autodetected by linux.

Here follows a lspci -vvvxxx of the device

5:01.0 Serial controller: Device 4348:7053 (rev 10) (prog-if 02 [16550])
Subsystem: Device 4348:3253
Control: I/O+ Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Interrupt: pin A routed to IRQ 17
Region 0: I/O ports at ec00 [size=8]
Region 1: I/O ports at e880 [size=8]
Region 2: I/O ports at e800 [size=8]
Region 3: I/O ports at e480 [size=16]
Kernel driver in use: parport_serial
00: 48 43 53 70 01 00 00 02 10 02 00 07 00 00 00 00
10: 01 ec 00 00 81 e8 00 00 01 e8 00 00 81 e4 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 48 43 53 32
30: 00 00 00 00 00 00 00 00 00 00 00 00 0a 01 00 00

I tried to patch parport_serial.c which, as I understand, is suited to provide all the parallel and serial ports to userspace. That driver is unmantained unfortunately but since the card is also a serial card I'm writing to you and this list to get feedback on what I've done.

After adding the IDs and descriptions I found unfortunately that the module 8250_pci reclaims the pci device as a serial card only and this prevents the successful registration of the parport_serial driver (which also depends on 8250_pci).

So I blacklisted the card in 8250_pci.c using the sofmodem blacklist mechanism already in place. I know this isn't really the right place to do it but it works and I didn't find another way.

Eventually the parallel and the serial ports are detected but only the parallel port works because the serial ports are wrongly recognized as XScale when they are actually 16550 clones: if I set the port type by hand with setserial they work.

So I changed the autoconfig function in 8250.c this way: checks also the RTOIE bit in IER to detect the port as XSCALE. I don't think this is really correct but I've no knowledge on XSCALE or my serial card hardware to do it better.

After this last change the ports are detected and work flawlessly.

In attachment you'll find all the patches.

Thank you.

drivers/parport/parport_serial.c +9 -1

index e9c3227..a8d65a2 100644
@@ -62,6 +62,7 @@ enum parport_pc_pci_cards {
 	timedia_9079a,
 	timedia_9079b,
 	timedia_9079c,
+	wch_ch353_2s1p,
 };
 
 /* each element directly indexed from enum list, above */

@@ -145,6 +146,7 @@ static struct parport_pc_pci cards[] __devinitdata = {
 	/* timedia_9079a */             { 1, { { 2, 3 }, } },
 	/* timedia_9079b */             { 1, { { 2, 3 }, } },
 	/* timedia_9079c */             { 1, { { 2, 3 }, } },
+	/* wch_ch353_2s1p*/             { 1, { { 2, -1}, } },
 };
 
 static struct pci_device_id parport_serial_pci_tbl[] = {

@@ -243,7 +245,7 @@ static struct pci_device_id parport_serial_pci_tbl[] = {
 	{ 0x1409, 0x7168, 0x1409, 0xb079, 0, 0, timedia_9079a },
 	{ 0x1409, 0x7168, 0x1409, 0xc079, 0, 0, timedia_9079b },
 	{ 0x1409, 0x7168, 0x1409, 0xd079, 0, 0, timedia_9079c },
-
+	{ 0x4348, 0x7053, 0x4348, 0x3253, 0, 0, wch_ch353_2s1p},
 	{ 0, } /* terminate list */
 };
 MODULE_DEVICE_TABLE(pci,parport_serial_pci_tbl);

@@ -460,6 +462,12 @@ static struct pciserial_board pci_parport_serial_boards[] __devinitdata = {
 		.base_baud	= 921600,
 		.uart_offset	= 8,
 	},
+	[wch_ch353_2s1p] = {
+		.flags          = FL_BASE0|FL_BASE_BARS,
+		.num_ports      = 2,
+		.base_baud      = 115200,
+		.uart_offset    = 8,
+	},
 };
 
 struct parport_serial_private {

drivers/parport/parport_serial.c +9 -1

index e9c3227..a8d65a2 100644
@@ -62,6 +62,7 @@ enum parport_pc_pci_cards {
 	timedia_9079a,
 	timedia_9079b,
 	timedia_9079c,
+	wch_ch353_2s1p,
 };
 
 /* each element directly indexed from enum list, above */

@@ -145,6 +146,7 @@ static struct parport_pc_pci cards[] __devinitdata = {
 	/* timedia_9079a */             { 1, { { 2, 3 }, } },
 	/* timedia_9079b */             { 1, { { 2, 3 }, } },
 	/* timedia_9079c */             { 1, { { 2, 3 }, } },
+	/* wch_ch353_2s1p*/             { 1, { { 2, -1}, } },
 };
 
 static struct pci_device_id parport_serial_pci_tbl[] = {

@@ -243,7 +245,7 @@ static struct pci_device_id parport_serial_pci_tbl[] = {
 	{ 0x1409, 0x7168, 0x1409, 0xb079, 0, 0, timedia_9079a },
 	{ 0x1409, 0x7168, 0x1409, 0xc079, 0, 0, timedia_9079b },
 	{ 0x1409, 0x7168, 0x1409, 0xd079, 0, 0, timedia_9079c },
-
+	{ 0x4348, 0x7053, 0x4348, 0x3253, 0, 0, wch_ch353_2s1p},
 	{ 0, } /* terminate list */
 };
 MODULE_DEVICE_TABLE(pci,parport_serial_pci_tbl);

@@ -460,6 +462,12 @@ static struct pciserial_board pci_parport_serial_boards[] __devinitdata = {
 		.base_baud	= 921600,
 		.uart_offset	= 8,
 	},
+	[wch_ch353_2s1p] = {
+		.flags          = FL_BASE0|FL_BASE_BARS,
+		.num_ports      = 2,
+		.base_baud      = 115200,
+		.uart_offset    = 8,
+	},
 };
 
 struct parport_serial_private {

drivers/tty/serial/8250/8250_pci.c +1 -0

index 28e7c7c..61aadfc 100644
@@ -2628,6 +2628,7 @@ static const struct pci_device_id softmodem_blacklist[] = {
 	{ PCI_VDEVICE(AL, 0x5457), }, /* ALi Corporation M5457 AC'97 Modem */
 	{ PCI_VDEVICE(MOTOROLA, 0x3052), }, /* Motorola Si3052-based modem */
 	{ PCI_DEVICE(0x1543, 0x3052), }, /* Si3052-based modem, default IDs */
+	{ PCI_DEVICE(0x4348, 0x7053), }, /* wch ch353 2S1P */
 };
 
 /*

drivers/tty/serial/8250/8250.c +11 -8

index 6e1958a..9693d5a 100644
@@ -980,14 +980,17 @@ static void autoconfig_16550a(struct uart_8250_port *up)
 		 */
 		serial_out(up, UART_IER, iersave | UART_IER_UUE);
 		if (serial_in(up, UART_IER) & UART_IER_UUE) {
-			/*
-			 * It's an Xscale.
-			 * We'll leave the UART_IER_UUE bit set to 1 (enabled).
-			 */
-			DEBUG_AUTOCONF("Xscale ");
-			up->port.type = PORT_XSCALE;
-			up->capabilities |= UART_CAP_UUE | UART_CAP_RTOIE;
-			return;
+			serial_out(up, UART_IER, iersave | UART_IER_RTOIE);
+			if (serial_in(up, UART_IER) & UART_IER_RTOIE) {
+				/*
+				 * It's an Xscale.
+				 * We'll leave the UART_IER_UUE bit set to 1 (enabled).
+				 */
+				DEBUG_AUTOCONF("Xscale ");
+				up->port.type = PORT_XSCALE;
+				up->capabilities |= UART_CAP_UUE | UART_CAP_RTOIE;
+				return;
+			}
 		}
 	} else {
 		/*

Alan Cox

2012-08-28 12:50 UTC #1 lore raw

Quoted 8 lines

After adding the IDs and descriptions I found unfortunately that the module 8250_pci reclaims the pci device as a serial card only and this prevents the successful registration of the parport_serial driver (which also depends on 8250_pci).
So I blacklisted the card in 8250_pci.c using the sofmodem blacklist mechanism already in place. I know this isn't really the right place to do it but it works and I didn't find another way.

Actually I think it is the right place to do it. Prior to that the only cases we've had to deal with are board specifc rules such as for the Timedia/Sunix card. The softmodem bit of the blacklist wants renaming and a comment adding to each group giving the reason.

Quoted 7 lines

Eventually the parallel and the serial ports are detected but only the parallel port works because the serial ports are wrongly recognized as XScale when they are actually 16550 clones: if I set the port type by hand with setserial they work.
So I changed the autoconfig function in 8250.c this way: checks also the RTOIE bit in IER to detect the port as XSCALE. I don't think this is really correct but I've no knowledge on XSCALE or my serial card hardware to do it better.

Trying to get autodetection right is always a little bit exciting and one device tends to break another.

In this case you know the device type and have a PCI identifier you should be able to set

	up->port.flags |= UPF_FIXED_TYPE;
	up->port.type = PORT_16550A;

before the call to serial8250_register_8250_port();

Alan

gianluca

author

2012-08-28 13:57 UTC #2 lore raw

Quoted 4 lines · Alan Cox

Actually I think it is the right place to do it. Prior to that the only cases we've had to deal with are board specifc rules such as for the Timedia/Sunix card. The softmodem bit of the blacklist wants renaming and a comment adding to each group giving the reason.

I renamed it to "blacklist"

Quoted 6 lines

Trying to get autodetection right is always a little bit exciting and one device tends to break another.
In this case you know the device type and have a PCI identifier you should be able to set

Quoted 6 lines

	up->port.flags |= UPF_FIXED_TYPE;
	up->port.type = PORT_16550A;
before the call to serial8250_register_8250_port();
Alan

I see, I missed the quirks in 8250_pci.c. I added a quirk for my card to set the options above and it works nicely thanks!

The new patch is attached.

Gianluca

drivers/parport/parport_serial.c +10 -1

index e9c3227..1631eea 100644
@@ -62,6 +62,7 @@ enum parport_pc_pci_cards {
 	timedia_9079a,
 	timedia_9079b,
 	timedia_9079c,
+	wch_ch353_2s1p,
 };
 
 /* each element directly indexed from enum list, above */

@@ -145,6 +146,7 @@ static struct parport_pc_pci cards[] __devinitdata = {
 	/* timedia_9079a */             { 1, { { 2, 3 }, } },
 	/* timedia_9079b */             { 1, { { 2, 3 }, } },
 	/* timedia_9079c */             { 1, { { 2, 3 }, } },
+	/* wch_ch353_2s1p*/             { 1, { { 2, -1}, } },
 };
 
 static struct pci_device_id parport_serial_pci_tbl[] = {

@@ -243,7 +245,8 @@ static struct pci_device_id parport_serial_pci_tbl[] = {
 	{ 0x1409, 0x7168, 0x1409, 0xb079, 0, 0, timedia_9079a },
 	{ 0x1409, 0x7168, 0x1409, 0xc079, 0, 0, timedia_9079b },
 	{ 0x1409, 0x7168, 0x1409, 0xd079, 0, 0, timedia_9079c },
-
+	/* WCH CARDS */
+	{ 0x4348, 0x7053, 0x4348, 0x3253, 0, 0, wch_ch353_2s1p},
 	{ 0, } /* terminate list */
 };
 MODULE_DEVICE_TABLE(pci,parport_serial_pci_tbl);

@@ -460,6 +463,12 @@ static struct pciserial_board pci_parport_serial_boards[] __devinitdata = {
 		.base_baud	= 921600,
 		.uart_offset	= 8,
 	},
+	[wch_ch353_2s1p] = {
+		.flags          = FL_BASE0|FL_BASE_BARS,
+		.num_ports      = 2,
+		.base_baud      = 115200,
+		.uart_offset    = 8,
+	},
 };
 
 struct parport_serial_private {

drivers/tty/serial/8250/8250_pci.c +30 -8

index 28e7c7c..226b4db 100644
@@ -1157,6 +1157,16 @@ pci_xr17c154_setup(struct serial_private *priv,
 	return pci_default_setup(priv, board, port, idx);
 }
 
+static int
+pci_wch_ch353_setup(struct serial_private *priv,
+                    const struct pciserial_board *board,
+                    struct uart_port *port, int idx)
+{
+	port->flags |= UPF_FIXED_TYPE;
+	port->type = PORT_16550A;
+	return pci_default_setup(priv, board, port, idx);
+}
+
 #define PCI_VENDOR_ID_SBSMODULARIO	0x124B
 #define PCI_SUBVENDOR_ID_SBSMODULARIO	0x124B
 #define PCI_DEVICE_ID_OCTPRO		0x0001

@@ -1727,6 +1737,14 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = {
 		.subdevice	= PCI_ANY_ID,
 		.setup		= pci_omegapci_setup,
 	 },
+	/* WCH CH353 2S1P card (16550 clone) */
+	{
+		.vendor         = 0x4348,
+		.device         = 0x7053,
+		.subvendor      = 0x4348,
+		.subdevice      = 0x3253,
+		.setup          = pci_wch_ch353_setup,
+	},
 	/*
 	 * Default "match everything" terminator entry
 	 */

@@ -2624,10 +2642,14 @@ static struct pciserial_board pci_boards[] __devinitdata = {
 	},
 };
 
-static const struct pci_device_id softmodem_blacklist[] = {
+static const struct pci_device_id blacklist[] = {
+	/* softmodems */
 	{ PCI_VDEVICE(AL, 0x5457), }, /* ALi Corporation M5457 AC'97 Modem */
 	{ PCI_VDEVICE(MOTOROLA, 0x3052), }, /* Motorola Si3052-based modem */
 	{ PCI_DEVICE(0x1543, 0x3052), }, /* Si3052-based modem, default IDs */
+
+	/* multi-io cards handled by parport_serial */
+	{ PCI_DEVICE(0x4348, 0x7053), }, /* WCH CH353 2S1P */
 };
 
 /*

@@ -2638,7 +2660,7 @@ static const struct pci_device_id softmodem_blacklist[] = {
 static int __devinit
 serial_pci_guess_board(struct pci_dev *dev, struct pciserial_board *board)
 {
-	const struct pci_device_id *blacklist;
+	const struct pci_device_id *bldev;
 	int num_iomem, num_port, first_port = -1, i;
 
 	/*

@@ -2655,13 +2677,13 @@ serial_pci_guess_board(struct pci_dev *dev, struct pciserial_board *board)
 
 	/*
 	 * Do not access blacklisted devices that are known not to
-	 * feature serial ports.
+	 * feature serial ports or are handled by other modules.
 	 */
-	for (blacklist = softmodem_blacklist;
-	     blacklist < softmodem_blacklist + ARRAY_SIZE(softmodem_blacklist);
-	     blacklist++) {
-		if (dev->vendor == blacklist->vendor &&
-		    dev->device == blacklist->device)
+	for (bldev = blacklist;
+	     bldev < blacklist + ARRAY_SIZE(blacklist);
+	     bldev++) {
+		if (dev->vendor == bldev->vendor &&
+		    dev->device == bldev->device)
 			return -ENODEV;
 	}

Alan Cox

2012-08-29 13:23 UTC #3 lore raw

Quoted 6 lines

I see, I missed the quirks in 8250_pci.c. I added a quirk for my card to set the options above and it works nicely thanks!
The new patch is attached.
Gianluca

This looks good to me - the only thing it needs to be applied is a signed-off-by line as per Documentation/SubmittingPatches

Alan

gianluca

author

2012-08-29 18:24 UTC #4 lore raw

PATCH 1/1 Add PCI IDs and quirks for WCH CH353 2S1P card

Add new PCI ids for WCH CH353 2Serial 1Parallel Multi-IO PCI Card to parport_serial.c.

To allow parport_serial to handle the card the same PCI ids are blacklisted in 8250_pci.c using the existing softmodem blacklist mechanism.

The blacklist array is also renamed because it now covers this new use case.

Since the two serial ports are autodetected as XScale instead of 16550A clones, add also a quirk to 8250_pci.c to skip autodetection and set the correct port type.

The patch is against linux v3.5.3 but applies cleanly also to latest git head.

Signed-off-by Gianluca Anzolin <[email protected]>

drivers/parport/parport_serial.c +10 -1

index e9c3227..1631eea 100644
@@ -62,6 +62,7 @@ enum parport_pc_pci_cards {
 	timedia_9079a,
 	timedia_9079b,
 	timedia_9079c,
+	wch_ch353_2s1p,
 };
 
 /* each element directly indexed from enum list, above */

@@ -145,6 +146,7 @@ static struct parport_pc_pci cards[] __devinitdata = {
 	/* timedia_9079a */             { 1, { { 2, 3 }, } },
 	/* timedia_9079b */             { 1, { { 2, 3 }, } },
 	/* timedia_9079c */             { 1, { { 2, 3 }, } },
+	/* wch_ch353_2s1p*/             { 1, { { 2, -1}, } },
 };
 
 static struct pci_device_id parport_serial_pci_tbl[] = {

@@ -243,7 +245,8 @@ static struct pci_device_id parport_serial_pci_tbl[] = {
 	{ 0x1409, 0x7168, 0x1409, 0xb079, 0, 0, timedia_9079a },
 	{ 0x1409, 0x7168, 0x1409, 0xc079, 0, 0, timedia_9079b },
 	{ 0x1409, 0x7168, 0x1409, 0xd079, 0, 0, timedia_9079c },
-
+	/* WCH CARDS */
+	{ 0x4348, 0x7053, 0x4348, 0x3253, 0, 0, wch_ch353_2s1p},
 	{ 0, } /* terminate list */
 };
 MODULE_DEVICE_TABLE(pci,parport_serial_pci_tbl);

@@ -460,6 +463,12 @@ static struct pciserial_board pci_parport_serial_boards[] __devinitdata = {
 		.base_baud	= 921600,
 		.uart_offset	= 8,
 	},
+	[wch_ch353_2s1p] = {
+		.flags          = FL_BASE0|FL_BASE_BARS,
+		.num_ports      = 2,
+		.base_baud      = 115200,
+		.uart_offset    = 8,
+	},
 };
 
 struct parport_serial_private {

drivers/tty/serial/8250/8250_pci.c +30 -8

index 28e7c7c..226b4db 100644
@@ -1157,6 +1157,16 @@ pci_xr17c154_setup(struct serial_private *priv,
 	return pci_default_setup(priv, board, port, idx);
 }
 
+static int
+pci_wch_ch353_setup(struct serial_private *priv,
+                    const struct pciserial_board *board,
+                    struct uart_port *port, int idx)
+{
+	port->flags |= UPF_FIXED_TYPE;
+	port->type = PORT_16550A;
+	return pci_default_setup(priv, board, port, idx);
+}
+
 #define PCI_VENDOR_ID_SBSMODULARIO	0x124B
 #define PCI_SUBVENDOR_ID_SBSMODULARIO	0x124B
 #define PCI_DEVICE_ID_OCTPRO		0x0001

@@ -1727,6 +1737,14 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = {
 		.subdevice	= PCI_ANY_ID,
 		.setup		= pci_omegapci_setup,
 	 },
+	/* WCH CH353 2S1P card (16550 clone) */
+	{
+		.vendor         = 0x4348,
+		.device         = 0x7053,
+		.subvendor      = 0x4348,
+		.subdevice      = 0x3253,
+		.setup          = pci_wch_ch353_setup,
+	},
 	/*
 	 * Default "match everything" terminator entry
 	 */

@@ -2624,10 +2642,14 @@ static struct pciserial_board pci_boards[] __devinitdata = {
 	},
 };
 
-static const struct pci_device_id softmodem_blacklist[] = {
+static const struct pci_device_id blacklist[] = {
+	/* softmodems */
 	{ PCI_VDEVICE(AL, 0x5457), }, /* ALi Corporation M5457 AC'97 Modem */
 	{ PCI_VDEVICE(MOTOROLA, 0x3052), }, /* Motorola Si3052-based modem */
 	{ PCI_DEVICE(0x1543, 0x3052), }, /* Si3052-based modem, default IDs */
+
+	/* multi-io cards handled by parport_serial */
+	{ PCI_DEVICE(0x4348, 0x7053), }, /* WCH CH353 2S1P */
 };
 
 /*

@@ -2638,7 +2660,7 @@ static const struct pci_device_id softmodem_blacklist[] = {
 static int __devinit
 serial_pci_guess_board(struct pci_dev *dev, struct pciserial_board *board)
 {
-	const struct pci_device_id *blacklist;
+	const struct pci_device_id *bldev;
 	int num_iomem, num_port, first_port = -1, i;
 
 	/*

@@ -2655,13 +2677,13 @@ serial_pci_guess_board(struct pci_dev *dev, struct pciserial_board *board)
 
 	/*
 	 * Do not access blacklisted devices that are known not to
-	 * feature serial ports.
+	 * feature serial ports or are handled by other modules.
 	 */
-	for (blacklist = softmodem_blacklist;
-	     blacklist < softmodem_blacklist + ARRAY_SIZE(softmodem_blacklist);
-	     blacklist++) {
-		if (dev->vendor == blacklist->vendor &&
-		    dev->device == blacklist->device)
+	for (bldev = blacklist;
+	     bldev < blacklist + ARRAY_SIZE(blacklist);
+	     bldev++) {
+		if (dev->vendor == bldev->vendor &&
+		    dev->device == bldev->device)
 			return -ENODEV;
 	}

Greg KH

maintainer

2012-09-05 23:40 UTC #5 lore raw

Re: [PATCH] Add PCI IDs and quirks for WCH CH353 2S1P card

Quoted 6 lines · gianluca

Add new PCI ids for WCH CH353 2Serial 1Parallel Multi-IO PCI Card to parport_serial.c.
To allow parport_serial to handle the card the same PCI ids are blacklisted in 8250_pci.c using the existing softmodem blacklist mechanism.

Quoted 7 lines

The blacklist array is also renamed because it now covers this new use case.
Since the two serial ports are autodetected as XScale instead of 16550A clones, add also a quirk to 8250_pci.c to skip autodetection and set the correct port type.
The patch is against linux v3.5.3 but applies cleanly also to latest git head.

This patch doesn't apply at all to the tty-next branch of the linux-next tree, which is where I need to apply it. Can you redo it against that branch and resend it so that I can apply it properly?

thanks,

greg k-h

gianluca

author

2012-09-06 06:41 UTC #6 lore raw

Re: [PATCH] Add PCI IDs and quirks for WCH CH353 2S1P card

Quoted 7 lines · Greg KH

This patch doesn't apply at all to the tty-next branch of the linux-next tree, which is where I need to apply it. Can you redo it against that branch and resend it so that I can apply it properly?
thanks,
greg k-h

no need for that, as it has been already submitted by Alan Cox and accepted in your tty-next tree as commit 6971c635af27b1d18d409e337e70bae25d2fa8ec
parport_serial: Add support for the WCH353 2S/1P multi-IO card

I see my name in the Author field was mispelled, but the patch is ok :)

Thank you

Gianluca