diff mbox series

arm64: dts: exynos: correct GIC CPU interfaces address range on Exynos7

Message ID 20210805072110.4730-1-krzysztof.kozlowski@canonical.com (mailing list archive)
State New, archived
Headers show
Series arm64: dts: exynos: correct GIC CPU interfaces address range on Exynos7 | expand

Commit Message

Krzysztof Kozlowski Aug. 5, 2021, 7:21 a.m. UTC
The GIC-400 CPU interfaces address range is defined as 0x2000-0x3FFF (by
ARM).

Reported-by: Sam Protsenko <semen.protsenko@linaro.org>
Reported-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Krzysztof Kozlowski Aug. 5, 2021, 7:36 a.m. UTC | #1
On 05/08/2021 09:21, Krzysztof Kozlowski wrote:
> The GIC-400 CPU interfaces address range is defined as 0x2000-0x3FFF (by
> ARM).
> 

I underestimated the issue - this is actually bug as there is a GICC_DIR
register at offset 0x1000. Therefore:

Fixes: b9024cbc937d ("arm64: dts: Add initial device tree support for exynos7")

> Reported-by: Sam Protsenko <semen.protsenko@linaro.org>
> Reported-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi b/arch/arm64/boot/dts/exynos/exynos7.dtsi
> index 8b06397ba6e7..c73a597ca66e 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
> @@ -137,7 +137,7 @@ gic: interrupt-controller@11001000 {
>  			#address-cells = <0>;
>  			interrupt-controller;
>  			reg =	<0x11001000 0x1000>,
> -				<0x11002000 0x1000>,
> +				<0x11002000 0x2000>,
>  				<0x11004000 0x2000>,
>  				<0x11006000 0x2000>;
>  		};
> 


Best regards,
Krzysztof
Alim Akhtar Aug. 5, 2021, 9:26 a.m. UTC | #2
Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Sent: 05 August 2021 13:07
> To: Rob Herring <robh+dt@kernel.org>; devicetree@vger.kernel.org; linux-
> arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: Alim Akhtar <alim.akhtar@samsung.com>; Chanwoo Choi
> <cw00.choi@samsung.com>; Pankaj Dubey <pankaj.dubey@samsung.com>;
> Sam Protsenko <semen.protsenko@linaro.org>; Marc Zyngier
> <maz@kernel.org>
> Subject: Re: [PATCH] arm64: dts: exynos: correct GIC CPU interfaces address
> range on Exynos7
> 
> On 05/08/2021 09:21, Krzysztof Kozlowski wrote:
> > The GIC-400 CPU interfaces address range is defined as 0x2000-0x3FFF
> > (by ARM).
> >
> 
Looking at DDI0471B_gic400_r0p1_trm.pdf, CPU interface range is 0x0000 ~ 0x10000

> I underestimated the issue - this is actually bug as there is a GICC_DIR
> register at offset 0x1000. Therefore:
> 
Looking at the exynos7 and exynos5433 UMs looks like GICC_DIR is at offset 0x2100 (from 0x1100_0000 GIC base)
It is possible for you to cross check once?

