Hi Anderson,
> -----Original Message-----
> From: Sean Anderson <sean...@gmail.com>
> Sent: Thursday, May 19, 2022 9:35 PM
> Subject: Re: [PATCH v2 07/11] spi-mem: Add dirmap API from Linux
>
> On 5/9/22 3:23 AM, Chin-Ting Kuo wrote:
> > This adds the dirmap API originally introduced in Linux commit aa167f3
> > ("spi: spi-mem: Add a new API to support direct mapping"). This also
> > includes several follow-up patches and fixes.
> >
> > Changes from Linux include:
> > * Added Kconfig option
> > * Changed struct device to struct udevice
> > * Changed struct spi_mem to struct spi_slave
> >
> > This patch is obtained from the following patch
> >
> https://patchwork.ozlabs.org/project/uboot/patch/20210205043924.149504
> > -3-sean...@gmail.com/
> >
> > Signed-off-by: Chin-Ting Kuo <chin-ting_...@aspeedtech.com>
> > Signed-off-by: Sean Anderson <sean...@gmail.com>
> > Acked-by: Pratyush Yadav <p.ya...@ti.com>
> > ---
> > v2: Remove "#if CONFIG_SPI_DIRMAP" compile wrapper.
> >
> > drivers/spi/Kconfig | 10 ++
> > drivers/spi/spi-mem.c | 268
> ++++++++++++++++++++++++++++++++++++++++++
> > include/spi-mem.h | 79 +++++++++++++
> > 3 files changed, 357 insertions(+)
> >
> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index
> > a616294910..297253714a 100644
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -40,6 +40,16 @@ config SPI_MEM
> > This extension is meant to simplify interaction with SPI
> memories
> > by providing an high-level interface to send memory-like
> commands.
> >
> > +config SPI_DIRMAP
> > + bool "SPI direct mapping"
> > + depends on SPI_MEM
> > + help
> > + Enable the SPI direct mapping API. Most modern SPI controllers can
> > + directly map a SPI memory (or a portion of the SPI memory) in the
> CPU
> > + address space. Most of the time this brings significant performance
> > + improvements as it automates the whole process of sending SPI
> memory
> > + operations every time a new region is accessed.
> > +
> > if DM_SPI
> >
> > config ALTERA_SPI
> > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index
> > 9c1ede1b61..8e8995fc53 100644
> > --- a/drivers/spi/spi-mem.c
> > +++ b/drivers/spi/spi-mem.c
> > @@ -21,6 +21,8 @@
> > #include <spi.h>
> > #include <spi-mem.h>
> > #include <dm/device_compat.h>
> > +#include <dm/devres.h>
> > +#include <linux/bug.h>
> > #endif
> >
> > #ifndef __UBOOT__
> > @@ -491,6 +493,272 @@ int spi_mem_adjust_op_size(struct spi_slave
> *slave, struct spi_mem_op *op)
> > }
> > EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
> >
> > +static ssize_t spi_mem_no_dirmap_read(struct spi_mem_dirmap_desc
> *desc,
> > + u64 offs, size_t len, void *buf) {
> > + struct spi_mem_op op = desc->info.op_tmpl;
> > + int ret;
> > +
> > + op.addr.val = desc->info.offset + offs;
> > + op.data.buf.in = buf;
> > + op.data.nbytes = len;
> > + ret = spi_mem_adjust_op_size(desc->slave, &op);
> > + if (ret)
> > + return ret;
> > +
> > + ret = spi_mem_exec_op(desc->slave, &op);
> > + if (ret)
> > + return ret;
> > +
> > + return op.data.nbytes;
> > +}
> > +
> > +static ssize_t spi_mem_no_dirmap_write(struct spi_mem_dirmap_desc
> *desc,
> > + u64 offs, size_t len, const void *buf) {
> > + struct spi_mem_op op = desc->info.op_tmpl;
> > + int ret;
> > +
> > + op.addr.val = desc->info.offset + offs;
> > + op.data.buf.out = buf;
> > + op.data.nbytes = len;
> > + ret = spi_mem_adjust_op_size(desc->slave, &op);
> > + if (ret)
> > + return ret;
> > +
> > + ret = spi_mem_exec_op(desc->slave, &op);
> > + if (ret)
> > + return ret;
> > +
> > + return op.data.nbytes;
> > +}
> > +
> > +/**
> > + * spi_mem_dirmap_create() - Create a direct mapping descriptor
> > + * @mem: SPI mem device this direct mapping should be created for
> > + * @info: direct mapping information
> > + *
> > + * This function is creating a direct mapping descriptor which can
> > +then be used
> > + * to access the memory using spi_mem_dirmap_read() or
> spi_mem_dirmap_write().
> > + * If the SPI controller driver does not support direct mapping, this
> > +function
> > + * falls back to an implementation using spi_mem_exec_op(), so that
> > +the caller
> > + * doesn't have to bother implementing a fallback on his own.
> > + *
> > + * Return: a valid pointer in case of success, and ERR_PTR() otherwise.
> > + */
> > +struct spi_mem_dirmap_desc *
> > +spi_mem_dirmap_create(struct spi_slave *slave,
> > + const struct spi_mem_dirmap_info *info) {
> > + struct udevice *bus = slave->dev->parent;
> > + struct dm_spi_ops *ops = spi_get_ops(bus);
> > + struct spi_mem_dirmap_desc *desc;
> > + int ret = -EOPNOTSUPP;
> > +
> > + /* Make sure the number of address cycles is between 1 and 8 bytes. */
> > + if (!info->op_tmpl.addr.nbytes || info->op_tmpl.addr.nbytes > 8)
> > + return ERR_PTR(-EINVAL);
> > +
> > + /* data.dir should either be SPI_MEM_DATA_IN or SPI_MEM_DATA_OUT.
> */
> > + if (info->op_tmpl.data.dir == SPI_MEM_NO_DATA)
> > + return ERR_PTR(-EINVAL);
> > +
> > + desc = kzalloc(sizeof(*desc), GFP_KERNEL);
> > + if (!desc)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + desc->slave = slave;
> > + desc->info = *info;
> > + if (ops->mem_ops && ops->mem_ops->dirmap_create)
> > + ret = ops->mem_ops->dirmap_create(desc);
> > +
> > + if (ret) {
> > + desc->nodirmap = true;
> > + if (!spi_mem_supports_op(desc->slave, &desc->info.op_tmpl))
> > + ret = -EOPNOTSUPP;
> > + else
> > + ret = 0;
> > + }
> > +
> > + if (ret) {
> > + kfree(desc);
> > + return ERR_PTR(ret);
> > + }
> > +
> > + return desc;
> > +}
> > +EXPORT_SYMBOL_GPL(spi_mem_dirmap_create);
> > +
> > +/**
> > + * spi_mem_dirmap_destroy() - Destroy a direct mapping descriptor
> > + * @desc: the direct mapping descriptor to destroy
> > + *
> > + * This function destroys a direct mapping descriptor previously
> > +created by
> > + * spi_mem_dirmap_create().
> > + */
> > +void spi_mem_dirmap_destroy(struct spi_mem_dirmap_desc *desc) {
> > + struct udevice *bus = desc->slave->dev->parent;
> > + struct dm_spi_ops *ops = spi_get_ops(bus);
> > +
> > + if (!desc->nodirmap && ops->mem_ops &&
> ops->mem_ops->dirmap_destroy)
> > + ops->mem_ops->dirmap_destroy(desc);
> > +
> > + kfree(desc);
> > +}
> > +EXPORT_SYMBOL_GPL(spi_mem_dirmap_destroy);
> > +
> > +#ifndef __UBOOT__
> > +static void devm_spi_mem_dirmap_release(struct udevice *dev, void
> > +*res) {
> > + struct spi_mem_dirmap_desc *desc = *(struct spi_mem_dirmap_desc
> > +**)res;
> > +
> > + spi_mem_dirmap_destroy(desc);
> > +}
> > +
> > +/**
> > + * devm_spi_mem_dirmap_create() - Create a direct mapping descriptor
> and attach
> > + * it to a device
> > + * @dev: device the dirmap desc will be attached to
> > + * @mem: SPI mem device this direct mapping should be created for
> > + * @info: direct mapping information
> > + *
> > + * devm_ variant of the spi_mem_dirmap_create() function. See
> > + * spi_mem_dirmap_create() for more details.
> > + *
> > + * Return: a valid pointer in case of success, and ERR_PTR() otherwise.
> > + */
> > +struct spi_mem_dirmap_desc *
> > +devm_spi_mem_dirmap_create(struct udevice *dev, struct spi_slave
> *slave,
> > + const struct spi_mem_dirmap_info *info) {
> > + struct spi_mem_dirmap_desc **ptr, *desc;
> > +
> > + ptr = devres_alloc(devm_spi_mem_dirmap_release, sizeof(*ptr),
> > + GFP_KERNEL);
> > + if (!ptr)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + desc = spi_mem_dirmap_create(slave, info);
> > + if (IS_ERR(desc)) {
> > + devres_free(ptr);
> > + } else {
> > + *ptr = desc;
> > + devres_add(dev, ptr);
> > + }
> > +
> > + return desc;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_spi_mem_dirmap_create);
> > +
> > +static int devm_spi_mem_dirmap_match(struct udevice *dev, void *res,
> > +void *data) {
> > + struct spi_mem_dirmap_desc **ptr = res;
> > +
> > + if (WARN_ON(!ptr || !*ptr))
> > + return 0;
> > +
> > + return *ptr == data;
> > +}
> > +
> > +/**
> > + * devm_spi_mem_dirmap_destroy() - Destroy a direct mapping
> descriptor attached
> > + * to a device
> > + * @dev: device the dirmap desc is attached to
> > + * @desc: the direct mapping descriptor to destroy
> > + *
> > + * devm_ variant of the spi_mem_dirmap_destroy() function. See
> > + * spi_mem_dirmap_destroy() for more details.
> > + */
> > +void devm_spi_mem_dirmap_destroy(struct udevice *dev,
> > + struct spi_mem_dirmap_desc *desc) {
> > + devres_release(dev, devm_spi_mem_dirmap_release,
> > + devm_spi_mem_dirmap_match, desc); }
> > +EXPORT_SYMBOL_GPL(devm_spi_mem_dirmap_destroy);
> > +#endif /* __UBOOT__ */
> > +
> > +/**
> > + * spi_mem_dirmap_read() - Read data through a direct mapping
> > + * @desc: direct mapping descriptor
> > + * @offs: offset to start reading from. Note that this is not an absolute
> > + * offset, but the offset within the direct mapping which already
> has
> > + * its own offset
> > + * @len: length in bytes
> > + * @buf: destination buffer. This buffer must be DMA-able
> > + *
> > + * This function reads data from a memory device using a direct
> > +mapping
> > + * previously instantiated with spi_mem_dirmap_create().
> > + *
> > + * Return: the amount of data read from the memory device or a
> > +negative error
> > + * code. Note that the returned size might be smaller than @len, and
> > +the caller
> > + * is responsible for calling spi_mem_dirmap_read() again when that
> happens.
> > + */
> > +ssize_t spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
> > + u64 offs, size_t len, void *buf) {
> > + struct udevice *bus = desc->slave->dev->parent;
> > + struct dm_spi_ops *ops = spi_get_ops(bus);
> > + ssize_t ret;
> > +
> > + if (desc->info.op_tmpl.data.dir != SPI_MEM_DATA_IN)
> > + return -EINVAL;
> > +
> > + if (!len)
> > + return 0;
> > +
> > + if (desc->nodirmap)
> > + ret = spi_mem_no_dirmap_read(desc, offs, len, buf);
> > + else if (ops->mem_ops && ops->mem_ops->dirmap_read)
> > + ret = ops->mem_ops->dirmap_read(desc, offs, len, buf);
> > + else
> > + ret = -EOPNOTSUPP;
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(spi_mem_dirmap_read);
> > +
> > +/**
> > + * spi_mem_dirmap_write() - Write data through a direct mapping
> > + * @desc: direct mapping descriptor
> > + * @offs: offset to start writing from. Note that this is not an absolute
> > + * offset, but the offset within the direct mapping which already
> has
> > + * its own offset
> > + * @len: length in bytes
> > + * @buf: source buffer. This buffer must be DMA-able
> > + *
> > + * This function writes data to a memory device using a direct
> > +mapping
> > + * previously instantiated with spi_mem_dirmap_create().
> > + *
> > + * Return: the amount of data written to the memory device or a
> > +negative error
> > + * code. Note that the returned size might be smaller than @len, and
> > +the caller
> > + * is responsible for calling spi_mem_dirmap_write() again when that
> happens.
> > + */
> > +ssize_t spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc,
> > + u64 offs, size_t len, const void *buf) {
> > + struct udevice *bus = desc->slave->dev->parent;
> > + struct dm_spi_ops *ops = spi_get_ops(bus);
> > + ssize_t ret;
> > +
> > + if (desc->info.op_tmpl.data.dir != SPI_MEM_DATA_OUT)
> > + return -EINVAL;
> > +
> > + if (!len)
> > + return 0;
> > +
> > + if (desc->nodirmap)
> > + ret = spi_mem_no_dirmap_write(desc, offs, len, buf);
> > + else if (ops->mem_ops && ops->mem_ops->dirmap_write)
> > + ret = ops->mem_ops->dirmap_write(desc, offs, len, buf);
> > + else
> > + ret = -EOPNOTSUPP;
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(spi_mem_dirmap_write);
> > +
> > #ifndef __UBOOT__
> > static inline struct spi_mem_driver *to_spi_mem_drv(struct
> device_driver *drv)
> > {
> > diff --git a/include/spi-mem.h b/include/spi-mem.h index
> > 32ffdc2e0f..e366ec3546 100644
> > --- a/include/spi-mem.h
> > +++ b/include/spi-mem.h
> > @@ -134,6 +134,48 @@ struct spi_mem_op {
> > .dummy = __dummy, \
> > .data = __data, \
> > }
> > +/**
> > + * struct spi_mem_dirmap_info - Direct mapping information
> > + * @op_tmpl: operation template that should be used by the direct
> mapping when
> > + * the memory device is accessed
> > + * @offset: absolute offset this direct mapping is pointing to
> > + * @length: length in byte of this direct mapping
> > + *
> > + * These information are used by the controller specific
> > +implementation to know
>
> nit: This information is
Okay, this modification will be included in the next version.
Chin-Ting
> > + * the portion of memory that is directly mapped and the spi_mem_op
> > +that should
> > + * be used to access the device.
> > + * A direct mapping is only valid for one direction (read or write)
> > +and this
> > + * direction is directly encoded in the ->op_tmpl.data.dir field.
> > + */
> > +struct spi_mem_dirmap_info {
> > + struct spi_mem_op op_tmpl;
> > + u64 offset;
> > + u64 length;
> > +};
> > +
> > +/**
> > + * struct spi_mem_dirmap_desc - Direct mapping descriptor
> > + * @mem: the SPI memory device this direct mapping is attached to
> > + * @info: information passed at direct mapping creation time
> > + * @nodirmap: set to 1 if the SPI controller does not implement
> > + * ->mem_ops->dirmap_create() or when this function
> returned an
> > + * error. If @nodirmap is true, all
> spi_mem_dirmap_{read,write}()
> > + * calls will use spi_mem_exec_op() to access the memory.
> This is a
> > + * degraded mode that allows spi_mem drivers to use the
> same code
> > + * no matter whether the controller supports direct
> mapping or not
> > + * @priv: field pointing to controller specific data
> > + *
> > + * Common part of a direct mapping descriptor. This object is created
> > +by
> > + * spi_mem_dirmap_create() and controller implementation of
> > +->create_dirmap()
> > + * can create/attach direct mapping resources to the descriptor in
> > +the ->priv
> > + * field.
> > + */
> > +struct spi_mem_dirmap_desc {
> > + struct spi_slave *slave;
> > + struct spi_mem_dirmap_info info;
> > + unsigned int nodirmap;
> > + void *priv;
> > +};
> >
> > #ifndef __UBOOT__
> > /**
> > @@ -183,10 +225,32 @@ static inline void *spi_mem_get_drvdata(struct
> spi_mem *mem)
> > * limitations)
> > * @supports_op: check if an operation is supported by the controller
> > * @exec_op: execute a SPI memory operation
> > + * @dirmap_create: create a direct mapping descriptor that can later be
> used to
> > + * access the memory device. This method is optional
> > + * @dirmap_destroy: destroy a memory descriptor previous created by
> > + * ->dirmap_create()
> > + * @dirmap_read: read data from the memory device using the direct
> mapping
> > + * created by ->dirmap_create(). The function can return less
> > + * data than requested (for example when the request is
> crossing
> > + * the currently mapped area), and the caller of
> > + * spi_mem_dirmap_read() is responsible for calling it again in
> > + * this case.
> > + * @dirmap_write: write data to the memory device using the direct
> mapping
> > + * created by ->dirmap_create(). The function can return less
> > + * data than requested (for example when the request is
> crossing
> > + * the currently mapped area), and the caller of
> > + * spi_mem_dirmap_write() is responsible for calling it again
> in
> > + * this case.
> > *
> > * This interface should be implemented by SPI controllers providing an
> > * high-level interface to execute SPI memory operation, which is
> usually the
> > * case for QSPI controllers.
> > + *
> > + * Note on ->dirmap_{read,write}(): drivers should avoid accessing
> > + the direct
> > + * mapping from the CPU because doing that can stall the CPU waiting
> > + for the
> > + * SPI mem transaction to finish, and this will make real-time
> > + maintainers
> > + * unhappy and might make your system less reactive. Instead, drivers
> > + should
> > + * use DMA to access this direct mapping.
> > */
> > struct spi_controller_mem_ops {
> > int (*adjust_op_size)(struct spi_slave *slave, struct spi_mem_op
> > *op); @@ -194,6 +258,12 @@ struct spi_controller_mem_ops {
> > const struct spi_mem_op *op);
> > int (*exec_op)(struct spi_slave *slave,
> > const struct spi_mem_op *op);
> > + int (*dirmap_create)(struct spi_mem_dirmap_desc *desc);
> > + void (*dirmap_destroy)(struct spi_mem_dirmap_desc *desc);
> > + ssize_t (*dirmap_read)(struct spi_mem_dirmap_desc *desc,
> > + u64 offs, size_t len, void *buf);
> > + ssize_t (*dirmap_write)(struct spi_mem_dirmap_desc *desc,
> > + u64 offs, size_t len, const void *buf);
> > };
> >
> > #ifndef __UBOOT__
> > @@ -260,6 +330,15 @@ int spi_mem_exec_op(struct spi_slave *slave,
> const struct spi_mem_op *op);
> > bool spi_mem_default_supports_op(struct spi_slave *mem,
> > const struct spi_mem_op *op);
> >
> > +struct spi_mem_dirmap_desc *
> > +spi_mem_dirmap_create(struct spi_slave *mem,
> > + const struct spi_mem_dirmap_info *info); void
> > +spi_mem_dirmap_destroy(struct spi_mem_dirmap_desc *desc); ssize_t
> > +spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
> > + u64 offs, size_t len, void *buf); ssize_t
> > +spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc,
> > + u64 offs, size_t len, const void *buf);
> > +
> > #ifndef __UBOOT__
> > int spi_mem_driver_register_with_owner(struct spi_mem_driver *drv,
> > struct module *owner);
> >
