Hi Uwe,
Thanks for the feedback.
> Subject: Re: [PATCH v12 6/6] pwm: Add Renesas RZ/G2L MTU3a PWM driver
>
> Hello Biju,
>
> On Thu, Feb 16, 2023 at 10:06:42AM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH v12 6/6] pwm: Add Renesas RZ/G2L MTU3a PWM
> > > driver On Wed, Feb 15, 2023 at 07:14:12PM +0000, Biju Das wrote:
> > > > > Subject: Re: [PATCH v12 6/6] pwm: Add Renesas RZ/G2L MTU3a PWM
> > > > > driver
> > > > >
> > > > > On Thu, Feb 02, 2023 at 04:57:32PM +0000, Biju Das wrote:
> > > > > > Add support for RZ/G2L MTU3a PWM driver. The IP supports
> > > > > > following PWM modes
> > > > > >
> > > > > > 1) PWM mode{1,2}
> > > > > > 2) Reset-synchronized PWM mode
> > > > > > 3) Complementary PWM mode{1,2,3}
> > > > >
> > > > > It's unclear to me what "PWM mode1" and the other modes are. I
> > > > > suspect this is some chip specific naming that isn't
> > > > > understandable for outsiders? Would be great to explain that a bit
> more.
> > > >
> > > > Ok I will add below to Limitation sections. Is it ok?
> > > >
> > > > PWM Mode 1: PWM waveforms are output from the MTIOCnA and MTIOCnC
> > > > pins by pairing TGRA with TGRB and TGRC with TGRD. The levels
> > > > specified by the TIOR.IOA[3:0] and IOC[3:0] bits are output from
> > > > the MTIOCnA and MTIOCnC pins at compare matches A and C, and the
> > > > level specified by the TIOR.IOB[3:0] and IOD[3:0] bits are output
> > > > at compare matches B and D
> > > (n = 0 to 4, 6, 7).
> > > >
> > > >
> > > > PWM Mode 2: PWM waveform output is generated using one TGR as the
> > > > cycle register and the others as duty registers.
> > > >
> > > > Reset-Synchronized PWM Mode: In the reset-synchronized PWM mode,
> > > > three phases of positive and negative PWM waveforms (six phases in
> > > > total) that share a common wave transition point can be output by
> > > > combining
> > > > MTU3 and MTU4 and
> > > > MTU6 and MTU7.
> > > >
> > > > Complementary PWM Mode: In complementary PWM mode, dead time can
> > > > be set for PWM waveforms to be output. The dead time is the period
> > > > during which the upper and lower arm transistors are set to the
> > > > inactive level in order to prevent short-circuiting of the
> > > > arms.Six positive-phase and six negative-phase PWM waveforms (12
> > > > phases in total)with dead time can be output by combining MTU3/ MTU4
> and MTU6/MTU7.
> > > >
> > > > In complementary PWM mode, nine registers (compare registers,
> > > > buffer registers, and temporary registers) are used to control the
> > > > duty ratio for
> > > the PWM output.
> > > > Complementary PWM mode 1 (transfer at crest) Complementary PWM
> > > > mode 2 (transfer at trough) Complementary PWM mode 3 (transfer at
> > > > crest and
> > > > trough)
> > >
> > > I read it five times now and still don't get it. The problem (maybe)
> > > is that there are many abbreviations I don't understand. Most critical
> is:
> > > What is a TGR?
> >
> > Basically it is Timer General register(TGR) functions as either output
> > compare or input capture or buffer registers.
> >
> > For the PWMMode1, it is just output-compare
> >
> > TGRA is used for setting period and also the compare-match output is used
> as clearing the counter.
> > TGRB is used for setting duty cycle.
> >
> > This will produce PWM output on MTIOC0A pin.
> >
> > The output of wave form depends upon setting on TIOR Currently it is
> > set as, Initial output is low. High output at compare match.
>
> Then I suggest to not talk at all about mode 1 or mode 2. Just mention that
> you use one counter and two match components to configure duty_cycle and
> period.
Agreed.
>
> > > > OK, will call get_rate() after enable. Runtime PM use
> > > > clockenable/disable Frequently, so unnecessary to use
> > > > clk_rate_exclusive_{get,put}. Is it ok?
> > >
> > > One doesn't have to do with the other. After calling
> > > clk_rate_exclusive_get() you can be sure that no other driver does
> > > anything to change the clk which would mess with the emitted signals.
> > >
> > > I don't know the exact semantics of clk_rate_exclusive_get(), but
> > > I'd assume that even if you disable the clock you should be able to
> > > rely on it running at the same rate after reenable.
> >
> > I believe usage of clk_rate_exclusive_get() is platform Dependent.
>
> Yeah, it depends on the platform if the clk will actually change behind your
> back. But making it explicit that the clk must not change is a good idea in
> general. Your driver might be used later on a platform where it matters or
> it might be used as a template for the next pwm driver.
OK, will use clk_rate_exclusive_get().
Cheers,
Biju
