Re: [PATCH v2 10/10] spi: atmel-quadspi: add support for sam9x60 qspi controller
From: Boris Brezillon <bbrezillon@kernel.org>
Date: 2019-01-31 16:32:25
Also in:
linux-arm-kernel, linux-spi, lkml
On Thu, 31 Jan 2019 16:15:51 +0000 [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Tudor Ambarus <redacted> The sam9x60 qspi controller uses 2 clocks, one for the peripheral register access, the other for the qspi core and phy. Both are mandatory. It uses dedicated register for Read Instruction Code Register (RICR) and Write Instruction Code Register (WICR). ICR/RICR/WICR have identical fields. Tested with sst26vf064b jedec,spi-nor flash. Backward compatibility test done on sama5d2 qspi controller and mx25l25635e jedec,spi-nor flash. Signed-off-by: Tudor Ambarus <redacted> --- v2: - rework clock handling - reorder setting of register values in set_cfg() calls -> move functions that can fail in the upper part of the function body. drivers/spi/atmel-quadspi.c | 296 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 239 insertions(+), 57 deletions(-)diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c index d3e76acf8517..80c934f3e479 100644 --- a/drivers/spi/atmel-quadspi.c +++ b/drivers/spi/atmel-quadspi.c@@ -19,6 +19,7 @@ #include <linux/kernel.h> #include <linux/module.h> #include <linux/of.h> +#include <linux/of_platform.h> #include <linux/platform_device.h> #include <linux/spi/spi-mem.h>@@ -35,7 +36,9 @@ #define QSPI_IAR 0x0030 /* Instruction Address Register */ #define QSPI_ICR 0x0034 /* Instruction Code Register */ +#define QSPI_WICR 0x0034 /* Write Instruction Code Register */ #define QSPI_IFR 0x0038 /* Instruction Frame Register */ +#define QSPI_RICR 0x003C /* Read Instruction Code Register */ #define QSPI_SMR 0x0040 /* Scrambling Mode Register */ #define QSPI_SKR 0x0044 /* Scrambling Key Register */@@ -88,7 +91,7 @@ #define QSPI_SCR_DLYBS_MASK GENMASK(23, 16) #define QSPI_SCR_DLYBS(n) (((n) << 16) & QSPI_SCR_DLYBS_MASK) -/* Bitfields in QSPI_ICR (Instruction Code Register) */ +/* Bitfields in QSPI_ICR (Read/Write Instruction Code Register) */ #define QSPI_ICR_INST_MASK GENMASK(7, 0) #define QSPI_ICR_INST(inst) (((inst) << 0) & QSPI_ICR_INST_MASK) #define QSPI_ICR_OPT_MASK GENMASK(23, 16)@@ -113,6 +116,8 @@ #define QSPI_IFR_OPTL_4BIT (2 << 8) #define QSPI_IFR_OPTL_8BIT (3 << 8) #define QSPI_IFR_ADDRL BIT(10) +#define QSPI_IFR_TFRTYP_TRSFR_MEM BIT(12) +#define QSPI_IFR_TFRTYP_TRSFR_REG (0 << 12)
You don't need to define TRSFR_REG, just set QSPI_IFR_TFRTYP_TRSFR_MEM when you do a mem transfer and do nothing when this is a regular transfer.
#define QSPI_IFR_TFRTYP_MASK GENMASK(13, 12) #define QSPI_IFR_TFRTYP_TRSFR_READ (0 << 12) #define QSPI_IFR_TFRTYP_TRSFR_READ_MEM (1 << 12)
Looks like the read/write flag is on bit 13. Can we just add #define QSPI_IFR_TFRTYP_TRSFR_WRITE BIT(13) and drop all others def? This way the implementation is consistent between sam9x60 and sama5d2.
quoted hunk ↗ jump to hunk
@@ -121,6 +126,8 @@ #define QSPI_IFR_CRM BIT(14) #define QSPI_IFR_NBDUM_MASK GENMASK(20, 16) #define QSPI_IFR_NBDUM(n) (((n) << 16) & QSPI_IFR_NBDUM_MASK) +#define QSPI_IFR_APBTFRTYP_WRITE (0 << 24)
As for the other defs, I don't think you need to define _WRITE.
quoted hunk ↗ jump to hunk
+#define QSPI_IFR_APBTFRTYP_READ BIT(24) /* Bitfields in QSPI_SMR (Scrambling Mode Register) */ #define QSPI_SMR_SCREN BIT(0)@@ -137,16 +144,37 @@ #define QSPI_WPSR_WPVSRC(src) (((src) << 8) & QSPI_WPSR_WPVSRC) +/* Describes register values. */ +struct atmel_qspi_cfg { + u32 icr; + u32 iar; + u32 ifr; +}; + +struct atmel_qspi_caps; + struct atmel_qspi { void __iomem *regs; void __iomem *mem; struct clk *clk;
Can we rename that on pclk?
+ struct clk *qspick; struct platform_device *pdev; + const struct atmel_qspi_caps *caps; u32 pending; u32 mr; struct completion cmd_completion; };
...
+
+static int atmel_sam9x60_qspi_set_cfg(void __iomem *base,
+ const struct spi_mem_op *op,
+ struct atmel_qspi_cfg *cfg)
+{
+ int ret = atmel_qspi_set_mode(cfg, op);
+
+ if (ret)
+ return ret;
+
+ ret = atmel_qspi_set_address_mode(cfg, op);
+ if (ret)
+ return ret;
+
+ cfg->ifr |= QSPI_IFR_INSTEN;
+ cfg->icr |= QSPI_ICR_INST(op->cmd.opcode);
+
+ /* Set data enable */
+ if (op->data.nbytes)
+ cfg->ifr |= QSPI_IFR_DATAEN;
+
+ if (!op->addr.nbytes) {
+ cfg->ifr |= QSPI_IFR_TFRTYP_TRSFR_REG;
+ if (op->data.dir == SPI_MEM_DATA_OUT)
+ cfg->ifr |= QSPI_IFR_APBTFRTYP_WRITE;
+ else
+ cfg->ifr |= QSPI_IFR_APBTFRTYP_READ;
+ } else {
+ cfg->ifr |= QSPI_IFR_TFRTYP_TRSFR_MEM;Can you try doing only regular transfers and let me know if it still works. Support for mem transfers can then be added along with dirmap support.
+ } /* Clear pending interrupts */ (void)readl_relaxed(base + QSPI_SR); /* Set QSPI Instruction Frame registers */ - writel_relaxed(iar, base + QSPI_IAR); - writel_relaxed(icr, base + QSPI_ICR); - writel_relaxed(ifr, base + QSPI_IFR); + writel_relaxed(cfg->iar, base + QSPI_IAR); + if (op->data.dir == SPI_MEM_DATA_OUT) + writel_relaxed(cfg->icr, base + QSPI_ICR); + else + writel_relaxed(cfg->icr, base + QSPI_RICR); + writel_relaxed(cfg->ifr, base + QSPI_IFR); + + return 0; +} +
...
quoted hunk ↗ jump to hunk
@@ -443,32 +578,52 @@ static int atmel_qspi_probe(struct platform_device *pdev) /* Enable the peripheral clock */ err = clk_prepare_enable(aq->clk); if (err) { - dev_err(&pdev->dev, "failed to enable the peripheral clock\n"); + dev_err(dev, "failed to enable the peripheral clock\n"); goto exit; } + if (caps->has_qspick) { + /* Get the QSPI system clock */ + aq->qspick = devm_clk_get(dev, "qspick"); + if (IS_ERR(aq->qspick)) { + dev_err(dev, "missing system clock\n"); + err = PTR_ERR(aq->qspick); + goto disable_clk; + } + + /* Enable the QSPI system clock */ + err = clk_prepare_enable(aq->qspick); + if (err) { + dev_err(dev, + "failed to enable the QSPI system clock\n"); + goto disable_clk; + } + } + /* Request the IRQ */ irq = platform_get_irq(pdev, 0); if (irq < 0) { - dev_err(&pdev->dev, "missing IRQ\n"); + dev_err(dev, "missing IRQ\n"); err = irq; - goto disable_clk; + goto disable_qspick; } - err = devm_request_irq(&pdev->dev, irq, atmel_qspi_interrupt, - 0, dev_name(&pdev->dev), aq); + err = devm_request_irq(dev, irq, atmel_qspi_interrupt, 0, + dev_name(dev), aq); if (err) - goto disable_clk; + goto disable_qspick; err = atmel_qspi_init(aq); if (err) - goto disable_clk; + goto disable_qspick; err = spi_register_controller(ctrl); if (err) - goto disable_clk; + goto disable_qspick; return 0; +disable_qspick: + clk_disable_unprepare(aq->qspick); disable_clk:
We should probably rename this label disable_pclk.
clk_disable_unprepare(aq->clk); exit: