linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] FDDI: defxx: CSR access fixes and improvements
@ 2021-03-10 12:03 Maciej W. Rozycki
  2021-03-10 12:03 ` [PATCH net/net-next 1/4] FDDI: defxx: Bail out gracefully with unassigned PCI resource for CSR Maciej W. Rozycki
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Maciej W. Rozycki @ 2021-03-10 12:03 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel

Hi,

 As a lab upgrade I have recently replaced a dated 32-bit x86 server with 
a new POWER9 system.  One of the purposes of the system has been providing 
network based resources to clients over my FDDI network.  As such the new 
server has also received a new DEFPA FDDI network adapter.

 As it turned out the interface did not work with the driver as shipped by 
the most recent stable Debian release (Linux version 5.9.15) for ppc64el.  
Symptoms were inconclusive, and the DEFPA adapter turned out to have a 
manufacturing defect as well, however eventually I have figured out the 
PCIe host bridge used with the system, Power Systems Host Bridge 4 (PHB4), 
does not (anymore) implement PCI I/O transactions, while the binary defxx 
driver as shipped by Debian comes configured for port I/O, and then a bug 
in resource handling causes the driver to try and use an unassigned port 
I/O range for adapter's PDQ main ASIC's CSR access.

 Fortunately the PFI PCI interface ASIC used with the DEFPA adapter has 
been designed such as to provide for both PCI I/O and PCI memory accesses 
to be used for PDQ CSR access, via a pair of BARs to be alternatively 
used.

 Originally the defxx driver only supported port I/O access, but in the 
course of interfacing it to the TURBOchannel bus I had to implement MMIO 
access too, and while at it I have added a kernel configuration option to 
globally switch between port I/O and MMIO at compilation time, however 
conservatively defaulting to port I/O for EISA bus support where the use 
of MMIO currently requires the adapter to have been suitably configured 
via ECU (EISA Configuration Utility), supplied externally.

 With the kernel configuration option set to MMIO the DEFPA interface 
works correctly with my POWER9 system.  Therefore I have prepared this 
small patch series consisting of a pair of conservative bug fixes, to be 
backported to stable branches, and then a pair of improvements for the 
robustness of the driver.

 So changes 1/4 and 2/4 apply both to net and net-next, and then changes 
3/4 and 4/4 apply on top of them to net-next only.  In particular there 
are diff context dependencies going like this: 1/4 -> 3/4 -> 4/4.  Let me 
know if this submission needs to be sorted differently.

 See individual change descriptions for further details as to the actual 
changes made.

 NB the ESIC interface chip used for slave address decoding with the DEFEA 
EISA adapter has decoding implemented for address bits 31:10 and therefore 
supports full 32-bit range for the allocation of the CSR decoding window.  
For DOS compatibility reasons ECU however only allows allocations between 
0x000c0000 and 0x000effff.

 Given that for other compatibility reasons EISA is subtractively decoded 
on mixed PCI/EISA systems we could allocate an MMIO region from arbitrary 
unoccupied memory space and program the ESIC suitably without regard for 
that compatibility limitation.  In fact I have a proof-of-concept change 
and it seems to work reliably.

 However with these patches applied the driver continues supporting port 
I/O as fallback and the EISA product ID register is located in the EISA 
slot-specific port I/O address space, so any EISA system however modern 
(sounds like a joke, eh?) also has to support port I/O access somehow.

 So while I think such a dynamic MMIO allocation would be an example of 
good engineering, but it would require changes to our EISA core and 
therefore it may have had sense 25 years ago when EISA was still 
mainstream, but not nowadays when EISA systems are I suppose more of a 
curiosity rather than the usual equipment.

 This patch series has been thoroughly verified with Linux 5.11.0 as 
released and then a Raptor Talos II POWER9 system and a Malta 5Kc MIPS64 
system for PCI DEFPA adapter support, an Advanced Integrated Research 
486EI x86 system for EISA DEFEA adapter support, and a Digital Equipment 
DECstation 5000 model 260 MIPS III system for TURBOchannel DEFTA adapter 
support, covering both port I/O and MMIO operation where applicable.

 Please apply.

  Maciej

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net/net-next 1/4] FDDI: defxx: Bail out gracefully with unassigned PCI resource for CSR
  2021-03-10 12:03 [PATCH 0/4] FDDI: defxx: CSR access fixes and improvements Maciej W. Rozycki
@ 2021-03-10 12:03 ` Maciej W. Rozycki
  2021-03-10 12:03 ` [PATCH net/net-next 2/4] FDDI: defxx: Make MMIO the configuration default except for EISA Maciej W. Rozycki
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Maciej W. Rozycki @ 2021-03-10 12:03 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, stable

Recent versions of the PCI Express specification have deprecated support 
for I/O transactions and actually some PCIe host bridges, such as Power 
Systems Host Bridge 4 (PHB4), do not implement them.

For those systems the PCI BARs that request a mapping in the I/O space 
have the length recorded in the corresponding PCI resource set to zero, 
which makes it unassigned:

# lspci -s 0031:02:04.0 -v
0031:02:04.0 FDDI network controller: Digital Equipment Corporation PCI-to-PDQ Interface Chip [PFI] FDDI (DEFPA) (rev 02)
	Subsystem: Digital Equipment Corporation FDDIcontroller/PCI (DEFPA)
	Flags: bus master, medium devsel, latency 136, IRQ 57, NUMA node 8
	Memory at 620c080020000 (32-bit, non-prefetchable) [size=128]
	I/O ports at <unassigned> [disabled]
	Memory at 620c080030000 (32-bit, non-prefetchable) [size=64K]
	Capabilities: [50] Power Management version 2
	Kernel driver in use: defxx
	Kernel modules: defxx

# 

Regardless the driver goes ahead and requests it (here observed with a 
Raptor Talos II POWER9 system), resulting in an odd /proc/ioport entry:

# cat /proc/ioports
00000000-ffffffffffffffff : 0031:02:04.0
# 

Furthermore, the system gets confused as the driver actually continues 
and pokes at those locations, causing a flood of messages being output 
to the system console by the underlying system firmware, like:

defxx: v1.11 2014/07/01  Lawrence V. Stefani and others
defxx 0031:02:04.0: enabling device (0140 -> 0142)
LPC[000]: Got SYNC no-response error. Error address reg: 0xd0010000
IPMI: dropping non severe PEL event
LPC[000]: Got SYNC no-response error. Error address reg: 0xd0010014
IPMI: dropping non severe PEL event
LPC[000]: Got SYNC no-response error. Error address reg: 0xd0010014
IPMI: dropping non severe PEL event

and so on and so on (possibly intermixed actually, as there's no locking 
between the kernel and the firmware in console port access with this 
particular system, but cleaned up above for clarity), and once some 10k 
of such pairs of the latter two messages have been produced an interace 
eventually shows up in a useless state:

0031:02:04.0: DEFPA at I/O addr = 0x0, IRQ = 57, Hardware addr = 00-00-00-00-00-00

This was not expected to happen as resource handling was added to the 
driver a while ago, because it was not known at that time that a PCI 
system would be possible that cannot assign port I/O resources, and 
oddly enough `request_region' does not fail, which would have caught it.

