| Message ID | 87lfhm70s6.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: renesas: enable HDMI Display/Sound on R-Car M3-W+ Salvator-XS | expand |
On 07.09.2020 5:59, Kuninori Morimoto wrote: > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > This patch adds VSP device nodes for R-Car M3-W+ (r8a77961) SoC. > This patch is test on R-Car M3-W+ Salvator-XS board. Was tested? > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> [...] MBR, Sergei
On 07.09.2020 11:08, Sergei Shtylyov wrote: >> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> >> >> This patch adds VSP device nodes for R-Car M3-W+ (r8a77961) SoC. >> This patch is test on R-Car M3-W+ Salvator-XS board. > > Was tested? The same comment to the patches 6 & 7. >> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > [...] MBR, Sergei
Hi Morimoto-san, On 07/09/2020 03:59, Kuninori Morimoto wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > This patch adds VSP device nodes for R-Car M3-W+ (r8a77961) SoC. > This patch is test on R-Car M3-W+ Salvator-XS board. > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > arch/arm64/boot/dts/renesas/r8a77961.dtsi | 55 +++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > > diff --git a/arch/arm64/boot/dts/renesas/r8a77961.dtsi b/arch/arm64/boot/dts/renesas/r8a77961.dtsi > index fe0db11b9cb9..c2a6918ed5e6 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77961.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi > @@ -2056,6 +2056,61 @@ fcpvd2: fcp@fea37000 { > iommus = <&ipmmu_vi0 10>; > }; The FCP's added are: fcpf0: fcp@fe950000 { fcpf1: fcp@fe951000 { fcpvb0: fcp@fe96f000 { fcpvb1: fcp@fe92f000 { fcpvi0: fcp@fe9af000 { fcpvi1: fcp@fe9bf000 { fcpvd0: fcp@fea27000 { fcpvd1: fcp@fea2f000 { fcpvd2: fcp@fea37000 { So indeed, the first fcpf0 comes before fe960000. Do we keep the items grouped by the first occurrence? or sort the nodes based on address? for some reason I thought we were ordering based on address, but I see other situations where we group too - so I'm confused (and wishing there was an automatic tool to get the sorting correct without fuss). Is there a set policy? -- Kieran > + vspb: vsp@fe960000 { > + compatible = "renesas,vsp2"; > + reg = <0 0xfe960000 0 0x8000>; > + interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 626>; > + power-domains = <&sysc R8A77961_PD_A3VC>; > + resets = <&cpg 626>; > + > + renesas,fcp = <&fcpvb0>; > + }; > + > + vspd0: vsp@fea20000 { > + compatible = "renesas,vsp2"; > + reg = <0 0xfea20000 0 0x5000>; > + interrupts = <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 623>; > + power-domains = <&sysc R8A77961_PD_ALWAYS_ON>; > + resets = <&cpg 623>; > + > + renesas,fcp = <&fcpvd0>; > + }; > + > + vspd1: vsp@fea28000 { > + compatible = "renesas,vsp2"; > + reg = <0 0xfea28000 0 0x5000>; > + interrupts = <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 622>; > + power-domains = <&sysc R8A77961_PD_ALWAYS_ON>; > + resets = <&cpg 622>; > + > + renesas,fcp = <&fcpvd1>; > + }; > + > + vspd2: vsp@fea30000 { > + compatible = "renesas,vsp2"; > + reg = <0 0xfea30000 0 0x5000>; > + interrupts = <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 621>; > + power-domains = <&sysc R8A77961_PD_ALWAYS_ON>; > + resets = <&cpg 621>; > + > + renesas,fcp = <&fcpvd2>; > + }; > + > + vspi0: vsp@fe9a0000 { > + compatible = "renesas,vsp2"; > + reg = <0 0xfe9a0000 0 0x8000>; > + interrupts = <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 631>; > + power-domains = <&sysc R8A77961_PD_A3VC>; > + resets = <&cpg 631>; > + > + renesas,fcp = <&fcpvi0>; > + }; > + > csi20: csi2@fea80000 { > reg = <0 0xfea80000 0 0x10000>; > /* placeholder */ >
Hi Kieran > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > This patch adds VSP device nodes for R-Car M3-W+ (r8a77961) SoC. > > This patch is test on R-Car M3-W+ Salvator-XS board. > > > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > --- (snip) > Do we keep the items grouped by the first occurrence? or sort the nodes > based on address? (snip) > Is there a set policy? The order is same as r8a77960.dtsi, not my policy. Will sort in v2 Thank you for your help !! Best regards --- Kuninori Morimoto
On Mon, Sep 7, 2020 at 4:59 AM Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > This patch adds VSP device nodes for R-Car M3-W+ (r8a77961) SoC. > This patch is test on R-Car M3-W+ Salvator-XS board. > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
Hi Kieran, On Mon, Sep 7, 2020 at 5:55 PM Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> wrote: > On 07/09/2020 03:59, Kuninori Morimoto wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > This patch adds VSP device nodes for R-Car M3-W+ (r8a77961) SoC. > > This patch is test on R-Car M3-W+ Salvator-XS board. > > > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > --- > > arch/arm64/boot/dts/renesas/r8a77961.dtsi | 55 +++++++++++++++++++++++ > > 1 file changed, 55 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/renesas/r8a77961.dtsi b/arch/arm64/boot/dts/renesas/r8a77961.dtsi > > index fe0db11b9cb9..c2a6918ed5e6 100644 > > --- a/arch/arm64/boot/dts/renesas/r8a77961.dtsi > > +++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi > > @@ -2056,6 +2056,61 @@ fcpvd2: fcp@fea37000 { > > iommus = <&ipmmu_vi0 10>; > > }; > > The FCP's added are: > > fcpf0: fcp@fe950000 { > fcpf1: fcp@fe951000 { > fcpvb0: fcp@fe96f000 { > fcpvb1: fcp@fe92f000 { > fcpvi0: fcp@fe9af000 { > fcpvi1: fcp@fe9bf000 { > fcpvd0: fcp@fea27000 { > fcpvd1: fcp@fea2f000 { > fcpvd2: fcp@fea37000 { > > So indeed, the first fcpf0 comes before fe960000. > > Do we keep the items grouped by the first occurrence? or sort the nodes > based on address? > > for some reason I thought we were ordering based on address, but I see > other situations where we group too - so I'm confused (and wishing there > was an automatic tool to get the sorting correct without fuss). > > Is there a set policy? For nodes with a unit-address, we usually[*] sort by unit-address, but we keep similar nodes grouped. Hence I prefer this v1 over v2. [*] Seems like FCP/VSP are interleaved in r8a77990.dsi, doh. > > + vspb: vsp@fe960000 { > > + compatible = "renesas,vsp2"; > > + reg = <0 0xfe960000 0 0x8000>; > > + interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&cpg CPG_MOD 626>; > > + power-domains = <&sysc R8A77961_PD_A3VC>; > > + resets = <&cpg 626>; > > + > > + renesas,fcp = <&fcpvb0>; > > + }; Gr{oetje,eeting}s, Geert
Hi Geert, On 10/09/2020 10:44, Geert Uytterhoeven wrote: > Hi Kieran, > > On Mon, Sep 7, 2020 at 5:55 PM Kieran Bingham > <kieran.bingham+renesas@ideasonboard.com> wrote: >> On 07/09/2020 03:59, Kuninori Morimoto wrote: >>> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> >>> >>> This patch adds VSP device nodes for R-Car M3-W+ (r8a77961) SoC. >>> This patch is test on R-Car M3-W+ Salvator-XS board. >>> >>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> >>> --- >>> arch/arm64/boot/dts/renesas/r8a77961.dtsi | 55 +++++++++++++++++++++++ >>> 1 file changed, 55 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/renesas/r8a77961.dtsi b/arch/arm64/boot/dts/renesas/r8a77961.dtsi >>> index fe0db11b9cb9..c2a6918ed5e6 100644 >>> --- a/arch/arm64/boot/dts/renesas/r8a77961.dtsi >>> +++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi >>> @@ -2056,6 +2056,61 @@ fcpvd2: fcp@fea37000 { >>> iommus = <&ipmmu_vi0 10>; >>> }; >> >> The FCP's added are: >> >> fcpf0: fcp@fe950000 { >> fcpf1: fcp@fe951000 { >> fcpvb0: fcp@fe96f000 { >> fcpvb1: fcp@fe92f000 { >> fcpvi0: fcp@fe9af000 { >> fcpvi1: fcp@fe9bf000 { >> fcpvd0: fcp@fea27000 { >> fcpvd1: fcp@fea2f000 { >> fcpvd2: fcp@fea37000 { >> >> So indeed, the first fcpf0 comes before fe960000. >> >> Do we keep the items grouped by the first occurrence? or sort the nodes >> based on address? >> >> for some reason I thought we were ordering based on address, but I see >> other situations where we group too - so I'm confused (and wishing there >> was an automatic tool to get the sorting correct without fuss). >> >> Is there a set policy? > > For nodes with a unit-address, we usually[*] sort by unit-address, but we keep > similar nodes grouped. Hence I prefer this v1 over v2. I assume then the groups are sorted by the first entry, I.e. hypothetically: fdp@0 fcp@1 vsp@2 fdp@3 fcp@4 vsp@5 cmm@6 cmm@7 would become fdp@0 fdp@3 fcp@1 fcp@4 vsp@2 vsp@5 cmm@6 cmm@7 Has anyone already created any scripting/validation to automate the sorting requirements? > > [*] Seems like FCP/VSP are interleaved in r8a77990.dsi, doh. > Personally I prefer that - but my opinion doesn't matter here - so as long as the rules are defined (or even better, automatically enforceable) that's fine. >>> + vspb: vsp@fe960000 { >>> + compatible = "renesas,vsp2"; >>> + reg = <0 0xfe960000 0 0x8000>; >>> + interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>; >>> + clocks = <&cpg CPG_MOD 626>; >>> + power-domains = <&sysc R8A77961_PD_A3VC>; >>> + resets = <&cpg 626>; >>> + >>> + renesas,fcp = <&fcpvb0>; >>> + }; > > Gr{oetje,eeting}s, > > Geert >
Hi Kieran, On Thu, Sep 10, 2020 at 12:34 PM Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> wrote: > On 10/09/2020 10:44, Geert Uytterhoeven wrote: > > On Mon, Sep 7, 2020 at 5:55 PM Kieran Bingham > > <kieran.bingham+renesas@ideasonboard.com> wrote: > >> On 07/09/2020 03:59, Kuninori Morimoto wrote: > >>> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > >>> > >>> This patch adds VSP device nodes for R-Car M3-W+ (r8a77961) SoC. > >>> This patch is test on R-Car M3-W+ Salvator-XS board. > >>> > >>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > >>> --- > >>> arch/arm64/boot/dts/renesas/r8a77961.dtsi | 55 +++++++++++++++++++++++ > >>> 1 file changed, 55 insertions(+) > >>> > >>> diff --git a/arch/arm64/boot/dts/renesas/r8a77961.dtsi b/arch/arm64/boot/dts/renesas/r8a77961.dtsi > >>> index fe0db11b9cb9..c2a6918ed5e6 100644 > >>> --- a/arch/arm64/boot/dts/renesas/r8a77961.dtsi > >>> +++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi > >>> @@ -2056,6 +2056,61 @@ fcpvd2: fcp@fea37000 { > >>> iommus = <&ipmmu_vi0 10>; > >>> }; > >> > >> The FCP's added are: > >> > >> fcpf0: fcp@fe950000 { > >> fcpf1: fcp@fe951000 { > >> fcpvb0: fcp@fe96f000 { > >> fcpvb1: fcp@fe92f000 { > >> fcpvi0: fcp@fe9af000 { > >> fcpvi1: fcp@fe9bf000 { > >> fcpvd0: fcp@fea27000 { > >> fcpvd1: fcp@fea2f000 { > >> fcpvd2: fcp@fea37000 { > >> > >> So indeed, the first fcpf0 comes before fe960000. > >> > >> Do we keep the items grouped by the first occurrence? or sort the nodes > >> based on address? > >> > >> for some reason I thought we were ordering based on address, but I see > >> other situations where we group too - so I'm confused (and wishing there > >> was an automatic tool to get the sorting correct without fuss). > >> > >> Is there a set policy? > > > > For nodes with a unit-address, we usually[*] sort by unit-address, but we keep > > similar nodes grouped. Hence I prefer this v1 over v2. > > I assume then the groups are sorted by the first entry, > > I.e. hypothetically: > > fdp@0 > fcp@1 > vsp@2 > fdp@3 > fcp@4 > vsp@5 > cmm@6 > cmm@7 > > would become > > fdp@0 > fdp@3 > fcp@1 > fcp@4 > vsp@2 > vsp@5 > cmm@6 > cmm@7 Exactly. That's how we (Reneas SoCs) have been (trying to) doing it. I am aware there are some deviations (e.g. do you keep all 4 possible SCIF types together (they're all serial@), or do you group them per type? And some nodes (ipmmu) may be sorted alphabetically by label). > Has anyone already created any scripting/validation to automate the > sorting requirements? Not yet, AFAIK. I've been thinking about doing that several times, though ;-) > > [*] Seems like FCP/VSP are interleaved in r8a77990.dsi, doh. > > > > Personally I prefer that - but my opinion doesn't matter here - so as > long as the rules are defined (or even better, automatically > enforceable) that's fine. Indeed. Perhaps creating rules is something to be handled at a higher level (i.e. common for all DTS files)? Summarizing "our" rules: 1. Nodes without unit-address are sorted alphabetically, by node name, 2. Nodes with unit-address are sorted numerically, by unit-address, a. Nodes of the same type are grouped together, i.e. the whole group is sorted w.r.t. to other nodes/groups based on the unit-address of the first member of the group. 3. Anchors are sorted alphabetically, but anchor name. Do they make sense? > >>> + vspb: vsp@fe960000 { > >>> + compatible = "renesas,vsp2"; > >>> + reg = <0 0xfe960000 0 0x8000>; > >>> + interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>; > >>> + clocks = <&cpg CPG_MOD 626>; > >>> + power-domains = <&sysc R8A77961_PD_A3VC>; > >>> + resets = <&cpg 626>; > >>> + > >>> + renesas,fcp = <&fcpvb0>; > >>> + }; Gr{oetje,eeting}s, Geert
On Thu, Sep 10, 2020 at 1:09 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Thu, Sep 10, 2020 at 12:34 PM Kieran Bingham > <kieran.bingham+renesas@ideasonboard.com> wrote: > > On 10/09/2020 10:44, Geert Uytterhoeven wrote: > > > On Mon, Sep 7, 2020 at 5:55 PM Kieran Bingham > > > <kieran.bingham+renesas@ideasonboard.com> wrote: > > >> On 07/09/2020 03:59, Kuninori Morimoto wrote: > > >>> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > >>> > > >>> This patch adds VSP device nodes for R-Car M3-W+ (r8a77961) SoC. > > >>> This patch is test on R-Car M3-W+ Salvator-XS board. > > >>> --- a/arch/arm64/boot/dts/renesas/r8a77961.dtsi > > >>> +++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi > > >>> @@ -2056,6 +2056,61 @@ fcpvd2: fcp@fea37000 { > > >>> iommus = <&ipmmu_vi0 10>; > > >>> }; > > >> > > >> The FCP's added are: > > >> > > >> fcpf0: fcp@fe950000 { > > >> fcpf1: fcp@fe951000 { > > >> fcpvb0: fcp@fe96f000 { > > >> fcpvb1: fcp@fe92f000 { > > >> fcpvi0: fcp@fe9af000 { > > >> fcpvi1: fcp@fe9bf000 { > > >> fcpvd0: fcp@fea27000 { > > >> fcpvd1: fcp@fea2f000 { > > >> fcpvd2: fcp@fea37000 { > > >> > > >> So indeed, the first fcpf0 comes before fe960000. > > >> > > >> Do we keep the items grouped by the first occurrence? or sort the nodes > > >> based on address? > > >> > > >> for some reason I thought we were ordering based on address, but I see > > >> other situations where we group too - so I'm confused (and wishing there > > >> was an automatic tool to get the sorting correct without fuss). > > >> > > >> Is there a set policy? > > > > > > For nodes with a unit-address, we usually[*] sort by unit-address, but we keep > > > similar nodes grouped. Hence I prefer this v1 over v2. > > > > I assume then the groups are sorted by the first entry, > > > > I.e. hypothetically: > > > > fdp@0 > > fcp@1 > > vsp@2 > > fdp@3 > > fcp@4 > > vsp@5 > > cmm@6 > > cmm@7 > > > > would become > > > > fdp@0 > > fdp@3 > > fcp@1 > > fcp@4 > > vsp@2 > > vsp@5 > > cmm@6 > > cmm@7 > > Exactly. That's how we (Reneas SoCs) have been (trying to) doing it. > I am aware there are some deviations (e.g. do you keep all 4 possible > SCIF types together (they're all serial@), or do you group them per > type? And some nodes (ipmmu) may be sorted alphabetically by label). > > > Has anyone already created any scripting/validation to automate the > > sorting requirements? > > Not yet, AFAIK. I've been thinking about doing that several times, > though ;-) > > > > [*] Seems like FCP/VSP are interleaved in r8a77990.dsi, doh. > > > > > > > Personally I prefer that - but my opinion doesn't matter here - so as > > long as the rules are defined (or even better, automatically > > enforceable) that's fine. > > Indeed. > > Perhaps creating rules is something to be handled at a higher level > (i.e. common for all DTS files)? > > Summarizing "our" rules: > 1. Nodes without unit-address are sorted alphabetically, by node name, > 2. Nodes with unit-address are sorted numerically, by unit-address, > a. Nodes of the same type are grouped together, i.e. the whole > group is sorted w.r.t. to other nodes/groups based on the > unit-address of the first member of the group. > 3. Anchors are sorted alphabetically, but anchor name. > > Do they make sense? For comparison, scripts/dtc/dtx_diff uses "dtc -s" for sorting, which sorts everything (nodes and properties) alphabetically. While I can agree on sorting all nodes alphabetically, sorting properties alphabetically goes IMHO a bit too far. E.g. it's established practice to put "compatible" and "reg" first. Gr{oetje,eeting}s, Geert
diff --git a/arch/arm64/boot/dts/renesas/r8a77961.dtsi b/arch/arm64/boot/dts/renesas/r8a77961.dtsi index fe0db11b9cb9..c2a6918ed5e6 100644 --- a/arch/arm64/boot/dts/renesas/r8a77961.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi @@ -2056,6 +2056,61 @@ fcpvd2: fcp@fea37000 { iommus = <&ipmmu_vi0 10>; }; + vspb: vsp@fe960000 { + compatible = "renesas,vsp2"; + reg = <0 0xfe960000 0 0x8000>; + interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 626>; + power-domains = <&sysc R8A77961_PD_A3VC>; + resets = <&cpg 626>; + + renesas,fcp = <&fcpvb0>; + }; + + vspd0: vsp@fea20000 { + compatible = "renesas,vsp2"; + reg = <0 0xfea20000 0 0x5000>; + interrupts = <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 623>; + power-domains = <&sysc R8A77961_PD_ALWAYS_ON>; + resets = <&cpg 623>; + + renesas,fcp = <&fcpvd0>; + }; + + vspd1: vsp@fea28000 { + compatible = "renesas,vsp2"; + reg = <0 0xfea28000 0 0x5000>; + interrupts = <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 622>; + power-domains = <&sysc R8A77961_PD_ALWAYS_ON>; + resets = <&cpg 622>; + + renesas,fcp = <&fcpvd1>; + }; + + vspd2: vsp@fea30000 { + compatible = "renesas,vsp2"; + reg = <0 0xfea30000 0 0x5000>; + interrupts = <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 621>; + power-domains = <&sysc R8A77961_PD_ALWAYS_ON>; + resets = <&cpg 621>; + + renesas,fcp = <&fcpvd2>; + }; + + vspi0: vsp@fe9a0000 { + compatible = "renesas,vsp2"; + reg = <0 0xfe9a0000 0 0x8000>; + interrupts = <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 631>; + power-domains = <&sysc R8A77961_PD_A3VC>; + resets = <&cpg 631>; + + renesas,fcp = <&fcpvi0>; + }; + csi20: csi2@fea80000 { reg = <0 0xfea80000 0 0x10000>; /* placeholder */