[RFC v2 2/3] ath79: add NAND driver for Mikrotik RB91xG series

Sergey Ryazanov ryazanov.s.a at gmail.com
Sun May 23 18:36:04 PDT 2021


On Fri, May 21, 2021 at 2:05 PM Denis Kalashnikov <denis281089 at gmail.com> wrote:
> Main part is copied from ar71xx original driver rb91x_nand
> written by Gabor Juhos <juhosg at openwrt.org>.
>
> What is done:
> * Support of kernel 5.4 and 5.10,
> * DTS support,
> * New gpio API (gpiod_*) support.
>
> Signed-off-by: Denis Kalashnikov <denis281089 at gmail.com>
> ---
>
> Changelog:
>
> v1-->v2:
> - Do not use MFD driver API anymore.
>
> ---
>  .../files/drivers/mtd/nand/raw/rb91x_nand.c   | 414 ++++++++++++++++++
>  target/linux/ath79/mikrotik/config-default    |   1 +
>  .../patches-5.10/939-mikrotik-rb91x.patch     |  24 +
>  .../patches-5.4/939-mikrotik-rb91x.patch      |  21 +
>  4 files changed, 460 insertions(+)
>  create mode 100644 target/linux/ath79/files/drivers/mtd/nand/raw/rb91x_nand.c
>
> diff --git a/target/linux/ath79/files/drivers/mtd/nand/raw/rb91x_nand.c b/target/linux/ath79/files/drivers/mtd/nand/raw/rb91x_nand.c
> new file mode 100644
> index 0000000000..069c2978f7
> --- /dev/null
> +++ b/target/linux/ath79/files/drivers/mtd/nand/raw/rb91x_nand.c
> @@ -0,0 +1,414 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *  NAND flash driver for the MikroTik RouterBOARD 91x series
> + *
> + *  Main part is copied from original driver written by Gabor Juhos.
> + *
> + *  Copyright (C) 2013-2014 Gabor Juhos <juhosg at openwrt.org>
> + */
> +
> +/*
> + * WARNING: to speed up NAND reading/writing we are working with SoC GPIO
> + * controller registers directly -- not through standard GPIO API.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mtd/rawnand.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/platform_device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/version.h>
> +#include <linux/of_platform.h>
> +
> +/* TODO: ar71xx regs (SoC GPIO controller ones) should be get from DTS? */
> +#include <asm/mach-ath79/ar71xx_regs.h>
> +
> +#define RB91X_NAND_DRIVER_NAME  "rb91x-nand"

It is used only in one place for driver structure filling. So the
special macro is not really required.

> +static void __iomem *ath79_gpio_base;

Should this global variable be placed in the main driver state container?

> +#define RB91X_NAND_DRIVER_DESC "MikrotTik RB91x NAND flash driver"

You do not need this macro. Besides the odd trace message in the probe
function, it is only used to fill the kernel module metadata.

