diff mbox

[v1] mtd: nand: tango: import driver for tango controller

Message ID 57C94E33.6070304@sigmadesigns.com
State Superseded
Headers show

Commit Message

Marc Gonzalez Sept. 2, 2016, 10:02 a.m. UTC
This driver supports the NAND Flash controller embedded in recent
Tango chips, such as SMP8758 and SMP8759.

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
 drivers/mtd/nand/Kconfig      |   6 +
 drivers/mtd/nand/Makefile     |   1 +
 drivers/mtd/nand/tango_nand.c | 376 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 383 insertions(+)
 create mode 100644 drivers/mtd/nand/tango_nand.c

Comments

Boris Brezillon Sept. 5, 2016, 7:14 a.m. UTC | #1
On Fri, 2 Sep 2016 12:02:27 +0200
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> wrote:

> This driver supports the NAND Flash controller embedded in recent
> Tango chips, such as SMP8758 and SMP8759.
> 
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
>  drivers/mtd/nand/Kconfig      |   6 +
>  drivers/mtd/nand/Makefile     |   1 +
>  drivers/mtd/nand/tango_nand.c | 376 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 383 insertions(+)
>  create mode 100644 drivers/mtd/nand/tango_nand.c
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index f05e0e9eb2f7..22eb5457c9f7 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -205,6 +205,12 @@ config MTD_NAND_S3C2410_CLKSTOP
>  	  when the is NAND chip selected or released, but will save
>  	  approximately 5mA of power when there is nothing happening.
>  
> +config MTD_NAND_TANGO
> +	tristate "NAND Flash support for Tango chips"
> +	depends on ARCH_TANGO
> +	help
> +	  Enables the NAND Flash controller on Tango chips.
> +
>  config MTD_NAND_DISKONCHIP
>  	tristate "DiskOnChip 2000, Millennium and Millennium Plus (NAND reimplementation)"
>  	depends on HAS_IOMEM
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index f55335373f7c..647f727223ef 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_MTD_NAND_DENALI_DT)	+= denali_dt.o
>  obj-$(CONFIG_MTD_NAND_AU1550)		+= au1550nd.o
>  obj-$(CONFIG_MTD_NAND_BF5XX)		+= bf5xx_nand.o
>  obj-$(CONFIG_MTD_NAND_S3C2410)		+= s3c2410.o
> +obj-$(CONFIG_MTD_NAND_TANGO)		+= tango_nand.o
>  obj-$(CONFIG_MTD_NAND_DAVINCI)		+= davinci_nand.o
>  obj-$(CONFIG_MTD_NAND_DISKONCHIP)	+= diskonchip.o
>  obj-$(CONFIG_MTD_NAND_DOCG4)		+= docg4.o
> diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
> new file mode 100644
> index 000000000000..dfd27081d5ec
> --- /dev/null
> +++ b/drivers/mtd/nand/tango_nand.c
> @@ -0,0 +1,376 @@
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/mtd/nand.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/platform_device.h>
> +
> +/* Offsets relative to chip->base */
> +#define PBUS_CMD	0
> +#define PBUS_ADDR	4
> +#define PBUS_DATA	8
> +
> +/* Offsets relative to reg_base */
> +#define NFC_STATUS_REG	0x00
> +#define NFC_FLASH_CMD	0x04
> +#define NFC_DEVICE_CFG	0x08
> +#define NFC_TIMING1	0x0c
> +#define NFC_TIMING2	0x10
> +#define NFC_XFER_CFG	0x14
> +#define NFC_PKT_0_CFG	0x18
> +#define NFC_PKT_N_CFG	0x1c
> +#define NFC_BB_CFG	0x20
> +#define NFC_ADDR_PAGE	0x24
> +#define NFC_ADDR_OFFSET	0x28
> +#define NFC_XFER_STATUS	0x2c
> +
> +/* NFC_FLASH_CMD values */
> +#define NFC_READ	1
> +#define NFC_WRITE	2
> +
> +/* NFC_XFER_STATUS values */
> +#define PAGE_IS_EMPTY	BIT(16)
> +
> +/* Offsets relative to mem_base */
> +#define ERROR_REPORT	0x1c0
> +
> +/* ERROR_REPORT values */
> +#define DECODE_ERR_ON_PKT_0(v)	(~v & BIT(7))
> +#define DECODE_ERR_ON_PKT_N(v)	(~v & BIT(15))

Is this really packed N? I'd say it's packet 1.

How about:

#define DECODE_ERR_ON_PKT(pkt, v)	(~(v) & BIT(((pkt) * 8) + 7))

> +#define ERR_COUNT_PKT_0(v)	((v >> 0) & 0x3f)
> +#define ERR_COUNT_PKT_N(v)	((v >> 8) & 0x3f)

Ditto.

> +
> +/* Offsets relative to pbus_base */
> +#define PBUS_CS_CTRL	0x83c
> +#define PBUS_PAD_MODE	0x8f0
> +
> +/* PBUS_CS_CTRL values */
> +#define PBUS_IORDY	BIT(31)
> +
> +/* PBUS_PAD_MODE values */
> +#define MODE_RAW	0
> +#define MODE_MLC	BIT(31)
> +
> +#define METADATA_SIZE	4
> +#define BBM_SIZE	6
> +#define FIELD_ORDER	15
> +
> +#define MAX_CS		4
> +
> +struct tango_nfc {
> +	struct nand_hw_control hw;
> +	void __iomem *reg_base, *mem_base, *pbus_base;
> +	struct tango_chip *chips[MAX_CS];
> +	struct dma_chan *chan;
> +};
> +
> +#define to_tango_nfc(ptr) container_of(ptr, struct tango_nfc, hw)
> +
> +struct tango_chip {
> +	struct nand_chip chip;
> +	void __iomem *base;

I think it better to encode the CS id, and calculate the __iomem offset
based on that at run-time. Especially if you want to support multi-CS
(multi dies) chips, which you don't seem to support here.

> +	u32 timing1, timing2, xfer_cfg, pkt_0_cfg, pkt_n_cfg, bb_cfg;

Please, one field per line in struct definitions.

> +};
> +
> +#define to_tango_chip(ptr) container_of(ptr, struct tango_chip, chip)
> +
> +#define XFER_CFG(cs, page_count, steps, metadata_size)	\
> +	((cs) << 24 | (page_count) << 16 | (steps) << 8 | (metadata_size) << 0)
> +
> +#define PKT_CFG(size, strength) ((size) << 16 | (strength) << 0)
> +
> +#define BB_CFG(bb_offset, bb_size) ((bb_offset) << 16 | (bb_size) << 0)
> +
> +#define TIMING(t0, t1, t2, t3) (t0 << 24 | t1 << 16 | t2 << 8 | t3)
> +
> +#define NFC_BUSY(base) (readl_relaxed(base + NFC_STATUS_REG) & 0xf)

