[PATCH 1/5] Char: isicom, fix locking in isr

[PATCH 1/5] Char: isicom, fix locking in isr

[Jiri Slaby]

isicom, fix locking in isr

2 spin_unlocks are omitted in the interrupt handler. Put them there to fix
up deadlocking on UP.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

---
commit f2d37e8d3de070f8cda48a454f7b991d29b310be
tree 23027dcdc3215fbb488577edb9610322956edb0b
parent 5df5a993999b94d728cedfa669eba2b0b58e16d7
author Jiri Slaby <jirislaby@gmail.com> Wed, 13 Dec 2006 23:10:48 +0100
committer Jiri Slaby <jirislaby@gmail.com> Fri, 15 Dec 2006 20:29:34 +0059

 drivers/char/isicom.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/char/isicom.c b/drivers/char/isicom.c
index 0362740..836e967 100644
--- a/drivers/char/isicom.c
+++ b/drivers/char/isicom.c
@@ -563,6 +563,7 @@ static irqreturn_t isicom_interrupt(int irq, void *dev_id)
 	port = card->ports + channel;
 	if (!(port->flags & ASYNC_INITIALIZED)) {
 		outw(0x0000, base+0x04); /* enable interrupts */
+		spin_unlock(&card->card_lock);
 		return IRQ_HANDLED;
 	}
 
@@ -677,6 +678,7 @@ static irqreturn_t isicom_interrupt(int irq, void *dev_id)
 		tty_flip_buffer_push(tty);
 	}
 	outw(0x0000, base+0x04); /* enable interrupts */
+	spin_unlock(&card->card_lock);
 
 	return IRQ_HANDLED;
 }

[PATCH 4/5] Char: isicom, check card state in isr

[Jiri Slaby]

isicom, check card state in isr

Check if the card really interrupted us by reading its IO space and
eventualy return IRQ_NONE.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

---
commit 601667e4ee38183358ea8f7980537bb8c09d8728
tree ccb1c085309ad35178f8d741e7c074308ae277ee
parent 405c17b09b010b41f6ec2388a11777e4048c7976
author Jiri Slaby <jirislaby@gmail.com> Fri, 15 Dec 2006 23:29:57 +0059
committer Jiri Slaby <jirislaby@gmail.com> Fri, 15 Dec 2006 23:29:57 +0059

 drivers/char/isicom.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/char/isicom.c b/drivers/char/isicom.c
index 7968160..f4faa76 100644
--- a/drivers/char/isicom.c
+++ b/drivers/char/isicom.c
@@ -539,6 +539,11 @@ static irqreturn_t isicom_interrupt(int irq, void *dev_id)
 		return IRQ_NONE;
 
 	base = card->base;
+
+	/* did the card interrupt us? */
+	if (!(inw(base + 0x0e) & 0x02))
+		return IRQ_NONE;
+
 	spin_lock(&card->card_lock);
 
 	/*

[PATCH 2/5] Char: isicom, fix probe race

[Jiri Slaby]

isicom, fix probe race

Fix two race conditions in the probe function with mutex.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

---
commit e7087b32ad4b5ee1240fa7f9ba46a9b4566fe424
tree 28bc5ad2a47c03e1b7a09fce22afbe7000955e97
parent f2d37e8d3de070f8cda48a454f7b991d29b310be
author Jiri Slaby <jirislaby@gmail.com> Fri, 15 Dec 2006 21:08:11 +0059
committer Jiri Slaby <jirislaby@gmail.com> Fri, 15 Dec 2006 21:08:11 +0059

 drivers/char/isicom.c |   26 ++++++++++++++++----------
 1 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/char/isicom.c b/drivers/char/isicom.c
index 836e967..5d2c345 100644
--- a/drivers/char/isicom.c
+++ b/drivers/char/isicom.c
@@ -1732,22 +1732,25 @@ end:
 /*
  *	Insmod can set static symbols so keep these static
  */
