Re: [PATCH] net: fddi: skfp: remove generic PCI defines from skfbi.h
From: Puranjay Mohan
Date: Thu Jun 20 2019 - 00:23:17 EST
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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx since you're making PCI-related changes.
> > Signed-off-by: Puranjay Mohan <puranjay12@xxxxxxxxx>
> > ---
> > 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.