diff mbox series

[v5,19/39] mtd: rawnand: add support for ts72xx

Message ID 20231122-ep93xx-v5-19-d59a76d5df29@maquefel.me
State New
Headers show
Series ep93xx device tree conversion | expand

Commit Message

Nikita Shubin via B4 Relay Nov. 22, 2023, 8:59 a.m. UTC
From: Nikita Shubin <nikita.shubin@maquefel.me>

Technologic Systems has it's own nand controller implementation in CPLD.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 drivers/mtd/nand/raw/Kconfig                       |   7 +
 drivers/mtd/nand/raw/Makefile                      |   1 +
 drivers/mtd/nand/raw/technologic-nand-controller.c | 223 +++++++++++++++++++++
 3 files changed, 231 insertions(+)

Comments

Andy Shevchenko Nov. 22, 2023, 12:24 p.m. UTC | #1
On Wed, Nov 22, 2023 at 11:59:57AM +0300, Nikita Shubin wrote:
> Technologic Systems has it's own nand controller implementation in CPLD.

...

> +static int ts72xx_nand_attach_chip(struct nand_chip *chip)
> +{
> +	switch (chip->ecc.engine_type) {
> +	case NAND_ECC_ENGINE_TYPE_SOFT:
> +		if (chip->ecc.algo == NAND_ECC_ALGO_UNKNOWN)
> +			chip->ecc.algo = NAND_ECC_ALGO_HAMMING;
> +		chip->ecc.algo = NAND_ECC_ALGO_HAMMING;
> +		break;
> +	case NAND_ECC_ENGINE_TYPE_ON_HOST:
> +		return -EINVAL;

> +	default:
> +		break;
> +	}
> +
> +	return 0;

Move this to default.

> +}

...

> +		for (i = 0; i < instr->ctx.addr.naddrs; i++)
> +			iowrite8(instr->ctx.addr.addrs[i], data->base);

iowrite8_rep() ?

> +	case NAND_OP_DATA_IN_INSTR:
> +		ioread8_rep(data->base, instr->ctx.data.buf.in, instr->ctx.data.len);

Hehe, you are even using it...

...

> +	if (instr->delay_ns)

What will happen if you drop this check?

> +		ndelay(instr->delay_ns);

...

> +	int ret;
> +
> +	ret = mtd_device_unregister(nand_to_mtd(chip));
> +	WARN_ON(ret);

Is this a requirement by MTD to have return value being checked?
Miquel Raynal Nov. 22, 2023, 1:08 p.m. UTC | #2
Hi Andy,

> > +	int ret;
> > +
> > +	ret = mtd_device_unregister(nand_to_mtd(chip));
> > +	WARN_ON(ret);  
> 
> Is this a requirement by MTD to have return value being checked?

Yes, for now this is the preferred way, with the hope some day to turn
the return value into void.

Thanks,
Miquèl
Nikita Shubin Dec. 8, 2023, 11:24 a.m. UTC | #3
Hello Andy!

On Wed, 2023-11-22 at 14:24 +0200, Andy Shevchenko wrote:
> On Wed, Nov 22, 2023 at 11:59:57AM +0300, Nikita Shubin wrote:
> > Technologic Systems has it's own nand controller implementation in
> > CPLD.
> 
> ...
> 
> > +static int ts72xx_nand_attach_chip(struct nand_chip *chip)
> > +{
> > +       switch (chip->ecc.engine_type) {
> > +       case NAND_ECC_ENGINE_TYPE_SOFT:
> > +               if (chip->ecc.algo == NAND_ECC_ALGO_UNKNOWN)
> > +                       chip->ecc.algo = NAND_ECC_ALGO_HAMMING;
> > +               chip->ecc.algo = NAND_ECC_ALGO_HAMMING;
> > +               break;
> > +       case NAND_ECC_ENGINE_TYPE_ON_HOST:
> > +               return -EINVAL;
> 
> > +       default:
> > +               break;
> > +       }
> > +
> > +       return 0;
> 
> Move this to default.
> 
> > +}
> 
> ...
> 
> > +               for (i = 0; i < instr->ctx.addr.naddrs; i++)
> > +                       iowrite8(instr->ctx.addr.addrs[i], data-
> > >base);
> 
> iowrite8_rep() ?
> 
> > +       case NAND_OP_DATA_IN_INSTR:
> > +               ioread8_rep(data->base, instr->ctx.data.buf.in,
> > instr->ctx.data.len);
> 
> Hehe, you are even using it...
> 
> ...
> 
> > +       if (instr->delay_ns)
> 
> What will happen if you drop this check?
> 
> > +               ndelay(instr->delay_ns);

