linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rapidio/tsi721: modify mport name assignment
@ 2012-08-21 14:23 Alexandre Bounine
  2012-08-21 14:23 ` [PATCH] rapidio: apply RX/TX enable to active switch ports only Alexandre Bounine
  2012-08-24 21:02 ` [PATCH] rapidio/tsi721: modify mport name assignment Andrew Morton
  0 siblings, 2 replies; 6+ messages in thread
From: Alexandre Bounine @ 2012-08-21 14:23 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel, linuxppc-dev; +Cc: Alexandre Bounine, Matt Porter

Modify mport device name assignment to provide clear reference to devices
in systems with multiple Tsi721 bridges.

This patch is applicable to kernel versions starting from v3.2.

Signed-off-by: Alexandre Bounine <alexandre.bounine@idt.com>
Cc: Matt Porter <mporter@kernel.crashing.org>
---
 drivers/rapidio/devices/tsi721.c |    3 ++-
 include/linux/rio.h              |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/rapidio/devices/tsi721.c b/drivers/rapidio/devices/tsi721.c
index 5d44252..8533f36 100644
--- a/drivers/rapidio/devices/tsi721.c
+++ b/drivers/rapidio/devices/tsi721.c
@@ -2165,7 +2165,8 @@ static int __devinit tsi721_setup_mport(struct tsi721_device *priv)
 	rio_init_dbell_res(&mport->riores[RIO_DOORBELL_RESOURCE], 0, 0xffff);
 	rio_init_mbox_res(&mport->riores[RIO_INB_MBOX_RESOURCE], 0, 3);
 	rio_init_mbox_res(&mport->riores[RIO_OUTB_MBOX_RESOURCE], 0, 3);
-	strcpy(mport->name, "Tsi721 mport");
+	snprintf(mport->name, RIO_MAX_MPORT_NAME, "%s(%s)",
+		 dev_driver_string(&pdev->dev), dev_name(&pdev->dev));
 
 	/* Hook up interrupt handler */
 
diff --git a/include/linux/rio.h b/include/linux/rio.h
index a90ebad..1a7b6c7 100644
--- a/include/linux/rio.h
+++ b/include/linux/rio.h
@@ -30,6 +30,7 @@
 #define RIO_MAX_MPORTS		8
 #define RIO_MAX_MPORT_RESOURCES	16
 #define RIO_MAX_DEV_RESOURCES	16
+#define RIO_MAX_MPORT_NAME	40
 
 #define RIO_GLOBAL_TABLE	0xff	/* Indicates access of a switch's
 					   global routing table if it
