| Message ID | 20190205173254.16388-1-tudor.ambarus@microchip.com |
|---|---|
| Headers | show |
| Series | spi: atmel-quadspi: introduce sam9x60 qspi controller | expand |
On Tue, 5 Feb 2019 17:33:38 +0000 <Tudor.Ambarus@microchip.com> wrote: > From: Tudor Ambarus <tudor.ambarus@microchip.com> > > 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 > different transfer type bits in IFR register. It has dedicated registers > to specify a read or a write instruction: 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 <tudor.ambarus@microchip.com> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > v6: add a caps instance to the sama5d2 entry instead of allowing caps > to be NULL > v5: > - use WICR for sam9x60 > - remove ops hooks and introduce caps->has_ricr > - get rid of the cfg struct > - group IO accesses together in atmel_qspi_set_cfg() > v4: > - drop local variables that kept aq->regs and &pdev->dev, the compiler > should be smart enough to store them in a register > - add comment saying QSPI_IFR_APBTFRTYP_READ is defined in sam9x60 > - s/sama5d2_qspi_modes/atmel_qspi_modes, modes are the same both > controllers > - fix kernel doc header > - move comment in function body > v3: > - reorganize the code and change ops functions pointers to avoid code > duplication. From the IP perspective, the transfer type bits are > different, and what registers are written: ricr/wicr instead of icr. > - treat just regular spi transfers. Mem transfers will be added together > with dirmap support. > 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 | 163 ++++++++++++++++++++++++++++++++++---------- > 1 file changed, 128 insertions(+), 35 deletions(-) > > diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c > index 336501d962e5..fffc21cd5f79 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) > @@ -118,6 +121,7 @@ > #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_READ BIT(24) /* Defined in SAM9X60 */ > > /* Bitfields in QSPI_SMR (Scrambling Mode Register) */ > #define QSPI_SMR_SCREN BIT(0) > @@ -133,12 +137,18 @@ > #define QSPI_WPSR_WPVSRC_MASK GENMASK(15, 8) > #define QSPI_WPSR_WPVSRC(src) (((src) << 8) & QSPI_WPSR_WPVSRC) > > +struct atmel_qspi_caps { > + bool has_qspick; > + bool has_ricr; > +}; > > struct atmel_qspi { > void __iomem *regs; > void __iomem *mem; > struct clk *pclk; > + struct clk *qspick; > struct platform_device *pdev; > + const struct atmel_qspi_caps *caps; > u32 pending; > u32 mr; > struct completion cmd_completion; > @@ -151,7 +161,7 @@ struct atmel_qspi_mode { > u32 config; > }; > > -static const struct atmel_qspi_mode sama5d2_qspi_modes[] = { > +static const struct atmel_qspi_mode atmel_qspi_modes[] = { > { 1, 1, 1, QSPI_IFR_WIDTH_SINGLE_BIT_SPI }, > { 1, 1, 2, QSPI_IFR_WIDTH_DUAL_OUTPUT }, > { 1, 1, 4, QSPI_IFR_WIDTH_QUAD_OUTPUT }, > @@ -180,8 +190,8 @@ static int atmel_qspi_find_mode(const struct spi_mem_op *op) > { > u32 i; > > - for (i = 0; i < ARRAY_SIZE(sama5d2_qspi_modes); i++) > - if (atmel_qspi_is_compatible(op, &sama5d2_qspi_modes[i])) > + for (i = 0; i < ARRAY_SIZE(atmel_qspi_modes); i++) > + if (atmel_qspi_is_compatible(op, &atmel_qspi_modes[i])) > return i; > > return -ENOTSUPP; > @@ -201,36 +211,37 @@ static bool atmel_qspi_supports_op(struct spi_mem *mem, > return true; > } > > -static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op) > +static int atmel_qspi_set_cfg(struct atmel_qspi *aq, > + const struct spi_mem_op *op, u32 *offset) > { > - struct atmel_qspi *aq = spi_controller_get_devdata(mem->spi->master); > - int mode; > + u32 iar, icr, ifr; > u32 dummy_cycles = 0; > - u32 iar, icr, ifr, sr; > - int err = 0; > + int mode; > > iar = 0; > icr = QSPI_ICR_INST(op->cmd.opcode); > ifr = QSPI_IFR_INSTEN; > > - /* > - * If the QSPI controller is set in regular SPI mode, set it in > - * Serial Memory Mode (SMM). > - */ > - if (aq->mr != QSPI_MR_SMM) { > - writel_relaxed(QSPI_MR_SMM, aq->regs + QSPI_MR); > - aq->mr = QSPI_MR_SMM; > - } > - > mode = atmel_qspi_find_mode(op); > if (mode < 0) > return mode; > - > - ifr |= sama5d2_qspi_modes[mode].config; > + ifr |= atmel_qspi_modes[mode].config; > > if (op->dummy.buswidth && op->dummy.nbytes) > dummy_cycles = op->dummy.nbytes * 8 / op->dummy.buswidth; > > + /* > + * The controller allows 24 and 32-bit addressing while NAND-flash > + * requires 16-bit long. Handling 8-bit long addresses is done using > + * the option field. For the 16-bit addresses, the workaround depends > + * of the number of requested dummy bits. If there are 8 or more dummy > + * cycles, the address is shifted and sent with the first dummy byte. > + * Otherwise opcode is disabled and the first byte of the address > + * contains the command opcode (works only if the opcode and address > + * use the same buswidth). The limitation is when the 16-bit address is > + * used without enough dummy cycles and the opcode is using a different > + * buswidth than the address. > + */ > if (op->addr.buswidth) { > switch (op->addr.nbytes) { > case 0: > @@ -264,6 +275,9 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op) > } > } > > + /* offset of the data access in the QSPI memory space */ > + *offset = iar; > + > /* Set number of dummy cycles */ > if (dummy_cycles) > ifr |= QSPI_IFR_NBDUM(dummy_cycles); > @@ -272,16 +286,51 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op) > if (op->data.nbytes) > ifr |= QSPI_IFR_DATAEN; > > - if (op->data.dir == SPI_MEM_DATA_OUT) > - ifr |= QSPI_IFR_SAMA5D2_WRITE_TRSFR; > + /* > + * If the QSPI controller is set in regular SPI mode, set it in > + * Serial Memory Mode (SMM). > + */ > + if (aq->mr != QSPI_MR_SMM) { > + writel_relaxed(QSPI_MR_SMM, aq->regs + QSPI_MR); > + aq->mr = QSPI_MR_SMM; > + } > > /* Clear pending interrupts */ > (void)readl_relaxed(aq->regs + QSPI_SR); > > - /* Set QSPI Instruction Frame registers */ > - writel_relaxed(iar, aq->regs + QSPI_IAR); > - writel_relaxed(icr, aq->regs + QSPI_ICR); > - writel_relaxed(ifr, aq->regs + QSPI_IFR); > + if (aq->caps->has_ricr) { > + if (!op->addr.nbytes && op->data.dir == SPI_MEM_DATA_IN) > + ifr |= QSPI_IFR_APBTFRTYP_READ; > + > + /* Set QSPI Instruction Frame registers */ > + writel_relaxed(iar, aq->regs + QSPI_IAR); > + if (op->data.dir == SPI_MEM_DATA_IN) > + writel_relaxed(icr, aq->regs + QSPI_RICR); > + else > + writel_relaxed(icr, aq->regs + QSPI_WICR); > + writel_relaxed(ifr, aq->regs + QSPI_IFR); > + } else { > + if (op->data.dir == SPI_MEM_DATA_OUT) > + ifr |= QSPI_IFR_SAMA5D2_WRITE_TRSFR; > + > + /* Set QSPI Instruction Frame registers */ > + writel_relaxed(iar, aq->regs + QSPI_IAR); > + writel_relaxed(icr, aq->regs + QSPI_ICR); > + writel_relaxed(ifr, aq->regs + QSPI_IFR); > + } > + > + return 0; > +} > + > +static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op) > +{ > + struct atmel_qspi *aq = spi_controller_get_devdata(mem->spi->master); > + u32 sr, offset; > + int err; > + > + err = atmel_qspi_set_cfg(aq, op, &offset); > + if (err) > + return err; > > /* Skip to the final steps if there is no data */ > if (op->data.nbytes) { > @@ -290,11 +339,11 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op) > > /* Send/Receive data */ > if (op->data.dir == SPI_MEM_DATA_IN) > - _memcpy_fromio(op->data.buf.in, > - aq->mem + iar, op->data.nbytes); > + _memcpy_fromio(op->data.buf.in, aq->mem + offset, > + op->data.nbytes); > else > - _memcpy_toio(aq->mem + iar, > - op->data.buf.out, op->data.nbytes); > + _memcpy_toio(aq->mem + offset, op->data.buf.out, > + op->data.nbytes); > > /* Release the chip-select */ > writel_relaxed(QSPI_CR_LASTXFER, aq->regs + QSPI_CR); > @@ -450,28 +499,55 @@ static int atmel_qspi_probe(struct platform_device *pdev) > goto exit; > } > > + aq->caps = of_device_get_match_data(&pdev->dev); > + if (!aq->caps) { > + dev_err(&pdev->dev, "Could not retrieve QSPI caps\n"); > + err = -EINVAL; > + goto exit; > + } > + > + if (aq->caps->has_qspick) { > + /* Get the QSPI system clock */ > + aq->qspick = devm_clk_get(&pdev->dev, "qspick"); > + if (IS_ERR(aq->qspick)) { > + dev_err(&pdev->dev, "missing system clock\n"); > + err = PTR_ERR(aq->qspick); > + goto disable_pclk; > + } > + > + /* Enable the QSPI system clock */ > + err = clk_prepare_enable(aq->qspick); > + if (err) { > + dev_err(&pdev->dev, > + "failed to enable the QSPI system clock\n"); > + goto disable_pclk; > + } > + } > + > /* Request the IRQ */ > irq = platform_get_irq(pdev, 0); > if (irq < 0) { > dev_err(&pdev->dev, "missing IRQ\n"); > err = irq; > - goto disable_pclk; > + goto disable_qspick; > } > err = devm_request_irq(&pdev->dev, irq, atmel_qspi_interrupt, > 0, dev_name(&pdev->dev), aq); > if (err) > - goto disable_pclk; > + goto disable_qspick; > > err = atmel_qspi_init(aq); > if (err) > - goto disable_pclk; > + goto disable_qspick; > > err = spi_register_controller(ctrl); > if (err) > - goto disable_pclk; > + goto disable_qspick; > > return 0; > > +disable_qspick: > + clk_disable_unprepare(aq->qspick); > disable_pclk: > clk_disable_unprepare(aq->pclk); > exit: > @@ -487,6 +563,7 @@ static int atmel_qspi_remove(struct platform_device *pdev) > > spi_unregister_controller(ctrl); > writel_relaxed(QSPI_CR_QSPIDIS, aq->regs + QSPI_CR); > + clk_disable_unprepare(aq->qspick); > clk_disable_unprepare(aq->pclk); > return 0; > } > @@ -495,6 +572,7 @@ static int __maybe_unused atmel_qspi_suspend(struct device *dev) > { > struct atmel_qspi *aq = dev_get_drvdata(dev); > > + clk_disable_unprepare(aq->qspick); > clk_disable_unprepare(aq->pclk); > > return 0; > @@ -505,6 +583,7 @@ static int __maybe_unused atmel_qspi_resume(struct device *dev) > struct atmel_qspi *aq = dev_get_drvdata(dev); > > clk_prepare_enable(aq->pclk); > + clk_prepare_enable(aq->qspick); > > return atmel_qspi_init(aq); > } > @@ -512,8 +591,22 @@ static int __maybe_unused atmel_qspi_resume(struct device *dev) > static SIMPLE_DEV_PM_OPS(atmel_qspi_pm_ops, atmel_qspi_suspend, > atmel_qspi_resume); > > +static const struct atmel_qspi_caps atmel_sama5d2_qspi_caps = {}; > + > +static const struct atmel_qspi_caps atmel_sam9x60_qspi_caps = { > + .has_qspick = true, > + .has_ricr = true, > +}; > + > static const struct of_device_id atmel_qspi_dt_ids[] = { > - { .compatible = "atmel,sama5d2-qspi" }, > + { > + .compatible = "atmel,sama5d2-qspi", > + .data = &atmel_sama5d2_qspi_caps, > + }, > + { > + .compatible = "microchip,sam9x60-qspi", > + .data = &atmel_sam9x60_qspi_caps, > + }, > { /* sentinel */ } > }; >
On Tue, Feb 05, 2019 at 05:33:06PM +0000, Tudor.Ambarus@microchip.com wrote: > From: Tudor Ambarus <tudor.ambarus@microchip.com> > > Set the controller by default in Serial Memory Mode (SMM) at probe. > Cache Mode Register (MR) value to avoid write access when setting > the controller in serial memory mode at exec_op(). > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > v6: no change > v5: collect R-b You say you've collected a reviewed-by for this but there's no reviewed-by on the patch?
On Tue, 5 Feb 2019 17:33:06 +0000 <Tudor.Ambarus@microchip.com> wrote: > From: Tudor Ambarus <tudor.ambarus@microchip.com> > > Set the controller by default in Serial Memory Mode (SMM) at probe. > Cache Mode Register (MR) value to avoid write access when setting > the controller in serial memory mode at exec_op(). > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> Add my R-b back Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > v6: no change > v5: collect R-b > v4: s/smm/mr, init controller in serial memory mode by default > v3: update smm value when different. rename mr/smm > v2: cache MR value instead of moving the write access at probe > > drivers/spi/atmel-quadspi.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c > index ddc712410812..d6864d29f294 100644 > --- a/drivers/spi/atmel-quadspi.c > +++ b/drivers/spi/atmel-quadspi.c > @@ -155,6 +155,7 @@ struct atmel_qspi { > struct clk *clk; > struct platform_device *pdev; > u32 pending; > + u32 mr; > struct completion cmd_completion; > }; > > @@ -238,7 +239,14 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op) > icr = QSPI_ICR_INST(op->cmd.opcode); > ifr = QSPI_IFR_INSTEN; > > - qspi_writel(aq, QSPI_MR, QSPI_MR_SMM); > + /* > + * If the QSPI controller is set in regular SPI mode, set it in > + * Serial Memory Mode (SMM). > + */ > + if (aq->mr != QSPI_MR_SMM) { > + qspi_writel(aq, QSPI_MR, QSPI_MR_SMM); > + aq->mr = QSPI_MR_SMM; > + } > > mode = find_mode(op); > if (mode < 0) > @@ -381,6 +389,10 @@ static int atmel_qspi_init(struct atmel_qspi *aq) > /* Reset the QSPI controller */ > qspi_writel(aq, QSPI_CR, QSPI_CR_SWRST); > > + /* Set the QSPI controller by default in Serial Memory Mode */ > + qspi_writel(aq, QSPI_MR, QSPI_MR_SMM); > + aq->mr = QSPI_MR_SMM; > + > /* Enable the QSPI controller */ > qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIEN); >
On 02/06/2019 06:08 PM, Mark Brown wrote: > On Tue, Feb 05, 2019 at 05:33:06PM +0000, Tudor.Ambarus@microchip.com wrote: >> From: Tudor Ambarus <tudor.ambarus@microchip.com> >> >> Set the controller by default in Serial Memory Mode (SMM) at probe. >> Cache Mode Register (MR) value to avoid write access when setting >> the controller in serial memory mode at exec_op(). >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> >> --- >> v6: no change >> v5: collect R-b > > You say you've collected a reviewed-by for this but there's no > reviewed-by on the patch? > Not intended. 8/13 has the same problem. I guess I added the R-b tags after formatting the patches, this may explain why they're gone now. Boris, can you please add your R-b tag on 8/13 too?
From: Tudor Ambarus <tudor.ambarus@microchip.com> Patches from 1 to 11 are minor fixes or cosmetics. Patches 12 and 13 introduce the sam9x60 qspi controller. sam9x60 qspi controller tested with sst26vf064b jedec,spi-nor flash. Backward compatibility test done on sama5d2 qspi controller and mx25l25635e jedec,spi-nor flash. v6: patch 13/13: add a caps instance to the sama5d2 entry instead of allowing caps to be NULL v5: - use WICR for sam9x60 - remove ops hooks and introduce caps->has_ricr - get rid of the cfg struct - group IO accesses together in atmel_qspi_set_cfg() v4: - s/smm/mr, init controller in serial memory mode by default - drop local variables that kept aq->regs and &pdev->dev, the compiler should be smart enough to store them in a register - introduce QSPI_IFR_TFRTYP_MEM - add comment saying QSPI_IFR_APBTFRTYP_READ is defined in sam9x60 - s/sama5d2_qspi_modes/atmel_qspi_modes, modes are the same both controllers - fix kernel doc header - move comment in function body - collect R-b tags v3: - update smm value when different. - treat just regular spi transfers when introducing sam9x60 qspi IP. Mem transfers will be added together with dirmap support. - reorganize the code and change ops functions pointers to avoid code duplication. - rename aq->clk to aq->pclk to indicate that it's a peripheral clock. - drop unused and NOP transfer macros. - add Suggested-by tags, reword some commits. v2: - cache MR value, - drop iomem wrappers, - make "pclk" clock-name mandatory even for sama5d2, - rework clock handling, - reorder setting of register values in set_cfg() calls, - collect R-b tags. Tudor Ambarus (13): spi: atmel-quadspi: cache MR value to avoid a write access spi: atmel-quadspi: order header files inclusion alphabetically spi: atmel-quadspi: drop wrappers for iomem accesses spi: atmel-quadspi: fix naming scheme spi: atmel-quadspi: remove unnecessary cast spi: atmel-quadspi: return appropriate error code spi: atmel-quadspi: switch to SPDX license identifiers spi: atmel-quadspi: rework transfer macros dt-bindings: spi: atmel-quadspi: update example to new clock binding dt-bindings: spi: atmel-quadspi: make "pclk" mandatory spi: atmel-quadspi: add support for named peripheral clock dt-bindings: spi: atmel-quadspi: QuadSPI driver for Microchip SAM9X60 spi: atmel-quadspi: add support for sam9x60 qspi controller .../devicetree/bindings/spi/atmel-quadspi.txt | 12 +- drivers/spi/atmel-quadspi.c | 270 +++++++++++++-------- 2 files changed, 184 insertions(+), 98 deletions(-)