| Message ID | 20221025064623.22808-3-mika.westerberg@linux.intel.com |
|---|---|
| State | Accepted |
| Commit | 8a9a784fb337cfd07f305faf5358335d4c12a788 |
| Headers | show |
| Series | spi: intel: Add support for SFDP opcode | expand |
On 10/28/2022 11:55 AM, Mika Westerberg wrote: > Hi, > > On Fri, Oct 28, 2022 at 11:42:09AM +0530, Gole, Dhruva wrote: >> Hi Mika, >> >> On 10/25/2022 12:16 PM, Mika Westerberg wrote: >>> This allows us to get rid of the checks in the intel_spi_[sh]w_cycle() >>> and makes it possible for the SPI-NOR core to split the transaction into >>> smaller chunks as needed. >> If I understand correctly, the controller ops are called from spi-mem.c, and >> >> spi_mem_adjust_op does exist and on it's own does "split a data transfer >> operation into multiple ones" >> >> So is this really something that you require the controller to do and >> >> can we not rely on spi-mem.c to do it's thing instead? > How does it know the size supported by the hardware? I think this is the > reason spi_mem_adjust_op was introduced but I may be mistaken.' The following piece of code might help: op->data.nbytes = min3((size_t)op->data.nbytes, spi_max_transfer_size(mem->spi), spi_max_message_size(mem->spi) I believe this will help it know the size supported by the hardware. and on the controller side: in case of cadence I have seen it pickup the fifo depth from DTSI, for eg. arch/arm64/boot/dts/ti/k3-am62-main.dtsi: cdns,fifo-depth = <256>; > >> I would like to point you to another controller spi-cadence-quadspi.c >> >> that also doesn't have it's own adjust_op_size but I haven't seen any issues >> as such >> >> inspite. This is because everything first goes through spi-mem.c then onto >> the controllers drivers. > Well Intel SPI driver did not not have any issues previously either > because it handled the read/write split internally but SFDP read is done > through "register read side" which only supported max 64-byte read until > now :-)
diff --git a/drivers/spi/spi-intel.c b/drivers/spi/spi-intel.c index b3685460d848..13a3a61239d2 100644 --- a/drivers/spi/spi-intel.c +++ b/drivers/spi/spi-intel.c @@ -363,10 +363,6 @@ static int intel_spi_hw_cycle(struct intel_spi *ispi, val = readl(ispi->base + HSFSTS_CTL); val &= ~(HSFSTS_CTL_FCYCLE_MASK | HSFSTS_CTL_FDBC_MASK); - - if (len > INTEL_SPI_FIFO_SZ) - return -EINVAL; - val |= (len - 1) << HSFSTS_CTL_FDBC_SHIFT; val |= HSFSTS_CTL_FCERR | HSFSTS_CTL_FDONE; val |= HSFSTS_CTL_FGO; @@ -397,9 +393,6 @@ static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, size_t len, if (ret < 0) return ret; - if (len > INTEL_SPI_FIFO_SZ) - return -EINVAL; - /* * Always clear it after each SW sequencer operation regardless * of whether it is successful or not. @@ -704,6 +697,12 @@ static int intel_spi_erase(struct intel_spi *ispi, const struct spi_mem *mem, return 0; } +static int intel_spi_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op) +{ + op->data.nbytes = clamp_val(op->data.nbytes, 0, INTEL_SPI_FIFO_SZ); + return 0; +} + static bool intel_spi_cmp_mem_op(const struct intel_spi_mem_op *iop, const struct spi_mem_op *op) { @@ -844,6 +843,7 @@ static ssize_t intel_spi_dirmap_write(struct spi_mem_dirmap_desc *desc, u64 offs } static const struct spi_controller_mem_ops intel_spi_mem_ops = { + .adjust_op_size = intel_spi_adjust_op_size, .supports_op = intel_spi_supports_mem_op, .exec_op = intel_spi_exec_mem_op, .get_name = intel_spi_get_name,
This allows us to get rid of the checks in the intel_spi_[sh]w_cycle() and makes it possible for the SPI-NOR core to split the transaction into smaller chunks as needed. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/spi/spi-intel.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)