linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Evgeny Novikov <novikov@ispras.ru>
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrew Lunn <andrew@lunn.ch>,
	Mike Turquette <mturquette@linaro.org>,
	Kirill Shilimanov <kirill.shilimanov@huawei.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	ldv-project@linuxtesting.org
Subject: Re: [PATCH] usb: ehci-orion: Handle errors of clk_prepare_enable() in probe
Date: Fri, 27 Aug 2021 14:51:56 +0300	[thread overview]
Message-ID: <20210827115156.GK7722@kadam> (raw)
In-Reply-To: <4d91f982-99df-29d2-c335-1df0c23acbc8@ispras.ru>

On Thu, Aug 26, 2021 at 07:26:22PM +0300, Evgeny Novikov wrote:
> I added Dan to the discussion since he is a developer of one of such
> tools.

Thanks for that...  :P

I never warn about "forgot to check the return" bugs except in the case
of error pointers or allocation failures.  There are too many false
positives.  If people want to do that they should add a __must_check
attribute to the function.

You linked to another thread: https://lkml.org/lkml/2021/8/17/239

That patch isn't correct.  Miquel was on the right track but not 100%.
The nand_scan() calls mxic_nfc_clk_enable() so we should disable it
until it has been successfully enabled.  The current code can trigger a
WARN() in __clk_disable().  In other words it should have been:

diff --git a/drivers/mtd/nand/raw/mxic_nand.c b/drivers/mtd/nand/raw/mxic_nand.c
index da1070993994..87aef98f5b9d 100644
--- a/drivers/mtd/nand/raw/mxic_nand.c
+++ b/drivers/mtd/nand/raw/mxic_nand.c
@@ -535,11 +535,11 @@ static int mxic_nfc_probe(struct platform_device *pdev)
 	err = devm_request_irq(&pdev->dev, irq, mxic_nfc_isr,
 			       0, "mxic-nfc", nfc);
 	if (err)
-		goto fail;
+		return err;
 
 	err = nand_scan(nand_chip, 1);
 	if (err)
-		goto fail;
+		return err;
 
 	err = mtd_device_register(mtd, NULL, 0);
 	if (err)

nand_scan() error handling is still leaky, but it's hard to fix without
re-working the API.

Anyway, thanks for the fixes.  I've been inspired by the Linux Driver
Verification project work.

regards,
dan carpenter

  reply	other threads:[~2021-08-27 11:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-25 17:09 [PATCH] usb: ehci-orion: Handle errors of clk_prepare_enable() in probe Evgeny Novikov
2021-08-25 17:29 ` Alan Stern
2021-08-26 13:30   ` Evgeny Novikov
2021-08-26 15:24     ` Alan Stern
2021-08-26 16:26       ` Evgeny Novikov
2021-08-27 11:51         ` Dan Carpenter [this message]
2021-08-28 10:37           ` Evgeny Novikov
2021-08-28 10:47       ` Evgeny Novikov
2021-08-28 15:11         ` Alan Stern
2021-08-25 17:33 ` Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210827115156.GK7722@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=andrew@lunn.ch \
    --cc=gregkh@linuxfoundation.org \
    --cc=kirill.shilimanov@huawei.com \
    --cc=ldv-project@linuxtesting.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mturquette@linaro.org \
    --cc=novikov@ispras.ru \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).