mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <sha@pengutronix.de>
To: Andrej Picej <andrej.picej@norik.com>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload
Date: Mon, 25 Jan 2021 10:15:47 +0100	[thread overview]
Message-ID: <20210125091547.GB8233@pengutronix.de> (raw)
In-Reply-To: <7df4659e-7237-c5fa-767f-7dedd5c44005@norik.com>

On Thu, Jan 21, 2021 at 11:28:39AM +0100, Andrej Picej wrote:
> Hi Sascha,
> 
> thanks for your comments.
> 
> Responses inline.
> 
> On 21. 01. 21 10:01, Sascha Hauer wrote:
> > Hi Andrej,
> > 
> > Some comments inline.
> > 
> > On Wed, Jan 20, 2021 at 01:51:06PM +0100, Andrej Picej wrote:
> > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > 
> > > Commit is based on initial Sascha Hauer's work. It implements PBL xload
> > > mechanism to load image from GPMI NAND flash.
> > > 
> > > Additional work was done, so that the NAND's size, page size and OOB's
> > > size are autodetected and not hardcoded. Detection method follows the
> > > same methods as used in NAND driver, meaning NAND ONFI support is probed
> > > and if NAND supports ONFI, NAND memory organization is read from ONFI
> > > parameter page otherwise "READ ID" is used.
> > > 
> > > Currently only implemented for i.MX6 familly of SoCs.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> > > Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> > > ---
> > >   arch/arm/mach-imx/Makefile             |    2 +-
> > >   arch/arm/mach-imx/include/mach/xload.h |    1 +
> > >   arch/arm/mach-imx/xload-gpmi-nand.c    | 1163 ++++++++++++++++++++++++
> > >   3 files changed, 1165 insertions(+), 1 deletion(-)
> > >   create mode 100644 arch/arm/mach-imx/xload-gpmi-nand.c
> > > 
> > > diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> > > index e45f758e9..d94c846a1 100644
> > > --- a/arch/arm/mach-imx/Makefile
> > > +++ b/arch/arm/mach-imx/Makefile
> > > +static int mxs_nand_get_info(struct mxs_nand_info *info, void *databuf)
> > > +{
> > > +	int ret, i;
> > > +
> > > +	ret = mxs_nand_check_onfi(info, databuf);
> > > +	if (ret) {
> > > +		if (ret != 1)
> > > +			return ret;
> > > +		pr_warn("ONFI not supported, try \"READ ID\"...\n");
> > 
> > You already printed a "ONFI not supported\n" message. Printing it once
> > is enough. Also this message appears with every non-ONFI nand, right? In
> > that case it should rather be pr_info()
> 
> OK, I agree, will fix it in v2.
> 
> > 
> > > +	/*
> > > +	 * If ONFI is not supported or if it fails try to get NAND's info from
> > > +	 * "READ ID" command.
> > > +	 */
> > > +	pr_debug("Trying \"READ ID\" command...\n");
> > > +	ret = mxs_nand_get_readid(info, databuf);
> > > +	if (ret) {
> > > +		if (ret != -EOVERFLOW)
> > > +			return ret;
> > > +
> > > +		/*
> > > +		 * If NAND's "READ ID" returns bad values, try to set them to
> > > +		 * default (most common NAND memory org.) and continue.
> > > +		 */
> > > +		pr_warn("NANDs READ ID command returned bad values" \
> > > +			" set them to default and try to continue!\n");
> > > +		info->organization.pagesize = 2048;
> > > +		info->organization.oobsize = 64;
> > > +		info->nand_size = SZ_1G;
> > 
> > Is this worth it? READ ID is the most basic command, when this doesn't
> > work I don't think there's a point in continuing.
> 
> We had a case with Samsung K9K8G08U0E when sometimes (reasons not known) the
> 5th byte returned 0xff instead of correct values, all other returned values
> were correct. In this case the device booted successfully because of this
> hook. Maybe a better solution would be to check only the 5th byte for 0xff
> value (5th byte is not supported from all NAND vendors), if this is the case
> set the NAND size to 1GB.
> Would that make more sense?

The best would be to track the issue down and to fix it ;)
It's not very nice to assume it in case of read id failures it's exactly
that NAND type you have troubles with.
Does it help to reset the NAND chip before reading the ID?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  reply	other threads:[~2021-01-25  9:15 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-20 12:51 [PATCH 0/3] GPMI NAND xload for i.MX6 Andrej Picej
2021-01-20 12:51 ` [PATCH 1/3] ARM: i.MX: move BCB structures to header file Andrej Picej
2021-01-20 12:51 ` [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload Andrej Picej
2021-01-20 21:45   ` Roland Hieber
2021-01-21  9:01   ` Sascha Hauer
2021-01-21 10:28     ` Andrej Picej
2021-01-25  9:15       ` Sascha Hauer [this message]
2021-01-25  9:43         ` Andrej Picej
2021-01-26 11:40           ` Andrej Picej
2021-01-26 12:08             ` Sascha Hauer
2021-01-26 12:18               ` Andrej Picej
2021-01-20 12:51 ` [PATCH 3/3] ARM: i.MX: xload-gpmi-nand: apply errata 007117 Andrej Picej
2021-01-21  9:11   ` Sascha Hauer
2021-01-21 12:23     ` Andrej Picej
2021-01-22  8:12       ` Sascha Hauer
2021-01-22  9:47         ` Andrej Picej
2021-01-25  8:57           ` Sascha Hauer

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=20210125091547.GB8233@pengutronix.de \
    --to=sha@pengutronix.de \
    --cc=andrej.picej@norik.com \
    --cc=barebox@lists.infradead.org \
    /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