[PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Alexandre Bounine]

The following two patches are produced as result of the discussion
referenced below:

http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-September/085829.html
http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-October/086226.html

Switches in RapidIO subsystem are presented the same way as endpoints - by
using rio_dev structure plus an additional switch-specific extension allocated
separately. This separation between two objects describing a RIO switch device
also is reflected in the way how RIO address is stored for endpoints and
switches. Proposed patches are attempt to address issues brought by differences
in endpoint and switch handling in RapidIO subsystem.  

1. Using one storage location common for switches and endpoints eliminates
unnecessary device type checks during maintenance access operations.
While destination IDs and hop counts have different meaning for endpoints and
switches, this does not prevent us from storing them in the primary RIO device
structure (rio_dev) for both types.
The logic that assigns destination IDs to RIO devices stays unchanged - as
before, switches use an associated destination ID because they do not have
their own physical ID. The hop_count is set to 0xff for endpoints and to the
actual value for switches. 

2. Convert RIO switch device structures (rio_dev + rio_switch) into single
allocation unit. This change is based on the fact that RIO switches are using
common RIO device objects anyway. Allocating RIO switch objects as RIO devices
with added space for switch information simplifies handling of RIO switch device
objects.


Alexandre Bounine (2):
  RapidIO: Use common destid storage for endpoints and switches
  RapidIO: Integrate rio_switch into rio_dev

 drivers/rapidio/rio-scan.c          |  139 ++++++++++++++++++-----------------
 drivers/rapidio/rio-sysfs.c         |    4 +-
 drivers/rapidio/rio.c               |   74 ++++++-------------
 drivers/rapidio/switches/idt_gen2.c |   93 ++++++++---------------
 drivers/rapidio/switches/idtcps.c   |    6 +-
 drivers/rapidio/switches/tsi568.c   |   13 +--
 drivers/rapidio/switches/tsi57x.c   |   56 ++++++--------
 include/linux/rio.h                 |   90 +++++++++++------------
 include/linux/rio_drv.h             |   72 +++---------------
 9 files changed, 214 insertions(+), 333 deletions(-)

-- 
1.7.3.1

[PATCH -mm 1/2] RapidIO: Use common destid storage for endpoints and switches

[Alexandre Bounine]

Changes code to use one storage location common for switches and endpoints.
This eliminates unnecessary device type checks during basic access
operations. Logic that assigns destid to RIO devices stays unchanged - as
before, switches use an associated destid because they do not have their own.

Signed-off-by: Alexandre Bounine <alexandre.bounine@idt.com>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Li Yang <leoli@freescale.com>
Cc: Thomas Moll <thomas.moll@sysgo.com>
Cc: Micha Nelissen <micha@neli.hopto.org>
---
 drivers/rapidio/rio-scan.c          |   84 ++++++++++++++++---------------
 drivers/rapidio/rio.c               |   74 ++++++++-------------------
 drivers/rapidio/switches/idt_gen2.c |   93 ++++++++++++-----------------------
 drivers/rapidio/switches/idtcps.c   |    6 +--
 drivers/rapidio/switches/tsi568.c   |   13 ++---
 drivers/rapidio/switches/tsi57x.c   |   56 +++++++++------------
 include/linux/rio.h                 |    8 +--
 include/linux/rio_drv.h             |   72 +++++----------------------
 8 files changed, 141 insertions(+), 265 deletions(-)

diff --git a/drivers/rapidio/rio-scan.c b/drivers/rapidio/rio-scan.c
index 1eb82c4..51f0af2 100644
--- a/drivers/rapidio/rio-scan.c
+++ b/drivers/rapidio/rio-scan.c
@@ -437,9 +437,15 @@ static struct rio_dev __devinit *rio_setup_device(struct rio_net *net,
 				next_destid++;
 		} else
 			rdev->destid = rio_get_device_id(port, destid, hopcount);
