2023-04-15 16:34:12

by Christophe JAILLET

Subject: [PATCH] media: ov5693: Simplify an error message

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 <[email protected]>
---
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) {
--
2.34.1

2023-04-17 08:06:11

by Dan Scally

Subject: Re: [PATCH] media: ov5693: Simplify an error message

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 <[email protected]>
> ---


Thanks for the patch:


Reviewed-by: Daniel Scally <[email protected]>

> 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) {

2023-04-21 07:49:43

by Matthias Schwarzott

Subject: Re: [PATCH] media: ov5693: Simplify an error message

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 <[email protected]>
> ---
> 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) {

2023-04-21 15:56:34

by Christophe JAILLET

Subject: Re: [PATCH] media: ov5693: Simplify an error message

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 <[email protected]>
>> ---
>>   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) {
>
>

2023-04-30 07:47:27

by Matthias Schwarzott

Subject: Re: [PATCH] media: ov5693: Simplify an error message

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 <[email protected]>
>>> ---
>>>   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) {
>>
>>
>