[PATCH v3 0/3] Fix problems fetching TBT3 DROM from AMD USB4 routers

[PATCH v3 0/3] Fix problems fetching TBT3 DROM from AMD USB4 routers

[Mario Limonciello]

TBT3 devices when connected to an AMD USB4 router occasionally fail to
properly respond to requests for the DROM via bit banging.

Depending upon which part of the request failed will impact the severity.
A number of workarounds have been put in place to let the driver handle
the failed requests:

commit e87491a9fd4e3 ("thunderbolt: Retry DROM reads for more failure scenarios")
commit a283de3ec646f ("thunderbolt: Do not resume routers if UID is not set")
commit 6915812bbd109 ("thunderbolt: Do not make DROM read success compulsory")
commit f022ff7bf377 ("thunderbolt: Retry DROM read once if parsing fails")

Still even with these changes the failures do make it through. In comparing
other CM implementations utilized on AMD systems, they all access the
DROM directly from the NVM.

To avoid triggering this issue, try to get the DROM directly from the NVM
in Linux as well when devices have an LC.

Mario Limonciello (3):
  thunderbolt: Adjust how NVM reading works
  thunderbolt: use `tb_eeprom_get_drom_offset` to discover DROM offset
  thunderbolt: Refactor DROM reading

 drivers/thunderbolt/eeprom.c | 150 +++++++++++++++++++----------------
 1 file changed, 82 insertions(+), 68 deletions(-)

-- 
2.34.1

[PATCH v3 1/3] thunderbolt: Adjust how NVM reading works

[Mario Limonciello]

Some TBT3 devices have a hard time reliably responding to bit banging
requests correctly when connected to AMD USB4 hosts running Linux.

These problems are not reported in any other CM supported on AMD platforms,
and comparing the Windows and Pre-OS implementations the Linux CM is the
only one that utilizes bit banging to access the DROM.
Other CM implementations access the DROM directly from the NVM instead of
bit banging.

Adjust the flow to use this method to fetch the NVM when the downstream
device has an LC that can be used to fetch the UUID later. The bit banging
method will only be used if this has failed or no LC is present.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2->v3:
    * Split out refactor
v1->v2:
    * Update commit message to indicate which CMs are tested
    * Adjust flow to only fetch DROM from NVM on TBT3 and bit bang on TBT1/2
---
 drivers/thunderbolt/eeprom.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
index c90d22f56d4e..177bc51316f7 100644
--- a/drivers/thunderbolt/eeprom.c
+++ b/drivers/thunderbolt/eeprom.c
@@ -640,6 +640,10 @@ int tb_drom_read(struct tb_switch *sw)
 		return 0;
 	}
 
+	/* We can use LC to get UUID later */
+	if (sw->cap_lc && tb_drom_copy_nvm(sw, &size) == 0)
+		goto parse;
+
 	res = tb_drom_read_n(sw, 14, (u8 *) &size, 2);
 	if (res)
 		return res;
-- 
2.34.1

[PATCH v3 2/3] thunderbolt: use `tb_eeprom_get_drom_offset` to discover DROM offset

[Mario Limonciello]

The static function `tb_eeprom_get_drom_offset` has more safety guards
for the DROM offset fetching.  Use this instead of just `tb_sw_read`

