RE: [PATCH v3 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a bindings

[Biju Das <biju.das.jz@bp.renesas.com>]

Hi Rob and all,

> Subject: RE: [PATCH v3 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
> bindings
> 
> Hi Rob,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH v3 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a
> > bindings
> >
> > On Thu, Oct 06, 2022 at 02:57:14PM +0100, Biju Das wrote:
> > > The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded
> in
> > > the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
> > > channels and one 32-bit timer channel. It supports the following
> > > functions
> > >  - Counter
> > >  - Timer
> > >  - PWM
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > > v2->v3:
> > >  * Dropped counter bindings and integrated with mfd as it has only
> > one property.
> > >  * Removed "#address-cells" and "#size-cells" as it do not have
> > children with
> > >    unit addresses.
> > >  * Removed quotes from counter and pwm.
> > >  * Provided full path for pwm bindings.
> > >  * Updated the example.
> > > v1->v2:
> > >  * Modelled counter and pwm as a single device that handles
> > >    multiple channels.
> > >  * Moved counter and pwm bindings to respective subsystems
> > >  * Dropped 'bindings' from MFD binding title.
> > >  * Updated the example
> > >  * Changed the compatible names.
> > > ---
> > >  .../bindings/mfd/renesas,rz-mtu3.yaml         | 304
> > ++++++++++++++++++
> > >  .../bindings/pwm/renesas,rz-mtu3-pwm.yaml     |  50 +++
> > >  2 files changed, 354 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> > >  create mode 100644
> > > Documentation/devicetree/bindings/pwm/renesas,rz-mtu3-pwm.yaml
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> > > b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> > > new file mode 100644
> > > index 000000000000..44c952ad8d35
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml
> > > @@ -0,0 +1,304 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
> 1.2
> > > +---
> > > +$id:
> > >
> > > +
> > > +title: Renesas RZ/G2L Multi-Function Timer Pulse Unit 3 (MTU3a)
> > > +
> > > +maintainers:
> > > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > > +
> > > +description: |
> > > +  This hardware block pconsisting of eight 16-bit timer channels
> > and
> > > +one
> > > +  32- bit timer channel. It supports the following
> specifications:
> > > +    - Pulse input/output: 28 lines max.
> > > +    - Pulse input 3 lines
> > > +    - Count clock 11 clocks for each channel (14 clocks for MTU0,
> > 12 clocks
> > > +      for MTU2, and 10 clocks for MTU5, four clocks for MTU1-MTU2
> > combination
> > > +      (when LWA = 1))
> > > +    - Operating frequency Up to 100 MHz
> > > +    - Available operations [MTU0 to MTU4, MTU6, MTU7, and MTU8]
> > > +        - Waveform output on compare match
> > > +        - Input capture function (noise filter setting available)
> > > +        - Counter-clearing operation
> > > +        - Simultaneous writing to multiple timer counters (TCNT)
> > > +          (excluding MTU8).
> > > +        - Simultaneous clearing on compare match or input capture
> > > +          (excluding MTU8).
> > > +        - Simultaneous input and output to registers in
> > synchronization with
> > > +          counter operations           (excluding MTU8).
> > > +        - Up to 12-phase PWM output in combination with
> synchronous
> > operation
> > > +          (excluding MTU8)
> > > +    - [MTU0 MTU3, MTU4, MTU6, MTU7, and MTU8]
> > > +        - Buffer operation specifiable
> > > +    - [MTU1, MTU2]
> > > +        - Phase counting mode can be specified independently
> > > +        - 32-bit phase counting mode can be specified for
> > interlocked operation
> > > +          of MTU1 and MTU2 (when TMDR3.LWA = 1)
> > > +        - Cascade connection operation available
> > > +    - [MTU3, MTU4, MTU6, and MTU7]
> > > +        - Through interlocked operation of MTU3/4 and MTU6/7, the
> > positive and
> > > +          negative signals in six phases (12 phases in total) can
> > be output in
> > > +          complementary PWM and reset-synchronized PWM operation.
> > > +        - In complementary PWM mode, values can be transferred
> from
> > buffer
> > > +          registers to temporary registers at crests and troughs
> of
> > the timer-
> > > +          counter values or when the buffer registers (TGRD
> > registers in MTU4
> > > +          and MTU7) are written to.
> > > +        - Double-buffering selectable in complementary PWM mode.
> > > +    - [MTU3 and MTU4]
> > > +        - Through interlocking with MTU0, a mode for driving AC
> > synchronous
> > > +          motors (brushless DC motors) by using complementary PWM
> > output and
> > > +          reset-synchronized PWM output is settable and allows
> the
> > selection
> > > +          of two types of waveform output (chopping or level).
> > > +    - [MTU5]
> > > +        - Capable of operation as a dead-time compensation
> counter.
> > > +    - [MTU0/MTU5, MTU1, MTU2, and MTU8]
> > > +        - 32-bit phase counting mode specifiable by combining
> MTU1
> > and MTU2 and
> > > +          through interlocked operation with MTU0/MTU5 and MTU8.
> > > +    - Interrupt-skipping function
> > > +        - In complementary PWM mode, interrupts on crests and
> > troughs of counter
> > > +          values and triggers to start conversion by the A/D
> > converter can be
> > > +          skipped.
> > > +    - Interrupt sources: 43 sources.
> > > +    - Buffer operation:
> > > +        - Automatic transfer of register data (transfer from the
> > buffer
> > > +          register to the timer register).
> > > +    - Trigger generation
> > > +        - A/D converter start triggers can be generated
> > > +        - A/D converter start request delaying function enables
> A/D
> > converter
> > > +          to be started with any desired timing and to be
> > synchronized with
> > > +          PWM output.
> > > +    - Low power consumption function
> > > +        - The MTU3a can be placed in the module-stop state.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - enum:
> > > +          - renesas,r9a07g044-mtu3  # RZ/G2{L,LC}
> > > +          - renesas,r9a07g054-mtu3  # RZ/V2L
> > > +      - const: renesas,rz-mtu3
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    items:
> > > +      - description: MTU0.TGRA input capture/compare match
> > > +      - description: MTU0.TGRB input capture/compare match
> > > +      - description: MTU0.TGRC input capture/compare match
> > > +      - description: MTU0.TGRD input capture/compare match
> > > +      - description: MTU0.TCNT overflow
> > > +      - description: MTU0.TGRE compare match
> > > +      - description: MTU0.TGRF compare match
> > > +      - description: MTU1.TGRA input capture/compare match
> > > +      - description: MTU1.TGRB input capture/compare match
> > > +      - description: MTU1.TCNT overflow
> > > +      - description: MTU1.TCNT underflow
> > > +      - description: MTU2.TGRA input capture/compare match
> > > +      - description: MTU2.TGRB input capture/compare match
> > > +      - description: MTU2.TCNT overflow
> > > +      - description: MTU2.TCNT underflow
> > > +      - description: MTU3.TGRA input capture/compare match
> > > +      - description: MTU3.TGRB input capture/compare match
> > > +      - description: MTU3.TGRC input capture/compare match
> > > +      - description: MTU3.TGRD input capture/compare match
> > > +      - description: MTU3.TCNT overflow
> > > +      - description: MTU4.TGRA input capture/compare match
> > > +      - description: MTU4.TGRB input capture/compare match
> > > +      - description: MTU4.TGRC input capture/compare match
> > > +      - description: MTU4.TGRD input capture/compare match
> > > +      - description: MTU4.TCNT overflow/underflow
> > > +      - description: MTU5.TGRU input capture/compare match
> > > +      - description: MTU5.TGRV input capture/compare match
> > > +      - description: MTU5.TGRW input capture/compare match
> > > +      - description: MTU6.TGRA input capture/compare match
> > > +      - description: MTU6.TGRB input capture/compare match
> > > +      - description: MTU6.TGRC input capture/compare match
> > > +      - description: MTU6.TGRD input capture/compare match
> > > +      - description: MTU6.TCNT overflow
> > > +      - description: MTU7.TGRA input capture/compare match
> > > +      - description: MTU7.TGRB input capture/compare match
> > > +      - description: MTU7.TGRC input capture/compare match
> > > +      - description: MTU7.TGRD input capture/compare match
> > > +      - description: MTU7.TCNT overflow/underflow
> > > +      - description: MTU8.TGRA input capture/compare match
> > > +      - description: MTU8.TGRB input capture/compare match
> > > +      - description: MTU8.TGRC input capture/compare match
> > > +      - description: MTU8.TGRD input capture/compare match
> > > +      - description: MTU8.TCNT overflow
> > > +      - description: MTU8.TCNT underflow
> > > +
> > > +  interrupt-names:
> > > +    items:
> > > +      - const: tgia0
> > > +      - const: tgib0
> > > +      - const: tgic0
> > > +      - const: tgid0
> > > +      - const: tgiv0
> > > +      - const: tgie0
> > > +      - const: tgif0
> > > +      - const: tgia1
> > > +      - const: tgib1
> > > +      - const: tgiv1
> > > +      - const: tgiu1
> > > +      - const: tgia2
> > > +      - const: tgib2
> > > +      - const: tgiv2
> > > +      - const: tgiu2
> > > +      - const: tgia3
> > > +      - const: tgib3
> > > +      - const: tgic3
> > > +      - const: tgid3
> > > +      - const: tgiv3
> > > +      - const: tgia4
> > > +      - const: tgib4
> > > +      - const: tgic4
> > > +      - const: tgid4
> > > +      - const: tgiv4
> > > +      - const: tgiu5
> > > +      - const: tgiv5
> > > +      - const: tgiw5
> > > +      - const: tgia6
> > > +      - const: tgib6
> > > +      - const: tgic6
> > > +      - const: tgid6
> > > +      - const: tgiv6
> > > +      - const: tgia7
> > > +      - const: tgib7
> > > +      - const: tgic7
> > > +      - const: tgid7
> > > +      - const: tgiv7
> > > +      - const: tgia8
> > > +      - const: tgib8
> > > +      - const: tgic8
> > > +      - const: tgid8
> > > +      - const: tgiv8
> > > +      - const: tgiu8
> > > +
> > > +  clocks:
> > > +    maxItems: 1
> > > +
> > > +  power-domains:
> > > +    maxItems: 1
> > > +
> > > +  resets:
> > > +    maxItems: 1
> > > +
> > > +  counter:
> > > +    description:
> > > +      There are two phase counting modes. 16-bit phase counting
> > mode in which
> > > +      MTU1 and MTU2 operate independently, and cascade connection
> > 32-bit phase
> > > +      counting mode in which MTU1 and MTU2 are cascaded.
> > > +
> > > +      In phase counting mode, the phase difference between two
> > external input
> > > +      clocks is detected and the corresponding TCNT is
> incremented
> > or
> > > +      decremented.
> > > +      The below counters are supported
> > > +        count0 - MTU1 16-bit phase counting
> > > +        count1 - MTU2 16-bit phase counting
> > > +        count2 - MTU1+ MTU2 32-bit phase counting
> > > +
> > > +    type: object
> > > +
> > > +    properties:
> > > +      compatible:
> > > +        const: renesas,rz-mtu3-counter
> > > +
> > > +    required:
> > > +      - compatible
> > > +
> > > +    additionalProperties: false
> > > +
> > > +  pwm:
> > > +    $ref: /schemas/pwm/renesas,rz-mtu3-pwm.yaml
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +  - interrupt-names
> > > +  - clocks
> > > +  - power-domains
> > > +  - resets
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/clock/r9a07g044-cpg.h>
> > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +
> > > +    mtu3: timer@10001200 {
> > > +      compatible = "renesas,r9a07g044-mtu3", "renesas,rz-mtu3";
> > > +      reg = <0x10001200 0xb00>;
> > > +      interrupts = <GIC_SPI 170 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 171 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 172 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 173 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 174 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 175 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 176 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 177 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 178 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 179 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 180 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 181 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 182 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 183 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 184 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 185 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 186 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 190 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 191 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 192 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 194 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 195 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 196 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 198 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 199 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 200 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 201 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 202 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 203 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 204 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 205 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 206 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 207 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 208 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 209 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 210 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 211 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 212 IRQ_TYPE_EDGE_RISING>,
> > > +                   <GIC_SPI 213 IRQ_TYPE_EDGE_RISING>;
> > > +      interrupt-names = "tgia0", "tgib0", "tgic0", "tgid0",
> > "tgiv0", "tgie0",
> > > +                        "tgif0",
> > > +                        "tgia1", "tgib1", "tgiv1", "tgiu1",
> > > +                        "tgia2", "tgib2", "tgiv2", "tgiu2",
> > > +                        "tgia3", "tgib3", "tgic3", "tgid3",
> > "tgiv3",
> > > +                        "tgia4", "tgib4", "tgic4", "tgid4",
> > "tgiv4",
> > > +                        "tgiu5", "tgiv5", "tgiw5",
> > > +                        "tgia6", "tgib6", "tgic6", "tgid6",
> > "tgiv6",
> > > +                        "tgia7", "tgib7", "tgic7", "tgid7",
> > "tgiv7",
> > > +                        "tgia8", "tgib8", "tgic8", "tgid8",
> > "tgiv8", "tgiu8";
> > > +      clocks = <&cpg CPG_MOD R9A07G044_MTU_X_MCK_MTU3>;
> > > +      power-domains = <&cpg>;
> > > +      resets = <&cpg R9A07G044_MTU_X_PRESET_MTU3>;
> > > +
> > > +      counter {
> > > +        compatible = "renesas,rz-mtu3-counter";
> >
> > You don't have any resources for the counter in DT, so you don't
> even
> > need a node here. Just have the parent driver instaniate the counter
> > driver.
> 

If I remove "renesas,rz-mtu3-counter" and "renesas,rz-mtu3-pwm" then instantiating 
the counter and pwm driver from parent driver by directly calling probe function is
giving cyclic dependency error[1].

So looks like either we need to use compatible "renesas,rz-mtu3-counter" and 
"renesas,rz-mtu3-pwm" if these functionalities to be in respective subsystem tree

or 

squash counter and pwm functionalities to MFD subsystem.

Please share your views on this. Is there any better way to handle this?

[1]
depmod: ../tools/depmod.c:1792: depmod_report_cycles_from_root: Assertion `is < stack_size' failed.
depmod: ERROR: Cycle detected: rz_mtu3 -> rz_mtu3_cnt -> rz_mtu3
depmod: ERROR: Cycle detected: rz_mtu3 -> rz_mtu3_pwm -> rz_mtu3
depmod: ERROR: Found 3 modules in dependency cycles!
make: *** [Makefile:1781: modules_install] Error 1

Cheers,
Biju

> 
> >
> > > +      };
> > > +
> > > +      pwm {
> > > +        compatible = "renesas,rz-mtu3-pwm";
> > > +        #pwm-cells = <2>;
> >
> > Here too, just declaring a PWM provider. Just move this to the
> parent
> > node.
> 
> OK, will remove node and compatible. Will add support for parent
> driver instaniate the PWM driver and also declaring as a PWM provider.
> 
> Cheers,
> Biju