Re: [PATCH] parport/serial: add support for Timedia/SUNIX cards to parport_serial

Re: [PATCH] parport/serial: add support for Timedia/SUNIX cards to parport_serial

[Alan Cox]

On Fri, 25 Jun 2010 20:23:43 -0400
Frédéric Brière <fbriere@fbriere.net> wrote:

> Timedia/SUNIX PCI cards with both serial and parallel ports are
> currently supported by 8250_pci and parport_pc individually.  Moving
> that support into parport_serial allows using both types of ports at the
> same time.

This seems to have fallen on silence so I'm getting around to looking at
it.  I'm not sure the sunix hack to avoid lots of entries is a good idea
- it'll eventually break horribly somewhere.


> + * Devices with a parallel port can be identified by their subdevice ID
> + * matching xx[7-9]x.  We thus mark them as class OTHER, so that they will be
> + * ignored by 8250_pci, and claimed by parport_serial instead.
> + */
> +static void __devinit quirk_timedia(struct pci_dev *dev)
> +{
> +	/* 0,2,3,5,6: serial only -- 7,8,9: serial + parallel */
> +	if ((dev->subsystem_device & 0x00f0) >= 0x70) {
> +		dev_info(&dev->dev, "Timedia %04x; "
> +			"changing class SERIAL to OTHER (use parport_serial)\n",
> +			dev->subsystem_device);
> +		dev->class = (PCI_CLASS_COMMUNICATION_OTHER << 8) |
> +		    (dev->class & 0xff);
> +	}
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TIMEDIA, PCI_DEVICE_ID_TIMEDIA_1889,
> +		quirk_timedia);

And this check probably should be done in the serial driver so that it
doesn't bind, rather than changing the class, which then lies to other
bits of the kernel so can have funny side effects.

If you did that check in the serial driver and returned -ENODEV to the
probe the id will be handed on to the next driver that matches (ie
parport_serial)

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] parport/serial: add support for Timedia/SUNIX cards to parport_serial

[Frédéric Brière]

On Thu, Jul 08, 2010 at 12:11:22PM +0100, Alan Cox wrote:
> If you did that check in the serial driver and returned -ENODEV to the
> probe the id will be handed on to the next driver that matches (ie
> parport_serial)

Thanks for the suggestion; it does look much cleaner that way.

Instead of sticking a big-ass if() in the function, I took the liberty
of adding a .probe alongside the .init/.setup quirks, in case this could
be useful for other devices.

Note that I did not bother to cache the result of find_quirk(), so it
will be called twice.  I did not find it worth to add extra complexity
just for that; let me know if you disagree.


-- 
On the Internet, no one knows you're using Windows NT
		-- Submitted by Ramiro Estrugo, restrugo@fateware.com

[PATCH 1/2] serial: 8250_pci: add .probe member to struct pci_serial_quirk

[Frédéric Brière]

This function, if present, is called early on by the 8250_pci probe; it
can be used to reject devices meant for parport_serial.  (The .init
function cannot be used for this purpose, as it is also called by
parport_serial.)

Signed-off-by: Frédéric Brière <fbriere@fbriere.net>
---
 drivers/serial/8250_pci.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/serial/8250_pci.c b/drivers/serial/8250_pci.c
