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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <eb115b92-35f7-2ea7-0cd2-b825e769186e@codethink.co.uk>
Date:   Tue, 11 Jul 2023 09:52:24 +0100
From:   Ben Dooks <ben.dooks@...ethink.co.uk>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc:     linux-i3c@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC 5/5] i3c: dw; add print if cannot get resources

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@...ethink.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