diff mbox series

[v7,08/34] i2c: tegra: Remove error message used for devm_request_irq() failure

Message ID 20200908224006.25636-9-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 error message prints number of vIRQ, which isn't a useful information.
In practice devm_request_irq() never fails, hence let's remove the bogus
message in order to make code cleaner.

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

Comments

Thierry Reding Sept. 17, 2020, 11:28 a.m. UTC | #1
On Wed, Sep 09, 2020 at 01:39:40AM +0300, Dmitry Osipenko wrote:
> The error message prints number of vIRQ, which isn't a useful information.
> In practice devm_request_irq() never fails, hence let's remove the bogus
> message in order to make code cleaner.
> 
> Reviewed-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

I think I have seen this fail occasionally when something's wrong in the
IRQ code, or when we are not properly configuring the IRQ in device tree
for example.

So I'd prefer if this error message stayed here. I agree that it's not a
terribly useful error message, so perhaps adding the error code to it
would make it more helpful?

Thierry

> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index a52c72135390..b813c0976c10 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -1807,10 +1807,8 @@ static int tegra_i2c_probe(struct platform_device *pdev)
>  
>  	ret = devm_request_irq(&pdev->dev, i2c_dev->irq, tegra_i2c_isr,
>  			       IRQF_NO_SUSPEND, dev_name(&pdev->dev), i2c_dev);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Failed to request irq %i\n", i2c_dev->irq);
> +	if (ret)
>  		goto release_dma;
> -	}
>  
>  	i2c_set_adapdata(&i2c_dev->adapter, i2c_dev);
>  	i2c_dev->adapter.owner = THIS_MODULE;
> -- 
> 2.27.0
>
Dmitry Osipenko Sept. 17, 2020, 2:59 p.m. UTC | #2
17.09.2020 14:28, Thierry Reding пишет:
> On Wed, Sep 09, 2020 at 01:39:40AM +0300, Dmitry Osipenko wrote:
>> The error message prints number of vIRQ, which isn't a useful information.
>> In practice devm_request_irq() never fails, hence let's remove the bogus
>> message in order to make code cleaner.
>>
>> Reviewed-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/i2c/busses/i2c-tegra.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> I think I have seen this fail occasionally when something's wrong in the
> IRQ code, or when we are not properly configuring the IRQ in device tree
> for example.
> 
> So I'd prefer if this error message stayed here. I agree that it's not a
> terribly useful error message, so perhaps adding the error code to it
> would make it more helpful?

We have dtbs_check nowadays and I assume you only saw a such problem
during of hardware bring-up, correct?

Realistically, this error should never happen "randomly" and even if it
will happen, then you will still know that I2C driver failed because
driver-core will tell you about that.

Hence there is no much value in the error message, it should be more
practical to remove it in order to keep the code cleaner.

>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>> index a52c72135390..b813c0976c10 100644
>> --- a/drivers/i2c/busses/i2c-tegra.c
>> +++ b/drivers/i2c/busses/i2c-tegra.c
>> @@ -1807,10 +1807,8 @@ static int tegra_i2c_probe(struct platform_device *pdev)
>>  
>>  	ret = devm_request_irq(&pdev->dev, i2c_dev->irq, tegra_i2c_isr,
>>  			       IRQF_NO_SUSPEND, dev_name(&pdev->dev), i2c_dev);
>> -	if (ret) {
>> -		dev_err(&pdev->dev, "Failed to request irq %i\n", i2c_dev->irq);
>> +	if (ret)
>>  		goto release_dma;
>> -	}
>>  
>>  	i2c_set_adapdata(&i2c_dev->adapter, i2c_dev);
>>  	i2c_dev->adapter.owner = THIS_MODULE;
>> -- 
>> 2.27.0
>>
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index a52c72135390..b813c0976c10 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1807,10 +1807,8 @@  static int tegra_i2c_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(&pdev->dev, i2c_dev->irq, tegra_i2c_isr,
 			       IRQF_NO_SUSPEND, dev_name(&pdev->dev), i2c_dev);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to request irq %i\n", i2c_dev->irq);
+	if (ret)
 		goto release_dma;
-	}
 
 	i2c_set_adapdata(&i2c_dev->adapter, i2c_dev);
 	i2c_dev->adapter.owner = THIS_MODULE;