From: "Sergio Miguéns Iglesias" <lonyelon@gmail.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: bhelgaas@google.com, rjw@rjwysocki.net, lenb@kernel.org,
boris.ostrovsky@oracle.com, jgross@suse.com,
sstabellini@kernel.org, konrad.wilk@oracle.com,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-acpi@vger.kernel.org, xen-devel@lists.xenproject.org,
"Sergio Miguéns Iglesias" <sergio@lony.xyz>
Subject: Re: [PATCH] PCI: Fix general code style
Date: Mon, 30 Aug 2021 19:52:36 +0200 [thread overview]
Message-ID: <20210830175236.7d62its7nwqosuwk@archlap> (raw)
In-Reply-To: <20210825204356.GA3601025@bjorn-Precision-5520>
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@lony.xyz>
> > ---
> > 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(-)
prev parent reply other threads:[~2021-08-30 15:52 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-04 22:28 [PATCH] PCI: Fix general code style Sergio Miguéns Iglesias
2021-08-25 20:43 ` Bjorn Helgaas
2021-08-30 17:52 ` Sergio Miguéns Iglesias [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210830175236.7d62its7nwqosuwk@archlap \
--to=lonyelon@gmail.com \
--cc=bhelgaas@google.com \
--cc=boris.ostrovsky@oracle.com \
--cc=helgaas@kernel.org \
--cc=jgross@suse.com \
--cc=konrad.wilk@oracle.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=rjw@rjwysocki.net \
--cc=sergio@lony.xyz \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).