2016-03-01 11:01:10

by Andre Przywara

[permalink] [raw]
Subject: Re: [PATCH 7/8] ARM64: dts: amlogic: Extend GXBaby GIC node

Hi Andreas,

On 29/02/16 23:44, Andreas Färber wrote:
> Add GICH and GICV resources for HYP mode - guess based on other vendors.

Do you know if the firmware allows the kernel to be entered in EL2
(which is the arm64 name for HYP)?
So can we run kvm?
If you have a booted kernel, can you grep for "EL2" and "kvm" in the dmesg?

Also you should merge this patch into 3/8, same for 8/8.

> Signed-off-by: Andreas Färber <[email protected]>
> ---
> arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> index 0ae089bd1806..5088ae3ff653 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> @@ -117,7 +117,9 @@
> gic: interrupt-controller@c4301000 {
> compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";

I think "arm,gic-400" is the name to use here these days, especially for
arm64.

> reg = <0x0 0xc4301000 0 0x1000>,
> - <0x0 0xc4302000 0 0x0100>;
> + <0x0 0xc4302000 0 0x0100>,

Please use 0x2000 for the size here. I guess this is really the GIC-400
from ARM, and in this case this is the right size, [1] is the reference
here. This will enable EOI mode 1 for KVM.

Cheers,
Andre.

[1]
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0471b/CHDIFAEE.html

> + <0x0 0xc4304000 0 0x2000>,
> + <0x0 0xc4306000 0 0x2000>;
> interrupt-controller;
> interrupts = <GIC_PPI 9
> (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
>


2016-03-01 11:18:28

by Andreas Färber

[permalink] [raw]
Subject: Re: [PATCH 7/8] ARM64: dts: amlogic: Extend GXBaby GIC node

Hi Andre,

Am 01.03.2016 um 12:01 schrieb Andre Przywara:
> On 29/02/16 23:44, Andreas Färber wrote:
>> Add GICH and GICV resources for HYP mode - guess based on other vendors.
>
> Do you know if the firmware allows the kernel to be entered in EL2
> (which is the arm64 name for HYP)?
> So can we run kvm?
> If you have a booted kernel, can you grep for "EL2" and "kvm" in the dmesg?

I do not have a rootfs yet (MMC v5 patches by Carlo are still waiting
for review), but with this change the KVM driver initializes okay - the
purpose of this patch!

> Also you should merge this patch into 3/8, same for 8/8.

If people confirm this is generally or specifically for this SoC correct
then sure. So far 3/8 seems a safe subset for lack of public documentation.

>> Signed-off-by: Andreas Färber <[email protected]>
>> ---
>> arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> index 0ae089bd1806..5088ae3ff653 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> @@ -117,7 +117,9 @@
>> gic: interrupt-controller@c4301000 {
>> compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>
> I think "arm,gic-400" is the name to use here these days, especially for
> arm64.

I took what /proc/device-tree showed on Android and verified that this
compatible is in use in mainline.

>> reg = <0x0 0xc4301000 0 0x1000>,
>> - <0x0 0xc4302000 0 0x0100>;
>> + <0x0 0xc4302000 0 0x0100>,
>
> Please use 0x2000 for the size here. I guess this is really the GIC-400
> from ARM, and in this case this is the right size, [1] is the reference
> here. This will enable EOI mode 1 for KVM.

Will test later.

Is there any easy way to find out whether or not this is that GIC-400?

Thanks,
Andreas

> [1]
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0471b/CHDIFAEE.html
>
>> + <0x0 0xc4304000 0 0x2000>,
>> + <0x0 0xc4306000 0 0x2000>;
>> interrupt-controller;
>> interrupts = <GIC_PPI 9
>> (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;

--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

2016-03-01 11:42:22

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH 7/8] ARM64: dts: amlogic: Extend GXBaby GIC node



On 01/03/16 12:18, Andreas Färber wrote:
> Hi Andre,
>
> Am 01.03.2016 um 12:01 schrieb Andre Przywara:
>> On 29/02/16 23:44, Andreas Färber wrote:
>>> Add GICH and GICV resources for HYP mode - guess based on other vendors.
>>
>> Do you know if the firmware allows the kernel to be entered in EL2
>> (which is the arm64 name for HYP)?
>> So can we run kvm?
>> If you have a booted kernel, can you grep for "EL2" and "kvm" in the dmesg?
>
> I do not have a rootfs yet (MMC v5 patches by Carlo are still waiting
> for review), but with this change the KVM driver initializes okay - the
> purpose of this patch!
>
>> Also you should merge this patch into 3/8, same for 8/8.
>
> If people confirm this is generally or specifically for this SoC correct
> then sure. So far 3/8 seems a safe subset for lack of public documentation.
>

Yes, this is generally correct. Actually without 8/8 you won't be able
to run an initramfs.

Regards,
Matthias

2016-03-01 12:43:54

by Andre Przywara

[permalink] [raw]
Subject: Re: [PATCH 7/8] ARM64: dts: amlogic: Extend GXBaby GIC node

Hi,

On 01/03/16 11:18, Andreas Färber wrote:
> Hi Andre,
>
> Am 01.03.2016 um 12:01 schrieb Andre Przywara:
>> On 29/02/16 23:44, Andreas Färber wrote:
>>> Add GICH and GICV resources for HYP mode - guess based on other vendors.
>>
>> Do you know if the firmware allows the kernel to be entered in EL2
>> (which is the arm64 name for HYP)?
>> So can we run kvm?
>> If you have a booted kernel, can you grep for "EL2" and "kvm" in the dmesg?
>
> I do not have a rootfs yet (MMC v5 patches by Carlo are still waiting
> for review), but with this change the KVM driver initializes okay - the
> purpose of this patch!
>
>> Also you should merge this patch into 3/8, same for 8/8.
>
> If people confirm this is generally or specifically for this SoC correct
> then sure. So far 3/8 seems a safe subset for lack of public documentation.

The GIC is an integral part of the SoC, so this clearly belongs in there.

>>> Signed-off-by: Andreas Färber <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 +++-

In general I was wondering if this naming is correct?
Shouldn't it be something with "s905" in it? Because this the SoC that
is driving all those hardware and the peripherals that you describe in
there are clearly within the SoC.
So something like meson-s905.dtsi or the like?

>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>> index 0ae089bd1806..5088ae3ff653 100644
>>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>> @@ -117,7 +117,9 @@
>>> gic: interrupt-controller@c4301000 {
>>> compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>>
>> I think "arm,gic-400" is the name to use here these days, especially for
>> arm64.
>
> I took what /proc/device-tree showed on Android and verified that this
> compatible is in use in mainline.

Some vendor Android kernel is not a good reference for mainline work ;-)
Better look at other DTs in arch/arm64/boot/dts.
You could keep "arm,cortex-a15-gic" in there if you care about
compatibility with older (vendor) kernels, but I guess there are other
issues which prevent this anyway, so you could drop this as well.

>>> reg = <0x0 0xc4301000 0 0x1000>,
>>> - <0x0 0xc4302000 0 0x0100>;
>>> + <0x0 0xc4302000 0 0x0100>,
>>
>> Please use 0x2000 for the size here. I guess this is really the GIC-400
>> from ARM, and in this case this is the right size, [1] is the reference
>> here. This will enable EOI mode 1 for KVM.
>
> Will test later.
>
> Is there any easy way to find out whether or not this is that GIC-400?

If you can read registers: GICD_IIDR and PIDRx have some info:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0471b/CHDIFAEE.html
So if your U-Boot for instance supports md, a dump of:
md.l c4301008 1
md.l c4301fd0 30

would help to identify the GIC.

Cheers,
Andre.

>
> Thanks,
> Andreas
>
>> [1]
>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0471b/CHDIFAEE.html
>>
>>> + <0x0 0xc4304000 0 0x2000>,
>>> + <0x0 0xc4306000 0 0x2000>;
>>> interrupt-controller;
>>> interrupts = <GIC_PPI 9
>>> (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
>

2016-03-01 12:53:46

by Carlo Caione

[permalink] [raw]
Subject: Re: [PATCH 7/8] ARM64: dts: amlogic: Extend GXBaby GIC node

On Tue, Mar 1, 2016 at 1:43 PM, Andre Przywara <[email protected]> wrote:
> Hi,
>
> On 01/03/16 11:18, Andreas Färber wrote:
>> Hi Andre,
>>
>> Am 01.03.2016 um 12:01 schrieb Andre Przywara:
>>> On 29/02/16 23:44, Andreas Färber wrote:
>>>> Add GICH and GICV resources for HYP mode - guess based on other vendors.
>>>
>>> Do you know if the firmware allows the kernel to be entered in EL2
>>> (which is the arm64 name for HYP)?
>>> So can we run kvm?
>>> If you have a booted kernel, can you grep for "EL2" and "kvm" in the dmesg?
>>
>> I do not have a rootfs yet (MMC v5 patches by Carlo are still waiting
>> for review), but with this change the KVM driver initializes okay - the
>> purpose of this patch!
>>
>>> Also you should merge this patch into 3/8, same for 8/8.
>>
>> If people confirm this is generally or specifically for this SoC correct
>> then sure. So far 3/8 seems a safe subset for lack of public documentation.
>
> The GIC is an integral part of the SoC, so this clearly belongs in there.
>
>>>> Signed-off-by: Andreas Färber <[email protected]>
>>>> ---
>>>> arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 +++-
>
> In general I was wondering if this naming is correct?
> Shouldn't it be something with "s905" in it? Because this the SoC that
> is driving all those hardware and the peripherals that you describe in
> there are clearly within the SoC.
> So something like meson-s905.dtsi or the like?

When I first submitted support for the meson8 and meson8b I picked up
the names according to the Amlogic SDK.
In the latest Amlogic drop this SoC is identified as meson-gxbb so
probably we should stick to this name.

--
Carlo Caione

2016-03-01 14:16:58

by Andre Przywara

[permalink] [raw]
Subject: Re: [PATCH 7/8] ARM64: dts: amlogic: Extend GXBaby GIC node

Hi,

On 01/03/16 12:53, Carlo Caione wrote:
> On Tue, Mar 1, 2016 at 1:43 PM, Andre Przywara <[email protected]> wrote:
>> Hi,
>>
>> On 01/03/16 11:18, Andreas Färber wrote:
>>> Hi Andre,
>>>
>>> Am 01.03.2016 um 12:01 schrieb Andre Przywara:
>>>> On 29/02/16 23:44, Andreas Färber wrote:
>>>>> Add GICH and GICV resources for HYP mode - guess based on other vendors.
>>>>
>>>> Do you know if the firmware allows the kernel to be entered in EL2
>>>> (which is the arm64 name for HYP)?
>>>> So can we run kvm?
>>>> If you have a booted kernel, can you grep for "EL2" and "kvm" in the dmesg?
>>>
>>> I do not have a rootfs yet (MMC v5 patches by Carlo are still waiting
>>> for review), but with this change the KVM driver initializes okay - the
>>> purpose of this patch!
>>>
>>>> Also you should merge this patch into 3/8, same for 8/8.
>>>
>>> If people confirm this is generally or specifically for this SoC correct
>>> then sure. So far 3/8 seems a safe subset for lack of public documentation.
>>
>> The GIC is an integral part of the SoC, so this clearly belongs in there.
>>
>>>>> Signed-off-by: Andreas Färber <[email protected]>
>>>>> ---
>>>>> arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 +++-
>>
>> In general I was wondering if this naming is correct?
>> Shouldn't it be something with "s905" in it? Because this the SoC that
>> is driving all those hardware and the peripherals that you describe in
>> there are clearly within the SoC.
>> So something like meson-s905.dtsi or the like?
>
> When I first submitted support for the meson8 and meson8b I picked up
> the names according to the Amlogic SDK.
> In the latest Amlogic drop this SoC is identified as meson-gxbb so
> probably we should stick to this name.

Oh, I guess in this case it's fine then. I was just wondering if we
should use a name that is more descriptive to the uninitiated reader.

Cheers,
Andre.

2016-03-01 22:47:05

by Andreas Färber

[permalink] [raw]
Subject: Re: [PATCH 7/8] ARM64: dts: amlogic: Extend GXBaby GIC node

Am 01.03.2016 um 13:43 schrieb Andre Przywara:
> On 01/03/16 11:18, Andreas Färber wrote:
>> Am 01.03.2016 um 12:01 schrieb Andre Przywara:
>>> On 29/02/16 23:44, Andreas Färber wrote:
>>>> reg = <0x0 0xc4301000 0 0x1000>,
>>>> - <0x0 0xc4302000 0 0x0100>;
>>>> + <0x0 0xc4302000 0 0x0100>,
>>>
>>> Please use 0x2000 for the size here. I guess this is really the GIC-400
>>> from ARM, and in this case this is the right size, [1] is the reference
>>> here. This will enable EOI mode 1 for KVM.
>>
>> Will test later.
>>
>> Is there any easy way to find out whether or not this is that GIC-400?
>
> If you can read registers: GICD_IIDR and PIDRx have some info:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0471b/CHDIFAEE.html
> So if your U-Boot for instance supports md, a dump of:
> md.l c4301008 1
> md.l c4301fd0 30
>
> would help to identify the GIC.

gxb_p200_v1#md.l c4301008 1
c4301008: 0200143b ;...
gxb_p200_v1#md.l c4301fd0 30
c4301fd0: 00000004 00000000 00000000 00000000 ................
c4301fe0: 00000090 000000b4 0000002b 00000000 ........+.......
c4301ff0: 0000000d 000000f0 00000005 000000b1 ................
c4302000: 00000060 000000f0 00000003 000003ff `...............
c4302010: 00000000 000000ff 000003ff 00000000 ................
c4302020: 00000000 00000000 00000000 00000000 ................
c4302030: 00000000 00000000 00000000 00000000 ................
c4302040: 00000000 00000000 00000000 00000000 ................
c4302050: 00000000 00000000 00000000 00000000 ................
c4302060: 00000000 00000000 00000000 00000000 ................
c4302070: 00000000 00000000 00000000 00000000 ................
c4302080: 00000000 00000000 00000000 00000000 ................

Cheers,
Andreas

--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

2016-03-01 22:59:26

by Andre Przywara

[permalink] [raw]
Subject: Re: [PATCH 7/8] ARM64: dts: amlogic: Extend GXBaby GIC node

On 01/03/16 22:46, Andreas Färber wrote:
> Am 01.03.2016 um 13:43 schrieb Andre Przywara:
>> On 01/03/16 11:18, Andreas Färber wrote:
>>> Am 01.03.2016 um 12:01 schrieb Andre Przywara:
>>>> On 29/02/16 23:44, Andreas Färber wrote:
>>>>> reg = <0x0 0xc4301000 0 0x1000>,
>>>>> - <0x0 0xc4302000 0 0x0100>;
>>>>> + <0x0 0xc4302000 0 0x0100>,
>>>>
>>>> Please use 0x2000 for the size here. I guess this is really the GIC-400
>>>> from ARM, and in this case this is the right size, [1] is the reference
>>>> here. This will enable EOI mode 1 for KVM.
>>>
>>> Will test later.
>>>
>>> Is there any easy way to find out whether or not this is that GIC-400?
>>
>> If you can read registers: GICD_IIDR and PIDRx have some info:
>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0471b/CHDIFAEE.html
>> So if your U-Boot for instance supports md, a dump of:
>> md.l c4301008 1
>> md.l c4301fd0 30
>>
>> would help to identify the GIC.
>
> gxb_p200_v1#md.l c4301008 1
> c4301008: 0200143b ;...
> gxb_p200_v1#md.l c4301fd0 30
> c4301fd0: 00000004 00000000 00000000 00000000 ................
> c4301fe0: 00000090 000000b4 0000002b 00000000 ........+.......
> c4301ff0: 0000000d 000000f0 00000005 000000b1 ................
> c4302000: 00000060 000000f0 00000003 000003ff `...............
> c4302010: 00000000 000000ff 000003ff 00000000 ................
> c4302020: 00000000 00000000 00000000 00000000 ................
> c4302030: 00000000 00000000 00000000 00000000 ................
> c4302040: 00000000 00000000 00000000 00000000 ................
> c4302050: 00000000 00000000 00000000 00000000 ................
> c4302060: 00000000 00000000 00000000 00000000 ................
> c4302070: 00000000 00000000 00000000 00000000 ................
> c4302080: 00000000 00000000 00000000 00000000 ................

Yes, that matches exactly the values from the GIC-400 TRM.

Thanks!
Andre.

2016-03-01 23:31:28

by Andreas Färber

[permalink] [raw]
Subject: Re: [PATCH 7/8] ARM64: dts: amlogic: Extend GXBaby GIC node

Hi,

Am 01.03.2016 um 13:43 schrieb Andre Przywara:
> On 01/03/16 11:18, Andreas Färber wrote:
>> Am 01.03.2016 um 12:01 schrieb Andre Przywara:
>>> On 29/02/16 23:44, Andreas Färber wrote:
>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>>> index 0ae089bd1806..5088ae3ff653 100644
>>>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>>> @@ -117,7 +117,9 @@
>>>> gic: interrupt-controller@c4301000 {
>>>> compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>>>
>>> I think "arm,gic-400" is the name to use here these days, especially for
>>> arm64.
>>
>> I took what /proc/device-tree showed on Android and verified that this
>> compatible is in use in mainline.
>
> Some vendor Android kernel is not a good reference for mainline work ;-)
> Better look at other DTs in arch/arm64/boot/dts.

Yes, that's what "in use in mainline" refers to:

$ git grep "cortex-a15-gic" -- arch/arm64/boot/dts/ | grep -v gic-400
arch/arm64/boot/dts/apm/apm-shadowcat.dtsi: compatible =
"arm,cortex-a15-gic";
arch/arm64/boot/dts/apm/apm-storm.dtsi: compatible = "arm,cortex-a15-gic";
arch/arm64/boot/dts/arm/foundation-v8.dts: compatible =
"arm,cortex-a15-gic", "arm,cortex-a9-gic";
arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts: compatible =
"arm,cortex-a15-gic", "arm,cortex-a9-gic";
arch/arm64/boot/dts/nvidia/tegra132.dtsi: compatible = "arm,cortex-a15-gic";

> You could keep "arm,cortex-a15-gic" in there if you care about
> compatibility with older (vendor) kernels, but I guess there are other
> issues which prevent this anyway, so you could drop this as well.

Yeah, I don't care about backwards compatibility with downstream
kernels, they use weird compatible strings with spaces anyway.

Regards,
Andreas

--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

2016-03-02 00:03:53

by Andre Przywara

[permalink] [raw]
Subject: Re: [PATCH 7/8] ARM64: dts: amlogic: Extend GXBaby GIC node

On 01/03/16 23:31, Andreas Färber wrote:
> Hi,
>
> Am 01.03.2016 um 13:43 schrieb Andre Przywara:
>> On 01/03/16 11:18, Andreas Färber wrote:
>>> Am 01.03.2016 um 12:01 schrieb Andre Przywara:
>>>> On 29/02/16 23:44, Andreas Färber wrote:
>>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>>>> index 0ae089bd1806..5088ae3ff653 100644
>>>>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>>>> @@ -117,7 +117,9 @@
>>>>> gic: interrupt-controller@c4301000 {
>>>>> compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>>>>
>>>> I think "arm,gic-400" is the name to use here these days, especially for
>>>> arm64.
>>>
>>> I took what /proc/device-tree showed on Android and verified that this
>>> compatible is in use in mainline.
>>
>> Some vendor Android kernel is not a good reference for mainline work ;-)
>> Better look at other DTs in arch/arm64/boot/dts.
>
> Yes, that's what "in use in mainline" refers to:
>
> $ git grep "cortex-a15-gic" -- arch/arm64/boot/dts/ | grep -v gic-400
> arch/arm64/boot/dts/apm/apm-shadowcat.dtsi: compatible =
> "arm,cortex-a15-gic";
> arch/arm64/boot/dts/apm/apm-storm.dtsi: compatible = "arm,cortex-a15-gic";
> arch/arm64/boot/dts/arm/foundation-v8.dts: compatible =
> "arm,cortex-a15-gic", "arm,cortex-a9-gic";
> arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts: compatible =
> "arm,cortex-a15-gic", "arm,cortex-a9-gic";
> arch/arm64/boot/dts/nvidia/tegra132.dtsi: compatible = "arm,cortex-a15-gic";

Which is mainly because those DTs predate the introduction of
"arm,gic-400". Other DTs kept the cortex strings in additionally to stay
compatible with older kernels.

>> You could keep "arm,cortex-a15-gic" in there if you care about
>> compatibility with older (vendor) kernels, but I guess there are other
>> issues which prevent this anyway, so you could drop this as well.
>
> Yeah, I don't care about backwards compatibility with downstream
> kernels, they use weird compatible strings with spaces anyway.

So please drop it and use only "arm,gic-400" to be in line with all the
other more recent SoCs.

Thanks!
Andre

2016-03-02 00:07:37

by Andreas Färber

[permalink] [raw]
Subject: Re: [PATCH 7/8] ARM64: dts: amlogic: Extend GXBaby GIC node

Am 02.03.2016 um 01:03 schrieb André Przywara:
> On 01/03/16 23:31, Andreas Färber wrote:
>> Am 01.03.2016 um 13:43 schrieb Andre Przywara:
>>> On 01/03/16 11:18, Andreas Färber wrote:
>>>> Am 01.03.2016 um 12:01 schrieb Andre Przywara:
>>>>> On 29/02/16 23:44, Andreas Färber wrote:
>>>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>>>>> index 0ae089bd1806..5088ae3ff653 100644
>>>>>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>>>>> @@ -117,7 +117,9 @@
>>>>>> gic: interrupt-controller@c4301000 {
>>>>>> compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>>>>>
>>>>> I think "arm,gic-400" is the name to use here these days, especially for
>>>>> arm64.
>>>>
>>>> I took what /proc/device-tree showed on Android and verified that this
>>>> compatible is in use in mainline.
>>>
>>> Some vendor Android kernel is not a good reference for mainline work ;-)
>>> Better look at other DTs in arch/arm64/boot/dts.
>>
>> Yes, that's what "in use in mainline" refers to:
>>
>> $ git grep "cortex-a15-gic" -- arch/arm64/boot/dts/ | grep -v gic-400
>> arch/arm64/boot/dts/apm/apm-shadowcat.dtsi: compatible =
>> "arm,cortex-a15-gic";
>> arch/arm64/boot/dts/apm/apm-storm.dtsi: compatible = "arm,cortex-a15-gic";
>> arch/arm64/boot/dts/arm/foundation-v8.dts: compatible =
>> "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>> arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts: compatible =
>> "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>> arch/arm64/boot/dts/nvidia/tegra132.dtsi: compatible = "arm,cortex-a15-gic";
>
> Which is mainly because those DTs predate the introduction of
> "arm,gic-400". Other DTs kept the cortex strings in additionally to stay
> compatible with older kernels.

Sure, but shouldn't we update them to have arm,gic-400 first then?

>>> You could keep "arm,cortex-a15-gic" in there if you care about
>>> compatibility with older (vendor) kernels, but I guess there are other
>>> issues which prevent this anyway, so you could drop this as well.
>>
>> Yeah, I don't care about backwards compatibility with downstream
>> kernels, they use weird compatible strings with spaces anyway.
>
> So please drop it and use only "arm,gic-400" to be in line with all the
> other more recent SoCs.

Already done, still inserting cbus/aobus nodes and moving uart nodes,
then need to re-test.

Cheers,
Andreas

--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)