solos-pci: Fix regression introduced by newest firmware
Subsystems (3)
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.
---
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; }
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
-1dev_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.
David Miller
maintainerFrom: 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.
Quoted 23 lines · David Miller
From: Ben Hutchings<[email protected]>
Date: Mon, 21 Mar 2011 03:01:36 +0000Quoted 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
David Miller
maintainerFrom: 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?
Quoted 6 lines · David Miller
From: Philip Prindeville<[email protected]>
Date: Sun, 20 Mar 2011 22:56:43 -0700Quoted 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.
Quoted 11 lines · Philip Prindeville
Quoted 6 lines · David Miller
From: Philip Prindeville<[email protected]>
Date: Sun, 20 Mar 2011 22:56:43 -0700Quoted 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