[U-Boot] [PATCH 3/6] sf: sf_ops: use dma to copy data from mmap region if platform supports

[Simon Glass]

Hi Magunthan,

On 2 December 2015 at 02:45, Mugunthan V N <mugunthanvnm at ti.com> wrote:
> On Wednesday 02 December 2015 03:24 AM, Simon Glass wrote:
>> Hi,
>>
>> On 1 December 2015 at 04:13, Mugunthan V N <mugunthanvnm at ti.com> wrote:
>>> Add dma memcpy api to the default spi_flash_copy_mmap(), so that
>>> dma will be used to copy data when DM_DMA is defined for the
>>> platform.
>>>
>>> Signed-off-by: Mugunthan V N <mugunthanvnm at ti.com>
>>> ---
>>>  drivers/mtd/spi/sf_ops.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
>>> index 3a56d7f..3c83f8a 100644
>>> --- a/drivers/mtd/spi/sf_ops.c
>>> +++ b/drivers/mtd/spi/sf_ops.c
>>> @@ -16,6 +16,7 @@
>>>  #include <watchdog.h>
>>>  #include <linux/compiler.h>
>>>  #include <linux/log2.h>
>>> +#include <dma.h>
>>>
>>>  #include "sf_internal.h"
>>>
>>> @@ -389,6 +390,10 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
>>>
>>>  void __weak spi_flash_copy_mmap(void *data, void *offset, size_t len)
>>>  {
>>> +#ifdef CONFIG_DM_DMA
>>> +       if (!dma_memcpy(data, offset, len))
>>> +               return;
>>> +#endif
>>>         memcpy(data, offset, len);
>>>  }
>>>
>>> --
>>> 2.6.3.368.gf34be46
>>>
>>
>> This looks like one driver (SPI flash) calling a weak function in
>> another (SPI). Why isn't this done with the driver interface?
>>
>
> This is suppose to be with SPI flash driver, as the flash driver can
> only decide whether to use memory map transfers or spi transfers.
>
> It was kept as a weak function so that platforms with DMA support can
> have their own spi_flash_copy_mmap() to transfer data with DMA support.
> But for some reasons this implementations landed in spi driver files
> which is not the best place, it should have to be done in DMA drivers.
>
> With CONFIG_DMA, this should go away with the above #ifdef CONFIG_DMA
> and the weak function attribute can be removed later.

OK that sounds good, please add a TODO for this.

Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Simon