[PATCH] spi: tegra: avoid unsigned->signed->unsigned promotion

[PATCH] spi: tegra: avoid unsigned->signed->unsigned promotion

[Michal Nazarewicz]

From: Michal Nazarewicz <mina86-deATy8a+UHjQT0dZR+AlfA@public.gmane.org>

u8 type, which is unsigned, is promoted to int, which is singned, when
doing a binary shift.  Then, on 64-bit machines, it is further promoted
to unsigned long which may lead to more significant half of the value
to be all ones.  To avoid this, explicitly promote to an unsigned type.

Signed-off-by: Michal Nazarewicz <mina86-deATy8a+UHjQT0dZR+AlfA@public.gmane.org>
---
 drivers/spi/spi-tegra114.c      | 4 ++--
 drivers/spi/spi-tegra20-slink.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
index aaecfb3..2dadc3f 100644
--- a/drivers/spi/spi-tegra114.c
+++ b/drivers/spi/spi-tegra114.c
@@ -315,7 +315,7 @@ static unsigned tegra_spi_fill_tx_fifo_from_client_txbuf(
 		for (count = 0; count < max_n_32bit; count++) {
 			x = 0;
 			for (i = 0; (i < 4) && nbytes; i++, nbytes--)
-				x |= (*tx_buf++) << (i*8);
+				x |= (unsigned)(*tx_buf++) << (i*8);
 			tegra_spi_writel(tspi, x, SPI_TX_FIFO);
 		}
 	} else {
@@ -326,7 +326,7 @@ static unsigned tegra_spi_fill_tx_fifo_from_client_txbuf(
 			x = 0;
 			for (i = 0; nbytes && (i < tspi->bytes_per_word);
 							i++, nbytes--)
-				x |= ((*tx_buf++) << i*8);
+				x |= (unsigned)(*tx_buf++) << (i*8);
 			tegra_spi_writel(tspi, x, SPI_TX_FIFO);
 		}
 	}
diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c
index e66715b..0fc4f7d 100644
--- a/drivers/spi/spi-tegra20-slink.c
+++ b/drivers/spi/spi-tegra20-slink.c
@@ -331,7 +331,7 @@ static unsigned tegra_slink_fill_tx_fifo_from_client_txbuf(
 		for (count = 0; count < max_n_32bit; count++) {
 			x = 0;
 			for (i = 0; (i < 4) && nbytes; i++, nbytes--)
-				x |= (*tx_buf++) << (i*8);
+				x |= (unsigned)(*tx_buf++) << (i*8);
 			tegra_slink_writel(tspi, x, SLINK_TX_FIFO);
 		}
 	} else {
@@ -342,7 +342,7 @@ static unsigned tegra_slink_fill_tx_fifo_from_client_txbuf(
 			x = 0;
 			for (i = 0; nbytes && (i < tspi->bytes_per_word);
 							i++, nbytes--)
-				x |= ((*tx_buf++) << i*8);
+				x |= (unsigned)(*tx_buf++) << (i*8);
 			tegra_slink_writel(tspi, x, SLINK_TX_FIFO);
 		}
 	}
-- 
1.8.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] spi: tegra: avoid unsigned->signed->unsigned promotion

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]

On Fri, Nov 29, 2013 at 06:12:44PM +0100, Michal Nazarewicz wrote:
> From: Michal Nazarewicz <mina86-deATy8a+UHjQT0dZR+AlfA@public.gmane.org>
> 
> u8 type, which is unsigned, is promoted to int, which is singned, when
> doing a binary shift.  Then, on 64-bit machines, it is further promoted
> to unsigned long which may lead to more significant half of the value
> to be all ones.  To avoid this, explicitly promote to an unsigned type.

So this would only become an problem on 64-bit Tegra SoCs, right? I
suspect that there are quite a few places where something similar is
done and which suffer from the same issue.

But I don't think there's a real issue here. Even on 64-bit platforms,
writel() will only write 32-bit registers, and therefore discard the
upper 32 bits.

I think in general perhaps a more proper fix would be to use only u32
instead of unsigned long for these cases, since then the problem goes
away. u32 is also the type of the value used by writel() so it makes
perfect sense to use it.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] spi: tegra: avoid unsigned->signed->unsigned promotion

[Michal Nazarewicz]

[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]

> On Fri, Nov 29, 2013 at 06:12:44PM +0100, Michal Nazarewicz wrote:
>> u8 type, which is unsigned, is promoted to int, which is singned, when
>> doing a binary shift.  Then, on 64-bit machines, it is further promoted
>> to unsigned long which may lead to more significant half of the value
>> to be all ones.  To avoid this, explicitly promote to an unsigned type.

On Fri, Nov 29 2013, Thierry Reding wrote:
> But I don't think there's a real issue here. Even on 64-bit platforms,
> writel() will only write 32-bit registers, and therefore discard the
> upper 32 bits.
>
> I think in general perhaps a more proper fix would be to use only u32
> instead of unsigned long for these cases, since then the problem goes
> away. u32 is also the type of the value used by writel() so it makes
> perfect sense to use it.

