Re: [RFC 5/5] i3c: dw; add print if cannot get resources

From: Ben Dooks <ben.dooks@codethink.co.uk>
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC 5/5] i3c: dw; add print if cannot get resources
Date: Tue, 11 Jul 2023 09:52:24 +0100	[thread overview]
Message-ID: <eb115b92-35f7-2ea7-0cd2-b825e769186e@codethink.co.uk> (raw)
In-Reply-To: <2023070421375530819b2c@mail.local>

On 04/07/2023 22:37, Alexandre Belloni wrote:
> Hello,
> 
> Please fix the typo in the subject.
> 
> On 21/06/2023 17:20:05+0100, Ben Dooks wrote:
>> The devm_reset_control_get_optional_exclusive() call does
>> not print any errors, neiterh does the clk_prepare_enable
> Also fix this one.
> 
>> or devm_request_irq() call.
>>
>> Add some basic error printing to make the probe failures
>> easier to debug.
> 
> I guess all those dev_err could be dev_dbg so we don't litter the driver
> with strings that will only ever be useful during bring up.

I think dev_err_probe() is probably the right one as we aren't going
to want to warn on any sort of probe defer errors. However most drivers
are printing errors if there are resources missing so I think an error
is the best way to report issues.

I'll correct the spelling when the discussions about the correct way to
print errors is.

>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>>   drivers/i3c/master/dw-i3c-master.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
>> index 9332ae5f6419..ffc84ff6225c 100644
>> --- a/drivers/i3c/master/dw-i3c-master.c
>> +++ b/drivers/i3c/master/dw-i3c-master.c
>> @@ -1429,12 +1429,16 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
>>   
>>   	master->core_rst = devm_reset_control_get_optional_exclusive(&pdev->dev,
>>   								    "core_rst");
>> -	if (IS_ERR(master->core_rst))
>> +	if (IS_ERR(master->core_rst)) {
>> +		dev_err(&pdev->dev, "cannot get core_rst\n");
>>   		return PTR_ERR(master->core_rst);
>> +	}
>>   
>>   	ret = clk_prepare_enable(master->core_clk);
>> -	if (ret)
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "cannot enable core_clk\n");
>>   		goto err_disable_core_clk;
>> +	}
>>   
>>   	reset_control_deassert(master->core_rst);
>>   
>> @@ -1446,8 +1450,10 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
>>   	ret = devm_request_irq(&pdev->dev, irq,
>>   			       dw_i3c_master_irq_handler, 0,
>>   			       dev_name(&pdev->dev), master);
>> -	if (ret)
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "cannot get irq\n");
>>   		goto err_assert_rst;
>> +	}
>>   
>>   	platform_set_drvdata(pdev, master);
>>   
>> -- 
>> 2.40.1
>>
> 

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c