RE: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

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

> Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver
> 
> On Sat, Oct 01, 2022 at 04:45:55PM +0000, Biju Das wrote:
> > Hi William Breathitt Gray,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter
> driver
> > >
> > > On Wed, Sep 28, 2022 at 06:14:57AM +0000, Biju Das wrote:
> > > > Hi William Breathitt Gray,
> > > >
> > > > Thanks for the feedback.
> > > >
> > > > > Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter
> > > driver
> > > > >
> > > > > On Mon, Sep 26, 2022 at 02:21:06PM +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
> > > > > >
> > > > > > This patch series aim to add MFD and counter driver for
> MTU3a.
> > > > > > Subsequent patch seies will add TImer and PWM driver support
> > > also
> > > > > > enhancements to counter driver.
> > > > >
> > > > > Hello Biju,
> > > > >
> > > > > I see this device consists of several channels, but only one
> > > > > Count is defined in the counter patch ("Channel 1 Count"). Do
> > > > > all
> > > channels
> > > > > support counting, or is it limited to just one channel?
> > > >
> > > > It is like this
> > > > MTU1 channel :- 1 16-bit phase counter MTU2-Channel :- 1 16-bit
> > > phase
> > > > counter
> > > > MTU1 + MTU2 channel combined:- 1 32-bit phase counter Other
> > > > channels are not supporting phase counting.
> > > >
> > > > Each counter device will have 1 channel. Currently it supports
> 16-
> > > bit
> > > > phase counting.
> > > >
> > > > Please see my test program. Am I missing something here?
> > > >
> > > > My test program:-
> > > >
> > > > echo 1 > /sys/bus/counter/devices/counter0/count0/enable
> > > > echo 50 > /sys/bus/counter/devices/counter0/count0/ceiling
> > > > devmem2 0x10001391 b 0x00 # Enable phase clock selection A for
> MTU2.
> > > > echo 1 > /sys/bus/counter/devices/counter1/count0/enable
> > > > echo 50 > /sys/bus/counter/devices/counter1/count0/ceiling
> > > >
> > > > for i in {1..5};
> > > > do cat /sys/bus/counter/devices/counter0/count0/count ; cat
> > > > /sys/bus/counter/devices/counter0/count0/direction;
> > > > cat /sys/bus/counter/devices/counter1/count0/count;
> > > > cat /sys/bus/counter/devices/counter1/count0/direction;
> > > > done
> > > >
> > > > Cheers,
> > > > Biju
> > >
> > > I'm not familiar with this hardware, but it looks like MTU1 and
> MTU2
> > > are on the same device. I think a more natural way to expose this
> > > functionality in the Counter subsystem would be to define a Count
> > > for each count value you can support; so something like this (all
> > > under
> > > /sys/bus/counter/devices/counter0):
> > >
> > > * count0 :- MTU1
> > > * count1 :- MTU2
> > > * count3 :- MTU1 + MTU2
> >
> > OK, sounds good. so count3 :- 32 bit phase counting count 0 or
> count1
> > or both then 16 bit phase counting
> 
> That "count3" should be "count2" (sorry for the typo), but yes all
> three Counts should be defined; if a particular Count can't be
> read/written due to the current phase counting mode selected, you can
> return -EBUSY or -EINVAL as appropriate.
> 

OK.

> To clarify one more time, do you have two 16-bit registers holding
> count values (one for MTU1 and one for MTU2), and when configured for
> 32-bit phase counting mode you combine both registers to give you a
> 32-bit count value?

Yes, that is correct.

> 
> > >
> > > You can then control the phase selection using a top-level Counter
> > > device extension (e.g. /sys/bus/counter/devices/counter0/phase)
> that
> > > configures whether you're in 16-bit phase or 32-phase counting
> mode.
> >
> > So I need to introduce a new sysfs called phase. Use that one for
> > Selecting the external clock pin for phase counting mode.
> > Please correct me if I am wrong??
> >
> > Hardware supports 4 pins for phase counting mode,
> >
> > MTCLKA Input External clock A input pin (MTU1/MTU2 phase counting
> mode
> > A phase input) MTCLKB Input External clock B input pin (MTU1/MTU2
> > phase counting mode B phase input) MTCLKC Input External clock C
> input
> > pin (MTU2 phase counting mode A phase input) MTCLKD Input External
> > clock D input pin (MTU2 phase counting mode B phase input)
> >
> > For MTU1, it is fixed MTCLKA and MTCLKB.
> > But for MTU2, it can be either 0-{ MTCLKA, MTCLKB} or 1 - { MTCLKC ,
> > MTCLKD} On reset it is set to { MTCLKC , MTCLKD}.
> >
> > Cheers,
> > Biju
> 
> It doesn't need to be named "phase" specifically, but it seems like a
> new sysfs file will be necessary in order to select the proper phase
> counting mode.
> 
> Are these MTCLK signals the quadrature A and B Signals you defined in
> the counter driver?

Yes, that is correct.

Cheers,
Biju