On 26. 01. 21 13:08, Sascha Hauer wrote:
On Tue, Jan 26, 2021 at 12:40:17PM +0100, Andrej Picej wrote:
On 25. 01. 21 10:43, Andrej Picej wrote:
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@xxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxx>
Signed-off-by: Primoz Fiser <primoz.fiser@xxxxxxxxx>
Signed-off-by: Andrej Picej <andrej.picej@xxxxxxxxx>
---
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.
I guess resetting fixed this problem, I got 50.000 boots without any
problems. Will fix this in v2 :).
Should I now completely abandon setting the NANDs memory to default values
and just return error on failure?
Yes please.
Something like this:
/*
* If ONFI is not supported or if it fails try to get NAND's info from
* "READ ID" command.
*/
ret = mxs_nand_reset(info, databuf);
if (ret)
return ret;
pr_debug("Trying \"READ ID\" command...\n");
ret = mxs_nand_get_readid(info, databuf);
if (ret) {
pr_err("xloader supports only ONFI and generic \"READ ID\" " \
"supported NANDs\n");
return -1;
}
I'm asking this because NANDs ID command is manufacturer specific, some
manufacturers follow generic values and other unfortunately don't. In
barebox NAND driver decoding is done based on manufacturer. I guess we could
do the same here but I don't want to expand the PBL part too much just for
reading FCB, where all NAND's memory parameters are decoded from.
We can do that once we have to later. Decoding the ID into page/block
sizes should also be done generically and not part of the i.MX6 NAND
loader. Maybe we could also utilize parts of the NAND layer instead of
adding new code.
Ok, will prepare v2 and submit it when done.
Thank you for your kind comments and guidance so far.
BR,
Andrej
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
