| Message ID | 928f2f70de241d0fa66801b46d736ad0f881eb72.1681576102.git.christophe.jaillet@wanadoo.fr |
|---|---|
| State | Accepted |
| Commit | 1e82d01b88eda8c4907e5901b9f5a7ff39970320 |
| Headers | show |
| Series | media: ov5693: Simplify an error message | expand |
Hi Christophe On 15/04/2023 17:28, Christophe JAILLET wrote: > dev_err_probe() already display the error code. There is no need to > duplicate it explicitly in the error message. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- Thanks for the patch: Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > drivers/media/i2c/ov5693.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c > index e3c3bed69ad6..d23786afd754 100644 > --- a/drivers/media/i2c/ov5693.c > +++ b/drivers/media/i2c/ov5693.c > @@ -404,8 +404,8 @@ static int ov5693_read_reg(struct ov5693_device *ov5693, u32 addr, u32 *value) > ret = i2c_transfer(client->adapter, msg, 2); > if (ret < 0) > return dev_err_probe(&client->dev, ret, > - "Failed to read register 0x%04x: %d\n", > - addr & OV5693_REG_ADDR_MASK, ret); > + "Failed to read register 0x%04x\n", > + addr & OV5693_REG_ADDR_MASK); > > *value = 0; > for (i = 0; i < len; ++i) {
Am 15.04.23 um 18:28 schrieb Christophe JAILLET: > dev_err_probe() already display the error code. There is no need to > duplicate it explicitly in the error message. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > drivers/media/i2c/ov5693.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c > index e3c3bed69ad6..d23786afd754 100644 > --- a/drivers/media/i2c/ov5693.c > +++ b/drivers/media/i2c/ov5693.c > @@ -404,8 +404,8 @@ static int ov5693_read_reg(struct ov5693_device *ov5693, u32 addr, u32 *value) > ret = i2c_transfer(client->adapter, msg, 2); > if (ret < 0) i2c_transfer returns the number of transmitted messages. So I think the values 0 <= ret < 2 also need to be handled. > return dev_err_probe(&client->dev, ret, > - "Failed to read register 0x%04x: %d\n", > - addr & OV5693_REG_ADDR_MASK, ret); > + "Failed to read register 0x%04x\n", > + addr & OV5693_REG_ADDR_MASK); > > *value = 0; > for (i = 0; i < len; ++i) {
Le 21/04/2023 à 09:38, Matthias Schwarzott a écrit : > Am 15.04.23 um 18:28 schrieb Christophe JAILLET: >> dev_err_probe() already display the error code. There is no need to >> duplicate it explicitly in the error message. >> >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >> --- >> drivers/media/i2c/ov5693.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c >> index e3c3bed69ad6..d23786afd754 100644 >> --- a/drivers/media/i2c/ov5693.c >> +++ b/drivers/media/i2c/ov5693.c >> @@ -404,8 +404,8 @@ static int ov5693_read_reg(struct ov5693_device >> *ov5693, u32 addr, u32 *value) >> ret = i2c_transfer(client->adapter, msg, 2); >> if (ret < 0) > > i2c_transfer returns the number of transmitted messages. So I think the > values 0 <= ret < 2 also need to be handled. Ok, agreed. If ok for you, I'll send a follow-up patch when/if this one is applied, because what you spotted is unrelated to the dev_err_probe() behavior. CJ > >> return dev_err_probe(&client->dev, ret, >> - "Failed to read register 0x%04x: %d\n", >> - addr & OV5693_REG_ADDR_MASK, ret); >> + "Failed to read register 0x%04x\n", >> + addr & OV5693_REG_ADDR_MASK); >> *value = 0; >> for (i = 0; i < len; ++i) { > >
Am 21.04.23 um 17:50 schrieb Christophe JAILLET: > Le 21/04/2023 à 09:38, Matthias Schwarzott a écrit : >> Am 15.04.23 um 18:28 schrieb Christophe JAILLET: >>> dev_err_probe() already display the error code. There is no need to >>> duplicate it explicitly in the error message. >>> >>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >>> --- >>> drivers/media/i2c/ov5693.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c >>> index e3c3bed69ad6..d23786afd754 100644 >>> --- a/drivers/media/i2c/ov5693.c >>> +++ b/drivers/media/i2c/ov5693.c >>> @@ -404,8 +404,8 @@ static int ov5693_read_reg(struct ov5693_device >>> *ov5693, u32 addr, u32 *value) >>> ret = i2c_transfer(client->adapter, msg, 2); >>> if (ret < 0) >> >> i2c_transfer returns the number of transmitted messages. So I think >> the values 0 <= ret < 2 also need to be handled. > > Ok, agreed. > > If ok for you, I'll send a follow-up patch when/if this one is applied, > because what you spotted is unrelated to the dev_err_probe() behavior. > Sure, fine for me. > CJ >> >>> return dev_err_probe(&client->dev, ret, >>> - "Failed to read register 0x%04x: %d\n", >>> - addr & OV5693_REG_ADDR_MASK, ret); >>> + "Failed to read register 0x%04x\n", >>> + addr & OV5693_REG_ADDR_MASK); >>> *value = 0; >>> for (i = 0; i < len; ++i) { >> >> >
diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c index e3c3bed69ad6..d23786afd754 100644 --- a/drivers/media/i2c/ov5693.c +++ b/drivers/media/i2c/ov5693.c @@ -404,8 +404,8 @@ static int ov5693_read_reg(struct ov5693_device *ov5693, u32 addr, u32 *value) ret = i2c_transfer(client->adapter, msg, 2); if (ret < 0) return dev_err_probe(&client->dev, ret, - "Failed to read register 0x%04x: %d\n", - addr & OV5693_REG_ADDR_MASK, ret); + "Failed to read register 0x%04x\n", + addr & OV5693_REG_ADDR_MASK); *value = 0; for (i = 0; i < len; ++i) {
dev_err_probe() already display the error code. There is no need to duplicate it explicitly in the error message. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- drivers/media/i2c/ov5693.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)