[PATCH 0/2] Tegra234 fabric fixes

[PATCH 0/2] Tegra234 fabric fixes

[Ivy Huang]

From: Yi Ju Huang <yijuh@nvidia.com>

These patches fix issues related to sce-fabric and dce-fabric
nodes of Tegra234.

Sumit Gupta (2):
  arm64: tegra: fix typo in Tegra234 dce-fabric compatible
  arm64: tegra: disable Tegra234 sce-fabric node

 arch/arm64/boot/dts/nvidia/tegra234.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH 1/2] arm64: tegra: fix typo in Tegra234 dce-fabric compatible

[Ivy Huang]

From: Sumit Gupta <sumitg@nvidia.com>

Fix typo in the compatible string of Tegra234 dce-fabric.

Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
Signed-off-by: Ivy Huang <yijuh@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra234.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index 984c85eab41a..d08faf6bb505 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -3995,7 +3995,7 @@
 		};
 
 		dce-fabric@de00000 {
-			compatible = "nvidia,tegra234-sce-fabric";
+			compatible = "nvidia,tegra234-dce-fabric";
 			reg = <0x0 0xde00000 0x0 0x40000>;
 			interrupts = <GIC_SPI 381 IRQ_TYPE_LEVEL_HIGH>;
 			status = "okay";
-- 
2.17.1

Re: [PATCH 1/2] arm64: tegra: fix typo in Tegra234 dce-fabric compatible

[Brad Griffis]

On 12/12/24 16:03, Ivy Huang wrote:
> From: Sumit Gupta <sumitg@nvidia.com>
> 
> Fix typo in the compatible string of Tegra234 dce-fabric.
> 
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> Signed-off-by: Ivy Huang <yijuh@nvidia.com>

Reviewed-by: Brad Griffis <bgriffis@nvidia.com>

Re: [PATCH 1/2] arm64: tegra: fix typo in Tegra234 dce-fabric compatible

[Jon Hunter]

On 13/12/2024 00:03, Ivy Huang wrote:
> From: Sumit Gupta <sumitg@nvidia.com>
> 
> Fix typo in the compatible string of Tegra234 dce-fabric.
> 
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> Signed-off-by: Ivy Huang <yijuh@nvidia.com>
> ---
>   arch/arm64/boot/dts/nvidia/tegra234.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index 984c85eab41a..d08faf6bb505 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -3995,7 +3995,7 @@
>   		};
>   
>   		dce-fabric@de00000 {
> -			compatible = "nvidia,tegra234-sce-fabric";
> +			compatible = "nvidia,tegra234-dce-fabric";
>   			reg = <0x0 0xde00000 0x0 0x40000>;
>   			interrupts = <GIC_SPI 381 IRQ_TYPE_LEVEL_HIGH>;
>   			status = "okay";


LGTM! However, we are missing a 'Fixes' tag. We should add the following 
above the 'signed-off-by'.

Fixes: 302e154000ec ("arm64: tegra: Add node for CBB 2.0 on Tegra234")

Thierry, do you want to apply the fixes tag when merging? Could also be 
a candidate for stable.

Otherwise ...

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Jon

-- 
nvpublic

Re: [PATCH 1/2] arm64: tegra: fix typo in Tegra234 dce-fabric compatible

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]

On Fri, Dec 13, 2024 at 09:44:23AM +0000, Jon Hunter wrote:
> 
> On 13/12/2024 00:03, Ivy Huang wrote:
> > From: Sumit Gupta <sumitg@nvidia.com>
> > 
> > Fix typo in the compatible string of Tegra234 dce-fabric.
> > 
> > Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> > Signed-off-by: Ivy Huang <yijuh@nvidia.com>
> > ---
> >   arch/arm64/boot/dts/nvidia/tegra234.dtsi | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> > index 984c85eab41a..d08faf6bb505 100644
> > --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> > +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> > @@ -3995,7 +3995,7 @@
> >   		};
> >   		dce-fabric@de00000 {
> > -			compatible = "nvidia,tegra234-sce-fabric";
> > +			compatible = "nvidia,tegra234-dce-fabric";
> >   			reg = <0x0 0xde00000 0x0 0x40000>;
> >   			interrupts = <GIC_SPI 381 IRQ_TYPE_LEVEL_HIGH>;
> >   			status = "okay";
> 
> 
> LGTM! However, we are missing a 'Fixes' tag. We should add the following
> above the 'signed-off-by'.
> 
> Fixes: 302e154000ec ("arm64: tegra: Add node for CBB 2.0 on Tegra234")
> 
> Thierry, do you want to apply the fixes tag when merging? Could also be a
> candidate for stable.

Yeah, I can add the fixes tag, no need to resend. That should also take
care of it getting picked up into stable automatically.

Thierry

> Otherwise ...
> 
> Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
> 
> Jon
> 
> -- 
> nvpublic
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] arm64: tegra: fix typo in Tegra234 dce-fabric compatible

