2015-02-02 08:34:47

by Nicholas Mc Guire

[permalink] [raw]
Subject: [PATCH 1/5] spi: match var type to return type of wait_for_completion

From: Nicholas Mc Guire <[email protected]>

return type of wait_for_completion_timeout is unsigned long not int, this
patch changes the type of m from int to unsigned long.

Signed-off-by: Nicholas Mc Guire <[email protected]>
---

The return type of wait_for_completion_timeout is unsigned long not
int. This patch resolves the type missmatch by changing the type
to unsigned long.

This patch was only compile tested with x86_64_defconfig + CONFIG_SPI=y

Patch is against 3.19.0-rc6 -next-20150130

drivers/spi/spi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b9fb711..c64a3e5 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -784,7 +784,7 @@ static int spi_transfer_one_message(struct spi_master *master,
struct spi_transfer *xfer;
bool keep_cs = false;
int ret = 0;
- int ms = 1;
+ unsigned long ms = 1;

spi_set_cs(msg->spi, true);

--
1.7.10.4


2015-02-02 08:34:52

by Nicholas Mc Guire

[permalink] [raw]
Subject: [PATCH 2/5] spi: mxs: cleanup wait_for_completion return handling

return type of wait_for_completion_timeout is unsigned long not int, this
patch uses the return value of wait_for_completion_timeout in the condition
directly rather than adding a additional appropriately typed variable.

Signed-off-by: Nicholas Mc Guire <[email protected]>
---

The return type of wait_for_completion_timeout is unsigned long not
int. This patch resolves the type mismatch by moving the call to
wait_for_completion_timeout into the condition.

This patch was only compile tested with mxs_defconfig

Patch is against 3.19.0-rc6 -next-20150130

drivers/spi/spi-mxs.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-mxs.c b/drivers/spi/spi-mxs.c
index 4045a1e..d051312 100644
--- a/drivers/spi/spi-mxs.c
+++ b/drivers/spi/spi-mxs.c
@@ -282,9 +282,8 @@ static int mxs_spi_txrx_dma(struct mxs_spi *spi,
dmaengine_submit(desc);
dma_async_issue_pending(ssp->dmach);

- ret = wait_for_completion_timeout(&spi->c,
- msecs_to_jiffies(SSP_TIMEOUT));
- if (!ret) {
+ if (!wait_for_completion_timeout(&spi->c,
+ msecs_to_jiffies(SSP_TIMEOUT))) {
dev_err(ssp->dev, "DMA transfer timeout\n");
ret = -ETIMEDOUT;
dmaengine_terminate_all(ssp->dmach);
--
1.7.10.4

2015-02-02 08:34:56

by Nicholas Mc Guire

[permalink] [raw]
Subject: [PATCH 3/5] spi: sh-msiof: cleanup wait_for_completion return handling

From: Nicholas Mc Guire <[email protected]>

return type of wait_for_completion_timeout is unsigned long not int, this
patch uses the return value of wait_for_completion_timeout in the condition
directly rather than assigning it to an incorrect type variable.

Signed-off-by: Nicholas Mc Guire <[email protected]>
---

The return type of wait_for_completion_timeout is unsigned long not
int. This patch resolves the type missmatch by moving the call to
wait_for_completion_timeout into the condition.

This patch was only compile tested with: polaris_defconfig
CONFIG_SPI=y, CONFIG_SPI_SH_MSIOF=m

Patch is against 3.19.0-rc6 -next-20150130

drivers/spi/spi-sh-msiof.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 9a884e6..9409873 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -640,8 +640,7 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
}

/* wait for tx fifo to be emptied / rx fifo to be filled */
- ret = wait_for_completion_timeout(&p->done, HZ);
- if (!ret) {
+ if (!wait_for_completion_timeout(&p->done, HZ)) {
dev_err(&p->pdev->dev, "PIO timeout\n");
ret = -ETIMEDOUT;
goto stop_reset;
@@ -751,8 +750,7 @@ static int sh_msiof_dma_once(struct sh_msiof_spi_priv *p, const void *tx,
}

/* wait for tx fifo to be emptied / rx fifo to be filled */
- ret = wait_for_completion_timeout(&p->done, HZ);
- if (!ret) {
+ if (!wait_for_completion_timeout(&p->done, HZ)) {
dev_err(&p->pdev->dev, "DMA timeout\n");
ret = -ETIMEDOUT;
goto stop_reset;
--
1.7.10.4

2015-02-02 08:35:00

by Nicholas Mc Guire

[permalink] [raw]
Subject: [PATCH 4/5] spi: spi-imx: cleanup wait_for_completion handling

return type of wait_for_completion_timeout is unsigned long not int and
always returns >=0 , this patch adds a suitable return variable and
simplifies the return value checking as there is no < 0 case.

Signed-off-by: Nicholas Mc Guire <[email protected]>
---

The return type of wait_for_completion_timeout is unsigned long not
int. This patch resolves the type missmatch by adding an appropriately
type return variable as well as simplifying the return value checking.
If the return was not 0 it only can be positive so the > 0 check is
unnecessary.

This patch was only compile tested with imx_v6_v7_defconfig
(implies CONFIG_SPI_IMX=y)

Patch is against 3.19.0-rc6 -next-20150130

drivers/spi/spi-imx.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 6a2ff75..5eaecbb 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -891,6 +891,7 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx,
{
struct dma_async_tx_descriptor *desc_tx = NULL, *desc_rx = NULL;
int ret;
+ unsigned long timeout;
u32 dma;
int left;
struct spi_master *master = spi_imx->bitbang.master;
@@ -938,17 +939,17 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx,
dma_async_issue_pending(master->dma_tx);
dma_async_issue_pending(master->dma_rx);
/* Wait SDMA to finish the data transfer.*/
- ret = wait_for_completion_timeout(&spi_imx->dma_tx_completion,
+ timeout = wait_for_completion_timeout(&spi_imx->dma_tx_completion,
IMX_DMA_TIMEOUT);
- if (!ret) {
+ if (!timeout) {
pr_warn("%s %s: I/O Error in DMA TX\n",
dev_driver_string(&master->dev),
dev_name(&master->dev));
dmaengine_terminate_all(master->dma_tx);
} else {
- ret = wait_for_completion_timeout(&spi_imx->dma_rx_completion,
- IMX_DMA_TIMEOUT);
- if (!ret) {
+ timeout = wait_for_completion_timeout(
+ &spi_imx->dma_rx_completion, IMX_DMA_TIMEOUT);
+ if (!timeout) {
pr_warn("%s %s: I/O Error in DMA RX\n",
dev_driver_string(&master->dev),
dev_name(&master->dev));
@@ -963,9 +964,9 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx,
spi_imx->dma_finished = 1;
spi_imx->devtype_data->trigger(spi_imx);

- if (!ret)
+ if (!timeout)
ret = -ETIMEDOUT;
- else if (ret > 0)
+ else
ret = transfer->len;

return ret;
--
1.7.10.4

2015-02-02 08:35:05

by Nicholas Mc Guire

[permalink] [raw]
Subject: [PATCH 5/5] spi: ti-qspi: cleanup wait_for_completion return handling

return type of wait_for_completion_timeout is unsigned long not int, this
patch uses the return value of wait_for_completion_timeout in the condition
directly rather than assigning it to an incorrect type variable.

Signed-off-by: Nicholas Mc Guire <[email protected]>
---

The return type of wait_for_completion_timeout is unsigned long not
int. This patch resolves the type mismatch by moving the call to
wait_for_completion_timeout into the condition.

This patch was only compile tested with omap2plus_defconfig

Patch is against 3.19.0-rc6 -next-20150130

drivers/spi/spi-ti-qspi.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
index 6146c4c..3364e3e 100644
--- a/drivers/spi/spi-ti-qspi.c
+++ b/drivers/spi/spi-ti-qspi.c
@@ -201,7 +201,7 @@ static void ti_qspi_restore_ctx(struct ti_qspi *qspi)

static int qspi_write_msg(struct ti_qspi *qspi, struct spi_transfer *t)
{
- int wlen, count, ret;
+ int wlen, count;
unsigned int cmd;
const u8 *txbuf;

@@ -230,9 +230,8 @@ static int qspi_write_msg(struct ti_qspi *qspi, struct spi_transfer *t)
}

ti_qspi_write(qspi, cmd, QSPI_SPI_CMD_REG);
- ret = wait_for_completion_timeout(&qspi->transfer_complete,
- QSPI_COMPLETION_TIMEOUT);
- if (ret == 0) {
+ if (!wait_for_completion_timeout(&qspi->transfer_complete,
+ QSPI_COMPLETION_TIMEOUT)) {
dev_err(qspi->dev, "write timed out\n");
return -ETIMEDOUT;
}
@@ -245,7 +244,7 @@ static int qspi_write_msg(struct ti_qspi *qspi, struct spi_transfer *t)

static int qspi_read_msg(struct ti_qspi *qspi, struct spi_transfer *t)
{
- int wlen, count, ret;
+ int wlen, count;
unsigned int cmd;
u8 *rxbuf;

@@ -268,9 +267,8 @@ static int qspi_read_msg(struct ti_qspi *qspi, struct spi_transfer *t)
while (count) {
dev_dbg(qspi->dev, "rx cmd %08x dc %08x\n", cmd, qspi->dc);
ti_qspi_write(qspi, cmd, QSPI_SPI_CMD_REG);
- ret = wait_for_completion_timeout(&qspi->transfer_complete,
- QSPI_COMPLETION_TIMEOUT);
- if (ret == 0) {
+ if (!wait_for_completion_timeout(&qspi->transfer_complete,
+ QSPI_COMPLETION_TIMEOUT)) {
dev_err(qspi->dev, "read timed out\n");
return -ETIMEDOUT;
}
--
1.7.10.4

2015-02-02 12:11:43

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/5] spi: mxs: cleanup wait_for_completion return handling

On Mon, Feb 02, 2015 at 03:30:33AM -0500, Nicholas Mc Guire wrote:

> - ret = wait_for_completion_timeout(&spi->c,
> - msecs_to_jiffies(SSP_TIMEOUT));
> - if (!ret) {
> + if (!wait_for_completion_timeout(&spi->c,
> + msecs_to_jiffies(SSP_TIMEOUT))) {

Your new indentation is pretty confusing here, I had to look twice to
realize that the msecs_to_jiffies() is still an argument to
wait_for_completion(). Generally arguments to functions should be
aligned with the opening ( for the function, or at least much more
deeply aligned than the function itself (as was the case with the
original code).


Attachments:
(No filename) (605.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-02-02 12:16:08

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 5/5] spi: ti-qspi: cleanup wait_for_completion return handling

On Mon, Feb 02, 2015 at 03:30:36AM -0500, Nicholas Mc Guire wrote:

> - ret = wait_for_completion_timeout(&qspi->transfer_complete,
> - QSPI_COMPLETION_TIMEOUT);
> - if (ret == 0) {
> + if (!wait_for_completion_timeout(&qspi->transfer_complete,
> + QSPI_COMPLETION_TIMEOUT)) {

Same indentation issue as with the earlier patch here. I also just
noticed that you're not CCing driver maintainers, please try to do that
so they get a chance to review things.


Attachments:
(No filename) (472.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-02-04 20:52:51

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/5] spi: match var type to return type of wait_for_completion

On Mon, Feb 02, 2015 at 03:30:32AM -0500, Nicholas Mc Guire wrote:
> From: Nicholas Mc Guire <[email protected]>
>
> return type of wait_for_completion_timeout is unsigned long not int, this
> patch changes the type of m from int to unsigned long.

applied, thanks.


Attachments:
(No filename) (266.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-02-04 20:53:11

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 3/5] spi: sh-msiof: cleanup wait_for_completion return handling

On Mon, Feb 02, 2015 at 03:30:34AM -0500, Nicholas Mc Guire wrote:
> From: Nicholas Mc Guire <[email protected]>
>
> return type of wait_for_completion_timeout is unsigned long not int, this
> patch uses the return value of wait_for_completion_timeout in the condition
> directly rather than assigning it to an incorrect type variable.

Applied, thanks.


Attachments:
(No filename) (355.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-02-04 20:53:34

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 4/5] spi: spi-imx: cleanup wait_for_completion handling

On Mon, Feb 02, 2015 at 03:30:35AM -0500, Nicholas Mc Guire wrote:
> return type of wait_for_completion_timeout is unsigned long not int and
> always returns >=0 , this patch adds a suitable return variable and
> simplifies the return value checking as there is no < 0 case.

Applied, thanks.


Attachments:
(No filename) (293.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments