public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
Cc: Richard Weinberger <richard@nod.at>,
	linux-mtd <linux-mtd@lists.infradead.org>,
	Iwo Mergler <iwo.mergler@netcommwireless.com>,
	Brian Norris <computersforpeace@gmail.com>
Subject: Re: [PATCH v1] mtd: nandbiterrs: Have init function return 0 on success
Date: Mon, 19 Dec 2016 13:20:35 +0100	[thread overview]
Message-ID: <20161219132035.08da2a93@bbrezillon> (raw)
In-Reply-To: <49b7d473-ad61-57fa-dfb0-f0b47ed32f76@sigmadesigns.com>

Hi Marc,

On Tue, 13 Dec 2016 15:36:07 +0100
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> wrote:

> The init function currently returns -EIO on success. This behavior
> was probably chosen in order to avoid a subsequent rmmod, but this
> complicates failure detection from user-space.
> 
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
> I'm not sure failures are reported as expected. I would expect
> the test to report a failure if the driver cannot fix less than
> $STRENGTH bit flips, but it doesn't, AFAICT.
> cf. incremental_errors_test which sets err to 0 in the 
> "After %d biterrors per subpage, read reported error %d\n"
> code path.

I'm not strongly opposed to this change, but please note that it's
changing the module behavior, and some people might depend on this
rather unusual 'module probe never succeeds' thing.

If all maintainers are okay with that, then I'll ack the patch, but I'd
still prefer if you could switch to the userspace equivalent (recently
added in mtd-utils) to do your regression tests.

One last thing: I'd really like to remove the in-kernel MTD tests at
some point (assuming all the tests have been ported to mtd-utils, or
kselftest), so it's probably not a good idea to design something that
is based on it.

Regards,

Boris
> ---
>  drivers/mtd/tests/nandbiterrs.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/mtd/tests/nandbiterrs.c b/drivers/mtd/tests/nandbiterrs.c
> index f26dec896afa..41050bcae9f1 100644
> --- a/drivers/mtd/tests/nandbiterrs.c
> +++ b/drivers/mtd/tests/nandbiterrs.c
> @@ -403,7 +403,6 @@ static int __init mtd_nandbiterrs_init(void)
>  	if (err)
>  		goto exit_error;
>  
> -	err = -EIO;
>  	pr_info("finished successfully.\n");
>  	printk(KERN_INFO "==================================================\n");
>  

  parent reply	other threads:[~2016-12-19 12:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-13 14:36 [PATCH v1] mtd: nandbiterrs: Have init function return 0 on success Marc Gonzalez
2016-12-19 10:30 ` Marc Gonzalez
2016-12-19 10:33   ` Richard Weinberger
2016-12-19 12:20 ` Boris Brezillon [this message]
2016-12-19 16:23   ` Marc Gonzalez

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=20161219132035.08da2a93@bbrezillon \
    --to=boris.brezillon@free-electrons.com \
    --cc=computersforpeace@gmail.com \
    --cc=iwo.mergler@netcommwireless.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marc_gonzalez@sigmadesigns.com \
    --cc=richard@nod.at \
    /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