@@ -255,7 +256,7 @@ struct rio_mport {
 				 */
 	enum rio_phy_type phy_type;	/* RapidIO phy type */
 	u32 phys_efptr;
-	unsigned char name[40];
+	unsigned char name[RIO_MAX_MPORT_NAME];
 	void *priv;		/* Master port private data */
 #ifdef CONFIG_RAPIDIO_DMA_ENGINE
 	struct dma_device	dma;
-- 
1.7.8.4


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

* [PATCH] rapidio: apply RX/TX enable to active switch ports only
  2012-08-21 14:23 [PATCH] rapidio/tsi721: modify mport name assignment Alexandre Bounine
@ 2012-08-21 14:23 ` Alexandre Bounine
  2012-08-24 21:04   ` Andrew Morton
  2012-08-24 21:02 ` [PATCH] rapidio/tsi721: modify mport name assignment Andrew Morton
  1 sibling, 1 reply; 6+ messages in thread
From: Alexandre Bounine @ 2012-08-21 14:23 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel, linuxppc-dev; +Cc: Alexandre Bounine, Matt Porter

Modify RIO enumeration to apply RX/TX enable operations only to active
switch ports. This will leave inactive ports in condition consistent with
their state.

This patch is applicable to kernel versions starting from v2.6.35.

Signed-off-by: Alexandre Bounine <alexandre.bounine@idt.com>
Cc: Matt Porter <mporter@kernel.crashing.org>
---
 drivers/rapidio/rio-scan.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/rapidio/rio-scan.c b/drivers/rapidio/rio-scan.c
index 2bebd79..02e686b 100644
--- a/drivers/rapidio/rio-scan.c
+++ b/drivers/rapidio/rio-scan.c
@@ -839,12 +839,10 @@ static int __devinit rio_enum_peer(struct rio_net *net, struct rio_mport *port,
 		for (port_num = 0;
 		     port_num < RIO_GET_TOTAL_PORTS(rdev->swpinfo);
 		     port_num++) {
-			/*Enable Input Output Port (transmitter reviever)*/
-			rio_enable_rx_tx_port(port, 0,
+			if (sw_inport == port_num) {
+				rio_enable_rx_tx_port(port, 0,
 					      RIO_ANY_DESTID(port->sys_size),
 					      hopcount, port_num);
-
-			if (sw_inport == port_num) {
 				rdev->rswitch->port_ok |= (1 << port_num);
 				continue;
 			}
@@ -857,6 +855,9 @@ static int __devinit rio_enum_peer(struct rio_net *net, struct rio_mport *port,
 				pr_debug(
 				    "RIO: scanning device on port %d\n",
 				    port_num);
+				rio_enable_rx_tx_port(port, 0,
+					      RIO_ANY_DESTID(port->sys_size),
+					      hopcount, port_num);
 				rdev->rswitch->port_ok |= (1 << port_num);
 				rio_route_add_entry(rdev, RIO_GLOBAL_TABLE,
 						RIO_ANY_DESTID(port->sys_size),
-- 
1.7.8.4


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

* Re: [PATCH] rapidio/tsi721: modify mport name assignment
  2012-08-21 14:23 [PATCH] rapidio/tsi721: modify mport name assignment Alexandre Bounine
  2012-08-21 14:23 ` [PATCH] rapidio: apply RX/TX enable to active switch ports only Alexandre Bounine
@ 2012-08-24 21:02 ` Andrew Morton
  2012-08-27 14:49   ` Bounine, Alexandre
  1 sibling, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2012-08-24 21:02 UTC (permalink / raw)
  To: Alexandre Bounine; +Cc: linux-kernel, linuxppc-dev, Matt Porter

On Tue, 21 Aug 2012 10:23:54 -0400
Alexandre Bounine <alexandre.bounine@idt.com> wrote:

> Modify mport device name assignment to provide clear reference to devices
> in systems with multiple Tsi721 bridges.
> 
> This patch is applicable to kernel versions starting from v3.2.

This seems to imply that you think the patch should be backported into
earlier kernels.  But no reason for doing this was provided.

> --- a/drivers/rapidio/devices/tsi721.c
> +++ b/drivers/rapidio/devices/tsi721.c
> @@ -2165,7 +2165,8 @@ static int __devinit tsi721_setup_mport(struct tsi721_device *priv)
>  	rio_init_dbell_res(&mport->riores[RIO_DOORBELL_RESOURCE], 0, 0xffff);
>  	rio_init_mbox_res(&mport->riores[RIO_INB_MBOX_RESOURCE], 0, 3);
>  	rio_init_mbox_res(&mport->riores[RIO_OUTB_MBOX_RESOURCE], 0, 3);
> -	strcpy(mport->name, "Tsi721 mport");
> +	snprintf(mport->name, RIO_MAX_MPORT_NAME, "%s(%s)",
> +		 dev_driver_string(&pdev->dev), dev_name(&pdev->dev));

And it's a non-back-compatible change to a userspace-visible interface!
As such we'd need extraordinary justification to merge it into
*future* kernels, let alone backport it.

Please, do provide much better changelogging than this.


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

* Re: [PATCH] rapidio: apply RX/TX enable to active switch ports only
  2012-08-21 14:23 ` [PATCH] rapidio: apply RX/TX enable to active switch ports only Alexandre Bounine
@ 2012-08-24 21:04   ` Andrew Morton
  2012-08-27 13:01     ` Bounine, Alexandre
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2012-08-24 21:04 UTC (permalink / raw)
  To: Alexandre Bounine; +Cc: linux-kernel, linuxppc-dev, Matt Porter

On Tue, 21 Aug 2012 10:23:55 -0400
Alexandre Bounine <alexandre.bounine@idt.com> wrote:

> Modify RIO enumeration to apply RX/TX enable operations only to active
> switch ports. This will leave inactive ports in condition consistent with
> their state.

It's unclear (to me) what the effects of this are.  Does it fix some
user-visible malfunction?  Or is it just some nice-to-have thing?  Or
what?

> This patch is applicable to kernel versions starting from v2.6.35.

Is that a recommendation that such a backport be performed?  If so,
please provide the reasoning.

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

* RE: [PATCH] rapidio: apply RX/TX enable to active switch ports only
  2012-08-24 21:04   ` Andrew Morton
@ 2012-08-27 13:01     ` Bounine, Alexandre
  0 siblings, 0 replies; 6+ messages in thread
From: Bounine, Alexandre @ 2012-08-27 13:01 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linuxppc-dev, linux-kernel

On Friday, August 24, 2012 5:04 PM
Andrew Morton <akpm@linux-foundation.org>
> 
> On Tue, 21 Aug 2012 10:23:55 -0400
> Alexandre Bounine <alexandre.bounine@idt.com> wrote:
> 
>> Modify RIO enumeration to apply RX/TX enable operations only to
>> active switch ports. This will leave inactive ports in condition consistent
>> with their state.
> 
> It's unclear (to me) what the effects of this are.  Does it fix some
> user-visible malfunction?  Or is it just some nice-to-have thing?  Or
> what?
> 

This patch was intended to keep inactive switch ports with inbound and
outbound packet transfers disabled to block unexpected packets during
hot insertion event. While it does not fix any visible malfunction it
was intended to prevent such events in future.   

Since this patch was published it was brought to my attention that some
existing switches may require inbound and outbound ports enabled for
inactive ports as well (not spec compliant).
Therefore, please remove this patch from your tree.     

>> This patch is applicable to kernel versions starting from v2.6.35.
> 
> Is that a recommendation that such a backport be performed?  If so,
> please provide the reasoning.



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

* RE: [PATCH] rapidio/tsi721: modify mport name assignment
  2012-08-24 21:02 ` [PATCH] rapidio/tsi721: modify mport name assignment Andrew Morton
@ 2012-08-27 14:49   ` Bounine, Alexandre
  0 siblings, 0 replies; 6+ messages in thread
From: Bounine, Alexandre @ 2012-08-27 14:49 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, linuxppc-dev, Matt Porter

On Friday, August 24, 2012 5:02 PM
Andrew Morton <akpm@linux-foundation.org>
> 
> On Tue, 21 Aug 2012 10:23:54 -0400
> Alexandre Bounine <alexandre.bounine@idt.com> wrote:
> 
>> Modify mport device name assignment to provide clear reference to devices
>> in systems with multiple Tsi721 bridges.
>>
>> This patch is applicable to kernel versions starting from v3.2.
> 
> This seems to imply that you think the patch should be backported into
> earlier kernels.  But no reason for doing this was provided.

We cannot prevent users of earlier versions from using more than one Tsi721
device in their systems. In that case the old name form will not provide
definitive reference to a particular mport. Please, see comment below.

> 
> > --- a/drivers/rapidio/devices/tsi721.c
> > +++ b/drivers/rapidio/devices/tsi721.c
> > @@ -2165,7 +2165,8 @@ static int __devinit tsi721_setup_mport(struct
> tsi721_device *priv)
> >  	rio_init_dbell_res(&mport->riores[RIO_DOORBELL_RESOURCE], 0,
> 0xffff);
> >  	rio_init_mbox_res(&mport->riores[RIO_INB_MBOX_RESOURCE], 0, 3);
> >  	rio_init_mbox_res(&mport->riores[RIO_OUTB_MBOX_RESOURCE], 0, 3);
> > -	strcpy(mport->name, "Tsi721 mport");
> > +	snprintf(mport->name, RIO_MAX_MPORT_NAME, "%s(%s)",
> > +		 dev_driver_string(&pdev->dev), dev_name(&pdev->dev));
> 
> And it's a non-back-compatible change to a userspace-visible interface!
> As such we'd need extraordinary justification to merge it into
> *future* kernels, let alone backport it.

An mport device do not provide a userspace-visible interface at this point.
This name is used internally in RapidIO subsystem for resource tracking and
debugging. For Tsi721 as a PCIe peripheral, having the mport name that may be
easily associated with its PCIe base helps with tracking of assigned PCIe
resources. This option looks to me better than adding simple indexing
component into existing name format, especially for systems that use add-on
cards with PCIe-to-SRIO bridges.

> 
> Please, do provide much better changelogging than this.

Modify RapidIO mport device name assignment to include device name of PCIe
side of Tsi721 bridge. The new name format is intended to provide definitive
reference between RapidIO and PCIe sides of the bridge in systems with
multiple Tsi721 bridges.



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

end of thread, other threads:[~2012-08-27 14:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-21 14:23 [PATCH] rapidio/tsi721: modify mport name assignment Alexandre Bounine
2012-08-21 14:23 ` [PATCH] rapidio: apply RX/TX enable to active switch ports only Alexandre Bounine
2012-08-24 21:04   ` Andrew Morton
2012-08-27 13:01     ` Bounine, Alexandre
2012-08-24 21:02 ` [PATCH] rapidio/tsi721: modify mport name assignment Andrew Morton
2012-08-27 14:49   ` Bounine, Alexandre

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