[Krzysztof Kozlowski]

On 13/12/2024 11:22, Thierry Reding wrote:
>>>
>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
>>> index 984c85eab41a..d08faf6bb505 100644
>>> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
>>> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
>>> @@ -3995,7 +3995,7 @@
>>>   		};
>>>   		dce-fabric@de00000 {
>>> -			compatible = "nvidia,tegra234-sce-fabric";
>>> +			compatible = "nvidia,tegra234-dce-fabric";
>>>   			reg = <0x0 0xde00000 0x0 0x40000>;
>>>   			interrupts = <GIC_SPI 381 IRQ_TYPE_LEVEL_HIGH>;
>>>   			status = "okay";
>>
>>
>> LGTM! However, we are missing a 'Fixes' tag. We should add the following
>> above the 'signed-off-by'.
>>
>> Fixes: 302e154000ec ("arm64: tegra: Add node for CBB 2.0 on Tegra234")
>>
>> Thierry, do you want to apply the fixes tag when merging? Could also be a
>> candidate for stable.
> 
> Yeah, I can add the fixes tag, no need to resend. That should also take
> care of it getting picked up into stable automatically.

No, this changed some days ago. If this is a real fix which should go to
stable, then with proper explanation what is the bug and how it affects
users, you need cc-stable tag.

Best regards,
Krzysztof

Re: [PATCH 1/2] arm64: tegra: fix typo in Tegra234 dce-fabric compatible

[Krzysztof Kozlowski]

On 13/12/2024 01:03, Ivy Huang wrote:
> From: Sumit Gupta <sumitg@nvidia.com>
> 
> Fix typo in the compatible string of Tegra234 dce-fabric.

In what way fix? How does it affect users?

Why no fixes tag?

You CC-ed an address, which suggests you do not work on mainline kernel
or you do not use get_maintainers.pl/b4/patman. Please rebase and always
work on mainline or start using mentioned tools, so correct addresses
will be used.

Best regards,
Krzysztof

Re: [PATCH 1/2] arm64: tegra: fix typo in Tegra234 dce-fabric compatible

[Brad Griffis]

On 12/13/24 03:10, Krzysztof Kozlowski wrote:
> In what way fix? How does it affect users?
> 
> Why no fixes tag?

Thanks for the review. Ivy has addressed these in v2.

[PATCH 2/2] arm64: tegra: disable Tegra234 sce-fabric node

[Ivy Huang]

From: Sumit Gupta <sumitg@nvidia.com>

Access to safety cluster engine (SCE) fabric registers was blocked
by firewall after the introduction of Functional Safety Island in
Tegra234. After that, any access by software to SCE registers is
correctly resulting in the internal bus error. However, when CPUs
try accessing the SCE-fabric registers to print error info,
another firewall error occurs as the fabric registers are also
firewall protected. This results in a second error to be printed.
Disable the SCE fabric node to avoid printing the misleading error.
The first error info will be printed by the interrupt from the
fabric causing the actual access.

Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
Signed-off-by: Ivy Huang <yijuh@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra234.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index d08faf6bb505..05a771ab1ed5 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -3815,7 +3815,7 @@
 			compatible = "nvidia,tegra234-sce-fabric";
 			reg = <0x0 0xb600000 0x0 0x40000>;
 			interrupts = <GIC_SPI 173 IRQ_TYPE_LEVEL_HIGH>;
-			status = "okay";
+			status = "disabled";
 		};
 
 		rce-fabric@be00000 {
-- 
2.17.1

Re: [PATCH 2/2] arm64: tegra: disable Tegra234 sce-fabric node

[Brad Griffis]

On 12/12/24 16:03, Ivy Huang wrote:
> From: Sumit Gupta <sumitg@nvidia.com>
> 
> Access to safety cluster engine (SCE) fabric registers was blocked
> by firewall after the introduction of Functional Safety Island in
> Tegra234. After that, any access by software to SCE registers is
> correctly resulting in the internal bus error. However, when CPUs
> try accessing the SCE-fabric registers to print error info,
> another firewall error occurs as the fabric registers are also
> firewall protected. This results in a second error to be printed.
> Disable the SCE fabric node to avoid printing the misleading error.
> The first error info will be printed by the interrupt from the
> fabric causing the actual access.
> 
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> Signed-off-by: Ivy Huang <yijuh@nvidia.com>

Reviewed-by: Brad Griffis <bgriffis@nvidia.com>

Re: [PATCH 2/2] arm64: tegra: disable Tegra234 sce-fabric node

[Jon Hunter]

On 13/12/2024 00:03, Ivy Huang wrote:
> From: Sumit Gupta <sumitg@nvidia.com>
> 
> Access to safety cluster engine (SCE) fabric registers was blocked
> by firewall after the introduction of Functional Safety Island in
> Tegra234. After that, any access by software to SCE registers is
> correctly resulting in the internal bus error. However, when CPUs
> try accessing the SCE-fabric registers to print error info,
> another firewall error occurs as the fabric registers are also
> firewall protected. This results in a second error to be printed.
> Disable the SCE fabric node to avoid printing the misleading error.
> The first error info will be printed by the interrupt from the
> fabric causing the actual access.
> 
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> Signed-off-by: Ivy Huang <yijuh@nvidia.com>
> ---
>   arch/arm64/boot/dts/nvidia/tegra234.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index d08faf6bb505..05a771ab1ed5 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -3815,7 +3815,7 @@
>   			compatible = "nvidia,tegra234-sce-fabric";
>   			reg = <0x0 0xb600000 0x0 0x40000>;
>   			interrupts = <GIC_SPI 173 IRQ_TYPE_LEVEL_HIGH>;
> -			status = "okay";
> +			status = "disabled";
>   		};
>   
>   		rce-fabric@be00000 {


We need the same fixes tag on this one as well ...

Fixes: 302e154000ec ("arm64: tegra: Add node for CBB 2.0 on Tegra234")

Otherwise ...

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Jon

-- 
nvpublic

Re: [PATCH 2/2] arm64: tegra: disable Tegra234 sce-fabric node

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2349 bytes --]

