[PATCH v3 0/3] net: fddi: skfp: Use PCI generic definitions instead of private duplicates

[PATCH v3 0/3] net: fddi: skfp: Use PCI generic definitions instead of private duplicates

[Puranjay Mohan]

This patch series removes the private duplicates of PCI definitions in
favour of generic definitions defined in pci_regs.h.

This driver only uses some of the generic PCI definitons,
which are included from pci_regs.h and thier private versions
are removed from skfbi.h with all other private defines.

The skfbi.h defines PCI_REV_ID and other private defines with different
names, these are renamed to Generic PCI names to make them
compatible with defines in pci_regs.h.

All unused defines are removed from skfbi.h.

Puranjay Mohan (3):
  net: fddi: skfp: Rename local PCI defines to match generic PCI defines
  net: fddi: skfp: Include generic PCI definitions
  net: fddi: skfp: Remove unused private PCI definitions

 drivers/net/fddi/skfp/drvfbi.c  |  3 +-
 drivers/net/fddi/skfp/h/skfbi.h | 80 +--------------------------------
 2 files changed, 4 insertions(+), 79 deletions(-)

-- 
2.21.0

[PATCH v3 1/3] net: fddi: skfp: Rename local PCI defines to match generic PCI defines

[Puranjay Mohan]

Rename the PCI_REV_ID and other local defines to Generic PCI define names
in skfbi.h and drvfbi.c to make it compatible with the pci_regs.h.

Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
 drivers/net/fddi/skfp/drvfbi.c  | 2 +-
 drivers/net/fddi/skfp/h/skfbi.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/fddi/skfp/drvfbi.c b/drivers/net/fddi/skfp/drvfbi.c
index bdd5700e71fa..b324c1acf195 100644
--- a/drivers/net/fddi/skfp/drvfbi.c
+++ b/drivers/net/fddi/skfp/drvfbi.c
@@ -127,7 +127,7 @@ static void card_start(struct s_smc *smc)
 	 *	 at very first before any other initialization functions is
 	 *	 executed.
 	 */
