From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurentiu Tudor Date: Thu, 21 Mar 2019 12:47:32 +0000 Subject: [U-Boot] [PATCH 2/2] armv8: ls1088a: add icid setup for platform devices In-Reply-To: References: <20190320143130.32726-1-laurentiu.tudor@nxp.com> <20190320143130.32726-2-laurentiu.tudor@nxp.com> Message-ID: <40b8cd74-08d3-dccb-1814-e73daa418a41@nxp.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Horia, On 21.03.2019 12:36, Horia Geanta wrote: > On 3/20/2019 4:31 PM, laurentiu.tudor at nxp.com wrote: >> +struct icid_id_table icid_tbl[] = { >> + SET_SDHC_ICID(FSL_SDMMC_STREAM_ID), >> + SET_USB_ICID(1, "snps,dwc3", FSL_USB1_STREAM_ID), >> + SET_USB_ICID(2, "snps,dwc3", FSL_USB2_STREAM_ID), >> + SET_SATA_ICID(1, "fsl,ls1088a-ahci", FSL_SATA1_STREAM_ID), >> + SET_SEC_JR_ICID_ENTRY(0, FSL_SEC_STREAM_ID), >> + SET_SEC_JR_ICID_ENTRY(1, FSL_SEC_STREAM_ID), >> + SET_SEC_JR_ICID_ENTRY(2, FSL_SEC_STREAM_ID), >> + SET_SEC_JR_ICID_ENTRY(3, FSL_SEC_STREAM_ID), >> + SET_SEC_RTIC_ICID_ENTRY(0, FSL_SEC_STREAM_ID), >> + SET_SEC_RTIC_ICID_ENTRY(1, FSL_SEC_STREAM_ID), >> + SET_SEC_RTIC_ICID_ENTRY(2, FSL_SEC_STREAM_ID), >> + SET_SEC_RTIC_ICID_ENTRY(3, FSL_SEC_STREAM_ID), >> + SET_SEC_DECO_ICID_ENTRY(0, FSL_SEC_STREAM_ID), >> + SET_SEC_DECO_ICID_ENTRY(1, FSL_SEC_STREAM_ID), >> + SET_SEC_DECO_ICID_ENTRY(2, FSL_SEC_STREAM_ID), >> + SET_SEC_DECO_ICID_ENTRY(3, FSL_SEC_STREAM_ID), >> +}; > A single ICID is allocated to all SEC sub-blocks able to initiate transactions. > I think at least the job rings should have different ICIDs, while the rest could > share another ICID. I agree here, problem is that on these chips ICIDs are scarce resource because DPAA2 gets a big chunk of them. I'd suggest to leave them like that and, if a user needs distinct, per JR ICIDs (s)he can easily tune them. >> #define SET_SEC_QI_ICID(streamid) \ >> - SET_ICID_ENTRY("fsl,sec-v4.0", streamid, \ >> + SET_ICID_ENTRY("fsl,sec-v4.0", SEC_ICID_REG_VAL(streamid), \ > Is this a fix for LS104x? Then it should be a separate patch. Not really. I added an intermediate macro, SEC_ICID_REG_VAL(streamid) that forms the correct register value starting from ICID, depending on the chip version (the register layouts are different between the two). >> +#else /* CONFIG_FSL_LSCH2 */ > [...] >> +#define SEC_ICID_REG_VAL(streamid) ((streamid) << 24) > ICID is in lower 6:0 bits, not in 31:24. That was also my initial impression but it didn't work (smmu global faults with icid 0). Probably there's an ambiguity related to endianness in the documentation. >> +#endif /* !CONFIG_FSL_LSCH2 */ > Nitpick: comment for #endif should match #if condition, which is CONFIG_FSL_LSCH2. > Ok. Thanks for the review! --- Best Regards, Laurentiu