2019-06-21 09:47:08

by Puranjay Mohan

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

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


2019-06-21 09:47:16

by Puranjay Mohan

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

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 <[email protected]>
---
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

2019-06-21 09:47:28

by Puranjay Mohan

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

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

Signed-off-by: Puranjay Mohan <[email protected]>
---
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

2019-06-21 09:47:34

by Puranjay Mohan

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

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

Signed-off-by: Puranjay Mohan <[email protected]>
---
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

2019-06-21 14:05:31

by Bjorn Helgaas

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

[+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 <[email protected]>

2019-06-21 15:53:49

by Alan Cox

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

On Fri, 21 Jun 2019 15:16:04 +0530
Puranjay Mohan <[email protected]> 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

2019-06-21 16:37:40

by Shuah Khan

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

On 6/21/19 9:20 AM, Alan Cox wrote:
> On Fri, 21 Jun 2019 15:16:04 +0530
> Puranjay Mohan <[email protected]> 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

2019-06-21 16:44:53

by Bjorn Helgaas

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

On Fri, Jun 21, 2019 at 04:20:24PM +0100, Alan Cox wrote:
> On Fri, 21 Jun 2019 15:16:04 +0530
> Puranjay Mohan <[email protected]> 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

2019-06-21 17:59:53

by Joe Perches

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

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 <[email protected]> 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.


2019-06-26 20:04:42

by David Miller

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

From: Shuah Khan <[email protected]>
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.