Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v4 1/3] driver core: add probe_err log helper
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        linux-kernel@vger.kernel.org,
        Javier Martinez Canillas <javierm@redhat.com>,
        linux-arm-kernel@lists.infradead.org, andy.shevchenko@gmail.com,
        Mark Brown <broonie@kernel.org>,
        Russell King - ARM Linux <linux@armlinux.org.uk>
From:   Andrzej Hajda <a.hajda@samsung.com>
Message-ID: <b9f7cb75-62a0-20f6-0e5b-1553852d81b2@samsung.com>
Date:   Thu, 20 Dec 2018 12:37:58 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
        Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: <20181220111403.GB10978@kroah.com>
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <20181220102247.4911-1-a.hajda@samsung.com>
        <CGME20181220102259eucas1p2f748c68e01cd4e09a266da879722e218@eucas1p2.samsung.com>
        <20181220102247.4911-2-a.hajda@samsung.com>
        <20181220111403.GB10978@kroah.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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
deferring.

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.

[1]:
https://lore.kernel.org/lkml/20181016072244.1216-4-a.hajda@samsung.com/T/#u


Regards

Andrzej