Skip to contentSkip to conversation

thunderbolt: Retry DROM read once if parsing fails

PATCH Applied Mika Westerberg 2 participants 4 messages Sep 7, 2020 – Sep 9, 2020

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 thunderbolt: Retry DROM read once if parsing fails

Kai-Heng reported that sometimes DROM parsing of ASUS PA27AC Thunderbolt 3 monitor fails. This makes the driver to fail to add the device so only DisplayPort tunneling is functional.

It is not clear what exactly happens but waiting for 100 ms and retrying the read seems to work this around so we do that here.

---
 drivers/thunderbolt/eeprom.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
drivers/thunderbolt/eeprom.c +16 -4
index 3ebca44ab3fa..0c8471be3e32 100644
@@ -7,6 +7,7 @@
  */
 
 #include <linux/crc32.h>
+#include <linux/delay.h>
 #include <linux/property.h>
 #include <linux/slab.h>
 #include "tb.h"
@@ -389,8 +390,8 @@ static int tb_drom_parse_entries(struct tb_switch *sw)
 		struct tb_drom_entry_header *entry = (void *) (sw->drom + pos);
 		if (pos + 1 == drom_size || pos + entry->len > drom_size
 				|| !entry->len) {
-			tb_sw_warn(sw, "drom buffer overrun, aborting\n");
-			return -EIO;
+			tb_sw_warn(sw, "DROM buffer overrun\n");
+			return -EILSEQ;
 		}
 
 		switch (entry->type) {
@@ -526,7 +527,8 @@ int tb_drom_read(struct tb_switch *sw)
 	u16 size;
 	u32 crc;
 	struct tb_drom_header *header;
-	int res;
+	int res, retries = 1;
+
 	if (sw->drom)
 		return 0;
 
@@ -612,7 +614,17 @@ int tb_drom_read(struct tb_switch *sw)
 		tb_sw_warn(sw, "drom device_rom_revision %#x unknown\n",
 			header->device_rom_revision);
 
-	return tb_drom_parse_entries(sw);
+	res = tb_drom_parse_entries(sw);
+	/* If the DROM parsing fails, wait a moment and retry once */
+	if (res == -EILSEQ && retries--) {
+		tb_sw_warn(sw, "parsing DROM failed, retrying\n");
+		msleep(100);
+		res = tb_drom_read_n(sw, 0, sw->drom, size);
+		if (!res)
+			goto parse;
+	}
+
+	return res;
 err:
 	kfree(sw->drom);
 	sw->drom = NULL;
Signature · 1 line
2.28.0
#1 lore raw
Quoted 3 lines · Mika Westerberg

Kai-Heng reported that sometimes DROM parsing of ASUS PA27AC Thunderbolt 3 monitor fails. This makes the driver to fail to add the device so only DisplayPort tunneling is functional.

Maybe the DROM is contained in an external EEPROM attached via slow SPI or i2c and cannot be read in time before a response on the control channel is due? Does it help to always delay 100 ms before performing the DROM read with this display? Perhaps a quirk specific for this display is more appropriate than a change which affects all devices? Because this way of working around problems is difficult to maintain long term.

Thanks,

Lukas

#2 lore raw
Quoted 5 lines · Lukas Wunner
Quoted 3 lines · Mika Westerberg

Kai-Heng reported that sometimes DROM parsing of ASUS PA27AC Thunderbolt 3 monitor fails. This makes the driver to fail to add the device so only DisplayPort tunneling is functional.

Quoted 7 lines

Maybe the DROM is contained in an external EEPROM attached via slow SPI or i2c and cannot be read in time before a response on the control channel is due? Does it help to always delay 100 ms before performing the DROM read with this display? Perhaps a quirk specific for this display is more appropriate than a change which affects all devices? Because this way of working around problems is difficult to maintain long term.

It helps to always wait the 100ms IIRC but I'm not sure we want to do that for every device. Also I'm not a fan of maintaining a list of quirks if we have the possibility to have more "general" solution to the issue (like what we do here, retry once).

author Applied
#3 lore raw
In reply to Mika Westerberg
Quoted 4 lines · Mika Westerberg

Kai-Heng reported that sometimes DROM parsing of ASUS PA27AC Thunderbolt 3 monitor fails. This makes the driver to fail to add the device so only DisplayPort tunneling is functional.

Quoted 8 lines

It is not clear what exactly happens but waiting for 100 ms and retrying the read seems to work this around so we do that here.

Applied to thunderbolt.git/fixes.