mbox series

[RFC,0/2] RZ/G2UL separate out SoC specific parts

Message ID 20220929172356.301342-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive)
Headers show
Series RZ/G2UL separate out SoC specific parts | expand

Message

Prabhakar Sept. 29, 2022, 5:23 p.m. UTC
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi All,

This patch series aims to split up the RZ/G2UL SoC DTSI into common parts
so that this can be shared with the RZ/Five SoC.

Implementation is based on the discussion [0] where I have used option#2.

The Renesas RZ/G2UL (ARM64) and RZ/Five (RISC-V) have almost the same
identical blocks to avoid duplication a base SoC dtsi (r9a07g043.dtsi) is
created which will be used by the RZ/G2UL (r9a07g043u.dtsi) and RZ/Five
(r9a07g043F.dtsi)

Sending this as an RFC to get some feedback.

r9a07g043f.dtsi will look something like below:

#include <dt-bindings/interrupt-controller/irq.h>

#define SOC_PERIPHERAL_IRQ_NUMBER(nr)	(nr + 32)
#define SOC_PERIPHERAL_IRQ(nr, na)	SOC_PERIPHERAL_IRQ_NUMBER(nr) na

#include <arm64/renesas/r9a07g043.dtsi>

/ {
   ...
   ...   
};

Although patch#2 can be merged into patch#1 just wanted to keep them separated
for easier review.

[0] https://lore.kernel.org/linux-arm-kernel/Yyt8s5+pyoysVNeC@spud/T/

Cheers,
Prabhakar

Lad Prabhakar (2):
  arm64: dts: renesas: r9a07g043: Introduce SOC_PERIPHERAL_IRQ() macro
    to specify interrupt property
  arm64: dts: renesas: r9a07g043: Split out RZ/G2UL SoC specific parts

 arch/arm64/boot/dts/renesas/r9a07g043.dtsi    | 362 +++++++-----------
 arch/arm64/boot/dts/renesas/r9a07g043u.dtsi   |  87 +++++
 .../boot/dts/renesas/r9a07g043u11-smarc.dts   |   2 +-
 3 files changed, 235 insertions(+), 216 deletions(-)
 create mode 100644 arch/arm64/boot/dts/renesas/r9a07g043u.dtsi

Comments

Prabhakar Oct. 10, 2022, 9:41 a.m. UTC | #1
Hi Rob, Krzysztof,

On Thu, Sep 29, 2022 at 6:24 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Hi All,
>
> This patch series aims to split up the RZ/G2UL SoC DTSI into common parts
> so that this can be shared with the RZ/Five SoC.
>
> Implementation is based on the discussion [0] where I have used option#2.
>
> The Renesas RZ/G2UL (ARM64) and RZ/Five (RISC-V) have almost the same
> identical blocks to avoid duplication a base SoC dtsi (r9a07g043.dtsi) is
> created which will be used by the RZ/G2UL (r9a07g043u.dtsi) and RZ/Five
> (r9a07g043F.dtsi)
>
> Sending this as an RFC to get some feedback.
>
> r9a07g043f.dtsi will look something like below:
>
> #include <dt-bindings/interrupt-controller/irq.h>
>
> #define SOC_PERIPHERAL_IRQ_NUMBER(nr)   (nr + 32)
> #define SOC_PERIPHERAL_IRQ(nr, na)      SOC_PERIPHERAL_IRQ_NUMBER(nr) na
>
> #include <arm64/renesas/r9a07g043.dtsi>
>
> / {
>    ...
>    ...
> };
>
> Although patch#2 can be merged into patch#1 just wanted to keep them separated
> for easier review.
>
> [0] https://lore.kernel.org/linux-arm-kernel/Yyt8s5+pyoysVNeC@spud/T/
>
> Cheers,
> Prabhakar
>
> Lad Prabhakar (2):
>   arm64: dts: renesas: r9a07g043: Introduce SOC_PERIPHERAL_IRQ() macro
>     to specify interrupt property

Can either of you please review patch #1.

Cheers,
Prabhakar

>   arm64: dts: renesas: r9a07g043: Split out RZ/G2UL SoC specific parts
>
>  arch/arm64/boot/dts/renesas/r9a07g043.dtsi    | 362 +++++++-----------
>  arch/arm64/boot/dts/renesas/r9a07g043u.dtsi   |  87 +++++
>  .../boot/dts/renesas/r9a07g043u11-smarc.dts   |   2 +-
>  3 files changed, 235 insertions(+), 216 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
>
> --
> 2.25.1
>
Krzysztof Kozlowski Oct. 12, 2022, 3:38 p.m. UTC | #2
On 10/10/2022 05:41, Lad, Prabhakar wrote:
> Hi Rob, Krzysztof,
> 
> On Thu, Sep 29, 2022 at 6:24 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
>>
>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>
>> Hi All,
>>
>> This patch series aims to split up the RZ/G2UL SoC DTSI into common parts
>> so that this can be shared with the RZ/Five SoC.
>>
>> Implementation is based on the discussion [0] where I have used option#2.
>>
>> The Renesas RZ/G2UL (ARM64) and RZ/Five (RISC-V) have almost the same
>> identical blocks to avoid duplication a base SoC dtsi (r9a07g043.dtsi) is
>> created which will be used by the RZ/G2UL (r9a07g043u.dtsi) and RZ/Five
>> (r9a07g043F.dtsi)
>>
>> Sending this as an RFC to get some feedback.
>>
>> r9a07g043f.dtsi will look something like below:
>>
>> #include <dt-bindings/interrupt-controller/irq.h>
>>
>> #define SOC_PERIPHERAL_IRQ_NUMBER(nr)   (nr + 32)
>> #define SOC_PERIPHERAL_IRQ(nr, na)      SOC_PERIPHERAL_IRQ_NUMBER(nr) na
>>
>> #include <arm64/renesas/r9a07g043.dtsi>
>>
>> / {
>>    ...
>>    ...
>> };
>>
>> Although patch#2 can be merged into patch#1 just wanted to keep them separated
>> for easier review.
>>
>> [0] https://lore.kernel.org/linux-arm-kernel/Yyt8s5+pyoysVNeC@spud/T/
>>
>> Cheers,
>> Prabhakar
>>
>> Lad Prabhakar (2):
>>   arm64: dts: renesas: r9a07g043: Introduce SOC_PERIPHERAL_IRQ() macro
>>     to specify interrupt property
> 
> Can either of you please review patch #1.
> 

