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 <sean...@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.brezil...@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