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

[William Breathitt Gray <william.gray@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 4716 bytes --]

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.

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?

> > 
> > 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?

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]