Re: [PATCH v12 6/6] pwm: Add Renesas RZ/G2L MTU3a PWM driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.
 
> > > 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.
 
Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux