diff mbox series

[spi,for-5.4,14/14] spi: spi-fsl-dspi: Remove impossible to reach error check

Message ID 20190818180115.31114-15-olteanv@gmail.com (mailing list archive)
State New, archived
Headers show
Series NXP DSPI driver cleanup | expand

Commit Message

Vladimir Oltean Aug. 18, 2019, 6:01 p.m. UTC 
dspi->devtype_data is under the total control of the driver. Therefore,
a bad value is a driver bug and checking it at runtime (and during an
ISR, at that!) is pointless.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/spi/spi-fsl-dspi.c | 26 ++++----------------------
 1 file changed, 4 insertions(+), 22 deletions(-)

Comments

Mark Brown Aug. 20, 2019, 12:59 p.m. UTC | #1 
On Sun, Aug 18, 2019 at 09:01:15PM +0300, Vladimir Oltean wrote:

> dspi->devtype_data is under the total control of the driver. Therefore,
> a bad value is a driver bug and checking it at runtime (and during an
> ISR, at that!) is pointless.

> -	trans_mode = dspi->devtype_data->trans_mode;
> -	switch (trans_mode) {
> -	case DSPI_EOQ_MODE:
> +	if (dspi->devtype_data->trans_mode == DSPI_EOQ_MODE)
>  		dspi_eoq_read(dspi);
> -		break;
> -	case DSPI_TCFQ_MODE:

The other reason for doing a switch statement is that it makes it easier
to add new condidions if more modes are added for some reason, the error
check is a nice side effect for robustness since it'll detect memory
corruption or other bugs but it's not the only reason to use this idiom.
diff mbox series

Patch

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 2993d15f640e..238bbe172b79 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -651,7 +651,6 @@  static irqreturn_t dspi_interrupt(int irq, void *dev_id)
 {
 	struct fsl_dspi *dspi = (struct fsl_dspi *)dev_id;
 	struct spi_message *msg = dspi->cur_msg;
-	enum dspi_trans_mode trans_mode;
 	u32 spi_sr, spi_tcr;
 	u16 spi_tcnt;
 
@@ -666,19 +665,10 @@  static irqreturn_t dspi_interrupt(int irq, void *dev_id)
 	/* Update total number of bytes that were transferred */
 	msg->actual_length += spi_tcnt * dspi->bytes_per_word;
 
-	trans_mode = dspi->devtype_data->trans_mode;
-	switch (trans_mode) {
-	case DSPI_EOQ_MODE:
+	if (dspi->devtype_data->trans_mode == DSPI_EOQ_MODE)
 		dspi_eoq_read(dspi);
-		break;
-	case DSPI_TCFQ_MODE:
+	else
 		dspi_tcfq_read(dspi);
-		break;
-	default:
-		dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
-			trans_mode);
-			return IRQ_HANDLED;
-	}
 
 	if (!dspi->len) {
 		dspi->waitflags = 1;
@@ -686,18 +676,10 @@  static irqreturn_t dspi_interrupt(int irq, void *dev_id)
 		return IRQ_HANDLED;
 	}
 
-	switch (trans_mode) {
-	case DSPI_EOQ_MODE:
+	if (dspi->devtype_data->trans_mode == DSPI_EOQ_MODE)
 		dspi_eoq_write(dspi);
-		break;
-	case DSPI_TCFQ_MODE:
+	else
 		dspi_tcfq_write(dspi);
-		break;
-	default:
-		dev_err(&dspi->pdev->dev,
-			"unsupported trans_mode %u\n",
-			trans_mode);
-	}
 
 	return IRQ_HANDLED;
 }