-static int card;
+static unsigned int card_count;
 
 static int __devinit isicom_probe(struct pci_dev *pdev,
 	const struct pci_device_id *ent)
 {
+	static DEFINE_MUTEX(probe_lock);
 	unsigned int ioaddr, signature, index;
 	int retval = -EPERM;
-	u8 pciirq;
 	struct isi_board *board = NULL;
 
-	if (card >= BOARD_COUNT)
+	mutex_lock(&probe_lock);
+	if (card_count >= BOARD_COUNT) {
+		mutex_unlock(&probe_lock);
 		goto err;
+	}
+	card_count++;
 
 	ioaddr = pci_resource_start(pdev, 3);
 	/* i.e at offset 0x1c in the PCI configuration register space. */
-	pciirq = pdev->irq;
 	dev_info(&pdev->dev, "ISI PCI Card(Device ID 0x%x)\n", ent->device);
 
 	/* allot the first empty slot in the array */
@@ -1759,8 +1762,9 @@ static int __devinit isicom_probe(struct pci_dev *pdev,
 
 	board->index = index;
 	board->base = ioaddr;
-	board->irq = pciirq;
-	card++;
+	board->irq = pdev->irq;
+
+	mutex_unlock(&probe_lock);
 
 	pci_set_drvdata(pdev, board);
 
@@ -1770,7 +1774,7 @@ static int __devinit isicom_probe(struct pci_dev *pdev,
 			"will be disabled.\n", board->base, board->base + 15,
 			index + 1);
 		retval = -EBUSY;
-		goto err;
+		goto err_dec;
  	}
 
 	retval = request_irq(board->irq, isicom_interrupt,
@@ -1799,8 +1803,10 @@ errunri:
 	free_irq(board->irq, board);
 errunrr:
 	pci_release_region(pdev, 3);
-err:
+err_dec:
+	card_count--;
 	board->base = 0;
+err:
 	return retval;
 }
 
@@ -1814,6 +1820,8 @@ static void __devexit isicom_remove(struct pci_dev *pdev)
 
 	free_irq(board->irq, board);
 	pci_release_region(pdev, 3);
+	card_count--;
+	board->base = 0;
 }
 
 static int __init isicom_init(void)
@@ -1821,8 +1829,6 @@ static int __init isicom_init(void)
 	int retval, idx, channel;
 	struct isi_port *port;
 