Yes, you're absolutely right.  I dunno why I missed the fact it's used
in writel only.  I'll prepare a new patch soon(ish).

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>--<xmpp:mina86-/eSpBmjxGS4dnm+yROfE0A@public.gmane.org>--ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 835 bytes --]

[PATCH 0/3] spi: tegra: use u32 for 32-bit register values

[Michal Nazarewicz]

This series has only been compile-tested, but it does not introduce any
behaviour changes.

Michal Nazarewicz (3):
  spi: tegra114: use u32 for 32-bit register values
  spi: tegra20-slink: use u32 for 32-bit register values
  spi: tegra20-sflash: use u32 for 32-bit register values

 drivers/spi/spi-tegra114.c       | 98 ++++++++++++++--------------------------
 drivers/spi/spi-tegra20-sflash.c | 22 ++++-----
 drivers/spi/spi-tegra20-slink.c  | 97 ++++++++++++++++-----------------------
 3 files changed, 84 insertions(+), 133 deletions(-)

-- 
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] spi: tegra114: use u32 for 32-bit register values

[Michal Nazarewicz]

Previously used “unsigned long” may lead to confusion should the code
be compiled for 64-bit machine.

This commit also removes some unused fields of the tegra_spi_data
structure as well as removes duplicated #defines.

Signed-off-by: Michal Nazarewicz <mina86-deATy8a+UHjQT0dZR+AlfA@public.gmane.org>
---
 drivers/spi/spi-tegra114.c | 98 ++++++++++++++++------------------------------
 1 file changed, 34 insertions(+), 64 deletions(-)

diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
index aaecfb3..272ac47 100644
--- a/drivers/spi/spi-tegra114.c
+++ b/drivers/spi/spi-tegra114.c
@@ -54,11 +54,8 @@
 #define SPI_CS_SS_VAL				(1 << 20)
 #define SPI_CS_SW_HW				(1 << 21)
 /* SPI_CS_POL_INACTIVE bits are default high */
-#define SPI_CS_POL_INACTIVE			22
-#define SPI_CS_POL_INACTIVE_0			(1 << 22)
-#define SPI_CS_POL_INACTIVE_1			(1 << 23)
-#define SPI_CS_POL_INACTIVE_2			(1 << 24)
-#define SPI_CS_POL_INACTIVE_3			(1 << 25)
+						/* n from 0 to 3 */
+#define SPI_CS_POL_INACTIVE(n)			(1 << (22 + (n)))
 #define SPI_CS_POL_INACTIVE_MASK		(0xF << 22)
 
 #define SPI_CS_SEL_0				(0 << 26)
@@ -165,9 +162,6 @@
 #define MAX_HOLD_CYCLES				16
 #define SPI_DEFAULT_SPEED			25000000
 
