From: Pratyush Yadav <p.yadav@ti.com> To: Mika Westerberg <mika.westerberg@linux.intel.com> Cc: Tudor Ambarus <tudor.ambarus@microchip.com>, Mark Brown <broonie@kernel.org>, Lee Jones <lee.jones@linaro.org>, Michael Walle <michael@walle.cc>, Miquel Raynal <miquel.raynal@bootlin.com>, Richard Weinberger <richard@nod.at>, Vignesh Raghavendra <vigneshr@ti.com>, Jonathan Corbet <corbet@lwn.net>, Mauro Lima <mauro.lima@eclypsium.com>, Alexander Sverdlin <alexander.sverdlin@nokia.com>, <linux-mtd@lists.infradead.org>, <linux-spi@vger.kernel.org> Subject: Re: [PATCH 2/3] mtd: spi-nor: intel-spi: Convert to SPI MEM Date: Mon, 4 Oct 2021 15:22:41 +0530 [thread overview] Message-ID: <20211004095239.dyowgkyq5lnfdag2@ti.com> (raw) In-Reply-To: <20210930100719.2176-3-mika.westerberg@linux.intel.com> On 30/09/21 01:07PM, Mika Westerberg wrote: > The preferred way to implement SPI-NOR controller drivers is through SPI > subsubsystem utilizing the SPI MEM core functions. This converts the > Intel SPI flash controller driver over the SPI MEM by moving the driver > from SPI-NOR subsystem to SPI subsystem and in one go make it use the > SPI MEM functions. The driver name will be changed from intel-spi to > spi-intel to match the convention used in the SPI subsystem. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/mtd/spi-nor/controllers/Kconfig | 36 --- > drivers/mtd/spi-nor/controllers/Makefile | 3 - > drivers/mtd/spi-nor/controllers/intel-spi.h | 21 -- > drivers/spi/Kconfig | 38 +++ > drivers/spi/Makefile | 3 + > .../intel-spi-pci.c => spi/spi-intel-pci.c} | 20 +- > .../spi-intel-platform.c} | 21 +- > .../intel-spi.c => spi/spi-intel.c} | 300 +++++++++++------- > drivers/spi/spi-intel.h | 19 ++ > include/linux/mfd/lpc_ich.h | 2 +- > .../x86/{intel-spi.h => spi-intel.h} | 6 +- > 11 files changed, 252 insertions(+), 217 deletions(-) > delete mode 100644 drivers/mtd/spi-nor/controllers/intel-spi.h > rename drivers/{mtd/spi-nor/controllers/intel-spi-pci.c => spi/spi-intel-pci.c} (86%) > rename drivers/{mtd/spi-nor/controllers/intel-spi-platform.c => spi/spi-intel-platform.c} (65%) > rename drivers/{mtd/spi-nor/controllers/intel-spi.c => spi/spi-intel.c} (80%) > create mode 100644 drivers/spi/spi-intel.h > rename include/linux/platform_data/x86/{intel-spi.h => spi-intel.h} (89%) > [...] > +static bool intel_spi_supports_mem_op(struct spi_mem *mem, > + const struct spi_mem_op *op) > +{ > + struct intel_spi *ispi = spi_master_get_devdata(mem->spi->master); > > - offs += erase_size; > - len -= erase_size; > + if (op->cmd.dtr) > + return false; > + > + if (ispi->swseq_reg && ispi->locked) { > + int i; > + > + for (i = 0; i < ARRAY_SIZE(ispi->opcodes); i++) { > + if (ispi->opcodes[i] == op->cmd.opcode) > + return true; > } > > - return 0; > + return false; > } > > - /* Not needed with HW sequencer erase, make sure it is cleared */ > - ispi->atomic_preopcode = 0; > + switch (op->cmd.opcode) { > + case SPINOR_OP_RDID: > + case SPINOR_OP_RDSR: > + case SPINOR_OP_WRSR: > + return true; > > - while (len > 0) { > - writel(offs, ispi->base + FADDR); > + case SPINOR_OP_READ: > + case SPINOR_OP_READ_FAST: > + case SPINOR_OP_READ_4B: > + case SPINOR_OP_READ_FAST_4B: > + return true; I think the checks need to be stricter here. For example, I don't see you pass in protocol width (dual, quad, octal, etc.) to intel_spi_read() so I assume it can only do a certain protocol. You need to make sure that other protocols are rejected here. Similarly, you also don't pass in dummy cycles to intel_spi_read(), so I assume it can only do a fix number of dummy cycles. Need to make sure you reject unsupported ones. Same for other opcodes/cases. > > - val = readl(ispi->base + HSFSTS_CTL); > - val &= ~(HSFSTS_CTL_FDBC_MASK | HSFSTS_CTL_FCYCLE_MASK); > - val |= HSFSTS_CTL_AEL | HSFSTS_CTL_FCERR | HSFSTS_CTL_FDONE; > - val |= cmd; > - val |= HSFSTS_CTL_FGO; > - writel(val, ispi->base + HSFSTS_CTL); > + case SPINOR_OP_PP: > + case SPINOR_OP_PP_4B: > + case SPINOR_OP_WREN: > + case SPINOR_OP_WRDI: > + return true; > > - ret = intel_spi_wait_hw_busy(ispi); > - if (ret) > - return ret; > + case SPINOR_OP_SE: > + case SPINOR_OP_SE_4B: > + return ispi->erase_64k; > > - status = readl(ispi->base + HSFSTS_CTL); > - if (status & HSFSTS_CTL_FCERR) > - return -EIO; > - else if (status & HSFSTS_CTL_AEL) > - return -EACCES; > + case SPINOR_OP_BE_4K: > + case SPINOR_OP_BE_4K_4B: > + return true; > + } > + > + return false; > +} > > - offs += erase_size; > - len -= erase_size; > +static int intel_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op) > +{ > + struct intel_spi *ispi = spi_master_get_devdata(mem->spi->master); > + size_t nbytes = op->data.nbytes; > + u8 opcode = op->cmd.opcode; > + > + if (op->addr.nbytes) { > + if (op->data.dir == SPI_MEM_DATA_IN) > + return intel_spi_read(ispi, op->addr.val, nbytes, > + op->data.buf.in); > + else if (op->data.dir == SPI_MEM_DATA_OUT) > + return intel_spi_write(ispi, op->addr.val, nbytes, > + op->data.buf.out); > + else if (op->data.dir == SPI_MEM_NO_DATA) > + return intel_spi_erase(ispi, opcode, op->addr.val); > + } else { > + if (op->data.dir == SPI_MEM_DATA_IN) > + return intel_spi_read_reg(ispi, opcode, op->data.buf.in, > + nbytes); > + else if (op->data.dir == SPI_MEM_DATA_OUT) > + return intel_spi_write_reg(ispi, opcode, op->data.buf.out, > + nbytes); > + else if (op->data.dir == SPI_MEM_NO_DATA) > + return intel_spi_write_reg(ispi, opcode, NULL, 0); > } > > - return 0; > + return -EINVAL; > +} > + [...] -- Regards, Pratyush Yadav Texas Instruments Inc. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
WARNING: multiple messages have this Message-ID (diff)
From: Pratyush Yadav <p.yadav@ti.com> To: Mika Westerberg <mika.westerberg@linux.intel.com> Cc: Tudor Ambarus <tudor.ambarus@microchip.com>, Mark Brown <broonie@kernel.org>, Lee Jones <lee.jones@linaro.org>, Michael Walle <michael@walle.cc>, Miquel Raynal <miquel.raynal@bootlin.com>, Richard Weinberger <richard@nod.at>, Vignesh Raghavendra <vigneshr@ti.com>, Jonathan Corbet <corbet@lwn.net>, Mauro Lima <mauro.lima@eclypsium.com>, Alexander Sverdlin <alexander.sverdlin@nokia.com>, <linux-mtd@lists.infradead.org>, <linux-spi@vger.kernel.org> Subject: Re: [PATCH 2/3] mtd: spi-nor: intel-spi: Convert to SPI MEM Date: Mon, 4 Oct 2021 15:22:41 +0530 [thread overview] Message-ID: <20211004095239.dyowgkyq5lnfdag2@ti.com> (raw) In-Reply-To: <20210930100719.2176-3-mika.westerberg@linux.intel.com> On 30/09/21 01:07PM, Mika Westerberg wrote: > The preferred way to implement SPI-NOR controller drivers is through SPI > subsubsystem utilizing the SPI MEM core functions. This converts the > Intel SPI flash controller driver over the SPI MEM by moving the driver > from SPI-NOR subsystem to SPI subsystem and in one go make it use the > SPI MEM functions. The driver name will be changed from intel-spi to > spi-intel to match the convention used in the SPI subsystem. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/mtd/spi-nor/controllers/Kconfig | 36 --- > drivers/mtd/spi-nor/controllers/Makefile | 3 - > drivers/mtd/spi-nor/controllers/intel-spi.h | 21 -- > drivers/spi/Kconfig | 38 +++ > drivers/spi/Makefile | 3 + > .../intel-spi-pci.c => spi/spi-intel-pci.c} | 20 +- > .../spi-intel-platform.c} | 21 +- > .../intel-spi.c => spi/spi-intel.c} | 300 +++++++++++------- > drivers/spi/spi-intel.h | 19 ++ > include/linux/mfd/lpc_ich.h | 2 +- > .../x86/{intel-spi.h => spi-intel.h} | 6 +- > 11 files changed, 252 insertions(+), 217 deletions(-) > delete mode 100644 drivers/mtd/spi-nor/controllers/intel-spi.h > rename drivers/{mtd/spi-nor/controllers/intel-spi-pci.c => spi/spi-intel-pci.c} (86%) > rename drivers/{mtd/spi-nor/controllers/intel-spi-platform.c => spi/spi-intel-platform.c} (65%) > rename drivers/{mtd/spi-nor/controllers/intel-spi.c => spi/spi-intel.c} (80%) > create mode 100644 drivers/spi/spi-intel.h > rename include/linux/platform_data/x86/{intel-spi.h => spi-intel.h} (89%) > [...] > +static bool intel_spi_supports_mem_op(struct spi_mem *mem, > + const struct spi_mem_op *op) > +{ > + struct intel_spi *ispi = spi_master_get_devdata(mem->spi->master); > > - offs += erase_size; > - len -= erase_size; > + if (op->cmd.dtr) > + return false; > + > + if (ispi->swseq_reg && ispi->locked) { > + int i; > + > + for (i = 0; i < ARRAY_SIZE(ispi->opcodes); i++) { > + if (ispi->opcodes[i] == op->cmd.opcode) > + return true; > } > > - return 0; > + return false; > } > > - /* Not needed with HW sequencer erase, make sure it is cleared */ > - ispi->atomic_preopcode = 0; > + switch (op->cmd.opcode) { > + case SPINOR_OP_RDID: > + case SPINOR_OP_RDSR: > + case SPINOR_OP_WRSR: > + return true; > > - while (len > 0) { > - writel(offs, ispi->base + FADDR); > + case SPINOR_OP_READ: > + case SPINOR_OP_READ_FAST: > + case SPINOR_OP_READ_4B: > + case SPINOR_OP_READ_FAST_4B: > + return true; I think the checks need to be stricter here. For example, I don't see you pass in protocol width (dual, quad, octal, etc.) to intel_spi_read() so I assume it can only do a certain protocol. You need to make sure that other protocols are rejected here. Similarly, you also don't pass in dummy cycles to intel_spi_read(), so I assume it can only do a fix number of dummy cycles. Need to make sure you reject unsupported ones. Same for other opcodes/cases. > > - val = readl(ispi->base + HSFSTS_CTL); > - val &= ~(HSFSTS_CTL_FDBC_MASK | HSFSTS_CTL_FCYCLE_MASK); > - val |= HSFSTS_CTL_AEL | HSFSTS_CTL_FCERR | HSFSTS_CTL_FDONE; > - val |= cmd; > - val |= HSFSTS_CTL_FGO; > - writel(val, ispi->base + HSFSTS_CTL); > + case SPINOR_OP_PP: > + case SPINOR_OP_PP_4B: > + case SPINOR_OP_WREN: > + case SPINOR_OP_WRDI: > + return true; > > - ret = intel_spi_wait_hw_busy(ispi); > - if (ret) > - return ret; > + case SPINOR_OP_SE: > + case SPINOR_OP_SE_4B: > + return ispi->erase_64k; > > - status = readl(ispi->base + HSFSTS_CTL); > - if (status & HSFSTS_CTL_FCERR) > - return -EIO; > - else if (status & HSFSTS_CTL_AEL) > - return -EACCES; > + case SPINOR_OP_BE_4K: > + case SPINOR_OP_BE_4K_4B: > + return true; > + } > + > + return false; > +} > > - offs += erase_size; > - len -= erase_size; > +static int intel_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op) > +{ > + struct intel_spi *ispi = spi_master_get_devdata(mem->spi->master); > + size_t nbytes = op->data.nbytes; > + u8 opcode = op->cmd.opcode; > + > + if (op->addr.nbytes) { > + if (op->data.dir == SPI_MEM_DATA_IN) > + return intel_spi_read(ispi, op->addr.val, nbytes, > + op->data.buf.in); > + else if (op->data.dir == SPI_MEM_DATA_OUT) > + return intel_spi_write(ispi, op->addr.val, nbytes, > + op->data.buf.out); > + else if (op->data.dir == SPI_MEM_NO_DATA) > + return intel_spi_erase(ispi, opcode, op->addr.val); > + } else { > + if (op->data.dir == SPI_MEM_DATA_IN) > + return intel_spi_read_reg(ispi, opcode, op->data.buf.in, > + nbytes); > + else if (op->data.dir == SPI_MEM_DATA_OUT) > + return intel_spi_write_reg(ispi, opcode, op->data.buf.out, > + nbytes); > + else if (op->data.dir == SPI_MEM_NO_DATA) > + return intel_spi_write_reg(ispi, opcode, NULL, 0); > } > > - return 0; > + return -EINVAL; > +} > + [...] -- Regards, Pratyush Yadav Texas Instruments Inc.
next prev parent reply other threads:[~2021-10-04 9:53 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-30 10:07 [PATCH 0/3] mtd: spi-nor / spi / MFD: Convert intel-spi to SPI MEM Mika Westerberg 2021-09-30 10:07 ` Mika Westerberg 2021-09-30 10:07 ` [PATCH 1/3] mtd: spi-nor: intel-spi: Disable write protection only if asked Mika Westerberg 2021-09-30 10:07 ` Mika Westerberg 2021-10-01 20:23 ` Mauro Lima 2021-10-01 20:23 ` Mauro Lima 2021-10-04 5:18 ` Mika Westerberg 2021-10-04 5:18 ` Mika Westerberg 2021-10-12 18:49 ` Mauro Lima 2021-10-12 18:49 ` Mauro Lima 2021-10-13 9:03 ` Mika Westerberg 2021-10-13 9:03 ` Mika Westerberg 2021-10-13 18:22 ` Mauro Lima 2021-10-13 18:22 ` Mauro Lima 2021-09-30 10:07 ` [PATCH 2/3] mtd: spi-nor: intel-spi: Convert to SPI MEM Mika Westerberg 2021-09-30 10:07 ` Mika Westerberg 2021-10-04 9:52 ` Pratyush Yadav [this message] 2021-10-04 9:52 ` Pratyush Yadav 2021-10-04 10:07 ` Mika Westerberg 2021-10-04 10:07 ` Mika Westerberg 2021-10-07 12:36 ` Pratyush Yadav 2021-10-07 12:36 ` Pratyush Yadav 2021-10-07 16:46 ` Mika Westerberg 2021-10-07 16:46 ` Mika Westerberg 2021-10-07 18:00 ` Pratyush Yadav 2021-10-07 18:00 ` Pratyush Yadav 2021-10-08 9:02 ` Mika Westerberg 2021-10-08 9:02 ` Mika Westerberg 2021-10-08 10:56 ` Pratyush Yadav 2021-10-08 10:56 ` Pratyush Yadav 2021-10-04 14:29 ` Andy Shevchenko 2021-10-04 14:29 ` Andy Shevchenko 2021-10-05 9:41 ` Mika Westerberg 2021-10-05 9:41 ` Mika Westerberg 2021-09-30 10:07 ` [PATCH 3/3] Documentation / MTD: Rename the intel-spi driver Mika Westerberg 2021-09-30 10:07 ` Mika Westerberg 2021-09-30 15:03 ` Alexander Sverdlin 2021-09-30 15:03 ` Alexander Sverdlin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20211004095239.dyowgkyq5lnfdag2@ti.com \ --to=p.yadav@ti.com \ --cc=alexander.sverdlin@nokia.com \ --cc=broonie@kernel.org \ --cc=corbet@lwn.net \ --cc=lee.jones@linaro.org \ --cc=linux-mtd@lists.infradead.org \ --cc=linux-spi@vger.kernel.org \ --cc=mauro.lima@eclypsium.com \ --cc=michael@walle.cc \ --cc=mika.westerberg@linux.intel.com \ --cc=miquel.raynal@bootlin.com \ --cc=richard@nod.at \ --cc=tudor.ambarus@microchip.com \ --cc=vigneshr@ti.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.