Re: [PATCH] arm64: Fix the bug in fdt module

[Vladimir 'phcoder' Serbinenko]

Le 5 nov. 2015 9:51 AM, "Fu Wei" <address@hidden> a écrit :
>
> Hi Vladimir
>
> Please correct me :-)
> Can I do this :
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 2ef10d1..ea5831f 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1665,7 +1665,6 @@ module = {
>    ia64_efi = loader/ia64/efi/linux.c;
>    arm = loader/arm/linux.c;
>    arm64 = loader/arm64/linux.c;
> -  fdt = lib/fdt.c;
>    common = loader/linux.c;
>    common = lib/cmdline.c;
>    enable = noemu;
> @@ -1674,7 +1673,9 @@ module = {
>  module = {
>    name = fdt;
>    arm64 = loader/arm64/fdt.c;
> -  enable = arm64;
> +  arm = loader/arm/fdt.c;
Where does this file come from?
> +  common = lib/fdt.c;
> +  enable = fdt;
>  };
>
>  module = {
>
> On 4 November 2015 at 19:16, Vladimir 'phcoder' Serbinenko
> <address@hidden> wrote:
> >
> > Le 4 nov. 2015 12:12 PM, "Vladimir 'phcoder' Serbinenko" <address@hidden>
> > a écrit :
> >>
> >>
> >> Le 4 nov. 2015 12:09 PM, "Fu Wei" <address@hidden> a écrit :
> >> >
> >> >  Hi Vladimir,
> >> >
> >> > On 4 November 2015 at 19:06, Fu Wei <address@hidden> wrote:
> >> > > Hi Vladimir,
> >> > >
> >> > > On 4 November 2015 at 18:47, Vladimir 'phcoder' Serbinenko
> >> > > <address@hidden> wrote:
> >> > >>
> >> > >> Le 4 nov. 2015 10:48 AM, "Fu Wei" <address@hidden> a écrit :
> >> > >>>
> >> > >>> Hi Vladimir,
> >> > >>>
> >> > >>> Great thanks for your help :-)
> >> > >>>
> >> > >>>
> >> > >>> On 4 November 2015 at 02:07, Vladimir 'phcoder' Serbinenko
> >> > >>> <address@hidden> wrote:
> >> > >>> >
> >> > >>> > Le 3 nov. 2015 9:56 AM, <address@hidden> a écrit :
> >> > >>> >>
> >> > >>> >> From: Fu Wei <address@hidden>
> >> > >>> >>
> >> > >>> >> This patch goes with commit:
> >> > >>> >> 4d0cb755387d6f109b901386ed4d3d475df239fe
> >> > >>> >>  arm64: Move FDT functions to separate module
> >> > >>> >>
> >> > >>> >> linux and xen_boot modules can't work without this patch.
> >> > >>> >>
> >> > >>> >> Signed-off-by: Fu Wei <address@hidden>
> >> > >>> >> ---
> >> > >>> >>  grub-core/Makefile.core.def  | 1 +
> >> > >>> >>  grub-core/loader/arm64/fdt.c | 5 +++++
> >> > >>> >>  2 files changed, 6 insertions(+)
> >> > >>> >>
> >> > >>> >> diff --git a/grub-core/Makefile.core.def
> >> > >>> >> b/grub-core/Makefile.core.def
> >> > >>> >> index 2ef10d1..3ea4e49 100644
> >> > >>> >> --- a/grub-core/Makefile.core.def
> >> > >>> >> +++ b/grub-core/Makefile.core.def
> >> > >>> >> @@ -1674,6 +1674,7 @@ module = {
> >> > >>> >>  module = {
> >> > >>> >>    name = fdt;
> >> > >>> >>    arm64 = loader/arm64/fdt.c;
> >> > >>> >> +  fdt = lib/fdt.c;
> >> > >>> >>    enable = arm64;
> >> > >>> >>  };
> >> > >>> >>
> >> > >>> > Please don't add same file to 2 different modules. Remove it from
> >> > >>> > Linux
> >> > >>> > module
> >> > >>>
> >> > >>> AFAIK, for now , only arm and arm64 are using lib/fdt.c
> >> > >>> So please allow me to separate all the fdt code from
> >> > >>> loader/arm/linux.c; just like arm64.
> >> > >>>
> >> > >> I don't think it's necessary at this point. Is xen_boot going to be
> >> > >> available for 32-bit arm as well?
> >> >
> >> > And I would like to help to sort out the code for your new fdt module
> >> > , If you don't mind.
> >> >
> >> > But according to my tests, I think we need "fdt = lib/fdt.c;" in
> >> > module. If we don't separate fdt code from arm, we also need "fdt =
> >> > lib/fdt.c;" in linux module.
> >> >
> >> > Please correct me if I misunderstand something, thanks
> >> >
> >> We need arm = lib/fdt.c in linux if we don't restructure. There is no
> >> reason to include lib/fdt.c in 2 different modules on arm64. Fdt in first
> >> part just mean arm and arm64 in this context
> >>
> > As an alternative: enable fdt module on all fdt platforms (enable = fdt) and
> > put lib/fdt.c
> >
> >> > >>> This patch may become a patchset :-)
> >> > >>>
> >> > >>> >> diff --git a/grub-core/loader/arm64/fdt.c
> >> > >>> >> b/grub-core/loader/arm64/fdt.c
> >> > >>> >> index 5202c14..d160ca0 100644
> >> > >>> >> --- a/grub-core/loader/arm64/fdt.c
> >> > >>> >> +++ b/grub-core/loader/arm64/fdt.c
> >> > >>> >> @@ -25,6 +25,10 @@
> >> > >>> >>  #include <grub/file.h>
> >> > >>> >>  #include <grub/efi/efi.h>
> >> > >>> >>
> >> > >>> >> +GRUB_MOD_LICENSE ("GPLv3+");
> >> > >>> >> +
> >> > >>> >> +static grub_dl_t my_mod;
> >> > >>> >> +
> >> > >>> > What's the reason for my_mod?
> >> > >>>
> >> > >>> this is for grub_dl_unref and grub_dl_ref. but I forgot to check
> >> > >>> this
> >> > >>> again for this, sorry,
> >> > >>> will add this later
> >> > >>>
> >> > >> Forget dl_ref and dl_unref. Linux module having a function reference
> >> > >> is
> >> > >> already good enough
> >> > >>> >>  static void *loaded_fdt;
> >> > >>> >>  static void *fdt;
> >> > >>> >>
> >> > >>> >> @@ -177,6 +181,7 @@ GRUB_MOD_INIT (fdt)
> >> > >>> >>    cmd_devicetree =
> >> > >>> >>      grub_register_command ("devicetree", grub_cmd_devicetree, 0,
> >> > >>> >>                            N_("Load DTB file."));
> >> > >>> >> +  my_mod = mod;
> >> > >>> >>  }
> >> > >>> >>
> >> > >>> >>  GRUB_MOD_FINI (fdt)
> >> > >>> >> --
> >> > >>> >> 2.4.3
> >> > >>> >>
> >> > >>>
> >> > >>>
> >> > >>>
> >> > >>> --
> >> > >>> Best regards,
> >> > >>>
> >> > >>> Fu Wei
> >> > >>> Software Engineer
> >> > >>> Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
> >> > >>> Ph: +86 21 61221326(direct)
> >> > >>> Ph: +86 186 2020 4684 (mobile)
> >> > >>> Room 1512, Regus One Corporate Avenue,Level 15,
> >> > >>> One Corporate Avenue,222 Hubin Road,Huangpu District,
> >> > >>> Shanghai,China 200021
> >> > >
> >> > >
> >> > >
> >> > > --
> >> > > Best regards,
> >> > >
> >> > > Fu Wei
> >> > > Software Engineer
> >> > > Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
> >> > > Ph: +86 21 61221326(direct)
> >> > > Ph: +86 186 2020 4684 (mobile)
> >> > > Room 1512, Regus One Corporate Avenue,Level 15,
> >> > > One Corporate Avenue,222 Hubin Road,Huangpu District,
> >> > > Shanghai,China 200021
> >> >
> >> >
> >> >
> >> > --
> >> > Best regards,
> >> >
> >> > Fu Wei
> >> > Software Engineer
> >> > Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
> >> > Ph: +86 21 61221326(direct)
> >> > Ph: +86 186 2020 4684 (mobile)
> >> > Room 1512, Regus One Corporate Avenue,Level 15,
> >> > One Corporate Avenue,222 Hubin Road,Huangpu District,
> >> > Shanghai,China 200021
>
>
>
> --
> Best regards,
>
> Fu Wei
> Software Engineer
> Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
> Ph: +86 21 61221326(direct)
> Ph: +86 186 2020 4684 (mobile)
> Room 1512, Regus One Corporate Avenue,Level 15,
> One Corporate Avenue,222 Hubin Road,Huangpu District,
> Shanghai,China 200021