diff mbox

[1/3] spi: Re-do the returning value of rspi_dma_check_then_transfer

Message ID 1430200551-20708-2-git-send-email-cm-hiep@jinso.co.jp (mailing list archive)
State New, archived
Headers show

Commit Message

Cao Minh Hiep April 28, 2015, 5:55 a.m. UTC
From: Hiep Cao Minh <cm-hiep@jinso.co.jp>

To reduce indentation and complex of code, insteeds of returning zero,
the function rspi_dma_check_then_transfer should be returned
rspi_dma_transfer directly after checking error.

Signed-off-by: Hiep Cao Minh <cm-hiep@jinso.co.jp>
---
 drivers/spi/spi-rspi.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

Comments

Geert Uytterhoeven April 28, 2015, 9:02 a.m. UTC | #1
Hi Hiep-san,

On Tue, Apr 28, 2015 at 7:55 AM, Cao Minh Hiep <cm-hiep@jinso.co.jp> wrote:
> To reduce indentation and complex of code, insteeds of returning zero,
> the function rspi_dma_check_then_transfer should be returned
> rspi_dma_transfer directly after checking error.

Thanks for the update!

Unfortunately it doesn't compile...

> --- a/drivers/spi/spi-rspi.c
> +++ b/drivers/spi/spi-rspi.c
> @@ -665,15 +665,12 @@ static bool rspi_can_dma(struct spi_master *master, struct spi_device *spi,
>  static int rspi_dma_check_then_transfer(struct rspi_data *rspi,
>                                          struct spi_transfer *xfer)
>  {
> -       if (rspi->master->can_dma && __rspi_can_dma(rspi, xfer)) {
> -               /* rx_buf can be NULL on RSPI on SH in TX-only Mode */
> -               int ret = rspi_dma_transfer(rspi, &xfer->tx_sg,
> -                                       xfer->rx_buf ? &xfer->rx_sg : NULL);
> -               if (ret != -EAGAIN)
> -                       return 0;
> -       }
> +       if (!rspi->master->can_dma || !__rspi_can_dma(rspi, xfer)) {

Superfluous opening curly brace.
Please compile and run test your patches.
Thanks!

> +               return -EAGAIN;
>
> -       return -EAGAIN;
> +       /* rx_buf can be NULL on RSPI on SH in TX-only Mode */
> +       return rspi_dma_transfer(rspi, &xfer->tx_sg,
> +                                       xfer->rx_buf ? &xfer->rx_sg : NULL);
>  }
>
>  static int rspi_common_transfer(struct rspi_data *rspi,
Sergei Shtylyov April 28, 2015, 11:45 a.m. UTC | #2
Hello.

On 4/28/2015 8:55 AM, Cao Minh Hiep wrote:

> From: Hiep Cao Minh <cm-hiep@jinso.co.jp>

> To reduce indentation and complex of code, insteeds of returning zero,

    Complexity?

> the function rspi_dma_check_then_transfer should be returned

    Returning? Or perhaps just "return", without "be"?

> rspi_dma_transfer directly after checking error.

> Signed-off-by: Hiep Cao Minh <cm-hiep@jinso.co.jp>
> ---
>   drivers/spi/spi-rspi.c | 13 +++++--------
>   1 file changed, 5 insertions(+), 8 deletions(-)

[...]

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cao Minh Hiep April 30, 2015, 12:26 a.m. UTC | #3
Hi Geert-san,

On 2015?04?28? 18:02, Geert Uytterhoeven wrote:
> Hi Hiep-san,
>
> On Tue, Apr 28, 2015 at 7:55 AM, Cao Minh Hiep <cm-hiep@jinso.co.jp> wrote:
>> To reduce indentation and complex of code, insteeds of returning zero,
>> the function rspi_dma_check_then_transfer should be returned
>> rspi_dma_transfer directly after checking error.
> Thanks for the update!
>
> Unfortunately it doesn't compile...
Sorry to bother you again.
>> --- a/drivers/spi/spi-rspi.c
>> +++ b/drivers/spi/spi-rspi.c
>> @@ -665,15 +665,12 @@ static bool rspi_can_dma(struct spi_master *master, struct spi_device *spi,
>>   static int rspi_dma_check_then_transfer(struct rspi_data *rspi,
>>                                           struct spi_transfer *xfer)
>>   {
>> -       if (rspi->master->can_dma && __rspi_can_dma(rspi, xfer)) {
>> -               /* rx_buf can be NULL on RSPI on SH in TX-only Mode */
>> -               int ret = rspi_dma_transfer(rspi, &xfer->tx_sg,
>> -                                       xfer->rx_buf ? &xfer->rx_sg : NULL);
>> -               if (ret != -EAGAIN)
>> -                       return 0;
>> -       }
>> +       if (!rspi->master->can_dma || !__rspi_can_dma(rspi, xfer)) {
> Superfluous opening curly brace.
> Please compile and run test your patches.
> Thanks!

Thanks, I'll fix it and send you at a moment.

>> +               return -EAGAIN;
>>
>> -       return -EAGAIN;
>> +       /* rx_buf can be NULL on RSPI on SH in TX-only Mode */
>> +       return rspi_dma_transfer(rspi, &xfer->tx_sg,
>> +                                       xfer->rx_buf ? &xfer->rx_sg : NULL);
>>   }
>>
>>   static int rspi_common_transfer(struct rspi_data *rspi,
>
>
>

Hiep.

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cao Minh Hiep April 30, 2015, 12:33 a.m. UTC | #4
Hi Sergei-san,

>> To reduce indentation and complex of code, insteeds of returning zero,
>
>    Complexity?
>

>> the function rspi_dma_check_then_transfer should be returned
>
>    Returning? Or perhaps just "return", without "be"?
>

Thanks for pointing this out. I'll update it.


Hiep.

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

Patch

diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
index 186924a..a17564a 100644
--- a/drivers/spi/spi-rspi.c
+++ b/drivers/spi/spi-rspi.c
@@ -665,15 +665,12 @@  static bool rspi_can_dma(struct spi_master *master, struct spi_device *spi,
 static int rspi_dma_check_then_transfer(struct rspi_data *rspi,
 					 struct spi_transfer *xfer)
 {
-	if (rspi->master->can_dma && __rspi_can_dma(rspi, xfer)) {
-		/* rx_buf can be NULL on RSPI on SH in TX-only Mode */
-		int ret = rspi_dma_transfer(rspi, &xfer->tx_sg,
-					xfer->rx_buf ? &xfer->rx_sg : NULL);
-		if (ret != -EAGAIN)
-			return 0;
-	}
+	if (!rspi->master->can_dma || !__rspi_can_dma(rspi, xfer)) {
+		return -EAGAIN;
 
-	return -EAGAIN;
+	/* rx_buf can be NULL on RSPI on SH in TX-only Mode */
+	return rspi_dma_transfer(rspi, &xfer->tx_sg,
+					xfer->rx_buf ? &xfer->rx_sg : NULL);
 }
 
 static int rspi_common_transfer(struct rspi_data *rspi,