Re: [PATCH v2] arm64: dts: renesas: r8a7795: Move nodes which have no reg property out of bus

[Rob Herring <robh+dt@kernel.org>]

On Wed, Nov 29, 2017 at 7:58 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Rob,
>
> On Wed, Nov 29, 2017 at 2:35 PM, Rob Herring <robh+dt@kernel.org> wrote:
>> On Tue, Nov 28, 2017 at 3:04 AM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Tue, Nov 28, 2017 at 9:56 AM, Simon Horman <horms@verge.net.au> wrote:
>>>> On Mon, Nov 27, 2017 at 12:15:39PM +0100, Geert Uytterhoeven wrote:
>>>>> On Mon, Nov 27, 2017 at 12:04 PM, Simon Horman <horms@verge.net.au> wrote:
>>>>> > I just noticed that with this patch applied I now see:
>>>>> >
>>>>> > arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dtb: Warning (interrupts_property): Missing interrupt-parent for /pmu_a57
>>>>> > arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dtb: Warning (interrupts_property): Missing interrupt-parent for /pmu_a53
>>>>> > arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dtb: Warning (interrupts_property): Missing interrupt-parent for /pmu_a57
>>>>> > arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dtb: Warning (interrupts_property): Missing interrupt-parent for /pmu_a53arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dtb: Warning (interrupts_property): Missing interrupt-parent for /timer
>>>>> >
>>>>> > arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dtb: Warning (interrupts_property): Missing interrupt-parent for /timer
>>>>>
>>>>> Right, the "interrupt-parent = <&gic>;" inside the /soc node applies to child
>>>>> nodes of the /soc node only.
>>>>>
>>>>> You can find this in two ways:
>>>
>>> s/find/fix/
>>>
>>>>>
>>>>>   1. Add "interrupt-parent = <&gic>;" to the /pmu_a57 and /pmu_a53 nodes.
>>>>>   2. Switch those nodes from "interrupt" to "interrupts-extended", e.g. turn
>>>>>
>>>>>         interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>,
>>>>>
>>>>>      into
>>>>>
>>>>>         interrupts-extended = <&gic GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>,
>>>>>
>>>>> The latter is what e.g. arch/arm/boot/dts/armada-375.dtsi does.
>>>>
>>>> Thanks, I took option 1 as it seems consistent with the rest of the
>>>> Renesas DT files. I also added it to the /timer node.
>>>
>>> Actually we recently had a discussion about this on IRC, triggered by a
>>> similar issue in board files (see e.g, Ethernet PHY interrupts).
>>>
>>> Given the following comment:
>>>
>>> drivers/of/irq.c:       /* Try the new-style interrupts-extended first */
>>> drivers/of/irq.c:       res = of_parse_phandle_with_args(device,
>>> "interrupts-extended",
>>>
>>> I think it would be better to use interrupts-extended for individual/isolated
>>> use outside the /soc node.
>>
>> I disagree. There's no point to use interrupts-extended unless you
>> have 2 or more interrupt parents. Just set interrupt-parent in the
>> root node.
>
> The on-SoC devices (all under the /soc node, except for the weird ones without
> reg properties that are now being moved out) all have "&gic" as their interrupt
> parents.
>
> Off-SoC devices use one of the on-SoC interrupt controllers for external
> interrupts ("&irqc", "&irqpin", "&irqpin0", "&irqpin1"), or one of the on-SoC
> GPIO controllers that can also serve external interrupts ("&gpioN").
>
> For the latter, I think interrupts-extended definitely makes sense.
>
> For the former (incl. the "pmu_aN" nodes discussed here), it is debatable.
> But using interrupts-extended makes it easier to catch mistakes in board files,
> as they will be flagged by dtc ("Missing interrupt-parent").
> With interrupt-parent in the root node, they may go undetected.

They would still likely get flagged since the GIC #interrupt-cells is
3 and it is likely you don't have a multiple of 3.

But I guess interrupts-extended does make sense in this case.

Rob