| Message ID | ZvQ27pvrnEYA8BB9@Emma (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm: dts: broadcom: Add missing required fields | expand |
Hi Karan, Am 25.09.24 um 18:14 schrieb Karan Sanghavi: > Added below mentioned required fields > 1. interrupt-controller > 2. #interrupt-cells > in the bcm2711.dtsi file for the > interrupt-controller@40000000 block as defined in the > bindings/interrupt-controller/brcm,bcm2836-l1-intc.yaml. > This issue was noticed while compiling the dtb file > for broadcom/bcm2711-rpi-4-b.dts file. > After including the above fields in the dtsi file > interrupt-conntroller error was resolved. looks like you made the same mistake like me [1]. This change breaks boot of Raspberry Pi 4 [2]. There are a lot of DT schema warnings to fix, but this doesn't belong to the trivial ones. [1] - https://lore.kernel.org/linux-arm-kernel/20240812200358.4061-5-wahrenst@gmx.net/ [2] - https://lore.kernel.org/linux-arm-kernel/CA+G9fYuncv0fuBSC0A1z1G_UOv_XuMQz=DsrLZDK-Wc=10ghag@mail.gmail.com/ > > Signed-off-by: Karan Sanghavi <karansanghvi98@gmail.com> > --- > arch/arm/boot/dts/broadcom/bcm2711.dtsi | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/boot/dts/broadcom/bcm2711.dtsi b/arch/arm/boot/dts/broadcom/bcm2711.dtsi > index e4e42af21ef3..313b1046d74f 100644 > --- a/arch/arm/boot/dts/broadcom/bcm2711.dtsi > +++ b/arch/arm/boot/dts/broadcom/bcm2711.dtsi > @@ -51,6 +51,8 @@ soc { > local_intc: interrupt-controller@40000000 { > compatible = "brcm,bcm2836-l1-intc"; > reg = <0x40000000 0x100>; > + interrupt-controller; > + #interrupt-cells = <2>; > }; > > gicv2: interrupt-controller@40041000 {
On 25/09/2024 18:39, Stefan Wahren wrote: > Hi Karan, > > Am 25.09.24 um 18:14 schrieb Karan Sanghavi: >> Added below mentioned required fields >> 1. interrupt-controller >> 2. #interrupt-cells >> in the bcm2711.dtsi file for the >> interrupt-controller@40000000 block as defined in the >> bindings/interrupt-controller/brcm,bcm2836-l1-intc.yaml. >> This issue was noticed while compiling the dtb file >> for broadcom/bcm2711-rpi-4-b.dts file. >> After including the above fields in the dtsi file >> interrupt-conntroller error was resolved. > looks like you made the same mistake like me [1]. This change breaks > boot of Raspberry Pi 4 [2]. > > There are a lot of DT schema warnings to fix, but this doesn't belong to > the trivial ones. > Karan, Entire commit msg lacks proper rationale for such significant change. Rationale is for example: "this is an interrupt controller", but here reason is rather "I want to fix error". Important for every work focusing on fixing warnings/errors is to fix the cause, not the warning/error itself. Karan, you fixed the warning in a way it went away, but this did no fix the cause of the problem. You must find the real causes. Usually understanding the problem is necessary for that. Best regards, Krzysztof
On 9/25/24 09:39, Stefan Wahren wrote: > Hi Karan, > > Am 25.09.24 um 18:14 schrieb Karan Sanghavi: >> Added below mentioned required fields >> 1. interrupt-controller >> 2. #interrupt-cells >> in the bcm2711.dtsi file for the >> interrupt-controller@40000000 block as defined in the >> bindings/interrupt-controller/brcm,bcm2836-l1-intc.yaml. >> This issue was noticed while compiling the dtb file >> for broadcom/bcm2711-rpi-4-b.dts file. >> After including the above fields in the dtsi file >> interrupt-conntroller error was resolved. > looks like you made the same mistake like me [1]. This change breaks > boot of Raspberry Pi 4 [2]. > > There are a lot of DT schema warnings to fix, but this doesn't belong to > the trivial ones. Including the #interrupt-cells would not have a functional impact however, and we ought to be able to do that. The 'interrupt-controller' property presence means that the controller will be picked up by of_irq_init() and that is was causes problems for people testing this. Stefan, do you know if the VPU firmware removes/inserts that property to tell Linux which interrupt controller (bcm2836-l1-intc or ARM GIC) to use or does it make use of the "status" property which would be the canonical way about doing that? Thanks!
On Thu, 26 Sept 2024 at 02:08, Florian Fainelli <florian.fainelli@broadcom.com> wrote: > > On 9/25/24 09:39, Stefan Wahren wrote: > > Hi Karan, > > > > Am 25.09.24 um 18:14 schrieb Karan Sanghavi: > >> Added below mentioned required fields > >> 1. interrupt-controller > >> 2. #interrupt-cells > >> in the bcm2711.dtsi file for the > >> interrupt-controller@40000000 block as defined in the > >> bindings/interrupt-controller/brcm,bcm2836-l1-intc.yaml. > >> This issue was noticed while compiling the dtb file > >> for broadcom/bcm2711-rpi-4-b.dts file. > >> After including the above fields in the dtsi file > >> interrupt-conntroller error was resolved. > > looks like you made the same mistake like me [1]. This change breaks > > boot of Raspberry Pi 4 [2]. > > > > There are a lot of DT schema warnings to fix, but this doesn't belong to > > the trivial ones. > > Including the #interrupt-cells would not have a functional impact > however, and we ought to be able to do that. > > The 'interrupt-controller' property presence means that the controller > will be picked up by of_irq_init() and that is was causes problems for > people testing this. Stefan, do you know if the VPU firmware > removes/inserts that property to tell Linux which interrupt controller > (bcm2836-l1-intc or ARM GIC) to use or does it make use of the "status" > property which would be the canonical way about doing that? > > Thanks! > -- > Florian Hello Florian, So should I send a patch with only #interrupt-cells to solve one problem? Thank you.
Hi Florian, Am 25.09.24 um 22:38 schrieb Florian Fainelli: > On 9/25/24 09:39, Stefan Wahren wrote: >> Hi Karan, >> >> Am 25.09.24 um 18:14 schrieb Karan Sanghavi: >>> Added below mentioned required fields >>> 1. interrupt-controller >>> 2. #interrupt-cells >>> in the bcm2711.dtsi file for the >>> interrupt-controller@40000000 block as defined in the >>> bindings/interrupt-controller/brcm,bcm2836-l1-intc.yaml. >>> This issue was noticed while compiling the dtb file >>> for broadcom/bcm2711-rpi-4-b.dts file. >>> After including the above fields in the dtsi file >>> interrupt-conntroller error was resolved. >> looks like you made the same mistake like me [1]. This change breaks >> boot of Raspberry Pi 4 [2]. >> >> There are a lot of DT schema warnings to fix, but this doesn't belong to >> the trivial ones. > > Including the #interrupt-cells would not have a functional impact > however, and we ought to be able to do that. > > The 'interrupt-controller' property presence means that the controller > will be picked up by of_irq_init() and that is was causes problems for > people testing this. Stefan, do you know if the VPU firmware > removes/inserts that property to tell Linux which interrupt controller > (bcm2836-l1-intc or ARM GIC) to use or does it make use of the > "status" property which would be the canonical way about doing that? There is a config.txt parameter for this, which is called "enable_gic". But if i use this i couldn't see any difference to /proc/device-tree. Also i couldn't see any modifications by the firmware to the node in general: interrupt-controller@40000000 { compatible = "brcm,bcm2836-l1-intc"; reg = <0x40000000 0x100>; phandle = <0x8e>; }; Except of this i don't have any clue about the VPU firmware. Regards > > Thanks!
Hi Stefan and Florian On Mon, 30 Sept 2024 at 19:36, Stefan Wahren <wahrenst@gmx.net> wrote: > > Hi Florian, > > Am 25.09.24 um 22:38 schrieb Florian Fainelli: > > On 9/25/24 09:39, Stefan Wahren wrote: > >> Hi Karan, > >> > >> Am 25.09.24 um 18:14 schrieb Karan Sanghavi: > >>> Added below mentioned required fields > >>> 1. interrupt-controller > >>> 2. #interrupt-cells > >>> in the bcm2711.dtsi file for the > >>> interrupt-controller@40000000 block as defined in the > >>> bindings/interrupt-controller/brcm,bcm2836-l1-intc.yaml. > >>> This issue was noticed while compiling the dtb file > >>> for broadcom/bcm2711-rpi-4-b.dts file. > >>> After including the above fields in the dtsi file > >>> interrupt-conntroller error was resolved. > >> looks like you made the same mistake like me [1]. This change breaks > >> boot of Raspberry Pi 4 [2]. > >> > >> There are a lot of DT schema warnings to fix, but this doesn't belong to > >> the trivial ones. > > > > Including the #interrupt-cells would not have a functional impact > > however, and we ought to be able to do that. > > > > The 'interrupt-controller' property presence means that the controller > > will be picked up by of_irq_init() and that is was causes problems for > > people testing this. Stefan, do you know if the VPU firmware > > removes/inserts that property to tell Linux which interrupt controller > > (bcm2836-l1-intc or ARM GIC) to use or does it make use of the > > "status" property which would be the canonical way about doing that? > There is a config.txt parameter for this, which is called "enable_gic". > But if i use this i couldn't see any difference to /proc/device-tree. > Also i couldn't see any modifications by the firmware to the node in > general: > > interrupt-controller@40000000 { > compatible = "brcm,bcm2836-l1-intc"; > reg = <0x40000000 0x100>; > phandle = <0x8e>; > }; > > Except of this i don't have any clue about the VPU firmware. cc Phil so he can correct me if I get this wrong. The firmware looks at the DTB and automatically sets the enable_gic property if DT /interrupt-parent points at a node with compatible string "arm,gic-400". It doesn't modify DT around the interrupt controller nodes. Manually setting enable_gic should only be necessary on a system which isn't using DT where they wish to control whether to use the GIC or bcm2836-l1-intc. Dave
diff --git a/arch/arm/boot/dts/broadcom/bcm2711.dtsi b/arch/arm/boot/dts/broadcom/bcm2711.dtsi index e4e42af21ef3..313b1046d74f 100644 --- a/arch/arm/boot/dts/broadcom/bcm2711.dtsi +++ b/arch/arm/boot/dts/broadcom/bcm2711.dtsi @@ -51,6 +51,8 @@ soc { local_intc: interrupt-controller@40000000 { compatible = "brcm,bcm2836-l1-intc"; reg = <0x40000000 0x100>; + interrupt-controller; + #interrupt-cells = <2>; }; gicv2: interrupt-controller@40041000 {
Added below mentioned required fields 1. interrupt-controller 2. #interrupt-cells in the bcm2711.dtsi file for the interrupt-controller@40000000 block as defined in the bindings/interrupt-controller/brcm,bcm2836-l1-intc.yaml. This issue was noticed while compiling the dtb file for broadcom/bcm2711-rpi-4-b.dts file. After including the above fields in the dtsi file interrupt-conntroller error was resolved. Signed-off-by: Karan Sanghavi <karansanghvi98@gmail.com> --- arch/arm/boot/dts/broadcom/bcm2711.dtsi | 2 ++ 1 file changed, 2 insertions(+)