No intended functional changes.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/thunderbolt/eeprom.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
index 177bc51316f7..a326cf16ca3d 100644
--- a/drivers/thunderbolt/eeprom.c
+++ b/drivers/thunderbolt/eeprom.c
@@ -471,14 +471,13 @@ static int tb_drom_copy_efi(struct tb_switch *sw, u16 *size)
 
 static int tb_drom_copy_nvm(struct tb_switch *sw, u16 *size)
 {
-	u32 drom_offset;
+	u16 drom_offset;
 	int ret;
 
 	if (!sw->dma_port)
 		return -ENODEV;
 
-	ret = tb_sw_read(sw, &drom_offset, TB_CFG_SWITCH,
-			 sw->cap_plug_events + 12, 1);
+	ret = tb_eeprom_get_drom_offset(sw, &drom_offset);
 	if (ret)
 		return ret;
 
-- 
2.34.1

[PATCH v3 3/3] thunderbolt: Refactor DROM reading

[Mario Limonciello]

The NVM reading code has a series of gotos that potentially introduce
unexpected behaviors with retries if something unexpected has failed
to parse.

Additionally the retry logic introduced in commit f022ff7bf377
("thunderbolt: Retry DROM read once if parsing fails") was added from
failures in bit banging, which aren't expected to be present when the
DROM is fetched directly from the NVM.

Refactor the code to remove the gotos and drop the retry logic.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2->v3:
    * Split out refactor
---
 drivers/thunderbolt/eeprom.c | 147 +++++++++++++++++++----------------
 1 file changed, 79 insertions(+), 68 deletions(-)

diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
index a326cf16ca3d..2a078c69f0d2 100644
--- a/drivers/thunderbolt/eeprom.c
+++ b/drivers/thunderbolt/eeprom.c
@@ -416,7 +416,7 @@ static int tb_drom_parse_entries(struct tb_switch *sw, size_t header_size)
 		if (pos + 1 == drom_size || pos + entry->len > drom_size
 				|| !entry->len) {
 			tb_sw_warn(sw, "DROM buffer overrun\n");
-			return -EILSEQ;
+			return -EIO;
 		}
 
 		switch (entry->type) {
@@ -543,7 +543,37 @@ static int tb_drom_read_n(struct tb_switch *sw, u16 offset, u8 *val,
 	return tb_eeprom_read_n(sw, offset, val, count);
 }
 
-static int tb_drom_parse(struct tb_switch *sw)
+static int tb_drom_bit_bang(struct tb_switch *sw, u16 *size)
+{
+	int res;
+
+	res = tb_drom_read_n(sw, 14, (u8 *) size, 2);
+	if (res)
+		return res;
+	*size &= 0x3ff;
+	*size += TB_DROM_DATA_START;
+	tb_sw_dbg(sw, "reading drom (length: %#x)\n", *size);
+	if (*size < sizeof(struct tb_drom_header)) {
+		tb_sw_warn(sw, "drom too small, aborting\n");
+		return -EIO;
+	}
+
+	sw->drom = kzalloc(*size, GFP_KERNEL);
+	if (!sw->drom)
+		return -ENOMEM;
+
+	res = tb_drom_read_n(sw, 0, sw->drom, *size);
+	if (res)
+		goto err;
+
+	return 0;
+err:
+	kfree(sw->drom);
+	sw->drom = NULL;
+	return res;
+}
+
+static int tb_drom_parse_v1(struct tb_switch *sw)
 {
 	const struct tb_drom_header *header =
 		(const struct tb_drom_header *)sw->drom;
@@ -554,7 +584,7 @@ static int tb_drom_parse(struct tb_switch *sw)
 		tb_sw_warn(sw,
 			"DROM UID CRC8 mismatch (expected: %#x, got: %#x)\n",
 			header->uid_crc8, crc);
-		return -EILSEQ;
+		return -EIO;
 	}
 	if (!sw->uid)
 		sw->uid = header->uid;
@@ -588,6 +618,43 @@ static int usb4_drom_parse(struct tb_switch *sw)
 	return tb_drom_parse_entries(sw, USB4_DROM_HEADER_SIZE);
 }
 
+static int tb_drom_parse(struct tb_switch *sw, u16 *size)
+{
+	struct tb_drom_header *header = (void *) sw->drom;
+	int res;
+
+	if (header->data_len + TB_DROM_DATA_START != *size) {
+		tb_sw_warn(sw, "drom size mismatch\n");
+		goto err;
+	}
+
+	tb_sw_dbg(sw, "DROM version: %d\n", header->device_rom_revision);
+
+	switch (header->device_rom_revision) {
+	case 3:
+		res = usb4_drom_parse(sw);
+		break;
+	default:
+		tb_sw_warn(sw, "DROM device_rom_revision %#x unknown\n",
+			   header->device_rom_revision);
+		fallthrough;
+	case 1:
+		res = tb_drom_parse_v1(sw);
+		break;
+	}
+
+	if (res) {
+		tb_sw_warn(sw, "parsing DROM failed\n");
+		goto err;
+	}
+
+	return 0;
+err:
+	kfree(sw->drom);
+	sw->drom = NULL;
+	return -EIO;
+}
+
 /**
  * tb_drom_read() - Copy DROM to sw->drom and parse it
  * @sw: Router whose DROM to read and parse
@@ -601,8 +668,7 @@ static int usb4_drom_parse(struct tb_switch *sw)
 int tb_drom_read(struct tb_switch *sw)
 {
 	u16 size;
-	struct tb_drom_header *header;
-	int res, retries = 1;
+	int res;
 
 	if (sw->drom)
 		return 0;
@@ -613,11 +679,11 @@ int tb_drom_read(struct tb_switch *sw)
 		 * in a device property. Use it if available.
 		 */
 		if (tb_drom_copy_efi(sw, &size) == 0)
-			goto parse;
+			return tb_drom_parse(sw, &size);
 
 		/* Non-Apple hardware has the DROM as part of NVM */
 		if (tb_drom_copy_nvm(sw, &size) == 0)
-			goto parse;
+			return tb_drom_parse(sw, &size);
 
 		/*
 		 * USB4 hosts may support reading DROM through router
@@ -626,7 +692,7 @@ int tb_drom_read(struct tb_switch *sw)
 		if (tb_switch_is_usb4(sw)) {
 			usb4_switch_read_uid(sw, &sw->uid);
 			if (!usb4_copy_host_drom(sw, &size))
-				goto parse;
+				return tb_drom_parse(sw, &size);
 		} else {
 			/*
 			 * The root switch contains only a dummy drom
@@ -640,67 +706,12 @@ int tb_drom_read(struct tb_switch *sw)
 	}
 
 	/* We can use LC to get UUID later */
-	if (sw->cap_lc && tb_drom_copy_nvm(sw, &size) == 0)
-		goto parse;
-
-	res = tb_drom_read_n(sw, 14, (u8 *) &size, 2);
+	if (sw->cap_lc)
+		res = tb_drom_copy_nvm(sw, &size);
+	else
+		res = tb_drom_bit_bang(sw, &size);
 	if (res)
 		return res;
-	size &= 0x3ff;
-	size += TB_DROM_DATA_START;
-	tb_sw_dbg(sw, "reading drom (length: %#x)\n", size);
-	if (size < sizeof(*header)) {
-		tb_sw_warn(sw, "drom too small, aborting\n");
-		return -EIO;
-	}
-
-	sw->drom = kzalloc(size, GFP_KERNEL);
-	if (!sw->drom)
-		return -ENOMEM;
-read:
-	res = tb_drom_read_n(sw, 0, sw->drom, size);
-	if (res)
-		goto err;
-
-parse:
-	header = (void *) sw->drom;
-
-	if (header->data_len + TB_DROM_DATA_START != size) {
-		tb_sw_warn(sw, "drom size mismatch\n");
-		if (retries--) {
-			msleep(100);
-			goto read;
-		}
-		goto err;
-	}
-
-	tb_sw_dbg(sw, "DROM version: %d\n", header->device_rom_revision);
-
-	switch (header->device_rom_revision) {
-	case 3:
-		res = usb4_drom_parse(sw);
-		break;
-	default:
-		tb_sw_warn(sw, "DROM device_rom_revision %#x unknown\n",
-			   header->device_rom_revision);
-		fallthrough;
-	case 1:
-		res = tb_drom_parse(sw);
-		break;
-	}
-
-	/* If the DROM parsing fails, wait a moment and retry once */
-	if (res == -EILSEQ && retries--) {
-		tb_sw_warn(sw, "parsing DROM failed\n");
-		msleep(100);
-		goto read;
-	}
 
-	if (!res)
-		return 0;
-
-err:
-	kfree(sw->drom);
-	sw->drom = NULL;
-	return -EIO;
+	return tb_drom_parse(sw, &size);
 }
