Thread (3 messages) 3 messages, 2 authors, 2021-08-30

Re: [PATCH] PCI: Fix general code style

From: Sergio Miguéns Iglesias <hidden>
Date: 2021-08-30 15:52:48
Also in: linux-pci, lkml, xen-devel

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:
quoted
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
quoted
Signed-off-by: Sergio Miguéns Iglesias <redacted>
---
 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(-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help