diff mbox series

media: ov5693: Simplify an error message

Message ID 928f2f70de241d0fa66801b46d736ad0f881eb72.1681576102.git.christophe.jaillet@wanadoo.fr
State New
Headers
Series media: ov5693: Simplify an error message |

Commit Message

Christophe JAILLET April 15, 2023, 4:28 p.m. UTC 
  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(-)

Comments

Dan Scally April 17, 2023, 8:05 a.m. UTC | #1 
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) {
Matthias Schwarzott April 21, 2023, 7:38 a.m. UTC | #2 
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) {
Christophe JAILLET April 21, 2023, 3:50 p.m. UTC | #3 
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) {
> 
>
Matthias Schwarzott April 30, 2023, 7:17 a.m. UTC | #4 
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 mbox series

Patch

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