Hi Nicolas,
On Thu, 10 Dec 2020 at 04:43, Nicolas Saenz Julienne
<nsaenzjulie...@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 <nsaenzjulie...@suse.de>
>
> ---
> Changes since v1:
> - Update commit message so as to explain better the reasoning behind
> this
>
> drivers/core/device.c | 24 ++++++++++++++++++++++++
> include/dm/device.h | 1 +
> 2 files changed, 25 insertions(+)
>
Firstly, please add a sandbox test for this. All driver model code is
tested this way.
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 4b3dcb3b37..4255bea24d 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -421,6 +421,28 @@ fail:
> return ret;
> }
>
> +void device_get_dma_constraints(struct udevice *dev)
> +{
> + phys_addr_t cpu;
> + dma_addr_t bus;
> + u64 size;
> + int ret;
> +
> + if (!dev_of_valid(dev))
> + return;
> +
> + ret = dev_get_dma_range(dev, &cpu, &bus, &size);
> + if (ret) {
> + /* Don't complain if no 'dma-ranges' were found */
> + if (ret != -ENODEV)
Don't use -ENODEV, that is very confusing. Maybe -ENOENT
> + dm_warn("%s: failed to get DMA range, %d\n",
> + dev->name, ret);
Needs to return an error
> + return;
> + }
> +
> + dev->dma_offset = cpu - bus;
> +}
> +
> int device_probe(struct udevice *dev)
> {
> const struct driver *drv;
> @@ -482,6 +504,8 @@ int device_probe(struct udevice *dev)
> goto fail;
> }
>
> + device_get_dma_constraints(dev);
Check error and fail
Also add a new CONFIG and use CONFIG_IS_ENABLED() to check whether to
use this feature. Some boards won't need it.
> +
> ret = uclass_pre_probe_device(dev);
> if (ret)
> goto fail;
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 5bef484247..59f711e3dd 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -161,6 +161,7 @@ struct udevice {
> #ifdef CONFIG_DEVRES
> struct list_head devres_head;
> #endif
> + u64 dma_offset;
ulong? Also needs a comment and needs to be conditional on CONFIG
> };
>
> /* Maximum sequence number supported */
> --
> 2.29.2
>
Regards,
Simon
