| Message ID | 20240716193246.1909697-2-stewart.hildebrand@amd.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: Align small (<4k) BARs | expand |
On Tue, 2024-07-16 at 15:32 -0400, Stewart Hildebrand wrote: > ... to reduce indentation level. Take the opportunity to remove > redundant info from debug print string. Is that intended to be the final commit message or is it still a draft? I'd call the commit "x86/PCI: Improve code readability" (or sth like that) since that is what the commit is about. It's not so much about the code move per se. and have a short message such as: " The indentation in pcibios_allocate_dev_resources() is unusally deep. Improve that by moving some of its code to the new function alloc_resource(). As we're at it, remove redundant information from dev_dbg(). " Regards, P. > > Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> > --- > v1->v2: > * new patch > --- > arch/x86/pci/i386.c | 38 +++++++++++++++++++++----------------- > 1 file changed, 21 insertions(+), 17 deletions(-) > > diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c > index f2f4a5d50b27..3abd55902dbc 100644 > --- a/arch/x86/pci/i386.c > +++ b/arch/x86/pci/i386.c > @@ -246,6 +246,25 @@ struct pci_check_idx_range { > int end; > }; > > +static void alloc_resource(struct pci_dev *dev, int idx, int pass) > +{ > + struct resource *r = &dev->resource[idx]; > + > + dev_dbg(&dev->dev, "BAR %d: reserving %pr (p=%d)\n", idx, r, > pass); > + > + if (pci_claim_resource(dev, idx) < 0) { > + if (r->flags & IORESOURCE_PCI_FIXED) { > + dev_info(&dev->dev, "BAR %d %pR is > immovable\n", > + idx, r); > + } else { > + /* We'll assign a new address later */ > + pcibios_save_fw_addr(dev, idx, r->start); > + r->end -= r->start; > + r->start = 0; > + } > + } > +} > + > static void pcibios_allocate_dev_resources(struct pci_dev *dev, int > pass) > { > int idx, disabled, i; > @@ -271,23 +290,8 @@ static void > pcibios_allocate_dev_resources(struct pci_dev *dev, int pass) > disabled = !(command & > PCI_COMMAND_IO); > else > disabled = !(command & > PCI_COMMAND_MEMORY); > - if (pass == disabled) { > - dev_dbg(&dev->dev, > - "BAR %d: reserving %pr (d=%d, > p=%d)\n", > - idx, r, disabled, pass); > - if (pci_claim_resource(dev, idx) < 0) > { > - if (r->flags & > IORESOURCE_PCI_FIXED) { > - dev_info(&dev->dev, > "BAR %d %pR is immovable\n", > - idx, r); > - } else { > - /* We'll assign a new > address later */ > - > pcibios_save_fw_addr(de > v, > - idx, > r->start); > - r->end -= r->start; > - r->start = 0; > - } > - } > - } > + if (pass == disabled) > + alloc_resource(dev, idx, pass); > } > if (!pass) { > r = &dev->resource[PCI_ROM_RESOURCE];
On 7/17/24 15:28, Philipp Stanner wrote: > On Tue, 2024-07-16 at 15:32 -0400, Stewart Hildebrand wrote: >> ... to reduce indentation level. Take the opportunity to remove >> redundant info from debug print string. > > Is that intended to be the final commit message or is it still a draft? > > I'd call the commit "x86/PCI: Improve code readability" (or sth like > that) since that is what the commit is about. It's not so much about > the code move per se. > > and have a short message such as: > > " > The indentation in pcibios_allocate_dev_resources() is unusally deep. > > Improve that by moving some of its code to the new function > alloc_resource(). > > As we're at it, remove redundant information from dev_dbg(). > " > > > Regards, > P. Thanks for the suggestion. I'll rework it.
diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c index f2f4a5d50b27..3abd55902dbc 100644 --- a/arch/x86/pci/i386.c +++ b/arch/x86/pci/i386.c @@ -246,6 +246,25 @@ struct pci_check_idx_range { int end; }; +static void alloc_resource(struct pci_dev *dev, int idx, int pass) +{ + struct resource *r = &dev->resource[idx]; + + dev_dbg(&dev->dev, "BAR %d: reserving %pr (p=%d)\n", idx, r, pass); + + if (pci_claim_resource(dev, idx) < 0) { + if (r->flags & IORESOURCE_PCI_FIXED) { + dev_info(&dev->dev, "BAR %d %pR is immovable\n", + idx, r); + } else { + /* We'll assign a new address later */ + pcibios_save_fw_addr(dev, idx, r->start); + r->end -= r->start; + r->start = 0; + } + } +} + static void pcibios_allocate_dev_resources(struct pci_dev *dev, int pass) { int idx, disabled, i; @@ -271,23 +290,8 @@ static void pcibios_allocate_dev_resources(struct pci_dev *dev, int pass) disabled = !(command & PCI_COMMAND_IO); else disabled = !(command & PCI_COMMAND_MEMORY); - if (pass == disabled) { - dev_dbg(&dev->dev, - "BAR %d: reserving %pr (d=%d, p=%d)\n", - idx, r, disabled, pass); - if (pci_claim_resource(dev, idx) < 0) { - if (r->flags & IORESOURCE_PCI_FIXED) { - dev_info(&dev->dev, "BAR %d %pR is immovable\n", - idx, r); - } else { - /* We'll assign a new address later */ - pcibios_save_fw_addr(dev, - idx, r->start); - r->end -= r->start; - r->start = 0; - } - } - } + if (pass == disabled) + alloc_resource(dev, idx, pass); } if (!pass) { r = &dev->resource[PCI_ROM_RESOURCE];
... to reduce indentation level. Take the opportunity to remove redundant info from debug print string. Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> --- v1->v2: * new patch --- arch/x86/pci/i386.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)