RE: [RFC 1/8] dt-bindings: soc: renesas: Add RZ/G2L POEG binding

From: Biju Das <biju.das.jz@bp.renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "Rob Herring" <robh@kernel.org>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"linux-pwm@vger.kernel.org" <linux-pwm@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"Geert Uytterhoeven" <geert+renesas@glider.be>,
	"Chris Paterson" <Chris.Paterson2@renesas.com>,
	"Biju Das" <biju.das@bp.renesas.com>,
	"Prabhakar Mahadev Lad" <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	"linux-renesas-soc@vger.kernel.org"
	<linux-renesas-soc@vger.kernel.org>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Subject: RE: [RFC 1/8] dt-bindings: soc: renesas: Add RZ/G2L POEG binding
Date: Thu, 19 May 2022 09:30:19 +0000	[thread overview]
Message-ID: <OS0PR01MB5922D0FFFA82AC5428F8C8D886D09@OS0PR01MB5922.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAMuHMdWPy4HmPrfnL8kZmFBBcHY-EoNm7Z6CoJyudhKornTS=g@mail.gmail.com>

Hi Geert,

Thanks for the feedback.

> Subject: Re: [RFC 1/8] dt-bindings: soc: renesas: Add RZ/G2L POEG binding
> 
> Hi Biju,
> 
> On Wed, May 18, 2022 at 8:34 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > Subject: Re: [RFC 1/8] dt-bindings: soc: renesas: Add RZ/G2L POEG
> > > binding On Wed, May 18, 2022 at 05:58:00AM +0000, Biju Das wrote:
> > > > > Subject: Re: [RFC 1/8] dt-bindings: soc: renesas: Add RZ/G2L
> > > > > POEG binding
> > > > >
> > > > > On Tue, May 10, 2022 at 04:11:05PM +0100, Biju Das wrote:
> > > > > > Add device tree bindings for the RZ/G2L Port Output Enable for
> > > > > > GPT
> > > > > (POEG).
> > > > > >
> > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> > > > > > +examples:
> > > > > > +  - |
> > > > > > +    #include <dt-bindings/clock/r9a07g044-cpg.h>
> > > > > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > > > > +
> > > > > > +    poeggd: poeg@10049400 {
> > > > > > +        compatible = "renesas,r9a07g044-poeg", "renesas,rzg2l-
> poeg";
> > > > > > +        reg = <0x10049400 0x4>;
> > > > >
> > > > > This looks like it is part of some larger block?
> > > >
> > > > There are 2 IP blocks GPT(PWM) and POEG with its own resources
> > > > like (register map, clk, reset and interrupts)
> > > >
> > > > Larger block is GPT, which has lot of functionalities. The output
> > > > from GPT block can be disabled by this IP either by external
> > > > trigger, request from GPT(Deadtime error, both output low/high) or
> > > > explicit software control). This IP has only a single register.
> > > > Currently I am not
> > > sure which framework to be used for this IP?? Or should it be merged
> > > with
> 
> Yeah, POEG is a weird beast.
> Some of it fits under pin control, but not all of it.
> From a quick glance, most of its configuration is intended to be static,
> i.e. could be done from DT, like pin control?
> I have no idea how to use the POEG interrupts, though.

If there is a GPT request(Dead time error or Both output low/high condition) output is disabled automatically and we get an 
Interrupt. May be to clear it , we need to implement interrupt. Otherwise output will be always disabled,
even if the outputs are out of phase after the fault condition.

I have done a quick test with interrupts previously for output disable using GPT request:- 
	Use both A and B in phase, output is disabled automatically and you get an interrupt in POEG block.
      If you inverse B, it is out of phase and fault condition is no more, but still output is disabled.
      In this condition, If we want to enable outputs, we need to clear interrupt status bits.

> 
> > > larger block GPT by combining the resources?
> > >
> > > Usually, IP blocks would have some minimum address alignment (typ 4K
> > > or 64K to be page aligned), but if there's no other IP in this
> > > address range as-is is fine. The question is what's before or after
> the above address?
> >
> > As per the HW manual, before GPT IP block and after POE3 block(Port
> Output Enable 3 (POE3) for MTU).
> >
> > Before
> > H'0_1004_8000 H'0_1004_87FF 2 Kbytes GPT
> >
> > After
> > H'0_1004_9800 H'0_1004_9BFF 1 Kbyte POE3
> >
> > Please find the address map for the IP blocks near to it.
> >
> > H'0_1004_A000 H'0_1004_A3FF 1 Kbyte SSIF ch1
> > H'0_1004_9C00 H'0_1004_9FFF 1 Kbyte SSIF ch0
> > H'0_1004_9800 H'0_1004_9BFF 1 Kbyte POE3
> > H'0_1004_9400 H'0_1004_97FF 1 Kbyte POEGD
> > H'0_1004_9000 H'0_1004_93FF 1 Kbyte POEGC
> > H'0_1004_8C00 H'0_1004_8FFF 1 Kbyte POEGB
> > H'0_1004_8800 H'0_1004_8BFF 1 Kbyte POEGA
> > H'0_1004_8000 H'0_1004_87FF 2 Kbytes GPT
> 
> This is actually 8 x 256 bytes, for 8 GPT instances.

Yes correct.

> 
> > H'0_1004_7000 H'0_1004_7FFF 4 Kbytes SRC (Reg)
> > H'0_1004_0000 H'0_1004_6FFF 28 Kbytes SRC (Memory)
> 
> So you can combine GPT and POEG[A-D] into a single block.
> However, doing so will make life harder when reusing the driver on an SoC
> with a different layout, or a different number of POEG blocks and GPT
> channels.

I agree. Modelling as a different driver gives lots of flexibility.

Cheers,
Biju

> 
> BTW, POE3 is a similar (in spirit) block on top of the MTU (Multi-Function
> Timer Pulse Unit 3, which seems to be an  enhanced version of the already-
> supported MTU2 on RZ/A1?).
> But the POE3 block is not located next to the MTU block, so you cannot
> combine them without overlap.
> 
> Note that the minimum page size on Cortex-A seems to be 4 kiB, and several
> blocks are spaced apart less, so even with a different OS than Linux you
> cannot implement page-based access control.
> 