Why? This is a DTS patch, isn't it? You should CC rather platform
maintainers, architecture maintainers and SoC folks (the latter you
missed for sure). You missed them, so please resend.

Best regards,
Krzysztof
Prabhakar Oct. 12, 2022, 7 p.m. UTC | #3
Hi Krzysztof,

On Wed, Oct 12, 2022 at 4:38 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 10/10/2022 05:41, Lad, Prabhakar wrote:
> > Hi Rob, Krzysztof,
> >
> > On Thu, Sep 29, 2022 at 6:24 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> >>
> >> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>
> >> Hi All,
> >>
> >> This patch series aims to split up the RZ/G2UL SoC DTSI into common parts
> >> so that this can be shared with the RZ/Five SoC.
> >>
> >> Implementation is based on the discussion [0] where I have used option#2.
> >>
> >> The Renesas RZ/G2UL (ARM64) and RZ/Five (RISC-V) have almost the same
> >> identical blocks to avoid duplication a base SoC dtsi (r9a07g043.dtsi) is
> >> created which will be used by the RZ/G2UL (r9a07g043u.dtsi) and RZ/Five
> >> (r9a07g043F.dtsi)
> >>
> >> Sending this as an RFC to get some feedback.
> >>
> >> r9a07g043f.dtsi will look something like below:
> >>
> >> #include <dt-bindings/interrupt-controller/irq.h>
> >>
> >> #define SOC_PERIPHERAL_IRQ_NUMBER(nr)   (nr + 32)
> >> #define SOC_PERIPHERAL_IRQ(nr, na)      SOC_PERIPHERAL_IRQ_NUMBER(nr) na
> >>
> >> #include <arm64/renesas/r9a07g043.dtsi>
> >>
> >> / {
> >>    ...
> >>    ...
> >> };
> >>
> >> Although patch#2 can be merged into patch#1 just wanted to keep them separated
> >> for easier review.
> >>
> >> [0] https://lore.kernel.org/linux-arm-kernel/Yyt8s5+pyoysVNeC@spud/T/
> >>
> >> Cheers,
> >> Prabhakar
> >>
> >> Lad Prabhakar (2):
> >>   arm64: dts: renesas: r9a07g043: Introduce SOC_PERIPHERAL_IRQ() macro
> >>     to specify interrupt property
> >
> > Can either of you please review patch #1.
> >
>
> Why? This is a DTS patch, isn't it? You should CC rather platform
> maintainers, architecture maintainers and SoC folks (the latter you
> missed for sure). You missed them, so please resend.
>
Mainly because we are using the SOC_PERIPHERAL_IRQ() macro while
specifying the interrupts property and just wanted to make sure the DT
maintainers are OK with that.

Sure I'll resend the patches CC'ing ARCH maintainers after v6.1-rc1.

Cheers,
Prabhakar
Geert Uytterhoeven Oct. 25, 2022, 12:40 p.m. UTC | #4
Hi Prabhakar,

On Thu, Sep 29, 2022 at 7:24 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> This patch series aims to split up the RZ/G2UL SoC DTSI into common parts
> so that this can be shared with the RZ/Five SoC.
>
> Implementation is based on the discussion [0] where I have used option#2.
>
> The Renesas RZ/G2UL (ARM64) and RZ/Five (RISC-V) have almost the same
> identical blocks to avoid duplication a base SoC dtsi (r9a07g043.dtsi) is
> created which will be used by the RZ/G2UL (r9a07g043u.dtsi) and RZ/Five
> (r9a07g043F.dtsi)

Thanks for your series!

> r9a07g043f.dtsi will look something like below:
>
> #include <dt-bindings/interrupt-controller/irq.h>
>
> #define SOC_PERIPHERAL_IRQ_NUMBER(nr)   (nr + 32)
> #define SOC_PERIPHERAL_IRQ(nr, na)      SOC_PERIPHERAL_IRQ_NUMBER(nr) na

Originally, when I assumed incorrectly that dtc does not support
arithmetic, I used "nr" and "na" in the macro I proposed to mean RISC-V
("r") resp. ARM ("a") interrupt number.  Apparently the names stuck,
although the second parameter now has a completely different meaning ;-)

However, as the NCEPLIC does support interrupt flags, unlike the SiFive
PLIC, there is no need to have the flags parameter in the macro.

Moreover,  it looks like the SOC_PERIPHERAL_IRQ_NUMBER()
intermediate is not needed, so you can just write:

    #define SOC_PERIPHERAL_IRQ(nr)  (nr + 32)

> #include <arm64/renesas/r9a07g043.dtsi>
>
> / {
>    ...
>    ...
> };

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds