RE: [PATCH v6 2/2] pwm: Add support for RZ/G2L GPT

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

 



Hi Uwe,

> Subject: Re: [PATCH v6 2/2] pwm: Add support for RZ/G2L GPT
> 
> Hello Biju,
> 
> On Sat, Sep 24, 2022 at 10:53:30AM +0000, Biju Das wrote:
> > > Subject: RE: [PATCH v6 2/2] pwm: Add support for RZ/G2L GPT
> > >
> > > Thanks for the feedback.
> > >
> > > > Subject: Re: [PATCH v6 2/2] pwm: Add support for RZ/G2L GPT
> > > >
> > > > Hello,
> > > >
> > > > On Wed, Sep 21, 2022 at 01:46:54PM +0000, Biju Das wrote:
> > > > > > Actually it's worse:
> > > > > >
> > > > > > - When both channels are used, setting the duty-cycle on one
> > > > aborts the
> > > > > >   currently running period on the other and starts it anew.
> > > > > >
> > > > > > (Did I get this correctly?)
> > > > >
> > > > > I think, I have fixed that issue with the below logic Which
> > > > > allows
> > > > to
> > > > > update duty cycle on the fly.
> > > > >
> > > > > Now the only limitation is w.r.to disabling channels as we
> need
> > > > > to disable together as stopping the counter affects both.
> > > > >
> > > > >       /*
> > > > > 	 * Counter must be stopped before modifying mode,
> prescaler,
> > > > timer
> > > > > 	 * counter and buffer enable registers. These registers are
> > > > shared
> > > > > 	 * between both channels. So allow updating these registers
> > > > > only
> > > > for the
> > > > > 	 * first enabled channel.
> > > > > 	 */
> > > > > 	if (rzg2l_gpt->user_count <= 1)
> > > > > 		rzg2l_gpt_disable(rzg2l_gpt);
> > > > >
> > > > > 	is_counter_running = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCR)
> &
> > > > RZG2L_GTCR_CST;
> > > > > 	if (!is_counter_running)
> > > > > 		/* GPT set operating mode (saw-wave up-counting) */
> > > > > 		rzg2l_gpt_modify(rzg2l_gpt, RZG2L_GTCR,
> RZG2L_GTCR_MD,
> > > > > 				 RZG2L_GTCR_MD_SAW_WAVE_PWM_MODE);
> > > >
> > > > So if the PWM is already running (e.g. from the bootloader) and
> > > > the mode is wrong, this isn't fixed? Similar problems in the if
> > > > blocks below.
> >
> > What is your thought on caching the registers that needs counter to
> be
> > stopped for updating values. Basically, we don't stop the counter if
> the values are same?
> 
> I don't see a very relevant difference between caching and reading the
> registers. Whatever is fine for you.

Ok.

> 
> > This allows updating period/duty cycle on the fly without stopping
> the
> > counter even for the single channel use case.
> 
> I didn't get the relevant difference, but the result sounds good.

OK. Will send v8, along with any feedback for v7 series [1]

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220921145741.901784-3-biju.das.jz@xxxxxxxxxxxxxx/

Note:
I have a plan to develop another PWM driver using MTU IP on the same SoC.
The work is not started yet.

For this IP, I planned to use MFD framework for the MTU driver and 
Will add counter driver, timer driver(clock source, clock event)
and pwm driver as child devices.

Currently the MFD driver and 16-Bit Phase Counting using counter framework
is almost done.

Changes,
Biju






[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