Hi Mislav, First of all, please do not top post and wrap your long lines...
On Thu, Oct 07, 2021 at 09:23:14AM +0000, Mislav Stublić wrote: > Hi, > > There is a lot of duplication as I have used existing implementation > as a reference (arm, aarch64 and fdt). But refactoring existing > implementation atop of implementing x86 didn’t seem realistic as those > modules are quite different... at least when I first looked at this. > My biggest question is how to unify existing memory allocation > procedures as those are different, at least in a sense x86 has > relocator and the setup around it (or can/should this unification be > avoided). Also, it seems there is a lot of divergence in UEFI > implementations for which I'm not sure it can be avoided without > bigger refactor of both UEFI and x86 code. But I would need to think > some more on your suggestion, maybe it's not so complicated. I am not sure you understand what I want. I do not ask you to unify UEFI device tree code with non-UEFI device tree code. This can be difficult (but not impossible). So, I want to have two device tree implementations: one for UEFI platforms and another for non-UEFI platforms (when you start working on this there is a chance we will see how to merge both into one; though let's start with two implementations for simplicity). Then every architecture/CPU implementation should use the former or the latter. Does it sound OK for you? Daniel > Mislav > > > -----Original Message----- > > From: Daniel Kiper [mailto:dki...@net-space.pl] > > Sent: Wednesday, October 06, 2021 2:29 PM > > To: Mislav Stublić > > Cc: grub-devel@gnu.org > > Subject: Re: [PATCH] loader/i386/linux: Add device tree support > > > > Hi Mislav, > > > > On Tue, Sep 21, 2021 at 03:37:45PM +0000, Mislav Stublić wrote: > > > Hi, > > > > > > This implements device tree support for x86. Unfortunately I haven't > > > been able to test this on latest master but I have tested against 2.04 > > > and it works fine. I will test on master later but I wanted to get > > > some initial comments if this is going in the right direction. What I > > > haven't tested is firmware DTB loading support which only calls > > > grub_efi_get_firmware_fdt from kern/efi implementation as I don't have > > > hardware to test this scenario. > > > > I skimmed through your patch and it seems to me you are duplicating at > > least some code from grub-core/loader/arm/linux.c. I would do this a bit > > differently. First of all I would factor out non-UEFI device tree code > > from grub-core/loader/arm/linux.c to a separate module. This should be > > a separate patch. Then I would try to unify the interface for UEFI and > > non-UEFI device tree code (again, separate patch). Of course if needed. > > Then the last patch should add device tree support to the x86. If you > > have any questions drop me a line. > > > > Anyway, thank you for taking a stab at this. > > > > Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel