[PATCH v3 06/12] dm: Introduce DMA constraints into the core device model

[Simon Glass]

Hi Nicolas,

On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
<nsaenzjulienne at suse.de> wrote:
>
> Calculating the DMA offset between a bus address space and CPU's every
> time we call phys_to_bus() and bus_to_phys() isn't ideal performance
> wise, as it implies traversing the device tree from the device's node up
> to the root. Since this information is static and available before the
> device's initialization, parse it before the probe call an provide the
> DMA offset in 'struct udevice' for the address translation code to use
> it.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne at suse.de>
>
> ---
> Changes since v2:
>  - Return/Fail on error
>  - Add config option
>  - use ulong instead for u64 for dev->dma_offset
>
> Changes since v1:
>  - Update commit message so as to explain better the reasoning behind
>    this
>
>  drivers/core/Kconfig  | 10 ++++++++++
>  drivers/core/device.c | 35 +++++++++++++++++++++++++++++++++++
>  include/dm/device.h   |  3 +++
>  3 files changed, 48 insertions(+)

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

nits below

>
> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> index ffae6f9795..153be25351 100644
> --- a/drivers/core/Kconfig
> +++ b/drivers/core/Kconfig
> @@ -113,6 +113,16 @@ config SPL_DM_SEQ_ALIAS
>           numbered devices (e.g. serial0 = &serial0). This feature can be
>           disabled if it is not required, to save code space in SPL.
>
> +config DM_DMA
> +       bool "Support per device DMA constraints"

per-device

> +       depends on DM
> +       default n
> +       help
> +         Enable this to extract per-device DMA constraints, only supported on
> +         device-tree systems for now. This is needed in order translate
> +         addresses on systems where different buses have different views of
> +         the physical address space.
> +
>  config REGMAP
>         bool "Support register maps"
>         depends on DM
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 4b3dcb3b37..21285432d8 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -421,6 +421,37 @@ fail:
>         return ret;
>  }
>
> +#if CONFIG_IS_ENABLED(DM_DMA)

I feel you could use

if (CONFIG_IS_ENABLED)

in the function below (returning at the start) and avoid an #ifdef?

To do that you'll need access macros - you can see things like that in
device.h and asm-generic/global_data.h

> +static int device_get_dma_constraints(struct udevice *dev)

Please add a comment to describe this function.

> +{
> +       struct udevice *parent = dev->parent;
> +       phys_addr_t cpu = 0;
> +       dma_addr_t bus = 0;
> +       u64 size = 0;
> +       int ret;
> +
> +       if (!parent || !dev_of_valid(parent))
> +               return 0;
> +
> +       /*
> +        * We start parsing for dma-ranges from the device's bus node. This is
> +        * specially important on nested buses.
> +        */
> +       ret = dev_get_dma_range(parent, &cpu, &bus, &size);
> +       /* Don't return an error if no 'dma-ranges' were found */
> +       if (ret && ret != -ENOENT) {
> +               dm_warn("%s: failed to get DMA range, %d\n", dev->name, ret);
> +               return ret;
> +       }
> +
> +       dev->dma_offset = cpu - bus;
> +
> +       return 0;
> +}
> +#else
> +static int device_get_dma_constraints(struct udevice *dev) { return 0; }
> +#endif
> +
>  int device_probe(struct udevice *dev)
>  {
>         const struct driver *drv;
> @@ -482,6 +513,10 @@ int device_probe(struct udevice *dev)
>                         goto fail;
>         }
>
> +       ret = device_get_dma_constraints(dev);
> +       if (ret)
> +               goto fail;
> +
>         ret = uclass_pre_probe_device(dev);
>         if (ret)
>                 goto fail;
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 5bef484247..9126dc00fe 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -161,6 +161,9 @@ struct udevice {
>  #ifdef CONFIG_DEVRES
>         struct list_head devres_head;
>  #endif
> +#ifdef CONFIG_DM_DMA
> +       ulong dma_offset;

Needs a comment

> +#endif
>  };
>
>  /* Maximum sequence number supported */
> --
> 2.29.2
>

Regards,
Simon