| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4904a283-8ede-3147-078a-b64318513b07@wanyeetech.com>
Date: Sun, 24 Jul 2022 00:53:10 +0800
From: Zhou Yanjie <zhouyanjie@...yeetech.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
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.
Hi Krzysztof,
On 2022/7/23 上午2:07, Krzysztof Kozlowski wrote:
> 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.
Sure, I will change it in the next version.
>> + 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.
Will change it.
>
>> +}
>> +
>> +/*
>> + * 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()?
Will add it in the next version.
Thanks and beset regards!
>> +
>> + 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