firewire: ohci: work around selfID junk due to wrong gap count

If a device's firmware initiates a bus reset by setting the IBR bit in
PHY register 1 without resetting the gap count field to 63 (and without
having sent a PHY configuration packet beforehand), the gap count of
this node will remain at the old value after the bus reset and thus be
inconsistent with the gap count on all other nodes.

The bus manager is supposed to detect the inconsistent gap count values
in the self ID packets and correct them by issuing another bus reset.

However, if the buggy device happens to be the cycle master, and if it
sends a cycle start packet immediately after the bus reset (which is
likely after a long bus reset), then the time between the end of the
selfID phase and the start of the cycle start packet will be based on
the too-small gap count value, so this gap will be too short to be
detected as a subaction gap by the other nodes.  This means that the
cycle start packet will be assumed to be self ID data, and will be
stored after the actual self ID quadlets in the self ID buffer.

This garbage in the self ID buffer made firewire-core ignore all of the
self ID data, and thus prevented the Linux bus manager from correcting
the problem.  Furthermore, because the bus reset handling was aborted
completely, asynchronous transfers would be no longer handled correctly,
and fw_run_transaction() would hang until the next bus reset.

To fix this, make the detection of inconsistent self IDs more
discriminating:  If the invalid data in the self ID buffer looks like
a cycle start packet, we can assume that the previous data in the buffer
is correctly received self ID information, and process it normally.

(We inspect only the first quadlet of the cycle start packet, because
this value is different enough from any valid self ID quadlet, and many
controllers do not store the cycle start packet in five quadlets because
they expect self ID data to have an even number of quadlets.)

This bug has been observed when a bus-powered DesktopKonnekt6 is
switched off with its power button.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

1 files changed, 16 insertions, 2 deletions

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index bffc2ad..b697714 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -1860,8 +1860,22 @@ static void bus_reset_work(struct work_struct *work)
 
 	for (i = 1, j = 0; j < self_id_count; i += 2, j++) {
 		if (ohci->self_id_cpu[i] != ~ohci->self_id_cpu[i + 1]) {
-			fw_notify("inconsistent self IDs\n");
-			return;
+			/*
+			 * If the invalid data looks like a cycle start packet,
+			 * it's likely to be the result of the cycle master
+			 * having a wrong gap count.  In this case, the self IDs
+			 * so far are valid and should be processed so that the
+			 * bus manager can then correct the gap count.
+			 */
+			if (cond_le32_to_cpu(ohci->self_id_cpu[i])
+							== 0xffff008f) {
+				fw_notify("ignoring spurious self IDs\n");
+				self_id_count = j;
+				break;
+			} else {
+				fw_notify("inconsistent self IDs\n");
+				return;
+			}
 		}
 		ohci->self_id_buffer[j] =
 				cond_le32_to_cpu(ohci->self_id_cpu[i]);