diff mbox series

[2/2] spi: intel: Implement dirmap hooks

Message ID 20220411113158.2037-2-mika.westerberg@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] spi: intel: Fix typo in kernel-doc of intel_spi_probe() | expand

Commit Message

Mika Westerberg April 11, 2022, 11:31 a.m. UTC
Currently the driver goes over the supported opcodes list each time
->exec_op() is called and finds the suitable for the given operation.
This consumes unnecessary amount of CPU cycles because the operation is
always the same. For this reason populate dirmap hooks for the driver so
that we cache the selected operation and then simply call it on each
read/write.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/spi-intel.c | 49 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

Comments

Mark Brown April 19, 2022, 12:12 p.m. UTC | #1
On Mon, Apr 11, 2022 at 02:31:58PM +0300, Mika Westerberg wrote:
> Currently the driver goes over the supported opcodes list each time
> ->exec_op() is called and finds the suitable for the given operation.
> This consumes unnecessary amount of CPU cycles because the operation is
> always the same. For this reason populate dirmap hooks for the driver so
> that we cache the selected operation and then simply call it on each
> read/write.

This breaks an x86 allmodconfig build:

/build/stage/linux/drivers/spi/spi-intel.c: In function ‘intel_spi_dirmap_read’:
/build/stage/linux/drivers/spi/spi-intel.c:808:38: error: passing argument 2 of ‘iop->exec_op’ from incompatible pointer type [-Werror=incompatible-pointer-types]
  808 |         ret = iop->exec_op(ispi, desc->mem, iop, &op);
      |                                  ~~~~^~~~~
      |                                      |
      |                                      struct spi_mem *
/build/stage/linux/drivers/spi/spi-intel.c:808:38: note: expected ‘const struct intel_spi_mem_op *’ but argument is of type ‘struct spi_mem *’
/build/stage/linux/drivers/spi/spi-intel.c:808:45: error: passing argument 3 of ‘iop->exec_op’ from incompatible pointer type [-Werror=incompatible-pointer-types]
  808 |         ret = iop->exec_op(ispi, desc->mem, iop, &op);
      |                                             ^~~
      |                                             |
      |                                             const struct intel_spi_mem_op *
/build/stage/linux/drivers/spi/spi-intel.c:808:45: note: expected ‘const struct spi_mem_op *’ but argument is of type ‘const struct intel_spi_mem_op *’
/build/stage/linux/drivers/spi/spi-intel.c:808:15: error: too many arguments to function ‘iop->exec_op’
  808 |         ret = iop->exec_op(ispi, desc->mem, iop, &op);
      |               ^~~
/build/stage/linux/drivers/spi/spi-intel.c: In function ‘intel_spi_dirmap_write’:
/build/stage/linux/drivers/spi/spi-intel.c:824:38: error: passing argument 2 of ‘iop->exec_op’ from incompatible pointer type [-Werror=incompatible-pointer-types]
  824 |         ret = iop->exec_op(ispi, desc->mem, iop, &op);
      |                                  ~~~~^~~~~
      |                                      |
      |                                      struct spi_mem *
/build/stage/linux/drivers/spi/spi-intel.c:824:38: note: expected ‘const struct intel_spi_mem_op *’ but argument is of type ‘struct spi_mem *’
/build/stage/linux/drivers/spi/spi-intel.c:824:45: error: passing argument 3 of ‘iop->exec_op’ from incompatible pointer type [-Werror=incompatible-pointer-types]
  824 |         ret = iop->exec_op(ispi, desc->mem, iop, &op);
      |                                             ^~~
      |                                             |
      |                                             const struct intel_spi_mem_op *
/build/stage/linux/drivers/spi/spi-intel.c:824:45: note: expected ‘const struct spi_mem_op *’ but argument is of type ‘const struct intel_spi_mem_op *’
/build/stage/linux/drivers/spi/spi-intel.c:824:15: error: too many arguments to function ‘iop->exec_op’
  824 |         ret = iop->exec_op(ispi, desc->mem, iop, &op);
      |               ^~~