-- 
2.34.1

Re: [PATCH v3 3/3] thunderbolt: Refactor DROM reading

[Mika Westerberg]

Hi Mario,

On Thu, Feb 16, 2023 at 02:19:10PM -0600, Mario Limonciello wrote:
> The NVM reading code has a series of gotos that potentially introduce
> unexpected behaviors with retries if something unexpected has failed
> to parse.
> 
> Additionally the retry logic introduced in commit f022ff7bf377
> ("thunderbolt: Retry DROM read once if parsing fails") was added from
> failures in bit banging, which aren't expected to be present when the
> DROM is fetched directly from the NVM.

Okay that's why you remove the EILSEQ returns below, right?

> Refactor the code to remove the gotos and drop the retry logic.

Thanks for doing this. Few minor stylistic comments below. I can also
fix these myself when applying if you prefer.

Note I will be on vacation next week but will be picking up patches once
v6.3-rc1 is released.

> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v2->v3:
>     * Split out refactor
> ---
>  drivers/thunderbolt/eeprom.c | 147 +++++++++++++++++++----------------
>  1 file changed, 79 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
> index a326cf16ca3d..2a078c69f0d2 100644
> --- a/drivers/thunderbolt/eeprom.c
> +++ b/drivers/thunderbolt/eeprom.c
> @@ -416,7 +416,7 @@ static int tb_drom_parse_entries(struct tb_switch *sw, size_t header_size)
>  		if (pos + 1 == drom_size || pos + entry->len > drom_size
>  				|| !entry->len) {
>  			tb_sw_warn(sw, "DROM buffer overrun\n");
> -			return -EILSEQ;
> +			return -EIO;
>  		}
>  
>  		switch (entry->type) {
> @@ -543,7 +543,37 @@ static int tb_drom_read_n(struct tb_switch *sw, u16 offset, u8 *val,
>  	return tb_eeprom_read_n(sw, offset, val, count);
>  }
>  
> -static int tb_drom_parse(struct tb_switch *sw)
> +static int tb_drom_bit_bang(struct tb_switch *sw, u16 *size)
> +{
> +	int res;

ret

> +
> +	res = tb_drom_read_n(sw, 14, (u8 *) size, 2);
> +	if (res)
> +		return res;

empty line here.

> +	*size &= 0x3ff;
> +	*size += TB_DROM_DATA_START;

here too.

> +	tb_sw_dbg(sw, "reading drom (length: %#x)\n", *size);
> +	if (*size < sizeof(struct tb_drom_header)) {
> +		tb_sw_warn(sw, "drom too small, aborting\n");

DROM

> +		return -EIO;
> +	}
> +
> +	sw->drom = kzalloc(*size, GFP_KERNEL);
> +	if (!sw->drom)
> +		return -ENOMEM;
> +
> +	res = tb_drom_read_n(sw, 0, sw->drom, *size);
> +	if (res)
> +		goto err;
> +
> +	return 0;

empty line

> +err:
> +	kfree(sw->drom);
> +	sw->drom = NULL;

empty line

> +	return res;
> +}
> +
> +static int tb_drom_parse_v1(struct tb_switch *sw)
>  {
>  	const struct tb_drom_header *header =
>  		(const struct tb_drom_header *)sw->drom;
> @@ -554,7 +584,7 @@ static int tb_drom_parse(struct tb_switch *sw)
>  		tb_sw_warn(sw,
>  			"DROM UID CRC8 mismatch (expected: %#x, got: %#x)\n",
>  			header->uid_crc8, crc);
> -		return -EILSEQ;
> +		return -EIO;
>  	}
>  	if (!sw->uid)
>  		sw->uid = header->uid;
> @@ -588,6 +618,43 @@ static int usb4_drom_parse(struct tb_switch *sw)
>  	return tb_drom_parse_entries(sw, USB4_DROM_HEADER_SIZE);
>  }
>  
> +static int tb_drom_parse(struct tb_switch *sw, u16 *size)
> +{
> +	struct tb_drom_header *header = (void *) sw->drom;
> +	int res;

ret

> +
> +	if (header->data_len + TB_DROM_DATA_START != *size) {
> +		tb_sw_warn(sw, "drom size mismatch\n");

DROM

> +		goto err;
> +	}
> +
> +	tb_sw_dbg(sw, "DROM version: %d\n", header->device_rom_revision);
> +
> +	switch (header->device_rom_revision) {
> +	case 3:
> +		res = usb4_drom_parse(sw);
> +		break;
> +	default:
> +		tb_sw_warn(sw, "DROM device_rom_revision %#x unknown\n",
> +			   header->device_rom_revision);
> +		fallthrough;
> +	case 1:
> +		res = tb_drom_parse_v1(sw);
> +		break;
> +	}
> +
> +	if (res) {
> +		tb_sw_warn(sw, "parsing DROM failed\n");
> +		goto err;
> +	}
> +
> +	return 0;

empty line

> +err:
> +	kfree(sw->drom);
> +	sw->drom = NULL;

empty line

> +	return -EIO;
> +}
> +
>  /**
>   * tb_drom_read() - Copy DROM to sw->drom and parse it
>   * @sw: Router whose DROM to read and parse
> @@ -601,8 +668,7 @@ static int usb4_drom_parse(struct tb_switch *sw)
>  int tb_drom_read(struct tb_switch *sw)
>  {
>  	u16 size;
> -	struct tb_drom_header *header;
> -	int res, retries = 1;
> +	int res;
>  
>  	if (sw->drom)
>  		return 0;
> @@ -613,11 +679,11 @@ int tb_drom_read(struct tb_switch *sw)
>  		 * in a device property. Use it if available.
>  		 */
>  		if (tb_drom_copy_efi(sw, &size) == 0)
> -			goto parse;
> +			return tb_drom_parse(sw, &size);
>  
>  		/* Non-Apple hardware has the DROM as part of NVM */
>  		if (tb_drom_copy_nvm(sw, &size) == 0)
> -			goto parse;
> +			return tb_drom_parse(sw, &size);
>  
>  		/*
>  		 * USB4 hosts may support reading DROM through router
> @@ -626,7 +692,7 @@ int tb_drom_read(struct tb_switch *sw)
>  		if (tb_switch_is_usb4(sw)) {
>  			usb4_switch_read_uid(sw, &sw->uid);
>  			if (!usb4_copy_host_drom(sw, &size))
> -				goto parse;
> +				return tb_drom_parse(sw, &size);
>  		} else {
>  			/*
>  			 * The root switch contains only a dummy drom
> @@ -640,67 +706,12 @@ int tb_drom_read(struct tb_switch *sw)
>  	}
>  
>  	/* We can use LC to get UUID later */
> -	if (sw->cap_lc && tb_drom_copy_nvm(sw, &size) == 0)
> -		goto parse;
> -
> -	res = tb_drom_read_n(sw, 14, (u8 *) &size, 2);
> +	if (sw->cap_lc)
> +		res = tb_drom_copy_nvm(sw, &size);
> +	else
> +		res = tb_drom_bit_bang(sw, &size);
>  	if (res)
>  		return res;
> -	size &= 0x3ff;
> -	size += TB_DROM_DATA_START;
> -	tb_sw_dbg(sw, "reading drom (length: %#x)\n", size);
> -	if (size < sizeof(*header)) {
> -		tb_sw_warn(sw, "drom too small, aborting\n");
> -		return -EIO;
> -	}
> -
> -	sw->drom = kzalloc(size, GFP_KERNEL);
> -	if (!sw->drom)
> -		return -ENOMEM;
> -read:
> -	res = tb_drom_read_n(sw, 0, sw->drom, size);
> -	if (res)
> -		goto err;
> -
> -parse:
> -	header = (void *) sw->drom;
> -
> -	if (header->data_len + TB_DROM_DATA_START != size) {
> -		tb_sw_warn(sw, "drom size mismatch\n");
> -		if (retries--) {
> -			msleep(100);
> -			goto read;
> -		}
> -		goto err;
> -	}
> -
> -	tb_sw_dbg(sw, "DROM version: %d\n", header->device_rom_revision);
> -
> -	switch (header->device_rom_revision) {
> -	case 3:
> -		res = usb4_drom_parse(sw);
> -		break;
> -	default:
> -		tb_sw_warn(sw, "DROM device_rom_revision %#x unknown\n",
> -			   header->device_rom_revision);
> -		fallthrough;
> -	case 1:
> -		res = tb_drom_parse(sw);
> -		break;
> -	}
> -
> -	/* If the DROM parsing fails, wait a moment and retry once */
> -	if (res == -EILSEQ && retries--) {
> -		tb_sw_warn(sw, "parsing DROM failed\n");
> -		msleep(100);
> -		goto read;
> -	}
>  
> -	if (!res)
> -		return 0;
> -
> -err:
> -	kfree(sw->drom);
> -	sw->drom = NULL;
> -	return -EIO;
> +	return tb_drom_parse(sw, &size);
>  }
> -- 
> 2.34.1