> +/* Bit masks for NAND data lines in ath79 gpio 32-bit register */
> +#define RB91X_NAND_NRW_BIT BIT(12)
> +#define RB91X_NAND_DATA_BITS (BIT(0) | BIT(1) | BIT(2) | BIT(3) | BIT(4) \
> +                           | BIT(13) | BIT(14) | BIT(15))
> +#define RB91X_NAND_LOW_DATA_MASK       0x1f
> +#define RB91X_NAND_HIGH_DATA_MASK      0xe0
> +#define RB91X_NAND_HIGH_DATA_SHIFT     8
> +
> +enum rb91x_nand_gpios {
> +       RB91X_NAND_ALE, /* Address Latch Enable */
> +       RB91X_NAND_CLE, /* Command Latch Enable */
> +       RB91X_NAND_NCE, /* Chip Enable. Active low */
> +       RB91X_NAND_NRW, /* Read/Write. Active low */
> +       RB91X_NAND_READ,/* Read */
> +       RB91X_NAND_RDY, /* NAND Ready */
> +       RB91X_NAND_NLE, /* Latch Enable. Active low */
> +
> +       RB91X_NAND_GPIOS,
> +};
> +
> +struct rb91x_nand_drvdata {
> +       struct nand_chip chip;
> +       struct device *dev;
> +       struct gpio_desc *gpio[RB91X_NAND_GPIOS];
> +};
> +
> +static void rb91x_nand_latch_lock(struct rb91x_nand_drvdata *drvdata, int lock)
> +{
> +       gpiod_set_value_cansleep(drvdata->gpio[RB91X_NAND_NLE], lock);
> +}
> +
> +static struct mtd_info *rb91x_nand_drvdata_to_mtd(struct rb91x_nand_drvdata *drvdata)
> +{
> +       return nand_to_mtd(&drvdata->chip);
> +}
> +
> +static int rb91x_ooblayout_ecc(struct mtd_info *mtd, int section,
> +                              struct mtd_oob_region *oobregion)
> +{
> +       switch (section) {
> +       case 0:
> +               oobregion->offset = 8;
> +               oobregion->length = 3;
> +               return 0;
> +       case 1:
> +               oobregion->offset = 13;
> +               oobregion->length = 3;
> +               return 0;
> +       default:
> +               return -ERANGE;
> +       }
> +}
> +
> +static int rb91x_ooblayout_free(struct mtd_info *mtd, int section,
> +                               struct mtd_oob_region *oobregion)
> +{
> +       switch (section) {
> +       case 0:
> +               oobregion->offset = 0;
> +               oobregion->length = 4;
> +               return 0;
> +       case 1:
> +               oobregion->offset = 4;
> +               oobregion->length = 1;
> +               return 0;
> +       case 2:
> +               oobregion->offset = 6;
> +               oobregion->length = 2;
> +               return 0;
> +       case 3:
> +               oobregion->offset = 11;
> +               oobregion->length = 2;
> +               return 0;
> +       default:
> +               return -ERANGE;
> +       }
> +}
> +
> +static const struct mtd_ooblayout_ops rb91x_nand_ecclayout_ops = {
> +       .ecc = rb91x_ooblayout_ecc,
> +       .free = rb91x_ooblayout_free,
> +};
> +
> +/* TODO: Should be in DTS */
> +static struct mtd_partition rb91x_nand_partitions[] = {

Looks like you could drop this array, since flash partitions are
already described by the DTS.

> +       {
> +               .name = "booter",
> +               .offset = 0,
> +               .size = (256 * 1024),
> +               .mask_flags = MTD_WRITEABLE,
> +       }, {
> +               .name = "kernel",
> +               .offset = (256 * 1024),
> +               .size = (4 * 1024 * 1024) - (256 * 1024),
> +       }, {
> +               .name = "ubi",
> +               .offset = MTDPART_OFS_NXTBLK,
> +               .size = MTDPART_SIZ_FULL,
> +       },
> +};
> +
> +static void rb91x_nand_write(struct rb91x_nand_drvdata *drvdata,
> +                            const u8 *buf,
> +                            unsigned len)
> +{
> +       void __iomem *base = ath79_gpio_base;
> +       u32 oe_reg;
> +       u32 out_reg;
> +       u32 out;
> +       unsigned i;
> +
> +       rb91x_nand_latch_lock(drvdata, 1);
> +
> +       oe_reg = __raw_readl(base + AR71XX_GPIO_REG_OE);
> +       out_reg = __raw_readl(base + AR71XX_GPIO_REG_OUT);
> +
> +       /* Set data lines to output mode */
> +       __raw_writel(oe_reg & ~(RB91X_NAND_DATA_BITS | RB91X_NAND_NRW_BIT),
> +                    base + AR71XX_GPIO_REG_OE);
> +
> +       out = out_reg & ~(RB91X_NAND_DATA_BITS | RB91X_NAND_NRW_BIT);
> +       for (i = 0; i != len; i++) {
> +               u32 data;
> +
> +               data = (buf[i] & RB91X_NAND_HIGH_DATA_MASK) <<
> +                       RB91X_NAND_HIGH_DATA_SHIFT;
> +               data |= buf[i] & RB91X_NAND_LOW_DATA_MASK;
> +               data |= out;
> +               __raw_writel(data, base + AR71XX_GPIO_REG_OUT);
> +
> +               /* Deactivate WE line */
> +               data |= RB91X_NAND_NRW_BIT;
> +               __raw_writel(data, base + AR71XX_GPIO_REG_OUT);
> +               /* Flush write */
> +               __raw_readl(base + AR71XX_GPIO_REG_OUT);
> +       }
> +
> +       /* Restore registers */
> +       __raw_writel(out_reg, base + AR71XX_GPIO_REG_OUT);
> +       __raw_writel(oe_reg, base + AR71XX_GPIO_REG_OE);
> +       /* Flush write */
> +       __raw_readl(base + AR71XX_GPIO_REG_OUT);
> +
> +       rb91x_nand_latch_lock(drvdata, 0);
> +}
> +
> +static void rb91x_nand_read(struct rb91x_nand_drvdata *drvdata,
> +                           u8 *read_buf,
> +                           unsigned len)
> +{
> +       void __iomem *base = ath79_gpio_base;
> +       u32 oe_reg;
> +       u32 out_reg;
> +       unsigned i;
> +
> +       /* Enable read mode */
> +       gpiod_set_value_cansleep(drvdata->gpio[RB91X_NAND_READ], 1);
> +
> +       rb91x_nand_latch_lock(drvdata, 1);
> +
> +       /* Save registers */
> +       oe_reg = __raw_readl(base + AR71XX_GPIO_REG_OE);
> +       out_reg = __raw_readl(base + AR71XX_GPIO_REG_OUT);
> +
> +       /* Set data lines to input mode */
> +       __raw_writel(oe_reg | RB91X_NAND_DATA_BITS,
> +                    base + AR71XX_GPIO_REG_OE);
> +
> +       for (i = 0; i < len; i++) {
> +               u32 in;
> +               u8 data;
> +
> +               /* Activate RE line */
> +               __raw_writel(RB91X_NAND_NRW_BIT, base + AR71XX_GPIO_REG_CLEAR);
> +               /* Flush write */
> +               __raw_readl(base + AR71XX_GPIO_REG_CLEAR);
> +
> +               /* Read input lines */
> +               in = __raw_readl(base + AR71XX_GPIO_REG_IN);
> +
> +               /* Deactivate RE line */
> +               __raw_writel(RB91X_NAND_NRW_BIT, base + AR71XX_GPIO_REG_SET);
> +
> +               data = (in & RB91X_NAND_LOW_DATA_MASK);
> +               data |= (in >> RB91X_NAND_HIGH_DATA_SHIFT) &
> +                       RB91X_NAND_HIGH_DATA_MASK;
> +
> +               read_buf[i] = data;
> +       }
> +
> +       /* Restore  registers */
> +       __raw_writel(out_reg, base + AR71XX_GPIO_REG_OUT);
> +       __raw_writel(oe_reg, base + AR71XX_GPIO_REG_OE);
> +       /* Flush write */
> +       __raw_readl(base + AR71XX_GPIO_REG_OUT);
> +
> +       rb91x_nand_latch_lock(drvdata, 0);
> +
> +       /* Disable read mode */
> +       gpiod_set_value_cansleep(drvdata->gpio[RB91X_NAND_READ], 0);
> +}
> +
> +static int rb91x_nand_dev_ready(struct nand_chip *chip)
> +{
> +       struct rb91x_nand_drvdata *drvdata = (struct rb91x_nand_drvdata *)(chip->priv);
> +
> +       return gpiod_get_value_cansleep(drvdata->gpio[RB91X_NAND_RDY]);
> +}
> +
> +static void rb91x_nand_cmd_ctrl(struct nand_chip *chip, int cmd,
> +                               unsigned int ctrl)
> +{
> +       struct rb91x_nand_drvdata *drvdata = chip->priv;
> +
> +       if (ctrl & NAND_CTRL_CHANGE) {
> +               gpiod_set_value_cansleep(drvdata->gpio[RB91X_NAND_CLE],
> +                                       (ctrl & NAND_CLE) ? 1 : 0);
> +               gpiod_set_value_cansleep(drvdata->gpio[RB91X_NAND_ALE],
> +                                       (ctrl & NAND_ALE) ? 1 : 0);
> +               gpiod_set_value_cansleep(drvdata->gpio[RB91X_NAND_NCE],
> +                                       (ctrl & NAND_NCE) ? 1 : 0);
> +       }
> +
> +       if (cmd != NAND_CMD_NONE) {
> +               u8 t = cmd;
> +
> +               rb91x_nand_write(drvdata, &t, 1);
> +       }
> +}
> +
> +static u8 rb91x_nand_read_byte(struct nand_chip *chip)
> +{
> +       u8 data = 0xff;
> +
> +       rb91x_nand_read(chip->priv, &data, 1);
> +
> +       return data;
> +}
> +
> +static void rb91x_nand_read_buf(struct nand_chip *chip, u8 *buf, int len)
> +{
> +       rb91x_nand_read(chip->priv, buf, len);
> +}
> +
> +static void rb91x_nand_write_buf(struct nand_chip *chip, const u8 *buf, int len)
> +{
> +       rb91x_nand_write(chip->priv, buf, len);
> +}
> +
> +static void rb91x_nand_release(struct rb91x_nand_drvdata *drvdata)
> +{
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,8,0)
> +       mtd_device_unregister(rb91x_nand_drvdata_to_mtd(drvdata));
> +       nand_cleanup(&drvdata->chip);
> +#else
> +       nand_release(&drvdata->chip);
> +#endif
> +}
> +
> +static int rb91x_nand_probe(struct platform_device *pdev)
> +{
> +       struct rb91x_nand_drvdata *drvdata;
> +       struct mtd_info *mtd;
> +       int i, r;
> +       struct device *dev = &pdev->dev;
> +       struct {
> +               const char *name;
> +               int flags;
> +       } gpio_info[] = {

This array could be a 'static const'.

> +               [RB91X_NAND_ALE] = { "ale", GPIOD_OUT_LOW },
> +               [RB91X_NAND_CLE] = { "cle", GPIOD_OUT_LOW },
> +               [RB91X_NAND_NCE] = { "nce", GPIOD_OUT_LOW },
> +               [RB91X_NAND_NRW] = { "nrw", GPIOD_OUT_LOW },
> +               [RB91X_NAND_READ] = { "read", GPIOD_OUT_LOW },
> +               [RB91X_NAND_RDY] = { "rdy", GPIOD_IN },
> +               [RB91X_NAND_NLE] = { "latch-enable", GPIOD_OUT_LOW },
> +       };
> +
> +       pr_info(RB91X_NAND_DRIVER_DESC "\n");

Please remove this custom tracing output on the final submission. It
produces no meaningful information, only noise.

> +       drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +       if (!drvdata)
> +               return -ENOMEM;
> +
> +       platform_set_drvdata(pdev, drvdata);
> +
> +       for (i = 0, r = 0; i < ARRAY_SIZE(drvdata->gpio); i++) {
> +               drvdata->gpio[i] = gpiod_get(dev, gpio_info[i].name,
> +                       gpio_info[i].flags);
> +               if (IS_ERR(drvdata->gpio[i])) {
> +                       dev_err(dev, "gpiod_get() on '%s' failed: %d\n",
> +                               gpio_info[i].name, (int)drvdata->gpio[i]);
> +                       r = 1;
> +               }
> +       }
> +
> +       if (r)
> +               return -EINVAL;
> +
> +       /* TODO: Should be get from DTS? */
> +       ath79_gpio_base = ioremap(AR71XX_GPIO_BASE, AR71XX_GPIO_SIZE);
> +
> +       drvdata->dev = &pdev->dev;
> +
> +       drvdata->chip.priv = drvdata;
> +
> +       drvdata->chip.legacy.cmd_ctrl = rb91x_nand_cmd_ctrl;
> +       drvdata->chip.legacy.dev_ready = rb91x_nand_dev_ready;
> +       drvdata->chip.legacy.read_byte = rb91x_nand_read_byte;
> +       drvdata->chip.legacy.write_buf = rb91x_nand_write_buf;
> +       drvdata->chip.legacy.read_buf = rb91x_nand_read_buf;
> +
> +       drvdata->chip.legacy.chip_delay = 25;
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,9,0)
> +       drvdata->chip.ecc.engine_type      = NAND_ECC_ENGINE_TYPE_SOFT;
> +       drvdata->chip.ecc.algo             = NAND_ECC_ALGO_HAMMING;
> +#else
> +       drvdata->chip.ecc.mode             = NAND_ECC_SOFT;
> +       drvdata->chip.ecc.algo             = NAND_ECC_HAMMING;
> +#endif
> +       drvdata->chip.options = NAND_NO_SUBPAGE_WRITE;
> +
> +       mtd = rb91x_nand_drvdata_to_mtd(drvdata);
> +       mtd->owner = THIS_MODULE;
> +       r = nand_scan(&drvdata->chip, 1);
> +       if (r) {
> +               dev_err(dev, "nand_scan() failed: %d\n", r);
> +               return r;
> +       }
> +
> +       if (mtd->writesize == 512)
> +               mtd_set_ooblayout(mtd, &rb91x_nand_ecclayout_ops);
> +
> +       r = mtd_device_register(mtd, rb91x_nand_partitions,
> +                               ARRAY_SIZE(rb91x_nand_partitions));
> +       if (r) {
> +               dev_err(dev, "mtd_device_register() failed: %d\n",
> +                       r);
> +               goto err_release_nand;
> +       }
> +
> +       return 0;
> +
> +err_release_nand:
> +       rb91x_nand_release(drvdata);
> +       return r;
> +}
> +
> +static int rb91x_nand_remove(struct platform_device *pdev)
> +{
> +       struct rb91x_nand_drvdata *drvdata = platform_get_drvdata(pdev);
> +
> +       rb91x_nand_release(drvdata);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id rb91x_nand_match[] = {
> +       { .compatible = "rb91x-nand" },

Should this be "mikrotik,rb91x-nand"?

> +       {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, rb91x_nand_match);
> +
> +static struct platform_driver rb91x_nand_driver = {
> +       .probe  = rb91x_nand_probe,
> +       .remove = rb91x_nand_remove,
> +       .driver = {
> +               .name   = RB91X_NAND_DRIVER_NAME,
> +               .owner  = THIS_MODULE,
> +               .of_match_table = rb91x_nand_match,
> +       },
> +};
> +
> +module_platform_driver(rb91x_nand_driver);
> +
> +MODULE_DESCRIPTION(RB91X_NAND_DRIVER_DESC);
> +MODULE_VERSION(DRV_VERSION);
> +MODULE_AUTHOR("Gabor Juhos <juhosg at openwrt.org>");
> +MODULE_AUTHOR("Denis Kalashnikov <denis281089 at gmail.com>");
> +MODULE_LICENSE("GPL v2");

--
Sergey



More information about the openwrt-devel mailing list