[v1] DynamicTablesPkg: FdtHwInfoParserLib: Fix compatible string

[edk2-devel] [PATCH] DynamicTablesPkg: FdtHwInfoParserLib: Fix compatible string

Posted by Moritz Fischer via groups.io 3 years, 1 month ago

Linux' cpu DT bindings call out arm,armv8 while the code previously
used arm,arm-v8.

Fixes: e366a41ef0 ("DynamicTablesPkg: FdtHwInfoParser: Add GICC parser")
Signed-off-by: Moritz Fischer <moritzf@google.com>
---
 DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
index 2c72bd0c58..dc6e7a4d15 100644
--- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
+++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
@@ -22,7 +22,7 @@
 */
 STATIC CONST COMPATIBILITY_STR  CpuCompatibleStr[] = {
   { "arm,arm-v7"     },
-  { "arm,arm-v8"     },
+  { "arm,armv8"      },
   { "arm,cortex-a15" },
   { "arm,cortex-a7"  },
   { "arm,cortex-a57" }
-- 
2.39.0.314.g84b9a713c41-goog



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98036): https://edk2.groups.io/g/devel/message/98036
Mute This Topic: https://groups.io/mt/96083588/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] DynamicTablesPkg: FdtHwInfoParserLib: Fix compatible string

Posted by Ard Biesheuvel 3 years, 1 month ago

On Fri, 6 Jan 2023 at 00:07, Moritz Fischer via groups.io
<moritzf=google.com@groups.io> wrote:
>
> Linux' cpu DT bindings call out arm,armv8 while the code previously
> used arm,arm-v8.
>
> Fixes: e366a41ef0 ("DynamicTablesPkg: FdtHwInfoParser: Add GICC parser")
> Signed-off-by: Moritz Fischer <moritzf@google.com>
> ---
>  DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> index 2c72bd0c58..dc6e7a4d15 100644
> --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> @@ -22,7 +22,7 @@
>  */
>  STATIC CONST COMPATIBILITY_STR  CpuCompatibleStr[] = {
>    { "arm,arm-v7"     },
> -  { "arm,arm-v8"     },
> +  { "arm,armv8"      },

Thanks for the patch

Should we keep both instead of removing the old one?

>    { "arm,cortex-a15" },
>    { "arm,cortex-a7"  },
>    { "arm,cortex-a57" }
> --
> 2.39.0.314.g84b9a713c41-goog
>
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98086): https://edk2.groups.io/g/devel/message/98086
Mute This Topic: https://groups.io/mt/96083588/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] DynamicTablesPkg: FdtHwInfoParserLib: Fix compatible string

Posted by Ard Biesheuvel 3 years, 1 month ago

(cc Sami)

On Fri, 6 Jan 2023 at 10:14, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 6 Jan 2023 at 00:07, Moritz Fischer via groups.io
> <moritzf=google.com@groups.io> wrote:
> >
> > Linux' cpu DT bindings call out arm,armv8 while the code previously
> > used arm,arm-v8.
> >
> > Fixes: e366a41ef0 ("DynamicTablesPkg: FdtHwInfoParser: Add GICC parser")
> > Signed-off-by: Moritz Fischer <moritzf@google.com>
> > ---
> >  DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> > index 2c72bd0c58..dc6e7a4d15 100644
> > --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> > +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> > @@ -22,7 +22,7 @@
> >  */
> >  STATIC CONST COMPATIBILITY_STR  CpuCompatibleStr[] = {
> >    { "arm,arm-v7"     },
> > -  { "arm,arm-v8"     },
> > +  { "arm,armv8"      },
>
> Thanks for the patch
>
> Should we keep both instead of removing the old one?
>
> >    { "arm,cortex-a15" },
> >    { "arm,cortex-a7"  },
> >    { "arm,cortex-a57" }
> > --
> > 2.39.0.314.g84b9a713c41-goog
> >
> >
> >
> > 
> >
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98087): https://edk2.groups.io/g/devel/message/98087
Mute This Topic: https://groups.io/mt/96083588/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] DynamicTablesPkg: FdtHwInfoParserLib: Fix compatible string

