Message ID | 1365806683-26717-2-git-send-email-yinghai@kernel.org (mailing list archive) |
---|---|
State | New, archived |
Delegated to: | Bjorn Helgaas |
Headers | show |
On Fri, Apr 12, 2013 at 4:44 PM, Yinghai Lu <yinghai@kernel.org> wrote: > Before every quirk_io_region calling, pci_read_config_word is called. > We can fold that calling into quirk_io_region() to make > code more readable. > > According to Bjorn, split this one as separated patch from > addon resource patch. > > Signed-off-by: Yinghai Lu <yinghai@kernel.org> I applied this to my pci/misc branch for v3.10. Nice cleanup, thanks. I changed quirk_io_region() so that instead of filling in res->start/end with bus addresses, then copying those into bus_region, and finally converting bus-to-region, we just fill in bus_region directly. That is a couple lines short and res never contains a bus address. There's no reason to keep bad examples of putting bus addresses into a struct resource. Bjorn > --- > drivers/pci/quirks.c | 132 ++++++++++++++++++--------------------------------- > 1 file changed, 47 insertions(+), 85 deletions(-) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index 4273a2d..2dac170 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -324,29 +324,32 @@ static void quirk_cs5536_vsa(struct pci_dev *dev) > } > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CS5536_ISA, quirk_cs5536_vsa); > > -static void quirk_io_region(struct pci_dev *dev, unsigned region, > - unsigned size, int nr, const char *name) > +static void quirk_io_region(struct pci_dev *dev, int port, > + unsigned size, int nr, const char *name) > { > - region &= ~(size-1); > - if (region) { > - struct pci_bus_region bus_region; > - struct resource *res = dev->resource + nr; > + u16 region; > + struct pci_bus_region bus_region; > + struct resource *res = dev->resource + nr; > > - res->name = pci_name(dev); > - res->start = region; > - res->end = region + size - 1; > - res->flags = IORESOURCE_IO; > + pci_read_config_word(dev, port, ®ion); > + region &= ~(size - 1); > > - /* Convert from PCI bus to resource space. */ > - bus_region.start = res->start; > - bus_region.end = res->end; > - pcibios_bus_to_resource(dev, res, &bus_region); > + if (!region) > + return; > > - if (pci_claim_resource(dev, nr) == 0) > - dev_info(&dev->dev, "quirk: %pR claimed by %s\n", > - res, name); > - } > -} > + res->name = pci_name(dev); > + res->start = region; > + res->end = region + size - 1; > + res->flags = IORESOURCE_IO; > + > + /* Convert from PCI bus to resource space. */ > + bus_region.start = res->start; > + bus_region.end = res->end; > + pcibios_bus_to_resource(dev, res, &bus_region); > + > + if (!pci_claim_resource(dev, nr)) > + dev_info(&dev->dev, "quirk: %pR claimed by %s\n", res, name); > +} > > /* > * ATI Northbridge setups MCE the processor if you even > @@ -374,12 +377,8 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_RS100, quirk_ati_ > */ > static void quirk_ali7101_acpi(struct pci_dev *dev) > { > - u16 region; > - > - pci_read_config_word(dev, 0xE0, ®ion); > - quirk_io_region(dev, region, 64, PCI_BRIDGE_RESOURCES, "ali7101 ACPI"); > - pci_read_config_word(dev, 0xE2, ®ion); > - quirk_io_region(dev, region, 32, PCI_BRIDGE_RESOURCES+1, "ali7101 SMB"); > + quirk_io_region(dev, 0xE0, 64, PCI_BRIDGE_RESOURCES, "ali7101 ACPI"); > + quirk_io_region(dev, 0xE2, 32, PCI_BRIDGE_RESOURCES+1, "ali7101 SMB"); > } > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AL, PCI_DEVICE_ID_AL_M7101, quirk_ali7101_acpi); > > @@ -442,12 +441,10 @@ static void piix4_mem_quirk(struct pci_dev *dev, const char *name, unsigned int > */ > static void quirk_piix4_acpi(struct pci_dev *dev) > { > - u32 region, res_a; > + u32 res_a; > > - pci_read_config_dword(dev, 0x40, ®ion); > - quirk_io_region(dev, region, 64, PCI_BRIDGE_RESOURCES, "PIIX4 ACPI"); > - pci_read_config_dword(dev, 0x90, ®ion); > - quirk_io_region(dev, region, 16, PCI_BRIDGE_RESOURCES+1, "PIIX4 SMB"); > + quirk_io_region(dev, 0x40, 64, PCI_BRIDGE_RESOURCES, "PIIX4 ACPI"); > + quirk_io_region(dev, 0x90, 16, PCI_BRIDGE_RESOURCES+1, "PIIX4 SMB"); > > /* Device resource A has enables for some of the other ones */ > pci_read_config_dword(dev, 0x5c, &res_a); > @@ -491,7 +488,6 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82443MX_3, qui > */ > static void quirk_ich4_lpc_acpi(struct pci_dev *dev) > { > - u32 region; > u8 enable; > > /* > @@ -503,22 +499,14 @@ static void quirk_ich4_lpc_acpi(struct pci_dev *dev) > */ > > pci_read_config_byte(dev, ICH_ACPI_CNTL, &enable); > - if (enable & ICH4_ACPI_EN) { > - pci_read_config_dword(dev, ICH_PMBASE, ®ion); > - region &= PCI_BASE_ADDRESS_IO_MASK; > - if (region >= PCIBIOS_MIN_IO) > - quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, > - "ICH4 ACPI/GPIO/TCO"); > - } > + if (enable & ICH4_ACPI_EN) > + quirk_io_region(dev, ICH_PMBASE, 128, PCI_BRIDGE_RESOURCES, > + "ICH4 ACPI/GPIO/TCO"); > > pci_read_config_byte(dev, ICH4_GPIO_CNTL, &enable); > - if (enable & ICH4_GPIO_EN) { > - pci_read_config_dword(dev, ICH4_GPIOBASE, ®ion); > - region &= PCI_BASE_ADDRESS_IO_MASK; > - if (region >= PCIBIOS_MIN_IO) > - quirk_io_region(dev, region, 64, > - PCI_BRIDGE_RESOURCES + 1, "ICH4 GPIO"); > - } > + if (enable & ICH4_GPIO_EN) > + quirk_io_region(dev, ICH4_GPIOBASE, 64, PCI_BRIDGE_RESOURCES+1, > + "ICH4 GPIO"); > } > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AA_0, quirk_ich4_lpc_acpi); > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AB_0, quirk_ich4_lpc_acpi); > @@ -533,26 +521,17 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB_1, qui > > static void ich6_lpc_acpi_gpio(struct pci_dev *dev) > { > - u32 region; > u8 enable; > > pci_read_config_byte(dev, ICH_ACPI_CNTL, &enable); > - if (enable & ICH6_ACPI_EN) { > - pci_read_config_dword(dev, ICH_PMBASE, ®ion); > - region &= PCI_BASE_ADDRESS_IO_MASK; > - if (region >= PCIBIOS_MIN_IO) > - quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, > - "ICH6 ACPI/GPIO/TCO"); > - } > + if (enable & ICH6_ACPI_EN) > + quirk_io_region(dev, ICH_PMBASE, 128, PCI_BRIDGE_RESOURCES, > + "ICH6 ACPI/GPIO/TCO"); > > pci_read_config_byte(dev, ICH6_GPIO_CNTL, &enable); > - if (enable & ICH6_GPIO_EN) { > - pci_read_config_dword(dev, ICH6_GPIOBASE, ®ion); > - region &= PCI_BASE_ADDRESS_IO_MASK; > - if (region >= PCIBIOS_MIN_IO) > - quirk_io_region(dev, region, 64, > - PCI_BRIDGE_RESOURCES + 1, "ICH6 GPIO"); > - } > + if (enable & ICH6_GPIO_EN) > + quirk_io_region(dev, ICH6_GPIOBASE, 64, PCI_BRIDGE_RESOURCES+1, > + "ICH6 GPIO"); > } > > static void ich6_lpc_generic_decode(struct pci_dev *dev, unsigned reg, const char *name, int dynsize) > @@ -650,13 +629,9 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH10_1, qui > */ > static void quirk_vt82c586_acpi(struct pci_dev *dev) > { > - u32 region; > - > - if (dev->revision & 0x10) { > - pci_read_config_dword(dev, 0x48, ®ion); > - region &= PCI_BASE_ADDRESS_IO_MASK; > - quirk_io_region(dev, region, 256, PCI_BRIDGE_RESOURCES, "vt82c586 ACPI"); > - } > + if (dev->revision & 0x10) > + quirk_io_region(dev, 0x48, 256, PCI_BRIDGE_RESOURCES, > + "vt82c586 ACPI"); > } > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_3, quirk_vt82c586_acpi); > > @@ -668,18 +643,12 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_3, quirk_vt > */ > static void quirk_vt82c686_acpi(struct pci_dev *dev) > { > - u16 hm; > - u32 smb; > - > quirk_vt82c586_acpi(dev); > > - pci_read_config_word(dev, 0x70, &hm); > - hm &= PCI_BASE_ADDRESS_IO_MASK; > - quirk_io_region(dev, hm, 128, PCI_BRIDGE_RESOURCES + 1, "vt82c686 HW-mon"); > + quirk_io_region(dev, 0x70, 128, PCI_BRIDGE_RESOURCES+1, > + "vt82c686 HW-mon"); > > - pci_read_config_dword(dev, 0x90, &smb); > - smb &= PCI_BASE_ADDRESS_IO_MASK; > - quirk_io_region(dev, smb, 16, PCI_BRIDGE_RESOURCES + 2, "vt82c686 SMB"); > + quirk_io_region(dev, 0x90, 16, PCI_BRIDGE_RESOURCES+2, "vt82c686 SMB"); > } > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_4, quirk_vt82c686_acpi); > > @@ -690,15 +659,8 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_4, quirk_vt > */ > static void quirk_vt8235_acpi(struct pci_dev *dev) > { > - u16 pm, smb; > - > - pci_read_config_word(dev, 0x88, &pm); > - pm &= PCI_BASE_ADDRESS_IO_MASK; > - quirk_io_region(dev, pm, 128, PCI_BRIDGE_RESOURCES, "vt8235 PM"); > - > - pci_read_config_word(dev, 0xd0, &smb); > - smb &= PCI_BASE_ADDRESS_IO_MASK; > - quirk_io_region(dev, smb, 16, PCI_BRIDGE_RESOURCES + 1, "vt8235 SMB"); > + quirk_io_region(dev, 0x88, 128, PCI_BRIDGE_RESOURCES, "vt8235 PM"); > + quirk_io_region(dev, 0xd0, 16, PCI_BRIDGE_RESOURCES+1, "vt8235 SMB"); > } > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8235, quirk_vt8235_acpi); > > -- > 1.8.1.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 4273a2d..2dac170 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -324,29 +324,32 @@ static void quirk_cs5536_vsa(struct pci_dev *dev) } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CS5536_ISA, quirk_cs5536_vsa); -static void quirk_io_region(struct pci_dev *dev, unsigned region, - unsigned size, int nr, const char *name) +static void quirk_io_region(struct pci_dev *dev, int port, + unsigned size, int nr, const char *name) { - region &= ~(size-1); - if (region) { - struct pci_bus_region bus_region; - struct resource *res = dev->resource + nr; + u16 region; + struct pci_bus_region bus_region; + struct resource *res = dev->resource + nr; - res->name = pci_name(dev); - res->start = region; - res->end = region + size - 1; - res->flags = IORESOURCE_IO; + pci_read_config_word(dev, port, ®ion); + region &= ~(size - 1); - /* Convert from PCI bus to resource space. */ - bus_region.start = res->start; - bus_region.end = res->end; - pcibios_bus_to_resource(dev, res, &bus_region); + if (!region) + return; - if (pci_claim_resource(dev, nr) == 0) - dev_info(&dev->dev, "quirk: %pR claimed by %s\n", - res, name); - } -} + res->name = pci_name(dev); + res->start = region; + res->end = region + size - 1; + res->flags = IORESOURCE_IO; + + /* Convert from PCI bus to resource space. */ + bus_region.start = res->start; + bus_region.end = res->end; + pcibios_bus_to_resource(dev, res, &bus_region); + + if (!pci_claim_resource(dev, nr)) + dev_info(&dev->dev, "quirk: %pR claimed by %s\n", res, name); +} /* * ATI Northbridge setups MCE the processor if you even @@ -374,12 +377,8 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_RS100, quirk_ati_ */ static void quirk_ali7101_acpi(struct pci_dev *dev) { - u16 region; - - pci_read_config_word(dev, 0xE0, ®ion); - quirk_io_region(dev, region, 64, PCI_BRIDGE_RESOURCES, "ali7101 ACPI"); - pci_read_config_word(dev, 0xE2, ®ion); - quirk_io_region(dev, region, 32, PCI_BRIDGE_RESOURCES+1, "ali7101 SMB"); + quirk_io_region(dev, 0xE0, 64, PCI_BRIDGE_RESOURCES, "ali7101 ACPI"); + quirk_io_region(dev, 0xE2, 32, PCI_BRIDGE_RESOURCES+1, "ali7101 SMB"); } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AL, PCI_DEVICE_ID_AL_M7101, quirk_ali7101_acpi); @@ -442,12 +441,10 @@ static void piix4_mem_quirk(struct pci_dev *dev, const char *name, unsigned int */ static void quirk_piix4_acpi(struct pci_dev *dev) { - u32 region, res_a; + u32 res_a; - pci_read_config_dword(dev, 0x40, ®ion); - quirk_io_region(dev, region, 64, PCI_BRIDGE_RESOURCES, "PIIX4 ACPI"); - pci_read_config_dword(dev, 0x90, ®ion); - quirk_io_region(dev, region, 16, PCI_BRIDGE_RESOURCES+1, "PIIX4 SMB"); + quirk_io_region(dev, 0x40, 64, PCI_BRIDGE_RESOURCES, "PIIX4 ACPI"); + quirk_io_region(dev, 0x90, 16, PCI_BRIDGE_RESOURCES+1, "PIIX4 SMB"); /* Device resource A has enables for some of the other ones */ pci_read_config_dword(dev, 0x5c, &res_a); @@ -491,7 +488,6 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82443MX_3, qui */ static void quirk_ich4_lpc_acpi(struct pci_dev *dev) { - u32 region; u8 enable; /* @@ -503,22 +499,14 @@ static void quirk_ich4_lpc_acpi(struct pci_dev *dev) */ pci_read_config_byte(dev, ICH_ACPI_CNTL, &enable); - if (enable & ICH4_ACPI_EN) { - pci_read_config_dword(dev, ICH_PMBASE, ®ion); - region &= PCI_BASE_ADDRESS_IO_MASK; - if (region >= PCIBIOS_MIN_IO) - quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, - "ICH4 ACPI/GPIO/TCO"); - } + if (enable & ICH4_ACPI_EN) + quirk_io_region(dev, ICH_PMBASE, 128, PCI_BRIDGE_RESOURCES, + "ICH4 ACPI/GPIO/TCO"); pci_read_config_byte(dev, ICH4_GPIO_CNTL, &enable); - if (enable & ICH4_GPIO_EN) { - pci_read_config_dword(dev, ICH4_GPIOBASE, ®ion); - region &= PCI_BASE_ADDRESS_IO_MASK; - if (region >= PCIBIOS_MIN_IO) - quirk_io_region(dev, region, 64, - PCI_BRIDGE_RESOURCES + 1, "ICH4 GPIO"); - } + if (enable & ICH4_GPIO_EN) + quirk_io_region(dev, ICH4_GPIOBASE, 64, PCI_BRIDGE_RESOURCES+1, + "ICH4 GPIO"); } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AA_0, quirk_ich4_lpc_acpi); DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AB_0, quirk_ich4_lpc_acpi); @@ -533,26 +521,17 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB_1, qui static void ich6_lpc_acpi_gpio(struct pci_dev *dev) { - u32 region; u8 enable; pci_read_config_byte(dev, ICH_ACPI_CNTL, &enable); - if (enable & ICH6_ACPI_EN) { - pci_read_config_dword(dev, ICH_PMBASE, ®ion); - region &= PCI_BASE_ADDRESS_IO_MASK; - if (region >= PCIBIOS_MIN_IO) - quirk_io_region(dev, region, 128, PCI_BRIDGE_RESOURCES, - "ICH6 ACPI/GPIO/TCO"); - } + if (enable & ICH6_ACPI_EN) + quirk_io_region(dev, ICH_PMBASE, 128, PCI_BRIDGE_RESOURCES, + "ICH6 ACPI/GPIO/TCO"); pci_read_config_byte(dev, ICH6_GPIO_CNTL, &enable); - if (enable & ICH6_GPIO_EN) { - pci_read_config_dword(dev, ICH6_GPIOBASE, ®ion); - region &= PCI_BASE_ADDRESS_IO_MASK; - if (region >= PCIBIOS_MIN_IO) - quirk_io_region(dev, region, 64, - PCI_BRIDGE_RESOURCES + 1, "ICH6 GPIO"); - } + if (enable & ICH6_GPIO_EN) + quirk_io_region(dev, ICH6_GPIOBASE, 64, PCI_BRIDGE_RESOURCES+1, + "ICH6 GPIO"); } static void ich6_lpc_generic_decode(struct pci_dev *dev, unsigned reg, const char *name, int dynsize) @@ -650,13 +629,9 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH10_1, qui */ static void quirk_vt82c586_acpi(struct pci_dev *dev) { - u32 region; - - if (dev->revision & 0x10) { - pci_read_config_dword(dev, 0x48, ®ion); - region &= PCI_BASE_ADDRESS_IO_MASK; - quirk_io_region(dev, region, 256, PCI_BRIDGE_RESOURCES, "vt82c586 ACPI"); - } + if (dev->revision & 0x10) + quirk_io_region(dev, 0x48, 256, PCI_BRIDGE_RESOURCES, + "vt82c586 ACPI"); } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_3, quirk_vt82c586_acpi); @@ -668,18 +643,12 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_3, quirk_vt */ static void quirk_vt82c686_acpi(struct pci_dev *dev) { - u16 hm; - u32 smb; - quirk_vt82c586_acpi(dev); - pci_read_config_word(dev, 0x70, &hm); - hm &= PCI_BASE_ADDRESS_IO_MASK; - quirk_io_region(dev, hm, 128, PCI_BRIDGE_RESOURCES + 1, "vt82c686 HW-mon"); + quirk_io_region(dev, 0x70, 128, PCI_BRIDGE_RESOURCES+1, + "vt82c686 HW-mon"); - pci_read_config_dword(dev, 0x90, &smb); - smb &= PCI_BASE_ADDRESS_IO_MASK; - quirk_io_region(dev, smb, 16, PCI_BRIDGE_RESOURCES + 2, "vt82c686 SMB"); + quirk_io_region(dev, 0x90, 16, PCI_BRIDGE_RESOURCES+2, "vt82c686 SMB"); } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_4, quirk_vt82c686_acpi); @@ -690,15 +659,8 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_4, quirk_vt */ static void quirk_vt8235_acpi(struct pci_dev *dev) { - u16 pm, smb; - - pci_read_config_word(dev, 0x88, &pm); - pm &= PCI_BASE_ADDRESS_IO_MASK; - quirk_io_region(dev, pm, 128, PCI_BRIDGE_RESOURCES, "vt8235 PM"); - - pci_read_config_word(dev, 0xd0, &smb); - smb &= PCI_BASE_ADDRESS_IO_MASK; - quirk_io_region(dev, smb, 16, PCI_BRIDGE_RESOURCES + 1, "vt8235 SMB"); + quirk_io_region(dev, 0x88, 128, PCI_BRIDGE_RESOURCES, "vt8235 PM"); + quirk_io_region(dev, 0xd0, 16, PCI_BRIDGE_RESOURCES+1, "vt8235 SMB"); } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8235, quirk_vt8235_acpi);
Before every quirk_io_region calling, pci_read_config_word is called. We can fold that calling into quirk_io_region() to make code more readable. According to Bjorn, split this one as separated patch from addon resource patch. Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- drivers/pci/quirks.c | 132 ++++++++++++++++++--------------------------------- 1 file changed, 47 insertions(+), 85 deletions(-)