[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1a0245c3-5659-573a-c74d-c2145a564b76@linaro.org>
Date: Fri, 22 Jul 2022 20:07:46 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: 周琰杰 (Zhou Yanjie)
<zhouyanjie@...yeetech.com>, tudor.ambarus@...rochip.com,
p.yadav@...com, michael@...le.cc, miquel.raynal@...tlin.com,
richard@....at, vigneshr@...com, broonie@...nel.org,
robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org
Cc: linux-mtd@...ts.infradead.org, linux-spi@...r.kernel.org,
linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, aidanmacdonald.0x0@...il.com,
tmn505@...il.com, paul@...pouillou.net, dongsheng.qiu@...enic.com,
aric.pzqi@...enic.com, rick.tyliu@...enic.com,
jinghui.liu@...enic.com, sernia.zhou@...mail.com,
reimu@...omaker.com
Subject: Re: [PATCH 3/3] SPI: Ingenic: Add SFC support for Ingenic SoCs.
On 22/07/2022 18:48, 周琰杰 (Zhou Yanjie) wrote:
> Add SFC support for the X1000 SoC, the X1600 SoC, and the X2000 SoC
> from Ingenic.
>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@...yeetech.com>
> ---
> drivers/spi/Kconfig | 9 +
> drivers/spi/Makefile | 1 +
> drivers/spi/spi-ingenic-sfc.c | 662 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 672 insertions(+)
> create mode 100644 drivers/spi/spi-ingenic-sfc.c
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 3b1044e..1077bd3 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -437,6 +437,15 @@ config SPI_INGENIC
> To compile this driver as a module, choose M here: the module
> will be called spi-ingenic.
>
> +config SPI_INGENIC_SFC
> + tristate "Ingenic SoCs SPI Flash Controller"
> + depends on MACH_INGENIC || COMPILE_TEST
> + help
> + This enables support for the Ingenic SoCs SPI flash controller.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called ingenic-sfc.
> +
> config SPI_INTEL
> tristate
>
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 0f44eb6..f3e42c0 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_SPI_HISI_SFC_V3XX) += spi-hisi-sfc-v3xx.o
> obj-$(CONFIG_SPI_IMG_SPFI) += spi-img-spfi.o
> obj-$(CONFIG_SPI_IMX) += spi-imx.o
> obj-$(CONFIG_SPI_INGENIC) += spi-ingenic.o
> +obj-$(CONFIG_SPI_INGENIC_SFC) += spi-ingenic-sfc.o
> obj-$(CONFIG_SPI_INTEL) += spi-intel.o
> obj-$(CONFIG_SPI_INTEL_PCI) += spi-intel-pci.o
> obj-$(CONFIG_SPI_INTEL_PLATFORM) += spi-intel-platform.o
> diff --git a/drivers/spi/spi-ingenic-sfc.c b/drivers/spi/spi-ingenic-sfc.c
> new file mode 100644
> index 00000000..a565546
> --- /dev/null
> +++ b/drivers/spi/spi-ingenic-sfc.c
> @@ -0,0 +1,662 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/*
> + * Ingenic SoCs SPI Flash Controller Driver
> + * Copyright (c) 2022 周琰杰 (Zhou Yanjie) <zhouyanjie@...yeetech.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/spi/spi.h>
> +#include <linux/spi/spi-mem.h>
> +
> +/* SFC register offsets */
> +#define SFC_REG_GLB 0x0000
> +#define SFC_REG_DEV_CONF 0x0004
> +#define SFC_REG_DEV_STA_EXP 0x0008
> +#define SFC_REG_DEV_STA_RT 0x000c
> +#define SFC_REG_DEV_STA_MSK 0x0010
> +#define SFC_REG_TRAN_CONF(n) (0x0014 + n * 4)
> +#define SFC_REG_TRAN_CFG0(n) (0x0014 + n * 4)
> +#define SFC_REG_TRAN_LEN 0x002c
> +#define SFC_REG_DEV_ADDR(n) (0x0030 + n * 4)
> +#define SFC_REG_DEV_ADDR_PLUS(n) (0x0048 + n * 4)
> +#define SFC_REG_MEM_ADDR 0x0060
> +#define SFC_REG_TRIG 0x0064
> +#define SFC_REG_SR 0x0068
> +#define SFC_REG_SCR 0x006c
> +#define SFC_REG_INTC 0x0070
> +#define SFC_REG_FSM 0x0074
> +#define SFC_REG_CGE 0x0078
> +#define SFC_REG_TRAN_CFG1(n) (0x009c + n * 4)
> +#define SFC_REG_DR 0x1000
> +
> +/* bits within the GLB register */
> +#define GLB_TRAN_DIR_MASK GENMASK(13, 13)
> +#define GLB_TRAN_DIR_WRITE 0x1
> +#define GLB_TRAN_DIR_READ 0x0
> +#define GLB_THRESHOLD_MASK GENMASK(12, 7)
> +#define GLB_OP_MODE_MASK GENMASK(6, 6)
> +#define GLB_OP_MODE_DMA 0x1
> +#define GLB_OP_MODE_SLAVE 0x0
> +#define GLB_PHASE_NUM_MASK GENMASK(5, 3)
> +#define GLB_WP_EN BIT(2)
> +#define GLB_BURST_MD_MASK GENMASK(1, 0)
> +#define GLB_BURST_MD_INCR32 0x3
> +#define GLB_BURST_MD_INCR16 0x2
> +#define GLB_BURST_MD_INCR8 0x1
> +#define GLB_BURST_MD_INCR4 0x0
> +
> +/* bits within the DEV_CONF register */
> +#define DEV_CONF_SMP_DELAY_MASK GENMASK(20, 16)
> +#define DEV_CONF_SMP_DELAY_180DEG 0x4
> +#define DEV_CONF_SMP_DELAY_HALF_CYCLE 0x1
> +#define DEV_CONF_CMD_TYPE_MASK GENMASK(15, 15)
> +#define DEV_CONF_CMD_TYPE_16BIT 0x1
> +#define DEV_CONF_CMD_TYPE_8BIT 0x0
> +#define DEV_CONF_STA_TYPE_MASK GENMASK(14, 13)
> +#define DEV_CONF_THOLD_MASK GENMASK(12, 11)
> +#define DEV_CONF_TSETUP_MASK GENMASK(10, 9)
> +#define DEV_CONF_TSH_MASK GENMASK(8, 5)
> +#define DEV_CONF_CPHA BIT(4)
> +#define DEV_CONF_CPOL BIT(3)
> +#define DEV_CONF_CE_DL BIT(2)
> +#define DEV_CONF_HOLD_DL BIT(1)
> +#define DEV_CONF_WP_DL BIT(0)
> +
> +/* bits within the TRAN_CONF(n) register */
> +#define TRAN_CONF_TRAN_MODE_MASK GENMASK(31, 29)
> +#define TRAN_CONF_ADDR_WIDTH_MASK GENMASK(28, 26)
> +#define TRAN_CONF_POLL_EN BIT(25)
> +#define TRAN_CONF_CMD_EN BIT(24)
> +#define TRAN_CONF_PHASE_FORMAT_MASK GENMASK(23, 23)
> +#define TRAN_CONF_DMY_BITS_MASK GENMASK(22, 17)
> +#define TRAN_CONF_DATA_EN BIT(16)
> +#define TRAN_CONF_CMD_MASK GENMASK(15, 0)
> +
> +/* bits within the TRIG register */
> +#define TRIG_FLUSH BIT(2)
> +#define TRIG_STOP BIT(1)
> +#define TRIG_START BIT(0)
> +
> +/* bits within the SR register */
> +#define SR_FIFO_NUM_MASK GENMASK(22, 16)
> +#define SR_END BIT(4)
> +#define SR_TRAN_REQ BIT(3)
> +#define SR_RECE_REQ BIT(2)
> +#define SR_OVER BIT(1)
> +#define SR_UNDER BIT(0)
> +
> +/* bits within the SCR register */
> +#define SCR_CLR_END BIT(4)
> +#define SCR_CLR_TREQ BIT(3)
> +#define SCR_CLR_RREQ BIT(2)
> +#define SCR_CLR_OVER BIT(1)
> +#define SCR_CLR_UNDER BIT(0)
> +
> +/* bits within the INTC register */
> +#define INTC_MASK_END BIT(4)
> +#define INTC_MASK_TREQ BIT(3)
> +#define INTC_MASK_RREQ BIT(2)
> +#define INTC_MASK_OVER BIT(1)
> +#define INTC_MASK_UNDER BIT(0)
> +
> +/* bits within the TRAN_CFG1(n) register */
> +#define TRAN_CFG1_TRAN_MODE_MASK GENMASK(7, 4)
> +
> +#define TRAN_MODE_STANDARD 0
> +#define TRAN_MODE_DUAL_DATA 1
> +#define TRAN_MODE_DUAL_IO 2
> +#define TRAN_MODE_DUAL_FULL 3
> +#define TRAN_MODE_QUAD_DATA 5
> +#define TRAN_MODE_QUAD_IO 6
> +#define TRAN_MODE_QUAD_FULL 7
> +#define TRAN_MODE_OCTAL_DATA 9
> +#define TRAN_MODE_OCTAL_IO 10
> +#define TRAN_MODE_OCTAL_FULL 11
> +
> +#define INGENIC_SFC_FIFO_SIZE (64 * 4)
> +
> +#define INGENIC_SFC_TRANSFER_TIMEOUT 1000
> +
> +enum ingenic_sfc_version {
> + ID_X1000,
> + ID_X1600,
> + ID_X2000,
> +};
> +
> +struct ingenic_soc_info {
> + enum ingenic_sfc_version version;
> +
No need for blank line.
> + unsigned int max_bus_width;
> +
Remove as well.
> + const u32 tran_mode_mask;
> +};
> +
> +struct ingenic_sfc {
> + const struct ingenic_soc_info *soc_info;
> +
> + void __iomem *base;
> + struct device *dev;
> + struct clk *clk;
> + int irq;
> +
> + struct completion completion;
> +};
> +
> +static irqreturn_t ingenic_sfc_irq_handler(int irq, void *data)
> +{
> + struct ingenic_sfc *sfc = data;
> +
> + writel(0x1f, sfc->base + SFC_REG_INTC);
> +
> + complete(&sfc->completion);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ingenic_sfc_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
> +{
> + uintptr_t addr = (uintptr_t)op->data.buf.in;
> +
> + if (op->data.nbytes > INGENIC_SFC_FIFO_SIZE && !IS_ALIGNED(addr, 4))
> + op->data.nbytes = INGENIC_SFC_FIFO_SIZE;
> +
> + return 0;
> +}
> +
> +static bool ingenic_sfc_supports_op(struct spi_mem *mem, const struct spi_mem_op *op)
> +{
> + struct spi_device *spi = mem->spi;
> + struct ingenic_sfc *sfc = spi_controller_get_devdata(spi->master);
> + uintptr_t addr = (uintptr_t)op->data.buf.in;
> +
> + /* The controller only supports Standard SPI mode, Duall mode, Quad mode and Octal mode. */
> + if (op->cmd.buswidth > sfc->soc_info->max_bus_width ||
> + op->addr.buswidth > sfc->soc_info->max_bus_width ||
> + op->dummy.buswidth > sfc->soc_info->max_bus_width ||
> + op->data.buswidth > sfc->soc_info->max_bus_width)
> + return false;
> +
> + /* Max 32 dummy clock cycles supported */
> + if (op->dummy.nbytes && op->dummy.nbytes * 8 / op->dummy.buswidth > 32)
> + return false;
> +
> + /* Max rx data length, check controller limits and alignment */
> + if (op->data.dir == SPI_MEM_DATA_IN &&
> + op->data.nbytes > INGENIC_SFC_FIFO_SIZE && !IS_ALIGNED(addr, 4))
> + return false;
> +
> + /* Max 6 bytes address width supported */
> + if (op->addr.nbytes > 6)
> + return false;
> +
> + return spi_mem_default_supports_op(mem, op);
> +}
> +
> +static void ingenic_sfc_set_transfer_mode(struct ingenic_sfc *sfc, const struct spi_mem_op *op)
> +{
> + int val;
> +
> + val = readl(sfc->base + (sfc->soc_info->version >= ID_X1600 ?
> + SFC_REG_TRAN_CFG1(0) : SFC_REG_TRAN_CONF(0)));
This is no really readable.
> + val &= ~sfc->soc_info->tran_mode_mask;
> + if (op->cmd.buswidth == 8)
> + val |= (TRAN_MODE_OCTAL_FULL << (ffs(sfc->soc_info->tran_mode_mask) - 1)) &
> + sfc->soc_info->tran_mode_mask;
> + else if (op->cmd.buswidth == 4)
> + val |= (TRAN_MODE_QUAD_FULL << (ffs(sfc->soc_info->tran_mode_mask) - 1)) &
> + sfc->soc_info->tran_mode_mask;
> + else if (op->cmd.buswidth == 2)
> + val |= (TRAN_MODE_DUAL_FULL << (ffs(sfc->soc_info->tran_mode_mask) - 1)) &
> + sfc->soc_info->tran_mode_mask;
> + else if (op->addr.buswidth == 8)
> + val |= (TRAN_MODE_OCTAL_IO << (ffs(sfc->soc_info->tran_mode_mask) - 1)) &
> + sfc->soc_info->tran_mode_mask;
> + else if (op->addr.buswidth == 4)
> + val |= (TRAN_MODE_QUAD_IO << (ffs(sfc->soc_info->tran_mode_mask) - 1)) &
> + sfc->soc_info->tran_mode_mask;
> + else if (op->addr.buswidth == 2)
> + val |= (TRAN_MODE_DUAL_IO << (ffs(sfc->soc_info->tran_mode_mask) - 1)) &
> + sfc->soc_info->tran_mode_mask;
> + else if (op->data.buswidth == 8)
> + val |= (TRAN_MODE_OCTAL_DATA << (ffs(sfc->soc_info->tran_mode_mask) - 1)) &
> + sfc->soc_info->tran_mode_mask;
> + else if (op->data.buswidth == 4)
> + val |= (TRAN_MODE_QUAD_DATA << (ffs(sfc->soc_info->tran_mode_mask) - 1)) &
> + sfc->soc_info->tran_mode_mask;
> + else if (op->data.buswidth == 2)
> + val |= (TRAN_MODE_DUAL_DATA << (ffs(sfc->soc_info->tran_mode_mask) - 1)) &
> + sfc->soc_info->tran_mode_mask;
> + else
> + val |= (TRAN_MODE_STANDARD << (ffs(sfc->soc_info->tran_mode_mask) - 1)) &
> + sfc->soc_info->tran_mode_mask;
> + writel(val, sfc->base + (sfc->soc_info->version >= ID_X1600 ?
> + SFC_REG_TRAN_CFG1(0) : SFC_REG_TRAN_CONF(0)));
Not readable.
> +}
> +
> +/*
> + * We only need PIO mode to handle the SPI_MEM_NO_DATA transfers,
> + * and the unaligned accesses in SPI_MEM_DATA_IN transfers.
> + */
(...)
> + sfc->dev = &pdev->dev;
> +
> + platform_set_drvdata(pdev, sfc);
> +
> + ret = devm_request_irq(&pdev->dev, sfc->irq, ingenic_sfc_irq_handler, 0,
> + dev_name(&pdev->dev), sfc);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to request irq%d, ret = %d\n", sfc->irq, ret);
> + goto err_put_master;
> + }
> +
> + ctlr->bus_num = -1;
> + ctlr->num_chipselect = 1;
> + ctlr->mem_ops = &ingenic_sfc_mem_ops;
> + ctlr->dev.of_node = pdev->dev.of_node;
> + ctlr->setup = ingenic_sfc_setup;
> + ctlr->mode_bits = SPI_CPHA | SPI_CPOL |
> + SPI_RX_DUAL | SPI_RX_QUAD | SPI_TX_DUAL | SPI_TX_QUAD;
> + if (sfc->soc_info->version >= ID_X2000)
> + ctlr->mode_bits |= SPI_RX_OCTAL | SPI_TX_OCTAL;
> +
> + ret = devm_spi_register_controller(&pdev->dev, ctlr);
> + if (ret)
> + goto err_put_master;
> +
> + return 0;
> +
> +err_put_master:
> + spi_master_put(ctlr);
What about unpreparing clocks? Is it part of spi_master_put()?
> +
> + return ret;
> +}
> +
> +static const struct ingenic_soc_info x1000_soc_info = {
> + .version = ID_X1000,
> +
No need for blank lines after each field.
Best regards,
Krzysztof
Powered by blists - more mailing lists