[U-Boot] [PATCH v5 02/15] dma: add channels support

Álvaro Fernández Rojas
2018-03-09T18:42:32Z

Hi Grygorii,

El 09/03/2018 a las 0:07, Grygorii Strashko escribió:
> Hi Álvaro,
>
> On 03/05/2018 02:05 PM, Álvaro Fernández Rojas wrote:
>> This adds channels support for dma controllers that have multiple channels
>> which can transfer data to/from different devices (enet, usb...).
>>
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> ---
>> v5: remove unneeded dma.h include
>> v4: no changes
>> v3: Introduce changes reported by Simon Glass:
>> - Improve dma-uclass.h documentation.
>> - Switch to live tree API.
>>
>> drivers/dma/Kconfig | 7 ++
>> drivers/dma/dma-uclass.c | 188 +++++++++++++++++++++++++++++++++++++++++++++--
>> include/dma-uclass.h | 78 ++++++++++++++++++++
>> include/dma.h | 174 ++++++++++++++++++++++++++++++++++++++++++-
>> 4 files changed, 439 insertions(+), 8 deletions(-)
> Small note first. I don't know if this is common practice for u-boot -
> but Isn't it preferable to send new version of the series *not as reply* to the old one?
I was sending it as a reply to the first version, not the last one, but
I will do that from now on.
>
> I've tried this and, in general, it works for me (unfortunately I can't post code yet).
> Thanks a lot for you work.
Thanks to you for testing :).
>
> But... (it's always but ;)
¯\_(ツ)_/¯
> [...]
>> + /**
>> + * receive() - Receive a DMA transfer.
>> + *
>> + * @dma: The DMA Channel to manipulate.
>> + * @dst: The destination pointer.
>> + * @return zero on success, or -ve error code.
>> + */
>> + int (*receive)(struct dma *dma, void **dst);
>> + /**
>> + * send() - Send a DMA transfer.
>> + *
>> + * @dma: The DMA Channel to manipulate.
>> + * @src: The source pointer.
>> + * @len: Length of the data to be sent (number of bytes).
>> + * @return zero on success, or -ve error code.
>> + */
>> + int (*send)(struct dma *dma, void *src, size_t len);
> Can we have additional *optional* parameter in above two callbacks and in dma_receive/dma_send() API?
> Like:
> int (*send)(struct dma *dma, void *src, size_t len, void *metadata)
> int (*receive)(struct dma *dma, void **dst, void **metadata);
>
> Reason:
> It's not a common practice to implement Networking DMA using generic DMA frameworks, simply
> because Networking DMA HW is terribly different between different SoCs :(, so it's mostly impossible
> to fit all of them in any generic DMA framework (at least this has never ever worked for Linux kernel):
> - totally different HW rings/queues impl, multi-queues & multi DMA channels
> - special requirements for IRQ handling
> - necessity to pass additional information within each Net DMA descriptor
> - availability and support of different Net traffic HW acceleration features
>
> But in case of u-boot, it, theoretically, might work because most of Net
> drivers have much more simplified implementation comparing to Linux kernel
> - UP, polling, one RX/TX channel and disabled Net traffic HW acceleration features.
>
> As result, Proposed here interface can be used with much more different HW and
> drivers (and especially Net drivers), but only if it will be possible to pass
> additional DMA driver's specific information from DMA user to DMA driver
> per each send/rsv request.
> For example, two TI driver cpsw.c and keystone_net.c required to pass
> minimum one additional parameter with each sending packet - Destination Port number,
> which must be passed to DMA in DMA descriptor. And Source Port number
> has to be passed back with each received packet.
>
> ! I'm not insisting here ! and be happy to hear third opinion.
>
> (if proposition will not be accepted - .. :( I'll just need to do the same later,
> but there might be hight number of active users of this interface in u-boot already)
If everyone else agrees I can add those two parameters in the next
version even though my implementation doesn't need them.
>
>> +#endif /* CONFIG_DMA_CHANNELS */
>> /**
>> * transfer() - Issue a DMA transfer. The implementation must
>> * wait until the transfer is done.
>> diff --git a/include/dma.h b/include/dma.h
>> index 89320f10d9..bf8123fa9e 100644
>> --- a/include/dma.h
>> +++ b/include/dma.h
>> @@ -1,6 +1,7 @@
>> /*
>> - * (C) Copyright 2015
>> - * Texas Instruments Incorporated, <www.ti.com>
>> + * Copyright (C) 2018 Álvaro Fernández Rojas <noltari@gmail.com>
>> + * Copyright (C) 2015 Texas Instruments Incorporated <www.ti.com>
>> + * Written by Mugunthan V N <mugunthanvnm@ti.com>
>> *
>> * SPDX-License-Identifier: GPL-2.0+
>> */
>> @@ -8,6 +9,9 @@
>> #ifndef _DMA_H_
>> #define _DMA_H_
>>
>> +#include <linux/errno.h>
>> +#include <linux/types.h>
>> +
>> /*
>> * enum dma_direction - dma transfer direction indicator
>> * @DMA_MEM_TO_MEM: Memcpy mode
>> @@ -37,6 +41,172 @@ struct dma_dev_priv {
>> u32 supported;
>> };
>>
>> +#ifdef CONFIG_DMA_CHANNELS
>> +/**
>> + * A DMA is a feature of computer systems that allows certain hardware
>> + * subsystems to access main system memory, independent of the CPU.
>> + * DMA channels are typically generated externally to the HW module
>> + * consuming them, by an entity this API calls a DMA provider. This API
>> + * provides a standard means for drivers to enable and disable DMAs, and to
>> + * copy, send and receive data using DMA.
>> + *
>> + * A driver that implements UCLASS_DMA is a DMA provider. A provider will
>> + * often implement multiple separate DMAs, since the hardware it manages
>> + * often has this capability. dma_uclass.h describes the interface which
>> + * DMA providers must implement.
>> + *
>> + * DMA consumers/clients are the HW modules driven by the DMA channels. This
>> + * header file describes the API used by drivers for those HW modules.
>> + */
>> +
>> +struct udevice;
>> +
>> +/**
>> + * struct dma - A handle to (allowing control of) a single DMA.
>> + *
>> + * Clients provide storage for DMA handles. The content of the structure is
>> + * managed solely by the DMA API and DMA drivers. A DMA struct is
>> + * initialized by "get"ing the DMA struct. The DMA struct is passed to all
>> + * other DMA APIs to identify which DMA channel to operate upon.
>> + *
>> + * @dev: The device which implements the DMA channel.
>> + * @id: The DMA channel ID within the provider.
>> + *
>> + * Currently, the DMA API assumes that a single integer ID is enough to
>> + * identify and configure any DMA channel for any DMA provider. If this
>> + * assumption becomes invalid in the future, the struct could be expanded to
>> + * either (a) add more fields to allow DMA providers to store additional
>> + * information, or (b) replace the id field with an opaque pointer, which the
>> + * provider would dynamically allocated during its .of_xlate op, and process
>> + * during is .request op. This may require the addition of an extra op to clean
>> + * up the allocation.
>> + */
>> +struct dma {
>> + struct udevice *dev;
>> + /*
>> + * Written by of_xlate. We assume a single id is enough for now. In the
>> + * future, we might add more fields here.
>> + */
>> + unsigned long id;
>> +};
>> +
>> +# if CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(DMA)
>> +struct phandle_1_arg;
>> +int dma_get_by_index_platdata(struct udevice *dev, int index,
>> + struct phandle_1_arg *cells, struct dma *dma);
>> +
>> +/**
>> + * dma_get_by_index - Get/request a DMA by integer index.
>> + *
>> + * This looks up and requests a DMA. The index is relative to the client
>> + * device; each device is assumed to have n DMAs associated with it somehow,
>> + * and this function finds and requests one of them. The mapping of client
>> + * device DMA indices to provider DMAs may be via device-tree properties,
>> + * board-provided mapping tables, or some other mechanism.
>> + *
>> + * @dev: The client device.
>> + * @index: The index of the DMA to request, within the client's list of
>> + * DMA channels.
>> + * @dma: A pointer to a DMA struct to initialize.
>> + * @return 0 if OK, or a negative error code.
>> + */
>> +int dma_get_by_index(struct udevice *dev, int index, struct dma *dma);
>> +
>> +/**
>> + * dma_get_by_name - Get/request a DMA by name.
>> + *
>> + * This looks up and requests a DMA. The name is relative to the client
>> + * device; each device is assumed to have n DMAs associated with it somehow,
>> + * and this function finds and requests one of them. The mapping of client
>> + * device DMA names to provider DMAs may be via device-tree properties,
>> + * board-provided mapping tables, or some other mechanism.
>> + *
>> + * @dev: The client device.
>> + * @name: The name of the DMA to request, within the client's list of
>> + * DMA channels.
>> + * @dma: A pointer to a DMA struct to initialize.
>> + * @return 0 if OK, or a negative error code.
>> + */
>> +int dma_get_by_name(struct udevice *dev, const char *name, struct dma *dma);
>> +# else
>> +static inline int dma_get_by_index(struct udevice *dev, int index,
>> + struct dma *dma)
>> +{
>> + return -ENOSYS;
>> +}
>> +
>> +static inline int dma_get_by_name(struct udevice *dev, const char *name,
>> + struct dma *dma)
>> +{
>> + return -ENOSYS;
>> +}
>> +# endif
>> +
>> +/**
>> + * dma_request - Request a DMA by provider-specific ID.
>> + *
>> + * This requests a DMA using a provider-specific ID. Generally, this function
>> + * should not be used, since dma_get_by_index/name() provide an interface that
>> + * better separates clients from intimate knowledge of DMA providers.
>> + * However, this function may be useful in core SoC-specific code.
>> + *
>> + * @dev: The DMA provider device.
>> + * @dma: A pointer to a DMA struct to initialize. The caller must
>> + * have already initialized any field in this struct which the
>> + * DMA provider uses to identify the DMA channel.
>> + * @return 0 if OK, or a negative error code.
>> + */
>> +int dma_request(struct udevice *dev, struct dma *dma);
>> +
>> +/**
>> + * dma_free - Free a previously requested DMA.
>> + *
>> + * @dma: A DMA struct that was previously successfully requested by
>> + * dma_request/get_by_*().
>> + * @return 0 if OK, or a negative error code.
>> + */
>> +int dma_free(struct dma *dma);
>> +
>> +/**
>> + * dma_enable() - Enable (turn on) a DMA channel.
>> + *
>> + * @dma: A DMA struct that was previously successfully requested by
>> + * dma_request/get_by_*().
>> + * @return zero on success, or -ve error code.
>> + */
>> +int dma_enable(struct dma *dma);
>> +
>> +/**
>> + * dma_disable() - Disable (turn off) a DMA channel.
>> + *
>> + * @dma: A DMA struct that was previously successfully requested by
>> + * dma_request/get_by_*().
>> + * @return zero on success, or -ve error code.
>> + */
>> +int dma_disable(struct dma *dma);
>> +
>> +/**
>> + * dma_receive() - Receive a DMA transfer.
>> + *
>> + * @dma: A DMA struct that was previously successfully requested by
>> + * dma_request/get_by_*().
>> + * @dst: The destination pointer.
>> + * @return zero on success, or -ve error code.
>> + */
>> +int dma_receive(struct dma *dma, void **dst);
>> +
>> +/**
>> + * dma_send() - Send a DMA transfer.
>> + *
>> + * @dma: A DMA struct that was previously successfully requested by
>> + * dma_request/get_by_*().
>> + * @src: The source pointer.
>> + * @len: Length of the data to be sent (number of bytes).
>> + * @return zero on success, or -ve error code.
>> + */
>> +int dma_send(struct dma *dma, void *src, size_t len);
>> +#endif /* CONFIG_DMA_CHANNELS */
>> +
>> /*
>> * dma_get_device - get a DMA device which supports transfer
>> * type of transfer_type
>>
~Álvaro.