From: Richard Weinberger <richard@nod.at>
To: Ben Shelton <ben.shelton@ni.com>
Cc: linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
dwmw2@infradead.org, computersforpeace@gmail.com,
punnaiah.choudary.kalluri@xilinx.com
Subject: Re: [PATCH 1/3] mtd: nand: Add on-die ECC support
Date: Tue, 28 Apr 2015 00:19:16 +0200 [thread overview]
Message-ID: <553EB5E4.3050309@nod.at> (raw)
In-Reply-To: <20150427213558.GA22780@bshelton-desktop>
Am 27.04.2015 um 23:35 schrieb Ben Shelton:
> I tested this against the latest version of the PL353 NAND driver that Punnaiah
> has been working to upstream (copying her on this message). With a few changes
> to that driver, I got it most of the way through initialization with on-die ECC
> enabled, but it segfaults here with a null pointer dereference because the
> PL353 driver does not implement chip->cmd_ctrl. Instead, it implements a
> custom override of cmd->cmdfunc that does not call cmd_ctrl. Looking through
> the other in-tree NAND drivers, it looks like most of them do implement
> cmd_ctrl, but quite a few of them do not (e.g. au1550nd, denali, docg4).
>
> What do you think would be the best way to handle this? It seems like this gap
> could be bridged from either side -- either the PL353 driver could implement
> cmd_ctrl, at least as a stub version that provides the expected behavior in
> this case; or the on-die framework could break this out into a callback
> function with a default implementation that the driver could override to
> perform this behavior in the manner of its choosing.
Oh, I thought every driver has to implement that function. ;-\
But you're right there is a corner case.
What we could do is just using chip->cmdfunc() with a custom NAND command.
i.e. chip->cmdfunc(mtd, NAND_CMD_READMODE, -1, -1);
Gerhard Sittig tried to introduce such a command some time ago:
http://lists.infradead.org/pipermail/linux-mtd/2014-April/053115.html
Maybe Brian can bring some light into that too...
> When I build this without CONFIG_MTD_NAND_ECC_ON_DIE enabled, I get the
> following warning here:
>
> In file included from drivers/mtd/nand/nand_base.c:46:0:
> include/linux/mtd/nand_ondie.h: In function 'nand_read_subpage_on_die':
> include/linux/mtd/nand_ondie.h:28:1: warning: no return statement in function returning non-void [-Wreturn-type]
> include/linux/mtd/nand_ondie.h: In function 'nand_read_page_on_die':
> include/linux/mtd/nand_ondie.h:34:1: warning: no return statement in function returning non-void [-Wreturn-type]
>
> Perhaps return an error code here, even though you'll never get past the BUG()?
What gcc is this?
gcc 4.8 here does not warn, I thought it is smart enough that this function does never
return. Can it be that your .config has CONFIG_BUG=n?
Anyway, this functions clearly needs a return statement. :)
Thanks,
//richard
next prev parent reply other threads:[~2015-04-27 22:19 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-25 14:02 [RFC] On-die ECC support Richard Weinberger
2015-03-25 14:02 ` [PATCH 1/3] mtd: nand: Add on-die " Richard Weinberger
2015-03-25 20:39 ` Paul Bolle
2015-04-27 21:35 ` Ben Shelton
2015-04-27 22:19 ` Richard Weinberger [this message]
2015-04-27 22:36 ` Ben Shelton
2015-04-27 22:42 ` Richard Weinberger
2015-04-27 22:53 ` Brian Norris
2015-04-27 22:57 ` Richard Weinberger
2015-04-27 23:10 ` Brian Norris
2015-04-27 23:15 ` Richard Weinberger
2015-04-27 23:19 ` Brian Norris
2015-04-27 23:23 ` Brian Norris
2015-04-28 2:48 ` punnaiah choudary kalluri
2015-04-28 3:22 ` Brian Norris
2015-04-28 3:44 ` punnaiah choudary kalluri
2015-04-28 14:03 ` Josh Cartwright
2015-04-28 16:19 ` punnaiah choudary kalluri
2015-05-08 21:26 ` Ben Shelton
2015-05-08 21:39 ` Brian Norris
2015-05-08 21:43 ` Richard Weinberger
2015-04-28 3:15 ` punnaiah choudary kalluri
2015-03-25 14:02 ` [PATCH 2/3] mtd: nand: Add support for raw access when using on-die ECC Richard Weinberger
2015-03-25 14:02 ` [PATCH 3/3] mtd: nand: Wire up on-die ECC support Richard Weinberger
2015-04-21 12:31 ` [RFC] On-die " Richard Weinberger
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=553EB5E4.3050309@nod.at \
--to=richard@nod.at \
--cc=ben.shelton@ni.com \
--cc=computersforpeace@gmail.com \
--cc=dwmw2@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=punnaiah.choudary.kalluri@xilinx.com \
/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