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: Thu, 21 Jan 2021 10:01:48 +0100 [thread overview]
Message-ID: <20210121090148.GF19063@pengutronix.de> (raw)
In-Reply-To: <20210120125107.736121-3-andrej.picej@norik.com>
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()
> + /*
> + * 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.
> + }
> +succ:
> + pr_debug("NAND page_size: %d\n", info->organization.pagesize);
> + pr_debug("NAND block_size: %d\n",
> + info->organization.pages_per_eraseblock
> + * info->organization.pagesize);
> + pr_debug("NAND oob_size: %d\n", info->organization.oobsize);
> + pr_debug("NAND nand_size: %lu\n", info->nand_size);
> + pr_debug("NAND bits_per_cell: %d\n", info->organization.bits_per_cell);
> + pr_debug("NAND planes_per_lun: %d\n",
> + info->organization.planes_per_lun);
> + pr_debug("NAND luns_per_target: %d\n",
> + info->organization.luns_per_target);
> + pr_debug("NAND eraseblocks_per_lun: %d\n",
> + info->organization.eraseblocks_per_lun);
> + pr_debug("NAND ntargets: %d\n", info->organization.ntargets);
> +
> +
> + return 0;
> +}
> +
> +/* ---------------------------- BCB handling part -------------------------- */
> +
> +static uint32_t calc_chksum(void *buf, size_t size)
> +{
> + u32 chksum = 0;
> + u8 *bp = buf;
> + size_t i;
> +
> + for (i = 0; i < size; i++)
> + chksum += bp[i];
> +
> + return ~chksum;
> +}
> +
> +static int get_fcb(struct mxs_nand_info *info, void *databuf)
> +{
> + int i, pagenum, ret;
> + uint32_t checksum;
> + struct fcb_block *fcb = &info->fcb;
> +
> + /* First page read fails, this shouldn't be necessary */
> + mxs_nand_read_page(info, info->organization.pagesize,
> + info->organization.oobsize, 0, databuf, 1);
> +
> + for (i = 0; i < 4; i++) {
> + pagenum = 64 * i;
I haven't looked at the documentation, but the '64' should probably be pages
per block, so better info->organization.pages_per_eraseblock?
> +
> + ret = mxs_nand_read_page(info, info->organization.pagesize,
> + info->organization.oobsize, pagenum, databuf, 1);
> + if (ret)
> + continue;
> +
> + memcpy(fcb, databuf + mxs_nand_aux_status_offset(),
> + sizeof(*fcb));
> +
> + if (fcb->FingerPrint != FCB_FINGERPRINT) {
> + pr_err("No FCB found on page %d\n", pagenum);
> + continue;
> + }
> +
> + checksum = calc_chksum((void *)fcb +
> + sizeof(uint32_t), sizeof(*fcb) - sizeof(uint32_t));
> +
> + if (checksum != fcb->Checksum) {
> + pr_err("FCB on page %d has invalid checksum. " \
> + "Expected: 0x%08x, calculated: 0x%08x",
> + pagenum, fcb->Checksum, checksum);
> + continue;
> + }
> +
> + pr_debug("Found FCB:\n");
> + pr_debug("PageDataSize: 0x%08x\n", fcb->PageDataSize);
> + pr_debug("TotalPageSize: 0x%08x\n", fcb->TotalPageSize);
> + pr_debug("SectorsPerBlock: 0x%08x\n", fcb->SectorsPerBlock);
> + pr_debug("FW1_startingPage: 0x%08x\n",
> + fcb->Firmware1_startingPage);
> + pr_debug("PagesInFW1: 0x%08x\n", fcb->PagesInFirmware1);
> + pr_debug("FW2_startingPage: 0x%08x\n",
> + fcb->Firmware2_startingPage);
> + pr_debug("PagesInFW2: 0x%08x\n", fcb->PagesInFirmware2);
> +
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int get_dbbt(struct mxs_nand_info *info, void *databuf)
> +{
> + int i, ret;
> + int page;
> + int startpage = info->fcb.DBBTSearchAreaStartAddress;
> + struct dbbt_block dbbt;
> +
> + for (i = 0; i < 4; i++) {
> + page = startpage + i * 64;
Same here.
Regards,
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
next prev parent reply other threads:[~2021-01-21 9:01 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 [this message]
2021-01-21 10:28 ` Andrej Picej
2021-01-25 9:15 ` Sascha Hauer
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=20210121090148.GF19063@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