Re: [PATCH v3 3/3] thunderbolt: Refactor DROM reading

[Mario Limonciello]

On 2/17/23 07:36, Mika Westerberg wrote:
> Hi Mario,
>
> On Thu, Feb 16, 2023 at 02:19:10PM -0600, Mario Limonciello wrote:
>> The NVM reading code has a series of gotos that potentially introduce
>> unexpected behaviors with retries if something unexpected has failed
>> to parse.
>>
>> Additionally the retry logic introduced in commit f022ff7bf377
>> ("thunderbolt: Retry DROM read once if parsing fails") was added from
>> failures in bit banging, which aren't expected to be present when the
>> DROM is fetched directly from the NVM.
> Okay that's why you remove the EILSEQ returns below, right?


Yep, that's right.


>> Refactor the code to remove the gotos and drop the retry logic.
> Thanks for doing this.


Sure.


> Few minor stylistic comments below. I can also
> fix these myself when applying if you prefer.


If there is any other feedback on the other patches in the series from 
you or anyone else before 6.3-rc1 I'll send out a v4 fixup the style 
stuff outlined below.

Otherwise, I'll take up your offer thanks for that.


> Note I will be on vacation next week but will be picking up patches once
> v6.3-rc1 is released.

OK, appreciated.


>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> v2->v3:
>>      * Split out refactor
>> ---
>>   drivers/thunderbolt/eeprom.c | 147 +++++++++++++++++++----------------
>>   1 file changed, 79 insertions(+), 68 deletions(-)
>>
>> diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
>> index a326cf16ca3d..2a078c69f0d2 100644
>> --- a/drivers/thunderbolt/eeprom.c
>> +++ b/drivers/thunderbolt/eeprom.c
>> @@ -416,7 +416,7 @@ static int tb_drom_parse_entries(struct tb_switch *sw, size_t header_size)
>>   		if (pos + 1 == drom_size || pos + entry->len > drom_size
>>   				|| !entry->len) {
>>   			tb_sw_warn(sw, "DROM buffer overrun\n");
>> -			return -EILSEQ;
>> +			return -EIO;
>>   		}
>>   
>>   		switch (entry->type) {
>> @@ -543,7 +543,37 @@ static int tb_drom_read_n(struct tb_switch *sw, u16 offset, u8 *val,
>>   	return tb_eeprom_read_n(sw, offset, val, count);
>>   }
>>   
>> -static int tb_drom_parse(struct tb_switch *sw)
>> +static int tb_drom_bit_bang(struct tb_switch *sw, u16 *size)
>> +{
>> +	int res;
> ret
>
>> +
>> +	res = tb_drom_read_n(sw, 14, (u8 *) size, 2);
>> +	if (res)
>> +		return res;
> empty line here.
>
>> +	*size &= 0x3ff;
>> +	*size += TB_DROM_DATA_START;
> here too.
>
>> +	tb_sw_dbg(sw, "reading drom (length: %#x)\n", *size);
>> +	if (*size < sizeof(struct tb_drom_header)) {
>> +		tb_sw_warn(sw, "drom too small, aborting\n");
> DROM
>
>> +		return -EIO;
>> +	}
>> +
>> +	sw->drom = kzalloc(*size, GFP_KERNEL);
>> +	if (!sw->drom)
>> +		return -ENOMEM;
>> +
>> +	res = tb_drom_read_n(sw, 0, sw->drom, *size);
>> +	if (res)
>> +		goto err;
>> +
>> +	return 0;
> empty line
>
>> +err:
>> +	kfree(sw->drom);
>> +	sw->drom = NULL;
> empty line
>
>> +	return res;
>> +}
>> +
>> +static int tb_drom_parse_v1(struct tb_switch *sw)
>>   {
>>   	const struct tb_drom_header *header =
>>   		(const struct tb_drom_header *)sw->drom;
>> @@ -554,7 +584,7 @@ static int tb_drom_parse(struct tb_switch *sw)
>>   		tb_sw_warn(sw,
>>   			"DROM UID CRC8 mismatch (expected: %#x, got: %#x)\n",
>>   			header->uid_crc8, crc);
>> -		return -EILSEQ;
>> +		return -EIO;
>>   	}
>>   	if (!sw->uid)
>>   		sw->uid = header->uid;
>> @@ -588,6 +618,43 @@ static int usb4_drom_parse(struct tb_switch *sw)
>>   	return tb_drom_parse_entries(sw, USB4_DROM_HEADER_SIZE);
>>   }
>>   
>> +static int tb_drom_parse(struct tb_switch *sw, u16 *size)
>> +{
>> +	struct tb_drom_header *header = (void *) sw->drom;
>> +	int res;
> ret
>
>> +
>> +	if (header->data_len + TB_DROM_DATA_START != *size) {
>> +		tb_sw_warn(sw, "drom size mismatch\n");
> DROM
>
>> +		goto err;
>> +	}
>> +
>> +	tb_sw_dbg(sw, "DROM version: %d\n", header->device_rom_revision);
>> +
>> +	switch (header->device_rom_revision) {
>> +	case 3:
>> +		res = usb4_drom_parse(sw);
>> +		break;
>> +	default:
>> +		tb_sw_warn(sw, "DROM device_rom_revision %#x unknown\n",
>> +			   header->device_rom_revision);
>> +		fallthrough;
>> +	case 1:
>> +		res = tb_drom_parse_v1(sw);
>> +		break;
>> +	}
>> +
>> +	if (res) {
>> +		tb_sw_warn(sw, "parsing DROM failed\n");
>> +		goto err;
>> +	}
>> +
>> +	return 0;
> empty line
>
>> +err:
>> +	kfree(sw->drom);
>> +	sw->drom = NULL;
> empty line
>
>> +	return -EIO;
>> +}
>> +
>>   /**
>>    * tb_drom_read() - Copy DROM to sw->drom and parse it
>>    * @sw: Router whose DROM to read and parse
>> @@ -601,8 +668,7 @@ static int usb4_drom_parse(struct tb_switch *sw)
>>   int tb_drom_read(struct tb_switch *sw)
>>   {
>>   	u16 size;
>> -	struct tb_drom_header *header;
>> -	int res, retries = 1;
>> +	int res;
>>   
>>   	if (sw->drom)
>>   		return 0;
>> @@ -613,11 +679,11 @@ int tb_drom_read(struct tb_switch *sw)
>>   		 * in a device property. Use it if available.
>>   		 */
>>   		if (tb_drom_copy_efi(sw, &size) == 0)
>> -			goto parse;
>> +			return tb_drom_parse(sw, &size);
>>   
>>   		/* Non-Apple hardware has the DROM as part of NVM */
>>   		if (tb_drom_copy_nvm(sw, &size) == 0)
>> -			goto parse;
>> +			return tb_drom_parse(sw, &size);
>>   
>>   		/*
>>   		 * USB4 hosts may support reading DROM through router
>> @@ -626,7 +692,7 @@ int tb_drom_read(struct tb_switch *sw)
>>   		if (tb_switch_is_usb4(sw)) {
>>   			usb4_switch_read_uid(sw, &sw->uid);
>>   			if (!usb4_copy_host_drom(sw, &size))
>> -				goto parse;
>> +				return tb_drom_parse(sw, &size);
>>   		} else {
>>   			/*
>>   			 * The root switch contains only a dummy drom
>> @@ -640,67 +706,12 @@ int tb_drom_read(struct tb_switch *sw)
>>   	}
>>   
>>   	/* We can use LC to get UUID later */
>> -	if (sw->cap_lc && tb_drom_copy_nvm(sw, &size) == 0)
>> -		goto parse;
>> -
>> -	res = tb_drom_read_n(sw, 14, (u8 *) &size, 2);
>> +	if (sw->cap_lc)
>> +		res = tb_drom_copy_nvm(sw, &size);
>> +	else
>> +		res = tb_drom_bit_bang(sw, &size);
>>   	if (res)
>>   		return res;
>> -	size &= 0x3ff;
>> -	size += TB_DROM_DATA_START;
>> -	tb_sw_dbg(sw, "reading drom (length: %#x)\n", size);
>> -	if (size < sizeof(*header)) {
>> -		tb_sw_warn(sw, "drom too small, aborting\n");
>> -		return -EIO;
>> -	}
>> -
>> -	sw->drom = kzalloc(size, GFP_KERNEL);
>> -	if (!sw->drom)
>> -		return -ENOMEM;
>> -read:
>> -	res = tb_drom_read_n(sw, 0, sw->drom, size);
>> -	if (res)
>> -		goto err;
>> -
>> -parse:
>> -	header = (void *) sw->drom;
>> -
>> -	if (header->data_len + TB_DROM_DATA_START != size) {
>> -		tb_sw_warn(sw, "drom size mismatch\n");
>> -		if (retries--) {
>> -			msleep(100);
>> -			goto read;
>> -		}
>> -		goto err;
>> -	}
>> -
>> -	tb_sw_dbg(sw, "DROM version: %d\n", header->device_rom_revision);
>> -
>> -	switch (header->device_rom_revision) {
>> -	case 3:
>> -		res = usb4_drom_parse(sw);
>> -		break;
>> -	default:
>> -		tb_sw_warn(sw, "DROM device_rom_revision %#x unknown\n",
>> -			   header->device_rom_revision);
>> -		fallthrough;
>> -	case 1:
>> -		res = tb_drom_parse(sw);
>> -		break;
>> -	}
>> -
>> -	/* If the DROM parsing fails, wait a moment and retry once */
>> -	if (res == -EILSEQ && retries--) {
>> -		tb_sw_warn(sw, "parsing DROM failed\n");
>> -		msleep(100);
>> -		goto read;
>> -	}
>>   
>> -	if (!res)
>> -		return 0;
>> -
>> -err:
>> -	kfree(sw->drom);
>> -	sw->drom = NULL;
>> -	return -EIO;
>> +	return tb_drom_parse(sw, &size);
>>   }
>> -- 
>> 2.34.1