From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E83D0C43334 for ; Fri, 1 Jul 2022 09:37:02 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 6518D84433; Fri, 1 Jul 2022 11:37:00 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=kaod.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 5300483E60; Fri, 1 Jul 2022 11:36:58 +0200 (CEST) Received: from 3.mo548.mail-out.ovh.net (3.mo548.mail-out.ovh.net [188.165.32.156]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 43E5783AC3 for ; Fri, 1 Jul 2022 11:36:52 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=kaod.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=clg@kaod.org Received: from mxplan5.mail.ovh.net (unknown [10.108.4.141]) by mo548.mail-out.ovh.net (Postfix) with ESMTPS id 39ECE21080; Fri, 1 Jul 2022 09:36:51 +0000 (UTC) Received: from kaod.org (37.59.142.98) by DAG4EX1.mxp5.local (172.16.2.31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.9; Fri, 1 Jul 2022 11:36:50 +0200 Authentication-Results: garm.ovh; auth=pass (GARM-98R002b1ce94af-a61a-4026-b327-37de1076848d, D594F7ECFE8920C5FAE37748FDFD57525B476B5E) smtp.auth=clg@kaod.org X-OVh-ClientIp: 82.64.250.170 Message-ID: <016a6a24-b17c-df8c-8a37-40fe46e9fadf@kaod.org> Date: Fri, 1 Jul 2022 11:36:49 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [v4 07/12] spi-mem: Add dirmap API from Linux Content-Language: en-US To: Chin-Ting Kuo , , , , , , , , , References: <20220524055650.1115899-1-chin-ting_kuo@aspeedtech.com> <20220524055650.1115899-8-chin-ting_kuo@aspeedtech.com> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= In-Reply-To: <20220524055650.1115899-8-chin-ting_kuo@aspeedtech.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [37.59.142.98] X-ClientProxiedBy: DAG4EX2.mxp5.local (172.16.2.32) To DAG4EX1.mxp5.local (172.16.2.31) X-Ovh-Tracer-GUID: b99c5390-0da1-49fa-9e8d-d6cea665b47e X-Ovh-Tracer-Id: 13849131805609069490 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: 0 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedvfedrudehfedgudekucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecuhedttdenucenucfjughrpefkffggfgfuvfhfhfgjtgfgihesthejredttdefjeenucfhrhhomhepveorughrihgtpgfnvggpifhorghtvghruceotghlgheskhgrohgurdhorhhgqeenucggtffrrghtthgvrhhnpeeggfdujeeljeduvefhteeghedukeehhffgtdeuleejffefvdevveelueeileejffenucffohhmrghinhepohiilhgrsghsrdhorhhgpdgsuhhfrdhinhenucfkpheptddrtddrtddrtddpfeejrdehledrudegvddrleeknecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmohguvgepshhmthhpohhuthdphhgvlhhopehmgihplhgrnhehrdhmrghilhdrohhvhhdrnhgvthdpihhnvghtpedtrddtrddtrddtpdhmrghilhhfrhhomheptghlgheskhgrohgurdhorhhgpdhnsggprhgtphhtthhopedupdhrtghpthhtohepphdrhigruggrvhesthhirdgtohhmpdfovfetjfhoshhtpehmohehgeek X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean 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-seanga2@gmail.com/ It has been sent long ago. Is there an issue with the backport from Linux ? Is it the lack of drivers using it ? Thanks, C. > > Signed-off-by: Chin-Ting Kuo > Signed-off-by: Sean Anderson > Acked-by: Pratyush Yadav > --- > 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 > #include > #include > +#include > +#include > #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);