I'd turn that one into an inline function taking a nand_chip pointer in
parameter. Or drop it completely, since you only have one user.

And please document why you use this 0xf mask? I guess there's one bit
per CS, so masking with 0xf is not necessarily a good idea...

> +
> +static void tango_cmd_ctrl(struct mtd_info *mtd, int dat, unsigned int ctrl)
> +{
> +	struct tango_chip *chip = to_tango_chip(mtd_to_nand(mtd));
> +
> +	if (ctrl & NAND_CLE)
> +		writeb_relaxed(dat, chip->base + PBUS_CMD);
> +
> +	if (ctrl & NAND_ALE)
> +		writeb_relaxed(dat, chip->base + PBUS_ADDR);
> +}
> +
> +static int tango_dev_ready(struct mtd_info *mtd)
> +{
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
> +
> +	return readl_relaxed(nfc->pbus_base + PBUS_CS_CTRL) & PBUS_IORDY;
> +}
> +
> +static uint8_t tango_read_byte(struct mtd_info *mtd)
> +{
> +	struct tango_chip *chip = to_tango_chip(mtd_to_nand(mtd));
> +
> +	return readb_relaxed(chip->base + PBUS_DATA);
> +}
> +
> +static void tango_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> +{
> +	struct tango_chip *chip = to_tango_chip(mtd_to_nand(mtd));
> +
> +	ioread8_rep(chip->base + PBUS_DATA, buf, len);
> +}
> +
> +static int decode_error_report(struct tango_nfc *nfc)
> +{
> +	u32 status, res;
> +
> +	status = readl_relaxed(nfc->reg_base + NFC_XFER_STATUS);
> +	if (status & PAGE_IS_EMPTY)
> +		return 0;
> +
> +	res = readl_relaxed(nfc->mem_base + ERROR_REPORT);
> +
> +	if (DECODE_ERR_ON_PKT_0(res) || DECODE_ERR_ON_PKT_N(res))
> +		return -EBADMSG;

Hm, you're assuming you'll always have 2 packets in your layout, is
this true? Don't you have layouts where you only have one packet (2k
pages with 2k packets) ?