No idea! I was asked to keep it by Miquel:

https://lore.kernel.org/lkml/8bbe66a23eb5c8a2404b72d754b1bcb6f4d23867.camel@maquefel.me/T/

But it looks some one can at least still do some calculation like:

https://elixir.bootlin.com/linux/latest/source/include/linux/delay.h#L50

At worst try to divide something.

Do you think ndelay(instr->delay_ns) is safe enough ?


> 
> ...
> 
> > +       int ret;
> > +
> > +       ret = mtd_device_unregister(nand_to_mtd(chip));
> > +       WARN_ON(ret);
> 
> Is this a requirement by MTD to have return value being checked?
> 

Aswered by Miquel.
Miquel Raynal Dec. 8, 2023, 11:33 a.m. UTC | #4
Hi Nikita,

> > > +               for (i = 0; i < instr->ctx.addr.naddrs; i++)
> > > +                       iowrite8(instr->ctx.addr.addrs[i], data-  
> > > >base);  
> > 
> > iowrite8_rep() ?
> >   
> > > +       case NAND_OP_DATA_IN_INSTR:
> > > +               ioread8_rep(data->base, instr->ctx.data.buf.in,
> > > instr->ctx.data.len);  
> > 
> > Hehe, you are even using it...
> > 
> > ...
> >   
> > > +       if (instr->delay_ns)  
> > 
> > What will happen if you drop this check?
> >   
> > > +               ndelay(instr->delay_ns);  
> 
> No idea! I was asked to keep it by Miquel:

Your controller is very simple and just queues whatever command you
ask, precisely when you ask it to do so. But the NAND bus is a bit more
complex and there are minimum delays between certain instructions. This
delay is meant to respect that. Sometimes it will be 0, and sometimes
not. It depends what NAND op you do. You can check the value of these
delays in the core.

Thanks,
Miquèl
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index cbf8ae85e1ae..3937c10dea1c 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -449,6 +449,13 @@  config MTD_NAND_RENESAS
 	  Enables support for the NAND controller found on Renesas R-Car
 	  Gen3 and RZ/N1 SoC families.
 
+config MTD_NAND_TS72XX
+	tristate "ts72xx NAND controller"
+	depends on ARCH_EP93XX && HAS_IOMEM
+	help
+	  Enables support for NAND controller on ts72xx SBCs.
+	  This is a legacy driver based on gen_nand.
+
 comment "Misc"
 
 config MTD_SM_COMMON
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index 25120a4afada..d0b0e6b83568 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -34,6 +34,7 @@  obj-$(CONFIG_MTD_NAND_MLC_LPC32XX)      += lpc32xx_mlc.o
 obj-$(CONFIG_MTD_NAND_SH_FLCTL)		+= sh_flctl.o
 obj-$(CONFIG_MTD_NAND_MXC)		+= mxc_nand.o
 obj-$(CONFIG_MTD_NAND_SOCRATES)		+= socrates_nand.o
+obj-$(CONFIG_MTD_NAND_TS72XX)           += technologic-nand-controller.o
 obj-$(CONFIG_MTD_NAND_TXX9NDFMC)	+= txx9ndfmc.o
 obj-$(CONFIG_MTD_NAND_MPC5121_NFC)	+= mpc5121_nfc.o
 obj-$(CONFIG_MTD_NAND_VF610_NFC)	+= vf610_nfc.o
