serial: rp2: New driver for Comtrol RocketPort 2 cards

PATCH Superseded Kevin Cernekee 3 participants 6 messages Dec 30, 2012 – Jan 16, 2013

Subsystems (2)

TTY layer and serial drivers Rocketport express/infinity driver

lore

Share & tools

lore

Kevin Cernekee

author maintainer

2012-12-30 07:37 UTC #0 lore raw

PATCH v2 1/3 serial: rp2: New driver for Comtrol RocketPort 2 cards

Signed-off-by Kevin Cernekee <[email protected]>

---
 drivers/tty/serial/Kconfig       |   24 ++
 drivers/tty/serial/Makefile      |    1 +
 drivers/tty/serial/rp2.c         |  882 ++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/serial_core.h |    3 +
 4 files changed, 910 insertions(+)
 create mode 100644 drivers/tty/serial/rp2.c

V1->V2 changes:

- Fix tty locking in rp2_rx_chars()
- Add readl() before udelay(), to make sure posted writes have cleared
- Use udev firmware loading for the microcode blob
- Remove deprecated __devinit and __devexit macros
- Change device name to "ttyRP" to avoid conflicts with rocket.c
- Add MODULE_DEVICE_TABLE

I set up a system with both old (rocket.c) and new (rp2.c) cards, and verified that the appropriate driver(s) are loaded based on the PCI device tables.

drivers/tty/serial/Kconfig +24 -0

index 59c23d0..555f3ca 100644
@@ -1447,4 +1447,28 @@ config SERIAL_ARC_NR_PORTS
 	  Set this to the number of serial ports you want the driver
 	  to support.
 
+config SERIAL_RP2
+	tristate "Comtrol RocketPort EXPRESS/INFINITY support"
+	depends on PCI
+	select SERIAL_CORE
+	help
+	  This driver supports the Comtrol RocketPort EXPRESS and
+	  RocketPort INFINITY families of PCI/PCIe multiport serial adapters.
+	  These adapters use a "RocketPort 2" ASIC that is not compatible
+	  with the original RocketPort driver (CONFIG_ROCKETPORT).
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rp2.
+
+	  If you want to compile this driver into the kernel, say Y here.  If
+	  you don't have a suitable RocketPort card installed, say N.
+
+config SERIAL_RP2_NR_UARTS
+	int "Maximum number of RocketPort EXPRESS/INFINITY ports"
+	depends on SERIAL_RP2
+	default "32"
+	help
+	  If multiple cards are present, the default limit of 32 ports may
+	  need to be increased.
+
 endmenu

drivers/tty/serial/Makefile +1 -0

index df1b998..a582ee9 100644
@@ -83,3 +83,4 @@ obj-$(CONFIG_SERIAL_SIRFSOC) += sirfsoc_uart.o
 obj-$(CONFIG_SERIAL_AR933X)   += ar933x_uart.o
 obj-$(CONFIG_SERIAL_EFM32_UART) += efm32-uart.o
 obj-$(CONFIG_SERIAL_ARC)	+= arc_uart.o
+obj-$(CONFIG_SERIAL_RP2)	+= rp2.o

drivers/tty/serial/rp2.c new file +882 -0

882 lines not on this page · raw message

include/uapi/linux/serial_core.h +3 -0

index 78f99d9..9dd47a5 100644
@@ -219,4 +219,7 @@
 /* ARC (Synopsys) on-chip UART */
 #define PORT_ARC       101
 
+/* Rocketport EXPRESS/INFINITY */
+#define PORT_RP2	102
+
 #endif /* _UAPILINUX_SERIAL_CORE_H */

Signature · 1 line

1.7.10.4

Kevin Cernekee

author maintainer

2012-12-30 07:37 UTC #1 lore raw

PATCH 2/3 tty: rocket: Explicitly list supported PCI IDs

Matching PCI_ANY_ID causes conflicts with RocketPort 2 adapters, which are supported by a different driver.

Signed-off-by Kevin Cernekee <[email protected]>

---
 drivers/tty/rocket.c |   31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

drivers/tty/rocket.c +28 -3

index e42009a..c73ca4e 100644
@@ -1758,8 +1758,32 @@ static void rp_flush_buffer(struct tty_struct *tty)
 
 #ifdef CONFIG_PCI
 
