Skip to contentSkip to conversation

solos-pci: Fix regression introduced by newest firmware

PATCH In review Philip Prindeville 4 participants 7 messages Mar 21, 2011 – Apr 29, 2011

Subsystems (3)
Networking Network drivers ATM
lore
Share & tools
lore

Philip Prindeville

author
#0 lore raw

PATCH v2 1/1 solos-pci: Fix regression introduced by newest firmware

The newest FPGA firmware on the Solos processors correctly signals carrier transitions, bitrate, etc.

The driver previously ignored these messages, and the physical state was always ATM_PHY_SIG_UNKNOWN.

Now that the board reports its state, we expose a bug whereby the transition from UNKNOWN to LOST causes us to release all VC's.

We don't delete any VC's, but instead just send an indication of carrier change.

Signed-off-by Philip A Prindeville <[email protected]>

---

drivers/atm/solos-pci.c +2 -0
@@ -382,8 +382,10 @@ static int process_status(struct solos_c
 
 	/* Anything but 'Showtime' is down */
 	if (strcmp(state_str, "Showtime")) {
 		atm_dev_signal_change(card->atmdev[port], ATM_PHY_SIG_LOST);
+#if 0
 		atm_dev_release_vccs(card->atmdev[port]);
+#endif
 		dev_info(&card->dev->dev, "Port %d: %s\n", port, state_str);

		return 0;
	}
#1 lore raw
Quoted 18 lines · Philip Prindeville

The newest FPGA firmware on the Solos processors correctly signals carrier transitions, bitrate, etc.

The driver previously ignored these messages, and the physical state was always ATM_PHY_SIG_UNKNOWN.

Now that the board reports its state, we expose a bug whereby the transition from UNKNOWN to LOST causes us to release all VC's.

We don't delete any VC's, but instead just send an indication of carrier change.

Signed-off-by Philip A Prindeville <[email protected]>

---

drivers/atm/solos-pci.c static int process_status(struct solos_c · line 388

Quoted 7 lines
drivers/atm/solos-pci.c +2 -0
@@ -382,8 +382,10 @@ static int process_status(struct solos_c
 
  	/* Anything but 'Showtime' is down */
  	if (strcmp(state_str, "Showtime")) {
  		atm_dev_signal_change(card->atmdev[port], ATM_PHY_SIG_LOST);
+#if 0
  		atm_dev_release_vccs(card->atmdev[port]);
+#endif

Either remove it or don't. #if 0 is for people without version control.

Ben.

Quoted 9 lines
drivers/atm/solos-pci.c +0 -1
  		dev_info(&card->dev->dev, "Port %d: %s\n", port, state_str);
 		return 0;
 	}
 
 
--

To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 3 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
maintainer
#2 lore raw

From: Ben Hutchings <[email protected]>
Date: Mon, 21 Mar 2011 03:01:36 +0000

Quoted 20 lines · Philip Prindeville
Quoted 19 lines

The newest FPGA firmware on the Solos processors correctly signals carrier transitions, bitrate, etc.

The driver previously ignored these messages, and the physical state was always ATM_PHY_SIG_UNKNOWN.

Now that the board reports its state, we expose a bug whereby the transition from UNKNOWN to LOST causes us to release all VC's.

We don't delete any VC's, but instead just send an indication of carrier change.

Signed-off-by Philip A Prindeville <[email protected]>

---

drivers/atm/solos-pci.c +0 -0
@@ -382,8 +382,10 @@ static int process_status(struct solos_c
Quoted 8 lines

drivers/atm/solos-pci.c static int process_status(struct solos_c · line 388

Quoted 6 lines
drivers/atm/solos-pci.c +2 -0
  	/* Anything but 'Showtime' is down */
  	if (strcmp(state_str, "Showtime")) {
  		atm_dev_signal_change(card->atmdev[port], ATM_PHY_SIG_LOST);
+#if 0
  		atm_dev_release_vccs(card->atmdev[port]);
+#endif

Either remove it or don't. #if 0 is for people without version control.

Also, this would seem to break those using the older firmware.

author
#3 lore raw
Quoted 23 lines · David Miller

From: Ben Hutchings<[email protected]>
Date: Mon, 21 Mar 2011 03:01:36 +0000

Quoted 20 lines · Philip Prindeville
Quoted 19 lines

The newest FPGA firmware on the Solos processors correctly signals carrier transitions, bitrate, etc.

The driver previously ignored these messages, and the physical state was always ATM_PHY_SIG_UNKNOWN.

Now that the board reports its state, we expose a bug whereby the transition from UNKNOWN to LOST causes us to release all VC's.

We don't delete any VC's, but instead just send an indication of carrier change.

Signed-off-by Philip A Prindeville<[email protected]>

---

drivers/atm/solos-pci.c +0 -0
@@ -382,8 +382,10 @@ static int process_status(struct solos_c
Quoted 8 lines
Quoted 7 lines

drivers/atm/solos-pci.c static int process_status(struct solos_c · line 388

Quoted 6 lines
drivers/atm/solos-pci.c +2 -0
   	/* Anything but 'Showtime' is down */
   	if (strcmp(state_str, "Showtime")) {
   		atm_dev_signal_change(card->atmdev[port], ATM_PHY_SIG_LOST);
+#if 0
   		atm_dev_release_vccs(card->atmdev[port]);
+#endif

Either remove it or don't. #if 0 is for people without version control.

Also, this would seem to break those using the older firmware.

It's not clear that dropping all VCs abruptly when carrier flapped was ever the right thing to do.

-Philip

maintainer
#4 lore raw

From: Philip Prindeville <[email protected]>
Date: Sun, 20 Mar 2011 22:56:43 -0700

Quoted 2 lines

It's not clear that dropping all VCs abruptly when carrier flapped was ever the right thing to do.

So you've tested your change with the older firmware present?

author
#5 lore raw
Quoted 6 lines · David Miller

From: Philip Prindeville<[email protected]>
Date: Sun, 20 Mar 2011 22:56:43 -0700

Quoted 2 lines

It's not clear that dropping all VCs abruptly when carrier flapped was ever the right thing to do.

So you've tested your change with the older firmware present?

I haven't, no: back-revving firmware has been known to brick cards.

I'm waiting to hear back from Guy and Nathan, they have old cards on hand that they can test.

#6 lore raw
Quoted 11 lines · Philip Prindeville
Quoted 6 lines · David Miller

From: Philip Prindeville<[email protected]>
Date: Sun, 20 Mar 2011 22:56:43 -0700

Quoted 2 lines

It's not clear that dropping all VCs abruptly when carrier flapped was ever the right thing to do.

So you've tested your change with the older firmware present?

I haven't, no: back-revving firmware has been known to brick cards.

I'm waiting to hear back from Guy and Nathan, they have old cards on hand that they can test.

I have JTAG on one of mine and can test, but there's no real need in this case. If you want to simulate the absence of the 'state changed' notification, just *ignore* it when it does arrive.

Signature · 2 lines
David Woodhouse                            Open Source Technology Centre
[email protected]                              Intel Corporation