[lkml]   [2018]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
SubjectRe: [PATCH v4 1/3] driver core: add probe_err log helper
On 20.12.2018 12:14, Greg Kroah-Hartman wrote:
> On Thu, Dec 20, 2018 at 11:22:45AM +0100, Andrzej Hajda wrote:
>> During probe every time driver gets resource it should usually check for error
>> printk some message if it is not -EPROBE_DEFER and return the error. This
>> pattern is simple but requires adding few lines after any resource acquisition
>> code, as a result it is often omited or implemented only partially.
>> probe_err helps to replace such code sequences with simple call, so code:
>> if (err != -EPROBE_DEFER)
>> dev_err(dev, ...);
>> return err;
>> becomes:
>> return probe_err(dev, err, ...);
> Can you show a driver being converted to use this to show if it really
> will save a bunch of lines and make things simpler? Usually you are
> requesting lots of resources so you need to do more than just return,
> you need to clean stuff up first.

I have posted sample conversion patch (generated by cocci) in previous
version of this patchset [1].

I did not re-posted it again as it is quite big patch and it will not be
applied without prior splitting it per subsystem.

Regarding stuff cleaning: devm_* usually makes it unnecessary, but also
even with necessary cleaning you can profit from probe_err, you just
calls it without leaving probe - you have still handled correctly probe

Here is sample usage (taken from beginning of the mentioned patch):

diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 4b900fc659f7..52e891fe1586 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -581,11 +581,8 @@ int ahci_platform_init_host(struct platform_device *pdev,
int i, irq, n_ports, rc;

irq = platform_get_irq(pdev, 0);
- if (irq <= 0) {
- if (irq != -EPROBE_DEFER)
- dev_err(dev, "no irq\n");
- return irq;
- }
+ if (irq <= 0)
+ return probe_err(dev, irq, "no irq\n");


And there is plenty of such places, with new version of cocci script I can locate about 2700 such cases, and it is still far from completeness.



 \ /
  Last update: 2018-12-20 12:38    [W:0.136 / U:4.524 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site