From: Andrej Picej <andrej.picej@norik.com>
To: Sascha Hauer <sha@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload
Date: Mon, 25 Jan 2021 10:43:57 +0100 [thread overview]
Message-ID: <200bfc2a-e616-0e34-b2c4-91ef67915836@norik.com> (raw)
In-Reply-To: <20210125091547.GB8233@pengutronix.de>
On 25. 01. 21 10:15, Sascha Hauer wrote:
> 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
>
I totally agree with fixing the issue and will look into it a bit more.
I will reset the NAND before reading the ID in the same way as it is
done before reading parameter page. I will leave the testing on over
night (reading the ID failed in about 1 out of 500 boots) and report the
results tomorrow.
BR,
Andrej
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
next prev parent reply other threads:[~2021-01-25 9:44 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
2021-01-25 9:43 ` Andrej Picej [this message]
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=200bfc2a-e616-0e34-b2c4-91ef67915836@norik.com \
--to=andrej.picej@norik.com \
--cc=barebox@lists.infradead.org \
--cc=sha@pengutronix.de \
/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