[U-Boot] [upstream-release] [PATCH v2 5/7] armv8: ls1046a: initial icid setup support
Laurentiu Tudor
laurentiu.tudor at nxp.com
Tue Jul 3 15:27:04 UTC 2018
Hi Bharat,
Thanks for the review! Comments inline.
On 03.07.2018 17:09, Bharat Bhushan wrote:
>
>
>> -----Original Message-----
>> From: upstream-release-bounces at linux.freescale.net [mailto:upstream-
>> release-bounces at linux.freescale.net] On Behalf Of Laurentiu Tudor
>> Sent: Tuesday, July 3, 2018 5:42 PM
>> To: York Sun <york.sun at nxp.com>; Prabhakar Kushwaha
>> <prabhakar.kushwaha at nxp.com>; u-boot at lists.denx.de
>> Cc: Laurentiu Tudor <laurentiu.tudor at nxp.com>
>> Subject: [upstream-release] [PATCH v2 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);
>
> Icid_tbl[] is currently defined for ls1046 but this code is generic and later can be used for ls1043 later.
> We can let caller provide table pointer and size.
Right, but "icid_tbl" is defined in the SoC specific ls1046_ids.c file.
I think that if we add the corresponding ls1043_ids.c this code can be
reused as is.
>> +}
>> +
>> +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");
>> + if (ret > 0) {
>> + printf("WARNING unable to set dma-coherent: %s\n",
>> + fdt_strerror(off));
>> + return off;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +int fdt_fixup_icid_tbl(void *blob, int smmu_ph,
>> + struct icid_id_table *tbl, int size)
>
> What we set in device tree is stream-id, while stream-id is = [PL: BMT : ICID];
Also TBU_ID. As far as i know, PL, BMT bits are not implemented on this
platform.
> I think we should use stream-id to the point when we are patching device-tree and value set in h/w.
Not sure i understand this statement? Are you commenting that i should
have used a different naming scheme, that is stream_id instead of icid?
> We can say PL = BMT = 0 for now and leave space for setting this in future for any specific device for any platform?
I think so. As i mentioned, PL, BMT bits are not implemented on these chips.
>> +{
>> + int i, err, off;
>> +
>> + for (i = 0; i < size; i++) {
>> + if (!tbl[i].compat)
>> + continue;
>> +
>> + off = fdt_node_offset_by_compat_reg(blob,
>> + tbl[i].compat,
>> + tbl[i].compat_addr);
>> + if (off > 0) {
>> + err = fdt_set_iommu_prop(blob, off, smmu_ph,
>> + &tbl[i].id, 1);
>> + if (err)
>> + return err;
>> + } else {
>> + printf("WARNING could not find node %s: %s.\n",
>> + tbl[i].compat, fdt_strerror(off));
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +int fdt_get_smmu_phandle(void *blob)
>> +{
>> + int noff, smmu_ph;
>> +
>> + noff = fdt_node_offset_by_compatible(blob, -1, "arm,mmu-500");
>> + if (noff < 0) {
>> + printf("WARNING failed to get smmu node: %s\n",
>> + fdt_strerror(noff));
>> + return noff;
>> + }
>> +
>> + smmu_ph = fdt_get_phandle(blob, noff);
>> + if (!smmu_ph) {
>> + smmu_ph = fdt_create_phandle(blob, noff);
>> + if (!smmu_ph) {
>> + printf("WARNING failed to get smmu phandle\n");
>> + return -1;
>> + }
>> + }
>> +
>> + return smmu_ph;
>> +}
>> +
>> +void fdt_fixup_icid(void *blob)
>> +{
>> + int smmu_ph;
>> +
>> + smmu_ph = fdt_get_smmu_phandle(blob);
>> + if (smmu_ph < 0)
>> + return;
>> +
>> + fdt_fixup_icid_tbl(blob, smmu_ph, icid_tbl, icid_tbl_sz);
>> +}
>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
>> b/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
>> new file mode 100644
>> index 0000000000..d9e03fcb89
>> --- /dev/null
>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
>> @@ -0,0 +1,29 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright 2018 NXP
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/arch-fsl-layerscape/immap_lsch2.h>
>> +#include <asm/arch-fsl-layerscape/fsl_icid.h>
>> +
>> +struct icid_id_table icid_tbl[] = {
>> +#ifdef CONFIG_SYS_DPAA_QBMAN
>> + SET_QMAN_ICID(FSL_DPAA1_STREAM_ID_START),
>> + SET_BMAN_ICID(FSL_DPAA1_STREAM_ID_START + 1),
>> +#endif
>> +
>> + SET_SDHC_ICID(FSL_SDHC_STREAM_ID),
>> +
>> + SET_USB_ICID(1, "snps,dwc3", FSL_USB1_STREAM_ID),
>> + SET_USB_ICID(2, "snps,dwc3", FSL_USB2_STREAM_ID),
>> + SET_USB_ICID(3, "snps,dwc3", FSL_USB3_STREAM_ID),
>
> Last parameter is STREAM_ID ?
>
>> +
>> + SET_SATA_ICID(FSL_SATA_STREAM_ID),
>> + SET_QDMA_ICID(FSL_QDMA_STREAM_ID),
>> + SET_EDMA_ICID(FSL_EDMA_STREAM_ID),
>> + SET_ETR_ICID(FSL_ETR_STREAM_ID),
>> + SET_DEBUG_ICID(FSL_DEBUG_STREAM_ID),
>> +};
>> +
>> +int icid_tbl_sz = ARRAY_SIZE(icid_tbl);
>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>> b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>> index bfd663942a..5c5df5b7ef 100644
>> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>> @@ -13,6 +13,7 @@
>> #include <asm/io.h>
>> #include <asm/global_data.h>
>> #include <asm/arch-fsl-layerscape/config.h>
>> +#include <asm/arch-fsl-layerscape/fsl_icid.h>
>> #ifdef CONFIG_LAYERSCAPE_NS_ACCESS
>> #include <fsl_csu.h>
>> #endif
>> @@ -674,6 +675,8 @@ void fsl_lsch2_early_init_f(void)
>> erratum_a009798();
>> erratum_a008997();
>> erratum_a009007();
>> +
>> + set_icids();
>> }
>> #endif
>>
>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
>> b/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
>> new file mode 100644
>> index 0000000000..cf0f3d1121
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
>> @@ -0,0 +1,80 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Copyright 2018 NXP
>> + */
>> +
>> +#ifndef _FSL_ICID_H_
>> +#define _FSL_ICID_H_
>> +
>> +#include <asm/types.h>
>> +#include <fsl_qbman.h>
>> +
>> +struct icid_id_table {
>> + const char *compat;
>> + u32 id;
>> + u32 reg;
>> + phys_addr_t compat_addr;
>> + phys_addr_t reg_addr;
>> +};
>> +
>> +u32 get_ppid_icid(int ppid_tbl_idx, int ppid);
>> +int fdt_get_smmu_phandle(void *blob);
>> +int fdt_set_iommu_prop(void *blob, int off, int smmu_ph, u32 *ids, int
>> num_ids);
>> +void set_icids(void);
>> +void fdt_fixup_icid(void *blob);
>> +
>> +#define SET_ICID_ENTRY(name, idA, regA, addr, compataddr) \
>> + { .compat = name, \
>> + .id = idA, \
>> + .reg = regA, \
>> + .compat_addr = compataddr, \
>> + .reg_addr = addr, \
>> + }
>> +
>> +#define SET_SCFG_ICID(compat, icid, name, compataddr) \
>> + SET_ICID_ENTRY(compat, icid, (((icid) << 24) | (1 << 23)), \
>> + offsetof(struct ccsr_scfg, name) +
>> CONFIG_SYS_FSL_SCFG_ADDR, \
>> + compataddr)
>> +
>> +#define SET_USB_ICID(usb_num, compat, icid) \
>
> And this and below say "icid" as last parameter.
Right. I went with the icid nomenclature as it was shorter.
The
>
>> + SET_SCFG_ICID(compat, icid, usb##usb_num##_icid,\
>> + CONFIG_SYS_XHCI_USB##usb_num##_ADDR)
>> +
>> +#define SET_SATA_ICID(icid) \
>> + SET_SCFG_ICID("fsl,ls1046a-ahci", icid, sata_icid,\
>> + AHCI_BASE_ADDR)
>> +
>> +#define SET_SDHC_ICID(icid) \
>> + SET_SCFG_ICID("fsl,esdhc", icid, sdhc_icid,\
>> + CONFIG_SYS_FSL_ESDHC_ADDR)
>> +
>> +#define SET_QDMA_ICID(icid) \
>> + SET_SCFG_ICID("fsl,ls1046a-qdma", icid, dma_icid,\
>
> This is generic code and we should be passing "fsl,ls1046a-qdma"
Very good point. I'll update here and other places.
IIRC, The *_STREAM_ID was already there in the headers so I just re-used it.
---
Best Regards, Laurentiu
More information about the U-Boot
mailing list