Hi Cédric,

> -----Original Message-----
> From: Cédric Le Goater <c...@kaod.org>
> Sent: Friday, July 1, 2022 5:37 PM
> Subject: Re: [v4 07/12] spi-mem: Add dirmap API from Linux
> 
> On 5/24/22 07:56, 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/
> 
> 
> It has been sent long ago. Is there an issue with the backport from Linux ?

No.

> Is it the lack of drivers using it ?

Yes and thus, it is postponed at that time.

> 
> Thanks,
> 
> C.
> 
> >
> > 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.
> > v3: Fix a grammatical error in spi-mem.h.
> >
> >   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..b07cf2ed83 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
> > + *
> > + * This information is used by the controller specific implementation
> > +to know
> > + * 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);

Reply via email to