| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240506155322.002346d0@xps-13>
Date: Mon, 6 May 2024 15:53:22 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Arseniy Krasnov <avkrasnov@...utedevices.com>
Cc: Richard Weinberger <richard@....at>, Vignesh Raghavendra
<vigneshr@...com>, Rob Herring <robh+dt@...nel.org>, Krzysztof Kozlowski
<krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
Neil Armstrong <neil.armstrong@...aro.org>, Kevin Hilman
<khilman@...libre.com>, Jerome Brunet <jbrunet@...libre.com>, Martin
Blumenstingl <martin.blumenstingl@...glemail.com>,
<linux-mtd@...ts.infradead.org>, <devicetree@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-amlogic@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<oxffffaa@...il.com>, <kernel@...rdevices.ru>
Subject: Re: [PATCH v5 2/2] mtd: rawnand: meson: support R/W mode for boot
ROM
Hi Arseniy,
avkrasnov@...utedevices.com wrote on Tue, 16 Apr 2024 11:51:01 +0300:
> Boot ROM code on Meson requires that some pages on NAND must be written
> in special mode: "short" ECC mode where each block is 384 bytes and
> scrambling mode is on. Such pages located with the specified interval
> within specified offset. Both interval and offset are located in the
> device tree and used by driver if 'nand-is-boot-medium' is set for
> NAND chip.
>
> Signed-off-by: Arseniy Krasnov <avkrasnov@...utedevices.com>
> ---
> drivers/mtd/nand/raw/meson_nand.c | 88 +++++++++++++++++++++----------
> 1 file changed, 59 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
> index 00ce0e5bb970..9ee11243b257 100644
> --- a/drivers/mtd/nand/raw/meson_nand.c
> +++ b/drivers/mtd/nand/raw/meson_nand.c
> @@ -35,6 +35,7 @@
> #define NFC_CMD_RB BIT(20)
> #define NFC_CMD_SCRAMBLER_ENABLE BIT(19)
> #define NFC_CMD_SCRAMBLER_DISABLE 0
> +#define NFC_CMD_SHORTMODE_ENABLE 1
> #define NFC_CMD_SHORTMODE_DISABLE 0
> #define NFC_CMD_RB_INT BIT(14)
> #define NFC_CMD_RB_INT_NO_PIN ((0xb << 10) | BIT(18) | BIT(16))
> @@ -78,6 +79,8 @@
> #define DMA_DIR(dir) ((dir) ? NFC_CMD_N2M : NFC_CMD_M2N)
> #define DMA_ADDR_ALIGN 8
>
> +#define NFC_SHORT_MODE_ECC_SZ 384
> +
> #define ECC_CHECK_RETURN_FF (-1)
>
> #define NAND_CE0 (0xe << 10)
> @@ -125,6 +128,8 @@ struct meson_nfc_nand_chip {
> u32 twb;
> u32 tadl;
> u32 tbers_max;
> + u32 boot_pages;
> + u32 boot_page_step;
>
> u32 bch_mode;
> u8 *data_buf;
> @@ -298,28 +303,49 @@ static void meson_nfc_cmd_seed(struct meson_nfc *nfc, u32 seed)
> nfc->reg_base + NFC_REG_CMD);
> }
>
> -static void meson_nfc_cmd_access(struct nand_chip *nand, int raw, bool dir,
> - int scrambler)
> +static int meson_nfc_page_is_boot(struct nand_chip *nand, int page)
meson_nfc_is_boot_page() is easier to read
> +{
> + const struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
> +
> + return (nand->options & NAND_IS_BOOT_MEDIUM) &&
> + !(page % meson_chip->boot_page_step) &&
I would dedicate all the space below ->boot_pages to the bootrom, no?
Using space in between sounds silly.
> + (page < meson_chip->boot_pages);
> +}
> +
> +static void meson_nfc_cmd_access(struct nand_chip *nand, bool raw, bool dir, int page)
> {
> + const struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
> struct mtd_info *mtd = nand_to_mtd(nand);
> struct meson_nfc *nfc = nand_get_controller_data(mtd_to_nand(mtd));
> - struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
> - u32 bch = meson_chip->bch_mode, cmd;
> int len = mtd->writesize, pagesize, pages;
> + int scrambler;
> + u32 cmd;
>
> - pagesize = nand->ecc.size;
> + if (nand->options & NAND_NEED_SCRAMBLING)
> + scrambler = NFC_CMD_SCRAMBLER_ENABLE;
> + else
> + scrambler = NFC_CMD_SCRAMBLER_DISABLE;
That is a separate feature?
>
> if (raw) {
> len = mtd->writesize + mtd->oobsize;
> cmd = len | scrambler | DMA_DIR(dir);
> - writel(cmd, nfc->reg_base + NFC_REG_CMD);
> - return;
> - }
> + } else if (meson_nfc_page_is_boot(nand, page)) {
> + pagesize = NFC_SHORT_MODE_ECC_SZ >> 3;
> + pages = mtd->writesize / 512;
> +
> + scrambler = NFC_CMD_SCRAMBLER_ENABLE;
> + cmd = CMDRWGEN(DMA_DIR(dir), scrambler, NFC_ECC_BCH8_1K,
> + NFC_CMD_SHORTMODE_ENABLE, pagesize, pages);
> + } else {
> + pagesize = nand->ecc.size >> 3;
> + pages = len / nand->ecc.size;
>
> - pages = len / nand->ecc.size;
> + cmd = CMDRWGEN(DMA_DIR(dir), scrambler, meson_chip->bch_mode,
> + NFC_CMD_SHORTMODE_DISABLE, pagesize, pages);
> + }
>
> - cmd = CMDRWGEN(DMA_DIR(dir), scrambler, bch,
> - NFC_CMD_SHORTMODE_DISABLE, pagesize, pages);
> + if (scrambler == NFC_CMD_SCRAMBLER_ENABLE)
> + meson_nfc_cmd_seed(nfc, page);
>
> writel(cmd, nfc->reg_base + NFC_REG_CMD);
> }
> @@ -743,15 +769,7 @@ static int meson_nfc_write_page_sub(struct nand_chip *nand,
> if (ret)
> return ret;
>
> - if (nand->options & NAND_NEED_SCRAMBLING) {
> - meson_nfc_cmd_seed(nfc, page);
> - meson_nfc_cmd_access(nand, raw, DIRWRITE,
> - NFC_CMD_SCRAMBLER_ENABLE);
> - } else {
> - meson_nfc_cmd_access(nand, raw, DIRWRITE,
> - NFC_CMD_SCRAMBLER_DISABLE);
> - }
> -
Ok I get it, the feature already exist but is handled differently.
Please split this patch:
- improve scrambler handling to facilitate boot page support
- add boot pages support
> + meson_nfc_cmd_access(nand, raw, DIRWRITE, page);
> cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_PAGEPROG;
> writel(cmd, nfc->reg_base + NFC_REG_CMD);
> meson_nfc_queue_rb(nand, PSEC_TO_MSEC(sdr->tPROG_max), false);
> @@ -829,15 +847,7 @@ static int meson_nfc_read_page_sub(struct nand_chip *nand,
> if (ret)
> return ret;
>
> - if (nand->options & NAND_NEED_SCRAMBLING) {
> - meson_nfc_cmd_seed(nfc, page);
> - meson_nfc_cmd_access(nand, raw, DIRREAD,
> - NFC_CMD_SCRAMBLER_ENABLE);
> - } else {
> - meson_nfc_cmd_access(nand, raw, DIRREAD,
> - NFC_CMD_SCRAMBLER_DISABLE);
> - }
> -
> + meson_nfc_cmd_access(nand, raw, DIRREAD, page);
> ret = meson_nfc_wait_dma_finish(nfc);
> meson_nfc_check_ecc_pages_valid(nfc, nand, raw);
>
> @@ -1436,6 +1446,26 @@ meson_nfc_nand_chip_init(struct device *dev,
> if (ret)
> return ret;
>
> + if (nand->options & NAND_IS_BOOT_MEDIUM) {
> + ret = of_property_read_u32(np, "amlogic,boot-pages",
> + &meson_chip->boot_pages);
> + if (ret) {
> + dev_err(dev, "could not retrieve 'amlogic,boot-pages' property: %d",
> + ret);
> + nand_cleanup(nand);
> + return ret;
> + }
> +
> + ret = of_property_read_u32(np, "amlogic,boot-page-step",
> + &meson_chip->boot_page_step);
> + if (ret) {
> + dev_err(dev, "could not retrieve 'amlogic,boot-page-step' property: %d",
> + ret);
> + nand_cleanup(nand);
> + return ret;
> + }
> + }
> +
> ret = mtd_device_register(mtd, NULL, 0);
> if (ret) {
> dev_err(dev, "failed to register MTD device: %d\n", ret);
Thanks,
Miquèl
Powered by blists - more mailing lists