| Message ID | 20190916204158.6889-3-efremov@linux.com |
|---|---|
| State | Deferred |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [RESEND,v3,01/26] PCI: Add define for the number of standard PCI BARs | expand |
> -----Original Message----- > From: Denis Efremov <efremov@linux.com> > Sent: Monday, September 16, 2019 4:42 PM > To: Bjorn Helgaas <bhelgaas@google.com> > Cc: Denis Efremov <efremov@linux.com>; linux-kernel@vger.kernel.org; > linux-pci@vger.kernel.org; Andrew Murray <andrew.murray@arm.com>; > linux-hyperv@vger.kernel.org; KY Srinivasan <kys@microsoft.com>; Haiyang > Zhang <haiyangz@microsoft.com>; Stephen Hemminger > <sthemmin@microsoft.com>; Sasha Levin <sashal@kernel.org> > Subject: [PATCH v3 02/26] PCI: hv: Use PCI_STD_NUM_BARS > > Replace the magic constant (6) with define PCI_STD_NUM_BARS > representing the number of PCI BARs. > > Cc: "K. Y. Srinivasan" <kys@microsoft.com> > Cc: Haiyang Zhang <haiyangz@microsoft.com> > Cc: Stephen Hemminger <sthemmin@microsoft.com> > Cc: Sasha Levin <sashal@kernel.org> > Signed-off-by: Denis Efremov <efremov@linux.com> > --- > drivers/pci/controller/pci-hyperv.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci- > hyperv.c > index 40b625458afa..1665c23b649f 100644 > --- a/drivers/pci/controller/pci-hyperv.c > +++ b/drivers/pci/controller/pci-hyperv.c > @@ -307,7 +307,7 @@ struct pci_bus_relations { struct > pci_q_res_req_response { > struct vmpacket_descriptor hdr; > s32 status; /* negative values are failures */ > - u32 probed_bar[6]; > + u32 probed_bar[PCI_STD_NUM_BARS]; > } __packed; > > struct pci_set_power { > @@ -503,7 +503,7 @@ struct hv_pci_dev { > * What would be observed if one wrote 0xFFFFFFFF to a BAR and > then > * read it back, for each of the BAR offsets within config space. > */ > - u32 probed_bar[6]; > + u32 probed_bar[PCI_STD_NUM_BARS]; > }; > > struct hv_pci_compl { > @@ -1327,7 +1327,7 @@ static void survey_child_resources(struct > hv_pcibus_device *hbus) > * so it's sufficient to just add them up without tracking alignment. > */ > list_for_each_entry(hpdev, &hbus->children, list_entry) { > - for (i = 0; i < 6; i++) { > + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > if (hpdev->probed_bar[i] & > PCI_BASE_ADDRESS_SPACE_IO) > dev_err(&hbus->hdev->device, > "There's an I/O BAR in this list!\n"); > @@ -1401,7 +1401,7 @@ static void prepopulate_bars(struct > hv_pcibus_device *hbus) > /* Pick addresses for the BARs. */ > do { > list_for_each_entry(hpdev, &hbus->children, list_entry) { > - for (i = 0; i < 6; i++) { > + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > bar_val = hpdev->probed_bar[i]; > if (bar_val == 0) > continue; > @@ -1558,7 +1558,7 @@ static void q_resource_requirements(void > *context, struct pci_response *resp, > "query resource requirements failed: %x\n", > resp->status); > } else { > - for (i = 0; i < 6; i++) { > + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > completion->hpdev->probed_bar[i] = > q_res_req->probed_bar[i]; > } Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com> Thanks.
On Mon, Sep 16, 2019 at 11:41:34PM +0300, Denis Efremov wrote: > Replace the magic constant (6) with define PCI_STD_NUM_BARS representing > the number of PCI BARs. For some reason patches 0 and 1 didn't make it to the list. Can you resend them?
On Thu, Sep 26, 2019 at 05:05:31PM -0500, Bjorn Helgaas wrote: > On Mon, Sep 16, 2019 at 11:41:34PM +0300, Denis Efremov wrote: > > Replace the magic constant (6) with define PCI_STD_NUM_BARS representing > > the number of PCI BARs. > > For some reason patches 0 and 1 didn't make it to the list. Can you > resend them? (No need to resend the whole series, which might annoy all the other maintainers. Just send 0 (the cover letter) and 1 (which I assume adds the PCI_STD_NUM_BARS definition)).
Code that iterates over all standard PCI BARs typically uses PCI_STD_RESOURCE_END, but this is error-prone because it requires "i <= PCI_STD_RESOURCE_END" rather than something like "i < PCI_STD_NUM_BARS". We could add such a definition and use it the same way PCI_SRIOV_NUM_BARS is used. The patchset also replaces constant (6) with new define PCI_STD_NUM_BARS where appropriate and removes local declarations for the number of PCI BARs. Changes in v3: - Updated commits description. - Refactored "< PCI_ROM_RESOURCE" with "< PCI_STD_NUM_BARS" in loops. - Refactored "<= BAR_5" with "< PCI_STD_NUM_BARS" in loops. - Removed local define GASKET_NUM_BARS. - Removed local define PCI_NUM_BAR_RESOURCES. Changes in v2: - Reversed checks in pci_iomap_range,pci_iomap_wc_range. - Refactored loops in vfio_pci to keep PCI_STD_RESOURCES. - Added 2 new patches to replace the magic constant with new define. - Splitted net patch in v1 to separate stmmac and dwc-xlgmac patches. Denis Efremov (26): PCI: Add define for the number of standard PCI BARs PCI: hv: Use PCI_STD_NUM_BARS PCI: dwc: Use PCI_STD_NUM_BARS PCI: endpoint: Use PCI_STD_NUM_BARS misc: pci_endpoint_test: Use PCI_STD_NUM_BARS s390/pci: Use PCI_STD_NUM_BARS x86/PCI: Loop using PCI_STD_NUM_BARS alpha/PCI: Use PCI_STD_NUM_BARS ia64: Use PCI_STD_NUM_BARS stmmac: pci: Loop using PCI_STD_NUM_BARS net: dwc-xlgmac: Loop using PCI_STD_NUM_BARS ixgb: use PCI_STD_NUM_BARS e1000: Use PCI_STD_NUM_BARS rapidio/tsi721: Loop using PCI_STD_NUM_BARS efifb: Loop using PCI_STD_NUM_BARS fbmem: use PCI_STD_NUM_BARS vfio_pci: Loop using PCI_STD_NUM_BARS scsi: pm80xx: Use PCI_STD_NUM_BARS ata: sata_nv: Use PCI_STD_NUM_BARS staging: gasket: Use PCI_STD_NUM_BARS serial: 8250_pci: Use PCI_STD_NUM_BARS pata_atp867x: Use PCI_STD_NUM_BARS memstick: use PCI_STD_NUM_BARS USB: core: Use PCI_STD_NUM_BARS usb: pci-quirks: Use PCI_STD_NUM_BARS devres: use PCI_STD_NUM_BARS arch/alpha/kernel/pci-sysfs.c | 8 ++--- arch/ia64/sn/pci/pcibr/pcibr_dma.c | 4 +-- arch/s390/include/asm/pci.h | 5 +-- arch/s390/include/asm/pci_clp.h | 6 ++-- arch/s390/pci/pci.c | 16 +++++----- arch/s390/pci/pci_clp.c | 6 ++-- arch/x86/pci/common.c | 2 +- arch/x86/pci/intel_mid_pci.c | 2 +- drivers/ata/pata_atp867x.c | 2 +- drivers/ata/sata_nv.c | 2 +- drivers/memstick/host/jmb38x_ms.c | 2 +- drivers/misc/pci_endpoint_test.c | 8 ++--- drivers/net/ethernet/intel/e1000/e1000.h | 1 - drivers/net/ethernet/intel/e1000/e1000_main.c | 2 +- drivers/net/ethernet/intel/ixgb/ixgb.h | 1 - drivers/net/ethernet/intel/ixgb/ixgb_main.c | 2 +- .../net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 +-- .../net/ethernet/synopsys/dwc-xlgmac-pci.c | 2 +- drivers/pci/controller/dwc/pci-dra7xx.c | 2 +- .../pci/controller/dwc/pci-layerscape-ep.c | 2 +- drivers/pci/controller/dwc/pcie-artpec6.c | 2 +- .../pci/controller/dwc/pcie-designware-plat.c | 2 +- drivers/pci/controller/dwc/pcie-designware.h | 2 +- drivers/pci/controller/pci-hyperv.c | 10 +++--- drivers/pci/endpoint/functions/pci-epf-test.c | 10 +++--- drivers/pci/pci-sysfs.c | 4 +-- drivers/pci/pci.c | 13 ++++---- drivers/pci/proc.c | 4 +-- drivers/pci/quirks.c | 4 +-- drivers/rapidio/devices/tsi721.c | 2 +- drivers/scsi/pm8001/pm8001_hwi.c | 2 +- drivers/scsi/pm8001/pm8001_init.c | 2 +- drivers/staging/gasket/gasket_constants.h | 3 -- drivers/staging/gasket/gasket_core.c | 12 +++---- drivers/staging/gasket/gasket_core.h | 4 +-- drivers/tty/serial/8250/8250_pci.c | 8 ++--- drivers/usb/core/hcd-pci.c | 2 +- drivers/usb/host/pci-quirks.c | 2 +- drivers/vfio/pci/vfio_pci.c | 11 ++++--- drivers/vfio/pci/vfio_pci_config.c | 32 ++++++++++--------- drivers/vfio/pci/vfio_pci_private.h | 4 +-- drivers/video/fbdev/core/fbmem.c | 4 +-- drivers/video/fbdev/efifb.c | 2 +- include/linux/pci-epc.h | 2 +- include/linux/pci.h | 2 +- include/uapi/linux/pci_regs.h | 1 + lib/devres.c | 2 +- 47 files changed, 112 insertions(+), 115 deletions(-)
On Sat, Sep 28, 2019 at 02:40:26AM +0300, Denis Efremov wrote: > Code that iterates over all standard PCI BARs typically uses > PCI_STD_RESOURCE_END, but this is error-prone because it requires > "i <= PCI_STD_RESOURCE_END" rather than something like > "i < PCI_STD_NUM_BARS". We could add such a definition and use it the same > way PCI_SRIOV_NUM_BARS is used. The patchset also replaces constant (6) > with new define PCI_STD_NUM_BARS where appropriate and removes local > declarations for the number of PCI BARs. > > Changes in v3: > - Updated commits description. > - Refactored "< PCI_ROM_RESOURCE" with "< PCI_STD_NUM_BARS" in loops. > - Refactored "<= BAR_5" with "< PCI_STD_NUM_BARS" in loops. > - Removed local define GASKET_NUM_BARS. > - Removed local define PCI_NUM_BAR_RESOURCES. > > Changes in v2: > - Reversed checks in pci_iomap_range,pci_iomap_wc_range. > - Refactored loops in vfio_pci to keep PCI_STD_RESOURCES. > - Added 2 new patches to replace the magic constant with new define. > - Splitted net patch in v1 to separate stmmac and dwc-xlgmac patches. > > Denis Efremov (26): > PCI: Add define for the number of standard PCI BARs > PCI: hv: Use PCI_STD_NUM_BARS > PCI: dwc: Use PCI_STD_NUM_BARS > PCI: endpoint: Use PCI_STD_NUM_BARS > misc: pci_endpoint_test: Use PCI_STD_NUM_BARS > s390/pci: Use PCI_STD_NUM_BARS > x86/PCI: Loop using PCI_STD_NUM_BARS > alpha/PCI: Use PCI_STD_NUM_BARS > ia64: Use PCI_STD_NUM_BARS > stmmac: pci: Loop using PCI_STD_NUM_BARS > net: dwc-xlgmac: Loop using PCI_STD_NUM_BARS > ixgb: use PCI_STD_NUM_BARS > e1000: Use PCI_STD_NUM_BARS > rapidio/tsi721: Loop using PCI_STD_NUM_BARS > efifb: Loop using PCI_STD_NUM_BARS > fbmem: use PCI_STD_NUM_BARS > vfio_pci: Loop using PCI_STD_NUM_BARS > scsi: pm80xx: Use PCI_STD_NUM_BARS > ata: sata_nv: Use PCI_STD_NUM_BARS > staging: gasket: Use PCI_STD_NUM_BARS > serial: 8250_pci: Use PCI_STD_NUM_BARS > pata_atp867x: Use PCI_STD_NUM_BARS > memstick: use PCI_STD_NUM_BARS > USB: core: Use PCI_STD_NUM_BARS > usb: pci-quirks: Use PCI_STD_NUM_BARS > devres: use PCI_STD_NUM_BARS > > arch/alpha/kernel/pci-sysfs.c | 8 ++--- > arch/ia64/sn/pci/pcibr/pcibr_dma.c | 4 +-- > arch/s390/include/asm/pci.h | 5 +-- > arch/s390/include/asm/pci_clp.h | 6 ++-- > arch/s390/pci/pci.c | 16 +++++----- > arch/s390/pci/pci_clp.c | 6 ++-- > arch/x86/pci/common.c | 2 +- > arch/x86/pci/intel_mid_pci.c | 2 +- > drivers/ata/pata_atp867x.c | 2 +- > drivers/ata/sata_nv.c | 2 +- > drivers/memstick/host/jmb38x_ms.c | 2 +- > drivers/misc/pci_endpoint_test.c | 8 ++--- > drivers/net/ethernet/intel/e1000/e1000.h | 1 - > drivers/net/ethernet/intel/e1000/e1000_main.c | 2 +- > drivers/net/ethernet/intel/ixgb/ixgb.h | 1 - > drivers/net/ethernet/intel/ixgb/ixgb_main.c | 2 +- > .../net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 +-- > .../net/ethernet/synopsys/dwc-xlgmac-pci.c | 2 +- > drivers/pci/controller/dwc/pci-dra7xx.c | 2 +- > .../pci/controller/dwc/pci-layerscape-ep.c | 2 +- > drivers/pci/controller/dwc/pcie-artpec6.c | 2 +- > .../pci/controller/dwc/pcie-designware-plat.c | 2 +- > drivers/pci/controller/dwc/pcie-designware.h | 2 +- > drivers/pci/controller/pci-hyperv.c | 10 +++--- > drivers/pci/endpoint/functions/pci-epf-test.c | 10 +++--- > drivers/pci/pci-sysfs.c | 4 +-- > drivers/pci/pci.c | 13 ++++---- > drivers/pci/proc.c | 4 +-- > drivers/pci/quirks.c | 4 +-- > drivers/rapidio/devices/tsi721.c | 2 +- > drivers/scsi/pm8001/pm8001_hwi.c | 2 +- > drivers/scsi/pm8001/pm8001_init.c | 2 +- > drivers/staging/gasket/gasket_constants.h | 3 -- > drivers/staging/gasket/gasket_core.c | 12 +++---- > drivers/staging/gasket/gasket_core.h | 4 +-- > drivers/tty/serial/8250/8250_pci.c | 8 ++--- > drivers/usb/core/hcd-pci.c | 2 +- > drivers/usb/host/pci-quirks.c | 2 +- > drivers/vfio/pci/vfio_pci.c | 11 ++++--- > drivers/vfio/pci/vfio_pci_config.c | 32 ++++++++++--------- > drivers/vfio/pci/vfio_pci_private.h | 4 +-- > drivers/video/fbdev/core/fbmem.c | 4 +-- > drivers/video/fbdev/efifb.c | 2 +- > include/linux/pci-epc.h | 2 +- > include/linux/pci.h | 2 +- > include/uapi/linux/pci_regs.h | 1 + > lib/devres.c | 2 +- > 47 files changed, 112 insertions(+), 115 deletions(-) Applied to pci/resource for v5.5, thanks! I ended up squashing these all together because they're all related and tiny.
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c index 40b625458afa..1665c23b649f 100644 --- a/drivers/pci/controller/pci-hyperv.c +++ b/drivers/pci/controller/pci-hyperv.c @@ -307,7 +307,7 @@ struct pci_bus_relations { struct pci_q_res_req_response { struct vmpacket_descriptor hdr; s32 status; /* negative values are failures */ - u32 probed_bar[6]; + u32 probed_bar[PCI_STD_NUM_BARS]; } __packed; struct pci_set_power { @@ -503,7 +503,7 @@ struct hv_pci_dev { * What would be observed if one wrote 0xFFFFFFFF to a BAR and then * read it back, for each of the BAR offsets within config space. */ - u32 probed_bar[6]; + u32 probed_bar[PCI_STD_NUM_BARS]; }; struct hv_pci_compl { @@ -1327,7 +1327,7 @@ static void survey_child_resources(struct hv_pcibus_device *hbus) * so it's sufficient to just add them up without tracking alignment. */ list_for_each_entry(hpdev, &hbus->children, list_entry) { - for (i = 0; i < 6; i++) { + for (i = 0; i < PCI_STD_NUM_BARS; i++) { if (hpdev->probed_bar[i] & PCI_BASE_ADDRESS_SPACE_IO) dev_err(&hbus->hdev->device, "There's an I/O BAR in this list!\n"); @@ -1401,7 +1401,7 @@ static void prepopulate_bars(struct hv_pcibus_device *hbus) /* Pick addresses for the BARs. */ do { list_for_each_entry(hpdev, &hbus->children, list_entry) { - for (i = 0; i < 6; i++) { + for (i = 0; i < PCI_STD_NUM_BARS; i++) { bar_val = hpdev->probed_bar[i]; if (bar_val == 0) continue; @@ -1558,7 +1558,7 @@ static void q_resource_requirements(void *context, struct pci_response *resp, "query resource requirements failed: %x\n", resp->status); } else { - for (i = 0; i < 6; i++) { + for (i = 0; i < PCI_STD_NUM_BARS; i++) { completion->hpdev->probed_bar[i] = q_res_req->probed_bar[i]; }
Replace the magic constant (6) with define PCI_STD_NUM_BARS representing the number of PCI BARs. Cc: "K. Y. Srinivasan" <kys@microsoft.com> Cc: Haiyang Zhang <haiyangz@microsoft.com> Cc: Stephen Hemminger <sthemmin@microsoft.com> Cc: Sasha Levin <sashal@kernel.org> Signed-off-by: Denis Efremov <efremov@linux.com> --- drivers/pci/controller/pci-hyperv.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)