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

[Sean Anderson]

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

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