[PATCH 2/3] spi: Remove uses of #ifdef __U_BOOT__ from spi-mem.c

Pratyush Yadav p.yadav at ti.com
Fri Aug 7 18:48:27 CEST 2020


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 at 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 at 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


More information about the U-Boot mailing list