[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1551724129.4932.5@crapouillou.net>
Date: Mon, 04 Mar 2019 19:28:49 +0100
From: Paul Cercueil <paul@...pouillou.net>
To: Miquel Raynal <miquel.raynal@...tlin.com>
Cc: David Woodhouse <dwmw2@...radead.org>,
Brian Norris <computersforpeace@...il.com>,
Boris Brezillon <bbrezillon@...nel.org>,
Marek Vasut <marek.vasut@...il.com>,
Richard Weinberger <richard@....at>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Harvey Hunt <harveyhuntnexus@...il.com>,
linux-mtd@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 7/9] mtd: rawnand: ingenic: Add support for the JZ4740
On Mon, Mar 4, 2019 at 11:34 AM, Miquel Raynal
<miquel.raynal@...tlin.com> wrote:
> Hi Paul,
>
> Paul Cercueil <paul@...pouillou.net <mailto:paul@...pouillou.net>>
> wrote on Sat, 9 Feb 2019 16:23:03
> -0300:
>
>> Add support for probing the ingenic-nand driver on the JZ4740 SoC
>> from
>> Ingenic, and the jz4740-ecc driver to support the JZ4740-specific
>> ECC hardware.
>>
>> Signed-off-by: Paul Cercueil <paul@...pouillou.net
>> <mailto:paul@...pouillou.net>>
>> ---
>>
>> Changes:
>>
>> v2: New patch
>>
>> v3: Also add support for the hardware ECC of the JZ4740 in this
>> patch
>>
>> v4: - Fix formatting issues
>> - Add MODULE_* macros
>>
>> drivers/mtd/nand/raw/ingenic/Kconfig | 10 ++
>> drivers/mtd/nand/raw/ingenic/Makefile | 1 +
>> drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 48 +++++--
>> drivers/mtd/nand/raw/ingenic/jz4740_ecc.c | 196
>> ++++++++++++++++++++++++++++
>> 4 files changed, 244 insertions(+), 11 deletions(-)
>> create mode 100644 drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
>>
>
> [...]
>
>> switch (chip->ecc.mode) {
>> case NAND_ECC_HW:
>> @@ -270,8 +279,8 @@ static int ingenic_nand_init_chip(struct
>> platform_device *pdev,
>> return -ENOMEM;
>> mtd->dev.parent = dev;
>>
>> - chip->legacy.IO_ADDR_R = cs->base + OFFSET_DATA;
>> - chip->legacy.IO_ADDR_W = cs->base + OFFSET_DATA;
>> + chip->legacy.IO_ADDR_R = cs->base + nfc->soc_info->data_offset;
>> + chip->legacy.IO_ADDR_W = cs->base + nfc->soc_info->data_offset;
>> chip->legacy.chip_delay = RB_DELAY_US;
>> chip->options = NAND_NO_SUBPAGE_WRITE;
>> chip->legacy.select_chip = ingenic_nand_select_chip;
>
> I think Boris already asked for it, but it would be really great that
> you update this driver to not use any legacy interface anymore.
I thought I'd send a patch later. But I don't mind doing the update in
this patchset.
>> diff --git a/drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
>> b/drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
>> new file mode 100644
>> index 000000000000..83b42881720e
>> --- /dev/null
>> +++ b/drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
>> @@ -0,0 +1,196 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * JZ4740 ECC controller driver
>> + *
>> + * Copyright (c) 2019 Paul Cercueil <paul@...pouillou.net
>> <mailto:paul@...pouillou.net>>
>> + *
>> + * based on jz4740-nand.c
>> + */
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "ingenic_ecc.h"
>> +
>> +#define JZ_REG_NAND_ECC_CTRL 0x00
>> +#define JZ_REG_NAND_DATA 0x04
>> +#define JZ_REG_NAND_PAR0 0x08
>> +#define JZ_REG_NAND_PAR1 0x0C
>> +#define JZ_REG_NAND_PAR2 0x10
>> +#define JZ_REG_NAND_IRQ_STAT 0x14
>> +#define JZ_REG_NAND_IRQ_CTRL 0x18
>> +#define JZ_REG_NAND_ERR(x) (0x1C + ((x) << 2))
>> +
>> +#define JZ_NAND_ECC_CTRL_PAR_READY BIT(4)
>> +#define JZ_NAND_ECC_CTRL_ENCODING BIT(3)
>> +#define JZ_NAND_ECC_CTRL_RS BIT(2)
>> +#define JZ_NAND_ECC_CTRL_RESET BIT(1)
>> +#define JZ_NAND_ECC_CTRL_ENABLE BIT(0)
>> +
>> +#define JZ_NAND_STATUS_ERR_COUNT (BIT(31) | BIT(30) | BIT(29))
>> +#define JZ_NAND_STATUS_PAD_FINISH BIT(4)
>> +#define JZ_NAND_STATUS_DEC_FINISH BIT(3)
>> +#define JZ_NAND_STATUS_ENC_FINISH BIT(2)
>> +#define JZ_NAND_STATUS_UNCOR_ERROR BIT(1)
>> +#define JZ_NAND_STATUS_ERROR BIT(0)
>> +
>> +static const uint8_t empty_block_ecc[] = {
>> + 0xcd, 0x9d, 0x90, 0x58, 0xf4, 0x8b, 0xff, 0xb7, 0x6f
>> +};
>> +
>> +static void jz4740_ecc_init(struct ingenic_ecc *ecc, bool encode)
>> +{
>> + uint32_t reg;
>> +
>> + /* Clear interrupt status */
>> + writel(0, ecc->base + JZ_REG_NAND_IRQ_STAT);
>> +
>> + /* Initialize and enable ECC hardware */
>> + reg = readl(ecc->base + JZ_REG_NAND_ECC_CTRL);
>> + reg |= JZ_NAND_ECC_CTRL_RESET;
>> + reg |= JZ_NAND_ECC_CTRL_ENABLE;
>> + reg |= JZ_NAND_ECC_CTRL_RS;
>> + if (encode)
>> + reg |= JZ_NAND_ECC_CTRL_ENCODING;
>> + else
>> + reg &= ~JZ_NAND_ECC_CTRL_ENCODING;
>> +
>> + writel(reg, ecc->base + JZ_REG_NAND_ECC_CTRL);
>> +}
>> +
>> +static int jz4740_ecc_calculate(struct ingenic_ecc *ecc,
>> + struct ingenic_ecc_params *params,
>> + const u8 *buf, u8 *ecc_code)
>> +{
>> + uint32_t reg, status;
>> + unsigned int timeout = 1000;
>> + int i;
>> +
>> + jz4740_ecc_init(ecc, true);
>> +
>> + do {
>> + status = readl(ecc->base + JZ_REG_NAND_IRQ_STAT);
>> + } while (!(status & JZ_NAND_STATUS_ENC_FINISH) && --timeout);
>> +
>> + if (timeout == 0)
>> + return -ETIMEDOUT;
>> +
>> + reg = readl(ecc->base + JZ_REG_NAND_ECC_CTRL);
>> + reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
>> + writel(reg, ecc->base + JZ_REG_NAND_ECC_CTRL);
>> +
>> + for (i = 0; i < params->bytes; ++i)
>> + ecc_code[i] = readb(ecc->base + JZ_REG_NAND_PAR0 + i);
>> +
>> + /* If the written data is completely 0xff, we also want to write
>> 0xff as
>> + * ecc, otherwise we will get in trouble when doing subpage
>> writes.
>> + */
>
> Comment formatting
>
> s/ecc/ECC/ in plain English
>
>> + if (memcmp(ecc_code, empty_block_ecc,
>> ARRAY_SIZE(empty_block_ecc)) == 0)
>> + memset(ecc_code, 0xff, ARRAY_SIZE(empty_block_ecc));
>> +
>> + return 0;
>> +}
>> +
>
>
> Thanks,
> Miquèl
Powered by blists - more mailing lists