-	card = 0;
-
 	for(idx = 0; idx < BOARD_COUNT; idx++) {
 		port = &isi_ports[idx * 16];
 		isi_card[idx].ports = port;

[PATCH 3/5] Char: isicom, augment card_reset

[Jiri Slaby]

isicom, augment card_reset

- add 0xee to signatures
- change long delays to sleeps
- make one sleep shorter not to wait 3s
- portcount == 16 is also correct

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

---
commit 405c17b09b010b41f6ec2388a11777e4048c7976
tree 4954b33027ee87193bfe91109d6fbc8b12c4e71a
parent e7087b32ad4b5ee1240fa7f9ba46a9b4566fe424
author Jiri Slaby <jirislaby@gmail.com> Fri, 15 Dec 2006 22:20:14 +0059
committer Jiri Slaby <jirislaby@gmail.com> Fri, 15 Dec 2006 22:20:14 +0059

 drivers/char/isicom.c |   39 ++++++++++++++++++++++-----------------
 1 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/char/isicom.c b/drivers/char/isicom.c
index 5d2c345..7968160 100644
--- a/drivers/char/isicom.c
+++ b/drivers/char/isicom.c
@@ -1501,7 +1501,7 @@ static int __devinit reset_card(struct pci_dev *pdev,
 {
 	struct isi_board *board = pci_get_drvdata(pdev);
 	unsigned long base = board->base;
-	unsigned int portcount = 0;
+	unsigned int sig, portcount = 0;
 	int retval = 0;
 
 	dev_dbg(&pdev->dev, "ISILoad:Resetting Card%d at 0x%lx\n", card + 1,
@@ -1509,27 +1509,35 @@ static int __devinit reset_card(struct pci_dev *pdev,
 
 	inw(base + 0x8);
 
-	mdelay(10);
+	msleep(10);
 
 	outw(0, base + 0x8); /* Reset */
 
-	msleep(3000);
+	msleep(1000);
 
-	*signature = inw(base + 0x4) & 0xff;
+	sig = inw(base + 0x4) & 0xff;
+
+	if (sig != 0xa5 && sig != 0xbb && sig != 0xcc && sig != 0xdd &&
+			sig != 0xee) {
+		dev_warn(&pdev->dev, "ISILoad:Card%u reset failure (Possible "
+			"bad I/O Port Address 0x%lx).\n", card + 1, base);
+		dev_dbg(&pdev->dev, "Sig=0x%x\n", sig);
+		retval = -EIO;
+		goto end;
+	}
+
+	msleep(10);
 
 	portcount = inw(base + 0x2);
-	if (!(inw(base + 0xe) & 0x1) || ((portcount != 0) &&
-			(portcount != 4) && (portcount != 8))) {
-		dev_dbg(&pdev->dev, "base+0x2=0x%lx, base+0xe=0x%lx\n",
-			inw(base + 0x2), inw(base + 0xe));
-		dev_err(&pdev->dev, "ISILoad:PCI Card%d reset failure "
-			"(Possible bad I/O Port Address 0x%lx).\n",
-			card + 1, base);
+	if (!inw(base + 0xe) & 0x1 || (portcount != 0 && portcount != 4 &&
+				portcount != 8 && portcount != 16)) {
+		dev_err(&pdev->dev, "ISILoad:PCI Card%d reset failure.",
+			card + 1);
 		retval = -EIO;
 		goto end;
 	}
 
-	switch (*signature) {
+	switch (sig) {
 	case 0xa5:
 	case 0xbb:
 	case 0xdd:
@@ -1537,16 +1545,13 @@ static int __devinit reset_card(struct pci_dev *pdev,
 		board->shift_count = 12;
 		break;
 	case 0xcc:
+	case 0xee:
 		board->port_count = 16;
 		board->shift_count = 11;
 		break;
-	default:
-		dev_warn(&pdev->dev, "ISILoad:Card%d reset failure (Possible "
-			"bad I/O Port Address 0x%lx).\n", card + 1, base);
-		dev_dbg(&pdev->dev, "Sig=0x%lx\n", signature);
-		retval = -EIO;
 	}
 	dev_info(&pdev->dev, "-Done\n");
+	*signature = sig;
 
 end:
 	return retval;

[PATCH 5/5] Char: isicom, support higher rates

[Jiri Slaby]

isicom, support higher rates

Add support for higher baud rates (coming from original isi driver).

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

---
commit 8b380d8b1c3ff7d09d68d467d2f135774cab4086
tree d1e9332d7dc76c5f1d80f936bca71312d0bcb07b
parent 601667e4ee38183358ea8f7980537bb8c09d8728
author Jiri Slaby <jirislaby@gmail.com> Sat, 16 Dec 2006 02:05:20 +0059
committer Jiri Slaby <jirislaby@gmail.com> Sat, 16 Dec 2006 02:05:20 +0059

 drivers/char/isicom.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/char/isicom.c b/drivers/char/isicom.c
index f4faa76..60df87c 100644
--- a/drivers/char/isicom.c
+++ b/drivers/char/isicom.c
@@ -183,7 +183,7 @@ static DEFINE_TIMER(tx, isicom_tx, 0, 0);
 /*   baud index mappings from linux defns to isi */
 
 static signed char linuxb_to_isib[] = {
-	-1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 13, 15, 16, 17, 18, 19
+	-1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 13, 15, 16, 17, 18, 19, 20, 21
 };
 
 struct	isi_board {
@@ -709,7 +709,8 @@ static void isicom_config_port(struct isi_port *port)
 		 *  respectively.
 		 */
 
-		if (baud < 1 || baud > 2)
+		/* 1,2,3,4 => 57.6, 115.2, 230, 460 kbps resp. */
+		if (baud < 1 || baud > 4)
 			port->tty->termios->c_cflag &= ~CBAUDEX;
 		else
 			baud += 15;
@@ -725,6 +726,10 @@ static void isicom_config_port(struct isi_port *port)
 			baud++; /*  57.6 Kbps */
 		if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_VHI)
 			baud +=2; /*  115  Kbps */
+		if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_SHI)
+			baud += 3; /* 230 kbps*/
+		if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_WARP)
+			baud += 4; /* 460 kbps*/
 	}
 	if (linuxb_to_isib[baud] == -1) {
 		/* hang up */

Re: [PATCH 2/5] Char: isicom, fix probe race

[Andrew Morton]

On Sat, 16 Dec 2006 02:09:48 +0100 (CET)
Jiri Slaby <jirislaby@gmail.com> wrote:

> isicom, fix probe race
> 
> Fix two race conditions in the probe function with mutex.
> 
> ...
>
>  static int __devinit isicom_probe(struct pci_dev *pdev,
>  	const struct pci_device_id *ent)
>  {
> +	static DEFINE_MUTEX(probe_lock);

hm.  How can isicom_probe() race with itself?  Even with the dreaded
multithreaded-pci-probing?  It's only called once, by a single thread.

Confused.

Re: [PATCH 2/5] Char: isicom, fix probe race

[Jiri Slaby]

Andrew Morton wrote:
> On Sat, 16 Dec 2006 02:09:48 +0100 (CET)
> Jiri Slaby <jirislaby@gmail.com> wrote:
> 
>> isicom, fix probe race
>>
>> Fix two race conditions in the probe function with mutex.
>>
>> ...
>>
>>  static int __devinit isicom_probe(struct pci_dev *pdev,
>>  	const struct pci_device_id *ent)
>>  {
>> +	static DEFINE_MUTEX(probe_lock);
> 
> hm.  How can isicom_probe() race with itself?  Even with the dreaded
> multithreaded-pci-probing?  It's only called once, by a single thread.
> 
> Confused.

Yeah, I'm a little bit too now. One of developers want me to do this some time
ago and I did it without deep thinking about that. Now, I did it again and as
you wrote, it's completely unreasonable. Please, throw it
(char-isicom-fix-probe-race.patch) away.

thanks,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E