[PATCH 1/9] Staging: ipack/bridges/tpci200: remove gotos in tpci200_free_irq().

[PATCH 1/9] Staging: ipack/bridges/tpci200: remove gotos in tpci200_free_irq().

[Miguel Gómez]

Handle error conditions with simple returns instead of usig gotos.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |   22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index c39499a..31f6b5d 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -512,24 +512,19 @@ static void __tpci200_free_irq(struct tpci200_board *tpci200,
 
 static int tpci200_free_irq(struct ipack_device *dev)
 {
-	int res;
 	struct slot_irq *slot_irq;
 	struct tpci200_board *tpci200;
 
 	tpci200 = check_slot(dev);
-	if (tpci200 == NULL) {
-		res = -EINVAL;
-		goto out;
-	}
+	if (tpci200 == NULL)
+		return -EINVAL;
 
-	if (mutex_lock_interruptible(&tpci200->mutex)) {
-		res = -ERESTARTSYS;
-		goto out;
-	}
+	if (mutex_lock_interruptible(&tpci200->mutex))
+		return -ERESTARTSYS;
 
 	if (tpci200->slots[dev->slot].irq == NULL) {
-		res = -EINVAL;
-		goto out_unlock;
+		mutex_unlock(&tpci200->mutex);
+		return -EINVAL;
 	}
 
 	__tpci200_free_irq(tpci200, dev);
@@ -537,10 +532,7 @@ static int tpci200_free_irq(struct ipack_device *dev)
 	tpci200->slots[dev->slot].irq = NULL;
 	kfree(slot_irq);
 
-out_unlock:
-	mutex_unlock(&tpci200->mutex);
-out:
-	return res;
+	return 0;
 }
 
 static int tpci200_slot_unmap_space(struct ipack_device *dev, int space)
-- 
1.7.9.5

[PATCH 2/9] Staging: ipack/bridges/tpci200: remove "out" label in tpci200_slot_map_space()

[Miguel Gómez]

Remove the "out" label from tpci200_slot_map_space(), as it can directly return
the error code instead of jumping.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index 31f6b5d..3eaf387 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -625,15 +625,11 @@ static int tpci200_slot_map_space(struct ipack_device *dev,
 	struct tpci200_board *tpci200;
 
 	tpci200 = check_slot(dev);
-	if (tpci200 == NULL) {
-		res = -EINVAL;
-		goto out;
-	}
+	if (tpci200 == NULL)
+		return -EINVAL;
 
-	if (mutex_lock_interruptible(&tpci200->mutex)) {
-		res = -ERESTARTSYS;
-		goto out;
-	}
+	if (mutex_lock_interruptible(&tpci200->mutex))
+		return -ERESTARTSYS;
 
 	switch (space) {
 	case IPACK_IO_SPACE:
@@ -698,7 +694,6 @@ static int tpci200_slot_map_space(struct ipack_device *dev,
 
 out_unlock:
 	mutex_unlock(&tpci200->mutex);
-out:
 	return res;
 }
 
-- 
1.7.9.5

[PATCH 3/9] Staging: ipack/bridges/tpci200: remove useless break in tpci200_slot_map_space()

[Miguel Gómez]

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index 3eaf387..216c6df 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -685,7 +685,6 @@ static int tpci200_slot_map_space(struct ipack_device *dev,
 			tpci200->number, dev->slot, space);
 		res = -EINVAL;
 		goto out_unlock;
-		break;
 	}
 
 	virt_addr_space->size = size_to_map;
-- 
1.7.9.5

[PATCH 4/9] Staging: ipack/bridges/tpci200: remove "out" label in tpci200_request_irq()

[Miguel Gómez]

Remove the "out" label from tpci200_request_irq(), as it can directly return
the error code instead of jumping.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index 216c6df..29c91d6 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -704,15 +704,11 @@ static int tpci200_request_irq(struct ipack_device *dev, int vector,
 	struct tpci200_board *tpci200;
 
 	tpci200 = check_slot(dev);
-	if (tpci200 == NULL) {
-		res = -EINVAL;
-		goto out;
-	}
+	if (tpci200 == NULL)
+		return -EINVAL;
 
-	if (mutex_lock_interruptible(&tpci200->mutex)) {
-		res = -ERESTARTSYS;
-		goto out;
-	}
+	if (mutex_lock_interruptible(&tpci200->mutex))
+		return -ERESTARTSYS;
 
 	if (tpci200->slots[dev->slot].irq != NULL) {
 		dev_err(&dev->dev,
@@ -746,7 +742,6 @@ static int tpci200_request_irq(struct ipack_device *dev, int vector,
 
 out_unlock:
 	mutex_unlock(&tpci200->mutex);
-out:
 	return res;
 }
 
-- 
1.7.9.5

[PATCH 5/9] Staging: ipack/bridges/tpci200: remove gotos in tpci200_install()

[Miguel Gómez]

Remove the gotos when handling error conditions, as the code gets clearer
and the gotos are not really avoiding code replication.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |   19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index 29c91d6..51448c7 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -776,23 +776,18 @@ static int tpci200_install(struct tpci200_board *tpci200)
 
 	tpci200->slots = kzalloc(
 		TPCI200_NB_SLOT * sizeof(struct tpci200_slot), GFP_KERNEL);
-	if (tpci200->slots == NULL) {
-		res = -ENOMEM;
-		goto out_err;
-	}
+	if (tpci200->slots == NULL)
+		return -ENOMEM;
 
 	res = tpci200_register(tpci200);
-	if (res)
-		goto out_free;
+	if (res) {
+		kfree(tpci200->slots);
+		tpci200->slots = NULL;
+		return res;
+	}
 
 	mutex_init(&tpci200->mutex);
 	return 0;
-
-out_free:
-	kfree(tpci200->slots);
-	tpci200->slots = NULL;
-out_err:
-	return res;
 }
 
 static int tpci200_pciprobe(struct pci_dev *pdev,
-- 
1.7.9.5

[PATCH 6/9] Staging: ipack/devices/ipoctal: remove ipoctal_config structure.

[Miguel Gómez]

The configuration of the communication channel is handled by the tty
abstraction, so the ipoctal_config structure has become useless and it's
only used to store values that are never accesed. Remove it.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/ipack/devices/ipoctal.c |   11 -----------
 drivers/staging/ipack/devices/ipoctal.h |   17 -----------------
 2 files changed, 28 deletions(-)

diff --git a/drivers/staging/ipack/devices/ipoctal.c b/drivers/staging/ipack/devices/ipoctal.c
index 17aa35a..ff88800 100644
--- a/drivers/staging/ipack/devices/ipoctal.c
+++ b/drivers/staging/ipack/devices/ipoctal.c
@@ -49,7 +49,6 @@ struct ipoctal {
 	char				*buffer[NR_CHANNELS];
 	unsigned int			nb_bytes[NR_CHANNELS];
 	unsigned int			count_wr[NR_CHANNELS];
-	struct ipoctal_config		chan_config[NR_CHANNELS];
 	wait_queue_head_t		queue[NR_CHANNELS];
 	unsigned short			error_flag[NR_CHANNELS];
 	spinlock_t			lock[NR_CHANNELS];
@@ -671,22 +670,18 @@ static void ipoctal_set_termios(struct tty_struct *tty,
 		if (cflag & CRTSCTS) {
 			mr1 |= MR1_RxRTS_CONTROL_ON;
 			mr2 |= MR2_TxRTS_CONTROL_OFF | MR2_CTS_ENABLE_TX_ON;
-			ipoctal->chan_config[channel].flow_control = 1;
 		} else {
 			mr1 |= MR1_RxRTS_CONTROL_OFF;
 			mr2 |= MR2_TxRTS_CONTROL_OFF | MR2_CTS_ENABLE_TX_OFF;
-			ipoctal->chan_config[channel].flow_control = 0;
 		}
 		break;
 	case IP_OCTAL_422_ID:
 		mr1 |= MR1_RxRTS_CONTROL_OFF;
 		mr2 |= MR2_TxRTS_CONTROL_OFF | MR2_CTS_ENABLE_TX_OFF;
-		ipoctal->chan_config[channel].flow_control = 0;
 		break;
 	case IP_OCTAL_485_ID:
 		mr1 |= MR1_RxRTS_CONTROL_OFF;
 		mr2 |= MR2_TxRTS_CONTROL_ON | MR2_CTS_ENABLE_TX_OFF;
-		ipoctal->chan_config[channel].flow_control = 0;
 		break;
 	default:
 		return;
@@ -750,12 +745,6 @@ static void ipoctal_set_termios(struct tty_struct *tty,
 	ipoctal_write_io_reg(ipoctal, &ipoctal->chan_regs[channel].u.w.mr, mr2);
 	ipoctal_write_io_reg(ipoctal, &ipoctal->chan_regs[channel].u.w.csr, csr);
 
-	/* save the setup in the structure */
-	ipoctal->chan_config[channel].baud = tty_get_baud_rate(tty);
-	ipoctal->chan_config[channel].bits_per_char = cflag & CSIZE;
-	ipoctal->chan_config[channel].parity = cflag & PARENB;
-	ipoctal->chan_config[channel].stop_bits = cflag & CSTOPB;
-
 	/* Enable again the RX */
 	ipoctal_write_io_reg(ipoctal, &ipoctal->chan_regs[channel].u.w.cr,
 			     CR_ENABLE_RX);
diff --git a/drivers/staging/ipack/devices/ipoctal.h b/drivers/staging/ipack/devices/ipoctal.h
index 266f361..b3c901c 100644
--- a/drivers/staging/ipack/devices/ipoctal.h
+++ b/drivers/staging/ipack/devices/ipoctal.h
@@ -42,23 +42,6 @@ enum uart_error	{
 };
 
 /**
- * struct ipoctal_config - Serial configuration
- *
- * @baud: Baud rate
- * @stop_bits: Stop bits (1 or 2)
- * @bits_per_char: data size in bits
- * @parity
- * @flow_control: Flow control management (RTS/CTS) (0 disabled, 1 enabled)
- */
-struct ipoctal_config {
-	unsigned int baud;
-	unsigned int stop_bits;
-	unsigned int bits_per_char;
-	unsigned short parity;
-	unsigned int flow_control;
-};
-
-/**
  * struct ipoctal_stats -- Stats since last reset
  *
  * @tx: Number of transmitted bytes
-- 
1.7.9.5

[PATCH 7/9] Staging: ipack/devices/ipoctal: remove error_flag field from ipoctal struct.

[Miguel Gómez]

Remove the error_flag field from the ipoctal structure, as the error code
is handled through the tty abstraction. Remove the values definition as well.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/ipack/devices/ipoctal.c |    8 --------
 drivers/staging/ipack/devices/ipoctal.h |   13 -------------
 2 files changed, 21 deletions(-)

diff --git a/drivers/staging/ipack/devices/ipoctal.c b/drivers/staging/ipack/devices/ipoctal.c
index ff88800..2921467 100644
--- a/drivers/staging/ipack/devices/ipoctal.c
+++ b/drivers/staging/ipack/devices/ipoctal.c
@@ -50,7 +50,6 @@ struct ipoctal {
 	unsigned int			nb_bytes[NR_CHANNELS];
 	unsigned int			count_wr[NR_CHANNELS];
 	wait_queue_head_t		queue[NR_CHANNELS];
-	unsigned short			error_flag[NR_CHANNELS];
 	spinlock_t			lock[NR_CHANNELS];
 	unsigned int			pointer_read[NR_CHANNELS];
 	unsigned int			pointer_write[NR_CHANNELS];
@@ -275,23 +274,19 @@ static int ipoctal_irq_handler(void *arg)
 						     CR_CMD_RESET_ERR_STATUS);
 
 				if (sr & SR_OVERRUN_ERROR) {
-					ipoctal->error_flag[channel] |= UART_OVERRUN;
 					ipoctal->chan_stats[channel].overrun_err++;
 					/* Overrun doesn't affect the current character*/
 					tty_insert_flip_char(tty, 0, TTY_OVERRUN);
 				}
 				if (sr & SR_PARITY_ERROR) {
-					ipoctal->error_flag[channel] |= UART_PARITY;
 					ipoctal->chan_stats[channel].parity_err++;
 					flag = TTY_PARITY;
 				}
 				if (sr & SR_FRAMING_ERROR) {
-					ipoctal->error_flag[channel] |= UART_FRAMING;
 					ipoctal->chan_stats[channel].framing_err++;
 					flag = TTY_FRAME;
 				}
 				if (sr & SR_RECEIVED_BREAK) {
-					ipoctal->error_flag[channel] |= UART_BREAK;
 					ipoctal->chan_stats[channel].rcv_break++;
 					flag = TTY_BREAK;
 				}
@@ -493,7 +488,6 @@ static int ipoctal_inst_slot(struct ipoctal *ipoctal, unsigned int bus_nr,
 		ipoctal_reset_stats(&ipoctal->chan_stats[i]);
 		ipoctal->nb_bytes[i] = 0;
 		init_waitqueue_head(&ipoctal->queue[i]);
-		ipoctal->error_flag[i] = UART_NOERROR;
 
 		spin_lock_init(&ipoctal->lock[i]);
 		ipoctal->pointer_read[i] = 0;
@@ -552,8 +546,6 @@ static int ipoctal_write(struct ipoctal *ipoctal, unsigned int channel,
 
 	ipoctal_copy_write_buffer(ipoctal, channel, buf, count);
 
-	ipoctal->error_flag[channel] = UART_NOERROR;
-
 	/* As the IP-OCTAL 485 only supports half duplex, do it manually */
 	if (ipoctal->board_id == IP_OCTAL_485_ID) {
 		ipoctal_write_io_reg(ipoctal,
diff --git a/drivers/staging/ipack/devices/ipoctal.h b/drivers/staging/ipack/devices/ipoctal.h
index b3c901c..2aae170 100644
--- a/drivers/staging/ipack/devices/ipoctal.h
+++ b/drivers/staging/ipack/devices/ipoctal.h
@@ -29,19 +29,6 @@ enum uart_parity_e {
 };
 
 /**
- * enum uart_error - UART error type
- *
- */
-enum uart_error	{
-	UART_NOERROR = 0,      /* No error during transmission */
-	UART_TIMEOUT = 1 << 0, /* Timeout error */
-	UART_OVERRUN = 1 << 1, /* Overrun error */
-	UART_PARITY  = 1 << 2, /* Parity error */
-	UART_FRAMING = 1 << 3, /* Framing error */
-	UART_BREAK   = 1 << 4, /* Received break */
-};
-
-/**
  * struct ipoctal_stats -- Stats since last reset
  *
  * @tx: Number of transmitted bytes
-- 
1.7.9.5

[PATCH 8/9] Staging: ipack/devices/ipoctal: remove unused enum uart_parity_e.

[Miguel Gómez]

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/ipack/devices/ipoctal.h |    9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/staging/ipack/devices/ipoctal.h b/drivers/staging/ipack/devices/ipoctal.h
index 2aae170..c5b4ed4 100644
--- a/drivers/staging/ipack/devices/ipoctal.h
+++ b/drivers/staging/ipack/devices/ipoctal.h
@@ -20,15 +20,6 @@
 #define RELEVANT_IFLAG(iflag) ((iflag) & (IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK))
 
 /**
- * enum uart_parity_e - UART supported parity.
- */
-enum uart_parity_e {
-	UART_NONE  = 0,
-	UART_ODD   = 1,
-	UART_EVEN  = 2,
-};
-
-/**
  * struct ipoctal_stats -- Stats since last reset
  *
  * @tx: Number of transmitted bytes
-- 
1.7.9.5

[PATCH 9/9] Staging: ipack/bridges/tpci200: change device table definition and export it.

[Miguel Gómez]

Use DEFINE_PCI_DEVICE_TABLE() to create the device table and add
MODULE_DEVICE_TABLE() to export it.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index 51448c7..75bb7e2 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -870,12 +870,14 @@ static void __devexit tpci200_pci_remove(struct pci_dev *dev)
 	}
 }
 
-static struct pci_device_id tpci200_idtable[2] = {
+static DEFINE_PCI_DEVICE_TABLE(tpci200_idtable) = {
 	{ TPCI200_VENDOR_ID, TPCI200_DEVICE_ID, TPCI200_SUBVENDOR_ID,
 	  TPCI200_SUBDEVICE_ID },
 	{ 0, },
 };
 
+MODULE_DEVICE_TABLE(pci, tpci200_idtable);
+
 static struct pci_driver tpci200_pci_drv = {
 	.name = "tpci200",
 	.id_table = tpci200_idtable,
-- 
1.7.9.5

Re: [PATCH 1/9] Staging: ipack/bridges/tpci200: remove gotos in tpci200_free_irq().

[Dan Carpenter]

On Thu, Jun 07, 2012 at 10:24:50AM +0200, Miguel Gómez wrote:
> Handle error conditions with simple returns instead of usig gotos.
> 
> Signed-off-by: Miguel Gómez <magomez@igalia.com>

Nope.  It was still supposed to release the lock on success.

Keep the out_unlock label, and remove the out label.

regards,
dan carpenter

[PATCH v2 1/9] Staging: ipack/bridges/tpci200: remove gotos in tpci200_free_irq().

[Miguel Gómez]

Handle error conditions with simple returns instead of usig gotos.

Signed-off-by: Miguel Gómez <magomez@igalia.com>
---
v2: release lock on success
---
 drivers/staging/ipack/bridges/tpci200.c |   21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index c39499a..68bcc55 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -512,24 +512,19 @@ static void __tpci200_free_irq(struct tpci200_board *tpci200,
 
 static int tpci200_free_irq(struct ipack_device *dev)
 {
-	int res;
 	struct slot_irq *slot_irq;
 	struct tpci200_board *tpci200;
 
 	tpci200 = check_slot(dev);
-	if (tpci200 == NULL) {
-		res = -EINVAL;
-		goto out;
-	}
+	if (tpci200 == NULL)
+		return -EINVAL;
 
-	if (mutex_lock_interruptible(&tpci200->mutex)) {
-		res = -ERESTARTSYS;
-		goto out;
-	}
+	if (mutex_lock_interruptible(&tpci200->mutex))
+		return -ERESTARTSYS;
 
 	if (tpci200->slots[dev->slot].irq == NULL) {
-		res = -EINVAL;
-		goto out_unlock;
+		mutex_unlock(&tpci200->mutex);
+		return -EINVAL;
 	}
 
 	__tpci200_free_irq(tpci200, dev);
@@ -537,10 +532,8 @@ static int tpci200_free_irq(struct ipack_device *dev)
 	tpci200->slots[dev->slot].irq = NULL;
 	kfree(slot_irq);
 
-out_unlock:
 	mutex_unlock(&tpci200->mutex);
-out:
-	return res;
+	return 0;
 }
 
 static int tpci200_slot_unmap_space(struct ipack_device *dev, int space)
-- 
1.7.9.5