Posted by Sami Mujawar 3 years, 1 month ago

Hi Ard, Mortiz,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

On 06/01/2023 09:15 am, Ard Biesheuvel wrote:
> (cc Sami)
>
> On Fri, 6 Jan 2023 at 10:14, Ard Biesheuvel <ardb@kernel.org> wrote:
>> On Fri, 6 Jan 2023 at 00:07, Moritz Fischer via groups.io
>> <moritzf=google.com@groups.io> wrote:
>>> Linux' cpu DT bindings call out arm,armv8 while the code previously
>>> used arm,arm-v8.
>>>
>>> Fixes: e366a41ef0 ("DynamicTablesPkg: FdtHwInfoParser: Add GICC parser")
>>> Signed-off-by: Moritz Fischer <moritzf@google.com>
>>> ---
>>>   DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
>>> index 2c72bd0c58..dc6e7a4d15 100644
>>> --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
>>> +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
>>> @@ -22,7 +22,7 @@
>>>   */
>>>   STATIC CONST COMPATIBILITY_STR  CpuCompatibleStr[] = {
>>>     { "arm,arm-v7"     },
>>> -  { "arm,arm-v8"     },
>>> +  { "arm,armv8"      },
>> Thanks for the patch
>>
>> Should we keep both instead of removing the old one?

[SAMI] I think we would need to keep both as the CPU nodes generated by 
Kvmtool have compatible string as "arm,arm-v8", see 
https://github.com/kvmtool/kvmtool/blob/master/arm/aarch64/arm-cpu.c#L31.

Mortiz: Is it possible to share some information about your use case, 
please?

[/SAMI]

>>
>>>     { "arm,cortex-a15" },
>>>     { "arm,cortex-a7"  },
>>>     { "arm,cortex-a57" }
>>> --
>>> 2.39.0.314.g84b9a713c41-goog
>>>
>>>
>>>
>>> 
>>>
>>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98095): https://edk2.groups.io/g/devel/message/98095
Mute This Topic: https://groups.io/mt/96083588/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] DynamicTablesPkg: FdtHwInfoParserLib: Fix compatible string

Posted by Moritz Fischer via groups.io 3 years, 1 month ago

Sami,

On Fri, Jan 6, 2023 at 1:55 AM Sami Mujawar <sami.mujawar@arm.com> wrote:
>
> Hi Ard, Mortiz,
>
> Please find my response inline marked [SAMI].
>
> Regards,
>
> Sami Mujawar
>
> On 06/01/2023 09:15 am, Ard Biesheuvel wrote:
> > (cc Sami)
> >
> > On Fri, 6 Jan 2023 at 10:14, Ard Biesheuvel <ardb@kernel.org> wrote:
> >> On Fri, 6 Jan 2023 at 00:07, Moritz Fischer via groups.io
> >> <moritzf=google.com@groups.io> wrote:
> >>> Linux' cpu DT bindings call out arm,armv8 while the code previously
> >>> used arm,arm-v8.
> >>>
> >>> Fixes: e366a41ef0 ("DynamicTablesPkg: FdtHwInfoParser: Add GICC parser")
> >>> Signed-off-by: Moritz Fischer <moritzf@google.com>
> >>> ---
> >>>   DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c | 2 +-
> >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> >>> index 2c72bd0c58..dc6e7a4d15 100644
> >>> --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> >>> +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> >>> @@ -22,7 +22,7 @@
> >>>   */
> >>>   STATIC CONST COMPATIBILITY_STR  CpuCompatibleStr[] = {
> >>>     { "arm,arm-v7"     },
> >>> -  { "arm,arm-v8"     },
> >>> +  { "arm,armv8"      },
> >> Thanks for the patch
> >>
> >> Should we keep both instead of removing the old one?

I think adding arm,armv8 is better. I don't know why I didn't think
about this from the beginning.

I can resend a v2. Can't hurt to support both?
>
> [SAMI] I think we would need to keep both as the CPU nodes generated by
> Kvmtool have compatible string as "arm,arm-v8", see
> https://github.com/kvmtool/kvmtool/blob/master/arm/aarch64/arm-cpu.c#L31.
>
> Mortiz: Is it possible to share some information about your use case,
> please?

I have to deal with a DT binary that uses the other one :-)