-#define MAX_CHIP_SELECT				4
-#define SPI_FIFO_DEPTH				64
-
 struct tegra_spi_data {
 	struct device				*dev;
 	struct spi_master			*master;
@@ -184,7 +178,6 @@ struct tegra_spi_data {
 	struct spi_device			*cur_spi;
 	struct spi_device			*cs_control;
 	unsigned				cur_pos;
-	unsigned				cur_len;
 	unsigned				words_per_32bit;
 	unsigned				bytes_per_word;
 	unsigned				curr_dma_words;
@@ -204,12 +197,10 @@ struct tegra_spi_data {
 	u32					rx_status;
 	u32					status_reg;
 	bool					is_packed;
-	unsigned long				packed_size;
 
 	u32					command1_reg;
 	u32					dma_control_reg;
 	u32					def_command1_reg;
-	u32					spi_cs_timing;
 
 	struct completion			xfer_completion;
 	struct spi_transfer			*curr_xfer;
@@ -227,14 +218,14 @@ struct tegra_spi_data {
 static int tegra_spi_runtime_suspend(struct device *dev);
 static int tegra_spi_runtime_resume(struct device *dev);
 
-static inline unsigned long tegra_spi_readl(struct tegra_spi_data *tspi,
+static inline u32 tegra_spi_readl(struct tegra_spi_data *tspi,
 		unsigned long reg)
 {
 	return readl(tspi->base + reg);
 }
 
 static inline void tegra_spi_writel(struct tegra_spi_data *tspi,
-		unsigned long val, unsigned long reg)
+		u32 val, unsigned long reg)
 {
 	writel(val, tspi->base + reg);
 
@@ -245,7 +236,7 @@ static inline void tegra_spi_writel(struct tegra_spi_data *tspi,
 
 static void tegra_spi_clear_status(struct tegra_spi_data *tspi)
 {
-	unsigned long val;
+	u32 val;
 
 	/* Write 1 to clear status register */
 	val = tegra_spi_readl(tspi, SPI_TRANS_STATUS);
@@ -296,10 +287,9 @@ static unsigned tegra_spi_fill_tx_fifo_from_client_txbuf(
 {
 	unsigned nbytes;
 	unsigned tx_empty_count;
-	unsigned long fifo_status;
+	u32 fifo_status;
 	unsigned max_n_32bit;
 	unsigned i, count;
-	unsigned long x;
 	unsigned int written_words;
 	unsigned fifo_words_left;
 	u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_tx_pos;
@@ -313,9 +303,9 @@ static unsigned tegra_spi_fill_tx_fifo_from_client_txbuf(
 		nbytes = written_words * tspi->bytes_per_word;
 		max_n_32bit = DIV_ROUND_UP(nbytes, 4);
 		for (count = 0; count < max_n_32bit; count++) {
-			x = 0;
+			u32 x = 0;
 			for (i = 0; (i < 4) && nbytes; i++, nbytes--)
-				x |= (*tx_buf++) << (i*8);
+				x |= (u32)(*tx_buf++) << (i * 8);
 			tegra_spi_writel(tspi, x, SPI_TX_FIFO);
 		}
 	} else {
@@ -323,10 +313,10 @@ static unsigned tegra_spi_fill_tx_fifo_from_client_txbuf(
 		written_words = max_n_32bit;
 		nbytes = written_words * tspi->bytes_per_word;
 		for (count = 0; count < max_n_32bit; count++) {
-			x = 0;
+			u32 x = 0;
 			for (i = 0; nbytes && (i < tspi->bytes_per_word);
 							i++, nbytes--)
-				x |= ((*tx_buf++) << i*8);
+				x |= (u32)(*tx_buf++) << (i * 8);
 			tegra_spi_writel(tspi, x, SPI_TX_FIFO);
 		}
 	}
@@ -338,9 +328,8 @@ static unsigned int tegra_spi_read_rx_fifo_to_client_rxbuf(
 		struct tegra_spi_data *tspi, struct spi_transfer *t)
 {
 	unsigned rx_full_count;
-	unsigned long fifo_status;
+	u32 fifo_status;
 	unsigned i, count;
-	unsigned long x;
 	unsigned int read_words = 0;
 	unsigned len;
 	u8 *rx_buf = (u8 *)t->rx_buf + tspi->cur_rx_pos;
@@ -350,20 +339,16 @@ static unsigned int tegra_spi_read_rx_fifo_to_client_rxbuf(
 	if (tspi->is_packed) {
 		len = tspi->curr_dma_words * tspi->bytes_per_word;
 		for (count = 0; count < rx_full_count; count++) {
-			x = tegra_spi_readl(tspi, SPI_RX_FIFO);
+			u32 x = tegra_spi_readl(tspi, SPI_RX_FIFO);
 			for (i = 0; len && (i < 4); i++, len--)
 				*rx_buf++ = (x >> i*8) & 0xFF;
 		}
 		tspi->cur_rx_pos += tspi->curr_dma_words * tspi->bytes_per_word;
 		read_words += tspi->curr_dma_words;
 	} else {
-		unsigned int rx_mask;
-		unsigned int bits_per_word = t->bits_per_word;
-
-		rx_mask = (1 << bits_per_word) - 1;
+		u32 rx_mask = ((u32)1 << t->bits_per_word) - 1;
 		for (count = 0; count < rx_full_count; count++) {
-			x = tegra_spi_readl(tspi, SPI_RX_FIFO);
-			x &= rx_mask;
+			u32 x = tegra_spi_readl(tspi, SPI_RX_FIFO) & rx_mask;
 			for (i = 0; (i < tspi->bytes_per_word); i++)
 				*rx_buf++ = (x >> (i*8)) & 0xFF;
 		}
@@ -376,27 +361,24 @@ static unsigned int tegra_spi_read_rx_fifo_to_client_rxbuf(
 static void tegra_spi_copy_client_txbuf_to_spi_txbuf(
 		struct tegra_spi_data *tspi, struct spi_transfer *t)
 {
-	unsigned len;
-
 	/* Make the dma buffer to read by cpu */
 	dma_sync_single_for_cpu(tspi->dev, tspi->tx_dma_phys,
 				tspi->dma_buf_size, DMA_TO_DEVICE);
 
 	if (tspi->is_packed) {
-		len = tspi->curr_dma_words * tspi->bytes_per_word;
+		unsigned len = tspi->curr_dma_words * tspi->bytes_per_word;
 		memcpy(tspi->tx_dma_buf, t->tx_buf + tspi->cur_pos, len);
 	} else {
 		unsigned int i;
 		unsigned int count;
 		u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_tx_pos;
 		unsigned consume = tspi->curr_dma_words * tspi->bytes_per_word;
-		unsigned int x;
 
 		for (count = 0; count < tspi->curr_dma_words; count++) {
-			x = 0;
+			u32 x = 0;
 			for (i = 0; consume && (i < tspi->bytes_per_word);
 							i++, consume--)
-				x |= ((*tx_buf++) << i * 8);
+				x |= (u32)(*tx_buf++) << (i * 8);
 			tspi->tx_dma_buf[count] = x;
 		}
 	}
@@ -410,27 +392,21 @@ static void tegra_spi_copy_client_txbuf_to_spi_txbuf(
 static void tegra_spi_copy_spi_rxbuf_to_client_rxbuf(
 		struct tegra_spi_data *tspi, struct spi_transfer *t)
 {
-	unsigned len;
-
 	/* Make the dma buffer to read by cpu */
 	dma_sync_single_for_cpu(tspi->dev, tspi->rx_dma_phys,
 		tspi->dma_buf_size, DMA_FROM_DEVICE);
 
 	if (tspi->is_packed) {
-		len = tspi->curr_dma_words * tspi->bytes_per_word;
+		unsigned len = tspi->curr_dma_words * tspi->bytes_per_word;
 		memcpy(t->rx_buf + tspi->cur_rx_pos, tspi->rx_dma_buf, len);
 	} else {
 		unsigned int i;
 		unsigned int count;
 		unsigned char *rx_buf = t->rx_buf + tspi->cur_rx_pos;
-		unsigned int x;
-		unsigned int rx_mask;
-		unsigned int bits_per_word = t->bits_per_word;
+		u32 rx_mask = ((u32)1 << t->bits_per_word) - 1;
 
-		rx_mask = (1 << bits_per_word) - 1;
 		for (count = 0; count < tspi->curr_dma_words; count++) {
-			x = tspi->rx_dma_buf[count];
-			x &= rx_mask;
+			u32 x = tspi->rx_dma_buf[count] & rx_mask;
 			for (i = 0; (i < tspi->bytes_per_word); i++)
 				*rx_buf++ = (x >> (i*8)) & 0xFF;
 		}
@@ -490,16 +466,16 @@ static int tegra_spi_start_rx_dma(struct tegra_spi_data *tspi, int len)
 static int tegra_spi_start_dma_based_transfer(
 		struct tegra_spi_data *tspi, struct spi_transfer *t)
 {
-	unsigned long val;
+	u32 val;
 	unsigned int len;
 	int ret = 0;
-	unsigned long status;
+	u32 status;
 
 	/* Make sure that Rx and Tx fifo are empty */
 	status = tegra_spi_readl(tspi, SPI_FIFO_STATUS);
 	if ((status & SPI_FIFO_EMPTY) != SPI_FIFO_EMPTY) {
-		dev_err(tspi->dev,
-			"Rx/Tx fifo are not empty status 0x%08lx\n", status);
+		dev_err(tspi->dev, "Rx/Tx fifo are not empty status 0x%08x\n",
+			(unsigned)status);
 		return -EIO;
 	}
 
@@ -564,7 +540,7 @@ static int tegra_spi_start_dma_based_transfer(
 static int tegra_spi_start_cpu_based_transfer(
 		struct tegra_spi_data *tspi, struct spi_transfer *t)
 {
-	unsigned long val;
+	u32 val;
 	unsigned cur_words;
 
 	if (tspi->cur_direction & DATA_DIR_TX)
@@ -677,13 +653,13 @@ static void tegra_spi_deinit_dma_param(struct tegra_spi_data *tspi,
 	dma_release_channel(dma_chan);
 }
 
-static unsigned long tegra_spi_setup_transfer_one(struct spi_device *spi,
+static u32 tegra_spi_setup_transfer_one(struct spi_device *spi,
 		struct spi_transfer *t, bool is_first_of_msg)
 {
 	struct tegra_spi_data *tspi = spi_master_get_devdata(spi->master);
 	u32 speed = t->speed_hz;
 	u8 bits_per_word = t->bits_per_word;
-	unsigned long command1;
+	u32 command1;
 	int req_mode;
 
 	if (speed != tspi->cur_speed) {
@@ -738,7 +714,7 @@ static unsigned long tegra_spi_setup_transfer_one(struct spi_device *spi,
 }
 
 static int tegra_spi_start_transfer_one(struct spi_device *spi,
-		struct spi_transfer *t, unsigned long command1)
+		struct spi_transfer *t, u32 command1)
 {
 	struct tegra_spi_data *tspi = spi_master_get_devdata(spi->master);
 	unsigned total_fifo_words;
@@ -763,8 +739,8 @@ static int tegra_spi_start_transfer_one(struct spi_device *spi,
 	tegra_spi_writel(tspi, command1, SPI_COMMAND1);
 	tspi->command1_reg = command1;
 
-	dev_dbg(tspi->dev, "The def 0x%x and written 0x%lx\n",
-				tspi->def_command1_reg, command1);
+	dev_dbg(tspi->dev, "The def 0x%x and written 0x%x\n",
+		tspi->def_command1_reg, (unsigned)command1);
 
 	if (total_fifo_words > SPI_FIFO_DEPTH)
 		ret = tegra_spi_start_dma_based_transfer(tspi, t);
@@ -776,15 +752,9 @@ static int tegra_spi_start_transfer_one(struct spi_device *spi,
 static int tegra_spi_setup(struct spi_device *spi)
 {
 	struct tegra_spi_data *tspi = spi_master_get_devdata(spi->master);
-	unsigned long val;
+	u32 val;
 	unsigned long flags;
 	int ret;
-	unsigned int cs_pol_bit[MAX_CHIP_SELECT] = {
-			SPI_CS_POL_INACTIVE_0,
-			SPI_CS_POL_INACTIVE_1,
-			SPI_CS_POL_INACTIVE_2,
-			SPI_CS_POL_INACTIVE_3,
-	};
 
 	dev_dbg(&spi->dev, "setup %d bpw, %scpol, %scpha, %dHz\n",
 		spi->bits_per_word,
@@ -806,9 +776,9 @@ static int tegra_spi_setup(struct spi_device *spi)
 	spin_lock_irqsave(&tspi->lock, flags);
 	val = tspi->def_command1_reg;
 	if (spi->mode & SPI_CS_HIGH)
-		val &= ~cs_pol_bit[spi->chip_select];
+		val &= ~SPI_CS_POL_INACTIVE(spi->chip_select);
 	else
-		val |= cs_pol_bit[spi->chip_select];
+		val |= SPI_CS_POL_INACTIVE(spi->chip_select);
 	tspi->def_command1_reg = val;
 	tegra_spi_writel(tspi, tspi->def_command1_reg, SPI_COMMAND1);
 	spin_unlock_irqrestore(&tspi->lock, flags);
@@ -842,7 +812,7 @@ static int tegra_spi_transfer_one_message(struct spi_master *master,
 	msg->actual_length = 0;
 
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
-		unsigned long cmd1;
+		u32 cmd1;
 
 		reinit_completion(&tspi->xfer_completion);
 
-- 
1.8.4

[PATCH 2/3] spi: tegra20-slink: use u32 for 32-bit register values

[Michal Nazarewicz]

Previously used “unsigned long” may lead to confusion should the code
be compiled for 64-bit machine.

Signed-off-by: Michal Nazarewicz <mina86-deATy8a+UHjQT0dZR+AlfA@public.gmane.org>
---
 drivers/spi/spi-tegra20-slink.c | 97 +++++++++++++++++------------------------
 1 file changed, 40 insertions(+), 57 deletions(-)

diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c
index e66715b..0dcd6f9 100644
--- a/drivers/spi/spi-tegra20-slink.c
+++ b/drivers/spi/spi-tegra20-slink.c
@@ -196,7 +196,7 @@ struct tegra_slink_data {
 	u32					rx_status;
 	u32					status_reg;
 	bool					is_packed;
-	unsigned long				packed_size;
+	u32					packed_size;
 
 	u32					command_reg;
 	u32					command2_reg;
@@ -220,14 +220,14 @@ struct tegra_slink_data {
 static int tegra_slink_runtime_suspend(struct device *dev);
 static int tegra_slink_runtime_resume(struct device *dev);
 
-static inline unsigned long tegra_slink_readl(struct tegra_slink_data *tspi,
+static inline u32 tegra_slink_readl(struct tegra_slink_data *tspi,
 		unsigned long reg)
 {
 	return readl(tspi->base + reg);
 }
 
 static inline void tegra_slink_writel(struct tegra_slink_data *tspi,
-		unsigned long val, unsigned long reg)
+		u32 val, unsigned long reg)
 {
 	writel(val, tspi->base + reg);
 
@@ -238,38 +238,30 @@ static inline void tegra_slink_writel(struct tegra_slink_data *tspi,
 
 static void tegra_slink_clear_status(struct tegra_slink_data *tspi)
 {
-	unsigned long val;
-	unsigned long val_write = 0;
+	u32 val_write;
 
-	val = tegra_slink_readl(tspi, SLINK_STATUS);
+	tegra_slink_readl(tspi, SLINK_STATUS);
 
 	/* Write 1 to clear status register */
 	val_write = SLINK_RDY | SLINK_FIFO_ERROR;
 	tegra_slink_writel(tspi, val_write, SLINK_STATUS);
 }
 
-static unsigned long tegra_slink_get_packed_size(struct tegra_slink_data *tspi,
+static u32 tegra_slink_get_packed_size(struct tegra_slink_data *tspi,
 				  struct spi_transfer *t)
 {
-	unsigned long val;
-
 	switch (tspi->bytes_per_word) {
 	case 0:
-		val = SLINK_PACK_SIZE_4;
-		break;
+		return SLINK_PACK_SIZE_4;
 	case 1:
-		val = SLINK_PACK_SIZE_8;
-		break;
+		return SLINK_PACK_SIZE_8;
 	case 2:
-		val = SLINK_PACK_SIZE_16;
-		break;
+		return SLINK_PACK_SIZE_16;
 	case 4:
-		val = SLINK_PACK_SIZE_32;
-		break;
+		return SLINK_PACK_SIZE_32;
 	default:
-		val = 0;
+		return 0;
 	}
-	return val;
 }
 
 static unsigned tegra_slink_calculate_curr_xfer_param(
@@ -312,10 +304,9 @@ static unsigned tegra_slink_fill_tx_fifo_from_client_txbuf(
 {
 	unsigned nbytes;
 	unsigned tx_empty_count;
-	unsigned long fifo_status;
+	u32 fifo_status;
 	unsigned max_n_32bit;
 	unsigned i, count;
-	unsigned long x;
 	unsigned int written_words;
 	unsigned fifo_words_left;
 	u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_tx_pos;
@@ -329,9 +320,9 @@ static unsigned tegra_slink_fill_tx_fifo_from_client_txbuf(
 		nbytes = written_words * tspi->bytes_per_word;
 		max_n_32bit = DIV_ROUND_UP(nbytes, 4);
 		for (count = 0; count < max_n_32bit; count++) {
-			x = 0;
+			u32 x = 0;
 			for (i = 0; (i < 4) && nbytes; i++, nbytes--)
-				x |= (*tx_buf++) << (i*8);
+				x |= (u32)(*tx_buf++) << (i * 8);
 			tegra_slink_writel(tspi, x, SLINK_TX_FIFO);
 		}
 	} else {
@@ -339,10 +330,10 @@ static unsigned tegra_slink_fill_tx_fifo_from_client_txbuf(
 		written_words = max_n_32bit;
 		nbytes = written_words * tspi->bytes_per_word;
 		for (count = 0; count < max_n_32bit; count++) {
-			x = 0;
+			u32 x = 0;
 			for (i = 0; nbytes && (i < tspi->bytes_per_word);
 							i++, nbytes--)
-				x |= ((*tx_buf++) << i*8);
+				x |= (u32)(*tx_buf++) << (i * 8);
 			tegra_slink_writel(tspi, x, SLINK_TX_FIFO);
 		}
 	}
@@ -354,9 +345,8 @@ static unsigned int tegra_slink_read_rx_fifo_to_client_rxbuf(
 		struct tegra_slink_data *tspi, struct spi_transfer *t)
 {
 	unsigned rx_full_count;
-	unsigned long fifo_status;
+	u32 fifo_status;
 	unsigned i, count;
-	unsigned long x;
 	unsigned int read_words = 0;
 	unsigned len;
 	u8 *rx_buf = (u8 *)t->rx_buf + tspi->cur_rx_pos;
@@ -366,7 +356,7 @@ static unsigned int tegra_slink_read_rx_fifo_to_client_rxbuf(
 	if (tspi->is_packed) {
 		len = tspi->curr_dma_words * tspi->bytes_per_word;
 		for (count = 0; count < rx_full_count; count++) {
-			x = tegra_slink_readl(tspi, SLINK_RX_FIFO);
+			u32 x = tegra_slink_readl(tspi, SLINK_RX_FIFO);
 			for (i = 0; len && (i < 4); i++, len--)
 				*rx_buf++ = (x >> i*8) & 0xFF;
 		}
@@ -374,7 +364,7 @@ static unsigned int tegra_slink_read_rx_fifo_to_client_rxbuf(
 		read_words += tspi->curr_dma_words;
 	} else {
 		for (count = 0; count < rx_full_count; count++) {
-			x = tegra_slink_readl(tspi, SLINK_RX_FIFO);
+			u32 x = tegra_slink_readl(tspi, SLINK_RX_FIFO);
 			for (i = 0; (i < tspi->bytes_per_word); i++)
 				*rx_buf++ = (x >> (i*8)) & 0xFF;
 		}
@@ -387,27 +377,24 @@ static unsigned int tegra_slink_read_rx_fifo_to_client_rxbuf(
 static void tegra_slink_copy_client_txbuf_to_spi_txbuf(
 		struct tegra_slink_data *tspi, struct spi_transfer *t)
 {
-	unsigned len;
-
 	/* Make the dma buffer to read by cpu */
 	dma_sync_single_for_cpu(tspi->dev, tspi->tx_dma_phys,
 				tspi->dma_buf_size, DMA_TO_DEVICE);
 
 	if (tspi->is_packed) {
-		len = tspi->curr_dma_words * tspi->bytes_per_word;
+		unsigned len = tspi->curr_dma_words * tspi->bytes_per_word;
 		memcpy(tspi->tx_dma_buf, t->tx_buf + tspi->cur_pos, len);
 	} else {
 		unsigned int i;
 		unsigned int count;
 		u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_tx_pos;
 		unsigned consume = tspi->curr_dma_words * tspi->bytes_per_word;
-		unsigned int x;
 
 		for (count = 0; count < tspi->curr_dma_words; count++) {
-			x = 0;
+			u32 x = 0;
 			for (i = 0; consume && (i < tspi->bytes_per_word);
 							i++, consume--)
-				x |= ((*tx_buf++) << i * 8);
+				x |= (u32)(*tx_buf++) << (i * 8);
 			tspi->tx_dma_buf[count] = x;
 		}
 	}
@@ -434,14 +421,10 @@ static void tegra_slink_copy_spi_rxbuf_to_client_rxbuf(
 		unsigned int i;
 		unsigned int count;
 		unsigned char *rx_buf = t->rx_buf + tspi->cur_rx_pos;
-		unsigned int x;
-		unsigned int rx_mask, bits_per_word;
+		u32 rx_mask = ((u32)1 << t->bits_per_word) - 1;
 
-		bits_per_word = t->bits_per_word;
-		rx_mask = (1 << bits_per_word) - 1;
 		for (count = 0; count < tspi->curr_dma_words; count++) {
-			x = tspi->rx_dma_buf[count];
-			x &= rx_mask;
+			u32 x = tspi->rx_dma_buf[count] & rx_mask;
 			for (i = 0; (i < tspi->bytes_per_word); i++)
 				*rx_buf++ = (x >> (i*8)) & 0xFF;
 		}
@@ -501,17 +484,16 @@ static int tegra_slink_start_rx_dma(struct tegra_slink_data *tspi, int len)
 static int tegra_slink_start_dma_based_transfer(
 		struct tegra_slink_data *tspi, struct spi_transfer *t)
 {
-	unsigned long val;
-	unsigned long test_val;
+	u32 val;
 	unsigned int len;
 	int ret = 0;
-	unsigned long status;
+	u32 status;
 
 	/* Make sure that Rx and Tx fifo are empty */
 	status = tegra_slink_readl(tspi, SLINK_STATUS);
 	if ((status & SLINK_FIFO_EMPTY) != SLINK_FIFO_EMPTY) {
-		dev_err(tspi->dev,
-			"Rx/Tx fifo are not empty status 0x%08lx\n", status);
+		dev_err(tspi->dev, "Rx/Tx fifo are not empty status 0x%08x\n",
+			(unsigned)status);
 		return -EIO;
 	}
 
@@ -551,9 +533,9 @@ static int tegra_slink_start_dma_based_transfer(
 		}
 
 		/* Wait for tx fifo to be fill before starting slink */
-		test_val = tegra_slink_readl(tspi, SLINK_STATUS);
-		while (!(test_val & SLINK_TX_FULL))
-			test_val = tegra_slink_readl(tspi, SLINK_STATUS);
+		status = tegra_slink_readl(tspi, SLINK_STATUS);
+		while (!(status & SLINK_TX_FULL))
+			status = tegra_slink_readl(tspi, SLINK_STATUS);
 	}
 
 	if (tspi->cur_direction & DATA_DIR_RX) {
@@ -587,7 +569,7 @@ static int tegra_slink_start_dma_based_transfer(
 static int tegra_slink_start_cpu_based_transfer(
 		struct tegra_slink_data *tspi, struct spi_transfer *t)
 {
-	unsigned long val;
+	u32 val;
 	unsigned cur_words;
 
 	val = tspi->packed_size;
@@ -714,8 +696,8 @@ static int tegra_slink_start_transfer_one(struct spi_device *spi,
 	u8 bits_per_word;
 	unsigned total_fifo_words;
 	int ret;
-	unsigned long command;
-	unsigned long command2;
+	u32 command;
+	u32 command2;
 
 	bits_per_word = t->bits_per_word;
 	speed = t->speed_hz;
@@ -762,17 +744,18 @@ static int tegra_slink_start_transfer_one(struct spi_device *spi,
 
 static int tegra_slink_setup(struct spi_device *spi)
 {
-	struct tegra_slink_data *tspi = spi_master_get_devdata(spi->master);
-	unsigned long val;
-	unsigned long flags;
-	int ret;
-	unsigned int cs_pol_bit[MAX_CHIP_SELECT] = {
+	static const u32 cs_pol_bit[MAX_CHIP_SELECT] = {
 			SLINK_CS_POLARITY,
 			SLINK_CS_POLARITY1,
 			SLINK_CS_POLARITY2,
 			SLINK_CS_POLARITY3,
 	};
 
+	struct tegra_slink_data *tspi = spi_master_get_devdata(spi->master);
+	u32 val;
+	unsigned long flags;
+	int ret;
+
 	dev_dbg(&spi->dev, "setup %d bpw, %scpol, %scpha, %dHz\n",
 		spi->bits_per_word,
 		spi->mode & SPI_CPOL ? "" : "~",
-- 
1.8.4

[PATCH 3/3] spi: tegra20-sflash: use u32 for 32-bit register values

[Michal Nazarewicz]

Previously used “unsigned long” may lead to confusion should the code
be compiled for 64-bit machine.

Signed-off-by: Michal Nazarewicz <mina86-deATy8a+UHjQT0dZR+AlfA@public.gmane.org>
---
 drivers/spi/spi-tegra20-sflash.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-tegra20-sflash.c b/drivers/spi/spi-tegra20-sflash.c
index 4dc8e81..ce9589c 100644
--- a/drivers/spi/spi-tegra20-sflash.c
+++ b/drivers/spi/spi-tegra20-sflash.c
@@ -148,14 +148,14 @@ struct tegra_sflash_data {
 static int tegra_sflash_runtime_suspend(struct device *dev);
 static int tegra_sflash_runtime_resume(struct device *dev);
 
-static inline unsigned long tegra_sflash_readl(struct tegra_sflash_data *tsd,
+static inline u32 tegra_sflash_readl(struct tegra_sflash_data *tsd,
 		unsigned long reg)
 {
 	return readl(tsd->base + reg);
 }
 
 static inline void tegra_sflash_writel(struct tegra_sflash_data *tsd,
-		unsigned long val, unsigned long reg)
+		u32 val, unsigned long reg)
 {
 	writel(val, tsd->base + reg);
 }
@@ -185,7 +185,7 @@ static unsigned tegra_sflash_fill_tx_fifo_from_client_txbuf(
 	struct tegra_sflash_data *tsd, struct spi_transfer *t)
 {
 	unsigned nbytes;
-	unsigned long status;
+	u32 status;
 	unsigned max_n_32bit = tsd->curr_xfer_words;
 	u8 *tx_buf = (u8 *)t->tx_buf + tsd->cur_tx_pos;
 
@@ -196,11 +196,11 @@ static unsigned tegra_sflash_fill_tx_fifo_from_client_txbuf(
 	status = tegra_sflash_readl(tsd, SPI_STATUS);
 	while (!(status & SPI_TXF_FULL)) {
 		int i;
-		unsigned int x = 0;
+		u32 x = 0;
 
 		for (i = 0; nbytes && (i < tsd->bytes_per_word);
 							i++, nbytes--)
-				x |= ((*tx_buf++) << i*8);
+			x |= (u32)(*tx_buf++) << (i * 8);
 		tegra_sflash_writel(tsd, x, SPI_TX_FIFO);
 		if (!nbytes)
 			break;
@@ -214,16 +214,14 @@ static unsigned tegra_sflash_fill_tx_fifo_from_client_txbuf(
 static int tegra_sflash_read_rx_fifo_to_client_rxbuf(
 		struct tegra_sflash_data *tsd, struct spi_transfer *t)
 {
-	unsigned long status;
+	u32 status;
 	unsigned int read_words = 0;
 	u8 *rx_buf = (u8 *)t->rx_buf + tsd->cur_rx_pos;
 
 	status = tegra_sflash_readl(tsd, SPI_STATUS);
 	while (!(status & SPI_RXF_EMPTY)) {
 		int i;
-		unsigned long x;
-
-		x = tegra_sflash_readl(tsd, SPI_RX_FIFO);
+		u32 x = tegra_sflash_readl(tsd, SPI_RX_FIFO);
 		for (i = 0; (i < tsd->bytes_per_word); i++)
 			*rx_buf++ = (x >> (i*8)) & 0xFF;
 		read_words++;
@@ -236,7 +234,7 @@ static int tegra_sflash_read_rx_fifo_to_client_rxbuf(
 static int tegra_sflash_start_cpu_based_transfer(
 		struct tegra_sflash_data *tsd, struct spi_transfer *t)
 {
-	unsigned long val = 0;
+	u32 val = 0;
 	unsigned cur_words;
 
 	if (tsd->cur_direction & DATA_DIR_TX)
@@ -266,7 +264,7 @@ static int tegra_sflash_start_transfer_one(struct spi_device *spi,
 {
 	struct tegra_sflash_data *tsd = spi_master_get_devdata(spi->master);
 	u32 speed;
-	unsigned long command;
+	u32 command;
 
 	speed = t->speed_hz;
 	if (speed != tsd->cur_speed) {
@@ -313,7 +311,7 @@ static int tegra_sflash_start_transfer_one(struct spi_device *spi,
 	tegra_sflash_writel(tsd, command, SPI_COMMAND);
 	tsd->command_reg = command;
 
-	return  tegra_sflash_start_cpu_based_transfer(tsd, t);
+	return tegra_sflash_start_cpu_based_transfer(tsd, t);
 }
 
 static int tegra_sflash_setup(struct spi_device *spi)
-- 
1.8.4

Re: [PATCH 0/3] spi: tegra: use u32 for 32-bit register values

[Stephen Warren]

On 12/08/2013 08:35 AM, Michal Nazarewicz wrote:
> This series has only been compile-tested, but it does not introduce any
> behaviour changes.

The series,
Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Acked-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

... although I will observe that the patch subjects, and even patch
descriptions, don't describe all the other unrelated code
transformations that are rolled into these patches...

Re: [PATCH 0/3] spi: tegra: use u32 for 32-bit register values

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 259 bytes --]

On Sun, Dec 08, 2013 at 04:35:08PM +0100, Michal Nazarewicz wrote:
> This series has only been compile-tested, but it does not introduce any
> behaviour changes.

Applied, thanks.  Like Stephen says please do make sure your commits do
as they're described...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]