The tvp5150_write() function calls i2c_smbus_write_byte_data() that
can fail but does not propagate the error to the caller. Instead it
just prints a debug, so callers can't know if the operation failed.
So change the function to return the error code to the caller so it
knows that the write failed and also print an error instead of just
printing a debug information.
While being there remove the inline keyword from tvp5150_write() to
make it consistent with tvp5150_read() and also because it's called
in a lot of places, so making inline is in fact counter productive
since it makes the kernel image size to be much bigger (~16 KiB).
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/media/i2c/tvp5150.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index e5003d94f262..4a2e851b6a3b 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -83,7 +83,7 @@ static int tvp5150_read(struct v4l2_subdev *sd, unsigned char addr)
return rc;
}
-static inline void tvp5150_write(struct v4l2_subdev *sd, unsigned char addr,
+static int tvp5150_write(struct v4l2_subdev *sd, unsigned char addr,
unsigned char value)
{
struct i2c_client *c = v4l2_get_subdevdata(sd);
@@ -92,7 +92,9 @@ static inline void tvp5150_write(struct v4l2_subdev *sd, unsigned char addr,
v4l2_dbg(2, debug, sd, "tvp5150: writing 0x%02x 0x%02x\n", addr, value);
rc = i2c_smbus_write_byte_data(c, addr, value);
if (rc < 0)
- v4l2_dbg(0, debug, sd, "i2c i/o error: rc == %d\n", rc);
+ v4l2_err(sd, "i2c i/o error: rc == %d\n", rc);
+
+ return rc;
}
static void dump_reg_range(struct v4l2_subdev *sd, char *s, u8 init,
--
2.5.5
The tvp5150_write() function can fail so don't return 0 unconditionally
in tvp5150_s_register() but propagate what's returned by tvp5150_write().
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/media/i2c/tvp5150.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 4a2e851b6a3b..7be456d1b071 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -1161,8 +1161,7 @@ static int tvp5150_g_register(struct v4l2_subdev *sd, struct v4l2_dbg_register *
static int tvp5150_s_register(struct v4l2_subdev *sd, const struct v4l2_dbg_register *reg)
{
- tvp5150_write(sd, reg->reg & 0xff, reg->val & 0xff);
- return 0;
+ return tvp5150_write(sd, reg->reg & 0xff, reg->val & 0xff);
}
#endif
--
2.5.5
On Fri, Apr 15, 2016 at 2:00 AM, Javier Martinez Canillas
<[email protected]> wrote:
> The tvp5150_write() function calls i2c_smbus_write_byte_data() that
> can fail but does not propagate the error to the caller. Instead it
> just prints a debug, so callers can't know if the operation failed.
>
> So change the function to return the error code to the caller so it
> knows that the write failed and also print an error instead of just
> printing a debug information.
>
> While being there remove the inline keyword from tvp5150_write() to
> make it consistent with tvp5150_read() and also because it's called
> in a lot of places, so making inline is in fact counter productive
> since it makes the kernel image size to be much bigger (~16 KiB).
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
Acked-by: Lad, Prabhakar <[email protected]>
Cheers,
--Prabhakar Lad
> ---
>
> drivers/media/i2c/tvp5150.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index e5003d94f262..4a2e851b6a3b 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -83,7 +83,7 @@ static int tvp5150_read(struct v4l2_subdev *sd, unsigned char addr)
> return rc;
> }
>
> -static inline void tvp5150_write(struct v4l2_subdev *sd, unsigned char addr,
> +static int tvp5150_write(struct v4l2_subdev *sd, unsigned char addr,
> unsigned char value)
> {
> struct i2c_client *c = v4l2_get_subdevdata(sd);
> @@ -92,7 +92,9 @@ static inline void tvp5150_write(struct v4l2_subdev *sd, unsigned char addr,
> v4l2_dbg(2, debug, sd, "tvp5150: writing 0x%02x 0x%02x\n", addr, value);
> rc = i2c_smbus_write_byte_data(c, addr, value);
> if (rc < 0)
> - v4l2_dbg(0, debug, sd, "i2c i/o error: rc == %d\n", rc);
> + v4l2_err(sd, "i2c i/o error: rc == %d\n", rc);
> +
> + return rc;
> }
>
> static void dump_reg_range(struct v4l2_subdev *sd, char *s, u8 init,
> --
> 2.5.5
>
On Fri, Apr 15, 2016 at 2:00 AM, Javier Martinez Canillas
<[email protected]> wrote:
> The tvp5150_write() function can fail so don't return 0 unconditionally
> in tvp5150_s_register() but propagate what's returned by tvp5150_write().
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
>
Acked-by: Lad, Prabhakar <[email protected]>
Cheers,
--Prabhakar Lad
> ---
>
> drivers/media/i2c/tvp5150.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 4a2e851b6a3b..7be456d1b071 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -1161,8 +1161,7 @@ static int tvp5150_g_register(struct v4l2_subdev *sd, struct v4l2_dbg_register *
>
> static int tvp5150_s_register(struct v4l2_subdev *sd, const struct v4l2_dbg_register *reg)
> {
> - tvp5150_write(sd, reg->reg & 0xff, reg->val & 0xff);
> - return 0;
> + return tvp5150_write(sd, reg->reg & 0xff, reg->val & 0xff);
> }
> #endif
>
> --
> 2.5.5
>