Messages in this thread | | | Date | Tue, 17 Nov 2020 10:05:12 -0800 (PST) | From | matthew.gerlach@linux ... | Subject | RE: [PATCH 1/2] fpga: dfl: refactor cci_enumerate_feature_devs() |
| |
On Tue, 17 Nov 2020, Wu, Hao wrote:
>> Subject: [PATCH 1/2] fpga: dfl: refactor cci_enumerate_feature_devs() >> >> From: Matthew Gerlach <matthew.gerlach@linux.intel.com> >> >> In preparation of looking for dfls based on a vendor >> specific pcie capability, move code that assumes >> Bar0/offset0 as start of DFL to its own function. >> >> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> >> --- >> drivers/fpga/dfl-pci.c | 86 ++++++++++++++++++++++++------------------ >> 1 file changed, 49 insertions(+), 37 deletions(-) >> >> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c >> index a2203d03c9e2..b1b157b41942 100644 >> --- a/drivers/fpga/dfl-pci.c >> +++ b/drivers/fpga/dfl-pci.c >> @@ -119,49 +119,20 @@ static int *cci_pci_create_irq_table(struct pci_dev >> *pcidev, unsigned int nvec) >> return table; >> } >> >> -/* enumerate feature devices under pci device */ >> -static int cci_enumerate_feature_devs(struct pci_dev *pcidev) >> +static int find_dfl_in_bar0(struct pci_dev *pcidev, >> + struct dfl_fpga_enum_info *info) >> { >> - struct cci_drvdata *drvdata = pci_get_drvdata(pcidev); >> - int port_num, bar, i, nvec, ret = 0; >> - struct dfl_fpga_enum_info *info; >> - struct dfl_fpga_cdev *cdev; >> resource_size_t start, len; >> + int port_num, bar, i; >> void __iomem *base; >> - int *irq_table; >> + int ret = 0; >> u32 offset; >> u64 v; >> >> - /* allocate enumeration info via pci_dev */ >> - info = dfl_fpga_enum_info_alloc(&pcidev->dev); >> - if (!info) >> - return -ENOMEM; >> - >> - /* add irq info for enumeration if the device support irq */ >> - nvec = cci_pci_alloc_irq(pcidev); >> - if (nvec < 0) { >> - dev_err(&pcidev->dev, "Fail to alloc irq %d.\n", nvec); >> - ret = nvec; >> - goto enum_info_free_exit; >> - } else if (nvec) { >> - irq_table = cci_pci_create_irq_table(pcidev, nvec); >> - if (!irq_table) { >> - ret = -ENOMEM; >> - goto irq_free_exit; >> - } >> - >> - ret = dfl_fpga_enum_info_add_irq(info, nvec, irq_table); >> - kfree(irq_table); >> - if (ret) >> - goto irq_free_exit; >> - } >> - >> - /* start to find Device Feature List in Bar 0 */ >> + /* start to find Device Feature List from Bar 0 */ >> base = cci_pci_ioremap_bar0(pcidev); >> - if (!base) { >> - ret = -ENOMEM; >> - goto irq_free_exit; >> - } >> + if (!base) >> + return -ENOMEM; >> >> /* >> * PF device has FME and Ports/AFUs, and VF device only has one >> @@ -208,12 +179,53 @@ static int cci_enumerate_feature_devs(struct >> pci_dev *pcidev) >> dfl_fpga_enum_info_add_dfl(info, start, len); >> } else { >> ret = -ENODEV; >> - goto irq_free_exit; >> } >> >> /* release I/O mappings for next step enumeration */ >> pcim_iounmap_regions(pcidev, BIT(0)); >> >> + > > We don't need 2 blank line here, remove one please.
I will remove this line in v2. > >> + return ret; >> +} >> + >> +/* enumerate feature devices under pci device */ >> +static int cci_enumerate_feature_devs(struct pci_dev *pcidev) >> +{ >> + struct cci_drvdata *drvdata = pci_get_drvdata(pcidev); >> + struct dfl_fpga_enum_info *info; >> + struct dfl_fpga_cdev *cdev; >> + int nvec, ret = 0; >> + int *irq_table; >> + >> + /* allocate enumeration info via pci_dev */ >> + info = dfl_fpga_enum_info_alloc(&pcidev->dev); >> + if (!info) >> + return -ENOMEM; >> + >> + /* add irq info for enumeration if the device support irq */ >> + nvec = cci_pci_alloc_irq(pcidev); >> + if (nvec < 0) { >> + dev_err(&pcidev->dev, "Fail to alloc irq %d.\n", nvec); >> + ret = nvec; >> + goto enum_info_free_exit; >> + } else if (nvec) { >> + irq_table = cci_pci_create_irq_table(pcidev, nvec); >> + if (!irq_table) { >> + ret = -ENOMEM; >> + goto irq_free_exit; >> + } >> + >> + ret = dfl_fpga_enum_info_add_irq(info, nvec, irq_table); >> + kfree(irq_table); >> + if (ret) >> + goto irq_free_exit; >> + } >> + >> + ret = find_dfl_in_bar0(pcidev, info); >> + > > Remove this blank line, and maybe switch to a better function name here.
I will remove blank line in v2 and use your suggested function name, find_dfls_by_default. > > Thanks > Hao > >> + if (ret) >> + goto irq_free_exit; >> + >> /* start enumeration with prepared enumeration information */ >> cdev = dfl_fpga_feature_devs_enumerate(info); >> if (IS_ERR(cdev)) { >> -- >> 2.25.2 > >
| |