[PATCH] arm64: tegra: Fix Tegra234 PCIe interrupt-map

[PATCH] arm64: tegra: Fix Tegra234 PCIe interrupt-map

[Brad Griffis]

For interrupt-map entries, the DTS specification requires
that #address-cells is defined for both the child node and the
interrupt parent.  For the PCIe interrupt-map entries, the parent
node ("gic") has not specified #address-cells. The existing layout
of the PCIe interrupt-map entries indicates that it assumes
that #address-cells is zero for this node.

Explicitly set #address-cells to zero for "gic" so that it complies
with the device tree specification.

Signed-off-by: Brad Griffis <bgriffis@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra234.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index 984c85eab41a..e1c07c99e9bd 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -4010,6 +4010,7 @@ ccplex@e000000 {
 
 		gic: interrupt-controller@f400000 {
 			compatible = "arm,gic-v3";
+			#address-cells = <0>;
 			reg = <0x0 0x0f400000 0x0 0x010000>, /* GICD */
 			      <0x0 0x0f440000 0x0 0x200000>; /* GICR */
 			interrupt-parent = <&gic>;
-- 
2.34.1

Re: [PATCH] arm64: tegra: Fix Tegra234 PCIe interrupt-map

[Jon Hunter]

On 12/12/2024 21:11, Brad Griffis wrote:
> For interrupt-map entries, the DTS specification requires
> that #address-cells is defined for both the child node and the
> interrupt parent.  For the PCIe interrupt-map entries, the parent
> node ("gic") has not specified #address-cells. The existing layout
> of the PCIe interrupt-map entries indicates that it assumes
> that #address-cells is zero for this node.
> 
> Explicitly set #address-cells to zero for "gic" so that it complies
> with the device tree specification.
> 
> Signed-off-by: Brad Griffis <bgriffis@nvidia.com>
> ---
>   arch/arm64/boot/dts/nvidia/tegra234.dtsi | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index 984c85eab41a..e1c07c99e9bd 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -4010,6 +4010,7 @@ ccplex@e000000 {
>   
>   		gic: interrupt-controller@f400000 {
>   			compatible = "arm,gic-v3";
> +			#address-cells = <0>;
>   			reg = <0x0 0x0f400000 0x0 0x010000>, /* GICD */
>   			      <0x0 0x0f440000 0x0 0x200000>; /* GICR */
>   			interrupt-parent = <&gic>;


Can you add the fixes tag for this?

BTW, I had a quick look and this appears to be a common problem across 
all Tegra devices, Tegra210, Tegra186 and Tegra194. Should we fix up all 
while we are at it?

Thanks
Jon

-- 
nvpublic

Re: [PATCH] arm64: tegra: Fix Tegra234 PCIe interrupt-map

[Thierry Reding]

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

On Fri, Dec 13, 2024 at 09:51:30AM +0000, Jon Hunter wrote:
> 
> On 12/12/2024 21:11, Brad Griffis wrote:
> > For interrupt-map entries, the DTS specification requires
> > that #address-cells is defined for both the child node and the
> > interrupt parent.  For the PCIe interrupt-map entries, the parent
> > node ("gic") has not specified #address-cells. The existing layout
> > of the PCIe interrupt-map entries indicates that it assumes
> > that #address-cells is zero for this node.
> > 
> > Explicitly set #address-cells to zero for "gic" so that it complies
> > with the device tree specification.
> > 
> > Signed-off-by: Brad Griffis <bgriffis@nvidia.com>
> > ---
> >   arch/arm64/boot/dts/nvidia/tegra234.dtsi | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> > index 984c85eab41a..e1c07c99e9bd 100644
> > --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> > +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> > @@ -4010,6 +4010,7 @@ ccplex@e000000 {
> >   		gic: interrupt-controller@f400000 {
> >   			compatible = "arm,gic-v3";
> > +			#address-cells = <0>;
> >   			reg = <0x0 0x0f400000 0x0 0x010000>, /* GICD */
> >   			      <0x0 0x0f440000 0x0 0x200000>; /* GICR */
> >   			interrupt-parent = <&gic>;
> 
> 
> Can you add the fixes tag for this?
> 
> BTW, I had a quick look and this appears to be a common problem across all
> Tegra devices, Tegra210, Tegra186 and Tegra194. Should we fix up all while
> we are at it?

Yeah, I think we should fix these up going as far back as Tegra20. I
don't think we've ever seen any fallout from this being broken and I
don't exactly understand why. I'll dig through some code and run some
tests to see if I can get to the bottom of this.

I can take care of fixing up the older boards.

Thierry

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

Re: [PATCH] arm64: tegra: Fix Tegra234 PCIe interrupt-map

[Krzysztof Kozlowski]

On 12/12/2024 22:11, Brad Griffis wrote:
> For interrupt-map entries, the DTS specification requires
> that #address-cells is defined for both the child node and the
> interrupt parent.  For the PCIe interrupt-map entries, the parent
> node ("gic") has not specified #address-cells. The existing layout
> of the PCIe interrupt-map entries indicates that it assumes
> that #address-cells is zero for this node.
> 
> Explicitly set #address-cells to zero for "gic" so that it complies
> with the device tree specification.
> 
> Signed-off-by: Brad Griffis <bgriffis@nvidia.com>
> ---

So entire nvidia works on old kernel and sends patches rebased on old
kernel? Please start working on mainline.

I give the same feedback multiple times and still nvidia repeats the
same mistakes.

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] arm64: tegra: Fix Tegra234 PCIe interrupt-map

[Jon Hunter]

Hi Krzysztof!

On 13/12/2024 11:11, Krzysztof Kozlowski wrote:
> On 12/12/2024 22:11, Brad Griffis wrote:
>> For interrupt-map entries, the DTS specification requires
>> that #address-cells is defined for both the child node and the
>> interrupt parent.  For the PCIe interrupt-map entries, the parent
>> node ("gic") has not specified #address-cells. The existing layout
>> of the PCIe interrupt-map entries indicates that it assumes
>> that #address-cells is zero for this node.
>>
>> Explicitly set #address-cells to zero for "gic" so that it complies
>> with the device tree specification.
>>
>> Signed-off-by: Brad Griffis <bgriffis@nvidia.com>
>> ---
> 
> So entire nvidia works on old kernel and sends patches rebased on old
> kernel? Please start working on mainline.


You know that is not the case! Yes there may be a few cases of people 
using your old email, but not all of us :-)

> I give the same feedback multiple times and still nvidia repeats the
> same mistakes.
> 
> 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.

This applies on top of today's -next and so I don't see any need to resend.

FYI, Brad has been working improve our testing for mainline and -next 
and so I know he is working on mainline.

So are we perfect? No. But we are working on getting better!

Cheers
Jon

-- 
nvpublic

Re: [PATCH] arm64: tegra: Fix Tegra234 PCIe interrupt-map

[Krzysztof Kozlowski]

On 13/12/2024 12:37, Jon Hunter wrote:
> Hi Krzysztof!
> 
> On 13/12/2024 11:11, Krzysztof Kozlowski wrote:
>> On 12/12/2024 22:11, Brad Griffis wrote:
>>> For interrupt-map entries, the DTS specification requires
>>> that #address-cells is defined for both the child node and the
>>> interrupt parent.  For the PCIe interrupt-map entries, the parent
>>> node ("gic") has not specified #address-cells. The existing layout
>>> of the PCIe interrupt-map entries indicates that it assumes
>>> that #address-cells is zero for this node.
>>>
>>> Explicitly set #address-cells to zero for "gic" so that it complies
>>> with the device tree specification.
>>>
>>> Signed-off-by: Brad Griffis <bgriffis@nvidia.com>
>>> ---
>>
>> So entire nvidia works on old kernel and sends patches rebased on old
>> kernel? Please start working on mainline.
> 
> 
> You know that is not the case! Yes there may be a few cases of people 
> using your old email, but not all of us :-)

Yeah, that was just an assumption, but three independent people are
Ccing address which does not exist in the kernel and it is impossible to
get/deduce/invent. Three patchsets from three different people...

Best regards,
Krzysztof

Re: [PATCH] arm64: tegra: Fix Tegra234 PCIe interrupt-map

[Brad Griffis]

On 12/13/24 05:26, Krzysztof Kozlowski wrote:
> Yeah, that was just an assumption, but three independent people are
> Ccing address which does not exist in the kernel and it is impossible to
> get/deduce/invent. Three patchsets from three different people...
> 
> Best regards,
> Krzysztof

I am developing and testing on mainline.  However, when I checked the 
get_maintainer.pl output on first glance it looked unchanged and I 
copied/pasted an old "git send-email" command that had the older email 
address.

And you are probably getting multiple of them due to me "helping" other 
people by sharing some of the commands/syntax.

I apologize for the mistake.  I will check that more carefully from now 
on. I sent out v2 and made sure to get this correct!

Re: [PATCH] arm64: tegra: Fix Tegra234 PCIe interrupt-map

[Krzysztof Kozlowski]

On 14/12/2024 01:35, Brad Griffis wrote:
> On 12/13/24 05:26, Krzysztof Kozlowski wrote:
>> Yeah, that was just an assumption, but three independent people are
>> Ccing address which does not exist in the kernel and it is impossible to
>> get/deduce/invent. Three patchsets from three different people...
>>
>> Best regards,
>> Krzysztof
> 
> I am developing and testing on mainline.  However, when I checked the 
> get_maintainer.pl output on first glance it looked unchanged and I 
> copied/pasted an old "git send-email" command that had the older email 
> address.


You should not "cache" the output of get_maintainers.pl EVER. Addresses
change, people are added or removed. Just use b4 or simple
scripts/aliases (like Rob's send email identity or
https://github.com/krzk/tools/blob/master/linux/.bash_aliases_linux#L92 ).

Best regards,
Krzysztof