[U-Boot] [PATCH 3/4 v2] test: dm: Add a test for PCI Enhanced Allocation

Alexandru Marginean alexandru.marginean at nxp.com
Thu Jun 6 07:38:36 UTC 2019


Hi Bin,

On 6/5/2019 1:05 PM, Bin Meng wrote:
> Hi Alex,
> 
> On Tue, Jun 4, 2019 at 8:46 PM Alex Marginean <alexm.osslist at gmail.com> wrote:
>>
>> This test is built on top of the existing swap_case driver.  It adds EA
>> capability structure support to swap_case and uses that to map BARs.
>> BAR1 works as it used to, swapping upper/lower case.  BARs 2,4 map to a
>> couple of magic values.
>>
>> Signed-off-by: Alex Marginean <alexm.osslist at gmail.com>
>> ---
>>
>> Changes in v2:
>>          - new patch, v1 didn't have a test
>>
>>   arch/sandbox/dts/test.dts       |   8 +++
>>   arch/sandbox/include/asm/test.h |  13 ++++
>>   drivers/misc/swap_case.c        | 102 +++++++++++++++++++++++++++++++-
>>   test/dm/pci.c                   |  50 ++++++++++++++++
>>   4 files changed, 172 insertions(+), 1 deletion(-)
>>
> 
> Well done!
> 
> Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> Tested-by: Bin Meng <bmeng.cn at gmail.com>
> 
> But please see some nits below:

I'm replying from the nxp account, apparently google decided this is
just spam and it's not worth sending out through gmail.

I'll send a v3 with fixes for you comments, should I keep either of your
two tags on this patch?

Thank you!
Alex