Correct the problem then by checking for the length of zero for the CSR 
resource and bail out gracefully refusing to register an interface if 
that turns out to be the case, producing messages like:

defxx: v1.11 2014/07/01  Lawrence V. Stefani and others
0031:02:04.0: Cannot use I/O, no address set, aborting
0031:02:04.0: Recompile driver with "CONFIG_DEFXX_MMIO=y"

Keep the original check for the EISA MMIO resource as implemented, 
because in that case the length is hardwired to 0x400 as a consequence 
of how the compare/mask address decoding works in the ESIC chip and it 
is only the base address that is set to zero if MMIO has been disabled 
for the adapter in EISA configuration, which in turn could be a valid 
bus address in a legacy-free system implementing PCI, especially for 
port I/O.

Where the EISA MMIO resource has been disabled for the adapter in EISA 
configuration this arrangement keeps producing messages like:

eisa 00:05: EISA: slot 5: DEC3002 detected
defxx: v1.11 2014/07/01  Lawrence V. Stefani and others
00:05: Cannot use MMIO, no address set, aborting
00:05: Recompile driver with "CONFIG_DEFXX_MMIO=n"
00:05: Or run ECU and set adapter's MMIO location

with the last two lines now swapped for easier handling in the driver.

There is no need to check for and catch the case of a port I/O resource 
not having been assigned for EISA as the adapter uses the slot-specific 
I/O space, which gets assigned by how EISA has been specified and maps 
directly to the particular slot an option card has been placed in.  And 
the EISA variant of the adapter has additional registers that are only 
accessible via the port I/O space anyway.

While at it factor out the error message calls into helpers and fix an 
argument order bug with the `pr_err' call now in `dfx_register_res_err'.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Fixes: 4d0438e56a8f ("defxx: Clean up DEFEA resource management")
Cc: stable@vger.kernel.org # v3.19+
---
 drivers/net/fddi/defxx.c |   47 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 17 deletions(-)

Index: linux-defxx/drivers/net/fddi/defxx.c
===================================================================
--- linux-defxx.orig/drivers/net/fddi/defxx.c
+++ linux-defxx/drivers/net/fddi/defxx.c
@@ -495,6 +495,25 @@ static const struct net_device_ops dfx_n
 	.ndo_set_mac_address	= dfx_ctl_set_mac_address,
 };
 