> +
> +	return max(ERR_COUNT_PKT_0(res), ERR_COUNT_PKT_N(res));
> +}
> +
> +static void tango_dma_callback(void *arg)
> +{
> +	complete(arg);
> +}
> +
> +static int do_dma(struct nand_chip *nand, int dir, int cmd, void *buf, int len, int page)
> +{
> +	struct tango_nfc *nfc = to_tango_nfc(nand->controller);
> +	struct tango_chip *chip = to_tango_chip(nand);
> +	struct dma_async_tx_descriptor *desc;
> +	struct scatterlist sg;
> +	struct completion tx_done;
> +	int err = -EIO;
> +
> +	sg_init_one(&sg, buf, len);
> +	if (dma_map_sg(nfc->chan->device->dev, &sg, 1, dir) != 1)
> +		goto leave;
> +
> +	desc = dmaengine_prep_slave_sg(nfc->chan, &sg, 1, dir, DMA_PREP_INTERRUPT);
> +	if (!desc)
> +		goto dma_unmap;
> +
> +	desc->callback = tango_dma_callback;
> +	desc->callback_param = &tx_done;
> +	init_completion(&tx_done);
> +
> +	writel_relaxed(MODE_MLC, nfc->pbus_base + PBUS_PAD_MODE);

Not sure I understand what MLC means here? Can you give more detail?

> +
> +	err = 0;
> +	dmaengine_submit(desc);
> +	dma_async_issue_pending(nfc->chan);
> +
> +	writel_relaxed(chip->timing1,	nfc->reg_base + NFC_TIMING1);
> +	writel_relaxed(chip->timing2,	nfc->reg_base + NFC_TIMING2);
> +	writel_relaxed(chip->xfer_cfg,	nfc->reg_base + NFC_XFER_CFG);
> +	writel_relaxed(chip->pkt_0_cfg,	nfc->reg_base + NFC_PKT_0_CFG);
> +	writel_relaxed(chip->pkt_n_cfg,	nfc->reg_base + NFC_PKT_N_CFG);
> +	writel_relaxed(chip->bb_cfg,	nfc->reg_base + NFC_BB_CFG);
> +	writel_relaxed(page, nfc->reg_base + NFC_ADDR_PAGE);
> +	writel_relaxed(0, nfc->reg_base + NFC_ADDR_OFFSET);
> +	writel(cmd, nfc->reg_base + NFC_FLASH_CMD);
> +
> +	wait_for_completion(&tx_done);
> +
> +	while (NFC_BUSY(nfc->reg_base))
> +		cpu_relax();
> +
> +	writel_relaxed(MODE_RAW, nfc->pbus_base + PBUS_PAD_MODE);
> +dma_unmap:
> +	dma_unmap_sg(nfc->chan->device->dev, &sg, 1, dir);
> +leave:
> +	return err;
> +}
> +
> +static int tango_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> +			uint8_t *buf, int oob_required, int page)
> +{
> +	int err, len = mtd->writesize;
> +
> +	err = do_dma(chip, DMA_FROM_DEVICE, NFC_READ, buf, len, page);
> +	if (err)
> +		return err;
> +
> +	return decode_error_report(to_tango_nfc(chip->controller));
> +}
> +
> +static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> +			const uint8_t *buf, int oob_required, int page)
> +{
> +	int len = mtd->writesize;
> +
> +	return do_dma(chip, DMA_TO_DEVICE, NFC_WRITE, (void *)buf, len, page);
> +}
> +
> +static u32 to_ticks(int kHz, int ps)
> +{
> +	return DIV_ROUND_UP_ULL((u64)kHz * ps, NSEC_PER_SEC);
> +}
> +
> +static int set_timings(struct tango_chip *p, int kHz)
> +{
> +	u32 Trdy, Textw, Twc, Twpw, Tacc, Thold, Trpw, Textr;
> +	const struct nand_sdr_timings *sdr;
> +	int mode = onfi_get_async_timing_mode(&p->chip);
> +
> +	if (mode & ONFI_TIMING_MODE_UNKNOWN)
> +		return -ENODEV;
> +
> +	sdr = onfi_async_timing_mode_to_sdr_timings(fls(mode) - 1);
> +
> +	Trdy	= to_ticks(kHz, sdr->tCEA_max - sdr->tREA_max);
> +	Textw	= to_ticks(kHz, sdr->tWB_max);
> +	Twc	= to_ticks(kHz, sdr->tWC_min);
> +	Twpw	= to_ticks(kHz, sdr->tWC_min - sdr->tWP_min);
> +
> +	Tacc	= to_ticks(kHz, sdr->tREA_max);
> +	Thold	= to_ticks(kHz, sdr->tREH_min);
> +	Trpw	= to_ticks(kHz, sdr->tRC_min - sdr->tREH_min);
> +	Textr	= to_ticks(kHz, sdr->tRHZ_max);
> +
> +	p->timing1 = TIMING(Trdy, Textw, Twc, Twpw);
> +	p->timing2 = TIMING(Tacc, Thold, Trpw, Textr);
> +
> +	return 0;
> +}
> +
> +static int chip_init(struct device *dev, struct device_node *np, int kHz)
> +{
> +	int err;
> +	u32 cs, ecc_bits;
> +	struct nand_ecc_ctrl *ecc;
> +	struct tango_nfc *nfc = dev_get_drvdata(dev);
> +	struct tango_chip *p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL);
> +	if (!p)
> +		return -ENOMEM;
> +
> +	err = of_property_read_u32_index(np, "reg", 0, &cs);
> +	if (err)
> +		return err;
> +
> +	if (cs >= MAX_CS)
> +		return -ERANGE;
> +
> +	p->chip.read_byte	= tango_read_byte;
> +	p->chip.read_buf	= tango_read_buf;
> +	p->chip.cmd_ctrl	= tango_cmd_ctrl;
> +	p->chip.dev_ready	= tango_dev_ready;
> +	/* p->chip.options	= NAND_MAGIC_FOO */
> +	p->chip.controller	= &nfc->hw;
> +	p->base			= nfc->pbus_base + (cs * 256);
> +
> +	ecc		= &p->chip.ecc;
> +	ecc->mode	= NAND_ECC_HW;
> +	ecc->algo	= NAND_ECC_BCH;
> +	ecc->read_page	= tango_read_page;
> +	ecc->write_page	= tango_write_page;
> +
> +	nand_set_flash_node(&p->chip, np);
> +	err = nand_scan(&p->chip.mtd, 1);
> +	if (err)
> +		return err;
> +
> +	ecc_bits = p->chip.ecc.strength * FIELD_ORDER;

Are you sure the field order is always 15? I thought the ECC controller
was adapting it depending on the packet size (512 bytes packets => 13,
1024 bytes => 14, 2k => 15), but maybe I'm wrong.

> +	p->chip.ecc.bytes = DIV_ROUND_UP(ecc_bits, 8);
> +	set_timings(p, kHz);
> +
> +	err = mtd_device_register(&p->chip.mtd, NULL, 0);
> +	if (err)
> +		return err;
> +
> +	nfc->chips[cs] = p;
> +
> +	p->xfer_cfg	= XFER_CFG(cs, 1, ecc->steps, METADATA_SIZE);
> +	p->pkt_0_cfg	= PKT_CFG(ecc->size + METADATA_SIZE, ecc->strength);
> +	p->pkt_n_cfg	= PKT_CFG(ecc->size, ecc->strength);
> +	p->bb_cfg	= BB_CFG(p->chip.mtd.writesize, BBM_SIZE);
> +
> +	return 0;
> +}
> +
> +static int tango_nand_probe(struct platform_device *pdev)
> +{
> +	int i, kHz;
> +	struct resource *res;
> +	void __iomem *addr[3];
> +	struct tango_nfc *nfc;
> +	struct device_node *np;
> +
> +	struct clk *clk = clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
> +	if (!nfc)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < 3; ++i) {
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> +		addr[i] = devm_ioremap_resource(&pdev->dev, res);
> +		if (IS_ERR(addr[i]))
> +			return PTR_ERR(addr[i]);
> +	}
> +
> +	platform_set_drvdata(pdev, nfc);
> +	nand_hw_control_init(&nfc->hw);
> +	kHz = clk_get_rate(clk) / 1000;
> +
> +	nfc->reg_base	= addr[0];
> +	nfc->mem_base	= addr[1];
> +	nfc->pbus_base	= addr[2];

Why not doing

	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
	nfc->reg_base = devm_ioremap_resource(&pdev->dev, res);

	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
	nfc->mem_base = devm_ioremap_resource(&pdev->dev, res);
...