On Fri, Dec 13, 2024 at 12:03:05AM +0000, Ivy Huang wrote:
> From: Sumit Gupta <sumitg@nvidia.com>
> 
> Access to safety cluster engine (SCE) fabric registers was blocked
> by firewall after the introduction of Functional Safety Island in
> Tegra234. After that, any access by software to SCE registers is
> correctly resulting in the internal bus error. However, when CPUs
> try accessing the SCE-fabric registers to print error info,
> another firewall error occurs as the fabric registers are also
> firewall protected. This results in a second error to be printed.
> Disable the SCE fabric node to avoid printing the misleading error.
> The first error info will be printed by the interrupt from the
> fabric causing the actual access.
> 
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> Signed-off-by: Ivy Huang <yijuh@nvidia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra234.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index d08faf6bb505..05a771ab1ed5 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -3815,7 +3815,7 @@
>  			compatible = "nvidia,tegra234-sce-fabric";
>  			reg = <0x0 0xb600000 0x0 0x40000>;
>  			interrupts = <GIC_SPI 173 IRQ_TYPE_LEVEL_HIGH>;
> -			status = "okay";
> +			status = "disabled";
>  		};
>  
>  		rce-fabric@be00000 {

Hm... so what's the purpose of having this here if we can't use it? Are
there cases where we might want to access this? For example, could some
firmware *not* firewall this in some use-case, and would we want to use
this for error reporting in such cases?

I don't have a strong opinion on keeping this while being disabled. It
is a fairly small node, so it doesn't hurt much from that point of view,
but overall this patch seems like it's taking the easy way out.

For example if there's ever a case where we might want to use this, then
there should be some other entity (UEFI?) setting status = "disabled" at
runtime. Or perhaps it should be setting status = "okay" if it is not
firewalled. Generally if there's no mechanism that would ever change
status from "disabled" to "okay" at runtime, there's really no point in
having the node in DT.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] arm64: tegra: disable Tegra234 sce-fabric node

[Brad Griffis]

On 12/13/24 02:28, Thierry Reding wrote:
> Hm... so what's the purpose of having this here if we can't use it? Are
> there cases where we might want to access this? For example, could some
> firmware *not* firewall this in some use-case, and would we want to use
> this for error reporting in such cases?

Given that this has changed once, it has hard to say definitively 
whether it might ever change again. I think keeping it around in a 
disabled state is reasonable.

I also find it typical that most things in the soc.dtsi are disabled by 
default such that they are enabled as needed by upper layers.