diff --git a/drivers/mtd/nand/raw/technologic-nand-controller.c b/drivers/mtd/nand/raw/technologic-nand-controller.c
new file mode 100644
index 000000000000..0b4c5c2dee0b
--- /dev/null
+++ b/drivers/mtd/nand/raw/technologic-nand-controller.c
@@ -0,0 +1,223 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Technologic Systems TS72xx NAND controller driver
+ *
+ * Copyright (C) 2023 Nikita Shubin <nikita.shubin@maquefel.me>
+ *
+ * Derived from: plat_nand.c
+ *  Author: Vitaly Wool <vitalywool@gmail.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/platnand.h>
+
+#define TS72XX_NAND_CONTROL_ADDR_LINE	BIT(22)	/* 0xN0400000 */
+#define TS72XX_NAND_BUSY_ADDR_LINE	BIT(23)	/* 0xN0800000 */
+
+#define TS72XX_NAND_ALE			BIT(0)
+#define TS72XX_NAND_CLE			BIT(1)
+#define TS72XX_NAND_NCE			BIT(2)
+
+#define TS72XX_NAND_CTRL_CLE		(TS72XX_NAND_NCE | TS72XX_NAND_CLE)
+#define TS72XX_NAND_CTRL_ALE		(TS72XX_NAND_NCE | TS72XX_NAND_ALE)
+
+struct ts72xx_nand_data {
+	struct nand_controller	controller;
+	struct nand_chip	chip;
+	void __iomem		*base;
+	void __iomem		*ctrl;
+	void __iomem		*busy;
+};
+
+static inline struct ts72xx_nand_data *chip_to_ts72xx(struct nand_chip *chip)
+{
+	return container_of(chip, struct ts72xx_nand_data, chip);
+}
+
+static int ts72xx_nand_attach_chip(struct nand_chip *chip)
+{
+	switch (chip->ecc.engine_type) {
+	case NAND_ECC_ENGINE_TYPE_SOFT:
+		if (chip->ecc.algo == NAND_ECC_ALGO_UNKNOWN)
+			chip->ecc.algo = NAND_ECC_ALGO_HAMMING;
+		chip->ecc.algo = NAND_ECC_ALGO_HAMMING;
+		break;
+	case NAND_ECC_ENGINE_TYPE_ON_HOST:
+		return -EINVAL;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static void ts72xx_nand_ctrl(struct nand_chip *chip, u8 value)
+{
+	struct ts72xx_nand_data *data = chip_to_ts72xx(chip);
+	unsigned char bits = ioread8(data->ctrl) & ~GENMASK(2, 0);
+
+	iowrite8(bits | value, data->ctrl);
+}
+
+static int ts72xx_nand_exec_instr(struct nand_chip *chip,
+				const struct nand_op_instr *instr)
+{
+	struct ts72xx_nand_data *data = chip_to_ts72xx(chip);
+	unsigned int i, timeout_us;
+	u32 status;
+	int ret;
+
+	switch (instr->type) {
+	case NAND_OP_CMD_INSTR:
+		ts72xx_nand_ctrl(chip, TS72XX_NAND_CTRL_CLE);
+		iowrite8(instr->ctx.cmd.opcode, data->base);
+		ts72xx_nand_ctrl(chip, TS72XX_NAND_NCE);
+		break;
+
+	case NAND_OP_ADDR_INSTR:
+		ts72xx_nand_ctrl(chip, TS72XX_NAND_CTRL_ALE);
+		for (i = 0; i < instr->ctx.addr.naddrs; i++)
+			iowrite8(instr->ctx.addr.addrs[i], data->base);
+		ts72xx_nand_ctrl(chip, TS72XX_NAND_NCE);
+		break;
+
+	case NAND_OP_DATA_IN_INSTR:
+		ioread8_rep(data->base, instr->ctx.data.buf.in, instr->ctx.data.len);
+		break;
+
+	case NAND_OP_DATA_OUT_INSTR:
+		iowrite8_rep(data->base, instr->ctx.data.buf.in, instr->ctx.data.len);
+		break;
+
+	case NAND_OP_WAITRDY_INSTR:
+		timeout_us = instr->ctx.waitrdy.timeout_ms * 1000;
+		ret = readb_poll_timeout(data->busy, status, status & BIT(5), 0, timeout_us);
+		if (ret)
+			return ret;
+
+		break;
+	}
+
+	if (instr->delay_ns)
+		ndelay(instr->delay_ns);
+
+	return 0;
+}
+
+static int ts72xx_nand_exec_op(struct nand_chip *chip,
+			       const struct nand_operation *op, bool check_only)
+{
+	unsigned int i;
+	int ret;
+
+	if (check_only)
+		return 0;
+
+	for (i = 0; i < op->ninstrs; i++) {
+		ret = ts72xx_nand_exec_instr(chip, &op->instrs[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static const struct nand_controller_ops ts72xx_nand_ops = {
+	.attach_chip = ts72xx_nand_attach_chip,
+	.exec_op = ts72xx_nand_exec_op,
+};
+
+static int ts72xx_nand_probe(struct platform_device *pdev)
+{
+	struct ts72xx_nand_data *data;
+	struct fwnode_handle *child;
+	struct mtd_info *mtd;
+	int err;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	nand_controller_init(&data->controller);
+	data->controller.ops = &ts72xx_nand_ops;
+	data->chip.controller = &data->controller;
+
+	data->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(data->base))
+		return PTR_ERR(data->base);
+	data->ctrl = data->base + TS72XX_NAND_CONTROL_ADDR_LINE;
+	data->busy = data->base + TS72XX_NAND_BUSY_ADDR_LINE;
+
+	child = fwnode_get_next_child_node(dev_fwnode(&pdev->dev), NULL);
+	if (!child)
+		return dev_err_probe(&pdev->dev, -ENXIO,
+				"ts72xx controller node should have exactly one child\n");
+
+	nand_set_flash_node(&data->chip, to_of_node(child));
+	mtd = nand_to_mtd(&data->chip);
+	mtd->dev.parent = &pdev->dev;
+	platform_set_drvdata(pdev, data);
+
+	/*
+	 * This driver assumes that the default ECC engine should be TYPE_SOFT.
+	 * Set ->engine_type before registering the NAND devices in order to
+	 * provide a driver specific default value.
+	 */
+	data->chip.ecc.engine_type = NAND_ECC_ENGINE_TYPE_SOFT;
+
+	/* Scan to find existence of the device */
+	err = nand_scan(&data->chip, 1);
+	if (err)
+		goto err_handle_put;
+
+	err = mtd_device_parse_register(mtd, NULL, NULL, NULL, 0);
+	if (err)
+		goto err_clean_nand;
+
+	return 0;
+
+err_clean_nand:
+	nand_cleanup(&data->chip);
+err_handle_put:
+	fwnode_handle_put(child);
+	return err;
+}
+
+static void ts72xx_nand_remove(struct platform_device *pdev)
+{
+	struct ts72xx_nand_data *data = platform_get_drvdata(pdev);
+	struct nand_chip *chip = &data->chip;
+	int ret;
+
+	ret = mtd_device_unregister(nand_to_mtd(chip));
+	WARN_ON(ret);
+	nand_cleanup(chip);
+}
+
+static const struct of_device_id ts72xx_id_table[] = {
+	{ .compatible = "technologic,ts7200-nand" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ts72xx_id_table);
+
+static struct platform_driver ts72xx_nand_driver = {
+	.driver = {
+		.name = "ts72xx-nand",
+		.of_match_table = ts72xx_id_table,
+	},
+	.probe = ts72xx_nand_probe,
+	.remove_new = ts72xx_nand_remove,
+};
+module_platform_driver(ts72xx_nand_driver);
+
+MODULE_AUTHOR("Nikita Shubin <nikita.shubin@maquefel.me>");
+MODULE_DESCRIPTION("Technologic Systems TS72xx NAND controller driver");
+MODULE_LICENSE("GPL");