Re: [PATCH] PCI: Fix general code style

From: Sergio Miguéns Iglesias
Date: Mon Aug 30 2021 - 11:52:47 EST


I can not thank you enough for the amount of time you must have spent
writing this response. I will look into those things in the following
days for sure! ( I have already started looking into the "__ref" stuff)

Thanks again for this,
Sergio M. Iglesias.

On 21/08/25 03:43, Bjorn Helgaas wrote:
> On Thu, Aug 05, 2021 at 12:28:32AM +0200, Sergio Miguéns Iglesias wrote:
> > The code style for most files was fixed. This means that blank lines
> > were added when needed (normally after variable declarations), spaces
> > before tabs were removed, some code alignment issues were solved, block
> > comment style was fixed, every instance of "unsigned var" was replaced
> > with "unsigned int var"... Etc.
> >
> > This commit does not change the logic of the code, it just fixes
> > aesthetic problems.
>
> I generally *like* this, and it does fix some annoying things, but I
> think it's a little too much all at once. If we're working in a file
> and doing actual bug fixes or new functionality, and we want to fix
> some typos or something at the end, that might be OK, but I think the
> churn in the git history outweighs the benefit of this huge patch.
>
> So I would encourage you to use some of the PCI expertise you've
> gained by looking at all this code to work on something with a little
> more impact. Here are a couple ideas:
>
> - There are only two uses of __ref and __refdata in drivers/pci/.
> The fact that they're so rare makes me suspect that we don't need
> them. But I haven't investigated these to see. Somebody could
> check that out and remove them if we don't need them. Be aware
> that I will want a clear argument for why they're not needed :)
>
> - Coverity complains about several issues in drivers/pci/ [1]. Most
> of the time these are false positives, but not always. Sometimes
> there's an actual bug, and sometimes there's a way to restructure
> the code to avoid the warning (which usually means doing things
> the same way they are done elsewhere).
>
> - "make C=2 drivers/pci/" (sparse checker, [2]) complains about a
> few things. Leave the pci_power_t ones alone for now, but there
> are a couple other type issues that could be cleaned up.
>
> [1] https://docs.google.com/spreadsheets/d/19eyNDou83JACzf44j0NRzEWysva6g44G2_Z9IEXGVNk/edit?usp=sharing
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/sparse.rst?id=v5.13
>
> > Signed-off-by: Sergio Miguéns Iglesias <sergio@xxxxxxxx>
> > ---
> > drivers/pci/access.c | 22 +++++++++++++---------
> > drivers/pci/bus.c | 3 ++-
> > drivers/pci/msi.c | 12 +++++++-----
> > drivers/pci/pci-acpi.c | 3 ++-
> > drivers/pci/pci-driver.c | 19 +++++++++++++------
> > drivers/pci/pci-sysfs.c | 14 ++++++++++++--
> > drivers/pci/pci.c | 16 ++++++++++++----
> > drivers/pci/proc.c | 15 +++++++++++++++
> > drivers/pci/quirks.c | 35 ++++++++++++++++++++++++-----------
> > drivers/pci/remove.c | 1 +
> > drivers/pci/rom.c | 2 +-
> > drivers/pci/setup-bus.c | 5 ++++-
> > drivers/pci/setup-irq.c | 12 +++++++-----
> > drivers/pci/setup-res.c | 2 +-
> > drivers/pci/slot.c | 5 ++++-
> > drivers/pci/syscall.c | 5 +++--
> > drivers/pci/xen-pcifront.c | 20 ++++++++++++--------
> > 17 files changed, 133 insertions(+), 58 deletions(-)