+static void dfx_register_res_alloc_err(const char *print_name, bool mmio,
+				       bool eisa)
+{
+	pr_err("%s: Cannot use %s, no address set, aborting\n",
+	       print_name, mmio ? "MMIO" : "I/O");
+	pr_err("%s: Recompile driver with \"CONFIG_DEFXX_MMIO=%c\"\n",
+	       print_name, mmio ? 'n' : 'y');
+	if (eisa && mmio)
+		pr_err("%s: Or run ECU and set adapter's MMIO location\n",
+		       print_name);
+}
+
+static void dfx_register_res_err(const char *print_name, bool mmio,
+				 unsigned long start, unsigned long len)
+{
+	pr_err("%s: Cannot reserve %s resource 0x%lx @ 0x%lx, aborting\n",
+	       print_name, mmio ? "MMIO" : "I/O", len, start);
+}
+
 /*
  * ================
  * = dfx_register =
@@ -568,15 +587,12 @@ static int dfx_register(struct device *b
 	dev_set_drvdata(bdev, dev);
 
 	dfx_get_bars(bdev, bar_start, bar_len);
-	if (dfx_bus_eisa && dfx_use_mmio && bar_start[0] == 0) {
-		pr_err("%s: Cannot use MMIO, no address set, aborting\n",
-		       print_name);
-		pr_err("%s: Run ECU and set adapter's MMIO location\n",
-		       print_name);
-		pr_err("%s: Or recompile driver with \"CONFIG_DEFXX_MMIO=n\""
-		       "\n", print_name);
+	if (bar_len[0] == 0 ||
+	    (dfx_bus_eisa && dfx_use_mmio && bar_start[0] == 0)) {
+		dfx_register_res_alloc_err(print_name, dfx_use_mmio,
+					   dfx_bus_eisa);
 		err = -ENXIO;
-		goto err_out;
+		goto err_out_disable;
 	}
 
 	if (dfx_use_mmio)
@@ -585,18 +601,16 @@ static int dfx_register(struct device *b
 	else
 		region = request_region(bar_start[0], bar_len[0], print_name);
 	if (!region) {
-		pr_err("%s: Cannot reserve %s resource 0x%lx @ 0x%lx, "
-		       "aborting\n", dfx_use_mmio ? "MMIO" : "I/O", print_name,
-		       (long)bar_len[0], (long)bar_start[0]);
+		dfx_register_res_err(print_name, dfx_use_mmio,
+				     bar_start[0], bar_len[0]);
 		err = -EBUSY;
 		goto err_out_disable;
 	}
 	if (bar_start[1] != 0) {
 		region = request_region(bar_start[1], bar_len[1], print_name);
 		if (!region) {
-			pr_err("%s: Cannot reserve I/O resource "
-			       "0x%lx @ 0x%lx, aborting\n", print_name,
-			       (long)bar_len[1], (long)bar_start[1]);
+			dfx_register_res_err(print_name, 0,
+					     bar_start[1], bar_len[1]);
 			err = -EBUSY;
 			goto err_out_csr_region;
 		}
@@ -604,9 +618,8 @@ static int dfx_register(struct device *b
 	if (bar_start[2] != 0) {
 		region = request_region(bar_start[2], bar_len[2], print_name);
 		if (!region) {
-			pr_err("%s: Cannot reserve I/O resource "
-			       "0x%lx @ 0x%lx, aborting\n", print_name,
-			       (long)bar_len[2], (long)bar_start[2]);
+			dfx_register_res_err(print_name, 0,
+					     bar_start[2], bar_len[2]);
 			err = -EBUSY;
 			goto err_out_bh_region;
 		}

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net/net-next 2/4] FDDI: defxx: Make MMIO the configuration default except for EISA
  2021-03-10 12:03 [PATCH 0/4] FDDI: defxx: CSR access fixes and improvements Maciej W. Rozycki
  2021-03-10 12:03 ` [PATCH net/net-next 1/4] FDDI: defxx: Bail out gracefully with unassigned PCI resource for CSR Maciej W. Rozycki
@ 2021-03-10 12:03 ` Maciej W. Rozycki
  2021-03-10 12:03 ` [PATCH net-next 3/4] FDDI: defxx: Implement dynamic CSR I/O address space selection Maciej W. Rozycki
  2021-03-10 12:03 ` [PATCH net-next 4/4] FDDI: defxx: Use driver's name with resource requests Maciej W. Rozycki
  3 siblings, 0 replies; 5+ messages in thread
From: Maciej W. Rozycki @ 2021-03-10 12:03 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, stable

Recent versions of the PCI Express specification have deprecated support
for I/O transactions and actually some PCIe host bridges, such as Power
Systems Host Bridge 4 (PHB4), do not implement them.

The default kernel configuration choice for the defxx driver is the use 
of I/O ports rather than MMIO for PCI and EISA systems.  It may have 
made sense as a conservative backwards compatible choice back when MMIO 
operation support was added to the driver as a part of TURBOchannel bus 
support.  However nowadays this configuration choice makes the driver 
unusable with systems that do not implement I/O transactions for PCIe.

Make DEFXX_MMIO the configuration default then, except where configured 
for EISA.  This exception is because an EISA adapter can have its MMIO 
decoding disabled with ECU (EISA Configuration Utility) and therefore 
not available with the resource allocation infrastructure we implement, 
while port I/O is always readily available as it uses slot-specific 
addressing, directly mapped to the slot an option card has been placed 
in and handled with our EISA bus support core.  Conversely a kernel that 
supports modern systems which may not have I/O transactions implemented 
for PCIe will usually not be expected to handle legacy EISA systems.

The change of the default will make it easier for people, including but 
not limited to distribution packagers, to make a working choice for the 
driver.

Update the option description accordingly and while at it replace the 
potentially ambiguous PIO acronym with IOP for "port I/O" vs "I/O ports" 
according to our nomenclature used elsewhere.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Fixes: e89a2cfb7d7b ("[TC] defxx: TURBOchannel support")
Cc: stable@vger.kernel.org # v2.6.21+
---
 drivers/net/fddi/Kconfig |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Index: linux-defxx/drivers/net/fddi/Kconfig
===================================================================
--- linux-defxx.orig/drivers/net/fddi/Kconfig
+++ linux-defxx/drivers/net/fddi/Kconfig
@@ -40,17 +40,20 @@ config DEFXX
 
 config DEFXX_MMIO
 	bool
-	prompt "Use MMIO instead of PIO" if PCI || EISA
+	prompt "Use MMIO instead of IOP" if PCI || EISA
 	depends on DEFXX
-	default n if PCI || EISA
+	default n if EISA
 	default y
 	help
 	  This instructs the driver to use EISA or PCI memory-mapped I/O
-	  (MMIO) as appropriate instead of programmed I/O ports (PIO).
+	  (MMIO) as appropriate instead of programmed I/O ports (IOP).
 	  Enabling this gives an improvement in processing time in parts
-	  of the driver, but it may cause problems with EISA (DEFEA)
-	  adapters.  TURBOchannel does not have the concept of I/O ports,
-	  so MMIO is always used for these (DEFTA) adapters.
+	  of the driver, but it requires a memory window to be configured
+	  for EISA (DEFEA) adapters that may not always be available.
+	  Conversely some PCIe host bridges do not support IOP, so MMIO
+	  may be required to access PCI (DEFPA) adapters on downstream PCI
+	  buses with some systems.  TURBOchannel does not have the concept
+	  of I/O ports, so MMIO is always used for these (DEFTA) adapters.
 
 	  If unsure, say N.
 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net-next 3/4] FDDI: defxx: Implement dynamic CSR I/O address space selection
  2021-03-10 12:03 [PATCH 0/4] FDDI: defxx: CSR access fixes and improvements Maciej W. Rozycki
  2021-03-10 12:03 ` [PATCH net/net-next 1/4] FDDI: defxx: Bail out gracefully with unassigned PCI resource for CSR Maciej W. Rozycki
  2021-03-10 12:03 ` [PATCH net/net-next 2/4] FDDI: defxx: Make MMIO the configuration default except for EISA Maciej W. Rozycki
@ 2021-03-10 12:03 ` Maciej W. Rozycki
  2021-03-10 12:03 ` [PATCH net-next 4/4] FDDI: defxx: Use driver's name with resource requests Maciej W. Rozycki
  3 siblings, 0 replies; 5+ messages in thread
From: Maciej W. Rozycki @ 2021-03-10 12:03 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel

Recent versions of the PCI Express specification have deprecated support 
for I/O transactions and actually some PCIe host bridges, such as Power 
Systems Host Bridge 4 (PHB4), do not implement them.  Conversely a DEFEA 
adapter can have its MMIO decoding disabled with ECU (EISA Configuration 
Utility) and therefore not available for us with the resource allocation 
infrastructure we implement.

However either I/O address space will always be available for use with 
the DEFEA (EISA) and DEFPA (PCI) adapters and both have double address 
decoding implemented in hardware for Control and Status Register access.  
The two kinds of adapters can be present both at once in a single mixed 
PCI/EISA system.  For the DEFTA (TURBOchannel) variant there is no issue 
as there has been no port I/O address space defined for that bus.

To make people's life easier and the driver more robust remove the 
DEFXX_MMIO configuration option so as to rather than making the choice 
for the I/O address space to use at build time for all the adapters 
installed in the system let the driver choose the most suitable address 
space dynamically on a case-by-case basis at run time.  Make MMIO the 
default and resort to port I/O should the default fail for some reason.

This way multiple adapters installed in one system can use different I/O 
address spaces each, in particular in the presence of DEFEA adapters in 
a pure-EISA or a mixed EISA/PCI system (it is expected that DEFPA boards
will use MMIO in normal circumstances).

The choice of the I/O address space to use continues being reported by 
the driver on startup, e.g.:

eisa 00:05: EISA: slot 5: DEC3002 detected
defxx: v1.12 2021/03/10  Lawrence V. Stefani and others
00:05: DEFEA at I/O addr = 0x5000, IRQ = 10, Hardware addr = 00-00-f8-c8-b3-b6
00:05: registered as fddi0

and:

defxx: v1.12 2021/03/10  Lawrence V. Stefani and others
0031:02:04.0: DEFPA at MMIO addr = 0x620c080020000, IRQ = 57, Hardware addr = 00-60-6d-93-91-98
0031:02:04.0: registered as fddi0

and:

defxx: v1.12 2021/03/10  Lawrence V. Stefani and others
tc2: DEFTA at MMIO addr = 0x1f100000, IRQ = 21, Hardware addr = 08-00-2b-b0-8b-1e
tc2: registered as fddi0

so there is no need to add further information.

The change is supposed to cause a negligible performance hit as I/O 
accessors will now have code executed conditionally at run time.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
 drivers/net/fddi/Kconfig |   19 --------------
 drivers/net/fddi/defxx.c |   60 +++++++++++++++++------------------------------
 drivers/net/fddi/defxx.h |    3 ++
 3 files changed, 25 insertions(+), 57 deletions(-)

Index: linux-defxx/drivers/net/fddi/Kconfig
===================================================================
--- linux-defxx.orig/drivers/net/fddi/Kconfig
+++ linux-defxx/drivers/net/fddi/Kconfig
@@ -38,25 +38,6 @@ config DEFXX
 	  To compile this driver as a module, choose M here: the module
 	  will be called defxx.  If unsure, say N.
 
-config DEFXX_MMIO
-	bool
-	prompt "Use MMIO instead of IOP" if PCI || EISA
-	depends on DEFXX
-	default n if EISA
-	default y
-	help
-	  This instructs the driver to use EISA or PCI memory-mapped I/O
-	  (MMIO) as appropriate instead of programmed I/O ports (IOP).
-	  Enabling this gives an improvement in processing time in parts
-	  of the driver, but it requires a memory window to be configured
-	  for EISA (DEFEA) adapters that may not always be available.
-	  Conversely some PCIe host bridges do not support IOP, so MMIO
-	  may be required to access PCI (DEFPA) adapters on downstream PCI
-	  buses with some systems.  TURBOchannel does not have the concept
-	  of I/O ports, so MMIO is always used for these (DEFTA) adapters.
-
-	  If unsure, say N.
-
 config SKFP
 	tristate "SysKonnect FDDI PCI support"
 	depends on FDDI && PCI
Index: linux-defxx/drivers/net/fddi/defxx.c
===================================================================
--- linux-defxx.orig/drivers/net/fddi/defxx.c
+++ linux-defxx/drivers/net/fddi/defxx.c
@@ -197,6 +197,7 @@
  *		23 Oct 2006	macro		Big-endian host support.
  *		14 Dec 2006	macro		TURBOchannel support.
  *		01 Jul 2014	macro		Fixes for DMA on 64-bit hosts.
+ *		10 Mar 2021	macro		Dynamic MMIO vs port I/O.
  */
 
 /* Include files */
