spi: spi-ep93xx: use read,write instead of __raw_* variants

PATCH Applied H Hartley Sweeten 3 participants 4 messages Jun 28, 2013 – Jul 1, 2013 SPI

Share & tools

H Hartley Sweeten

author

2013-06-28 18:42 UTC #0 lore raw

PATCH 1/8 spi: spi-ep93xx: use read,write instead of __raw_* variants

The memory resource used by this driver is ioremap()'d and the normal read,write calls can be used instead of the __raw_* variants.

Remove the inline read,write helpers and just do the read,write directly in the callers.

Signed-off-by H Hartley Sweeten <[email protected]>

Cc Ryan Mallon <[email protected]>

Cc Mika Westerberg <[email protected]>

Cc Mark Brown <[email protected]>

Cc Grant Likely <[email protected]>

---
 drivers/spi/spi-ep93xx.c | 64 +++++++++++++++---------------------------------
 1 file changed, 20 insertions(+), 44 deletions(-)

drivers/spi/spi-ep93xx.c +20 ~~-44~~

index d7bac60..c633cd0 100644
@@ -158,30 +158,6 @@ struct ep93xx_spi_chip {
 /* converts bits per word to CR0.DSS value */
 #define bits_per_word_to_dss(bpw)	((bpw) - 1)
 
-static inline void
-ep93xx_spi_write_u8(const struct ep93xx_spi *espi, u16 reg, u8 value)
-{
-	__raw_writeb(value, espi->regs_base + reg);
-}
-
-static inline u8
-ep93xx_spi_read_u8(const struct ep93xx_spi *spi, u16 reg)
-{
-	return __raw_readb(spi->regs_base + reg);
-}
-
-static inline void
-ep93xx_spi_write_u16(const struct ep93xx_spi *espi, u16 reg, u16 value)
-{
-	__raw_writew(value, espi->regs_base + reg);
-}
-
-static inline u16
-ep93xx_spi_read_u16(const struct ep93xx_spi *spi, u16 reg)
-{
-	return __raw_readw(spi->regs_base + reg);
-}
-
 static int ep93xx_spi_enable(const struct ep93xx_spi *espi)
 {
 	u8 regval;

@@ -191,9 +167,9 @@ static int ep93xx_spi_enable(const struct ep93xx_spi *espi)
 	if (err)
 		return err;
 
-	regval = ep93xx_spi_read_u8(espi, SSPCR1);
+	regval = readb(espi->regs_base + SSPCR1);
 	regval |= SSPCR1_SSE;
-	ep93xx_spi_write_u8(espi, SSPCR1, regval);
+	writeb(regval, espi->regs_base + SSPCR1);
 
 	return 0;
 }

@@ -202,9 +178,9 @@ static void ep93xx_spi_disable(const struct ep93xx_spi *espi)
 {
 	u8 regval;
 
-	regval = ep93xx_spi_read_u8(espi, SSPCR1);
+	regval = readb(espi->regs_base + SSPCR1);
 	regval &= ~SSPCR1_SSE;
-	ep93xx_spi_write_u8(espi, SSPCR1, regval);
+	writeb(regval, espi->regs_base + SSPCR1);
 
 	clk_disable(espi->clk);
 }

@@ -213,18 +189,18 @@ static void ep93xx_spi_enable_interrupts(const struct ep93xx_spi *espi)
 {
 	u8 regval;
 
-	regval = ep93xx_spi_read_u8(espi, SSPCR1);
+	regval = readb(espi->regs_base + SSPCR1);
 	regval |= (SSPCR1_RORIE | SSPCR1_TIE | SSPCR1_RIE);
-	ep93xx_spi_write_u8(espi, SSPCR1, regval);
+	writeb(regval, espi->regs_base + SSPCR1);
 }
 
 static void ep93xx_spi_disable_interrupts(const struct ep93xx_spi *espi)
 {
 	u8 regval;
 
-	regval = ep93xx_spi_read_u8(espi, SSPCR1);
+	regval = readb(espi->regs_base + SSPCR1);
 	regval &= ~(SSPCR1_RORIE | SSPCR1_TIE | SSPCR1_RIE);
-	ep93xx_spi_write_u8(espi, SSPCR1, regval);
+	writeb(regval, espi->regs_base + SSPCR1);
 }
 
 /**

@@ -437,8 +413,8 @@ static void ep93xx_spi_chip_setup(const struct ep93xx_spi *espi,
 		chip->spi->mode, chip->div_cpsr, chip->div_scr, chip->dss);
 	dev_dbg(&espi->pdev->dev, "setup: cr0 %#x", cr0);
 
-	ep93xx_spi_write_u8(espi, SSPCPSR, chip->div_cpsr);
-	ep93xx_spi_write_u16(espi, SSPCR0, cr0);
+	writeb(chip->div_cpsr, espi->regs_base + SSPCPSR);
+	writew(cr0, espi->regs_base + SSPCR0);
 }
 
 static inline int bits_per_word(const struct ep93xx_spi *espi)

@@ -456,14 +432,14 @@ static void ep93xx_do_write(struct ep93xx_spi *espi, struct spi_transfer *t)
 
 		if (t->tx_buf)
 			tx_val = ((u16 *)t->tx_buf)[espi->tx];
-		ep93xx_spi_write_u16(espi, SSPDR, tx_val);
+		writew(tx_val, espi->regs_base + SSPDR);
 		espi->tx += sizeof(tx_val);
 	} else {
 		u8 tx_val = 0;
 
 		if (t->tx_buf)
 			tx_val = ((u8 *)t->tx_buf)[espi->tx];
-		ep93xx_spi_write_u8(espi, SSPDR, tx_val);
+		writeb(tx_val, espi->regs_base + SSPDR);
 		espi->tx += sizeof(tx_val);
 	}
 }

@@ -473,14 +449,14 @@ static void ep93xx_do_read(struct ep93xx_spi *espi, struct spi_transfer *t)
 	if (bits_per_word(espi) > 8) {
 		u16 rx_val;
 
-		rx_val = ep93xx_spi_read_u16(espi, SSPDR);
+		rx_val = readw(espi->regs_base + SSPDR);
 		if (t->rx_buf)
 			((u16 *)t->rx_buf)[espi->rx] = rx_val;
 		espi->rx += sizeof(rx_val);
 	} else {
 		u8 rx_val;
 
-		rx_val = ep93xx_spi_read_u8(espi, SSPDR);
+		rx_val = readb(espi->regs_base + SSPDR);
 		if (t->rx_buf)
 			((u8 *)t->rx_buf)[espi->rx] = rx_val;
 		espi->rx += sizeof(rx_val);

@@ -504,7 +480,7 @@ static int ep93xx_spi_read_write(struct ep93xx_spi *espi)
 	struct spi_transfer *t = msg->state;
 
 	/* read as long as RX FIFO has frames in it */
-	while ((ep93xx_spi_read_u8(espi, SSPSR) & SSPSR_RNE)) {
+	while ((readb(espi->regs_base + SSPSR) & SSPSR_RNE)) {
 		ep93xx_do_read(espi, t);
 		espi->fifo_level--;
 	}

@@ -831,14 +807,14 @@ static void ep93xx_spi_process_message(struct ep93xx_spi *espi,
 	 * Just to be sure: flush any data from RX FIFO.
 	 */
 	timeout = jiffies + msecs_to_jiffies(SPI_TIMEOUT);
-	while (ep93xx_spi_read_u16(espi, SSPSR) & SSPSR_RNE) {
+	while (readw(espi->regs_base + SSPSR) & SSPSR_RNE) {
 		if (time_after(jiffies, timeout)) {
 			dev_warn(&espi->pdev->dev,
 				 "timeout while flushing RX FIFO\n");
 			msg->status = -ETIMEDOUT;
 			return;
 		}
-		ep93xx_spi_read_u16(espi, SSPDR);
+		readw(espi->regs_base + SSPDR);
 	}
 
 	/*

@@ -917,7 +893,7 @@ static void ep93xx_spi_work(struct work_struct *work)
 static irqreturn_t ep93xx_spi_interrupt(int irq, void *dev_id)
 {
 	struct ep93xx_spi *espi = dev_id;
-	u8 irq_status = ep93xx_spi_read_u8(espi, SSPIIR);
+	u8 irq_status = readb(espi->regs_base + SSPIIR);
 
 	/*
 	 * If we got ROR (receive overrun) interrupt we know that something is

@@ -925,7 +901,7 @@ static irqreturn_t ep93xx_spi_interrupt(int irq, void *dev_id)
 	 */
 	if (unlikely(irq_status & SSPIIR_RORIS)) {
 		/* clear the overrun interrupt */
-		ep93xx_spi_write_u8(espi, SSPICR, 0);
+		writeb(0, espi->regs_base + SSPICR);
 		dev_warn(&espi->pdev->dev,
 			 "receive overrun, aborting the message\n");
 		espi->current_msg->status = -EIO;

@@ -1111,7 +1087,7 @@ static int ep93xx_spi_probe(struct platform_device *pdev)
 	espi->running = true;
 
 	/* make sure that the hardware is disabled */
-	ep93xx_spi_write_u8(espi, SSPCR1, 0);
+	writeb(0, espi->regs_base + SSPCR1);
 
 	error = spi_register_master(master);
 	if (error) {

Signature · 1 line

1.8.1.4

Ryan Mallon

2013-06-28 23:15 UTC #1 lore raw

Quoted 24 lines · H Hartley Sweeten

The memory resource used by this driver is ioremap()'d and the normal read,write calls can be used instead of the __raw_* variants.
Remove the inline read,write helpers and just do the read,write directly in the callers.
Signed-off-by H Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>
Cc Mika Westerberg <[email protected]>
Cc Mark Brown <[email protected]>
Cc Grant Likely <[email protected]>
---
 drivers/spi/spi-ep93xx.c | 64 +++++++++++++++---------------------------------
 1 file changed, 20 insertions(+), 44 deletions(-)
drivers/spi/spi-ep93xx.c +0 -1
index d7bac60..c633cd0 100644
@@ -158,30 +158,6 @@ struct ep93xx_spi_chip {
 /* converts bits per word to CR0.DSS value */
 #define bits_per_word_to_dss(bpw)	((bpw) - 1)
 
-static inline void

drivers/spi/spi-ep93xx.c struct ep93xx_spi_chip { · line 171

Quoted 10 lines

drivers/spi/spi-ep93xx.c +0 ~~-10~~

-ep93xx_spi_write_u8(const struct ep93xx_spi *espi, u16 reg, u8 value)
-{
-	__raw_writeb(value, espi->regs_base + reg);
-}
-
-static inline u8
-ep93xx_spi_read_u8(const struct ep93xx_spi *spi, u16 reg)
-{
-	return __raw_readb(spi->regs_base + reg);
-}

Is there a particular reason to drop these functions? It's basically just bike-shedding, but they can make the code more readable at very little cost. Even dropping the inline (which is preferred nowdays) the compiler will still inline these, and it would also make this patch much smaller to keep them.

~Ryan

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev

Mark Brown

maintainer

2013-07-01 10:57 UTC #2 lore raw

Quoted 13 lines · Ryan Mallon

drivers/spi/spi-ep93xx.c struct ep93xx_spi_chip { · line 171
Quoted 5 lines · H Hartley Sweeten
drivers/spi/spi-ep93xx.c +0 -5
-static inline u8
-ep93xx_spi_read_u8(const struct ep93xx_spi *spi, u16 reg)
-{
-	return __raw_readb(spi->regs_base + reg);
-}
Is there a particular reason to drop these functions? It's basically just bike-shedding, but they can make the code more readable at very little cost. Even dropping the inline (which is preferred nowdays) the compiler will still inline these, and it would also make this patch much smaller to keep them.

I tend to agree, it's much more normal to have the base + reg in a function than not.

H Hartley Sweeten

author

2013-07-01 18:17 UTC #3 lore raw

Quoted 15 lines · Mark Brown

Quoted 13 lines · Ryan Mallon
drivers/spi/spi-ep93xx.c struct ep93xx_spi_chip { · line 171
Quoted 5 lines · H Hartley Sweeten
drivers/spi/spi-ep93xx.c +0 -5
-static inline u8
-ep93xx_spi_read_u8(const struct ep93xx_spi *spi, u16 reg)
-{
-	return __raw_readb(spi->regs_base + reg);
-}
Is there a particular reason to drop these functions? It's basically just bike-shedding, but they can make the code more readable at very little cost. Even dropping the inline (which is preferred nowdays) the compiler will still inline these, and it would also make this patch much smaller to keep them.

Quoted 2 lines

I tend to agree, it's much more normal to have the base + reg in a function than not.

OK. I will redo this one to just remove the __raw_.

Regards,
Hartley

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev