From: Puranjay Mohan <puranjay12@gmail.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>,
netdev <netdev@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [PATCH] net: fddi: skfp: remove generic PCI defines from skfbi.h
Date: Thu, 20 Jun 2019 09:47:46 +0530 [thread overview]
Message-ID: <20190620041744.GA6343@arch> (raw)
In-Reply-To: <CAErSpo7-AjCAc8pGpTftd7U-W2kjp1jfbPzk3SOa=Bg5-d6W5w@mail.gmail.com>
On Wed, Jun 19, 2019 at 02:10:22PM -0500, Bjorn Helgaas wrote:
> On Wed, Jun 19, 2019 at 12:48 PM Puranjay Mohan <puranjay12@gmail.com> wrote:
> >
> > skfbi.h defines its own copies of PCI_COMMAND, PCI_STATUS, etc.
> > remove them in favor of the generic definitions in
> > include/uapi/linux/pci_regs.h
>
> 1) Since you're sending several related patches, send them as a
> "series" with a cover letter, e.g.,
>
> [PATCH v2 0/2] Use PCI generic definitions instead of private duplicates
> [PATCH v2 1/2] Include generic PCI definitions
> [PATCH v2 2/2] Remove unused private PCI definitions
>
> Patches 1/2 and 2/2 should be replies to the 0/2 cover letter. "git
> send-email" will do this for you if you figure out the right options.
>
> 2) Make sure all your subject lines match. One started with "Include"
> and the other with "remove". They should both be capitalized.
>
> 3) Start sentences with a capital letter, i.e., "Remove them" above.
>
> 4) This commit log needs to explicitly say that you're removing
> *unused* symbols. Since they're unused, you don't even need to refer
> to pci_regs.h.
>
> 5) "git grep PCI_ drivers/net/fddi/skfp" says there are many more
> unused PCI symbols than just the ones below. I would just remove them
> all at once.
>
> 6) Obviously you should compile this to make sure it builds. It must
> build cleanly after every patch, not just at the end. I assume you've
> done this already.
>
Yes, I build the driver after every change and I do it again before
sending the patch to be sure that it works.
> 7) Please cc: linux-pci@vger.kernel.org since you're making PCI-related changes.
>
sure.
> > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> > ---
> > drivers/net/fddi/skfp/h/skfbi.h | 23 -----------------------
> > 1 file changed, 23 deletions(-)
> >
> > diff --git a/drivers/net/fddi/skfp/h/skfbi.h b/drivers/net/fddi/skfp/h/skfbi.h
> > index 89557457b352..ed144a8e78d1 100644
> > --- a/drivers/net/fddi/skfp/h/skfbi.h
> > +++ b/drivers/net/fddi/skfp/h/skfbi.h
> > @@ -27,29 +27,6 @@
> > /*
> > * Configuration Space header
> > */
> > -#define PCI_VENDOR_ID 0x00 /* 16 bit Vendor ID */
> > -#define PCI_DEVICE_ID 0x02 /* 16 bit Device ID */
> > -#define PCI_COMMAND 0x04 /* 16 bit Command */
> > -#define PCI_STATUS 0x06 /* 16 bit Status */
> > -#define PCI_REV_ID 0x08 /* 8 bit Revision ID */
> > -#define PCI_CLASS_CODE 0x09 /* 24 bit Class Code */
> > -#define PCI_CACHE_LSZ 0x0c /* 8 bit Cache Line Size */
> > -#define PCI_LAT_TIM 0x0d /* 8 bit Latency Timer */
> > -#define PCI_HEADER_T 0x0e /* 8 bit Header Type */
> > -#define PCI_BIST 0x0f /* 8 bit Built-in selftest */
> > -#define PCI_BASE_1ST 0x10 /* 32 bit 1st Base address */
> > -#define PCI_BASE_2ND 0x14 /* 32 bit 2nd Base address */
> > -/* Byte 18..2b: Reserved */
> > -#define PCI_SUB_VID 0x2c /* 16 bit Subsystem Vendor ID */
> > -#define PCI_SUB_ID 0x2e /* 16 bit Subsystem ID */
> > -#define PCI_BASE_ROM 0x30 /* 32 bit Expansion ROM Base Address */
> > -/* Byte 34..33: Reserved */
> > -#define PCI_CAP_PTR 0x34 /* 8 bit (ML) Capabilities Ptr */
> > -/* Byte 35..3b: Reserved */
> > -#define PCI_IRQ_LINE 0x3c /* 8 bit Interrupt Line */
> > -#define PCI_IRQ_PIN 0x3d /* 8 bit Interrupt Pin */
> > -#define PCI_MIN_GNT 0x3e /* 8 bit Min_Gnt */
> > -#define PCI_MAX_LAT 0x3f /* 8 bit Max_Lat */
> > /* Device Dependent Region */
> > #define PCI_OUR_REG 0x40 /* 32 bit (DV) Our Register */
> > #define PCI_OUR_REG_1 0x40 /* 32 bit (ML) Our Register 1 */
> > --
> > 2.21.0
> >
Thanks for the feedback!
I will send the patch series soon.
Thanks
--Puranjay
next prev parent reply other threads:[~2019-06-20 4:18 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-19 17:46 [PATCH] net: fddi: skfp: remove generic PCI defines from skfbi.h Puranjay Mohan
2019-06-19 19:10 ` Bjorn Helgaas
2019-06-20 4:17 ` Puranjay Mohan [this message]
2019-06-21 2:35 ` kbuild test robot
2019-06-21 3:42 ` Puranjay Mohan
2019-06-21 3:51 ` kbuild test robot
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=20190620041744.GA6343@arch \
--to=puranjay12@gmail.com \
--cc=bhelgaas@google.com \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=skhan@linuxfoundation.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).