From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexandru Marginean Date: Thu, 6 Jun 2019 07:38:36 +0000 Subject: [U-Boot] [PATCH 3/4 v2] test: dm: Add a test for PCI Enhanced Allocation References: <20190604124628.31882-1-alexm.osslist@gmail.com> <20190604124628.31882-3-alexm.osslist@gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 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 >> --- >> >> 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 > Tested-by: Bin Meng > > 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 >