@@ -225,8 +226,8 @@
 
 /* Version information string should be updated prior to each new release!  */
 #define DRV_NAME "defxx"
-#define DRV_VERSION "v1.11"
-#define DRV_RELDATE "2014/07/01"
+#define DRV_VERSION "v1.12"
+#define DRV_RELDATE "2021/03/10"
 
 static const char version[] =
 	DRV_NAME ": " DRV_VERSION " " DRV_RELDATE
@@ -253,10 +254,10 @@ static const char version[] =
 #define DFX_BUS_TC(dev) 0
 #endif
 
-#ifdef CONFIG_DEFXX_MMIO
-#define DFX_MMIO 1
+#if defined(CONFIG_EISA) || defined(CONFIG_PCI)
+#define dfx_use_mmio bp->mmio
 #else
-#define DFX_MMIO 0
+#define dfx_use_mmio true
 #endif
 
 /* Define module-wide (static) routines */
@@ -374,8 +375,6 @@ static inline void dfx_outl(DFX_board_t
 static void dfx_port_write_long(DFX_board_t *bp, int offset, u32 data)
 {
 	struct device __maybe_unused *bdev = bp->bus_dev;
-	int dfx_bus_tc = DFX_BUS_TC(bdev);
-	int dfx_use_mmio = DFX_MMIO || dfx_bus_tc;
 
 	if (dfx_use_mmio)
 		dfx_writel(bp, offset, data);
@@ -398,8 +397,6 @@ static inline void dfx_inl(DFX_board_t *
 static void dfx_port_read_long(DFX_board_t *bp, int offset, u32 *data)
 {
 	struct device __maybe_unused *bdev = bp->bus_dev;
-	int dfx_bus_tc = DFX_BUS_TC(bdev);
-	int dfx_use_mmio = DFX_MMIO || dfx_bus_tc;
 
 	if (dfx_use_mmio)
 		dfx_readl(bp, offset, data);
@@ -421,7 +418,7 @@ static void dfx_port_read_long(DFX_board
  *   None
  *
  * Arguments:
- *   bdev	- pointer to device information
+ *   bp		- pointer to board information
  *   bar_start	- pointer to store the start addresses
  *   bar_len	- pointer to store the lengths of the areas
  *
@@ -431,13 +428,13 @@ static void dfx_port_read_long(DFX_board
  * Side Effects:
  *   None
  */
-static void dfx_get_bars(struct device *bdev,
+static void dfx_get_bars(DFX_board_t *bp,
 			 resource_size_t *bar_start, resource_size_t *bar_len)
 {
+	struct device *bdev = bp->bus_dev;
 	int dfx_bus_pci = dev_is_pci(bdev);
 	int dfx_bus_eisa = DFX_BUS_EISA(bdev);
 	int dfx_bus_tc = DFX_BUS_TC(bdev);
-	int dfx_use_mmio = DFX_MMIO || dfx_bus_tc;
 
 	if (dfx_bus_pci) {
 		int num = dfx_use_mmio ? 0 : 1;
@@ -495,18 +492,6 @@ static const struct net_device_ops dfx_n
 	.ndo_set_mac_address	= dfx_ctl_set_mac_address,
 };
 
-static void dfx_register_res_alloc_err(const char *print_name, bool mmio,
-				       bool eisa)
-{
-	pr_err("%s: Cannot use %s, no address set, aborting\n",
-	       print_name, mmio ? "MMIO" : "I/O");
-	pr_err("%s: Recompile driver with \"CONFIG_DEFXX_MMIO=%c\"\n",
-	       print_name, mmio ? 'n' : 'y');
-	if (eisa && mmio)
-		pr_err("%s: Or run ECU and set adapter's MMIO location\n",
-		       print_name);
-}
-
 static void dfx_register_res_err(const char *print_name, bool mmio,
 				 unsigned long start, unsigned long len)
 {
@@ -547,8 +532,6 @@ static int dfx_register(struct device *b
 	static int version_disp;
 	int dfx_bus_pci = dev_is_pci(bdev);
 	int dfx_bus_eisa = DFX_BUS_EISA(bdev);
-	int dfx_bus_tc = DFX_BUS_TC(bdev);
-	int dfx_use_mmio = DFX_MMIO || dfx_bus_tc;
 	const char *print_name = dev_name(bdev);
 	struct net_device *dev;
 	DFX_board_t	  *bp;			/* board pointer */
@@ -586,19 +569,24 @@ static int dfx_register(struct device *b
 	bp->bus_dev = bdev;
 	dev_set_drvdata(bdev, dev);
 
-	dfx_get_bars(bdev, bar_start, bar_len);
+	bp->mmio = true;
+
+	dfx_get_bars(bp, bar_start, bar_len);
 	if (bar_len[0] == 0 ||
 	    (dfx_bus_eisa && dfx_use_mmio && bar_start[0] == 0)) {
-		dfx_register_res_alloc_err(print_name, dfx_use_mmio,
-					   dfx_bus_eisa);
-		err = -ENXIO;
-		goto err_out_disable;
+		bp->mmio = false;
+		dfx_get_bars(bp, bar_start, bar_len);
 	}
 
-	if (dfx_use_mmio)
+	if (dfx_use_mmio) {
 		region = request_mem_region(bar_start[0], bar_len[0],
 					    print_name);
-	else
+		if (!region && (dfx_bus_eisa || dfx_bus_pci)) {
+			bp->mmio = false;
+			dfx_get_bars(bp, bar_start, bar_len);
+		}
+	}
+	if (!dfx_use_mmio)
 		region = request_region(bar_start[0], bar_len[0], print_name);
 	if (!region) {
 		dfx_register_res_err(print_name, dfx_use_mmio,
@@ -734,7 +722,6 @@ static void dfx_bus_init(struct net_devi
 	int dfx_bus_pci = dev_is_pci(bdev);
 	int dfx_bus_eisa = DFX_BUS_EISA(bdev);
 	int dfx_bus_tc = DFX_BUS_TC(bdev);
-	int dfx_use_mmio = DFX_MMIO || dfx_bus_tc;
 	u8 val;
 
 	DBG_printk("In dfx_bus_init...\n");
@@ -1054,7 +1041,6 @@ static int dfx_driver_init(struct net_de
 	int dfx_bus_pci = dev_is_pci(bdev);
 	int dfx_bus_eisa = DFX_BUS_EISA(bdev);
 	int dfx_bus_tc = DFX_BUS_TC(bdev);
-	int dfx_use_mmio = DFX_MMIO || dfx_bus_tc;
 	int alloc_size;			/* total buffer size needed */
 	char *top_v, *curr_v;		/* virtual addrs into memory block */
 	dma_addr_t top_p, curr_p;	/* physical addrs into memory block */
@@ -3708,8 +3694,6 @@ static void dfx_unregister(struct device
 	struct net_device *dev = dev_get_drvdata(bdev);
 	DFX_board_t *bp = netdev_priv(dev);
 	int dfx_bus_pci = dev_is_pci(bdev);
-	int dfx_bus_tc = DFX_BUS_TC(bdev);
-	int dfx_use_mmio = DFX_MMIO || dfx_bus_tc;
 	resource_size_t bar_start[3] = {0};	/* pointers to ports */
 	resource_size_t bar_len[3] = {0};	/* resource lengths */
 	int		alloc_size;		/* total buffer size used */
@@ -3729,7 +3713,7 @@ static void dfx_unregister(struct device
 
 	dfx_bus_uninit(dev);
 
-	dfx_get_bars(bdev, bar_start, bar_len);
+	dfx_get_bars(bp, bar_start, bar_len);
 	if (bar_start[2] != 0)
 		release_region(bar_start[2], bar_len[2]);
 	if (bar_start[1] != 0)
Index: linux-defxx/drivers/net/fddi/defxx.h
===================================================================
--- linux-defxx.orig/drivers/net/fddi/defxx.h
+++ linux-defxx/drivers/net/fddi/defxx.h
@@ -27,6 +27,7 @@
  *		04 Aug 2003	macro		Converted to the DMA API.
  *		23 Oct 2006	macro		Big-endian host support.
  *		14 Dec 2006	macro		TURBOchannel support.
+ *		10 Mar 2021	macro		Dynamic MMIO vs port I/O.
  */
 
 #ifndef _DEFXX_H_
@@ -1776,6 +1777,8 @@ typedef struct DFX_board_tag
 		int port;
 	} base;										/* base address */
 	struct device			*bus_dev;
+	/* Whether to use MMIO or port I/O.  */
+	bool				mmio;
 	u32				full_duplex_enb;				/* FDDI Full Duplex enable (1 == on, 2 == off) */
 	u32				req_ttrt;					/* requested TTRT value (in 80ns units) */
 	u32				burst_size;					/* adapter burst size (enumerated) */

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net-next 4/4] FDDI: defxx: Use driver's name with resource requests
  2021-03-10 12:03 [PATCH 0/4] FDDI: defxx: CSR access fixes and improvements Maciej W. Rozycki
                   ` (2 preceding siblings ...)
  2021-03-10 12:03 ` [PATCH net-next 3/4] FDDI: defxx: Implement dynamic CSR I/O address space selection Maciej W. Rozycki
@ 2021-03-10 12:03 ` Maciej W. Rozycki
  3 siblings, 0 replies; 5+ messages in thread
From: Maciej W. Rozycki @ 2021-03-10 12:03 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel

Replace repeated "defxx" strings with a reference to the DRV_NAME macro 
and then use the driver's name rather that the bus address with resource 
requests so as to have contents of /proc/iomem and /proc/ioports more 
meaningful to the user, in line with what drivers usually do.

So rather than say:

5000-50ff : DEC FDDIcontroller/EISA Adapter
  5000-503f : 00:05
  5040-5043 : 00:05
5400-54ff : DEC FDDIcontroller/EISA Adapter
5800-58ff : DEC FDDIcontroller/EISA Adapter
5c00-5cff : DEC FDDIcontroller/EISA Adapter
  5c80-5cbf : 00:05

or:

620c080020000-620c08002007f : 0031:02:04.0
  620c080020000-620c08002007f : 0031:02:04.0
620c080030000-620c08003ffff : 0031:02:04.0

or:

1f100000-1f10003f : tc2

we report:

5000-50ff : DEC FDDIcontroller/EISA Adapter
  5000-503f : defxx
  5040-5043 : defxx
5400-54ff : DEC FDDIcontroller/EISA Adapter
5800-58ff : DEC FDDIcontroller/EISA Adapter
5c00-5cff : DEC FDDIcontroller/EISA Adapter
  5c80-5cbf : defxx

and:

620c080020000-620c08002007f : 0031:02:04.0
  620c080020000-620c08002007f : defxx
620c080030000-620c08003ffff : 0031:02:04.0

and:

1f100000-1f10003f : defxx

respectively for the DEFEA (EISA), DEFPA (PCI), and DEFTA (TURBOchannel) 
adapters.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
 drivers/net/fddi/defxx.c |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Index: linux-defxx/drivers/net/fddi/defxx.c
===================================================================
--- linux-defxx.orig/drivers/net/fddi/defxx.c
+++ linux-defxx/drivers/net/fddi/defxx.c
@@ -580,14 +580,15 @@ static int dfx_register(struct device *b
 
 	if (dfx_use_mmio) {
 		region = request_mem_region(bar_start[0], bar_len[0],
-					    print_name);
+					    bdev->driver->name);
 		if (!region && (dfx_bus_eisa || dfx_bus_pci)) {
 			bp->mmio = false;
 			dfx_get_bars(bp, bar_start, bar_len);
 		}
 	}
 	if (!dfx_use_mmio)
-		region = request_region(bar_start[0], bar_len[0], print_name);
+		region = request_region(bar_start[0], bar_len[0],
+					bdev->driver->name);
 	if (!region) {
 		dfx_register_res_err(print_name, dfx_use_mmio,
 				     bar_start[0], bar_len[0]);
@@ -595,7 +596,8 @@ static int dfx_register(struct device *b
 		goto err_out_disable;
 	}
 	if (bar_start[1] != 0) {
-		region = request_region(bar_start[1], bar_len[1], print_name);
+		region = request_region(bar_start[1], bar_len[1],
+					bdev->driver->name);
 		if (!region) {
 			dfx_register_res_err(print_name, 0,
 					     bar_start[1], bar_len[1]);
@@ -604,7 +606,8 @@ static int dfx_register(struct device *b
 		}
 	}
 	if (bar_start[2] != 0) {
-		region = request_region(bar_start[2], bar_len[2], print_name);
+		region = request_region(bar_start[2], bar_len[2],
+					bdev->driver->name);
 		if (!region) {
 			dfx_register_res_err(print_name, 0,
 					     bar_start[2], bar_len[2]);
@@ -3745,7 +3748,7 @@ static const struct pci_device_id dfx_pc
 MODULE_DEVICE_TABLE(pci, dfx_pci_table);
 
 static struct pci_driver dfx_pci_driver = {
-	.name		= "defxx",
+	.name		= DRV_NAME,
 	.id_table	= dfx_pci_table,
 	.probe		= dfx_pci_register,
 	.remove		= dfx_pci_unregister,
@@ -3776,7 +3779,7 @@ MODULE_DEVICE_TABLE(eisa, dfx_eisa_table
 static struct eisa_driver dfx_eisa_driver = {
 	.id_table	= dfx_eisa_table,
 	.driver		= {
-		.name	= "defxx",
+		.name	= DRV_NAME,
 		.bus	= &eisa_bus_type,
 		.probe	= dfx_dev_register,
 		.remove	= dfx_dev_unregister,
@@ -3797,7 +3800,7 @@ MODULE_DEVICE_TABLE(tc, dfx_tc_table);
 static struct tc_driver dfx_tc_driver = {
 	.id_table	= dfx_tc_table,
 	.driver		= {
-		.name	= "defxx",
+		.name	= DRV_NAME,
 		.bus	= &tc_bus_type,
 		.probe	= dfx_dev_register,
 		.remove	= dfx_dev_unregister,

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-10 12:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10 12:03 [PATCH 0/4] FDDI: defxx: CSR access fixes and improvements Maciej W. Rozycki
2021-03-10 12:03 ` [PATCH net/net-next 1/4] FDDI: defxx: Bail out gracefully with unassigned PCI resource for CSR Maciej W. Rozycki
2021-03-10 12:03 ` [PATCH net/net-next 2/4] FDDI: defxx: Make MMIO the configuration default except for EISA Maciej W. Rozycki
2021-03-10 12:03 ` [PATCH net-next 3/4] FDDI: defxx: Implement dynamic CSR I/O address space selection Maciej W. Rozycki
2021-03-10 12:03 ` [PATCH net-next 4/4] FDDI: defxx: Use driver's name with resource requests Maciej W. Rozycki

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).