diff mbox series

[v7,27/34] i2c: tegra: Check errors for both positive and negative values

Message ID 20200908224006.25636-28-digetx@gmail.com
State New
Headers show
Series Improvements for Tegra I2C driver | expand

Checks

Context Check Description
tagr/GVS success 1122533
tagr/GVS pending 1122533
tagr/GVS-1122491 fail None
tagr/GVS-1122491 pending None

Commit Message

Dmitry Osipenko Sept. 8, 2020, 10:39 p.m. UTC
The driver's code is inconsistent in regards to the error values checking.
The correct way should be to check both positive and negative values.
This patch cleans up the error-checks in the code. Note that the
pm_runtime_get_sync() could return positive value on success, hence only
relevant parts of the code are changed by this patch.

Reviewed-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Thierry Reding Sept. 17, 2020, 12:09 p.m. UTC | #1
On Wed, Sep 09, 2020 at 01:39:59AM +0300, Dmitry Osipenko wrote:
> The driver's code is inconsistent in regards to the error values checking.
> The correct way should be to check both positive and negative values.
> This patch cleans up the error-checks in the code. Note that the
> pm_runtime_get_sync() could return positive value on success, hence only
> relevant parts of the code are changed by this patch.
> 
> Reviewed-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Why? All of these functions "return 0 on success or a negative error
code on failure", don't they?

I would actually argue that all of the other checks are wrong. As you
mention yourself, some functions may decide to return positive values on
success to convey some extra information, so making this just check for
non-zero carries a risk of its own.

Thierry
Andy Shevchenko Sept. 17, 2020, 1:50 p.m. UTC | #2
On Thu, Sep 17, 2020 at 3:09 PM Thierry Reding <thierry.reding@gmail.com> wrote:
> On Wed, Sep 09, 2020 at 01:39:59AM +0300, Dmitry Osipenko wrote:

> Why? All of these functions "return 0 on success or a negative error
> code on failure", don't they?

And what is the point of having ' < 0' in all those cases?
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 2f39366b6d55..fe672cfebe12 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -981,7 +981,7 @@  static int tegra_i2c_config_fifo_trig(struct tegra_i2c_dev *i2c_dev, size_t len)
 
 		slv_config.device_fc = true;
 		ret = dmaengine_slave_config(chan, &slv_config);
-		if (ret < 0) {
+		if (ret) {
 			dev_err(i2c_dev->dev, "DMA slave config failed: %d\n",
 				ret);
 			return ret;
@@ -1224,7 +1224,7 @@  static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 						   xfer_size,
 						   DMA_FROM_DEVICE);
 			err = tegra_i2c_dma_submit(i2c_dev, xfer_size);
-			if (err < 0) {
+			if (err) {
 				dev_err(i2c_dev->dev,
 					"starting RX DMA failed, err %d\n",
 					err);
@@ -1249,7 +1249,7 @@  static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 						   xfer_size,
 						   DMA_TO_DEVICE);
 			err = tegra_i2c_dma_submit(i2c_dev, xfer_size);
-			if (err < 0) {
+			if (err) {
 				dev_err(i2c_dev->dev,
 					"starting TX DMA failed, err %d\n",
 					err);