[U-Boot] [PATCH v3 5/7] armv8: ls1046a: initial icid setup support

[Laurentiu Tudor]

Hi Bharat,

On 10.07.2018 08:14, Bharat Bhushan wrote:
> 
> 
>> -----Original Message-----
>> From: Laurentiu Tudor
>> Sent: Monday, July 9, 2018 6:42 PM
>> To: Bharat Bhushan <bharat.bhushan at nxp.com>; York Sun
>> <york.sun at nxp.com>; Prabhakar Kushwaha
>> <prabhakar.kushwaha at nxp.com>; u-boot at lists.denx.de
>> Subject: Re: [PATCH v3 5/7] armv8: ls1046a: initial icid setup support
>>
>> Hi Bharat,
>>
>> On 09.07.2018 15:13, Bharat Bhushan wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Laurentiu Tudor [mailto:laurentiu.tudor at nxp.com]
>>>> Sent: Wednesday, July 4, 2018 7:44 PM
>>>> To: York Sun <york.sun at nxp.com>; Prabhakar Kushwaha
>>>> <prabhakar.kushwaha at nxp.com>; u-boot at lists.denx.de
>>>> Cc: Bharat Bhushan <bharat.bhushan at nxp.com>; Laurentiu Tudor
>>>> <laurentiu.tudor at nxp.com>
>>>> Subject: [PATCH v3 5/7] armv8: ls1046a: initial icid setup support
>>>>
>>>> Add infrastructure for ICID setup and device tree fixup on ARM platforms.
>>>> This include basic ICID setup for several devices.
>>>>
>>>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor at nxp.com>
>>>> ---
>>>>    arch/arm/cpu/armv8/fsl-layerscape/Makefile    |   1 +
>>>>    arch/arm/cpu/armv8/fsl-layerscape/icid.c      | 111
>> ++++++++++++++++++
>>>>    .../arm/cpu/armv8/fsl-layerscape/ls1046_ids.c |  29 +++++
>>>>    arch/arm/cpu/armv8/fsl-layerscape/soc.c       |   3 +
>>>>    .../asm/arch-fsl-layerscape/fsl_icid.h        |  80 +++++++++++++
>>>>    board/freescale/ls1046aqds/ls1046aqds.c       |   2 +
>>>>    board/freescale/ls1046ardb/ls1046ardb.c       |   3 +
>>>>    7 files changed, 229 insertions(+)
>>>>    create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/icid.c
>>>>    create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
>>>>    create mode 100644
>>>> arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
>>>>
>>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Makefile
>>>> b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
>>>> index 1e9e4680fe..5d6f68aad6 100644
>>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/Makefile
>>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
>>>> @@ -37,6 +37,7 @@ endif
>>>>
>>>>    ifneq ($(CONFIG_ARCH_LS1046A),)
>>>>    obj-$(CONFIG_SYS_HAS_SERDES) += ls1046a_serdes.o
>>>> +obj-y += icid.o ls1046_ids.o
>>>>    endif
>>>>
>>>>    ifneq ($(CONFIG_ARCH_LS1088A),)
>>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/icid.c
>>>> b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
>>>> new file mode 100644
>>>> index 0000000000..8694bd6fa1
>>>> --- /dev/null
>>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
>>>> @@ -0,0 +1,111 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +/*
>>>> + * Copyright 2018 NXP
>>>> + */
>>>> +
>>>> +#include <common.h>
>>>> +#include <linux/libfdt.h>
>>>> +#include <fdt_support.h>
>>>> +
>>>> +#include <asm/io.h>
>>>> +#include <asm/processor.h>
>>>> +#include <asm/arch-fsl-layerscape/fsl_icid.h>
>>>> +
>>>> +static void set_icid(struct icid_id_table *tbl, int size) {
>>>> +	int i;
>>>> +
>>>> +	for (i = 0; i < size; i++)
>>>> +		out_be32((u32 *)(tbl[i].reg_addr), tbl[i].reg); }
>>>> +
>>>> +void set_icids(void)
>>>> +{
>>>> +	/* setup general icid offsets */
>>>> +	set_icid(icid_tbl, icid_tbl_sz);
>>>> +}
>>>> +
>>>> +int fdt_set_iommu_prop(void *blob, int off, int smmu_ph, u32 *ids,
>>>> +int
>>>> +num_ids) {
>>>> +	int i, ret;
>>>> +	u32 prop[8];
>>>> +
>>>> +	for (i = 0; i < num_ids; i++) {
>>>> +		prop[i * 2] = cpu_to_fdt32(smmu_ph);
>>>> +		prop[i * 2 + 1] = cpu_to_fdt32(ids[i]);
>>>> +	}
>>>> +	ret = fdt_setprop(blob, off, "iommus",
>>>> +			  prop, sizeof(u32) * num_ids * 2);
>>>> +	if (ret > 0) {
>>>> +		printf("WARNING unable to set iommus: %s\n",
>>>> fdt_strerror(off));
>>>> +		return off;
>>>> +	}
>>>> +	ret = fdt_setprop_empty(blob, off, "dma-coherent");
>>>
>>> This forces "dma-coherent" for all SOCs, although there is no current
>> known SOCs which does not support non-coherent but maybe safe not to
>> set from common code.
>>
>> I see your point and will drop this.
>> Now, regarding a replacement solution, I don't have any other idea than
>> updating the device trees directly. Thoughts?
> 
> dma-coherent will be default in device tree and u-boot code should check if dma-coherent already set.

You mean add a check that dma-coherent isn't already there?

> Have another function exposed from generic code which set "dma-coherent" and platforms code will call that function if it is coherent.

Who would call such a function in the current design? Not sure I 
understand where this would fit in the current implementation.

On the other hand, I think that the dma coherency of a device is a hw 
property and it really makes sense to sit in the device tree.

---
Best Regards, Laurentiu