thunderbolt: Refactor DROM reading

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

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

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
[mw: renamed root switch to host router, split device handling too]
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
index 3b96a55295a0db7e84f04fe3e84b8de8c1f955ad..0f6099c18a9415b9c58edc4acac470d4aed421eb 100644 (file)
--- 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) {
@@ -512,7 +512,7 @@ err_free:
        return ret;
 }
 
-static int usb4_copy_host_drom(struct tb_switch *sw, u16 *size)
+static int usb4_copy_drom(struct tb_switch *sw, u16 *size)
 {
        int ret;
 
@@ -535,15 +535,40 @@ static int usb4_copy_host_drom(struct tb_switch *sw, u16 *size)
        return ret;
 }
 
-static int tb_drom_read_n(struct tb_switch *sw, u16 offset, u8 *val,
-                         size_t count)
+static int tb_drom_bit_bang(struct tb_switch *sw, u16 *size)
 {
-       if (tb_switch_is_usb4(sw))
-               return usb4_switch_drom_read(sw, offset, val, count);
-       return tb_eeprom_read_n(sw, offset, val, count);
+       int ret;
+
+       ret = tb_eeprom_read_n(sw, 14, (u8 *)size, 2);
+       if (ret)
+               return ret;
+
+       *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;
+
+       ret = tb_eeprom_read_n(sw, 0, sw->drom, *size);
+       if (ret)
+               goto err;
+
+       return 0;
+
+err:
+       kfree(sw->drom);
+       sw->drom = NULL;
+       return ret;
 }
 
-static int tb_drom_parse(struct tb_switch *sw)
+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 +579,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,85 +613,14 @@ static int usb4_drom_parse(struct tb_switch *sw)
        return tb_drom_parse_entries(sw, USB4_DROM_HEADER_SIZE);
 }
 
-/**
- * tb_drom_read() - Copy DROM to sw->drom and parse it
- * @sw: Router whose DROM to read and parse
- *
- * This function reads router DROM and if successful parses the entries and
- * populates the fields in @sw accordingly. Can be called for any router
- * generation.
- *
- * Returns %0 in case of success and negative errno otherwise.
- */
-int tb_drom_read(struct tb_switch *sw)
+static int tb_drom_parse(struct tb_switch *sw, u16 size)
 {
-       u16 size;
-       struct tb_drom_header *header;
-       int res, retries = 1;
-
-       if (sw->drom)
-               return 0;
-
-       if (tb_route(sw) == 0) {
-               /*
-                * Apple's NHI EFI driver supplies a DROM for the root switch
-                * in a device property. Use it if available.
-                */
-               if (tb_drom_copy_efi(sw, &size) == 0)
-                       goto parse;
-
-               /* Non-Apple hardware has the DROM as part of NVM */
-               if (tb_drom_copy_nvm(sw, &size) == 0)
-                       goto parse;
-
-               /*
-                * USB4 hosts may support reading DROM through router
-                * operations.
-                */
-               if (tb_switch_is_usb4(sw)) {
-                       usb4_switch_read_uid(sw, &sw->uid);
-                       if (!usb4_copy_host_drom(sw, &size))
-                               goto parse;
-               } else {
-                       /*
-                        * The root switch contains only a dummy drom
-                        * (header only, no entries). Hardcode the
-                        * configuration here.
-                        */
-                       tb_drom_read_uid_only(sw, &sw->uid);
-               }
-
-               return 0;
-       }
-
-       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(*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;
+       const struct tb_drom_header *header = (const void *)sw->drom;
+       int ret;
 
        if (header->data_len + TB_DROM_DATA_START != size) {
-               tb_sw_warn(sw, "drom size mismatch\n");
-               if (retries--) {
-                       msleep(100);
-                       goto read;
-               }
+               tb_sw_warn(sw, "DROM size mismatch\n");
+               ret = -EIO;
                goto err;
        }
 
@@ -674,29 +628,86 @@ parse:
 
        switch (header->device_rom_revision) {
        case 3:
-               res = usb4_drom_parse(sw);
+               ret = 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);
+               ret = tb_drom_parse_v1(sw);
                break;
        }
 
-       /* If the DROM parsing fails, wait a moment and retry once */
-       if (res == -EILSEQ && retries--) {
+       if (ret) {
                tb_sw_warn(sw, "parsing DROM failed\n");
-               msleep(100);
-               goto read;
+               goto err;
        }
 
-       if (!res)
-               return 0;
+       return 0;
 
 err:
        kfree(sw->drom);
        sw->drom = NULL;
-       return -EIO;
+
+       return ret;
+}
+
+static int tb_drom_host_read(struct tb_switch *sw)
+{
+       u16 size;
+
+       if (tb_switch_is_usb4(sw)) {
+               usb4_switch_read_uid(sw, &sw->uid);
+               if (!usb4_copy_drom(sw, &size))
+                       return tb_drom_parse(sw, size);
+       } else {
+               if (!tb_drom_copy_efi(sw, &size))
+                       return tb_drom_parse(sw, size);
+
+               if (!tb_drom_copy_nvm(sw, &size))
+                       return tb_drom_parse(sw, size);
+
+               tb_drom_read_uid_only(sw, &sw->uid);
+       }
+
+       return 0;
+}
+
+static int tb_drom_device_read(struct tb_switch *sw)
+{
+       u16 size;
+       int ret;
+
+       if (tb_switch_is_usb4(sw)) {
+               usb4_switch_read_uid(sw, &sw->uid);
+               ret = usb4_copy_drom(sw, &size);
+       } else {
+               ret = tb_drom_bit_bang(sw, &size);
+       }
+
+       if (ret)
+               return ret;
+
+       return tb_drom_parse(sw, size);
+}
+
+/**
+ * tb_drom_read() - Copy DROM to sw->drom and parse it
+ * @sw: Router whose DROM to read and parse
+ *
+ * This function reads router DROM and if successful parses the entries and
+ * populates the fields in @sw accordingly. Can be called for any router
+ * generation.
+ *
+ * Returns %0 in case of success and negative errno otherwise.
+ */
+int tb_drom_read(struct tb_switch *sw)
+{
+       if (sw->drom)
+               return 0;
+
+       if (!tb_route(sw))
+               return tb_drom_host_read(sw);
+       return tb_drom_device_read(sw);
 }