-static struct pci_device_id __used rocket_pci_ids[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_RP, PCI_ANY_ID) },
+#define RP_PCI_DEVICE(id) \
+	{ PCI_DEVICE(PCI_VENDOR_ID_RP, PCI_DEVICE_ID_##id) }
+
+static DEFINE_PCI_DEVICE_TABLE(rocket_pci_ids) = {
+	RP_PCI_DEVICE(RP4QUAD),
+	RP_PCI_DEVICE(RP8OCTA),
+	RP_PCI_DEVICE(URP8OCTA),
+	RP_PCI_DEVICE(RP8INTF),
+	RP_PCI_DEVICE(URP8INTF),
+	RP_PCI_DEVICE(RP8J),
+	RP_PCI_DEVICE(RP4J),
+	RP_PCI_DEVICE(RP8SNI),
+	RP_PCI_DEVICE(RP16SNI),
+	RP_PCI_DEVICE(RP16INTF),
+	RP_PCI_DEVICE(URP16INTF),
+	RP_PCI_DEVICE(CRP16INTF),
+	RP_PCI_DEVICE(RP32INTF),
+	RP_PCI_DEVICE(URP32INTF),
+	RP_PCI_DEVICE(RPP4),
+	RP_PCI_DEVICE(RPP8),
+	RP_PCI_DEVICE(RP2_232),
+	RP_PCI_DEVICE(RP2_422),
+	RP_PCI_DEVICE(RP6M),
+	RP_PCI_DEVICE(RP4M),
+	RP_PCI_DEVICE(UPCI_RM3_8PORT),
+	RP_PCI_DEVICE(UPCI_RM3_4PORT),
 	{ }
 };
 MODULE_DEVICE_TABLE(pci, rocket_pci_ids);

@@ -1781,7 +1805,8 @@ static __init int register_PCI(int i, struct pci_dev *dev)
 	WordIO_t ConfigIO = 0;
 	ByteIO_t UPCIRingInd = 0;
 
-	if (!dev || pci_enable_device(dev))
+	if (!dev || !pci_match_id(rocket_pci_ids, dev) ||
+	    pci_enable_device(dev))
 		return 0;
 
 	rcktpt_io_addr[i] = pci_resource_start(dev, 0);

Signature · 1 line

1.7.10.4

Greg KH

maintainer

2013-01-16 05:25 UTC #2 lore raw

Re: [PATCH 2/3] tty: rocket: Explicitly list supported PCI IDs

Quoted 13 lines · Kevin Cernekee

Matching PCI_ANY_ID causes conflicts with RocketPort 2 adapters, which are supported by a different driver.
Signed-off-by Kevin Cernekee <[email protected]>
---
 drivers/tty/rocket.c |   31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)
drivers/tty/rocket.c +0 -0
index e42009a..c73ca4e 100644

drivers/tty/rocket.c static void rp_flush_buffer(struct tty_struct *tty) · line 1765

Quoted 10 lines

drivers/tty/rocket.c +5 -2

@@ -1758,8 +1758,32 @@ static void rp_flush_buffer(struct tty_struct *tty)
 
 #ifdef CONFIG_PCI
 
-static struct pci_device_id __used rocket_pci_ids[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_RP, PCI_ANY_ID) },
+#define RP_PCI_DEVICE(id) \
+	{ PCI_DEVICE(PCI_VENDOR_ID_RP, PCI_DEVICE_ID_##id) }
+
+static DEFINE_PCI_DEVICE_TABLE(rocket_pci_ids) = {
+	RP_PCI_DEVICE(RP4QUAD),

Ick, no, please just use the PCI_DEVICE() macro and don't define a new one that makes it hard to grep for things. It's not that hard to just list them all properly.

thanks,

greg k-h

Kevin Cernekee

author maintainer

2012-12-30 07:37 UTC #3 lore raw

PATCH 3/3 rp2: Initial commit of Comtrol RocketPort 2 microcode

In reply to Kevin Cernekee

Signed-off-by Kevin Cernekee <[email protected]>

---
 WHENCE |   13 +++++++++++++
 rp2.fw |  Bin 0 -> 63 bytes
 2 files changed, 13 insertions(+)
 create mode 100644 rp2.fw

This is intended for the linux-firmware repo.

Also available from: git://github.com/cernekee/linux-firmware rp2

WHENCE +13 -0

index b7dcda3..3923185 100644
@@ -2034,3 +2034,16 @@ useful, but WITHOUT ANY WARRANTY; without even the implied warranty
 of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 
 --------------------------------------------------------------------------
+
+Driver: rp2 -- Comtrol RocketPort 2 serial driver
+
+File: rp2.fw
+
+Licence: Allegedly GPLv2+, but no source visible. Marked:
+
+  // RocketPort Infinity/Express device driver for Linux
+  // Copyright (C) 2004-2011 Comtrol, Inc.
+
+Found in hex form in the manufacturer's GPL out-of-tree driver sources.
+
+--------------------------------------------------------------------------

rp2.fw new file binary +0 -0

new file mode 100644
index 0000000000000000000000000000000000000000..a571e77925d479cbd4d17e72f49ee9b90bc8e2d7
GIT binary patch

Signature · 1 line

1.7.10.4

Ben Hutchings

2012-12-30 12:30 UTC #4 lore raw

Re: [PATCH -firmware 3/3] rp2: Initial commit of Comtrol RocketPort 2 microcode

Quoted 18 lines · Kevin Cernekee

Signed-off-by Kevin Cernekee <[email protected]>
---
 WHENCE |   13 +++++++++++++
 rp2.fw |  Bin 0 -> 63 bytes
 2 files changed, 13 insertions(+)
 create mode 100644 rp2.fw
This is intended for the linux-firmware repo.
Also available from: git://github.com/cernekee/linux-firmware rp2
WHENCE +0 -0
index b7dcda3..3923185 100644

WHENCE useful, but WITHOUT ANY WARRANTY; without even the implied warranty · line 2042

Quoted 9 lines

WHENCE +6 -0

@@ -2034,3 +2034,16 @@ useful, but WITHOUT ANY WARRANTY; without even the implied warranty
 of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 
 --------------------------------------------------------------------------
+
+Driver: rp2 -- Comtrol RocketPort 2 serial driver
+
+File: rp2.fw
+
+Licence: Allegedly GPLv2+, but no source visible. Marked:

[...]

The 'GPL' blobs already in the Linux source tree were grandfathered in but I don't think we should add any more like this. Please ask the vendor to clarify the licence or provide source for this.

Ben.

Signature · 2 lines

Ben Hutchings
It is easier to change the specification to fit the program than vice versa.

Ben Hutchings

2012-12-30 12:00 UTC #5 lore raw

In reply to Kevin Cernekee

On Sat, 2012-12-29 at 23:23 -0800, Kevin Cernekee wrote:
[...]

Quoted 2 lines

drivers/tty/serial/rp2.c new file +0 -0

[...]

Quoted 1 line

+static struct uart_ops rp2_uart_ops = {

const?

[...]

Quoted 11 lines

drivers/tty/serial/rp2.c new file +11 -0

+static void rp2_fw_cb(const struct firmware *fw, void *context)
+{
+	struct rp2_card *card = context;
+	resource_size_t phys_base;
+	int i, rc = -ENOENT;
+
+	if (!fw) {
+		dev_err(&card->pdev->dev, "cannot find '%s' firmware image\n",
+			RP2_FW_NAME);
+		goto no_fw;
+	}

[...]

drivers/tty/serial/rp2.c line 731

Quoted 10 lines

drivers/tty/serial/rp2.c new file +10 -0

+no_fw:
+	complete(&card->fw_loaded);
+
+	/*
+	 * rp2_fw_cb() is called from a workqueue long after rp2_probe()
+	 * has already returned success.  So if something failed here, we
+	 * need to manually yank the device.  This calls rp2_remove().
+	 */
+	if (rc)
+		device_release_driver(&card->pdev->dev);

[...]

The driver can be unbound and then re-bound immediately after this function calls complete(). This will then unbind it a second time!

Is it really necessary to unbind here?

Ben.

Signature · 2 lines

Ben Hutchings
It is easier to change the specification to fit the program than vice versa.