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

[Rob Herring <robh@kernel.org>]

On Thu, May 19, 2022 at 09:30:19AM +0000, Biju Das wrote:
> 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.

The question is different h/w blocks or 1, not driver(s). It's 
convenient when the answer is the same (i.e. h/w node:driver is 1:1), 
but h/w is sometimes messy.

In any case, that looks like different blocks to me.

Rob