[U-Boot] [PATCH] board: ls1028a: set up integrated PCI stream IDs and cache attributes

[Alex Marginean]

Hi Bin,

On 8/9/2019 12:58 PM, Bin Meng wrote:
> Hi Alex,
> 
> On Fri, Aug 9, 2019 at 3:59 PM Alex Marginean
> <alexandru.marginean at nxp.com> wrote:
>>
>> Configure stream IDs for integrated PCI devices.  There are hardware
>> defaults but unfortunately they are outside the acceptable range for
>> SMMU, so we need to tune them down.  Use values based on Linux device tree
>> iommu-map or, if missing, start from HW base value shifted down by 4.
>>
>> Signed-off-by: Alex Marginean <alexm.osslist at gmail.com>
>> ---
>>   board/freescale/ls1028a/ls1028a.c | 64 +++++++++++++++++++++++++++++++
>>   1 file changed, 64 insertions(+)
>>
>> diff --git a/board/freescale/ls1028a/ls1028a.c b/board/freescale/ls1028a/ls1028a.c
>> index 49a9292c31..05eac6f9c4 100644
>> --- a/board/freescale/ls1028a/ls1028a.c
>> +++ b/board/freescale/ls1028a/ls1028a.c
>> @@ -118,6 +118,67 @@ void detail_board_ddr_info(void)
>>   }
>>
>>   #ifdef CONFIG_OF_BOARD_SETUP
>> +
>> +/*
>> + * Hardware default stream IDs are 0x4000 + PCI function #, but that's outside
>> + * the acceptable range for SMMU.  Use Linux DT values instead or at least
>> + * smaller defaults.
>> + */
>> +#define ECAM_NUM_PFS                   7
>> +#define ECAM_IERB_BASE                 0x1F0800000
>> +#define ECAM_PFAMQ(pf, vf)             ((ECAM_IERB_BASE + 0x800 + (pf) * \
>> +                                         0x1000 + (vf) * 4))
>> +/* cache related transaction attributes for PCIe functions */
>> +#define ECAM_IERB_MSICAR               (ECAM_IERB_BASE + 0xa400)
>> +#define ECAM_IERB_MSICAR_VALUE         0x30
>> +
>> +/* number of VFs per PF, VFs have their own AMQ settings */
>> +static const u8 enetc_vfs[ECAM_NUM_PFS] = { 2, 2 };
>> +
>> +void setup_ecam_amq(void *blob)
> 
> nits: this should be static

I'll send a v2.

> 
>> +{
>> +       int streamid, sid_base, off;
>> +       int pf, vf, vfnn = 1;
>> +       u32 iommu_map[4];
>> +       int err;
>> +
>> +       /*
>> +        * Look up the stream ID settings in the DT, if found apply the values
>> +        * to HW, otherwise use HW values shifted down by 4.
>> +        */
>> +       off = fdt_node_offset_by_compatible(blob, 0, "pci-host-ecam-generic");
>> +       if (off < 0) {
>> +               debug("ECAM node not found\n");
>> +               return;
>> +       }
>> +
>> +       err = fdtdec_get_int_array(blob, off, "iommu-map", iommu_map, 4);
>> +       if (err) {
>> +               sid_base = in_le32(ECAM_PFAMQ(0, 0)) >> 4;
>> +               debug("\"iommu-map\" not found, using default SID base %04x\n",
>> +                     sid_base);
>> +       } else {
>> +               sid_base = iommu_map[2];
>> +       }
>> +       /* set up AMQs for all integrated PCI functions */
>> +       for (pf = 0; pf < ECAM_NUM_PFS; pf++) {
>> +               streamid = sid_base + pf;
>> +               out_le32(ECAM_PFAMQ(pf, 0), streamid);
>> +
>> +               /* set up AMQs for VFs, if any */
>> +               for (vf = 0; vf < enetc_vfs[pf]; vf++, vfnn++) {
>> +                       streamid = sid_base + ECAM_NUM_PFS + vfnn;
>> +                       out_le32(ECAM_PFAMQ(pf, vf + 1), streamid);
>> +               }
>> +       }
>> +}
>> +
>> +void setup_ecam_cacheattr(void)
> 
> ditto
> 
>> +{
>> +       /* set MSI cache attributes */
>> +       out_le32(ECAM_IERB_MSICAR, ECAM_IERB_MSICAR_VALUE);
>> +}
>> +
>>   int ft_board_setup(void *blob, bd_t *bd)
>>   {
>>          u64 base[CONFIG_NR_DRAM_BANKS];
>> @@ -143,6 +204,9 @@ int ft_board_setup(void *blob, bd_t *bd)
>>
>>          fdt_fixup_memory_banks(blob, base, size, 2);
>>
>> +       setup_ecam_amq(blob);
>> +       setup_ecam_cacheattr();
>> +
>>          return 0;
>>   }
>>   #endif
> 
> Not only programming the registers, but also I think we will need fix
> up the <msi-map> property used by the "pci-host-ecam-generic", just
> like what it was done in fdt_pcie_set_msi_map_entry() in
> pcie_layerscape_fixup.c

The DT loaded with the kernel has the two maps (iommu and msi) in sync,
they are both present and they start from same base ID.  For integrated
PCI we use that as a reference in U-Boot rather than assign the value
from U-Boot.  Layerscape PCI node as far as I remember doesn't have the
msi/iommu-map properties, they are added by U-Boot using IDs it
allocates, but we don't do that here.  The intent is to try to keep
fixups to a minimum, in this case the code is needed only because the HW
defaults are not OK.
For what is worth I tested ping in Linux with this U-Boot change and it
works.

I suppose one potential issue is to make sure U-Boot won't
allocate in the same range as the values used in DT for ECAM.  The
values we use now in Linux DT are OK in the sense that they are over the
range used by U-Boot (FSL_PEX_STREAM_ID_START-FSL_PEX_STREAM_ID_END).
There is no explicit check for overlap between layerscape PCI stream IDs
and ECAM stream IDs, I am guessing such a configuration issue would be
noticeable in Linux though.

Thank you!
Alex

> 
> Regards,
> Bin
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>