-	rev_id = inp(PCI_C(PCI_REV_ID)) ;
+	rev_id = inp(PCI_C(PCI_REVISION_ID)) ;
 	if ((rev_id & 0xf0) == SK_ML_ID_1 || (rev_id & 0xf0) == SK_ML_ID_2) {
 		smc->hw.hw_is_64bit = TRUE ;
 	} else {
diff --git a/drivers/net/fddi/skfp/h/skfbi.h b/drivers/net/fddi/skfp/h/skfbi.h
index 89557457b352..5f9b631e7515 100644
--- a/drivers/net/fddi/skfp/h/skfbi.h
+++ b/drivers/net/fddi/skfp/h/skfbi.h
@@ -31,7 +31,7 @@
 #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_REVISION_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 */
@@ -110,7 +110,7 @@
 #define PCI_66MHZCAP	0x0020	/* Bit 5:	66 MHz PCI bus clock capable */
 #define PCI_NEWCAP	0x0010	/* Bit 4:	New cap. list implemented */
 
-#define PCI_ERRBITS	(PCI_PERR|PCI_SERR|PCI_RMABORT|PCI_STABORT|PCI_DATAPERR)
+#define PCI_ERRBITS	(PCI_STATUS_DETECTED_PARITY | PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_SIG_TARGET_ABORT | PCI_STATUS_PARITY)
 
 /*	PCI_REV_ID	8 bit	Revision ID */
 /*	PCI_CLASS_CODE	24 bit	Class Code */
-- 
2.21.0

[PATCH v3 2/3] net: fddi: skfp: Include generic PCI definitions

[Puranjay Mohan]

Include the uapi/linux/pci_regs.h header file which contains the generic
PCI defines.

Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
 drivers/net/fddi/skfp/drvfbi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/fddi/skfp/drvfbi.c b/drivers/net/fddi/skfp/drvfbi.c
index b324c1acf195..9c8aa3a95463 100644
--- a/drivers/net/fddi/skfp/drvfbi.c
+++ b/drivers/net/fddi/skfp/drvfbi.c
@@ -20,6 +20,7 @@
 #include "h/supern_2.h"
 #include "h/skfbiinc.h"
 #include <linux/bitrev.h>
+#include <linux/pci_regs.h>
 
 #ifndef	lint
 static const char ID_sccs[] = "@(#)drvfbi.c	1.63 99/02/11 (C) SK " ;
-- 
2.21.0

[PATCH v3 3/3] net: fddi: skfp: Remove unused private PCI definitions

[Puranjay Mohan]

Remove unused private PCI definitions from skfbi.h because generic PCI
symbols are already included from pci_regs.h.

Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
 drivers/net/fddi/skfp/h/skfbi.h | 76 ---------------------------------
 1 file changed, 76 deletions(-)

diff --git a/drivers/net/fddi/skfp/h/skfbi.h b/drivers/net/fddi/skfp/h/skfbi.h
index 5f9b631e7515..0b5bd2e170a7 100644
--- a/drivers/net/fddi/skfp/h/skfbi.h
+++ b/drivers/net/fddi/skfp/h/skfbi.h
@@ -24,49 +24,6 @@
  *	(ML)	= only defined for Monalisa
  */
 
-/*
- * 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_REVISION_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 */
-#define	PCI_OUR_REG_2	0x44	/* 32 bit (ML)	Our Register 2 */
-/* Power Management Region */
-#define PCI_PM_CAP_ID	0x48	/*  8 bit (ML)	Power Management Cap. ID */
-#define PCI_PM_NITEM	0x49	/*  8 bit (ML)	Next Item Ptr */
-#define PCI_PM_CAP_REG	0x4a	/* 16 bit (ML)	Power Management Capabilities */
-#define PCI_PM_CTL_STS	0x4c	/* 16 bit (ML)	Power Manag. Control/Status */
-/* Byte 0x4e:	Reserved */
-#define PCI_PM_DAT_REG	0x4f	/*  8 bit (ML)	Power Manag. Data Register */
-/* VPD Region */
-#define	PCI_VPD_CAP_ID	0x50	/*  8 bit (ML)	VPD Cap. ID */
-#define PCI_VPD_NITEM	0x51	/*  8 bit (ML)	Next Item Ptr */
-#define PCI_VPD_ADR_REG	0x52	/* 16 bit (ML)	VPD Address Register */
-#define PCI_VPD_DAT_REG	0x54	/* 32 bit (ML)	VPD Data Register */
-/* Byte 58..ff:	Reserved */
 
 /*
  * I2C Address (PCI Config)
@@ -76,39 +33,6 @@
  */
 #define I2C_ADDR_VPD	0xA0	/* I2C address for the VPD EEPROM */ 
 
-/*
- * Define Bits and Values of the registers
- */
-/*	PCI_VENDOR_ID	16 bit	Vendor ID */
-/*	PCI_DEVICE_ID	16 bit	Device ID */
-/* Values for Vendor ID and Device ID shall be patched into the code */
-/*	PCI_COMMAND	16 bit	Command */
-#define	PCI_FBTEN	0x0200	/* Bit 9:	Fast Back-To-Back enable */
-#define	PCI_SERREN	0x0100	/* Bit 8:	SERR enable */
-#define	PCI_ADSTEP	0x0080	/* Bit 7:	Address Stepping */
-#define	PCI_PERREN	0x0040	/* Bit 6:	Parity Report Response enable */
-#define	PCI_VGA_SNOOP	0x0020	/* Bit 5:	VGA palette snoop */
-#define	PCI_MWIEN	0x0010	/* Bit 4:	Memory write an inv cycl ena */
-#define	PCI_SCYCEN	0x0008	/* Bit 3:	Special Cycle enable */
-#define	PCI_BMEN	0x0004	/* Bit 2:	Bus Master enable */
-#define	PCI_MEMEN	0x0002	/* Bit 1:	Memory Space Access enable */
-#define	PCI_IOEN	0x0001	/* Bit 0:	IO Space Access enable */
-
-/*	PCI_STATUS	16 bit	Status */
-#define	PCI_PERR	0x8000	/* Bit 15:	Parity Error */
-#define	PCI_SERR	0x4000	/* Bit 14:	Signaled SERR */
-#define	PCI_RMABORT	0x2000	/* Bit 13:	Received Master Abort */
-#define	PCI_RTABORT	0x1000	/* Bit 12:	Received Target Abort */
-#define	PCI_STABORT	0x0800	/* Bit 11:	Sent Target Abort */
-#define	PCI_DEVSEL	0x0600	/* Bit 10..9:	DEVSEL Timing */
-#define	PCI_DEV_FAST	(0<<9)	/*		fast */
-#define	PCI_DEV_MEDIUM	(1<<9)	/*		medium */
-#define	PCI_DEV_SLOW	(2<<9)	/*		slow */
-#define	PCI_DATAPERR	0x0100	/* Bit 8:	DATA Parity error detected */
-#define	PCI_FB2BCAP	0x0080	/* Bit 7:	Fast Back-to-Back Capability */
-#define	PCI_UDF		0x0040	/* Bit 6:	User Defined Features */
-#define PCI_66MHZCAP	0x0020	/* Bit 5:	66 MHz PCI bus clock capable */
-#define PCI_NEWCAP	0x0010	/* Bit 4:	New cap. list implemented */
 
 #define PCI_ERRBITS	(PCI_STATUS_DETECTED_PARITY | PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_SIG_TARGET_ABORT | PCI_STATUS_PARITY)
 
-- 
2.21.0

Re: [PATCH v3 0/3] net: fddi: skfp: Use PCI generic definitions instead of private duplicates

[Bjorn Helgaas]

[+cc Stephen]

On Fri, Jun 21, 2019 at 03:16:04PM +0530, Puranjay Mohan wrote:
> This patch series removes the private duplicates of PCI definitions in
> favour of generic definitions defined in pci_regs.h.
> 
> This driver only uses some of the generic PCI definitons,
> which are included from pci_regs.h and thier private versions
> are removed from skfbi.h with all other private defines.
> 
> The skfbi.h defines PCI_REV_ID and other private defines with different
> names, these are renamed to Generic PCI names to make them
> compatible with defines in pci_regs.h.
> 
> All unused defines are removed from skfbi.h.
> 
> Puranjay Mohan (3):
>   net: fddi: skfp: Rename local PCI defines to match generic PCI defines
>   net: fddi: skfp: Include generic PCI definitions
>   net: fddi: skfp: Remove unused private PCI definitions
> 
>  drivers/net/fddi/skfp/drvfbi.c  |  3 +-
>  drivers/net/fddi/skfp/h/skfbi.h | 80 +--------------------------------
>  2 files changed, 4 insertions(+), 79 deletions(-)

It's good form to CC people who have commented on previous versions of
your series, so I added Stephen.

FWIW,

Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>

Re: [PATCH v3 0/3] net: fddi: skfp: Use PCI generic definitions instead of private duplicates

[Alan Cox]

On Fri, 21 Jun 2019 15:16:04 +0530
Puranjay Mohan <puranjay12@gmail.com> wrote:

> This patch series removes the private duplicates of PCI definitions in
> favour of generic definitions defined in pci_regs.h.

Why bother ? It's an ancient obsolete card ?

Do you even have one to test ?

> 
> This driver only uses some of the generic PCI definitons,
> which are included from pci_regs.h and thier private versions
> are removed from skfbi.h with all other private defines.
> 
> The skfbi.h defines PCI_REV_ID and other private defines with different
> names, these are renamed to Generic PCI names to make them
> compatible with defines in pci_regs.h.
> 
> All unused defines are removed from skfbi.h.

I sincerely doubt anyone on the planet is using this card any more.

Alan

Re: [PATCH v3 0/3] net: fddi: skfp: Use PCI generic definitions instead of private duplicates

[Shuah Khan]

On 6/21/19 9:20 AM, Alan Cox wrote:
> On Fri, 21 Jun 2019 15:16:04 +0530
> Puranjay Mohan <puranjay12@gmail.com> wrote:
> 
>> This patch series removes the private duplicates of PCI definitions in
>> favour of generic definitions defined in pci_regs.h.
> 
> Why bother ? It's an ancient obsolete card ?
> 
> Do you even have one to test ?
> 
>>
>> This driver only uses some of the generic PCI definitons,
>> which are included from pci_regs.h and thier private versions
>> are removed from skfbi.h with all other private defines.
>>
>> The skfbi.h defines PCI_REV_ID and other private defines with different
>> names, these are renamed to Generic PCI names to make them
>> compatible with defines in pci_regs.h.
>>
>> All unused defines are removed from skfbi.h.
> 
> I sincerely doubt anyone on the planet is using this card any more.
> 
> Alan
> 

Thanks Alan!

Stephen Hemminger is suggesting removal as well. Makes sense to me.

David!

What would you recommend the next steps are? Would like driver removed?

thanks,
-- Shuah

Re: [PATCH v3 0/3] net: fddi: skfp: Use PCI generic definitions instead of private duplicates

[Bjorn Helgaas]

On Fri, Jun 21, 2019 at 04:20:24PM +0100, Alan Cox wrote:
> On Fri, 21 Jun 2019 15:16:04 +0530
> Puranjay Mohan <puranjay12@gmail.com> wrote:
> 
> > This patch series removes the private duplicates of PCI definitions in
> > favour of generic definitions defined in pci_regs.h.
> 
> Why bother ? It's an ancient obsolete card ?

That's a fair question.

Is there anything that would indicate that "this file is obsolete and
problems shouldn't be fixed"?  Nobody wants to waste time on things
that don't need to be fixed, but I don't know how to tell if something
is obsolete.

My naive assumption is that if something is in the tree, it's fair
game for fixes and cleanups.

Bjorn

Re: [PATCH v3 0/3] net: fddi: skfp: Use PCI generic definitions instead of private duplicates

[Joe Perches]

On Fri, 2019-06-21 at 11:44 -0500, Bjorn Helgaas wrote:
> On Fri, Jun 21, 2019 at 04:20:24PM +0100, Alan Cox wrote:
> > On Fri, 21 Jun 2019 15:16:04 +0530
> > Puranjay Mohan <puranjay12@gmail.com> wrote:
> > 
> > > This patch series removes the private duplicates of PCI definitions in
> > > favour of generic definitions defined in pci_regs.h.
> > 
> > Why bother ? It's an ancient obsolete card ?
> 
> That's a fair question.
> 
> Is there anything that would indicate that "this file is obsolete and
> problems shouldn't be fixed"?  Nobody wants to waste time on things
> that don't need to be fixed, but I don't know how to tell if something
> is obsolete.
> 
> My naive assumption is that if something is in the tree, it's fair
> game for fixes and cleanups.

I'd prefer to move the old, crufty, obsolete and generally
unsupported drivers to new directory trees and possibly
symlink those drivers to their current locations.

I suggested on the kernel summit list:
https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2019-June/006482.html

---

Perhaps a mechanism to move these old, generally unsupported
by an actual maintainer, and rarely tested drivers out of the
mainline drivers directory into a separate obsolete directory
would help isolate the whitespace and trivial api changes.

Re: [PATCH v3 0/3] net: fddi: skfp: Use PCI generic definitions instead of private duplicates

[David Miller]

From: Shuah Khan <skhan@linuxfoundation.org>
Date: Fri, 21 Jun 2019 10:36:02 -0600

> Stephen Hemminger is suggesting removal as well. Makes sense to me.
 ...
> What would you recommend the next steps are? Would like driver
> removed?

If you hadn't proposed the cleanups nobody would have said to remove
this driver.  Really if someone wants to go through the tree and
send removal patches for seemingly really unused drivers, that is
a separate piece of work unrelated to your cleanup.

While something still is in the tree we should clean it up from
stuff like this.

Therefore, I'll be applying v5 of your changes, thanks.