> 
>> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
>> index 46d8a56d0f..dd50a951a8 100644
>> --- a/arch/sandbox/dts/test.dts
>> +++ b/arch/sandbox/dts/test.dts
>> @@ -434,6 +434,14 @@
>>                                  compatible = "sandbox,swap-case";
>>                          };
>>                  };
>> +               pci at 1,0 {
>> +                       compatible = "pci-generic";
>> +                       reg = <0x0800 0 0 0 0>;
>> +                       emul at 0,0 {
>> +                               compatible = "sandbox,swap-case";
>> +                               use-ea;
>> +                       };
>> +               };
>>                  pci at 1f,0 {
>>                          compatible = "pci-generic";
>>                          reg = <0xf800 0 0 0 0>;
>> diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h
>> index e956a05262..32125f3037 100644
>> --- a/arch/sandbox/include/asm/test.h
>> +++ b/arch/sandbox/include/asm/test.h
>> @@ -19,6 +19,7 @@
>>   #define PCI_CAP_ID_PM_OFFSET           0x50
>>   #define PCI_CAP_ID_EXP_OFFSET          0x60
>>   #define PCI_CAP_ID_MSIX_OFFSET         0x70
>> +#define PCI_CAP_ID_EA_OFFSET           0x80
>>
>>   #define PCI_EXT_CAP_ID_ERR_OFFSET      0x100
>>   #define PCI_EXT_CAP_ID_VC_OFFSET       0x200
>> @@ -30,6 +31,18 @@
>>
>>   #define SANDBOX_CLK_RATE               32768
>>
>> +/* Macros used to test PCI EA capability structure */
>> +#define PCI_CAP_EA_BASE_LO0            0x00100000
>> +#define PCI_CAP_EA_BASE_LO1            0x00110000
>> +#define PCI_CAP_EA_BASE_LO2            0x00120000
>> +#define PCI_CAP_EA_BASE_LO4            0x00140000
>> +#define PCI_CAP_EA_BASE_HI2            0x00020000ULL
>> +#define PCI_CAP_EA_BASE_HI4            0x00040000ULL
>> +#define PCI_CAP_EA_SIZE_LO             0x0000ffff
>> +#define PCI_CAP_EA_SIZE_HI             0x00000010ULL
>> +#define PCI_EA_BAR2_MAGIC              0x72727272
>> +#define PCI_EA_BAR4_MAGIC              0x74747474
>> +
>>   /* System controller driver data */
>>   enum {
>>          SYSCON0         = 32,
>> diff --git a/drivers/misc/swap_case.c b/drivers/misc/swap_case.c
>> index fa608cec1b..949ef0fdd7 100644
>> --- a/drivers/misc/swap_case.c
>> +++ b/drivers/misc/swap_case.c
>> @@ -61,11 +61,63 @@ static int sandbox_swap_case_get_devfn(struct udevice *dev)
>>          return plat->devfn;
>>   }
>>
>> +static int sandbox_swap_use_ea(struct udevice *dev)
> 
> nits: for consistency, name it as "sandbox_swap_case_use_ea"
> 
>> +{
>> +       return !!ofnode_get_property(dev->node, "use-ea", NULL);
>> +}
>> +
>> +/* Please keep these macros in sync with ea_regs below */
>> +#define PCI_CAP_ID_EA_SIZE             (sizeof(ea_regs) + 4)
>> +#define PCI_CAP_ID_EA_ENTRY_CNT                4
>> +/* Hardcoded EA structure, excluding 1st DW. */
>> +static const u32 ea_regs[] = {
>> +       /* BEI=0, ES=2, BAR0 32b Base + 32b MaxOffset, I/O space */
>> +       (2 << 8) | 2,
>> +       PCI_CAP_EA_BASE_LO0,
>> +       0,
>> +       /* BEI=1, ES=2, BAR1 32b Base + 32b MaxOffset */
>> +       (1 << 4) | 2,
>> +       PCI_CAP_EA_BASE_LO1,
>> +       MEM_TEXT_SIZE - 1,
>> +       /* BEI=2, ES=3, BAR2 64b Base + 32b MaxOffset */
>> +       (2 << 4) | 3,
>> +       PCI_CAP_EA_BASE_LO2 | PCI_EA_IS_64,
>> +       PCI_CAP_EA_SIZE_LO,
>> +       PCI_CAP_EA_BASE_HI2,
>> +       /* BEI=4, ES=4, BAR4 63b Base + 64b MaxOffset */
> 
> nits: typo of '63b'
> 
>> +       (4 << 4) | 4,
>> +       PCI_CAP_EA_BASE_LO4 | PCI_EA_IS_64,
>> +       PCI_CAP_EA_SIZE_LO | PCI_EA_IS_64,
>> +       PCI_CAP_EA_BASE_HI4,
>> +       PCI_CAP_EA_SIZE_HI,
>> +};
>> +
>> +static int sandbox_swap_case_read_ea(struct udevice *emul, uint offset,
>> +                                    ulong *valuep, enum pci_size_t size)
>> +{
>> +       u32 reg;
>> +
>> +       offset = offset - PCI_CAP_ID_EA_OFFSET - 4;
>> +       reg = ea_regs[offset >> 2];
>> +       reg >>= (offset % 4) * 8;
>> +
>> +       *valuep = reg;
>> +       return 0;
>> +}
>> +
>>   static int sandbox_swap_case_read_config(struct udevice *emul, uint offset,
>>                                           ulong *valuep, enum pci_size_t size)
>>   {
>>          struct swap_case_platdata *plat = dev_get_platdata(emul);
>>
>> +       /*
>> +        * The content of the EA capability structure is handled elseware to
> 
> nits: elsewhere
> 
>> +        * keep the switch/case below sane
>> +        */
>> +       if (offset > PCI_CAP_ID_EA_OFFSET + PCI_CAP_LIST_NEXT &&
>> +           offset < PCI_CAP_ID_EA_OFFSET + PCI_CAP_ID_EA_SIZE)
>> +               return sandbox_swap_case_read_ea(emul, offset, valuep, size);
>> +
>>          switch (offset) {
>>          case PCI_COMMAND:
>>                  *valuep = plat->command;
>> @@ -134,9 +186,21 @@ static int sandbox_swap_case_read_config(struct udevice *emul, uint offset,
>>                  *valuep = PCI_CAP_ID_MSIX_OFFSET;
>>                  break;
>>          case PCI_CAP_ID_MSIX_OFFSET:
>> -               *valuep = PCI_CAP_ID_MSIX;
>> +               if (sandbox_swap_use_ea(emul))
>> +                       *valuep = (PCI_CAP_ID_EA_OFFSET << 8) | PCI_CAP_ID_MSIX;
>> +               else
>> +                       *valuep = PCI_CAP_ID_MSIX;
>>                  break;
>>          case PCI_CAP_ID_MSIX_OFFSET + PCI_CAP_LIST_NEXT:
>> +               if (sandbox_swap_use_ea(emul))
>> +                       *valuep = PCI_CAP_ID_EA_OFFSET;
>> +               else
>> +                       *valuep = 0;
>> +               break;
>> +       case PCI_CAP_ID_EA_OFFSET:
>> +               *valuep = (PCI_CAP_ID_EA_ENTRY_CNT << 16) | PCI_CAP_ID_EA;
>> +               break;
>> +       case PCI_CAP_ID_EA_OFFSET + PCI_CAP_LIST_NEXT:
>>                  *valuep = 0;
>>                  break;
>>          case PCI_EXT_CAP_ID_ERR_OFFSET:
>> @@ -257,6 +321,9 @@ int sandbox_swap_case_write_io(struct udevice *dev, unsigned int addr,
>>          return 0;
>>   }
>>
>> +static int pci_ea_bar2_magic = PCI_EA_BAR2_MAGIC;
>> +static int pci_ea_bar4_magic = PCI_EA_BAR4_MAGIC;
>> +
>>   static int sandbox_swap_case_map_physmem(struct udevice *dev,
>>                  phys_addr_t addr, unsigned long *lenp, void **ptrp)
>>   {
>> @@ -265,9 +332,42 @@ static int sandbox_swap_case_map_physmem(struct udevice *dev,
>>          int barnum;
>>          int ret;
>>
>> +       if (sandbox_swap_use_ea(dev)) {
>> +               /*
>> +                * only support mapping base address in EA test for now, we
>> +                * don't handle mapping an offset inside a BAR.  Seems good
>> +                * enough for the current test.
>> +                */
>> +               switch (addr) {
>> +               case (phys_addr_t)PCI_CAP_EA_BASE_LO0:
>> +                       *ptrp = &priv->op;
>> +                       *lenp = 4;
>> +                       break;
>> +               case (phys_addr_t)PCI_CAP_EA_BASE_LO1:
>> +                       *ptrp = priv->mem_text;
>> +                       *lenp = barinfo[1].size - 1;
>> +                       break;
>> +               case (phys_addr_t)((PCI_CAP_EA_BASE_HI2 << 32) |
>> +                                  PCI_CAP_EA_BASE_LO2):
>> +                       *ptrp = &pci_ea_bar2_magic;
>> +                       *lenp = PCI_CAP_EA_SIZE_LO;
>> +                       break;
>> +               case (phys_addr_t)((PCI_CAP_EA_BASE_HI4 << 32) |
>> +                                  PCI_CAP_EA_BASE_LO4):
>> +                       *ptrp = &pci_ea_bar4_magic;
>> +                       *lenp = (PCI_CAP_EA_SIZE_HI << 32) |
>> +                               PCI_CAP_EA_SIZE_LO;
>> +                       break;
>> +               default:
>> +                       return -ENOENT;
>> +               }
>> +               return 0;
>> +       }
>> +
>>          ret = sandbox_swap_case_find_bar(dev, addr, &barnum, &offset);
>>          if (ret)
>>                  return ret;
>> +
>>          if (barnum == 1) {
>>                  *ptrp = priv->mem_text + offset;
>>                  avail = barinfo[1].size - offset;
>> diff --git a/test/dm/pci.c b/test/dm/pci.c
>> index a1febd54b7..4657f5d68d 100644
>> --- a/test/dm/pci.c
>> +++ b/test/dm/pci.c
>> @@ -245,3 +245,53 @@ static int dm_test_pci_cap(struct unit_test_state *uts)
>>          return 0;
>>   }
>>   DM_TEST(dm_test_pci_cap, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
>> +
>> +/* Test looking up BARs in EA capability structure */
>> +static int dm_test_pci_ea(struct unit_test_state *uts)
>> +{
>> +       struct udevice *bus, *swap;
>> +       void *bar;
>> +       int cap;
>> +
>> +       /*
>> +        * use emulated device mapping function, we're not using real physical
>> +        * addresses in this test
>> +        */
>> +       sandbox_set_enable_pci_map(true);
>> +
>> +       ut_assertok(uclass_get_device_by_seq(UCLASS_PCI, 0, &bus));
>> +       ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(0, 0x01, 0), &swap));
>> +
>> +       /* look up PCI_CAP_ID_EA */
>> +       cap = dm_pci_find_capability(swap, PCI_CAP_ID_EA);
>> +       ut_asserteq(PCI_CAP_ID_EA_OFFSET, cap);
>> +
>> +       /* test swap case in BAR 1 */
>> +       bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_0, 0);
>> +       ut_assertnonnull(bar);
>> +       *(int *)bar = 2; /* swap upper/lower */
>> +
>> +       bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0);
>> +       ut_assertnonnull(bar);
>> +       strcpy(bar, "ea TEST");
>> +       unmap_sysmem(bar);
>> +       bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0);
>> +       ut_assertnonnull(bar);
>> +       ut_asserteq_str("EA test", bar);
>> +
>> +       /* test magic values in BARs2, 4;  BAR 3 is n/a */
>> +       bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_2, 0);
>> +       ut_assertnonnull(bar);
>> +       ut_asserteq(PCI_EA_BAR2_MAGIC, *(u32 *)bar);
>> +
>> +       bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_3, 0);
>> +       ut_assertnull(bar);
>> +
>> +       bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_4, 0);
>> +       ut_assertnonnull(bar);
>> +       ut_asserteq(PCI_EA_BAR4_MAGIC, *(u32 *)bar);
>> +
>> +       return 0;
>> +}
>> +
> 
> nits: remove this blank line
> 
>> +DM_TEST(dm_test_pci_ea, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
>> --
> 
> Regards,
> Bin
> 



More information about the U-Boot mailing list