optionroms: Use revision when finding Option ROM

[SeaBIOS] [PATCH] optionroms: Use revision when finding Option ROM

Posted by Paul Menzel 4 years, 1 month ago

From: Rob Barnes <robbarnes@google.com>
Date: Wed, 22 Jan 2020 14:09:21 -0700

Some devices have the same vendor and device ID but need different
Option ROM files. Change the look-up to include the revision, then
fallback to looking up without the revision.

BUG=b:148125384
TEST=Manual, boot trembyle and confirm SeaBIOS console is displayed.

Change-Id: I4c969b08727077fcb5ca1198e61cc6711c675019
Reviewed-on: 
https://chromium-review.googlesource.com/c/chromiumos/third_party/seabios/+/2015963
Reviewed-by: Raul E Rangel <rrangel@chromium.org>
Tested-by: Rob Barnes <robbarnes@google.com>
Commit-Queue: Rob Barnes <robbarnes@google.com>
Auto-Submit: Rob Barnes <robbarnes@google.com>
---
  src/optionroms.c | 13 ++++++++++---
  1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/optionroms.c b/src/optionroms.c
index e906ab9..c3aa9b0 100644
--- a/src/optionroms.c
+++ b/src/optionroms.c
@@ -329,10 +329,17 @@ init_pcirom(struct pci_device *pci, int isvga, u64 
*sources)
      dprintf(4, "Attempting to init PCI bdf %pP (vd %04x:%04x)\n"
              , pci, pci->vendor, pci->device);

-    char fname[17];
-    snprintf(fname, sizeof(fname), "pci%04x,%04x.rom"
-             , pci->vendor, pci->device);
+    char fname[20];
+    // Try to find rom file with revision included
+    snprintf(fname, sizeof(fname), "pci%04x,%04x,%02x.rom"
+             , pci->vendor, pci->device, pci->revision);
      struct romfile_s *file = romfile_find(fname);
+    if (!file) {
+        // Fallback to finding rom file without revision
+        snprintf(fname, sizeof(fname), "pci%04x,%04x.rom"
+             , pci->vendor, pci->device);
+        file = romfile_find(fname);
+    }
      struct rom_header *rom = NULL;
      if (file)
          rom = deploy_romfile(file);
-- 
2.25.1
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

[SeaBIOS] Re: [PATCH] optionroms: Use revision when finding Option ROM

Posted by Gerd Hoffmann 4 years, 1 month ago

On Mon, Mar 16, 2020 at 05:48:55PM +0100, Paul Menzel wrote:
> From: Rob Barnes <robbarnes@google.com>
> Date: Wed, 22 Jan 2020 14:09:21 -0700
> 
> Some devices have the same vendor and device ID but need different
> Option ROM files. Change the look-up to include the revision, then
> fallback to looking up without the revision.
> 
> BUG=b:148125384
> TEST=Manual, boot trembyle and confirm SeaBIOS console is displayed.

> -    char fname[17];
> -    snprintf(fname, sizeof(fname), "pci%04x,%04x.rom"
> -             , pci->vendor, pci->device);
> +    char fname[20];
> +    // Try to find rom file with revision included
> +    snprintf(fname, sizeof(fname), "pci%04x,%04x,%02x.rom"
> +             , pci->vendor, pci->device, pci->revision);
>      struct romfile_s *file = romfile_find(fname);
> +    if (!file) {
> +        // Fallback to finding rom file without revision
> +        snprintf(fname, sizeof(fname), "pci%04x,%04x.rom"
> +             , pci->vendor, pci->device);
> +        file = romfile_find(fname);
> +    }

Looks good too, but likewise doesn't apply.

Seems something is broken in your mail workflow.
Did you use "git send-email" which usually gets things right?

cheers,
  Gerd
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

[SeaBIOS] Re: [PATCH] optionroms: Use revision when finding Option ROM

Posted by Kevin O'Connor 4 years, 1 month ago

On Mon, Mar 16, 2020 at 05:48:55PM +0100, Paul Menzel wrote:
> From: Rob Barnes <robbarnes@google.com>
> Date: Wed, 22 Jan 2020 14:09:21 -0700
> 
> Some devices have the same vendor and device ID but need different
> Option ROM files. Change the look-up to include the revision, then
> fallback to looking up without the revision.

Thanks.  At a high-level I think this change is fine.  However, the
documentation would also need to be updated (specifically I'm thinking
about docs/Runtime_config.md).

-Kevin


> 
> BUG=b:148125384
> TEST=Manual, boot trembyle and confirm SeaBIOS console is displayed.
> 
> Change-Id: I4c969b08727077fcb5ca1198e61cc6711c675019
> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/seabios/+/2015963
> Reviewed-by: Raul E Rangel <rrangel@chromium.org>
> Tested-by: Rob Barnes <robbarnes@google.com>
> Commit-Queue: Rob Barnes <robbarnes@google.com>
> Auto-Submit: Rob Barnes <robbarnes@google.com>
> ---
>  src/optionroms.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/src/optionroms.c b/src/optionroms.c
> index e906ab9..c3aa9b0 100644
> --- a/src/optionroms.c
> +++ b/src/optionroms.c
> @@ -329,10 +329,17 @@ init_pcirom(struct pci_device *pci, int isvga, u64
> *sources)
>      dprintf(4, "Attempting to init PCI bdf %pP (vd %04x:%04x)\n"
>              , pci, pci->vendor, pci->device);
> 
> -    char fname[17];
> -    snprintf(fname, sizeof(fname), "pci%04x,%04x.rom"
> -             , pci->vendor, pci->device);
> +    char fname[20];
> +    // Try to find rom file with revision included
> +    snprintf(fname, sizeof(fname), "pci%04x,%04x,%02x.rom"
> +             , pci->vendor, pci->device, pci->revision);
>      struct romfile_s *file = romfile_find(fname);
> +    if (!file) {
> +        // Fallback to finding rom file without revision
> +        snprintf(fname, sizeof(fname), "pci%04x,%04x.rom"
> +             , pci->vendor, pci->device);
> +        file = romfile_find(fname);
> +    }
>      struct rom_header *rom = NULL;
>      if (file)
>          rom = deploy_romfile(file);
> -- 
> 2.25.1
> _______________________________________________
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-leave@seabios.org
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org