cc1: all warnings being treated as errors
Mika Westerberg April 20, 2022, 6:25 a.m. UTC | #2
Hi Mark,

On Tue, Apr 19, 2022 at 01:12:59PM +0100, Mark Brown wrote:
> On Mon, Apr 11, 2022 at 02:31:58PM +0300, Mika Westerberg wrote:
> > Currently the driver goes over the supported opcodes list each time
> > ->exec_op() is called and finds the suitable for the given operation.
> > This consumes unnecessary amount of CPU cycles because the operation is
> > always the same. For this reason populate dirmap hooks for the driver so
> > that we cache the selected operation and then simply call it on each
> > read/write.
> 
> This breaks an x86 allmodconfig build:
> 
> /build/stage/linux/drivers/spi/spi-intel.c: In function ‘intel_spi_dirmap_read’:
> /build/stage/linux/drivers/spi/spi-intel.c:808:38: error: passing argument 2 of ‘iop->exec_op’ from incompatible pointer type [-Werror=incompatible-pointer-types]
>   808 |         ret = iop->exec_op(ispi, desc->mem, iop, &op);
>       |                                  ~~~~^~~~~
>       |                                      |
>       |                                      struct spi_mem *

Oh, indeed :( I have another patch adding second chip select support
that adds the parameter and then I changed the ordering, and of course
forgot to reflect that to this patch. I will fix this up and re-send.
Sorry about this.
diff mbox series

Patch

diff --git a/drivers/spi/spi-intel.c b/drivers/spi/spi-intel.c
index 1bdb227e0ca2..43c3ddced86d 100644
--- a/drivers/spi/spi-intel.c
+++ b/drivers/spi/spi-intel.c
@@ -779,10 +779,59 @@  static const char *intel_spi_get_name(struct spi_mem *mem)
 	return dev_name(ispi->dev);
 }
 
+static int intel_spi_dirmap_create(struct spi_mem_dirmap_desc *desc)
+{
+	struct intel_spi *ispi = spi_master_get_devdata(desc->mem->spi->master);
+	const struct intel_spi_mem_op *iop;
+
+	iop = intel_spi_match_mem_op(ispi, &desc->info.op_tmpl);
+	if (!iop)
+		return -EOPNOTSUPP;
+
+	desc->priv = (void *)iop;
+	return 0;
+}
+
+static ssize_t intel_spi_dirmap_read(struct spi_mem_dirmap_desc *desc, u64 offs,
+				     size_t len, void *buf)
+{
+	struct intel_spi *ispi = spi_master_get_devdata(desc->mem->spi->master);
+	const struct intel_spi_mem_op *iop = desc->priv;
+	struct spi_mem_op op = desc->info.op_tmpl;
+	int ret;
+
+	/* Fill in the gaps */
+	op.addr.val = offs;
+	op.data.nbytes = len;
+	op.data.buf.in = buf;
+
+	ret = iop->exec_op(ispi, desc->mem, iop, &op);
+	return ret ? ret : len;
+}
+
+static ssize_t intel_spi_dirmap_write(struct spi_mem_dirmap_desc *desc, u64 offs,
+				      size_t len, const void *buf)
+{
+	struct intel_spi *ispi = spi_master_get_devdata(desc->mem->spi->master);
+	const struct intel_spi_mem_op *iop = desc->priv;
+	struct spi_mem_op op = desc->info.op_tmpl;
+	int ret;
+
+	op.addr.val = offs;
+	op.data.nbytes = len;
+	op.data.buf.out = buf;
+
+	ret = iop->exec_op(ispi, desc->mem, iop, &op);
+	return ret ? ret : len;
+}
+
 static const struct spi_controller_mem_ops intel_spi_mem_ops = {
 	.supports_op = intel_spi_supports_mem_op,
 	.exec_op = intel_spi_exec_mem_op,
 	.get_name = intel_spi_get_name,
+	.dirmap_create = intel_spi_dirmap_create,
+	.dirmap_read = intel_spi_dirmap_read,
+	.dirmap_write = intel_spi_dirmap_write,
 };
 
 #define INTEL_SPI_OP_ADDR(__nbytes)					\