> Fixes: b9024cbc937d ("arm64: dts: Add initial device tree support for
> exynos7")
> 
> > Reported-by: Sam Protsenko <semen.protsenko@linaro.org>
> > Reported-by: Marc Zyngier <maz@kernel.org>
> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> > ---
> >  arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi
> > b/arch/arm64/boot/dts/exynos/exynos7.dtsi
> > index 8b06397ba6e7..c73a597ca66e 100644
> > --- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
> > +++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
> > @@ -137,7 +137,7 @@ gic: interrupt-controller@11001000 {
> >  			#address-cells = <0>;
> >  			interrupt-controller;
> >  			reg =	<0x11001000 0x1000>,
> > -				<0x11002000 0x1000>,
> > +				<0x11002000 0x2000>,
> >  				<0x11004000 0x2000>,
> >  				<0x11006000 0x2000>;
> >  		};
> >
> 
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Aug. 5, 2021, 9:43 a.m. UTC | #3
On 05/08/2021 11:26, Alim Akhtar wrote:
> Hi Krzysztof,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>> Sent: 05 August 2021 13:07
>> To: Rob Herring <robh+dt@kernel.org>; devicetree@vger.kernel.org; linux-
>> arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Cc: Alim Akhtar <alim.akhtar@samsung.com>; Chanwoo Choi
>> <cw00.choi@samsung.com>; Pankaj Dubey <pankaj.dubey@samsung.com>;
>> Sam Protsenko <semen.protsenko@linaro.org>; Marc Zyngier
>> <maz@kernel.org>
>> Subject: Re: [PATCH] arm64: dts: exynos: correct GIC CPU interfaces address
>> range on Exynos7
>>
>> On 05/08/2021 09:21, Krzysztof Kozlowski wrote:
>>> The GIC-400 CPU interfaces address range is defined as 0x2000-0x3FFF
>>> (by ARM).
>>>
>>
> Looking at DDI0471B_gic400_r0p1_trm.pdf, CPU interface range is 0x0000 ~ 0x10000
> 
>> I underestimated the issue - this is actually bug as there is a GICC_DIR
>> register at offset 0x1000. Therefore:
>>
> Looking at the exynos7 and exynos5433 UMs looks like GICC_DIR is at offset 0x2100 (from 0x1100_0000 GIC base)
> It is possible for you to cross check once?
> 

That's a mistake in Exynos manual. GICC_DIR is at 0x1000:
https://developer.arm.com/documentation/ddi0471/b/programmers-model/cpu-interface-register-summary

We have discussion here:
https://lore.kernel.org/linux-samsung-soc/0277c701-cc25-cdc5-d3b9-cf2cc2ba4de5@canonical.com/T/#m1ced9a28bed27f5cf74e281fb68efe1b57d5609e

Range of 0x10000 is definitely wrong as it overlaps with two other
ranges.


Best regards,
Krzysztof
Krzysztof Kozlowski Aug. 5, 2021, 9:44 a.m. UTC | #4
On 05/08/2021 11:43, Krzysztof Kozlowski wrote:
> On 05/08/2021 11:26, Alim Akhtar wrote:
>> Hi Krzysztof,
>>
>>> -----Original Message-----
>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>>> Sent: 05 August 2021 13:07
>>> To: Rob Herring <robh+dt@kernel.org>; devicetree@vger.kernel.org; linux-
>>> arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
>>> kernel@vger.kernel.org
>>> Cc: Alim Akhtar <alim.akhtar@samsung.com>; Chanwoo Choi
>>> <cw00.choi@samsung.com>; Pankaj Dubey <pankaj.dubey@samsung.com>;
>>> Sam Protsenko <semen.protsenko@linaro.org>; Marc Zyngier
>>> <maz@kernel.org>
>>> Subject: Re: [PATCH] arm64: dts: exynos: correct GIC CPU interfaces address
>>> range on Exynos7
>>>
>>> On 05/08/2021 09:21, Krzysztof Kozlowski wrote:
>>>> The GIC-400 CPU interfaces address range is defined as 0x2000-0x3FFF
>>>> (by ARM).
>>>>
>>>
>> Looking at DDI0471B_gic400_r0p1_trm.pdf, CPU interface range is 0x0000 ~ 0x10000
>>
>>> I underestimated the issue - this is actually bug as there is a GICC_DIR
>>> register at offset 0x1000. Therefore:
>>>
>> Looking at the exynos7 and exynos5433 UMs looks like GICC_DIR is at offset 0x2100 (from 0x1100_0000 GIC base)
>> It is possible for you to cross check once?
>>
> 
> That's a mistake in Exynos manual. GICC_DIR is at 0x1000:

0x1000 from CPU base offset, so 0x3000 from GIC base.

> https://developer.arm.com/documentation/ddi0471/b/programmers-model/cpu-interface-register-summary
> 
> We have discussion here:
> https://lore.kernel.org/linux-samsung-soc/0277c701-cc25-cdc5-d3b9-cf2cc2ba4de5@canonical.com/T/#m1ced9a28bed27f5cf74e281fb68efe1b57d5609e
> 
> Range of 0x10000 is definitely wrong as it overlaps with two other
> ranges.


Best regards,
Krzysztof
Alim Akhtar Aug. 5, 2021, 9:54 a.m. UTC | #5
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Sent: 05 August 2021 15:15
> To: Alim Akhtar <alim.akhtar@samsung.com>; 'Rob Herring'
> <robh+dt@kernel.org>; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: 'Chanwoo Choi' <cw00.choi@samsung.com>; 'Pankaj Dubey'
> <pankaj.dubey@samsung.com>; 'Sam Protsenko'
> <semen.protsenko@linaro.org>; 'Marc Zyngier' <maz@kernel.org>
> Subject: Re: [PATCH] arm64: dts: exynos: correct GIC CPU interfaces address
> range on Exynos7
> 
> On 05/08/2021 11:43, Krzysztof Kozlowski wrote:
> > On 05/08/2021 11:26, Alim Akhtar wrote:
> >> Hi Krzysztof,
> >>
> >>> -----Original Message-----
> >>> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> >>> Sent: 05 August 2021 13:07
> >>> To: Rob Herring <robh+dt@kernel.org>; devicetree@vger.kernel.org;
> >>> linux- arm-kernel@lists.infradead.org;
> >>> linux-samsung-soc@vger.kernel.org; linux- kernel@vger.kernel.org
> >>> Cc: Alim Akhtar <alim.akhtar@samsung.com>; Chanwoo Choi
> >>> <cw00.choi@samsung.com>; Pankaj Dubey
> <pankaj.dubey@samsung.com>;
> >>> Sam Protsenko <semen.protsenko@linaro.org>; Marc Zyngier
> >>> <maz@kernel.org>
> >>> Subject: Re: [PATCH] arm64: dts: exynos: correct GIC CPU interfaces
> >>> address range on Exynos7
> >>>
> >>> On 05/08/2021 09:21, Krzysztof Kozlowski wrote:
> >>>> The GIC-400 CPU interfaces address range is defined as
> >>>> 0x2000-0x3FFF (by ARM).
> >>>>
> >>>
> >> Looking at DDI0471B_gic400_r0p1_trm.pdf, CPU interface range is
> >> 0x0000 ~ 0x10000
> >>
> >>> I underestimated the issue - this is actually bug as there is a
> >>> GICC_DIR register at offset 0x1000. Therefore:
> >>>
> >> Looking at the exynos7 and exynos5433 UMs looks like GICC_DIR is at
> >> offset 0x2100 (from 0x1100_0000 GIC base) It is possible for you to cross
> check once?
> >>
> >
> > That's a mistake in Exynos manual. GICC_DIR is at 0x1000:
> 
> 0x1000 from CPU base offset, so 0x3000 from GIC base.
> 
> > https://developer.arm.com/documentation/ddi0471/b/programmers-
> model/cp
> > u-interface-register-summary
> >
> > We have discussion here:
> > https://lore.kernel.org/linux-samsung-soc/0277c701-cc25-cdc5-d3b9-cf2c
> >
> c2ba4de5@canonical.com/T/#m1ced9a28bed27f5cf74e281fb68efe1b57d5609
> e
Thanks for pointing to this discussion, I missed this.
Agreed.
Feel Free to add
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

> >
> > Range of 0x10000 is definitely wrong as it overlaps with two other
> > ranges.
> 
> 
> Best regards,
> Krzysztof
Marc Zyngier Aug. 5, 2021, 10:01 a.m. UTC | #6
On Thu, 05 Aug 2021 10:26:14 +0100,
"Alim Akhtar" <alim.akhtar@samsung.com> wrote:
> 
> Hi Krzysztof,
> 
> > -----Original Message-----
> > From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> > Sent: 05 August 2021 13:07
> > To: Rob Herring <robh+dt@kernel.org>; devicetree@vger.kernel.org; linux-
> > arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Cc: Alim Akhtar <alim.akhtar@samsung.com>; Chanwoo Choi
> > <cw00.choi@samsung.com>; Pankaj Dubey <pankaj.dubey@samsung.com>;
> > Sam Protsenko <semen.protsenko@linaro.org>; Marc Zyngier
> > <maz@kernel.org>
> > Subject: Re: [PATCH] arm64: dts: exynos: correct GIC CPU interfaces address
> > range on Exynos7
> > 
> > On 05/08/2021 09:21, Krzysztof Kozlowski wrote:
> > > The GIC-400 CPU interfaces address range is defined as 0x2000-0x3FFF
> > > (by ARM).
> > >
> > 
> Looking at DDI0471B_gic400_r0p1_trm.pdf, CPU interface range is
> 0x0000 ~ 0x10000

I don't where you are getting this range from. The only 64kB range I'm
aware of is the optional integration trick to cope with 64kB pages
that was documented in the initial SBSA spec. The HW itself only
decodes 8kB for the CPU interface.

	M.
Alim Akhtar Aug. 5, 2021, 10:51 a.m. UTC | #7
> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: 05 August 2021 15:31
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: 'Krzysztof Kozlowski' <krzysztof.kozlowski@canonical.com>; 'Rob
Herring'
> <robh+dt@kernel.org>; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; 'Chanwoo Choi' <cw00.choi@samsung.com>;
> 'Pankaj Dubey' <pankaj.dubey@samsung.com>; 'Sam Protsenko'
> <semen.protsenko@linaro.org>
> Subject: Re: [PATCH] arm64: dts: exynos: correct GIC CPU interfaces
address
> range on Exynos7
> 
> On Thu, 05 Aug 2021 10:26:14 +0100,
> "Alim Akhtar" <alim.akhtar@samsung.com> wrote:
> >
> > Hi Krzysztof,
> >
> > > -----Original Message-----
> > > From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> > > Sent: 05 August 2021 13:07
> > > To: Rob Herring <robh+dt@kernel.org>; devicetree@vger.kernel.org;
> > > linux- arm-kernel@lists.infradead.org;
> > > linux-samsung-soc@vger.kernel.org; linux- kernel@vger.kernel.org
> > > Cc: Alim Akhtar <alim.akhtar@samsung.com>; Chanwoo Choi
> > > <cw00.choi@samsung.com>; Pankaj Dubey
> <pankaj.dubey@samsung.com>;
> > > Sam Protsenko <semen.protsenko@linaro.org>; Marc Zyngier
> > > <maz@kernel.org>
> > > Subject: Re: [PATCH] arm64: dts: exynos: correct GIC CPU interfaces
> > > address range on Exynos7
> > >
> > > On 05/08/2021 09:21, Krzysztof Kozlowski wrote:
> > > > The GIC-400 CPU interfaces address range is defined as
> > > > 0x2000-0x3FFF (by ARM).
> > > >
> > >
> > Looking at DDI0471B_gic400_r0p1_trm.pdf, CPU interface range is
> > 0x0000 ~ 0x10000
> 
> I don't where you are getting this range from. The only 64kB range I'm
aware
> of is the optional integration trick to cope with 64kB pages that was
> documented in the initial SBSA spec. The HW itself only decodes 8kB for
the
> CPU interface.
> 
There was a typo in above range, thanks for the clarification. Got it.

> 	M.
> 
> --
> Without deviation from the norm, progress is not possible.
Sam Protsenko Aug. 5, 2021, 11:26 a.m. UTC | #8
Hi Krzysztof,

On Thu, 5 Aug 2021 at 10:22, Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> The GIC-400 CPU interfaces address range is defined as 0x2000-0x3FFF (by
> ARM).
>
> Reported-by: Sam Protsenko <semen.protsenko@linaro.org>
> Reported-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

>  arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi b/arch/arm64/boot/dts/exynos/exynos7.dtsi
> index 8b06397ba6e7..c73a597ca66e 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
> @@ -137,7 +137,7 @@ gic: interrupt-controller@11001000 {
>                         #address-cells = <0>;
>                         interrupt-controller;
>                         reg =   <0x11001000 0x1000>,
> -                               <0x11002000 0x1000>,
> +                               <0x11002000 0x2000>,
>                                 <0x11004000 0x2000>,
>                                 <0x11006000 0x2000>;
>                 };
> --
> 2.30.2
>
Krzysztof Kozlowski Aug. 9, 2021, 10:38 a.m. UTC | #9
On Thu, 5 Aug 2021 09:21:10 +0200, Krzysztof Kozlowski wrote:
> The GIC-400 CPU interfaces address range is defined as 0x2000-0x3FFF (by
> ARM).

Applied, thanks!

[1/1] arm64: dts: exynos: correct GIC CPU interfaces address range on Exynos7
      commit: 01c72cad790cb6cd3ccbe4c1402b6cb6c6bbffd0

Best regards,
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi b/arch/arm64/boot/dts/exynos/exynos7.dtsi
index 8b06397ba6e7..c73a597ca66e 100644
--- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
@@ -137,7 +137,7 @@  gic: interrupt-controller@11001000 {
 			#address-cells = <0>;
 			interrupt-controller;
 			reg =	<0x11001000 0x1000>,
-				<0x11002000 0x1000>,
+				<0x11002000 0x2000>,
 				<0x11004000 0x2000>,
 				<0x11006000 0x2000>;
 		};