-	} else
-		/* Switch device has an associated destID */
-		rdev->destid = RIO_INVALID_DESTID;
+
+		rdev->hopcount = 0xff;
+	} else {
+		/* Switch device has an associated destID which
+		 * will be adjusted later
+		 */
+		rdev->destid = destid;
+		rdev->hopcount = hopcount;
+	}
 
 	/* If a PE has both switch and other functions, show it as a switch */
 	if (rio_is_switch(rdev)) {
@@ -450,8 +456,6 @@ static struct rio_dev __devinit *rio_setup_device(struct rio_net *net,
 		if (!rswitch)
 			goto cleanup;
 		rswitch->switchid = next_switchid;
-		rswitch->hopcount = hopcount;
-		rswitch->destid = destid;
 		rswitch->port_ok = 0;
 		rswitch->route_table = kzalloc(sizeof(u8)*
 					RIO_MAX_ROUTE_ENTRIES(port->sys_size),
@@ -632,8 +636,7 @@ rio_unlock_device(struct rio_mport *port, u16 destid, u8 hopcount)
 
 /**
  * rio_route_add_entry- Add a route entry to a switch routing table
- * @mport: Master port to send transaction
- * @rswitch: Switch device
+ * @rdev: RIO device
  * @table: Routing table ID
  * @route_destid: Destination ID to be routed
  * @route_port: Port number to be routed
@@ -647,31 +650,31 @@ rio_unlock_device(struct rio_mport *port, u16 destid, u8 hopcount)
  * on failure.
  */
 static int
-rio_route_add_entry(struct rio_mport *mport, struct rio_switch *rswitch,
+rio_route_add_entry(struct rio_dev *rdev,
 		    u16 table, u16 route_destid, u8 route_port, int lock)
 {
 	int rc;
 
 	if (lock) {
-		rc = rio_lock_device(mport, rswitch->destid,
-				     rswitch->hopcount, 1000);
+		rc = rio_lock_device(rdev->net->hport, rdev->destid,
+				     rdev->hopcount, 1000);
 		if (rc)
 			return rc;
 	}
 
-	rc = rswitch->add_entry(mport, rswitch->destid,
-					rswitch->hopcount, table,
-					route_destid, route_port);
+	rc = rdev->rswitch->add_entry(rdev->net->hport, rdev->destid,
+				      rdev->hopcount, table,
+				      route_destid, route_port);
 	if (lock)
-		rio_unlock_device(mport, rswitch->destid, rswitch->hopcount);
+		rio_unlock_device(rdev->net->hport, rdev->destid,
+				  rdev->hopcount);
 
 	return rc;
 }
 
 /**
  * rio_route_get_entry- Read a route entry in a switch routing table
- * @mport: Master port to send transaction
- * @rswitch: Switch device
+ * @rdev: RIO device
  * @table: Routing table ID
  * @route_destid: Destination ID to be routed
  * @route_port: Pointer to read port number into
@@ -685,23 +688,24 @@ rio_route_add_entry(struct rio_mport *mport, struct rio_switch *rswitch,
  * on failure.
  */
 static int
-rio_route_get_entry(struct rio_mport *mport, struct rio_switch *rswitch, u16 table,
+rio_route_get_entry(struct rio_dev *rdev, u16 table,
 		    u16 route_destid, u8 *route_port, int lock)
 {
 	int rc;
 
 	if (lock) {
-		rc = rio_lock_device(mport, rswitch->destid,
-				     rswitch->hopcount, 1000);
+		rc = rio_lock_device(rdev->net->hport, rdev->destid,
+				     rdev->hopcount, 1000);
 		if (rc)
 			return rc;
 	}
 
-	rc = rswitch->get_entry(mport, rswitch->destid,
-					rswitch->hopcount, table,
-					route_destid, route_port);
+	rc = rdev->rswitch->get_entry(rdev->net->hport, rdev->destid,
+				      rdev->hopcount, table,
+				      route_destid, route_port);
 	if (lock)
-		rio_unlock_device(mport, rswitch->destid, rswitch->hopcount);
+		rio_unlock_device(rdev->net->hport, rdev->destid,
+				  rdev->hopcount);
 
 	return rc;
 }
@@ -811,14 +815,14 @@ static int __devinit rio_enum_peer(struct rio_net *net, struct rio_mport *port,
 	if (rio_is_switch(rdev)) {
 		next_switchid++;
 		sw_inport = RIO_GET_PORT_NUM(rdev->swpinfo);
-		rio_route_add_entry(port, rdev->rswitch, RIO_GLOBAL_TABLE,
+		rio_route_add_entry(rdev, RIO_GLOBAL_TABLE,
 				    port->host_deviceid, sw_inport, 0);
 		rdev->rswitch->route_table[port->host_deviceid] = sw_inport;
 
 		for (destid = 0; destid < next_destid; destid++) {
 			if (destid == port->host_deviceid)
 				continue;
-			rio_route_add_entry(port, rdev->rswitch, RIO_GLOBAL_TABLE,
+			rio_route_add_entry(rdev, RIO_GLOBAL_TABLE,
 					    destid, sw_inport, 0);
 			rdev->rswitch->route_table[destid] = sw_inport;
 		}
@@ -850,8 +854,7 @@ static int __devinit rio_enum_peer(struct rio_net *net, struct rio_mport *port,
 				    "RIO: scanning device on port %d\n",
 				    port_num);
 				rdev->rswitch->port_ok |= (1 << port_num);
-				rio_route_add_entry(port, rdev->rswitch,
-						RIO_GLOBAL_TABLE,
+				rio_route_add_entry(rdev, RIO_GLOBAL_TABLE,
 						RIO_ANY_DESTID(port->sys_size),
 						port_num, 0);
 
@@ -865,7 +868,7 @@ static int __devinit rio_enum_peer(struct rio_net *net, struct rio_mport *port,
 					     destid < next_destid; destid++) {
 						if (destid == port->host_deviceid)
 							continue;
-						rio_route_add_entry(port, rdev->rswitch,
+						rio_route_add_entry(rdev,
 								    RIO_GLOBAL_TABLE,
 								    destid,
 								    port_num,
@@ -904,7 +907,7 @@ static int __devinit rio_enum_peer(struct rio_net *net, struct rio_mport *port,
 				next_destid++;
 		}
 
-		rdev->rswitch->destid = sw_destid;
+		rdev->destid = sw_destid;
 	} else
 		pr_debug("RIO: found %s (vid %4.4x did %4.4x)\n",
 		    rio_name(rdev), rdev->vid, rdev->did);
@@ -958,7 +961,7 @@ rio_disc_peer(struct rio_net *net, struct rio_mport *port, u16 destid,
 		next_switchid++;
 
 		/* Associated destid is how we accessed this switch */
-		rdev->rswitch->destid = destid;
+		rdev->destid = destid;
 
 		pr_debug(
 		    "RIO: found %s (vid %4.4x did %4.4x) with %d ports\n",
@@ -981,7 +984,7 @@ rio_disc_peer(struct rio_net *net, struct rio_mport *port, u16 destid,
 				for (ndestid = 0;
 				     ndestid < RIO_ANY_DESTID(port->sys_size);
 				     ndestid++) {
-					rio_route_get_entry(port, rdev->rswitch,
+					rio_route_get_entry(rdev,
 							    RIO_GLOBAL_TABLE,
 							    ndestid,
 							    &route_port, 0);
@@ -1076,7 +1079,7 @@ static void rio_update_route_tables(struct rio_mport *port)
 
 	list_for_each_entry(rdev, &rio_devices, global_list) {
 
-		destid = (rio_is_switch(rdev))?rdev->rswitch->destid:rdev->destid;
+		destid = rdev->destid;
 
 		list_for_each_entry(rswitch, &rio_switches, node) {
 
@@ -1085,13 +1088,13 @@ static void rio_update_route_tables(struct rio_mport *port)
 
 			if (RIO_INVALID_ROUTE == rswitch->route_table[destid]) {
 				/* Skip if destid ends in empty switch*/
-				if (rswitch->destid == destid)
+				if (rswitch->rdev->destid == destid)
 					continue;
 
 				sport = RIO_GET_PORT_NUM(rswitch->rdev->swpinfo);
 
 				if (rswitch->add_entry)	{
-					rio_route_add_entry(port, rswitch,
+					rio_route_add_entry(rswitch->rdev,
 						RIO_GLOBAL_TABLE, destid,
 						sport, 0);
 					rswitch->route_table[destid] = sport;
@@ -1203,21 +1206,20 @@ static void rio_build_route_tables(void)
 
 	list_for_each_entry(rdev, &rio_devices, global_list)
 		if (rio_is_switch(rdev)) {
-			rio_lock_device(rdev->net->hport, rdev->rswitch->destid,
-					rdev->rswitch->hopcount, 1000);
+			rio_lock_device(rdev->net->hport, rdev->destid,
+					rdev->hopcount, 1000);
 			for (i = 0;
 			     i < RIO_MAX_ROUTE_ENTRIES(rdev->net->hport->sys_size);
 			     i++) {
-				if (rio_route_get_entry
-				    (rdev->net->hport, rdev->rswitch,
-				     RIO_GLOBAL_TABLE, i, &sport, 0) < 0)
+				if (rio_route_get_entry(rdev,
+					RIO_GLOBAL_TABLE, i, &sport, 0) < 0)
 					continue;
 				rdev->rswitch->route_table[i] = sport;
 			}
 
 			rio_unlock_device(rdev->net->hport,
-					  rdev->rswitch->destid,
-					  rdev->rswitch->hopcount);
+					  rdev->destid,
+					  rdev->hopcount);
 		}
 }
 
diff --git a/drivers/rapidio/rio.c b/drivers/rapidio/rio.c
index 68cf0c9..8ebd6c5 100644
--- a/drivers/rapidio/rio.c
+++ b/drivers/rapidio/rio.c
@@ -471,16 +471,9 @@ exit:
  */
 int rio_set_port_lockout(struct rio_dev *rdev, u32 pnum, int lock)
 {
-	u8 hopcount = 0xff;
-	u16 destid = rdev->destid;
 	u32 regval;
 
-	if (rdev->rswitch) {
-		destid = rdev->rswitch->destid;
-		hopcount = rdev->rswitch->hopcount;
-	}
-
-	rio_mport_read_config_32(rdev->net->hport, destid, hopcount,
+	rio_read_config_32(rdev,
 				 rdev->phys_efptr + RIO_PORT_N_CTL_CSR(pnum),
 				 &regval);
 	if (lock)
@@ -488,7 +481,7 @@ int rio_set_port_lockout(struct rio_dev *rdev, u32 pnum, int lock)
 	else
 		regval &= ~RIO_PORT_N_CTL_LOCKOUT;
 
-	rio_mport_write_config_32(rdev->net->hport, destid, hopcount,
+	rio_write_config_32(rdev,
 				  rdev->phys_efptr + RIO_PORT_N_CTL_CSR(pnum),
 				  regval);
 	return 0;
@@ -507,7 +500,7 @@ static int
 rio_chk_dev_route(struct rio_dev *rdev, struct rio_dev **nrdev, int *npnum)
 {
 	u32 result;
-	int p_port, dstid, rc = -EIO;
+	int p_port, rc = -EIO;
 	struct rio_dev *prev = NULL;
 
 	/* Find switch with failed RIO link */
@@ -522,9 +515,7 @@ rio_chk_dev_route(struct rio_dev *rdev, struct rio_dev **nrdev, int *npnum)
 	if (prev == NULL)
 		goto err_out;
 
-	dstid = (rdev->pef & RIO_PEF_SWITCH) ?
-			rdev->rswitch->destid : rdev->destid;
-	p_port = prev->rswitch->route_table[dstid];
+	p_port = prev->rswitch->route_table[rdev->destid];
 
 	if (p_port != RIO_INVALID_ROUTE) {
 		pr_debug("RIO: link failed on [%s]-P%d\n",
@@ -567,15 +558,8 @@ rio_mport_chk_dev_access(struct rio_mport *mport, u16 destid, u8 hopcount)
  */
 static int rio_chk_dev_access(struct rio_dev *rdev)
 {
-	u8 hopcount = 0xff;
-	u16 destid = rdev->destid;
-
-	if (rdev->rswitch) {
-		destid = rdev->rswitch->destid;
-		hopcount = rdev->rswitch->hopcount;
-	}
-
-	return rio_mport_chk_dev_access(rdev->net->hport, destid, hopcount);
+	return rio_mport_chk_dev_access(rdev->net->hport,
+					rdev->destid, rdev->hopcount);
 }
 
 /**
@@ -588,23 +572,20 @@ static int rio_chk_dev_access(struct rio_dev *rdev)
 static int
 rio_get_input_status(struct rio_dev *rdev, int pnum, u32 *lnkresp)
 {
-	struct rio_mport *mport = rdev->net->hport;
-	u16 destid = rdev->rswitch->destid;
-	u8 hopcount = rdev->rswitch->hopcount;
 	u32 regval;
 	int checkcount;
 
 	if (lnkresp) {
 		/* Read from link maintenance response register
 		 * to clear valid bit */
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 			rdev->phys_efptr + RIO_PORT_N_MNT_RSP_CSR(pnum),
 			&regval);
 		udelay(50);
 	}
 
 	/* Issue Input-status command */
-	rio_mport_write_config_32(mport, destid, hopcount,
+	rio_write_config_32(rdev,
 		rdev->phys_efptr + RIO_PORT_N_MNT_REQ_CSR(pnum),
 		RIO_MNT_REQ_CMD_IS);
 
@@ -615,7 +596,7 @@ rio_get_input_status(struct rio_dev *rdev, int pnum, u32 *lnkresp)
 	checkcount = 3;
 	while (checkcount--) {
 		udelay(50);
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 			rdev->phys_efptr + RIO_PORT_N_MNT_RSP_CSR(pnum),
 			&regval);
 		if (regval & RIO_PORT_N_MNT_RSP_RVAL) {
@@ -635,15 +616,12 @@ rio_get_input_status(struct rio_dev *rdev, int pnum, u32 *lnkresp)
  */
 static int rio_clr_err_stopped(struct rio_dev *rdev, u32 pnum, u32 err_status)
 {
-	struct rio_mport *mport = rdev->net->hport;
-	u16 destid = rdev->rswitch->destid;
-	u8 hopcount = rdev->rswitch->hopcount;
 	struct rio_dev *nextdev = rdev->rswitch->nextdev[pnum];
 	u32 regval;
 	u32 far_ackid, far_linkstat, near_ackid;
 
 	if (err_status == 0)
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 			rdev->phys_efptr + RIO_PORT_N_ERR_STS_CSR(pnum),
 			&err_status);
 
@@ -661,7 +639,7 @@ static int rio_clr_err_stopped(struct rio_dev *rdev, u32 pnum, u32 err_status)
 			 pnum, regval);
 		far_ackid = (regval & RIO_PORT_N_MNT_RSP_ASTAT) >> 5;
 		far_linkstat = regval & RIO_PORT_N_MNT_RSP_LSTAT;
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 			rdev->phys_efptr + RIO_PORT_N_ACK_STS_CSR(pnum),
 			&regval);
 		pr_debug("RIO_EM: SP%d_ACK_STS_CSR=0x%08x\n", pnum, regval);
@@ -679,9 +657,8 @@ static int rio_clr_err_stopped(struct rio_dev *rdev, u32 pnum, u32 err_status)
 			/* Align near outstanding/outbound ackIDs with
 			 * far inbound.
 			 */
-			rio_mport_write_config_32(mport, destid,
-				hopcount, rdev->phys_efptr +
-					RIO_PORT_N_ACK_STS_CSR(pnum),
+			rio_write_config_32(rdev,
+				rdev->phys_efptr + RIO_PORT_N_ACK_STS_CSR(pnum),
 				(near_ackid << 24) |
 					(far_ackid << 8) | far_ackid);
 			/* Align far outstanding/outbound ackIDs with
@@ -698,7 +675,7 @@ static int rio_clr_err_stopped(struct rio_dev *rdev, u32 pnum, u32 err_status)
 				pr_debug("RIO_EM: Invalid nextdev pointer (NULL)\n");
 		}
 rd_err:
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 			rdev->phys_efptr + RIO_PORT_N_ERR_STS_CSR(pnum),
 			&err_status);
 		pr_debug("RIO_EM: SP%d_ERR_STS_CSR=0x%08x\n", pnum, err_status);
@@ -710,7 +687,7 @@ rd_err:
 				     RIO_GET_PORT_NUM(nextdev->swpinfo), NULL);
 		udelay(50);
 
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 			rdev->phys_efptr + RIO_PORT_N_ERR_STS_CSR(pnum),
 			&err_status);
 		pr_debug("RIO_EM: SP%d_ERR_STS_CSR=0x%08x\n", pnum, err_status);
@@ -730,9 +707,6 @@ rd_err:
 int rio_inb_pwrite_handler(union rio_pw_msg *pw_msg)
 {
 	struct rio_dev *rdev;
-	struct rio_mport *mport;
-	u8 hopcount;
-	u16 destid;
 	u32 err_status, em_perrdet, em_ltlerrdet;
 	int rc, portnum;
 
@@ -800,17 +774,13 @@ int rio_inb_pwrite_handler(union rio_pw_msg *pw_msg)
 		return 0;
 	}
 
-	mport = rdev->net->hport;
-	destid = rdev->rswitch->destid;
-	hopcount = rdev->rswitch->hopcount;
-
 	/*
 	 * Process the port-write notification from switch
 	 */
 	if (rdev->rswitch->em_handle)
 		rdev->rswitch->em_handle(rdev, portnum);
 
-	rio_mport_read_config_32(mport, destid, hopcount,
+	rio_read_config_32(rdev,
 			rdev->phys_efptr + RIO_PORT_N_ERR_STS_CSR(portnum),
 			&err_status);
 	pr_debug("RIO_PW: SP%d_ERR_STS_CSR=0x%08x\n", portnum, err_status);
@@ -840,7 +810,7 @@ int rio_inb_pwrite_handler(union rio_pw_msg *pw_msg)
 			rdev->rswitch->port_ok &= ~(1 << portnum);
 			rio_set_port_lockout(rdev, portnum, 1);
 
-			rio_mport_write_config_32(mport, destid, hopcount,
+			rio_write_config_32(rdev,
 				rdev->phys_efptr +
 					RIO_PORT_N_ACK_STS_CSR(portnum),
 				RIO_PORT_N_ACK_CLEAR);
@@ -851,28 +821,28 @@ int rio_inb_pwrite_handler(union rio_pw_msg *pw_msg)
 		}
 	}
 
-	rio_mport_read_config_32(mport, destid, hopcount,
+	rio_read_config_32(rdev,
 		rdev->em_efptr + RIO_EM_PN_ERR_DETECT(portnum), &em_perrdet);
 	if (em_perrdet) {
 		pr_debug("RIO_PW: RIO_EM_P%d_ERR_DETECT=0x%08x\n",
 			 portnum, em_perrdet);
 		/* Clear EM Port N Error Detect CSR */
-		rio_mport_write_config_32(mport, destid, hopcount,
+		rio_write_config_32(rdev,
 			rdev->em_efptr + RIO_EM_PN_ERR_DETECT(portnum), 0);
 	}
 
-	rio_mport_read_config_32(mport, destid, hopcount,
+	rio_read_config_32(rdev,
 		rdev->em_efptr + RIO_EM_LTL_ERR_DETECT, &em_ltlerrdet);
 	if (em_ltlerrdet) {
 		pr_debug("RIO_PW: RIO_EM_LTL_ERR_DETECT=0x%08x\n",
 			 em_ltlerrdet);
 		/* Clear EM L/T Layer Error Detect CSR */
-		rio_mport_write_config_32(mport, destid, hopcount,
+		rio_write_config_32(rdev,
 			rdev->em_efptr + RIO_EM_LTL_ERR_DETECT, 0);
 	}
 
 	/* Clear remaining error bits and Port-Write Pending bit */
-	rio_mport_write_config_32(mport, destid, hopcount,
+	rio_write_config_32(rdev,
 			rdev->phys_efptr + RIO_PORT_N_ERR_STS_CSR(portnum),
 			err_status);
 
diff --git a/drivers/rapidio/switches/idt_gen2.c b/drivers/rapidio/switches/idt_gen2.c
index 0bb871c..dd4b2b7 100644
--- a/drivers/rapidio/switches/idt_gen2.c
+++ b/drivers/rapidio/switches/idt_gen2.c
@@ -209,9 +209,6 @@ idtg2_get_domain(struct rio_mport *mport, u16 destid, u8 hopcount,
 static int
 idtg2_em_init(struct rio_dev *rdev)
 {
-	struct rio_mport *mport = rdev->net->hport;
-	u16 destid = rdev->rswitch->destid;
-	u8 hopcount = rdev->rswitch->hopcount;
 	u32 regval;
 	int i, tmp;
 
@@ -220,29 +217,25 @@ idtg2_em_init(struct rio_dev *rdev)
 	 * All standard EM configuration should be performed at upper level.
 	 */
 
-	pr_debug("RIO: %s [%d:%d]\n", __func__, destid, hopcount);
+	pr_debug("RIO: %s [%d:%d]\n", __func__, rdev->destid, rdev->hopcount);
 
 	/* Set Port-Write info CSR: PRIO=3 and CRF=1 */
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_PW_INFO_CSR, 0x0000e000);
+	rio_write_config_32(rdev, IDT_PW_INFO_CSR, 0x0000e000);
 
 	/*
 	 * Configure LT LAYER error reporting.
 	 */
 
 	/* Enable standard (RIO.p8) error reporting */
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_LT_ERR_REPORT_EN,
+	rio_write_config_32(rdev, IDT_LT_ERR_REPORT_EN,
 			REM_LTL_ERR_ILLTRAN | REM_LTL_ERR_UNSOLR |
 			REM_LTL_ERR_UNSUPTR);
 
 	/* Use Port-Writes for LT layer error reporting.
 	 * Enable per-port reset
 	 */
-	rio_mport_read_config_32(mport, destid, hopcount,
-			IDT_DEV_CTRL_1, &regval);
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_DEV_CTRL_1,
+	rio_read_config_32(rdev, IDT_DEV_CTRL_1, &regval);
+	rio_write_config_32(rdev, IDT_DEV_CTRL_1,
 			regval | IDT_DEV_CTRL_1_GENPW | IDT_DEV_CTRL_1_PRSTBEH);
 
 	/*
@@ -250,45 +243,40 @@ idtg2_em_init(struct rio_dev *rdev)
 	 */
 
 	/* Report all RIO.p8 errors supported by device */
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_PORT_ERR_REPORT_EN_BC, 0x807e8037);
+	rio_write_config_32(rdev, IDT_PORT_ERR_REPORT_EN_BC, 0x807e8037);
 
 	/* Configure reporting of implementation specific errors/events */
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_PORT_ISERR_REPORT_EN_BC, IDT_PORT_INIT_TX_ACQUIRED);
+	rio_write_config_32(rdev, IDT_PORT_ISERR_REPORT_EN_BC,
+			    IDT_PORT_INIT_TX_ACQUIRED);
 
 	/* Use Port-Writes for port error reporting and enable error logging */
 	tmp = RIO_GET_TOTAL_PORTS(rdev->swpinfo);
 	for (i = 0; i < tmp; i++) {
-		rio_mport_read_config_32(mport, destid, hopcount,
-				IDT_PORT_OPS(i), &regval);
-		rio_mport_write_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev, IDT_PORT_OPS(i), &regval);
+		rio_write_config_32(rdev,
 				IDT_PORT_OPS(i), regval | IDT_PORT_OPS_GENPW |
 				IDT_PORT_OPS_PL_ELOG |
 				IDT_PORT_OPS_LL_ELOG |
 				IDT_PORT_OPS_LT_ELOG);
 	}
 	/* Overwrite error log if full */
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_ERR_CAP, IDT_ERR_CAP_LOG_OVERWR);
+	rio_write_config_32(rdev, IDT_ERR_CAP, IDT_ERR_CAP_LOG_OVERWR);
 
 	/*
 	 * Configure LANE error reporting.
 	 */
 
 	/* Disable line error reporting */
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_LANE_ERR_REPORT_EN_BC, 0);
+	rio_write_config_32(rdev, IDT_LANE_ERR_REPORT_EN_BC, 0);
 
 	/* Use Port-Writes for lane error reporting (when enabled)
 	 * (do per-lane update because lanes may have different configuration)
 	 */
 	tmp = (rdev->did == RIO_DID_IDTCPS1848) ? 48 : 16;
 	for (i = 0; i < tmp; i++) {
-		rio_mport_read_config_32(mport, destid, hopcount,
-				IDT_LANE_CTRL(i), &regval);
-		rio_mport_write_config_32(mport, destid, hopcount,
-				IDT_LANE_CTRL(i), regval | IDT_LANE_CTRL_GENPW);
+		rio_read_config_32(rdev, IDT_LANE_CTRL(i), &regval);
+		rio_write_config_32(rdev, IDT_LANE_CTRL(i),
+				    regval | IDT_LANE_CTRL_GENPW);
 	}
 
 	/*
@@ -296,41 +284,32 @@ idtg2_em_init(struct rio_dev *rdev)
 	 */
 
 	/* Disable JTAG and I2C Error capture */
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_AUX_PORT_ERR_CAP_EN, 0);
+	rio_write_config_32(rdev, IDT_AUX_PORT_ERR_CAP_EN, 0);
 
 	/* Disable JTAG and I2C Error reporting/logging */
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_AUX_ERR_REPORT_EN, 0);
+	rio_write_config_32(rdev, IDT_AUX_ERR_REPORT_EN, 0);
 
 	/* Disable Port-Write notification from JTAG */
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_JTAG_CTRL, 0);
+	rio_write_config_32(rdev, IDT_JTAG_CTRL, 0);
 
 	/* Disable Port-Write notification from I2C */
-	rio_mport_read_config_32(mport, destid, hopcount,
-			IDT_I2C_MCTRL, &regval);
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_I2C_MCTRL,
-			regval & ~IDT_I2C_MCTRL_GENPW);
+	rio_read_config_32(rdev, IDT_I2C_MCTRL, &regval);
+	rio_write_config_32(rdev, IDT_I2C_MCTRL, regval & ~IDT_I2C_MCTRL_GENPW);
 
 	/*
 	 * Configure CFG_BLK error reporting.
 	 */
 
 	/* Disable Configuration Block error capture */
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_CFGBLK_ERR_CAPTURE_EN, 0);
+	rio_write_config_32(rdev, IDT_CFGBLK_ERR_CAPTURE_EN, 0);
 
 	/* Disable Port-Writes for Configuration Block error reporting */
-	rio_mport_read_config_32(mport, destid, hopcount,
-			IDT_CFGBLK_ERR_REPORT, &regval);
-	rio_mport_write_config_32(mport, destid, hopcount,
-			IDT_CFGBLK_ERR_REPORT,
-			regval & ~IDT_CFGBLK_ERR_REPORT_GENPW);
+	rio_read_config_32(rdev, IDT_CFGBLK_ERR_REPORT, &regval);
+	rio_write_config_32(rdev, IDT_CFGBLK_ERR_REPORT,
+			    regval & ~IDT_CFGBLK_ERR_REPORT_GENPW);
 
 	/* set TVAL = ~50us */
-	rio_mport_write_config_32(mport, destid, hopcount,
+	rio_write_config_32(rdev,
 		rdev->phys_efptr + RIO_PORT_LINKTO_CTL_CSR, 0x8e << 8);
 
 	return 0;
@@ -339,18 +318,15 @@ idtg2_em_init(struct rio_dev *rdev)
 static int
 idtg2_em_handler(struct rio_dev *rdev, u8 portnum)
 {
-	struct rio_mport *mport = rdev->net->hport;
-	u16 destid = rdev->rswitch->destid;
-	u8 hopcount = rdev->rswitch->hopcount;
 	u32 regval, em_perrdet, em_ltlerrdet;
 
-	rio_mport_read_config_32(mport, destid, hopcount,
+	rio_read_config_32(rdev,
 		rdev->em_efptr + RIO_EM_LTL_ERR_DETECT, &em_ltlerrdet);
 	if (em_ltlerrdet) {
 		/* Service Logical/Transport Layer Error(s) */
 		if (em_ltlerrdet & REM_LTL_ERR_IMPSPEC) {
 			/* Implementation specific error reported */
-			rio_mport_read_config_32(mport, destid, hopcount,
+			rio_read_config_32(rdev,
 					IDT_ISLTL_ADDRESS_CAP, &regval);
 
 			pr_debug("RIO: %s Implementation Specific LTL errors" \
@@ -358,13 +334,12 @@ idtg2_em_handler(struct rio_dev *rdev, u8 portnum)
 				 rio_name(rdev), em_ltlerrdet, regval);
 
 			/* Clear implementation specific address capture CSR */
-			rio_mport_write_config_32(mport, destid, hopcount,
-					IDT_ISLTL_ADDRESS_CAP, 0);
+			rio_write_config_32(rdev, IDT_ISLTL_ADDRESS_CAP, 0);
 
 		}
 	}
 
-	rio_mport_read_config_32(mport, destid, hopcount,
+	rio_read_config_32(rdev,
 		rdev->em_efptr + RIO_EM_PN_ERR_DETECT(portnum), &em_perrdet);
 	if (em_perrdet) {
 		/* Service Port-Level Error(s) */
@@ -372,14 +347,14 @@ idtg2_em_handler(struct rio_dev *rdev, u8 portnum)
 			/* Implementation Specific port error reported */
 
 			/* Get IS errors reported */
-			rio_mport_read_config_32(mport, destid, hopcount,
+			rio_read_config_32(rdev,
 					IDT_PORT_ISERR_DET(portnum), &regval);
 
 			pr_debug("RIO: %s Implementation Specific Port" \
 				 " errors 0x%x\n", rio_name(rdev), regval);
 
 			/* Clear all implementation specific events */
-			rio_mport_write_config_32(mport, destid, hopcount,
+			rio_write_config_32(rdev,
 					IDT_PORT_ISERR_DET(portnum), 0);
 		}
 	}
@@ -391,14 +366,10 @@ static ssize_t
 idtg2_show_errlog(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct rio_dev *rdev = to_rio_dev(dev);
-	struct rio_mport *mport = rdev->net->hport;
-	u16 destid = rdev->rswitch->destid;
-	u8 hopcount = rdev->rswitch->hopcount;
 	ssize_t len = 0;
 	u32 regval;
 
-	while (!rio_mport_read_config_32(mport, destid, hopcount,
-					 IDT_ERR_RD, &regval)) {
+	while (!rio_read_config_32(rdev, IDT_ERR_RD, &regval)) {
 		if (!regval)    /* 0 = end of log */
 			break;
 		len += snprintf(buf + len, PAGE_SIZE - len,
diff --git a/drivers/rapidio/switches/idtcps.c b/drivers/rapidio/switches/idtcps.c
index fc9f637..3a97107 100644
--- a/drivers/rapidio/switches/idtcps.c
+++ b/drivers/rapidio/switches/idtcps.c
@@ -117,10 +117,6 @@ idtcps_get_domain(struct rio_mport *mport, u16 destid, u8 hopcount,
 
 static int idtcps_switch_init(struct rio_dev *rdev, int do_enum)
 {
-	struct rio_mport *mport = rdev->net->hport;
-	u16 destid = rdev->rswitch->destid;
-	u8 hopcount = rdev->rswitch->hopcount;
-
 	pr_debug("RIO: %s for %s\n", __func__, rio_name(rdev));
 	rdev->rswitch->add_entry = idtcps_route_add_entry;
 	rdev->rswitch->get_entry = idtcps_route_get_entry;
@@ -132,7 +128,7 @@ static int idtcps_switch_init(struct rio_dev *rdev, int do_enum)
 
 	if (do_enum) {
 		/* set TVAL = ~50us */
-		rio_mport_write_config_32(mport, destid, hopcount,
+		rio_write_config_32(rdev,
 			rdev->phys_efptr + RIO_PORT_LINKTO_CTL_CSR, 0x8e << 8);
 	}
 
diff --git a/drivers/rapidio/switches/tsi568.c b/drivers/rapidio/switches/tsi568.c
index b9a389b..3994c00 100644
--- a/drivers/rapidio/switches/tsi568.c
+++ b/drivers/rapidio/switches/tsi568.c
@@ -113,22 +113,17 @@ tsi568_route_clr_table(struct rio_mport *mport, u16 destid, u8 hopcount,
 static int
 tsi568_em_init(struct rio_dev *rdev)
 {
-	struct rio_mport *mport = rdev->net->hport;
-	u16 destid = rdev->rswitch->destid;
-	u8 hopcount = rdev->rswitch->hopcount;
 	u32 regval;
 	int portnum;
 
-	pr_debug("TSI568 %s [%d:%d]\n", __func__, destid, hopcount);
+	pr_debug("TSI568 %s [%d:%d]\n", __func__, rdev->destid, rdev->hopcount);
 
 	/* Make sure that Port-Writes are disabled (for all ports) */
 	for (portnum = 0;
 	     portnum < RIO_GET_TOTAL_PORTS(rdev->swpinfo); portnum++) {
-		rio_mport_read_config_32(mport, destid, hopcount,
-				TSI568_SP_MODE(portnum), &regval);
-		rio_mport_write_config_32(mport, destid, hopcount,
-				TSI568_SP_MODE(portnum),
-				regval | TSI568_SP_MODE_PW_DIS);
+		rio_read_config_32(rdev, TSI568_SP_MODE(portnum), &regval);
+		rio_write_config_32(rdev, TSI568_SP_MODE(portnum),
+				    regval | TSI568_SP_MODE_PW_DIS);
 	}
 
 	return 0;
diff --git a/drivers/rapidio/switches/tsi57x.c b/drivers/rapidio/switches/tsi57x.c
index 2003fb6..1a62934 100644
--- a/drivers/rapidio/switches/tsi57x.c
+++ b/drivers/rapidio/switches/tsi57x.c
@@ -158,48 +158,45 @@ tsi57x_get_domain(struct rio_mport *mport, u16 destid, u8 hopcount,
 static int
 tsi57x_em_init(struct rio_dev *rdev)
 {
-	struct rio_mport *mport = rdev->net->hport;
-	u16 destid = rdev->rswitch->destid;
-	u8 hopcount = rdev->rswitch->hopcount;
 	u32 regval;
 	int portnum;
 
-	pr_debug("TSI578 %s [%d:%d]\n", __func__, destid, hopcount);
+	pr_debug("TSI578 %s [%d:%d]\n", __func__, rdev->destid, rdev->hopcount);
 
 	for (portnum = 0;
 	     portnum < RIO_GET_TOTAL_PORTS(rdev->swpinfo); portnum++) {
 		/* Make sure that Port-Writes are enabled (for all ports) */
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 				TSI578_SP_MODE(portnum), &regval);
-		rio_mport_write_config_32(mport, destid, hopcount,
+		rio_write_config_32(rdev,
 				TSI578_SP_MODE(portnum),
 				regval & ~TSI578_SP_MODE_PW_DIS);
 
 		/* Clear all pending interrupts */
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 				rdev->phys_efptr +
 					RIO_PORT_N_ERR_STS_CSR(portnum),
 				&regval);
-		rio_mport_write_config_32(mport, destid, hopcount,
+		rio_write_config_32(rdev,
 				rdev->phys_efptr +
 					RIO_PORT_N_ERR_STS_CSR(portnum),
 				regval & 0x07120214);
 
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 				TSI578_SP_INT_STATUS(portnum), &regval);
-		rio_mport_write_config_32(mport, destid, hopcount,
+		rio_write_config_32(rdev,
 				TSI578_SP_INT_STATUS(portnum),
 				regval & 0x000700bd);
 
 		/* Enable all interrupts to allow ports to send a port-write */
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 				TSI578_SP_CTL_INDEP(portnum), &regval);
-		rio_mport_write_config_32(mport, destid, hopcount,
+		rio_write_config_32(rdev,
 				TSI578_SP_CTL_INDEP(portnum),
 				regval | 0x000b0000);
 
 		/* Skip next (odd) port if the current port is in x4 mode */
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 				rdev->phys_efptr + RIO_PORT_N_CTL_CSR(portnum),
 				&regval);
 		if ((regval & RIO_PORT_N_CTL_PWIDTH) == RIO_PORT_N_CTL_PWIDTH_4)
@@ -207,7 +204,7 @@ tsi57x_em_init(struct rio_dev *rdev)
 	}
 
 	/* set TVAL = ~50us */
-	rio_mport_write_config_32(mport, destid, hopcount,
+	rio_write_config_32(rdev,
 		rdev->phys_efptr + RIO_PORT_LINKTO_CTL_CSR, 0x9a << 8);
 
 	return 0;
@@ -217,14 +214,12 @@ static int
 tsi57x_em_handler(struct rio_dev *rdev, u8 portnum)
 {
 	struct rio_mport *mport = rdev->net->hport;
-	u16 destid = rdev->rswitch->destid;
-	u8 hopcount = rdev->rswitch->hopcount;
 	u32 intstat, err_status;
 	int sendcount, checkcount;
 	u8 route_port;
 	u32 regval;
 
-	rio_mport_read_config_32(mport, destid, hopcount,
+	rio_read_config_32(rdev,
 			rdev->phys_efptr + RIO_PORT_N_ERR_STS_CSR(portnum),
 			&err_status);
 
@@ -232,15 +227,15 @@ tsi57x_em_handler(struct rio_dev *rdev, u8 portnum)
 	    (err_status & (RIO_PORT_N_ERR_STS_PW_OUT_ES |
 			  RIO_PORT_N_ERR_STS_PW_INP_ES))) {
 		/* Remove any queued packets by locking/unlocking port */
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 			rdev->phys_efptr + RIO_PORT_N_CTL_CSR(portnum),
 			&regval);
 		if (!(regval & RIO_PORT_N_CTL_LOCKOUT)) {
-			rio_mport_write_config_32(mport, destid, hopcount,
+			rio_write_config_32(rdev,
 				rdev->phys_efptr + RIO_PORT_N_CTL_CSR(portnum),
 				regval | RIO_PORT_N_CTL_LOCKOUT);
 			udelay(50);
-			rio_mport_write_config_32(mport, destid, hopcount,
+			rio_write_config_32(rdev,
 				rdev->phys_efptr + RIO_PORT_N_CTL_CSR(portnum),
 				regval);
 		}
@@ -248,7 +243,7 @@ tsi57x_em_handler(struct rio_dev *rdev, u8 portnum)
 		/* Read from link maintenance response register to clear
 		 * valid bit
 		 */
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 			rdev->phys_efptr + RIO_PORT_N_MNT_RSP_CSR(portnum),
 			&regval);
 
@@ -257,13 +252,12 @@ tsi57x_em_handler(struct rio_dev *rdev, u8 portnum)
 		 */
 		sendcount = 3;
 		while (sendcount) {
-			rio_mport_write_config_32(mport, destid, hopcount,
+			rio_write_config_32(rdev,
 					  TSI578_SP_CS_TX(portnum), 0x40fc8000);
 			checkcount = 3;
 			while (checkcount--) {
 				udelay(50);
-				rio_mport_read_config_32(
-					mport, destid, hopcount,
+				rio_read_config_32(rdev,
 					rdev->phys_efptr +
 						RIO_PORT_N_MNT_RSP_CSR(portnum),
 					&regval);
@@ -277,25 +271,23 @@ tsi57x_em_handler(struct rio_dev *rdev, u8 portnum)
 
 exit_es:
 	/* Clear implementation specific error status bits */
-	rio_mport_read_config_32(mport, destid, hopcount,
-				 TSI578_SP_INT_STATUS(portnum), &intstat);
+	rio_read_config_32(rdev, TSI578_SP_INT_STATUS(portnum), &intstat);
 	pr_debug("TSI578[%x:%x] SP%d_INT_STATUS=0x%08x\n",
-		 destid, hopcount, portnum, intstat);
+		 rdev->destid, rdev->hopcount, portnum, intstat);
 
 	if (intstat & 0x10000) {
-		rio_mport_read_config_32(mport, destid, hopcount,
+		rio_read_config_32(rdev,
 				TSI578_SP_LUT_PEINF(portnum), &regval);
 		regval = (mport->sys_size) ? (regval >> 16) : (regval >> 24);
 		route_port = rdev->rswitch->route_table[regval];
 		pr_debug("RIO: TSI578[%s] P%d LUT Parity Error (destID=%d)\n",
 			rio_name(rdev), portnum, regval);
-		tsi57x_route_add_entry(mport, destid, hopcount,
+		tsi57x_route_add_entry(mport, rdev->destid, rdev->hopcount,
 				RIO_GLOBAL_TABLE, regval, route_port);
 	}
 
-	rio_mport_write_config_32(mport, destid, hopcount,
-				  TSI578_SP_INT_STATUS(portnum),
-				  intstat & 0x000700bd);
+	rio_write_config_32(rdev, TSI578_SP_INT_STATUS(portnum),
+			    intstat & 0x000700bd);
 
 	return 0;
 }
diff --git a/include/linux/rio.h b/include/linux/rio.h
index 0bed941..f6e25b3 100644
--- a/include/linux/rio.h
+++ b/include/linux/rio.h
@@ -98,7 +98,8 @@ union rio_pw_msg;
  * @dev: Device model device
  * @riores: RIO resources this device owns
  * @pwcback: port-write callback function for this device
- * @destid: Network destination ID
+ * @destid: Network destination ID (or associated destid for switch)
+ * @hopcount: Hopcount to this device
  * @prev: Previous RIO device connected to the current one
  */
 struct rio_dev {
@@ -126,6 +127,7 @@ struct rio_dev {
 	struct resource riores[RIO_MAX_DEV_RESOURCES];
 	int (*pwcback) (struct rio_dev *rdev, union rio_pw_msg *msg, int step);
 	u16 destid;
+	u8 hopcount;
 	struct rio_dev *prev;
 };
 
@@ -229,8 +231,6 @@ struct rio_net {
  * @node: Node in global list of switches
  * @rdev: Associated RIO device structure
  * @switchid: Switch ID that is unique across a network
- * @hopcount: Hopcount to this switch
- * @destid: Associated destid in the path
  * @route_table: Copy of switch routing table
  * @port_ok: Status of each port (one bit per port) - OK=1 or UNINIT=0
  * @add_entry: Callback for switch-specific route add function
@@ -247,8 +247,6 @@ struct rio_switch {
 	struct list_head node;
 	struct rio_dev *rdev;
 	u16 switchid;
-	u16 hopcount;
-	u16 destid;
 	u8 *route_table;
 	u32 port_ok;
 	int (*add_entry) (struct rio_mport * mport, u16 destid, u8 hopcount,
diff --git a/include/linux/rio_drv.h b/include/linux/rio_drv.h
index edc55da..e09e565 100644
--- a/include/linux/rio_drv.h
+++ b/include/linux/rio_drv.h
@@ -150,16 +150,8 @@ static inline int rio_local_write_config_8(struct rio_mport *port, u32 offset,
 static inline int rio_read_config_32(struct rio_dev *rdev, u32 offset,
 				     u32 * data)
 {
-	u8 hopcount = 0xff;
-	u16 destid = rdev->destid;
-
-	if (rdev->rswitch) {
-		destid = rdev->rswitch->destid;
-		hopcount = rdev->rswitch->hopcount;
-	}
-
-	return rio_mport_read_config_32(rdev->net->hport, destid, hopcount,
-					offset, data);
+	return rio_mport_read_config_32(rdev->net->hport, rdev->destid,
+					rdev->hopcount, offset, data);
 };
 
 /**
@@ -174,16 +166,8 @@ static inline int rio_read_config_32(struct rio_dev *rdev, u32 offset,
 static inline int rio_write_config_32(struct rio_dev *rdev, u32 offset,
 				      u32 data)
 {
-	u8 hopcount = 0xff;
-	u16 destid = rdev->destid;
-
-	if (rdev->rswitch) {
-		destid = rdev->rswitch->destid;
-		hopcount = rdev->rswitch->hopcount;
-	}
-
-	return rio_mport_write_config_32(rdev->net->hport, destid, hopcount,
-					 offset, data);
+	return rio_mport_write_config_32(rdev->net->hport, rdev->destid,
+					 rdev->hopcount, offset, data);
 };
 
 /**
@@ -198,16 +182,8 @@ static inline int rio_write_config_32(struct rio_dev *rdev, u32 offset,
 static inline int rio_read_config_16(struct rio_dev *rdev, u32 offset,
 				     u16 * data)
 {
-	u8 hopcount = 0xff;
-	u16 destid = rdev->destid;
-
-	if (rdev->rswitch) {
-		destid = rdev->rswitch->destid;
-		hopcount = rdev->rswitch->hopcount;
-	}
-
-	return rio_mport_read_config_16(rdev->net->hport, destid, hopcount,
-					offset, data);
+	return rio_mport_read_config_16(rdev->net->hport, rdev->destid,
+					rdev->hopcount, offset, data);
 };
 
 /**
@@ -222,16 +198,8 @@ static inline int rio_read_config_16(struct rio_dev *rdev, u32 offset,
 static inline int rio_write_config_16(struct rio_dev *rdev, u32 offset,
 				      u16 data)
 {
-	u8 hopcount = 0xff;
-	u16 destid = rdev->destid;
-
-	if (rdev->rswitch) {
-		destid = rdev->rswitch->destid;
-		hopcount = rdev->rswitch->hopcount;
-	}
-
-	return rio_mport_write_config_16(rdev->net->hport, destid, hopcount,
-					 offset, data);
+	return rio_mport_write_config_16(rdev->net->hport, rdev->destid,
+					 rdev->hopcount, offset, data);
 };
 
 /**
@@ -245,16 +213,8 @@ static inline int rio_write_config_16(struct rio_dev *rdev, u32 offset,
  */
 static inline int rio_read_config_8(struct rio_dev *rdev, u32 offset, u8 * data)
 {
-	u8 hopcount = 0xff;
-	u16 destid = rdev->destid;
-
-	if (rdev->rswitch) {
-		destid = rdev->rswitch->destid;
-		hopcount = rdev->rswitch->hopcount;
-	}
-
-	return rio_mport_read_config_8(rdev->net->hport, destid, hopcount,
-				       offset, data);
+	return rio_mport_read_config_8(rdev->net->hport, rdev->destid,
+				       rdev->hopcount, offset, data);
 };
 
 /**
@@ -268,16 +228,8 @@ static inline int rio_read_config_8(struct rio_dev *rdev, u32 offset, u8 * data)
  */
 static inline int rio_write_config_8(struct rio_dev *rdev, u32 offset, u8 data)
 {
-	u8 hopcount = 0xff;
-	u16 destid = rdev->destid;
-
-	if (rdev->rswitch) {
-		destid = rdev->rswitch->destid;
-		hopcount = rdev->rswitch->hopcount;
-	}
-
-	return rio_mport_write_config_8(rdev->net->hport, destid, hopcount,
-					offset, data);
+	return rio_mport_write_config_8(rdev->net->hport, rdev->destid,
+					rdev->hopcount, offset, data);
 };
 
 extern int rio_mport_send_doorbell(struct rio_mport *mport, u16 destid,
-- 
1.7.3.1

[PATCH -mm 2/2] RapidIO: Integrate rio_switch into rio_dev

[Alexandre Bounine]

Convert RIO switches device structures (rio_dev + rio_switch) into
a single allocation unit.

This change is based on the fact that RIO switches are using
common RIO device objects anyway. Allocating RIO switch objects
as RIO devices with added space for switch information simplifies handling
of RIO switch devices.

Signed-off-by: Alexandre Bounine <alexandre.bounine@idt.com>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Li Yang <leoli@freescale.com>
Cc: Thomas Moll <thomas.moll@sysgo.com>
Cc: Micha Nelissen <micha@neli.hopto.org>
---
 drivers/rapidio/rio-scan.c  |   59 +++++++++++++++++--------------
 drivers/rapidio/rio-sysfs.c |    4 +-
 include/linux/rio.h         |   82 +++++++++++++++++++++---------------------
 3 files changed, 75 insertions(+), 70 deletions(-)

diff --git a/drivers/rapidio/rio-scan.c b/drivers/rapidio/rio-scan.c
index 51f0af2..45d14cd 100644
--- a/drivers/rapidio/rio-scan.c
+++ b/drivers/rapidio/rio-scan.c
@@ -378,12 +378,30 @@ static struct rio_dev __devinit *rio_setup_device(struct rio_net *net,
 	struct rio_dev *rdev;
 	struct rio_switch *rswitch = NULL;
 	int result, rdid;
+	size_t size;
+	u32 swpinfo = 0;
 
-	rdev = kzalloc(sizeof(struct rio_dev), GFP_KERNEL);
+	size = sizeof(struct rio_dev);
+	if (rio_mport_read_config_32(port, destid, hopcount,
+				     RIO_PEF_CAR, &result))
+		return NULL;
+
+	if (result & (RIO_PEF_SWITCH | RIO_PEF_MULTIPORT)) {
+		rio_mport_read_config_32(port, destid, hopcount,
+					 RIO_SWP_INFO_CAR, &swpinfo);
+		if (result & RIO_PEF_SWITCH) {
+			size += (RIO_GET_TOTAL_PORTS(swpinfo) *
+				sizeof(rswitch->nextdev[0])) + sizeof(*rswitch);
+		}
+	}
+
+	rdev = kzalloc(size, GFP_KERNEL);
 	if (!rdev)
 		return NULL;
 
 	rdev->net = net;
+	rdev->pef = result;
+	rdev->swpinfo = swpinfo;
 	rio_mport_read_config_32(port, destid, hopcount, RIO_DEV_ID_CAR,
 				 &result);
 	rdev->did = result >> 16;
@@ -397,8 +415,6 @@ static struct rio_dev __devinit *rio_setup_device(struct rio_net *net,
 	rio_mport_read_config_32(port, destid, hopcount, RIO_ASM_INFO_CAR,
 				 &result);
 	rdev->asm_rev = result >> 16;
-	rio_mport_read_config_32(port, destid, hopcount, RIO_PEF_CAR,
-				 &rdev->pef);
 	if (rdev->pef & RIO_PEF_EXT_FEATURES) {
 		rdev->efptr = result & 0xffff;
 		rdev->phys_efptr = rio_mport_get_physefb(port, 0, destid,
@@ -408,11 +424,6 @@ static struct rio_dev __devinit *rio_setup_device(struct rio_net *net,
 						hopcount, RIO_EFB_ERR_MGMNT);
 	}
 
-	if (rdev->pef & (RIO_PEF_SWITCH | RIO_PEF_MULTIPORT)) {
-		rio_mport_read_config_32(port, destid, hopcount,
-					 RIO_SWP_INFO_CAR, &rdev->swpinfo);
-	}
-
 	rio_mport_read_config_32(port, destid, hopcount, RIO_SRC_OPS_CAR,
 				 &rdev->src_ops);
 	rio_mport_read_config_32(port, destid, hopcount, RIO_DST_OPS_CAR,
@@ -449,12 +460,7 @@ static struct rio_dev __devinit *rio_setup_device(struct rio_net *net,
 
 	/* If a PE has both switch and other functions, show it as a switch */
 	if (rio_is_switch(rdev)) {
-		rswitch = kzalloc(sizeof(*rswitch) +
-				  RIO_GET_TOTAL_PORTS(rdev->swpinfo) *
-				  sizeof(rswitch->nextdev[0]),
-				  GFP_KERNEL);
-		if (!rswitch)
-			goto cleanup;
+		rswitch = rdev->rswitch;
 		rswitch->switchid = next_switchid;
 		rswitch->port_ok = 0;
 		rswitch->route_table = kzalloc(sizeof(u8)*
@@ -466,15 +472,13 @@ static struct rio_dev __devinit *rio_setup_device(struct rio_net *net,
 		for (rdid = 0; rdid < RIO_MAX_ROUTE_ENTRIES(port->sys_size);
 				rdid++)
 			rswitch->route_table[rdid] = RIO_INVALID_ROUTE;
-		rdev->rswitch = rswitch;
-		rswitch->rdev = rdev;
 		dev_set_name(&rdev->dev, "%02x:s:%04x", rdev->net->id,
-			     rdev->rswitch->switchid);
+			     rswitch->switchid);
 		rio_switch_init(rdev, do_enum);
 
-		if (do_enum && rdev->rswitch->clr_table)
-			rdev->rswitch->clr_table(port, destid, hopcount,
-						 RIO_GLOBAL_TABLE);
+		if (do_enum && rswitch->clr_table)
+			rswitch->clr_table(port, destid, hopcount,
+					   RIO_GLOBAL_TABLE);
 
 		list_add_tail(&rswitch->node, &rio_switches);
 
@@ -510,10 +514,9 @@ static struct rio_dev __devinit *rio_setup_device(struct rio_net *net,
 	return rdev;
 
 cleanup:
-	if (rswitch) {
+	if (rswitch->route_table)
 		kfree(rswitch->route_table);
-		kfree(rswitch);
-	}
+
 	kfree(rdev);
 	return NULL;
 }
@@ -1072,7 +1075,7 @@ static struct rio_net __devinit *rio_alloc_net(struct rio_mport *port)
  */
 static void rio_update_route_tables(struct rio_mport *port)
 {
-	struct rio_dev *rdev;
+	struct rio_dev *rdev, *swrdev;
 	struct rio_switch *rswitch;
 	u8 sport;
 	u16 destid;
@@ -1087,14 +1090,16 @@ static void rio_update_route_tables(struct rio_mport *port)
 				continue;
 
 			if (RIO_INVALID_ROUTE == rswitch->route_table[destid]) {
+				swrdev = sw_to_rio_dev(rswitch);
+
 				/* Skip if destid ends in empty switch*/
-				if (rswitch->rdev->destid == destid)
+				if (swrdev->destid == destid)
 					continue;
 
-				sport = RIO_GET_PORT_NUM(rswitch->rdev->swpinfo);
+				sport = RIO_GET_PORT_NUM(swrdev->swpinfo);
 
 				if (rswitch->add_entry)	{
-					rio_route_add_entry(rswitch->rdev,
+					rio_route_add_entry(swrdev,
 						RIO_GLOBAL_TABLE, destid,
 						sport, 0);
 					rswitch->route_table[destid] = sport;
diff --git a/drivers/rapidio/rio-sysfs.c b/drivers/rapidio/rio-sysfs.c
index 137ed93..76b4185 100644
--- a/drivers/rapidio/rio-sysfs.c
+++ b/drivers/rapidio/rio-sysfs.c
@@ -217,7 +217,7 @@ int rio_create_sysfs_dev_files(struct rio_dev *rdev)
 
 	err = device_create_bin_file(&rdev->dev, &rio_config_attr);
 
-	if (!err && rdev->rswitch) {
+	if (!err && (rdev->pef & RIO_PEF_SWITCH)) {
 		err = device_create_file(&rdev->dev, &dev_attr_routes);
 		if (!err && rdev->rswitch->sw_sysfs)
 			err = rdev->rswitch->sw_sysfs(rdev, RIO_SW_SYSFS_CREATE);
@@ -239,7 +239,7 @@ int rio_create_sysfs_dev_files(struct rio_dev *rdev)
 void rio_remove_sysfs_dev_files(struct rio_dev *rdev)
 {
 	device_remove_bin_file(&rdev->dev, &rio_config_attr);
-	if (rdev->rswitch) {
+	if (rdev->pef & RIO_PEF_SWITCH) {
 		device_remove_file(&rdev->dev, &dev_attr_routes);
 		if (rdev->rswitch->sw_sysfs)
 			rdev->rswitch->sw_sysfs(rdev, RIO_SW_SYSFS_REMOVE);
diff --git a/include/linux/rio.h b/include/linux/rio.h
index f6e25b3..9b55885 100644
--- a/include/linux/rio.h
+++ b/include/linux/rio.h
@@ -71,9 +71,47 @@ extern struct device rio_bus;
 extern struct list_head rio_devices;	/* list of all devices */
 
 struct rio_mport;
+struct rio_dev;
 union rio_pw_msg;
 
 /**
+ * struct rio_switch - RIO switch info
+ * @node: Node in global list of switches
+ * @switchid: Switch ID that is unique across a network
+ * @route_table: Copy of switch routing table
+ * @port_ok: Status of each port (one bit per port) - OK=1 or UNINIT=0
+ * @add_entry: Callback for switch-specific route add function
+ * @get_entry: Callback for switch-specific route get function
+ * @clr_table: Callback for switch-specific clear route table function
+ * @set_domain: Callback for switch-specific domain setting function
+ * @get_domain: Callback for switch-specific domain get function
+ * @em_init: Callback for switch-specific error management init function
+ * @em_handle: Callback for switch-specific error management handler function
+ * @sw_sysfs: Callback that initializes switch-specific sysfs attributes
+ * @nextdev: Array of per-port pointers to the next attached device
+ */
+struct rio_switch {
+	struct list_head node;
+	u16 switchid;
+	u8 *route_table;
+	u32 port_ok;
+	int (*add_entry) (struct rio_mport *mport, u16 destid, u8 hopcount,
+			  u16 table, u16 route_destid, u8 route_port);
+	int (*get_entry) (struct rio_mport *mport, u16 destid, u8 hopcount,
+			  u16 table, u16 route_destid, u8 *route_port);
+	int (*clr_table) (struct rio_mport *mport, u16 destid, u8 hopcount,
+			  u16 table);
+	int (*set_domain) (struct rio_mport *mport, u16 destid, u8 hopcount,
+			   u8 sw_domain);
+	int (*get_domain) (struct rio_mport *mport, u16 destid, u8 hopcount,
+			   u8 *sw_domain);
+	int (*em_init) (struct rio_dev *dev);
+	int (*em_handle) (struct rio_dev *dev, u8 swport);
+	int (*sw_sysfs) (struct rio_dev *dev, int create);
+	struct rio_dev *nextdev[0];
+};
+
+/**
  * struct rio_dev - RIO device info
  * @global_list: Node in list of all RIO devices
  * @net_list: Node in list of RIO devices in a network
@@ -93,7 +131,6 @@ union rio_pw_msg;
  * @phys_efptr: RIO device extended features pointer
  * @em_efptr: RIO Error Management features pointer
  * @dma_mask: Mask of bits of RIO address this device implements
- * @rswitch: Pointer to &struct rio_switch if valid for this device
  * @driver: Driver claiming this device
  * @dev: Device model device
  * @riores: RIO resources this device owns
@@ -101,6 +138,7 @@ union rio_pw_msg;
  * @destid: Network destination ID (or associated destid for switch)
  * @hopcount: Hopcount to this device
  * @prev: Previous RIO device connected to the current one
+ * @rswitch: struct rio_switch (if valid for this device)
  */
 struct rio_dev {
 	struct list_head global_list;	/* node in list of all RIO devices */
@@ -121,7 +159,6 @@ struct rio_dev {
 	u32 phys_efptr;
 	u32 em_efptr;
 	u64 dma_mask;
-	struct rio_switch *rswitch;	/* RIO switch info */
 	struct rio_driver *driver;	/* RIO driver claiming this device */
 	struct device dev;	/* LDM device structure */
 	struct resource riores[RIO_MAX_DEV_RESOURCES];
@@ -129,11 +166,13 @@ struct rio_dev {
 	u16 destid;
 	u8 hopcount;
 	struct rio_dev *prev;
+	struct rio_switch rswitch[0];	/* RIO switch info */
 };
 
 #define rio_dev_g(n) list_entry(n, struct rio_dev, global_list)
 #define rio_dev_f(n) list_entry(n, struct rio_dev, net_list)
 #define	to_rio_dev(n) container_of(n, struct rio_dev, dev)
+#define sw_to_rio_dev(n) container_of(n, struct rio_dev, rswitch[0])
 
 /**
  * struct rio_msg - RIO message event
@@ -226,45 +265,6 @@ struct rio_net {
 #define RIO_SW_SYSFS_CREATE	1	/* Create switch attributes */
 #define RIO_SW_SYSFS_REMOVE	0	/* Remove switch attributes */
 
-/**
- * struct rio_switch - RIO switch info
- * @node: Node in global list of switches
- * @rdev: Associated RIO device structure
- * @switchid: Switch ID that is unique across a network
- * @route_table: Copy of switch routing table
- * @port_ok: Status of each port (one bit per port) - OK=1 or UNINIT=0
- * @add_entry: Callback for switch-specific route add function
- * @get_entry: Callback for switch-specific route get function
- * @clr_table: Callback for switch-specific clear route table function
- * @set_domain: Callback for switch-specific domain setting function
- * @get_domain: Callback for switch-specific domain get function
- * @em_init: Callback for switch-specific error management initialization function
- * @em_handle: Callback for switch-specific error management handler function
- * @sw_sysfs: Callback that initializes switch-specific sysfs attributes
- * @nextdev: Array of per-port pointers to the next attached device
- */
-struct rio_switch {
-	struct list_head node;
-	struct rio_dev *rdev;
-	u16 switchid;
-	u8 *route_table;
-	u32 port_ok;
-	int (*add_entry) (struct rio_mport * mport, u16 destid, u8 hopcount,
-			  u16 table, u16 route_destid, u8 route_port);
-	int (*get_entry) (struct rio_mport * mport, u16 destid, u8 hopcount,
-			  u16 table, u16 route_destid, u8 * route_port);
-	int (*clr_table) (struct rio_mport *mport, u16 destid, u8 hopcount,
-			  u16 table);
-	int (*set_domain) (struct rio_mport *mport, u16 destid, u8 hopcount,
-			   u8 sw_domain);
-	int (*get_domain) (struct rio_mport *mport, u16 destid, u8 hopcount,
-			   u8 *sw_domain);
-	int (*em_init) (struct rio_dev *dev);
-	int (*em_handle) (struct rio_dev *dev, u8 swport);
-	int (*sw_sysfs) (struct rio_dev *dev, int create);
-	struct rio_dev *nextdev[0];
-};
-
 /* Low-level architecture-dependent routines */
 
 /**
-- 
1.7.3.1

Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Micha Nelissen]

Alexandre Bounine wrote:
> 1. Using one storage location common for switches and endpoints eliminates
> unnecessary device type checks during maintenance access operations.
> While destination IDs and hop counts have different meaning for endpoints and
> switches, this does not prevent us from storing them in the primary RIO device
> structure (rio_dev) for both types.

How can you say this? The two variables have different meanings, this 
logically implies you can't merge them. So how do you say 'this does not 
prevent us from ...' without providing a reason?

> 2. Convert RIO switch device structures (rio_dev + rio_switch) into single
> allocation unit. This change is based on the fact that RIO switches are using
> common RIO device objects anyway. Allocating RIO switch objects as RIO devices
> with added space for switch information simplifies handling of RIO switch device
> objects.

I still don't think that's a good idea because the rdev->rswitch pointer 
can be defined to point to the switch that a given rio_dev is connected 
to. This is useful for quick lookups. How else can to know to which 
switch a given device is connected?

Micha

RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Bounine, Alexandre]

Micha Nelissen <micha@neli.hopto.org> wrote:
> 
> Alexandre Bounine wrote:
> > 1. Using one storage location common for switches and endpoints
eliminates
> > unnecessary device type checks during maintenance access operations.
> > While destination IDs and hop counts have different meaning for
endpoints and
> > switches, this does not prevent us from storing them in the primary
RIO device
> > structure (rio_dev) for both types.
> 
> How can you say this? The two variables have different meanings, this
> logically implies you can't merge them. So how do you say 'this does
not
> prevent us from ...' without providing a reason?

Looks like I formulated it bad - better would be: they have different
interpretation by hardware but logically in RapidIO they have single
role - destid/hopcount are a device coordinates in the RIO network used
to access that device.

> > 2. Convert RIO switch device structures (rio_dev + rio_switch) into
single
> > allocation unit. This change is based on the fact that RIO switches
are using
> > common RIO device objects anyway. Allocating RIO switch objects as
RIO devices
> > with added space for switch information simplifies handling of RIO
switch device
> > objects.
> 
> I still don't think that's a good idea because the rdev->rswitch
pointer
> can be defined to point to the switch that a given rio_dev is
connected
> to. This is useful for quick lookups. How else can to know to which
> switch a given device is connected?

rdev->rswitch is not a pointer to the entire switch device object - it
is a pointer to the switch specific extension associated with given
rio_dev (if applicable). There is no other role for rdev->rswitch.

Why would you keep a pointer to device data extension instead of the
pointer to attached device object itself?

BTW, I have back and forward links added in previous patches and only
one link that may be added later is a forward link from mport to the
attached rio_dev (ptr to rio_switch will not work here because it can be
switchless connection). But this reference has to be added into
rio_mport.
 
Alex.

Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Micha Nelissen]

Bounine, Alexandre wrote:
> Micha Nelissen <micha@neli.hopto.org> wrote:
>> Alexandre Bounine wrote:
>> How can you say this? The two variables have different meanings, this
>> logically implies you can't merge them. So how do you say 'this does
> not
>> prevent us from ...' without providing a reason?
> 
> Looks like I formulated it bad - better would be: they have different
> interpretation by hardware but logically in RapidIO they have single
> role - destid/hopcount are a device coordinates in the RIO network used
> to access that device.

They are logically different as well (for a non-host).

rswitch->destid with hopcount is the way to reach that switch.

rswitch->rdev->destid should be the id associated with a given switch, 
so that every (processor) device can agree what id some switch has. For 
a non-host, the path to reach a switch may use a different id than the 
switch itself has; it's just the id by which it was discovered.

However, it's possible to fix that by fixing the id+hopcount once the 
switch is found using the path with its own id: then you know the right 
hopcount.

>> can be defined to point to the switch that a given rio_dev is
> connected
>> to. This is useful for quick lookups. How else can to know to which
>> switch a given device is connected?
> 
> rdev->rswitch is not a pointer to the entire switch device object - it
> is a pointer to the switch specific extension associated with given
> rio_dev (if applicable). There is no other role for rdev->rswitch.

I know this, it doesn't answer my question.

> Why would you keep a pointer to device data extension instead of the
> pointer to attached device object itself?

There is no particular reason, but this is a useful way to define the 
fields that are there.

My point is, now that you remove the pointer field, that information (to 
which switch is a particular device connected) cannot be stored in this 
way, so do you have an alternative proposal for that? Maybe add a new field.

> BTW, I have back and forward links added in previous patches and only
> one link that may be added later is a forward link from mport to the
> attached rio_dev (ptr to rio_switch will not work here because it can be
> switchless connection). But this reference has to be added into
> rio_mport.

Possible, but I suggest to put it in the rio_net: fields rdev_host, and 
rdev_self. You can see it in the patch I sent you.

Micha

RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Bounine, Alexandre]

Micha Nelissen <micha@neli.hopto.org> wrote:
> 
> Bounine, Alexandre wrote:
> > Looks like I formulated it bad - better would be: they have
different
> > interpretation by hardware but logically in RapidIO they have single
> > role - destid/hopcount are a device coordinates in the RIO network
used
> > to access that device.
> 
> They are logically different as well (for a non-host).
> 
> rswitch->destid with hopcount is the way to reach that switch.
> 

OK. This is moved to rdev->destid now to make access unified with
endpoints. 

> rswitch->rdev->destid should be the id associated with a given switch,
> so that every (processor) device can agree what id some switch has.
For
> a non-host, the path to reach a switch may use a different id than the
> switch itself has; it's just the id by which it was discovered.
> However, it's possible to fix that by fixing the id+hopcount once the
> switch is found using the path with its own id: then you know the
right
> hopcount.

I have got an impression that we are discussing slightly different
implementations
here. The suggested role of rswitch->rdev->destid is not clear to me.
I agree that destid and hopcount for switch will be different for every
processor.
There is nothing wrong with it because a switch physically does not have
its own ID.
If we will need to identify the same physical switch by different
processors we may use the component tag which now is unique for every
device.

This actually gives me another idea: instead of using global
next_switchid counter make rswitch->switchid = component_tag and
switches in sysfs will look identical for every processor (or just get
rid of rswitch->switchid and use component_tag directly for switches).


> >> can be defined to point to the switch that a given rio_dev is
> > connected
> >> to. This is useful for quick lookups. How else can to know to which
> >> switch a given device is connected?
> >
> > rdev->rswitch is not a pointer to the entire switch device object -
it
> > is a pointer to the switch specific extension associated with given
> > rio_dev (if applicable). There is no other role for rdev->rswitch.
> 
> I know this, it doesn't answer my question.
> 
> > Why would you keep a pointer to device data extension instead of the
> > pointer to attached device object itself?
> 
> There is no particular reason, but this is a useful way to define the
> fields that are there.
> 
> My point is, now that you remove the pointer field, that information
(to
> which switch is a particular device connected) cannot be stored in
this
> way, so do you have an alternative proposal for that? Maybe add a new
field.
>

See my comment below ;).

> > BTW, I have back and forward links added in previous patches and
only
> > one link that may be added later is a forward link from mport to the
> > attached rio_dev (ptr to rio_switch will not work here because it
can be
> > switchless connection). But this reference has to be added into
> > rio_mport.
> 
> Possible, but I suggest to put it in the rio_net: fields rdev_host,
and
> rdev_self. You can see it in the patch I sent you.

Yes, we may rework rio_net that way and use some good things from there.

Alex.

Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Micha Nelissen]

Bounine, Alexandre wrote:
> Micha Nelissen <micha@neli.hopto.org> wrote:
>> rswitch->rdev->destid should be the id associated with a given switch,
>> so that every (processor) device can agree what id some switch has.
> 
> If we will need to identify the same physical switch by different
> processors we may use the component tag which now is unique for every
> device.

Yes, identification is the point. I think it might be confusing to have 
a destid *and* a component tag id which are slightly different. The 
destid is unambiguous (if you know whether the device is a switch or 
endpoint) so I think it makes sense to use that if possible.

> This actually gives me another idea: instead of using global
> next_switchid counter make rswitch->switchid = component_tag and
> switches in sysfs will look identical for every processor (or just get
> rid of rswitch->switchid and use component_tag directly for switches).

I still prefer the destid as the single identification id.

Micha

RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Bounine, Alexandre]

Micha Nelissen <micha@neli.hopto.org> wrote:
> Bounine, Alexandre wrote:
> > If we will need to identify the same physical switch by different
> > processors we may use the component tag which now is unique for
every
> > device.
> 
> Yes, identification is the point. I think it might be confusing to
have
> a destid *and* a component tag id which are slightly different. The
> destid is unambiguous (if you know whether the device is a switch or
> endpoint) so I think it makes sense to use that if possible.

The component tag is the way to identify a RIO device (switch or
endpoint).
1. it is defined by RIO spec as a register existing in both types of
devices (this provides a universal access to the identification
information by any processor).
2. the Error Management specification already uses the CT as a device
identifier.
3. the CT value is large enough to be unique for max number of endpoints
in the large system and any reasonable number of switches. BTW, I am
planning to provide a patch that defines CT fields to ensure future
compatibility.

The destid does not exists as a HW element of switches and therefore
cannot be used as a universal identification token.

> > This actually gives me another idea: instead of using global
> > next_switchid counter make rswitch->switchid = component_tag and
> > switches in sysfs will look identical for every processor (or just
get
> > rid of rswitch->switchid and use component_tag directly for
switches).
> 
> I still prefer the destid as the single identification id.

As I answered above, destid cannot be used as a universal identification
token - it is a routing element. The destID has register in endpoints
only to provide a packet filtering.

In your patch you allocate individual destid for switches. This method
has two problems:
1. The destid for the switch needs an additional mechanism to share it
among processors in the RIO network,
2. It takes ID value away from the pool of available IDs, what will
reduce number of IDs that can be assigned to endpoints. (NOTE: I am
actually working on destID assignment scheme that will recycle destID in
case of hot-swap events, i.e. if device is extracted its destID will be
returned to the pool of available IDs and may be reused later for device
insertion).

The only case when assigning individual destid to the switch is
justified is an "empty" switch - one without any endpoints attached to
it. But that destid should be assigned to an endpoint as soon as it is
attached to that switch.  

Alex.

Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Micha Nelissen]

Bounine, Alexandre wrote:
> Micha Nelissen <micha@neli.hopto.org> wrote:
>>> rid of rswitch->switchid and use component_tag directly for
> switches).
>> I still prefer the destid as the single identification id.
> 
> In your patch you allocate individual destid for switches. This method
> has two problems:
> 1. The destid for the switch needs an additional mechanism to share it
> among processors in the RIO network,

? See comment for 2)

> 2. It takes ID value away from the pool of available IDs, what will

It does not take an ID away, it shares it with a connected endpoint to 
that switch. The tag uses one extra bit to identify the device as a 
switch instead of an endpoint. This provides the information to 
unambiguously identify a switch from an endpoint.

Micha

RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Bounine, Alexandre]

Micha Nelissen <micha@neli.hopto.org> wrote:
> 
> Bounine, Alexandre wrote:
> > 1. The destid for the switch needs an additional mechanism to share
it
> > among processors in the RIO network,
> 
> ? See comment for 2)
> 
> > 2. It takes ID value away from the pool of available IDs, what will
> 
> It does not take an ID away, it shares it with a connected endpoint to
> that switch. The tag uses one extra bit to identify the device as a
> switch instead of an endpoint. This provides the information to
> unambiguously identify a switch from an endpoint.

OK taking away #2. But do not see how it justifies storing two values of
destid.

And you have just confirmed using CT for unique identification. We
simply have differences in interpretation of CT: you are using component
tag to pass unique identification and I am using CT as a unique
identification. I prefer not to assume any relationship between routing
information and the component tag.

Alex.

Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Micha Nelissen]

Bounine, Alexandre wrote:
> Micha Nelissen <micha@neli.hopto.org> wrote:
>> that switch. The tag uses one extra bit to identify the device as a
>> switch instead of an endpoint. This provides the information to
>> unambiguously identify a switch from an endpoint.
> 
> OK taking away #2. But do not see how it justifies storing two values of
> destid.

I look at it this way: it prevents the need for another layer of 
indirection: translating component tag to a destid.

> And you have just confirmed using CT for unique identification. 

That's correct, but I never said (intended to say) I didn't.

> We
> simply have differences in interpretation of CT: you are using component
> tag to pass unique identification and I am using CT as a unique
> identification. I prefer not to assume any relationship between routing
> information and the component tag.

Why no relation? My experience is that during debugging it's useful to 
have the destid directly at hand, it's just very practical. (Otherwise 
any drawing of a random network would need two "identification" numbers 
per drawn node: the component tag (true identification), and destid 
since that's what everyone uses to identify a device, what needs to 
programmed into the LUTs of a switch, identification in sysfs, etc.).

Micha

RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Bounine, Alexandre]

Micha Nelissen <micha@neli.hopto.org> wrote:
> 
> I look at it this way: it prevents the need for another layer of
> indirection: translating component tag to a destid.

The destid alone is not enough. You will need an entire rio_dev object
for that device anyway.

> 
> Why no relation? My experience is that during debugging it's useful to
> have the destid directly at hand, it's just very practical. (Otherwise
> any drawing of a random network would need two "identification"
numbers
> per drawn node: the component tag (true identification), and destid
> since that's what everyone uses to identify a device, what needs to
> programmed into the LUTs of a switch, identification in sysfs, etc.).

I think we are mixing two things together here. I understand your idea
but do not see how it prevents me from having one common set of access
coordinates for RIO devices (the starting point of our discussion). 

Regardless of an implementation, having a way that ensures unified
identification of switches by all processor boards is better than the
current mainline implementation. Methods of forming a component tag may
differ but still serve the same purpose. Personally I prefer to avoid
any link of device identification to the destid because it may not be as
intuitive as it seems for large systems with hot-plug. I will discuss
this with some of RIO TWG guys to get their opinion on the best
approach.

I will make a patch that defines fields of component tag, probably just
one for now - identification field. This will ensure that any method
used to assign component tag (id part of it) will be compatible with RIO
spec part 8 error management.

As for switch identification, at this moment I still prefer replacing
rswitch->switchid with ID portion of the component tag because it is
very simple and does not require changes to enumeration algorithm.   

Alex.

           

Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Micha Nelissen]

Bounine, Alexandre wrote:
> Micha Nelissen <micha@neli.hopto.org> wrote:
>> I look at it this way: it prevents the need for another layer of
>> indirection: translating component tag to a destid.
> 
> The destid alone is not enough. You will need an entire rio_dev object
> for that device anyway.

?? I did not say a rio_dev object is not needed; on the contrary, I do 
have it.

In various parts of the enumeration and routing algorithm, it would need 
to lookup the tag to find the destid, if the destid is in the tag then 
this lookup is not needed.

> I think we are mixing two things together here. I understand your idea
> but do not see how it prevents me from having one common set of access
> coordinates for RIO devices (the starting point of our discussion). 

I'm trying to argue we do not want redundant identification in the first 
  place unless absolutely necessary.

> Regardless of an implementation, having a way that ensures unified
> identification of switches by all processor boards is better than the
> current mainline implementation. 

Well, we both know "the current mainline implementation" is easily 
improved to unique identification I proposed. Therefore this statement 
is misleading.

> Methods of forming a component tag may
> differ but still serve the same purpose. Personally I prefer to avoid
> any link of device identification to the destid because it may not be as
> intuitive as it seems for large systems with hot-plug. I will discuss
> this with some of RIO TWG guys to get their opinion on the best
> approach.

Please do, please elaborate: "may not be as intuitive" ... to what? for 
implementation of ...?

> I will make a patch that defines fields of component tag, probably just
> one for now - identification field. This will ensure that any method
> used to assign component tag (id part of it) will be compatible with RIO
> spec part 8 error management.

Again slightly misleading information. Any unique identification 
satisfies this requirement.

To prove my point: are you going to recycle the component tags as well 
in case of hot-swaps, just like the destids?

> As for switch identification, at this moment I still prefer replacing
> rswitch->switchid with ID portion of the component tag because it is
> very simple and does not require changes to enumeration algorithm.   

Yes, I agree switchid = tag, that's what I use also.

Micha

RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches

[Bounine, Alexandre]

Micha Nelissen <micha@neli.hopto.org> wrote:
> 
> In various parts of the enumeration and routing algorithm, it would
need
> to lookup the tag to find the destid, if the destid is in the tag then
> this lookup is not needed.

Let's keep this discussion within limits of the current implementation.
Existing method of forming CT may be replaced when that change is
justified by use.
Any method that may be implemented later is good if it ensures unique
identification of RIO devices. At this moment, we just need to define CT
format now to avoid future conflicts of enumeration methods.

> Well, we both know "the current mainline implementation" is easily
> improved to unique identification I proposed. Therefore this statement
> is misleading.

Your improvement will require assigning separate destID to any switch
that has no endpoints attached. I prefer to keep changes to enumeration
for later patches.

Alex.