* [PATCH 0/3] Misc fixes/cleanup
@ 2020-08-07 13:30 Sean Anderson
2020-08-07 13:30 ` [PATCH 1/3] doc: Fix typo in FIT documentation Sean Anderson
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Sean Anderson @ 2020-08-07 13:30 UTC (permalink / raw
To: u-boot
These are some miscelleneous fixes which can be applied separately from each
other. I have grouped them in the same series, but they do not depend on one
another.
Sean Anderson (3):
doc: Fix typo in FIT documentation
spi: Remove uses of #ifdef __U_BOOT__ from spi-mem.c
spi: Fix typo in header
doc/uImage.FIT/source_file_format.txt | 2 +-
drivers/spi/spi-mem.c | 283 --------------------------
include/spi.h | 2 +-
3 files changed, 2 insertions(+), 285 deletions(-)
--
2.28.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] doc: Fix typo in FIT documentation
2020-08-07 13:30 [PATCH 0/3] Misc fixes/cleanup Sean Anderson
@ 2020-08-07 13:30 ` Sean Anderson
2020-08-07 13:30 ` [PATCH 2/3] spi: Remove uses of #ifdef __U_BOOT__ from spi-mem.c Sean Anderson
2020-08-07 13:30 ` [PATCH 3/3] spi: Fix typo in header Sean Anderson
2 siblings, 0 replies; 6+ messages in thread
From: Sean Anderson @ 2020-08-07 13:30 UTC (permalink / raw
To: u-boot
u_boot should be u-boot
Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
doc/uImage.FIT/source_file_format.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt
index 884a58456f..633f227c59 100644
--- a/doc/uImage.FIT/source_file_format.txt
+++ b/doc/uImage.FIT/source_file_format.txt
@@ -172,7 +172,7 @@ the '/images' node should have the following layout:
- os : OS name, mandatory for types "kernel" and "ramdisk". Valid OS names
are: "openbsd", "netbsd", "freebsd", "4_4bsd", "linux", "svr4", "esix",
"solaris", "irix", "sco", "dell", "ncr", "lynxos", "vxworks", "psos", "qnx",
- "u_boot", "rtems", "unity", "integrity".
+ "u-boot", "rtems", "unity", "integrity".
- arch : Architecture name, mandatory for types: "standalone", "kernel",
"firmware", "ramdisk" and "fdt". Valid architecture names are: "alpha",
"arm", "i386", "ia64", "mips", "mips64", "ppc", "s390", "sh", "sparc",
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] spi: Remove uses of #ifdef __U_BOOT__ from spi-mem.c
2020-08-07 13:30 [PATCH 0/3] Misc fixes/cleanup Sean Anderson
2020-08-07 13:30 ` [PATCH 1/3] doc: Fix typo in FIT documentation Sean Anderson
@ 2020-08-07 13:30 ` Sean Anderson
2020-08-07 16:48 ` Pratyush Yadav
2020-08-07 13:30 ` [PATCH 3/3] spi: Fix typo in header Sean Anderson
2 siblings, 1 reply; 6+ messages in thread
From: Sean Anderson @ 2020-08-07 13:30 UTC (permalink / raw
To: u-boot
Preprocessing out large sections of the file is confusing and makes it
difficult to follow the control flow. Presumably these were initially added
to make porting easier, but this code has not been synced with Linux since
it was introduced two years ago.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
drivers/spi/spi-mem.c | 283 ------------------------------------------
1 file changed, 283 deletions(-)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index c095ae9505..36f3eb7f1e 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -6,109 +6,8 @@
* Author: Boris Brezillon <boris.brezillon@bootlin.com>
*/
-#ifndef __UBOOT__
-#include <log.h>
-#include <dm/devres.h>
-#include <linux/dmaengine.h>
-#include <linux/pm_runtime.h>
-#include "internals.h"
-#else
-#include <common.h>
-#include <dm.h>
-#include <errno.h>
-#include <malloc.h>
-#include <spi.h>
#include <spi.h>
#include <spi-mem.h>
-#include <dm/device_compat.h>
-#endif
-
-#ifndef __UBOOT__
-/**
- * spi_controller_dma_map_mem_op_data() - DMA-map the buffer attached to a
- * memory operation
- * @ctlr: the SPI controller requesting this dma_map()
- * @op: the memory operation containing the buffer to map
- * @sgt: a pointer to a non-initialized sg_table that will be filled by this
- * function
- *
- * Some controllers might want to do DMA on the data buffer embedded in @op.
- * This helper prepares everything for you and provides a ready-to-use
- * sg_table. This function is not intended to be called from spi drivers.
- * Only SPI controller drivers should use it.
- * Note that the caller must ensure the memory region pointed by
- * op->data.buf.{in,out} is DMA-able before calling this function.
- *
- * Return: 0 in case of success, a negative error code otherwise.
- */
-int spi_controller_dma_map_mem_op_data(struct spi_controller *ctlr,
- const struct spi_mem_op *op,
- struct sg_table *sgt)
-{
- struct device *dmadev;
-
- if (!op->data.nbytes)
- return -EINVAL;
-
- if (op->data.dir == SPI_MEM_DATA_OUT && ctlr->dma_tx)
- dmadev = ctlr->dma_tx->device->dev;
- else if (op->data.dir == SPI_MEM_DATA_IN && ctlr->dma_rx)
- dmadev = ctlr->dma_rx->device->dev;
- else
- dmadev = ctlr->dev.parent;
-
- if (!dmadev)
- return -EINVAL;
-
- return spi_map_buf(ctlr, dmadev, sgt, op->data.buf.in, op->data.nbytes,
- op->data.dir == SPI_MEM_DATA_IN ?
- DMA_FROM_DEVICE : DMA_TO_DEVICE);
-}
-EXPORT_SYMBOL_GPL(spi_controller_dma_map_mem_op_data);
-
-/**
- * spi_controller_dma_unmap_mem_op_data() - DMA-unmap the buffer attached to a
- * memory operation
- * @ctlr: the SPI controller requesting this dma_unmap()
- * @op: the memory operation containing the buffer to unmap
- * @sgt: a pointer to an sg_table previously initialized by
- * spi_controller_dma_map_mem_op_data()
- *
- * Some controllers might want to do DMA on the data buffer embedded in @op.
- * This helper prepares things so that the CPU can access the
- * op->data.buf.{in,out} buffer again.
- *
- * This function is not intended to be called from SPI drivers. Only SPI
- * controller drivers should use it.
- *
- * This function should be called after the DMA operation has finished and is
- * only valid if the previous spi_controller_dma_map_mem_op_data() call
- * returned 0.
- *
- * Return: 0 in case of success, a negative error code otherwise.
- */
-void spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
- const struct spi_mem_op *op,
- struct sg_table *sgt)
-{
- struct device *dmadev;
-
- if (!op->data.nbytes)
- return;
-
- if (op->data.dir == SPI_MEM_DATA_OUT && ctlr->dma_tx)
- dmadev = ctlr->dma_tx->device->dev;
- else if (op->data.dir == SPI_MEM_DATA_IN && ctlr->dma_rx)
- dmadev = ctlr->dma_rx->device->dev;
- else
- dmadev = ctlr->dev.parent;
-
- spi_unmap_buf(ctlr, dmadev, sgt,
- op->data.dir == SPI_MEM_DATA_IN ?
- DMA_FROM_DEVICE : DMA_TO_DEVICE);
-}
-EXPORT_SYMBOL_GPL(spi_controller_dma_unmap_mem_op_data);
-#endif /* __UBOOT__ */
static int spi_check_buswidth_req(struct spi_slave *slave, u8 buswidth, bool tx)
{
@@ -166,7 +65,6 @@ bool spi_mem_default_supports_op(struct spi_slave *slave,
return true;
}
-EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);
/**
* spi_mem_supports_op() - Check if a memory device and the controller it is
@@ -194,7 +92,6 @@ bool spi_mem_supports_op(struct spi_slave *slave,
return spi_mem_default_supports_op(slave, op);
}
-EXPORT_SYMBOL_GPL(spi_mem_supports_op);
/**
* spi_mem_exec_op() - Execute a memory operation
@@ -228,36 +125,8 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
return ret;
if (ops->mem_ops && ops->mem_ops->exec_op) {
-#ifndef __UBOOT__
- /*
- * Flush the message queue before executing our SPI memory
- * operation to prevent preemption of regular SPI transfers.
- */
- spi_flush_queue(ctlr);
-
- if (ctlr->auto_runtime_pm) {
- ret = pm_runtime_get_sync(ctlr->dev.parent);
- if (ret < 0) {
- dev_err(&ctlr->dev,
- "Failed to power device: %d\n",
- ret);
- return ret;
- }
- }
-
- mutex_lock(&ctlr->bus_lock_mutex);
- mutex_lock(&ctlr->io_mutex);
-#endif
ret = ops->mem_ops->exec_op(slave, op);
-#ifndef __UBOOT__
- mutex_unlock(&ctlr->io_mutex);
- mutex_unlock(&ctlr->bus_lock_mutex);
-
- if (ctlr->auto_runtime_pm)
- pm_runtime_put(ctlr->dev.parent);
-#endif
-
/*
* Some controllers only optimize specific paths (typically the
* read path) and expect the core to use the regular SPI
@@ -269,80 +138,6 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
}
}
-#ifndef __UBOOT__
- tmpbufsize = sizeof(op->cmd.opcode) + op->addr.nbytes +
- op->dummy.nbytes;
-
- /*
- * Allocate a buffer to transmit the CMD, ADDR cycles with kmalloc() so
- * we're guaranteed that this buffer is DMA-able, as required by the
- * SPI layer.
- */
- tmpbuf = kzalloc(tmpbufsize, GFP_KERNEL | GFP_DMA);
- if (!tmpbuf)
- return -ENOMEM;
-
- spi_message_init(&msg);
-
- tmpbuf[0] = op->cmd.opcode;
- xfers[xferpos].tx_buf = tmpbuf;
- xfers[xferpos].len = sizeof(op->cmd.opcode);
- xfers[xferpos].tx_nbits = op->cmd.buswidth;
- spi_message_add_tail(&xfers[xferpos], &msg);
- xferpos++;
- totalxferlen++;
-
- if (op->addr.nbytes) {
- int i;
-
- for (i = 0; i < op->addr.nbytes; i++)
- tmpbuf[i + 1] = op->addr.val >>
- (8 * (op->addr.nbytes - i - 1));
-
- xfers[xferpos].tx_buf = tmpbuf + 1;
- xfers[xferpos].len = op->addr.nbytes;
- xfers[xferpos].tx_nbits = op->addr.buswidth;
- spi_message_add_tail(&xfers[xferpos], &msg);
- xferpos++;
- totalxferlen += op->addr.nbytes;
- }
-
- if (op->dummy.nbytes) {
- memset(tmpbuf + op->addr.nbytes + 1, 0xff, op->dummy.nbytes);
- xfers[xferpos].tx_buf = tmpbuf + op->addr.nbytes + 1;
- xfers[xferpos].len = op->dummy.nbytes;
- xfers[xferpos].tx_nbits = op->dummy.buswidth;
- spi_message_add_tail(&xfers[xferpos], &msg);
- xferpos++;
- totalxferlen += op->dummy.nbytes;
- }
-
- if (op->data.nbytes) {
- if (op->data.dir == SPI_MEM_DATA_IN) {
- xfers[xferpos].rx_buf = op->data.buf.in;
- xfers[xferpos].rx_nbits = op->data.buswidth;
- } else {
- xfers[xferpos].tx_buf = op->data.buf.out;
- xfers[xferpos].tx_nbits = op->data.buswidth;
- }
-
- xfers[xferpos].len = op->data.nbytes;
- spi_message_add_tail(&xfers[xferpos], &msg);
- xferpos++;
- totalxferlen += op->data.nbytes;
- }
-
- ret = spi_sync(slave, &msg);
-
- kfree(tmpbuf);
-
- if (ret)
- return ret;
-
- if (msg.actual_length != totalxferlen)
- return -EIO;
-#else
-
if (op->data.nbytes) {
if (op->data.dir == SPI_MEM_DATA_IN)
rx_buf = op->data.buf.in;
@@ -407,11 +202,9 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
if (ret < 0)
return ret;
-#endif /* __UBOOT__ */
return 0;
}
-EXPORT_SYMBOL_GPL(spi_mem_exec_op);
/**
* spi_mem_adjust_op_size() - Adjust the data size of a SPI mem operation to
@@ -459,79 +252,3 @@ int spi_mem_adjust_op_size(struct spi_slave *slave, struct spi_mem_op *op)
return 0;
}
-EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
-
-#ifndef __UBOOT__
-static inline struct spi_mem_driver *to_spi_mem_drv(struct device_driver *drv)
-{
- return container_of(drv, struct spi_mem_driver, spidrv.driver);
-}
-
-static int spi_mem_probe(struct spi_device *spi)
-{
- struct spi_mem_driver *memdrv = to_spi_mem_drv(spi->dev.driver);
- struct spi_mem *mem;
-
- mem = devm_kzalloc(&spi->dev, sizeof(*mem), GFP_KERNEL);
- if (!mem)
- return -ENOMEM;
-
- mem->spi = spi;
- spi_set_drvdata(spi, mem);
-
- return memdrv->probe(mem);
-}
-
-static int spi_mem_remove(struct spi_device *spi)
-{
- struct spi_mem_driver *memdrv = to_spi_mem_drv(spi->dev.driver);
- struct spi_mem *mem = spi_get_drvdata(spi);
-
- if (memdrv->remove)
- return memdrv->remove(mem);
-
- return 0;
-}
-
-static void spi_mem_shutdown(struct spi_device *spi)
-{
- struct spi_mem_driver *memdrv = to_spi_mem_drv(spi->dev.driver);
- struct spi_mem *mem = spi_get_drvdata(spi);
-
- if (memdrv->shutdown)
- memdrv->shutdown(mem);
-}
-
-/**
- * spi_mem_driver_register_with_owner() - Register a SPI memory driver
- * @memdrv: the SPI memory driver to register
- * @owner: the owner of this driver
- *
- * Registers a SPI memory driver.
- *
- * Return: 0 in case of success, a negative error core otherwise.
- */
-
-int spi_mem_driver_register_with_owner(struct spi_mem_driver *memdrv,
- struct module *owner)
-{
- memdrv->spidrv.probe = spi_mem_probe;
- memdrv->spidrv.remove = spi_mem_remove;
- memdrv->spidrv.shutdown = spi_mem_shutdown;
-
- return __spi_register_driver(owner, &memdrv->spidrv);
-}
-EXPORT_SYMBOL_GPL(spi_mem_driver_register_with_owner);
-
-/**
- * spi_mem_driver_unregister_with_owner() - Unregister a SPI memory driver
- * @memdrv: the SPI memory driver to unregister
- *
- * Unregisters a SPI memory driver.
- */
-void spi_mem_driver_unregister(struct spi_mem_driver *memdrv)
-{
- spi_unregister_driver(&memdrv->spidrv);
-}
-EXPORT_SYMBOL_GPL(spi_mem_driver_unregister);
-#endif /* __UBOOT__ */
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] spi: Fix typo in header
2020-08-07 13:30 [PATCH 0/3] Misc fixes/cleanup Sean Anderson
2020-08-07 13:30 ` [PATCH 1/3] doc: Fix typo in FIT documentation Sean Anderson
2020-08-07 13:30 ` [PATCH 2/3] spi: Remove uses of #ifdef __U_BOOT__ from spi-mem.c Sean Anderson
@ 2020-08-07 13:30 ` Sean Anderson
2 siblings, 0 replies; 6+ messages in thread
From: Sean Anderson @ 2020-08-07 13:30 UTC (permalink / raw
To: u-boot
Spelling.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
include/spi.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/spi.h b/include/spi.h
index ef8c1f6692..2d34e4af11 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -51,7 +51,7 @@ struct dm_spi_bus {
* struct from a spi_slave, use dev_get_parent_platdata(dev) or
* dev_get_parent_platdata(slave->dev).
*
- * This data is immuatable. Each time the device is probed, @max_hz and @mode
+ * This data is immutable. Each time the device is probed, @max_hz and @mode
* will be copied to struct spi_slave.
*
* @cs: Chip select number (0..n-1)
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] spi: Remove uses of #ifdef __U_BOOT__ from spi-mem.c
2020-08-07 13:30 ` [PATCH 2/3] spi: Remove uses of #ifdef __U_BOOT__ from spi-mem.c Sean Anderson
@ 2020-08-07 16:48 ` Pratyush Yadav
2020-08-07 16:55 ` Sean Anderson
0 siblings, 1 reply; 6+ messages in thread
From: Pratyush Yadav @ 2020-08-07 16:48 UTC (permalink / raw
To: u-boot
Hi Sean,
> Subject: [PATCH 2/3] spi: Remove uses of #ifdef __U_BOOT__ from spi-mem.c
Maybe s/ifdef/ifndef/
On 07/08/20 09:30AM, Sean Anderson wrote:
> Preprocessing out large sections of the file is confusing and makes it
> difficult to follow the control flow. Presumably these were initially added
> to make porting easier, but this code has not been synced with Linux since
> it was introduced two years ago.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
> drivers/spi/spi-mem.c | 283 ------------------------------------------
> 1 file changed, 283 deletions(-)
>
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index c095ae9505..36f3eb7f1e 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -6,109 +6,8 @@
> * Author: Boris Brezillon <boris.brezillon@bootlin.com>
> */
>
> -#ifndef __UBOOT__
> -#include <log.h>
> -#include <dm/devres.h>
> -#include <linux/dmaengine.h>
> -#include <linux/pm_runtime.h>
> -#include "internals.h"
> -#else
> -#include <common.h>
> -#include <dm.h>
> -#include <errno.h>
> -#include <malloc.h>
> -#include <spi.h>
> #include <spi.h>
> #include <spi-mem.h>
> -#include <dm/device_compat.h>
Why remove these headers? They are not part of a #ifndef __UBOOT__. The
build fails with this patch applied on latest master:
drivers/spi/spi-mem.c: In function ?spi_mem_supports_op?:
drivers/spi/spi-mem.c:87:34: error: dereferencing pointer to incomplete type ?struct udevice?
struct udevice *bus = slave->dev->parent;
^~
drivers/spi/spi-mem.c: In function ?spi_mem_exec_op?:
drivers/spi/spi-mem.c:195:3: warning: implicit declaration of function ?debug?; did you mean ?pr_debug?? [-Wimplicit-function-declaration]
debug("%02x ", op_buf[i]);
^~~~~
pr_debug
> -#endif
> -
> -#ifndef __UBOOT__
> -/**
> - * spi_controller_dma_map_mem_op_data() - DMA-map the buffer attached to a
> - * memory operation
> - * @ctlr: the SPI controller requesting this dma_map()
> - * @op: the memory operation containing the buffer to map
> - * @sgt: a pointer to a non-initialized sg_table that will be filled by this
> - * function
> - *
> - * Some controllers might want to do DMA on the data buffer embedded in @op.
> - * This helper prepares everything for you and provides a ready-to-use
> - * sg_table. This function is not intended to be called from spi drivers.
> - * Only SPI controller drivers should use it.
> - * Note that the caller must ensure the memory region pointed by
> - * op->data.buf.{in,out} is DMA-able before calling this function.
> - *
> - * Return: 0 in case of success, a negative error code otherwise.
> - */
> -int spi_controller_dma_map_mem_op_data(struct spi_controller *ctlr,
> - const struct spi_mem_op *op,
> - struct sg_table *sgt)
> -{
> - struct device *dmadev;
> -
> - if (!op->data.nbytes)
> - return -EINVAL;
> -
> - if (op->data.dir == SPI_MEM_DATA_OUT && ctlr->dma_tx)
> - dmadev = ctlr->dma_tx->device->dev;
> - else if (op->data.dir == SPI_MEM_DATA_IN && ctlr->dma_rx)
> - dmadev = ctlr->dma_rx->device->dev;
> - else
> - dmadev = ctlr->dev.parent;
> -
> - if (!dmadev)
> - return -EINVAL;
> -
> - return spi_map_buf(ctlr, dmadev, sgt, op->data.buf.in, op->data.nbytes,
> - op->data.dir == SPI_MEM_DATA_IN ?
> - DMA_FROM_DEVICE : DMA_TO_DEVICE);
> -}
> -EXPORT_SYMBOL_GPL(spi_controller_dma_map_mem_op_data);
> -
> -/**
> - * spi_controller_dma_unmap_mem_op_data() - DMA-unmap the buffer attached to a
> - * memory operation
> - * @ctlr: the SPI controller requesting this dma_unmap()
> - * @op: the memory operation containing the buffer to unmap
> - * @sgt: a pointer to an sg_table previously initialized by
> - * spi_controller_dma_map_mem_op_data()
> - *
> - * Some controllers might want to do DMA on the data buffer embedded in @op.
> - * This helper prepares things so that the CPU can access the
> - * op->data.buf.{in,out} buffer again.
> - *
> - * This function is not intended to be called from SPI drivers. Only SPI
> - * controller drivers should use it.
> - *
> - * This function should be called after the DMA operation has finished and is
> - * only valid if the previous spi_controller_dma_map_mem_op_data() call
> - * returned 0.
> - *
> - * Return: 0 in case of success, a negative error code otherwise.
> - */
> -void spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
> - const struct spi_mem_op *op,
> - struct sg_table *sgt)
> -{
> - struct device *dmadev;
> -
> - if (!op->data.nbytes)
> - return;
> -
> - if (op->data.dir == SPI_MEM_DATA_OUT && ctlr->dma_tx)
> - dmadev = ctlr->dma_tx->device->dev;
> - else if (op->data.dir == SPI_MEM_DATA_IN && ctlr->dma_rx)
> - dmadev = ctlr->dma_rx->device->dev;
> - else
> - dmadev = ctlr->dev.parent;
> -
> - spi_unmap_buf(ctlr, dmadev, sgt,
> - op->data.dir == SPI_MEM_DATA_IN ?
> - DMA_FROM_DEVICE : DMA_TO_DEVICE);
> -}
> -EXPORT_SYMBOL_GPL(spi_controller_dma_unmap_mem_op_data);
> -#endif /* __UBOOT__ */
>
> static int spi_check_buswidth_req(struct spi_slave *slave, u8 buswidth, bool tx)
> {
> @@ -166,7 +65,6 @@ bool spi_mem_default_supports_op(struct spi_slave *slave,
>
> return true;
> }
> -EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);
Nitpick: Since this patch is about removing things in #ifndef __UBOOT_,
please don't remove things outside of it. Do them as a separate patch
instead. Same for all other places below.
I am neither endorsing this patch nor opposing it, but please at least
make sure it builds.
--
Regards,
Pratyush Yadav
Texas Instruments India
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] spi: Remove uses of #ifdef __U_BOOT__ from spi-mem.c
2020-08-07 16:48 ` Pratyush Yadav
@ 2020-08-07 16:55 ` Sean Anderson
0 siblings, 0 replies; 6+ messages in thread
From: Sean Anderson @ 2020-08-07 16:55 UTC (permalink / raw
To: u-boot
On 8/7/20 12:48 PM, Pratyush Yadav wrote:
> Hi Sean,
>
>> Subject: [PATCH 2/3] spi: Remove uses of #ifdef __U_BOOT__ from spi-mem.c
>
> Maybe s/ifdef/ifndef/
Sure.
>
> On 07/08/20 09:30AM, Sean Anderson wrote:
>> Preprocessing out large sections of the file is confusing and makes it
>> difficult to follow the control flow. Presumably these were initially added
>> to make porting easier, but this code has not been synced with Linux since
>> it was introduced two years ago.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>> drivers/spi/spi-mem.c | 283 ------------------------------------------
>> 1 file changed, 283 deletions(-)
>>
>> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
>> index c095ae9505..36f3eb7f1e 100644
>> --- a/drivers/spi/spi-mem.c
>> +++ b/drivers/spi/spi-mem.c
>> @@ -6,109 +6,8 @@
>> * Author: Boris Brezillon <boris.brezillon@bootlin.com>
>> */
>>
>> -#ifndef __UBOOT__
>> -#include <log.h>
>> -#include <dm/devres.h>
>> -#include <linux/dmaengine.h>
>> -#include <linux/pm_runtime.h>
>> -#include "internals.h"
>> -#else
>> -#include <common.h>
>> -#include <dm.h>
>> -#include <errno.h>
>> -#include <malloc.h>
>> -#include <spi.h>
>> #include <spi.h>
>> #include <spi-mem.h>
>> -#include <dm/device_compat.h>
>
> Why remove these headers? They are not part of a #ifndef __UBOOT__. The
> build fails with this patch applied on latest master:
Hm, I'm not sure how those got deleted. I originally planned to submit
this in another series, so perhaps I made an error when cherry-picking.
I will resubmit this with those headers included.
>
> drivers/spi/spi-mem.c: In function ?spi_mem_supports_op?:
> drivers/spi/spi-mem.c:87:34: error: dereferencing pointer to incomplete type ?struct udevice?
> struct udevice *bus = slave->dev->parent;
> ^~
> drivers/spi/spi-mem.c: In function ?spi_mem_exec_op?:
> drivers/spi/spi-mem.c:195:3: warning: implicit declaration of function ?debug?; did you mean ?pr_debug?? [-Wimplicit-function-declaration]
> debug("%02x ", op_buf[i]);
> ^~~~~
> pr_debug
>
>> -#endif
>> -
>> -#ifndef __UBOOT__
>> -/**
>> - * spi_controller_dma_map_mem_op_data() - DMA-map the buffer attached to a
>> - * memory operation
>> - * @ctlr: the SPI controller requesting this dma_map()
>> - * @op: the memory operation containing the buffer to map
>> - * @sgt: a pointer to a non-initialized sg_table that will be filled by this
>> - * function
>> - *
>> - * Some controllers might want to do DMA on the data buffer embedded in @op.
>> - * This helper prepares everything for you and provides a ready-to-use
>> - * sg_table. This function is not intended to be called from spi drivers.
>> - * Only SPI controller drivers should use it.
>> - * Note that the caller must ensure the memory region pointed by
>> - * op->data.buf.{in,out} is DMA-able before calling this function.
>> - *
>> - * Return: 0 in case of success, a negative error code otherwise.
>> - */
>> -int spi_controller_dma_map_mem_op_data(struct spi_controller *ctlr,
>> - const struct spi_mem_op *op,
>> - struct sg_table *sgt)
>> -{
>> - struct device *dmadev;
>> -
>> - if (!op->data.nbytes)
>> - return -EINVAL;
>> -
>> - if (op->data.dir == SPI_MEM_DATA_OUT && ctlr->dma_tx)
>> - dmadev = ctlr->dma_tx->device->dev;
>> - else if (op->data.dir == SPI_MEM_DATA_IN && ctlr->dma_rx)
>> - dmadev = ctlr->dma_rx->device->dev;
>> - else
>> - dmadev = ctlr->dev.parent;
>> -
>> - if (!dmadev)
>> - return -EINVAL;
>> -
>> - return spi_map_buf(ctlr, dmadev, sgt, op->data.buf.in, op->data.nbytes,
>> - op->data.dir == SPI_MEM_DATA_IN ?
>> - DMA_FROM_DEVICE : DMA_TO_DEVICE);
>> -}
>> -EXPORT_SYMBOL_GPL(spi_controller_dma_map_mem_op_data);
>> -
>> -/**
>> - * spi_controller_dma_unmap_mem_op_data() - DMA-unmap the buffer attached to a
>> - * memory operation
>> - * @ctlr: the SPI controller requesting this dma_unmap()
>> - * @op: the memory operation containing the buffer to unmap
>> - * @sgt: a pointer to an sg_table previously initialized by
>> - * spi_controller_dma_map_mem_op_data()
>> - *
>> - * Some controllers might want to do DMA on the data buffer embedded in @op.
>> - * This helper prepares things so that the CPU can access the
>> - * op->data.buf.{in,out} buffer again.
>> - *
>> - * This function is not intended to be called from SPI drivers. Only SPI
>> - * controller drivers should use it.
>> - *
>> - * This function should be called after the DMA operation has finished and is
>> - * only valid if the previous spi_controller_dma_map_mem_op_data() call
>> - * returned 0.
>> - *
>> - * Return: 0 in case of success, a negative error code otherwise.
>> - */
>> -void spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
>> - const struct spi_mem_op *op,
>> - struct sg_table *sgt)
>> -{
>> - struct device *dmadev;
>> -
>> - if (!op->data.nbytes)
>> - return;
>> -
>> - if (op->data.dir == SPI_MEM_DATA_OUT && ctlr->dma_tx)
>> - dmadev = ctlr->dma_tx->device->dev;
>> - else if (op->data.dir == SPI_MEM_DATA_IN && ctlr->dma_rx)
>> - dmadev = ctlr->dma_rx->device->dev;
>> - else
>> - dmadev = ctlr->dev.parent;
>> -
>> - spi_unmap_buf(ctlr, dmadev, sgt,
>> - op->data.dir == SPI_MEM_DATA_IN ?
>> - DMA_FROM_DEVICE : DMA_TO_DEVICE);
>> -}
>> -EXPORT_SYMBOL_GPL(spi_controller_dma_unmap_mem_op_data);
>> -#endif /* __UBOOT__ */
>>
>> static int spi_check_buswidth_req(struct spi_slave *slave, u8 buswidth, bool tx)
>> {
>> @@ -166,7 +65,6 @@ bool spi_mem_default_supports_op(struct spi_slave *slave,
>>
>> return true;
>> }
>> -EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);
>
> Nitpick: Since this patch is about removing things in #ifndef __UBOOT_,
> please don't remove things outside of it. Do them as a separate patch
> instead. Same for all other places below.
Ok, I can move the removal of these lines to its own patch.
>
> I am neither endorsing this patch nor opposing it, but please at least
> make sure it builds.
>
Thanks for the feedback.
--Sean
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-07 16:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-07 13:30 [PATCH 0/3] Misc fixes/cleanup Sean Anderson
2020-08-07 13:30 ` [PATCH 1/3] doc: Fix typo in FIT documentation Sean Anderson
2020-08-07 13:30 ` [PATCH 2/3] spi: Remove uses of #ifdef __U_BOOT__ from spi-mem.c Sean Anderson
2020-08-07 16:48 ` Pratyush Yadav
2020-08-07 16:55 ` Sean Anderson
2020-08-07 13:30 ` [PATCH 3/3] spi: Fix typo in header Sean Anderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.