grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] arm64: Move firmware fdt search into global function


From: Daniel Kiper
Subject: Re: [PATCH 1/2] arm64: Move firmware fdt search into global function
Date: Wed, 16 Nov 2016 11:28:22 +0100
User-agent: Mutt/1.3.28i

On Tue, Nov 15, 2016 at 10:07:39PM +0100, Alexander Graf wrote:
>
>
> On 15/11/2016 22:00, Daniel Kiper wrote:
> >On Sat, Nov 12, 2016 at 12:03:33PM +0300, Andrei Borzenkov wrote:
> >>29.02.2016 02:22, Alexander Graf ??????????:
> >>>Searching for a device tree that EFI passes to us via configuration
> >>>tables
> >>>is nothing architecture specific. Move it into generic code.
> >>>
> >>>Signed-off-by: Alexander Graf <address@hidden>
> >>>---
> >>> grub-core/kern/efi/init.c    | 22 ++++++++++++++++++++++
> >>> grub-core/loader/arm64/fdt.c | 24 +-----------------------
> >>> include/grub/efi/efi.h       |  2 ++
> >>> 3 files changed, 25 insertions(+), 23 deletions(-)
> >>>
> >>>diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
> >>>index e9c85de..fb90ecd 100644
> >>>--- a/grub-core/kern/efi/init.c
> >>>+++ b/grub-core/kern/efi/init.c
> >>>@@ -72,6 +72,28 @@ grub_machine_get_bootlocation (char **device, char
> >>>**path)
> >>>     }
> >>> }
> >>>
> >>>+void *
> >>>+grub_efi_get_firmware_fdt (void)
> >>>+{
> >>>+  grub_efi_configuration_table_t *tables;
> >>>+  grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
> >>>+  void *firmware_fdt = NULL;
> >>>+  unsigned int i;
> >>>+
> >>>+  /* Look for FDT in UEFI config tables. */
> >>>+  tables = grub_efi_system_table->configuration_table;
> >>>+
> >>>+  for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
> >>>+    if (grub_memcmp (&tables[i].vendor_guid, &fdt_guid, sizeof
> >>>(fdt_guid)) == 0)
> >>>+      {
> >>>+  firmware_fdt = tables[i].vendor_table;
> >>>+  grub_dprintf ("linux", "found registered FDT @ %p\n", firmware_fdt);
> >>>+  break;
> >>>+      }
> >>>+
> >>>+  return firmware_fdt;
> >>>+}
> >>>+
> >>
> >>Is it relevant for x86? I cannot even find anything related to FDT or
> >>"device tree" in UEFI spec, it falls under vendor extensions. What other
> >>use it has except in Linux loader?
> >>
> >>I really fail to see why it should be moved into core until at least one
> >>more non-trivial caller is present.
> >
> >Hmmm... It looks that I was too fast. TBH, I can agree to some extend.
> >Should we revert this patch or just #ifdef relevant code? Revert seems
> >better for me.
>
> Uh, the point of the function was to share code between 32bit and 64bit
> arm platforms which are completely separate archs.
>
> There's also effort underway to run with device tree on x86:
>
>   https://plus.google.com/+ChristopherFriedt/posts/SfDifuC1xsR

OK but right know we build FDT code for every EFI platform. This
is not nice because not every EFI platform uses/knows FDT (at least
today). So, I think that we should revert that patch and then you
can provide another patch which puts FDT code in separate file, e.g.
grub-core/kern/efi/fdt.c build only on platforms supporting FDT.
Does it make sense?

Daniel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]