>
> [/SAMI]
>
> >>
> >>>     { "arm,cortex-a15" },
> >>>     { "arm,cortex-a7"  },
> >>>     { "arm,cortex-a57" }
> >>> --
> >>> 2.39.0.314.g84b9a713c41-goog
> >>>
> >>>
> >>>
> >>> 
> >>>
> >>>
Cheers,
Moritz


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98153): https://edk2.groups.io/g/devel/message/98153
Mute This Topic: https://groups.io/mt/96083588/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] DynamicTablesPkg: FdtHwInfoParserLib: Fix compatible string

Posted by Sami Mujawar 3 years ago

Hi Moritz,

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 08/01/2023, 04:08, "Moritz Fischer" <moritzf@google.com> wrote:

    Sami,

    On Fri, Jan 6, 2023 at 1:55 AM Sami Mujawar <sami.mujawar@arm.com> wrote:
    >
    > Hi Ard, Mortiz,
    >
    > Please find my response inline marked [SAMI].
    >
    > Regards,
    >
    > Sami Mujawar
    >
    > On 06/01/2023 09:15 am, Ard Biesheuvel wrote:
    > > (cc Sami)
    > >
    > > On Fri, 6 Jan 2023 at 10:14, Ard Biesheuvel <ardb@kernel.org> wrote:
    > >> On Fri, 6 Jan 2023 at 00:07, Moritz Fischer via groups.io
    > >> <moritzf=google.com@groups.io> wrote:
    > >>> Linux' cpu DT bindings call out arm,armv8 while the code previously
    > >>> used arm,arm-v8.
    > >>>
    > >>> Fixes: e366a41ef0 ("DynamicTablesPkg: FdtHwInfoParser: Add GICC parser")
    > >>> Signed-off-by: Moritz Fischer <moritzf@google.com>
    > >>> ---
    > >>>   DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c | 2 +-
    > >>>   1 file changed, 1 insertion(+), 1 deletion(-)
    > >>>
    > >>> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
    > >>> index 2c72bd0c58..dc6e7a4d15 100644
    > >>> --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
    > >>> +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
    > >>> @@ -22,7 +22,7 @@
    > >>>   */
    > >>>   STATIC CONST COMPATIBILITY_STR  CpuCompatibleStr[] = {
    > >>>     { "arm,arm-v7"     },
    > >>> -  { "arm,arm-v8"     },
    > >>> +  { "arm,armv8"      },
    > >> Thanks for the patch
    > >>
    > >> Should we keep both instead of removing the old one?

    I think adding arm,armv8 is better. I don't know why I didn't think
    about this from the beginning.

    I can resend a v2. Can't hurt to support both?
[SAMI] Ye, please. We need the existing enry to support Kvmtool guest firmware.
    >
    > [SAMI] I think we would need to keep both as the CPU nodes generated by
    > Kvmtool have compatible string as "arm,arm-v8", see
    > https://github.com/kvmtool/kvmtool/blob/master/arm/aarch64/arm-cpu.c#L31.
    >
    > Mortiz: Is it possible to share some information about your use case,
    > please?

    I have to deal with a DT binary that uses the other one :-)

    >
    > [/SAMI]
    >
    > >>
    > >>>     { "arm,cortex-a15" },
    > >>>     { "arm,cortex-a7"  },
    > >>>     { "arm,cortex-a57" }
    > >>> --
    > >>> 2.39.0.314.g84b9a713c41-goog
    > >>>
    > >>>
    > >>>
    > >>> 
    > >>>
    > >>>
    Cheers,
    Moritz



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98177): https://edk2.groups.io/g/devel/message/98177
Mute This Topic: https://groups.io/mt/96083588/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-