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

["Álvaro Fernández Rojas" <noltari@gmail.com>]

Hi Joe,

El 07/03/2018 a las 22:27, Joe Hershberger escribió:
> On Mon, Mar 5, 2018 at 2:05 PM, Álvaro Fernández Rojas
> <noltari@gmail.com> 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(-)
>>
>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>> index 1b92c7789d..21b2c0dcaa 100644
>> --- a/drivers/dma/Kconfig
>> +++ b/drivers/dma/Kconfig
>> @@ -12,6 +12,13 @@ config DMA
>>            buses that is used to transfer data to and from memory.
>>            The uclass interface is defined in include/dma.h.
>>
>> +config DMA_CHANNELS
>> +       bool "Enable DMA channels support"
>> +       depends on DMA
>> +       help
>> +         Enable channels support for DMA. Some DMA controllers have multiple
>> +         channels which can either transfer data to/from different devices.
>> +
>>   config TI_EDMA3
>>          bool "TI EDMA3 driver"
>>          help
>> diff --git a/drivers/dma/dma-uclass.c b/drivers/dma/dma-uclass.c
>> index faa27a3a56..b5109aafc9 100644
>> --- a/drivers/dma/dma-uclass.c
>> +++ b/drivers/dma/dma-uclass.c
>> @@ -1,23 +1,199 @@
>>   /*
>>    * Direct Memory Access U-Class driver
>>    *
>> - * (C) Copyright 2015
>> - *     Texas Instruments Incorporated, <www.ti.com>
>> - *
>> - * Author: Mugunthan V N <mugunthanvnm@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+
>>    */
>>
>>   #include <common.h>
>>   #include <dm.h>
>> -#include <dm/uclass-internal.h>
>> -#include <dm/device-internal.h>
>> +#include <dm/read.h>
>>   #include <dma-uclass.h>
>> +#include <dt-structs.h>
>>   #include <errno.h>
>>
>>   DECLARE_GLOBAL_DATA_PTR;
>>
>> +#ifdef CONFIG_DMA_CHANNELS
>> +static inline struct dma_ops *dma_dev_ops(struct udevice *dev)
>> +{
>> +       return (struct dma_ops *)dev->driver->ops;
>> +}
>> +
>> +# if CONFIG_IS_ENABLED(OF_CONTROL)
>> +#  if CONFIG_IS_ENABLED(OF_PLATDATA)
>> +int dma_get_by_index_platdata(struct udevice *dev, int index,
>> +                             struct phandle_1_arg *cells, struct dma *dma)
>> +{
>> +       int ret;
>> +
>> +       if (index != 0)
>> +               return -ENOSYS;
>> +       ret = uclass_get_device(UCLASS_DMA, 0, &dma->dev);
>> +       if (ret)
>> +               return ret;
>> +       dma->id = cells[0].id;
>> +
>> +       return 0;
>> +}
>> +#  else
>> +static int dma_of_xlate_default(struct dma *dma,
>> +                               struct ofnode_phandle_args *args)
>> +{
>> +       debug("%s(dma=%p)\n", __func__, dma);
>> +
>> +       if (args->args_count > 1) {
>> +               pr_err("Invaild args_count: %d\n", args->args_count);
>> +               return -EINVAL;
>> +       }
>> +
>> +       if (args->args_count)
>> +               dma->id = args->args[0];
>> +       else
>> +               dma->id = 0;
>> +
>> +       return 0;
>> +}
>> +
>> +int dma_get_by_index(struct udevice *dev, int index, struct dma *dma)
>> +{
>> +       int ret;
>> +       struct ofnode_phandle_args args;
>> +       struct udevice *dev_dma;
>> +       const struct dma_ops *ops;
>> +
>> +       debug("%s(dev=%p, index=%d, dma=%p)\n", __func__, dev, index, dma);
>> +
>> +       assert(dma);
>> +       dma->dev = NULL;
>> +
>> +       ret = dev_read_phandle_with_args(dev, "dmas", "#dma-cells", 0, index,
>> +                                        &args);
>> +       if (ret) {
>> +               pr_err("%s: dev_read_phandle_with_args failed: err=%d\n",
>> +                      __func__, ret);
>> +               return ret;
>> +       }
>> +
>> +       ret = uclass_get_device_by_ofnode(UCLASS_DMA, args.node, &dev_dma);
>> +       if (ret) {
>> +               pr_err("%s: uclass_get_device_by_ofnode failed: err=%d\n",
>> +                      __func__, ret);
>> +               return ret;
>> +       }
>> +
>> +       dma->dev = dev_dma;
>> +
>> +       ops = dma_dev_ops(dev_dma);
>> +
>> +       if (ops->of_xlate)
>> +               ret = ops->of_xlate(dma, &args);
>> +       else
>> +               ret = dma_of_xlate_default(dma, &args);
>> +       if (ret) {
>> +               pr_err("of_xlate() failed: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       return dma_request(dev_dma, dma);
>> +}
>> +#  endif /* OF_PLATDATA */
>> +
>> +int dma_get_by_name(struct udevice *dev, const char *name, struct dma *dma)
>> +{
>> +       int index;
>> +
>> +       debug("%s(dev=%p, name=%s, dma=%p)\n", __func__, dev, name, dma);
>> +       dma->dev = NULL;
>> +
>> +       index = dev_read_stringlist_search(dev, "dma-names", name);
>> +       if (index < 0) {
>> +               pr_err("dev_read_stringlist_search() failed: %d\n", index);
>> +               return index;
>> +       }
>> +
>> +       return dma_get_by_index(dev, index, dma);
>> +}
>> +# endif /* OF_CONTROL */
>> +
>> +int dma_request(struct udevice *dev, struct dma *dma)
>> +{
>> +       struct dma_ops *ops = dma_dev_ops(dev);
>> +
>> +       debug("%s(dev=%p, dma=%p)\n", __func__, dev, dma);
>> +
>> +       dma->dev = dev;
>> +
>> +       if (!ops->request)
>> +               return 0;
>> +
>> +       return ops->request(dma);
>> +}
>> +
>> +int dma_free(struct dma *dma)
>> +{
>> +       struct dma_ops *ops = dma_dev_ops(dma->dev);
>> +
>> +       debug("%s(dma=%p)\n", __func__, dma);
>> +
>> +       if (!ops->free)
>> +               return 0;
>> +
>> +       return ops->free(dma);
>> +}
>> +
>> +int dma_enable(struct dma *dma)
>> +{
>> +       struct dma_ops *ops = dma_dev_ops(dma->dev);
>> +
>> +       debug("%s(dma=%p)\n", __func__, dma);
>> +
>> +       if (!ops->enable)
>> +               return -ENOSYS;
>> +
>> +       return ops->enable(dma);
>> +}
>> +
>> +int dma_disable(struct dma *dma)
>> +{
>> +       struct dma_ops *ops = dma_dev_ops(dma->dev);
>> +
>> +       debug("%s(dma=%p)\n", __func__, dma);
>> +
>> +       if (!ops->disable)
>> +               return -ENOSYS;
>> +
>> +       return ops->disable(dma);
>> +}
>> +
>> +int dma_receive(struct dma *dma, void **dst)
>> +{
>> +       struct dma_ops *ops = dma_dev_ops(dma->dev);
>> +
>> +       debug("%s(dma=%p)\n", __func__, dma);
>> +
>> +       if (!ops->receive)
>> +               return -1;
> Isn't there a better error code you could return? Such as -ENOSYS like above?
Ok, I will fix that in the next version.
>
>> +
>> +       return ops->receive(dma, dst);
>> +}
>> +
>> +int dma_send(struct dma *dma, void *src, size_t len)
>> +{
>> +       struct dma_ops *ops = dma_dev_ops(dma->dev);
>> +
>> +       debug("%s(dma=%p)\n", __func__, dma);
>> +
>> +       if (!ops->send)
>> +               return -1;
> Same here.
Ok, I will fix that in the next version.
>
>> +
>> +       return ops->send(dma, src, len);
>> +}
>> +#endif /* CONFIG_DMA_CHANNELS */
>> +
>>   int dma_get_device(u32 transfer_type, struct udevice **devp)
>>   {
>>          struct udevice *dev;
>> diff --git a/include/dma-uclass.h b/include/dma-uclass.h
>> index 3429f65ec4..b334adb68b 100644
>> --- a/include/dma-uclass.h
>> +++ b/include/dma-uclass.h
>> @@ -13,6 +13,8 @@
>>
>>   #include <dma.h>
>>
>> +struct ofnode_phandle_args;
>> +
>>   /*
>>    * struct dma_ops - Driver model DMA operations
>>    *
>> @@ -20,6 +22,82 @@
>>    * driver model.
>>    */
>>   struct dma_ops {
>> +#ifdef CONFIG_DMA_CHANNELS
>> +       /**
>> +        * of_xlate - Translate a client's device-tree (OF) DMA specifier.
>> +        *
>> +        * The DMA core calls this function as the first step in implementing
>> +        * a client's dma_get_by_*() call.
>> +        *
>> +        * If this function pointer is set to NULL, the DMA core will use a
>> +        * default implementation, which assumes #dma-cells = <1>, and that
>> +        * the DT cell contains a simple integer DMA Channel.
>> +        *
>> +        * At present, the DMA API solely supports device-tree. If this
>> +        * changes, other xxx_xlate() functions may be added to support those
>> +        * other mechanisms.
>> +        *
>> +        * @dma: The dma struct to hold the translation result.
>> +        * @args:       The dma specifier values from device tree.
>> +        * @return 0 if OK, or a negative error code.
>> +        */
>> +       int (*of_xlate)(struct dma *dma,
>> +                       struct ofnode_phandle_args *args);
>> +       /**
>> +        * request - Request a translated DMA.
>> +        *
>> +        * The DMA core calls this function as the second step in
>> +        * implementing a client's dma_get_by_*() call, following a successful
>> +        * xxx_xlate() call, or as the only step in implementing a client's
>> +        * dma_request() call.
>> +        *
>> +        * @dma: The DMA struct to request; this has been filled in by
>> +        *   a previoux xxx_xlate() function call, or by the caller of
>> +        *   dma_request().
>> +        * @return 0 if OK, or a negative error code.
>> +        */
>> +       int (*request)(struct dma *dma);
>> +       /**
>> +        * free - Free a previously requested dma.
>> +        *
>> +        * This is the implementation of the client dma_free() API.
>> +        *
>> +        * @dma: The DMA to free.
>> +        * @return 0 if OK, or a negative error code.
>> +        */
>> +       int (*free)(struct dma *dma);
>> +       /**
>> +        * enable() - Enable a DMA Channel.
>> +        *
>> +        * @dma: The DMA Channel to manipulate.
>> +        * @return zero on success, or -ve error code.
>> +        */
>> +       int (*enable)(struct dma *dma);
>> +       /**
>> +        * disable() - Disable a DMA Channel.
>> +        *
>> +        * @dma: The DMA Channel to manipulate.
>> +        * @return zero on success, or -ve error code.
>> +        */
>> +       int (*disable)(struct dma *dma);
>> +       /**
>> +        * receive() - Receive a DMA transfer.
>> +        *
>> +        * @dma: The DMA Channel to manipulate.
>> +        * @dst: The destination pointer.
>> +        * @return zero on success, or -ve error code.
>> +        */
> I haven't seen any implementation of this. Will it return -EAGAIN in
> the case where the packet isn't available?
First of all, this documentation is wrong, because it doesn't return 
zero on success, but the received packet length, and it returns -EAGAIN 
when packet is not available in order to facilitate enet 
implementations, but it can return something different and handle it 
from enet drivers.
I will fix that in the next version.
>
>> +       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);
>> +#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
>> --
>> 2.11.0
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot