[PATCH v5 5/6] nand: spi: Add generic SPI controller support
Boris Brezillon
boris.brezillon at free-electrons.com
Tue Apr 18 00:26:51 PDT 2017
On Mon, 10 Apr 2017 15:51:52 +0800
Peter Pan <peterpandong at micron.com> wrote:
> This commit supports to use generic spi controller
> as spi nand controller.
>
> Signed-off-by: Peter Pan <peterpandong at micron.com>
> ---
> drivers/mtd/nand/spi/Kconfig | 2 +
> drivers/mtd/nand/spi/Makefile | 1 +
> drivers/mtd/nand/spi/controllers/Kconfig | 5 +
> drivers/mtd/nand/spi/controllers/Makefile | 1 +
> drivers/mtd/nand/spi/controllers/generic-spi.c | 159 +++++++++++++++++++++++++
> 5 files changed, 168 insertions(+)
> create mode 100644 drivers/mtd/nand/spi/controllers/Kconfig
> create mode 100644 drivers/mtd/nand/spi/controllers/Makefile
> create mode 100644 drivers/mtd/nand/spi/controllers/generic-spi.c
>
> diff --git a/drivers/mtd/nand/spi/Kconfig b/drivers/mtd/nand/spi/Kconfig
> index d77c46e..6bd1c65 100644
> --- a/drivers/mtd/nand/spi/Kconfig
> +++ b/drivers/mtd/nand/spi/Kconfig
> @@ -3,3 +3,5 @@ menuconfig MTD_SPI_NAND
> depends on MTD_NAND
> help
> This is the framework for the SPI NAND device drivers.
> +
> +source "drivers/mtd/nand/spi/controllers/Kconfig"
> diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
> index df6c2ea..822e48d 100644
> --- a/drivers/mtd/nand/spi/Makefile
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -1,2 +1,3 @@
> obj-$(CONFIG_MTD_SPI_NAND) += core.o
> obj-$(CONFIG_MTD_SPI_NAND) += micron.o
> +obj-$(CONFIG_MTD_SPI_NAND) += controllers/
> diff --git a/drivers/mtd/nand/spi/controllers/Kconfig b/drivers/mtd/nand/spi/controllers/Kconfig
> new file mode 100644
> index 0000000..8ab7023
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/controllers/Kconfig
> @@ -0,0 +1,5 @@
> +config GENERIC_SPI_NAND
> + tristate "SPI NAND with generic SPI bus Support"
> + depends on MTD_SPI_NAND && SPI
> + help
> + This is to support SPI NAND device with generic SPI bus.
> diff --git a/drivers/mtd/nand/spi/controllers/Makefile b/drivers/mtd/nand/spi/controllers/Makefile
> new file mode 100644
> index 0000000..46cbf29
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/controllers/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_GENERIC_SPI_NAND) += generic-spi.o
> diff --git a/drivers/mtd/nand/spi/controllers/generic-spi.c b/drivers/mtd/nand/spi/controllers/generic-spi.c
> new file mode 100644
> index 0000000..d354874
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/controllers/generic-spi.c
> @@ -0,0 +1,159 @@
> +/*
> + * Copyright (c) 2009-2017 Micron Technology, Inc.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/mtd/spinand.h>
> +
> +struct gen_spi_spinand_controller {
> + struct spinand_controller ctrl;
> + struct spi_device *spi;
> +};
> +
> +#define to_gen_spi_spinand_controller(c) \
> + container_of(c, struct gen_spi_spinand_controller, ctrl)
> +
> +/*
> + * gen_spi_spinand_exec_op - to process a command to send to the
> + * SPI NAND by generic SPI bus
> + * @chip: SPI NAND device structure
> + * @op: SPI NAND operation descriptor
> + */
> +static int gen_spi_spinand_exec_op(struct spinand_device *chip,
> + struct spinand_op *op)
> +{
> + struct spi_message message;
> + struct spi_transfer x[3];
> + struct spinand_controller *scontroller = chip->controller.controller;
> + struct gen_spi_spinand_controller *controller;
> +
> + controller = to_gen_spi_spinand_controller(scontroller);
> + spi_message_init(&message);
> + memset(x, 0, sizeof(x));
> + x[0].len = 1;
> + x[0].tx_nbits = 1;
> + x[0].tx_buf = &op->cmd;
> + spi_message_add_tail(&x[0], &message);
> +
> + if (op->n_addr + op->dummy_bytes) {
> + x[1].len = op->n_addr + op->dummy_bytes;
> + x[1].tx_nbits = op->addr_nbits;
> + x[1].tx_buf = op->addr;
> + spi_message_add_tail(&x[1], &message);
> + }
Can you add empty lines to separate each block of code (this comment
applies to the whole contribution).
> + if (op->n_tx) {
> + x[2].len = op->n_tx;
> + x[2].tx_nbits = op->data_nbits;
> + x[2].tx_buf = op->tx_buf;
> + spi_message_add_tail(&x[2], &message);
> + } else if (op->n_rx) {
> + x[2].len = op->n_rx;
> + x[2].rx_nbits = op->data_nbits;
> + x[2].rx_buf = op->rx_buf;
> + spi_message_add_tail(&x[2], &message);
> + }
Empty line here too.
> + return spi_sync(controller->spi, &message);
> +}
> +
> +static struct spinand_controller_ops gen_spi_spinand_ops = {
> + .exec_op = gen_spi_spinand_exec_op,
> +};
> +
> +static int gen_spi_spinand_probe(struct spi_device *spi)
> +{
> + struct spinand_device *chip;
> + struct gen_spi_spinand_controller *controller;
> + struct spinand_controller *spinand_controller;
> + struct device *dev = &spi->dev;
> + u16 mode = spi->mode;
> + int ret;
> +
> + chip = spinand_alloc(dev);
> + if (IS_ERR(chip)) {
> + ret = PTR_ERR(chip);
> + goto err1;
> + }
Ditto.
> + controller = devm_kzalloc(dev, sizeof(*controller), GFP_KERNEL);
> + if (!controller) {
> + ret = -ENOMEM;
> + goto err2;
> + }
Ditto (I think you got it now, so I'll stop here ;)).
> + controller->spi = spi;
> + spinand_controller = &controller->ctrl;
> + spinand_controller->ops = &gen_spi_spinand_ops;
> + spinand_controller->caps = SPINAND_CAP_RD_X1 | SPINAND_CAP_WR_X1;
> +
> + if ((mode & SPI_RX_QUAD) && (mode & SPI_TX_QUAD))
> + spinand_controller->caps |= SPINAND_CAP_RD_QUAD;
> + if ((mode & SPI_RX_DUAL) && (mode & SPI_TX_DUAL))
> + spinand_controller->caps |= SPINAND_CAP_RD_DUAL;
> + if (mode & SPI_RX_QUAD)
> + spinand_controller->caps |= SPINAND_CAP_RD_X4;
> + if (mode & SPI_RX_DUAL)
> + spinand_controller->caps |= SPINAND_CAP_RD_X2;
> + if (mode & SPI_TX_QUAD)
> + spinand_controller->caps |= SPINAND_CAP_WR_QUAD |
> + SPINAND_CAP_WR_X4;
> + if (mode & SPI_TX_DUAL)
> + spinand_controller->caps |= SPINAND_CAP_WR_DUAL |
> + SPINAND_CAP_WR_X2;
> + chip->controller.controller = spinand_controller;
> + /*
> + * generic spi controller doesn't have ecc capability,
> + * so use on-die ecc.
> + */
> + chip->ecc.type = SPINAND_ECC_ONDIE;
Hm, can't we use the DT property to select that? It seems a bit weird
to have on-die ECC enabled by default.
> + spi_set_drvdata(spi, chip);
> +
> + ret = spinand_register(chip);
> + if (ret)
> + goto err3;
> +
> + return 0;
> +
> +err3:
> + devm_kfree(dev, controller);
> +err2:
Already said that earlier, try to find better names for your labels.
Here, err_free_spinand. But anyway, since everything is allocated with
devm_, I think you can just skip these steps.
> + spinand_free(chip);
> +err1:
> + return ret;
> +}
> +
> +static int gen_spi_spinand_remove(struct spi_device *spi)
> +{
> + struct spinand_device *chip = spi_get_drvdata(spi);
> + struct spinand_controller *scontroller = chip->controller.controller;
> + struct gen_spi_spinand_controller *controller;
> +
> + spinand_unregister(chip);
> + controller = to_gen_spi_spinand_controller(scontroller);
> + devm_kfree(&spi->dev, controller);
> + spinand_free(chip);
> +
> + return 0;
> +}
> +
> +static struct spi_driver gen_spi_spinand_driver = {
> + .driver = {
> + .name = "generic_spinand",
> + .owner = THIS_MODULE,
> + },
> + .probe = gen_spi_spinand_probe,
> + .remove = gen_spi_spinand_remove,
> +};
> +module_spi_driver(gen_spi_spinand_driver);
> +
> +MODULE_DESCRIPTION("Generic SPI controller to support SPI NAND");
> +MODULE_AUTHOR("Peter Pan<peterpandong at micron.com>");
> +MODULE_LICENSE("GPL v2");
More information about the linux-mtd
mailing list