On 10/28/18 1:43 PM, Jonathan Cameron wrote:
On Fri, 26 Oct 2018 23:00:01 -0300
Matheus Tavares <matheus.bernardino@xxxxxx> wrote:
Previously, ad2s90_probe ignored the return code from spi_setup, not
handling its possible failure. This patch makes ad2s90_probe check if
the code is an error code and, if so, do the following:
- Call dev_err with an appropriate error message.
- Return the spi_setup's error code, aborting the execution of the
rest of the function.
Signed-off-by: Matheus Tavares <matheus.bernardino@xxxxxx>
---
drivers/staging/iio/resolver/ad2s90.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c b/drivers/staging/iio/resolver/ad2s90.c
index 11fac9f90148..d6a42e3f1bd8 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -88,7 +88,12 @@ static int ad2s90_probe(struct spi_device *spi)
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 830000;
spi->mode = SPI_MODE_3;
- spi_setup(spi);
+ ret = spi_setup(spi);
+
+ if (ret < 0) {
+ dev_err(&spi->dev, "spi_setup failed!\n");
+ return ret;
+ }
I would have reordered this first to be before the iio_device_register call.
The reason being that it would avoid this comment.
Drop the return ret out of the block above and return ret unconditionally.
I don't mind too much as I know this is moving later, but I only know that
because of the earlier discussion ;) Few reviewers read the whole patch
set before responding to the early patches - it's just too much like hard
work. So if you can do things in an order that minimizes standard responses
then that's great.
Patch is fine though - could be solved by a comment in the intro that
says the code in question will move in patch X.
Ok! As a newcomer I'm not sure about the right way to add this comment.
Should I add it to the patch message or after the --- ? Also, how do I
reference patch X? Just giving the number of the patch in this series?
An alternative to adding the comment would be to drop the return ret,
and reinsert it in the next patch. Dosen't seem so good, to me, but
would avoid the problem you presented. What would be better?
Matheus.
Jonathan
return 0;
}
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