> +
> +	for_each_child_of_node(pdev->dev.of_node, np) {
> +		int err = chip_init(&pdev->dev, np, kHz);
> +		if (err)
> +			return err;
> +	}
> +
> +	nfc->chan = dma_request_chan(&pdev->dev, "mlc_flash_0");
> +	if (IS_ERR(nfc->chan))
> +		return PTR_ERR(nfc->chan);
> +
> +	/* TODO: tweak (?) Peripheral Bus setup (timings and CS config) */
> +
> +	return 0;
> +}
> +
> +static int tango_nand_remove(struct platform_device *pdev)
> +{
> +	int cs;
> +	struct tango_nfc *nfc = platform_get_drvdata(pdev);
> +
> +	dma_release_channel(nfc->chan);
> +	for (cs = 0; cs < MAX_CS; ++cs)
> +		if (nfc->chips[cs] != NULL)
> +			nand_release(&nfc->chips[cs]->chip.mtd);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id tango_nand_ids[] = {
> +	{ .compatible = "sigma,smp8758-nand" },
> +	{ /* sentinel */ }
> +};
> +
> +static struct platform_driver tango_nand_driver = {
> +	.probe	= tango_nand_probe,
> +	.remove	= tango_nand_remove,
> +	.driver	= {
> +		.name		= "tango-nand",
> +		.of_match_table	= tango_nand_ids,
> +	},
> +};
> +
> +module_platform_driver(tango_nand_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Sigma Designs");
> +MODULE_DESCRIPTION("Tango4 NAND Flash controller driver");
Mason Sept. 5, 2016, 10:18 a.m. UTC | #2
On 05/09/2016 09:14, Boris Brezillon wrote:

> Marc Gonzalez wrote:
> 
>> +/* ERROR_REPORT values */
>> +#define DECODE_ERR_ON_PKT_0(v)	(~v & BIT(7))
>> +#define DECODE_ERR_ON_PKT_N(v)	(~v & BIT(15))
> 
> Is this really packet N? I'd say it's packet 1.

NB: "packet_0" and "packet_n" are terms used in the controller's
documentation (which is not publicly available AFAIU).

I didn't want to change the names, for fear of confusing a
future (internal) maintainer of the code. But in my mind,
ERR_ON_FIRST_PKT and ERR_ON_OTHER_PKT are clearer. Perhaps
I can use these identifiers, with a comment mentioning the
"internal" names.

DECODE_ERR_ON_PKT_N(v) actually means:
"decode error on packet N, for N > 0"
(The HW supports splitting a page in 1 to 16 packets.)

> #define DECODE_ERR_ON_PKT(pkt, v)	(~(v) & BIT(((pkt) * 8) + 7))

Byte 0 is the report for packet 0.
Byte 1 is the report for other packets.

>> +#define ERR_COUNT_PKT_0(v)	((v >> 0) & 0x3f)
>> +#define ERR_COUNT_PKT_N(v)	((v >> 8) & 0x3f)

There are only two bytes in the error report.
ERR_COUNT_PKT_N(v) returns the max error count for N > 1

>> +struct tango_chip {
>> +	struct nand_chip chip;
>> +	void __iomem *base;
> 
> I think it better to encode the CS id, and calculate the __iomem offset
> based on that at run-time. Especially if you want to support multi-CS
> (multi dies) chips, which you don't seem to support here.

To support multi-CS chips, I would have to define a
select_chip callback, where I save the requested CS
inside the struct tango_chip and use that to compute
the offset later?


>> +	u32 timing1, timing2, xfer_cfg, pkt_0_cfg, pkt_n_cfg, bb_cfg;
> 
> Please, one field per line in struct definitions.

OK.


>> +#define NFC_BUSY(base) (readl_relaxed(base + NFC_STATUS_REG) & 0xf)
> 
> I'd turn that one into an inline function taking a nand_chip pointer in
> parameter. Or drop it completely, since you only have one user.

OK.


> And please document why you use this 0xf mask? I guess there's one bit
> per CS, so masking with 0xf is not necessarily a good idea...

There's a 4-bit status code, 0 means idle, non-0 means busy.
I'll document the mask.


>> +	res = readl_relaxed(nfc->mem_base + ERROR_REPORT);
>> +
>> +	if (DECODE_ERR_ON_PKT_0(res) || DECODE_ERR_ON_PKT_N(res))
>> +		return -EBADMSG;
> 
> Hm, you're assuming you'll always have 2 packets in your layout, is
> this true? Don't you have layouts where you only have one packet (2k
> pages with 2k packets) ?

The legacy driver hard-codes packet size to 1024.
Thus 2 packets for 2k pages, 4 packets for 4k pages.
Are there recent NAND chips with 1k pages?


>> +	writel_relaxed(MODE_MLC, nfc->pbus_base + PBUS_PAD_MODE);
> 
> Not sure I understand what MLC means here? Can you give more detail?

I'll rename it to MODE_NFC (for "NAND Flash Controller").

Basically, either we access NAND chips directly in "raw" mode,
or we go through the NAND Flash Controller.

My driver uses "raw" mode for everything except read_page and
write_page (because they require DMA and HW ECC).


>> +	ecc_bits = p->chip.ecc.strength * FIELD_ORDER;
> 
> Are you sure the field order is always 15? I thought the ECC controller
> was adapting it depending on the packet size (512 bytes packets => 13,
> 1024 bytes => 14, 2k => 15), but maybe I'm wrong.

If I'm reading the doc right, it's always 15.


>> +static int tango_nand_probe(struct platform_device *pdev)
>> +{
>> +	int i, kHz;
>> +	struct resource *res;
>> +	void __iomem *addr[3];
>> +	struct tango_nfc *nfc;
>> +	struct device_node *np;
>> +
>> +	struct clk *clk = clk_get(&pdev->dev, NULL);
>> +	if (IS_ERR(clk))
>> +		return PTR_ERR(clk);
>> +
>> +	nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
>> +	if (!nfc)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0; i < 3; ++i) {
>> +		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
>> +		addr[i] = devm_ioremap_resource(&pdev->dev, res);
>> +		if (IS_ERR(addr[i]))
>> +			return PTR_ERR(addr[i]);
>> +	}
>> +
>> +	platform_set_drvdata(pdev, nfc);
>> +	nand_hw_control_init(&nfc->hw);
>> +	kHz = clk_get_rate(clk) / 1000;
>> +
>> +	nfc->reg_base	= addr[0];
>> +	nfc->mem_base	= addr[1];
>> +	nfc->pbus_base	= addr[2];
> 
> Why not doing
> 
> 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> 	nfc->reg_base = devm_ioremap_resource(&pdev->dev, res);
> 
> 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> 	nfc->mem_base = devm_ioremap_resource(&pdev->dev, res);

Do you mean why do I have a loop for the 3 ioremaps?
IIUC, you'd prefer that I unroll the loop?

Regards.
Boris Brezillon Sept. 5, 2016, 11:15 a.m. UTC | #3
On Mon, 5 Sep 2016 12:18:26 +0200
Mason <slash.tmp@free.fr> wrote:

> On 05/09/2016 09:14, Boris Brezillon wrote:
> 
> > Marc Gonzalez wrote:
> >   
> >> +/* ERROR_REPORT values */
> >> +#define DECODE_ERR_ON_PKT_0(v)	(~v & BIT(7))
> >> +#define DECODE_ERR_ON_PKT_N(v)	(~v & BIT(15))  
> > 
> > Is this really packet N? I'd say it's packet 1.  
> 
> NB: "packet_0" and "packet_n" are terms used in the controller's
> documentation (which is not publicly available AFAIU).
> 
> I didn't want to change the names, for fear of confusing a
> future (internal) maintainer of the code. But in my mind,
> ERR_ON_FIRST_PKT and ERR_ON_OTHER_PKT are clearer. Perhaps
> I can use these identifiers, with a comment mentioning the
> "internal" names.

Or do it the other way around: keep the datasheet wording and add a
comment explaining what it means in your code.

> 
> DECODE_ERR_ON_PKT_N(v) actually means:
> "decode error on packet N, for N > 0"
> (The HW supports splitting a page in 1 to 16 packets.)
> 
> > #define DECODE_ERR_ON_PKT(pkt, v)	(~(v) & BIT(((pkt) * 8) + 7))  
> 
> Byte 0 is the report for packet 0.
> Byte 1 is the report for other packets.
> 
> >> +#define ERR_COUNT_PKT_0(v)	((v >> 0) & 0x3f)
> >> +#define ERR_COUNT_PKT_N(v)	((v >> 8) & 0x3f)  
> 
> There are only two bytes in the error report.
> ERR_COUNT_PKT_N(v) returns the max error count for N > 1

Okay, then again, explain that in a comment.

> 
> >> +struct tango_chip {
> >> +	struct nand_chip chip;
> >> +	void __iomem *base;  
> > 
> > I think it better to encode the CS id, and calculate the __iomem offset
> > based on that at run-time. Especially if you want to support multi-CS
> > (multi dies) chips, which you don't seem to support here.  
> 
> To support multi-CS chips, I would have to define a
> select_chip callback, where I save the requested CS
> inside the struct tango_chip and use that to compute
> the offset later?

Yes, that's a solution.

> 
> 
> >> +	u32 timing1, timing2, xfer_cfg, pkt_0_cfg, pkt_n_cfg, bb_cfg;  
> > 
> > Please, one field per line in struct definitions.  
> 
> OK.
> 
> 
> >> +#define NFC_BUSY(base) (readl_relaxed(base + NFC_STATUS_REG) & 0xf)  
> > 
> > I'd turn that one into an inline function taking a nand_chip pointer in
> > parameter. Or drop it completely, since you only have one user.  
> 
> OK.
> 
> 
> > And please document why you use this 0xf mask? I guess there's one bit
> > per CS, so masking with 0xf is not necessarily a good idea...  
> 
> There's a 4-bit status code, 0 means idle, non-0 means busy.
> I'll document the mask.
> 
> 
> >> +	res = readl_relaxed(nfc->mem_base + ERROR_REPORT);
> >> +
> >> +	if (DECODE_ERR_ON_PKT_0(res) || DECODE_ERR_ON_PKT_N(res))
> >> +		return -EBADMSG;  
> > 
> > Hm, you're assuming you'll always have 2 packets in your layout, is
> > this true? Don't you have layouts where you only have one packet (2k
> > pages with 2k packets) ?  
> 
> The legacy driver hard-codes packet size to 1024.
> Thus 2 packets for 2k pages, 4 packets for 4k pages.
> Are there recent NAND chips with 1k pages?

Nope. Actually I didn't understand the meaning of PKT_N(). Now that you
clarified the situation (it returns the max value for packet > 0), I
fine with your implementation.

> 
> 
> >> +	writel_relaxed(MODE_MLC, nfc->pbus_base + PBUS_PAD_MODE);  
> > 
> > Not sure I understand what MLC means here? Can you give more detail?  
> 
> I'll rename it to MODE_NFC (for "NAND Flash Controller").
> 
> Basically, either we access NAND chips directly in "raw" mode,
> or we go through the NAND Flash Controller.
> 
> My driver uses "raw" mode for everything except read_page and
> write_page (because they require DMA and HW ECC).

Okay, then yes, please choose a better name.

> 
> 
> >> +	ecc_bits = p->chip.ecc.strength * FIELD_ORDER;  
> > 
> > Are you sure the field order is always 15? I thought the ECC controller
> > was adapting it depending on the packet size (512 bytes packets => 13,
> > 1024 bytes => 14, 2k => 15), but maybe I'm wrong.  
> 
> If I'm reading the doc right, it's always 15.

Okay, so, no matter what packet size you choose, the number of ECC
bytes for a given strength will always be the same, right?

From a storage PoV, that means you'd better choose the biggest packet
size, but I understand that you want to support existing layouts, and
this may also have an impact if you want to support subpage I/Os.

> 
> 
> >> +static int tango_nand_probe(struct platform_device *pdev)
> >> +{
> >> +	int i, kHz;
> >> +	struct resource *res;
> >> +	void __iomem *addr[3];
> >> +	struct tango_nfc *nfc;
> >> +	struct device_node *np;
> >> +
> >> +	struct clk *clk = clk_get(&pdev->dev, NULL);
> >> +	if (IS_ERR(clk))
> >> +		return PTR_ERR(clk);
> >> +
> >> +	nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
> >> +	if (!nfc)
> >> +		return -ENOMEM;
> >> +
> >> +	for (i = 0; i < 3; ++i) {
> >> +		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> >> +		addr[i] = devm_ioremap_resource(&pdev->dev, res);
> >> +		if (IS_ERR(addr[i]))
> >> +			return PTR_ERR(addr[i]);
> >> +	}
> >> +
> >> +	platform_set_drvdata(pdev, nfc);
> >> +	nand_hw_control_init(&nfc->hw);
> >> +	kHz = clk_get_rate(clk) / 1000;
> >> +
> >> +	nfc->reg_base	= addr[0];
> >> +	nfc->mem_base	= addr[1];
> >> +	nfc->pbus_base	= addr[2];  
> > 
> > Why not doing
> > 
> > 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > 	nfc->reg_base = devm_ioremap_resource(&pdev->dev, res);
> > 
> > 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

I meant 

	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);

> > 	nfc->mem_base = devm_ioremap_resource(&pdev->dev, res);  
> 
> Do you mean why do I have a loop for the 3 ioremaps?
> IIUC, you'd prefer that I unroll the loop?

Having a loop would be appropriate if you were filling an nfc->iomems[]
array, but here you're just putting the values in a temporary table to
then assign each entry to a different field in your NFC struct.
It's not really useful in my opinion.
diff mbox

Patch

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index f05e0e9eb2f7..22eb5457c9f7 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -205,6 +205,12 @@  config MTD_NAND_S3C2410_CLKSTOP
 	  when the is NAND chip selected or released, but will save
 	  approximately 5mA of power when there is nothing happening.
 
+config MTD_NAND_TANGO
+	tristate "NAND Flash support for Tango chips"
+	depends on ARCH_TANGO
+	help
+	  Enables the NAND Flash controller on Tango chips.
+
 config MTD_NAND_DISKONCHIP
 	tristate "DiskOnChip 2000, Millennium and Millennium Plus (NAND reimplementation)"
 	depends on HAS_IOMEM
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index f55335373f7c..647f727223ef 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -16,6 +16,7 @@  obj-$(CONFIG_MTD_NAND_DENALI_DT)	+= denali_dt.o
 obj-$(CONFIG_MTD_NAND_AU1550)		+= au1550nd.o
 obj-$(CONFIG_MTD_NAND_BF5XX)		+= bf5xx_nand.o
 obj-$(CONFIG_MTD_NAND_S3C2410)		+= s3c2410.o
+obj-$(CONFIG_MTD_NAND_TANGO)		+= tango_nand.o
 obj-$(CONFIG_MTD_NAND_DAVINCI)		+= davinci_nand.o
 obj-$(CONFIG_MTD_NAND_DISKONCHIP)	+= diskonchip.o
 obj-$(CONFIG_MTD_NAND_DOCG4)		+= docg4.o
diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
new file mode 100644
index 000000000000..dfd27081d5ec
--- /dev/null
+++ b/drivers/mtd/nand/tango_nand.c
@@ -0,0 +1,376 @@ 
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/mtd/nand.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/platform_device.h>
+
+/* Offsets relative to chip->base */
+#define PBUS_CMD	0
+#define PBUS_ADDR	4
+#define PBUS_DATA	8
+
+/* Offsets relative to reg_base */
+#define NFC_STATUS_REG	0x00
+#define NFC_FLASH_CMD	0x04
+#define NFC_DEVICE_CFG	0x08
+#define NFC_TIMING1	0x0c
+#define NFC_TIMING2	0x10
+#define NFC_XFER_CFG	0x14
+#define NFC_PKT_0_CFG	0x18
+#define NFC_PKT_N_CFG	0x1c
+#define NFC_BB_CFG	0x20
+#define NFC_ADDR_PAGE	0x24
+#define NFC_ADDR_OFFSET	0x28
+#define NFC_XFER_STATUS	0x2c
+
+/* NFC_FLASH_CMD values */
+#define NFC_READ	1
+#define NFC_WRITE	2
+
+/* NFC_XFER_STATUS values */
+#define PAGE_IS_EMPTY	BIT(16)
+
+/* Offsets relative to mem_base */
+#define ERROR_REPORT	0x1c0
+
+/* ERROR_REPORT values */
+#define DECODE_ERR_ON_PKT_0(v)	(~v & BIT(7))
+#define DECODE_ERR_ON_PKT_N(v)	(~v & BIT(15))
+#define ERR_COUNT_PKT_0(v)	((v >> 0) & 0x3f)
+#define ERR_COUNT_PKT_N(v)	((v >> 8) & 0x3f)
+
+/* Offsets relative to pbus_base */
+#define PBUS_CS_CTRL	0x83c
+#define PBUS_PAD_MODE	0x8f0
+
+/* PBUS_CS_CTRL values */
+#define PBUS_IORDY	BIT(31)
+
+/* PBUS_PAD_MODE values */
+#define MODE_RAW	0
+#define MODE_MLC	BIT(31)
+
+#define METADATA_SIZE	4
+#define BBM_SIZE	6
+#define FIELD_ORDER	15
+
+#define MAX_CS		4
+
+struct tango_nfc {
+	struct nand_hw_control hw;
+	void __iomem *reg_base, *mem_base, *pbus_base;
+	struct tango_chip *chips[MAX_CS];
+	struct dma_chan *chan;
+};
+
+#define to_tango_nfc(ptr) container_of(ptr, struct tango_nfc, hw)
+
+struct tango_chip {
+	struct nand_chip chip;
+	void __iomem *base;
+	u32 timing1, timing2, xfer_cfg, pkt_0_cfg, pkt_n_cfg, bb_cfg;
+};
+
+#define to_tango_chip(ptr) container_of(ptr, struct tango_chip, chip)
+
+#define XFER_CFG(cs, page_count, steps, metadata_size)	\
+	((cs) << 24 | (page_count) << 16 | (steps) << 8 | (metadata_size) << 0)
+
+#define PKT_CFG(size, strength) ((size) << 16 | (strength) << 0)
+
+#define BB_CFG(bb_offset, bb_size) ((bb_offset) << 16 | (bb_size) << 0)
+
+#define TIMING(t0, t1, t2, t3) (t0 << 24 | t1 << 16 | t2 << 8 | t3)
+
+#define NFC_BUSY(base) (readl_relaxed(base + NFC_STATUS_REG) & 0xf)
+
+static void tango_cmd_ctrl(struct mtd_info *mtd, int dat, unsigned int ctrl)
+{
+	struct tango_chip *chip = to_tango_chip(mtd_to_nand(mtd));
+
+	if (ctrl & NAND_CLE)
+		writeb_relaxed(dat, chip->base + PBUS_CMD);
+
+	if (ctrl & NAND_ALE)
+		writeb_relaxed(dat, chip->base + PBUS_ADDR);
+}
+
+static int tango_dev_ready(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
+
+	return readl_relaxed(nfc->pbus_base + PBUS_CS_CTRL) & PBUS_IORDY;
+}
+
+static uint8_t tango_read_byte(struct mtd_info *mtd)
+{
+	struct tango_chip *chip = to_tango_chip(mtd_to_nand(mtd));
+
+	return readb_relaxed(chip->base + PBUS_DATA);
+}
+
+static void tango_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	struct tango_chip *chip = to_tango_chip(mtd_to_nand(mtd));
+
+	ioread8_rep(chip->base + PBUS_DATA, buf, len);
+}
+
+static int decode_error_report(struct tango_nfc *nfc)
+{
+	u32 status, res;
+
+	status = readl_relaxed(nfc->reg_base + NFC_XFER_STATUS);
+	if (status & PAGE_IS_EMPTY)
+		return 0;
+
+	res = readl_relaxed(nfc->mem_base + ERROR_REPORT);
+
+	if (DECODE_ERR_ON_PKT_0(res) || DECODE_ERR_ON_PKT_N(res))
+		return -EBADMSG;
+
+	return max(ERR_COUNT_PKT_0(res), ERR_COUNT_PKT_N(res));
+}
+
+static void tango_dma_callback(void *arg)
+{
+	complete(arg);
+}
+
+static int do_dma(struct nand_chip *nand, int dir, int cmd, void *buf, int len, int page)
+{
+	struct tango_nfc *nfc = to_tango_nfc(nand->controller);
+	struct tango_chip *chip = to_tango_chip(nand);
+	struct dma_async_tx_descriptor *desc;
+	struct scatterlist sg;
+	struct completion tx_done;
+	int err = -EIO;
+
+	sg_init_one(&sg, buf, len);
+	if (dma_map_sg(nfc->chan->device->dev, &sg, 1, dir) != 1)
+		goto leave;
+
+	desc = dmaengine_prep_slave_sg(nfc->chan, &sg, 1, dir, DMA_PREP_INTERRUPT);
+	if (!desc)
+		goto dma_unmap;
+
+	desc->callback = tango_dma_callback;
+	desc->callback_param = &tx_done;
+	init_completion(&tx_done);
+
+	writel_relaxed(MODE_MLC, nfc->pbus_base + PBUS_PAD_MODE);
+
+	err = 0;
+	dmaengine_submit(desc);
+	dma_async_issue_pending(nfc->chan);
+
+	writel_relaxed(chip->timing1,	nfc->reg_base + NFC_TIMING1);
+	writel_relaxed(chip->timing2,	nfc->reg_base + NFC_TIMING2);
+	writel_relaxed(chip->xfer_cfg,	nfc->reg_base + NFC_XFER_CFG);
+	writel_relaxed(chip->pkt_0_cfg,	nfc->reg_base + NFC_PKT_0_CFG);
+	writel_relaxed(chip->pkt_n_cfg,	nfc->reg_base + NFC_PKT_N_CFG);
+	writel_relaxed(chip->bb_cfg,	nfc->reg_base + NFC_BB_CFG);
+	writel_relaxed(page, nfc->reg_base + NFC_ADDR_PAGE);
+	writel_relaxed(0, nfc->reg_base + NFC_ADDR_OFFSET);
+	writel(cmd, nfc->reg_base + NFC_FLASH_CMD);
+
+	wait_for_completion(&tx_done);
+
+	while (NFC_BUSY(nfc->reg_base))
+		cpu_relax();
+
+	writel_relaxed(MODE_RAW, nfc->pbus_base + PBUS_PAD_MODE);
+dma_unmap:
+	dma_unmap_sg(nfc->chan->device->dev, &sg, 1, dir);
+leave:
+	return err;
+}
+
+static int tango_read_page(struct mtd_info *mtd, struct nand_chip *chip,
+			uint8_t *buf, int oob_required, int page)
+{
+	int err, len = mtd->writesize;
+
+	err = do_dma(chip, DMA_FROM_DEVICE, NFC_READ, buf, len, page);
+	if (err)
+		return err;
+
+	return decode_error_report(to_tango_nfc(chip->controller));
+}
+
+static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
+			const uint8_t *buf, int oob_required, int page)
+{
+	int len = mtd->writesize;
+
+	return do_dma(chip, DMA_TO_DEVICE, NFC_WRITE, (void *)buf, len, page);
+}
+
+static u32 to_ticks(int kHz, int ps)
+{
+	return DIV_ROUND_UP_ULL((u64)kHz * ps, NSEC_PER_SEC);
+}
+
+static int set_timings(struct tango_chip *p, int kHz)
+{
+	u32 Trdy, Textw, Twc, Twpw, Tacc, Thold, Trpw, Textr;
+	const struct nand_sdr_timings *sdr;
+	int mode = onfi_get_async_timing_mode(&p->chip);
+
+	if (mode & ONFI_TIMING_MODE_UNKNOWN)
+		return -ENODEV;
+
+	sdr = onfi_async_timing_mode_to_sdr_timings(fls(mode) - 1);
+
+	Trdy	= to_ticks(kHz, sdr->tCEA_max - sdr->tREA_max);
+	Textw	= to_ticks(kHz, sdr->tWB_max);
+	Twc	= to_ticks(kHz, sdr->tWC_min);
+	Twpw	= to_ticks(kHz, sdr->tWC_min - sdr->tWP_min);
+
+	Tacc	= to_ticks(kHz, sdr->tREA_max);
+	Thold	= to_ticks(kHz, sdr->tREH_min);
+	Trpw	= to_ticks(kHz, sdr->tRC_min - sdr->tREH_min);
+	Textr	= to_ticks(kHz, sdr->tRHZ_max);
+
+	p->timing1 = TIMING(Trdy, Textw, Twc, Twpw);
+	p->timing2 = TIMING(Tacc, Thold, Trpw, Textr);
+
+	return 0;
+}
+
+static int chip_init(struct device *dev, struct device_node *np, int kHz)
+{
+	int err;
+	u32 cs, ecc_bits;
+	struct nand_ecc_ctrl *ecc;
+	struct tango_nfc *nfc = dev_get_drvdata(dev);
+	struct tango_chip *p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
+	err = of_property_read_u32_index(np, "reg", 0, &cs);
+	if (err)
+		return err;
+
+	if (cs >= MAX_CS)
+		return -ERANGE;
+
+	p->chip.read_byte	= tango_read_byte;
+	p->chip.read_buf	= tango_read_buf;
+	p->chip.cmd_ctrl	= tango_cmd_ctrl;
+	p->chip.dev_ready	= tango_dev_ready;
+	/* p->chip.options	= NAND_MAGIC_FOO */
+	p->chip.controller	= &nfc->hw;
+	p->base			= nfc->pbus_base + (cs * 256);
+
+	ecc		= &p->chip.ecc;
+	ecc->mode	= NAND_ECC_HW;
+	ecc->algo	= NAND_ECC_BCH;
+	ecc->read_page	= tango_read_page;
+	ecc->write_page	= tango_write_page;
+
+	nand_set_flash_node(&p->chip, np);
+	err = nand_scan(&p->chip.mtd, 1);
+	if (err)
+		return err;
+
+	ecc_bits = p->chip.ecc.strength * FIELD_ORDER;
+	p->chip.ecc.bytes = DIV_ROUND_UP(ecc_bits, 8);
+	set_timings(p, kHz);
+
+	err = mtd_device_register(&p->chip.mtd, NULL, 0);
+	if (err)
+		return err;
+
+	nfc->chips[cs] = p;
+
+	p->xfer_cfg	= XFER_CFG(cs, 1, ecc->steps, METADATA_SIZE);
+	p->pkt_0_cfg	= PKT_CFG(ecc->size + METADATA_SIZE, ecc->strength);
+	p->pkt_n_cfg	= PKT_CFG(ecc->size, ecc->strength);
+	p->bb_cfg	= BB_CFG(p->chip.mtd.writesize, BBM_SIZE);
+
+	return 0;
+}
+
+static int tango_nand_probe(struct platform_device *pdev)
+{
+	int i, kHz;
+	struct resource *res;
+	void __iomem *addr[3];
+	struct tango_nfc *nfc;
+	struct device_node *np;
+
+	struct clk *clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
+	if (!nfc)
+		return -ENOMEM;
+
+	for (i = 0; i < 3; ++i) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		addr[i] = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(addr[i]))
+			return PTR_ERR(addr[i]);
+	}
+
+	platform_set_drvdata(pdev, nfc);
+	nand_hw_control_init(&nfc->hw);
+	kHz = clk_get_rate(clk) / 1000;
+
+	nfc->reg_base	= addr[0];
+	nfc->mem_base	= addr[1];
+	nfc->pbus_base	= addr[2];
+
+	for_each_child_of_node(pdev->dev.of_node, np) {
+		int err = chip_init(&pdev->dev, np, kHz);
+		if (err)
+			return err;
+	}
+
+	nfc->chan = dma_request_chan(&pdev->dev, "mlc_flash_0");
+	if (IS_ERR(nfc->chan))
+		return PTR_ERR(nfc->chan);
+
+	/* TODO: tweak (?) Peripheral Bus setup (timings and CS config) */
+
+	return 0;
+}
+
+static int tango_nand_remove(struct platform_device *pdev)
+{
+	int cs;
+	struct tango_nfc *nfc = platform_get_drvdata(pdev);
+
+	dma_release_channel(nfc->chan);
+	for (cs = 0; cs < MAX_CS; ++cs)
+		if (nfc->chips[cs] != NULL)
+			nand_release(&nfc->chips[cs]->chip.mtd);
+
+	return 0;
+}
+
+static const struct of_device_id tango_nand_ids[] = {
+	{ .compatible = "sigma,smp8758-nand" },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver tango_nand_driver = {
+	.probe	= tango_nand_probe,
+	.remove	= tango_nand_remove,
+	.driver	= {
+		.name		= "tango-nand",
+		.of_match_table	= tango_nand_ids,
+	},
+};
+
+module_platform_driver(tango_nand_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Sigma Designs");
+MODULE_DESCRIPTION("Tango4 NAND Flash controller driver");