public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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

  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