index 01c012d..d9b9a63 100644
--- a/drivers/serial/8250_pci.c
+++ b/drivers/serial/8250_pci.c
@@ -41,6 +41,7 @@ struct pci_serial_quirk {
 	u32	device;
 	u32	subvendor;
 	u32	subdevice;
+	int	(*probe)(struct pci_dev *dev);
 	int	(*init)(struct pci_dev *dev);
 	int	(*setup)(struct serial_private *,
 			 const struct pciserial_board *,
@@ -2470,11 +2471,19 @@ EXPORT_SYMBOL_GPL(pciserial_resume_ports);
 static int __devinit
 pciserial_init_one(struct pci_dev *dev, const struct pci_device_id *ent)
 {
+	struct pci_serial_quirk *quirk;
 	struct serial_private *priv;
 	const struct pciserial_board *board;
 	struct pciserial_board tmp;
 	int rc;
 
+	quirk = find_quirk(dev);
+	if (quirk->probe) {
+		rc = quirk->probe(dev);
+		if (rc)
+			return rc;
+	}
+
 	if (ent->driver_data >= ARRAY_SIZE(pci_boards)) {
 		printk(KERN_ERR "pci_init_one: invalid driver_data: %ld\n",
 			ent->driver_data);
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] parport/serial: add support for Timedia/SUNIX cards to parport_serial

[Frédéric Brière]

Timedia/SUNIX PCI cards with both serial and parallel ports are
currently supported by 8250_pci and parport_pc individually.  Moving
that support into parport_serial allows using both types of ports at the
same time.

This was successfully tested with a SUNIX 4079T.

Signed-off-by: Frédéric Brière <fbriere@fbriere.net>
---
 drivers/parport/parport_pc.c     |   54 -------------
 drivers/parport/parport_serial.c |  163 ++++++++++++++++++++++++++++++++++++++
 drivers/serial/8250_pci.c        |   23 ++++++
 3 files changed, 186 insertions(+), 54 deletions(-)

diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
index 0950fa4..e5d1d2f 100644
--- a/drivers/parport/parport_pc.c
+++ b/drivers/parport/parport_pc.c
@@ -2866,24 +2866,6 @@ enum parport_pc_pci_cards {
 	lava_parallel_dual_b,
 	boca_ioppar,
 	plx_9050,
-	timedia_4078a,
-	timedia_4079h,
-	timedia_4085h,
-	timedia_4088a,
-	timedia_4089a,
-	timedia_4095a,
-	timedia_4096a,
-	timedia_4078u,
-	timedia_4079a,
-	timedia_4085u,
-	timedia_4079r,
-	timedia_4079s,
-	timedia_4079d,
-	timedia_4079e,
-	timedia_4079f,
-	timedia_9079a,
-	timedia_9079b,
-	timedia_9079c,
 	timedia_4006a,
 	timedia_4014,
 	timedia_4008a,
@@ -2942,24 +2924,6 @@ static struct parport_pc_pci {
 	/* lava_parallel_dual_b */	{ 1, { { 0, -1 }, } },
 	/* boca_ioppar */		{ 1, { { 0, -1 }, } },
 	/* plx_9050 */			{ 2, { { 4, -1 }, { 5, -1 }, } },
-	/* timedia_4078a */		{ 1, { { 2, -1 }, } },
-	/* timedia_4079h */             { 1, { { 2, 3 }, } },
-	/* timedia_4085h */             { 2, { { 2, -1 }, { 4, -1 }, } },
-	/* timedia_4088a */             { 2, { { 2, 3 }, { 4, 5 }, } },
-	/* timedia_4089a */             { 2, { { 2, 3 }, { 4, 5 }, } },
-	/* timedia_4095a */             { 2, { { 2, 3 }, { 4, 5 }, } },
-	/* timedia_4096a */             { 2, { { 2, 3 }, { 4, 5 }, } },
-	/* timedia_4078u */             { 1, { { 2, -1 }, } },
-	/* timedia_4079a */             { 1, { { 2, 3 }, } },
-	/* timedia_4085u */             { 2, { { 2, -1 }, { 4, -1 }, } },
-	/* timedia_4079r */             { 1, { { 2, 3 }, } },
-	/* timedia_4079s */             { 1, { { 2, 3 }, } },
-	/* timedia_4079d */             { 1, { { 2, 3 }, } },
-	/* timedia_4079e */             { 1, { { 2, 3 }, } },
-	/* timedia_4079f */             { 1, { { 2, 3 }, } },
-	/* timedia_9079a */             { 1, { { 2, 3 }, } },
-	/* timedia_9079b */             { 1, { { 2, 3 }, } },
-	/* timedia_9079c */             { 1, { { 2, 3 }, } },
 	/* timedia_4006a */             { 1, { { 0, -1 }, } },
 	/* timedia_4014  */             { 2, { { 0, -1 }, { 2, -1 }, } },
 	/* timedia_4008a */             { 1, { { 0, 1 }, } },
@@ -3021,24 +2985,6 @@ static const struct pci_device_id parport_pc_pci_tbl[] = {
 	{ PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050,
 	  PCI_SUBVENDOR_ID_EXSYS, PCI_SUBDEVICE_ID_EXSYS_4014, 0, 0, plx_9050 },
 	/* PCI_VENDOR_ID_TIMEDIA/SUNIX has many differing cards ...*/
-	{ 0x1409, 0x7168, 0x1409, 0x4078, 0, 0, timedia_4078a },
-	{ 0x1409, 0x7168, 0x1409, 0x4079, 0, 0, timedia_4079h },
-	{ 0x1409, 0x7168, 0x1409, 0x4085, 0, 0, timedia_4085h },
-	{ 0x1409, 0x7168, 0x1409, 0x4088, 0, 0, timedia_4088a },
-	{ 0x1409, 0x7168, 0x1409, 0x4089, 0, 0, timedia_4089a },
-	{ 0x1409, 0x7168, 0x1409, 0x4095, 0, 0, timedia_4095a },
-	{ 0x1409, 0x7168, 0x1409, 0x4096, 0, 0, timedia_4096a },
-	{ 0x1409, 0x7168, 0x1409, 0x5078, 0, 0, timedia_4078u },
-	{ 0x1409, 0x7168, 0x1409, 0x5079, 0, 0, timedia_4079a },
-	{ 0x1409, 0x7168, 0x1409, 0x5085, 0, 0, timedia_4085u },
-	{ 0x1409, 0x7168, 0x1409, 0x6079, 0, 0, timedia_4079r },
-	{ 0x1409, 0x7168, 0x1409, 0x7079, 0, 0, timedia_4079s },
-	{ 0x1409, 0x7168, 0x1409, 0x8079, 0, 0, timedia_4079d },
-	{ 0x1409, 0x7168, 0x1409, 0x9079, 0, 0, timedia_4079e },
-	{ 0x1409, 0x7168, 0x1409, 0xa079, 0, 0, timedia_4079f },
-	{ 0x1409, 0x7168, 0x1409, 0xb079, 0, 0, timedia_9079a },
-	{ 0x1409, 0x7168, 0x1409, 0xc079, 0, 0, timedia_9079b },
-	{ 0x1409, 0x7168, 0x1409, 0xd079, 0, 0, timedia_9079c },
 	{ 0x1409, 0x7268, 0x1409, 0x0101, 0, 0, timedia_4006a },
 	{ 0x1409, 0x7268, 0x1409, 0x0102, 0, 0, timedia_4014 },
 	{ 0x1409, 0x7268, 0x1409, 0x0103, 0, 0, timedia_4008a },
diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c
index 40e208d..a64b275 100644
--- a/drivers/parport/parport_serial.c
+++ b/drivers/parport/parport_serial.c
@@ -41,6 +41,24 @@ enum parport_pc_pci_cards {
 	siig_2p1s_20x,
 	siig_1s1p_20x,
 	siig_2s1p_20x,
+	timedia_4078a,
+	timedia_4079h,
+	timedia_4085h,
+	timedia_4088a,
+	timedia_4089a,
+	timedia_4095a,
+	timedia_4096a,
+	timedia_4078u,
+	timedia_4079a,
+	timedia_4085u,
+	timedia_4079r,
+	timedia_4079s,
+	timedia_4079d,
+	timedia_4079e,
+	timedia_4079f,
+	timedia_9079a,
+	timedia_9079b,
+	timedia_9079c,
 };
 
 /* each element directly indexed from enum list, above */
@@ -105,6 +123,24 @@ static struct parport_pc_pci cards[] __devinitdata = {
 	/* siig_2p1s_20x */		{ 2, { { 1, 2 }, { 3, 4 }, } },
 	/* siig_1s1p_20x */		{ 1, { { 1, 2 }, } },
 	/* siig_2s1p_20x */		{ 1, { { 2, 3 }, } },
+	/* timedia_4078a */		{ 1, { { 2, -1 }, } },
+	/* timedia_4079h */             { 1, { { 2, 3 }, } },
+	/* timedia_4085h */             { 2, { { 2, -1 }, { 4, -1 }, } },
+	/* timedia_4088a */             { 2, { { 2, 3 }, { 4, 5 }, } },
+	/* timedia_4089a */             { 2, { { 2, 3 }, { 4, 5 }, } },
+	/* timedia_4095a */             { 2, { { 2, 3 }, { 4, 5 }, } },
+	/* timedia_4096a */             { 2, { { 2, 3 }, { 4, 5 }, } },
+	/* timedia_4078u */             { 1, { { 2, -1 }, } },
+	/* timedia_4079a */             { 1, { { 2, 3 }, } },
+	/* timedia_4085u */             { 2, { { 2, -1 }, { 4, -1 }, } },
+	/* timedia_4079r */             { 1, { { 2, 3 }, } },
+	/* timedia_4079s */             { 1, { { 2, 3 }, } },
+	/* timedia_4079d */             { 1, { { 2, 3 }, } },
+	/* timedia_4079e */             { 1, { { 2, 3 }, } },
+	/* timedia_4079f */             { 1, { { 2, 3 }, } },
+	/* timedia_9079a */             { 1, { { 2, 3 }, } },
+	/* timedia_9079b */             { 1, { { 2, 3 }, } },
+	/* timedia_9079c */             { 1, { { 2, 3 }, } },
 };
 
 static struct pci_device_id parport_serial_pci_tbl[] = {
@@ -176,6 +212,25 @@ static struct pci_device_id parport_serial_pci_tbl[] = {
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, siig_2s1p_20x },
 	{ PCI_VENDOR_ID_SIIG, PCI_DEVICE_ID_SIIG_2S1P_20x_850,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, siig_2s1p_20x },
+	/* PCI_VENDOR_ID_TIMEDIA/SUNIX has many differing cards ...*/
+	{ 0x1409, 0x7168, 0x1409, 0x4078, 0, 0, timedia_4078a },
+	{ 0x1409, 0x7168, 0x1409, 0x4079, 0, 0, timedia_4079h },
+	{ 0x1409, 0x7168, 0x1409, 0x4085, 0, 0, timedia_4085h },
+	{ 0x1409, 0x7168, 0x1409, 0x4088, 0, 0, timedia_4088a },
+	{ 0x1409, 0x7168, 0x1409, 0x4089, 0, 0, timedia_4089a },
+	{ 0x1409, 0x7168, 0x1409, 0x4095, 0, 0, timedia_4095a },
+	{ 0x1409, 0x7168, 0x1409, 0x4096, 0, 0, timedia_4096a },
+	{ 0x1409, 0x7168, 0x1409, 0x5078, 0, 0, timedia_4078u },
+	{ 0x1409, 0x7168, 0x1409, 0x5079, 0, 0, timedia_4079a },
+	{ 0x1409, 0x7168, 0x1409, 0x5085, 0, 0, timedia_4085u },
+	{ 0x1409, 0x7168, 0x1409, 0x6079, 0, 0, timedia_4079r },
+	{ 0x1409, 0x7168, 0x1409, 0x7079, 0, 0, timedia_4079s },
+	{ 0x1409, 0x7168, 0x1409, 0x8079, 0, 0, timedia_4079d },
+	{ 0x1409, 0x7168, 0x1409, 0x9079, 0, 0, timedia_4079e },
+	{ 0x1409, 0x7168, 0x1409, 0xa079, 0, 0, timedia_4079f },
+	{ 0x1409, 0x7168, 0x1409, 0xb079, 0, 0, timedia_9079a },
+	{ 0x1409, 0x7168, 0x1409, 0xc079, 0, 0, timedia_9079b },
+	{ 0x1409, 0x7168, 0x1409, 0xd079, 0, 0, timedia_9079c },
 
 	{ 0, } /* terminate list */
 };
@@ -267,6 +322,114 @@ static struct pciserial_board pci_parport_serial_boards[] __devinitdata = {
 		.base_baud	= 921600,
 		.uart_offset	= 8,
 	},
+	[timedia_4078a] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4079h] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4085h] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4088a] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4089a] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4095a] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4096a] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4078u] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4079a] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4085u] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4079r] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4079s] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4079d] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4079e] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_4079f] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_9079a] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_9079b] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
+	[timedia_9079c] = {
+		.flags		= FL_BASE0|FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 921600,
+		.uart_offset	= 8,
+	},
 };
 
 struct parport_serial_private {
diff --git a/drivers/serial/8250_pci.c b/drivers/serial/8250_pci.c
index d9b9a63..bf50a1f 100644
--- a/drivers/serial/8250_pci.c
+++ b/drivers/serial/8250_pci.c
@@ -574,6 +574,28 @@ static const struct timedia_struct {
 	{ 8, timedia_eight_port }
 };
 
+/*
+ * There are nearly 70 different Timedia/SUNIX PCI serial devices.  Instead of
+ * listing them individually, this driver merely grabs them all with
+ * PCI_ANY_ID.  Some of these devices, however, also feature a parallel port,
+ * and should be left free to be claimed by parport_serial instead.
+ */
+static int pci_timedia_probe(struct pci_dev *dev)
+{
+	/*
+	 * Check the third digit of the subdevice ID
+	 * (0,2,3,5,6: serial only -- 7,8,9: serial + parallel)
+	 */
+	if ((dev->subsystem_device & 0x00f0) >= 0x70) {
+		dev_info(&dev->dev,
+			"ignoring Timedia subdevice %04x for parport_serial\n",
+			dev->subsystem_device);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int pci_timedia_init(struct pci_dev *dev)
 {
 	const unsigned short *ids;
@@ -1343,6 +1365,7 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = {
 		.device		= PCI_DEVICE_ID_TIMEDIA_1889,
 		.subvendor	= PCI_VENDOR_ID_TIMEDIA,
 		.subdevice	= PCI_ANY_ID,
+		.probe		= pci_timedia_probe,
 		.init		= pci_timedia_init,
 		.setup		= pci_timedia_setup,
 	},
-- 
1.7.1


_______________________________________________
Linux-parport mailing list
Linux-parport@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-parport

Re: [PATCH] parport/serial: add support for Timedia/SUNIX cards to parport_serial

[Alan Cox]

> Note that I did not bother to cache the result of find_quirk(), so it
> will be called twice.  I did not find it worth to add extra complexity
> just for that; let me know if you disagree.

Without seeing the code its hard to be sure - but it sounds reasonable,
serial port loading isn't usually a hot benchmarking path 8)