Re: [PATCH] PCI: Fix general code style

[Sergio Miguéns Iglesias <lonyelon@xxxxxxxxx>]

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(-)