mbox series

[RFC,0/8] Add RZ/G2L MTU3a MFD and Counter driver

Message ID 20220926132114.60396-1-biju.das.jz@bp.renesas.com (mailing list archive)
Headers show
Series Add RZ/G2L MTU3a MFD and Counter driver | expand

Message

Biju Das Sept. 26, 2022, 1:21 p.m. UTC
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.

The 8/16/32 bit registers are mixed in each channel. The HW
specifications of the IP is described in patch#2.

Current patch set is tested for 16-bit phase counting mode on
MTU1 channel.

Please share your valuable comments on this patch series.

Biju Das (8):
  clk: renesas: r9a07g044: Add MTU3a clock and reset entry
  dt-bindings: mfd: Document RZ/G2L MTU3a bindings
  mfd: Add RZ/G2L MTU3 driver
  dt-bindings: mfd: rzg2l-mtu3: Document RZ/G2UL MTU3 counter
  counter: Add RZ/G2L MTU3 counter driver
  arm64: dts: renesas: r9a07g044: Add MTU3a node
  arm64: dts: renesas: r9a07g054: Add MTU3a node
  arm64: dts: renesas: rzg2l-smarc: [HACK] Enable MTU for 16-bit phase
    count testing

 .../bindings/mfd/renesas,rzg2l-mtu3.yaml      | 310 ++++++++++++++
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi    |  83 ++++
 .../boot/dts/renesas/r9a07g044l2-smarc.dts    |   2 -
 arch/arm64/boot/dts/renesas/r9a07g054.dtsi    |  83 ++++
 .../dts/renesas/rzg2l-smarc-pinfunction.dtsi  |  11 +
 arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi  |  25 +-
 drivers/clk/renesas/r9a07g044-cpg.c           |   5 +-
 drivers/counter/Kconfig                       |   9 +
 drivers/counter/Makefile                      |   1 +
 drivers/counter/rzg2l-mtu3-cnt.c              | 367 +++++++++++++++++
 drivers/mfd/Kconfig                           |   9 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/rzg2l-mtu3.c                      | 377 ++++++++++++++++++
 include/linux/mfd/rzg2l-mtu3.h                | 124 ++++++
 14 files changed, 1403 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
 create mode 100644 drivers/counter/rzg2l-mtu3-cnt.c
 create mode 100644 drivers/mfd/rzg2l-mtu3.c
 create mode 100644 include/linux/mfd/rzg2l-mtu3.h

Comments

William Breathitt Gray Sept. 27, 2022, 10:05 p.m. UTC | #1
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?

Thanks,

William Breathitt Gray
Biju Das Sept. 28, 2022, 6:14 a.m. UTC | #2
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
William Breathitt Gray Sept. 30, 2022, 10:57 p.m. UTC | #3
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

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.

William Breathitt Gray
Biju Das Oct. 1, 2022, 4:45 p.m. UTC | #4
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

> 
> 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
William Breathitt Gray Oct. 1, 2022, 5:05 p.m. UTC | #5
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
Biju Das Oct. 1, 2022, 5:12 p.m. UTC | #6
> 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
William Breathitt Gray Oct. 1, 2022, 5:43 p.m. UTC | #7
On Sat, Oct 01, 2022 at 05:12:56PM +0000, Biju Das wrote:
> > > > 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

You should define a Signal then for each of the four MTCLK inputs.
Create synapse arrays for each Count respectively; e.g. Count 0 will
have Synapses for MTCLKA and MTCLKB, but Count 1 will probably need
Synapses for all four Signals (the action mode for two of them will be
COUNTER_SYNAPSE_ACTION_NONE depending on the configuration set).

What is the configuration when 32-bit phase counting mode is selected?
Does MTCLKA and MTCLKB serve as the counting signals in this case, with
overflows on the MTU1 register incrementing the MTU2 register?

William Breathitt Gray
Biju Das Oct. 1, 2022, 6:03 p.m. UTC | #8
> -----Original Message-----
> From: William Breathitt Gray <william.gray@linaro.org>
> Sent: 01 October 2022 18:44
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: William Breathitt Gray <wbg@kernel.org>; Rob Herring
> <robh+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>; Philipp Zabel
> <p.zabel@pengutronix.de>; Michael Turquette <mturquette@baylibre.com>;
> Stephen Boyd <sboyd@kernel.org>; Geert Uytterhoeven
> <geert+renesas@glider.be>; Lee Jones <lee@kernel.org>; Uwe Kleine-
> König <u.kleine-koenig@pengutronix.de>; linux-pwm@vger.kernel.org;
> linux-iio@vger.kernel.org; linux-clk@vger.kernel.org;
> devicetree@vger.kernel.org; Chris Paterson
> <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>;
> Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>;
> linux-renesas-soc@vger.kernel.org
> Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver
> 
> On Sat, Oct 01, 2022 at 05:12:56PM +0000, Biju Das wrote:
> > > > > 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
> 
> You should define a Signal then for each of the four MTCLK inputs.
> Create synapse arrays for each Count respectively; e.g. Count 0 will
> have Synapses for MTCLKA and MTCLKB, but Count 1 will probably need
> Synapses for all four Signals (the action mode for two of them will be
> COUNTER_SYNAPSE_ACTION_NONE depending on the configuration set).

OK.

> 
> What is the configuration when 32-bit phase counting mode is selected?

LWA Bit (MTU1/MTU2 Combination Longword Access Control) needs to set for 32-bit phase counting mode.

b0 LWA 0 R/W MTU1/MTU2 Combination Longword Access Control
0: 16-bit access is enabled.
1: 32-bit access is enabled.

> Does MTCLKA and MTCLKB serve as the counting signals in this case,

For 16-bit and 32-bit counting signals same. We can set 

1) MTU 1 and MTU2 signals as MTCLKA and MTCLKB

Or 

2) MTU 1 signal as MTCLKA and MTCLKB and MTU2 signals as MTCLKC and MTCLKD


b1 PHCKSEL 1 R/W External Input Phase Clock Select
Selects the external clock pin for phase counting mode.
0: MTCLKA and MTCLKB are selected for the external phase clock.
1: MTCLKC and MTCLKD are selected for the external phase clock

> with overflows on the MTU1 register incrementing the MTU2 register?

No. that won't happen as we need to use different register for Long word access

These are the regiters used
16-bit:- TCNT{MTU1,MTU2}, TGRA{MTU1,MTU2},  and TGRB{MTU1,MTU2},
32-bit:- MTU1.TCNT_1_LW, MTU1.TGRA_1_LW and MTU1.TGRB_1_LW

Counter in MTU1   MTU1.TCNT Word MTU1.TCNT_1_LW Longword
Counter in MTU2   MTU2.TCNT Word

General register A in MTU1 MTU1.TGRA Word MTU1.TGRA_1_LW Longword
General register A in MTU2 MTU2.TGRA Word

General register B in MTU1 MTU1.TGRB Word MTU1.TGRB_1_LW Longword
General register B in MTU2 MTU2.TGRB Word

Cheers,
Biju
William Breathitt Gray Oct. 1, 2022, 6:34 p.m. UTC | #9
On Sat, Oct 01, 2022 at 06:03:37PM +0000, Biju Das wrote:
> > What is the configuration when 32-bit phase counting mode is selected?
> 
> LWA Bit (MTU1/MTU2 Combination Longword Access Control) needs to set for 32-bit phase counting mode.
> 
> b0 LWA 0 R/W MTU1/MTU2 Combination Longword Access Control
> 0: 16-bit access is enabled.
> 1: 32-bit access is enabled.
> 
> > Does MTCLKA and MTCLKB serve as the counting signals in this case,
> 
> For 16-bit and 32-bit counting signals same. We can set 
> 
> 1) MTU 1 and MTU2 signals as MTCLKA and MTCLKB
> 
> Or 
> 
> 2) MTU 1 signal as MTCLKA and MTCLKB and MTU2 signals as MTCLKC and MTCLKD

I'm having trouble understanding this case. If 32-bit access is enabled
by setting the LWA bit, and the MTU1 signals are configured as MTCLKA
and MTCLKB while at the same time the MTU2 signals are configured as
MTCLKC and MTCLKD, how is the 32-bit count value determined -- wouldn't
MTU1 and MTU2 be counting independently if they each had separate input
clocks fed to them?

William Breathitt Gray

> 
> 
> b1 PHCKSEL 1 R/W External Input Phase Clock Select
> Selects the external clock pin for phase counting mode.
> 0: MTCLKA and MTCLKB are selected for the external phase clock.
> 1: MTCLKC and MTCLKD are selected for the external phase clock
> 
> > with overflows on the MTU1 register incrementing the MTU2 register?
> 
> No. that won't happen as we need to use different register for Long word access
> 
> These are the regiters used
> 16-bit:- TCNT{MTU1,MTU2}, TGRA{MTU1,MTU2},  and TGRB{MTU1,MTU2},
> 32-bit:- MTU1.TCNT_1_LW, MTU1.TGRA_1_LW and MTU1.TGRB_1_LW
> 
> Counter in MTU1   MTU1.TCNT Word MTU1.TCNT_1_LW Longword
> Counter in MTU2   MTU2.TCNT Word
> 
> General register A in MTU1 MTU1.TGRA Word MTU1.TGRA_1_LW Longword
> General register A in MTU2 MTU2.TGRA Word
> 
> General register B in MTU1 MTU1.TGRB Word MTU1.TGRB_1_LW Longword
> General register B in MTU2 MTU2.TGRB Word
> 
> Cheers,
> Biju
Biju Das Oct. 1, 2022, 6:51 p.m. UTC | #10
> Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver
> 
> On Sat, Oct 01, 2022 at 06:03:37PM +0000, Biju Das wrote:
> > > What is the configuration when 32-bit phase counting mode is
> selected?
> >
> > LWA Bit (MTU1/MTU2 Combination Longword Access Control) needs to set
> for 32-bit phase counting mode.
> >
> > b0 LWA 0 R/W MTU1/MTU2 Combination Longword Access Control
> > 0: 16-bit access is enabled.
> > 1: 32-bit access is enabled.
> >
> > > Does MTCLKA and MTCLKB serve as the counting signals in this case,
> >
> > For 16-bit and 32-bit counting signals same. We can set
> >
> > 1) MTU 1 and MTU2 signals as MTCLKA and MTCLKB
> >
> > Or
> >
> > 2) MTU 1 signal as MTCLKA and MTCLKB and MTU2 signals as MTCLKC and
> > MTCLKD
> 
> I'm having trouble understanding this case. If 32-bit access is
> enabled by setting the LWA bit, and the MTU1 signals are configured as
> MTCLKA and MTCLKB while at the same time the MTU2 signals are
> configured as MTCLKC and MTCLKD, how is the 32-bit count value
> determined -- wouldn't
> MTU1 and MTU2 be counting independently if they each had separate
> input clocks fed to them?

It is taken care by the HW. We just configure the register as mentioned below
and hardware provide counter values once feeding the signals to 
either
{MTCLKA and MTCLKB} for both MTU1 and  MTU2 

or 

MTU1{MTCLKA and MTCLKB} and MTU2{MTCLKC and MTCLKD}

The signal feeding is same for 16-bit and 32-bit phase modes.

Note:- I haven't tested 32-bit mode yet. 

Cheers,
Biju

> 
> >
> >
> > b1 PHCKSEL 1 R/W External Input Phase Clock Select Selects the
> > external clock pin for phase counting mode.
> > 0: MTCLKA and MTCLKB are selected for the external phase clock.
> > 1: MTCLKC and MTCLKD are selected for the external phase clock
> >
> > > with overflows on the MTU1 register incrementing the MTU2
> register?
> >
> > No. that won't happen as we need to use different register for Long
> > word access
> >
> > These are the regiters used
> > 16-bit:- TCNT{MTU1,MTU2}, TGRA{MTU1,MTU2},  and TGRB{MTU1,MTU2},
> > 32-bit:- MTU1.TCNT_1_LW, MTU1.TGRA_1_LW and MTU1.TGRB_1_LW
> >
> > Counter in MTU1   MTU1.TCNT Word MTU1.TCNT_1_LW Longword
> > Counter in MTU2   MTU2.TCNT Word
> >
> > General register A in MTU1 MTU1.TGRA Word MTU1.TGRA_1_LW Longword
> > General register A in MTU2 MTU2.TGRA Word
> >
> > General register B in MTU1 MTU1.TGRB Word MTU1.TGRB_1_LW Longword
> > General register B in MTU2 MTU2.TGRB Word
> >
> > Cheers,
> > Biju
William Breathitt Gray Oct. 1, 2022, 7:04 p.m. UTC | #11
On Sat, Oct 01, 2022 at 06:51:48PM +0000, Biju Das wrote:
> > On Sat, Oct 01, 2022 at 06:03:37PM +0000, Biju Das wrote:
> > > > What is the configuration when 32-bit phase counting mode is
> > selected?
> > >
> > > LWA Bit (MTU1/MTU2 Combination Longword Access Control) needs to set
> > for 32-bit phase counting mode.
> > >
> > > b0 LWA 0 R/W MTU1/MTU2 Combination Longword Access Control
> > > 0: 16-bit access is enabled.
> > > 1: 32-bit access is enabled.
> > >
> > > > Does MTCLKA and MTCLKB serve as the counting signals in this case,
> > >
> > > For 16-bit and 32-bit counting signals same. We can set
> > >
> > > 1) MTU 1 and MTU2 signals as MTCLKA and MTCLKB
> > >
> > > Or
> > >
> > > 2) MTU 1 signal as MTCLKA and MTCLKB and MTU2 signals as MTCLKC and
> > > MTCLKD
> > 
> > I'm having trouble understanding this case. If 32-bit access is
> > enabled by setting the LWA bit, and the MTU1 signals are configured as
> > MTCLKA and MTCLKB while at the same time the MTU2 signals are
> > configured as MTCLKC and MTCLKD, how is the 32-bit count value
> > determined -- wouldn't
> > MTU1 and MTU2 be counting independently if they each had separate
> > input clocks fed to them?
> 
> It is taken care by the HW. We just configure the register as mentioned below
> and hardware provide counter values once feeding the signals to 
> either
> {MTCLKA and MTCLKB} for both MTU1 and  MTU2 
> 
> or 
> 
> MTU1{MTCLKA and MTCLKB} and MTU2{MTCLKC and MTCLKD}
> 
> The signal feeding is same for 16-bit and 32-bit phase modes.
> 
> Note:- I haven't tested 32-bit mode yet. 
> 
> Cheers,
> Biju

I'm not quite grokking it yet, but I'll trust that you're right for now.
I suspect it'll make more sense to me once your next revision is
submitted and I've had time to evaluate the code more closely.

Thanks,

William Breathitt Gray

> 
> > 
> > >
> > >
> > > b1 PHCKSEL 1 R/W External Input Phase Clock Select Selects the
> > > external clock pin for phase counting mode.
> > > 0: MTCLKA and MTCLKB are selected for the external phase clock.
> > > 1: MTCLKC and MTCLKD are selected for the external phase clock
> > >
> > > > with overflows on the MTU1 register incrementing the MTU2
> > register?
> > >
> > > No. that won't happen as we need to use different register for Long
> > > word access
> > >
> > > These are the regiters used
> > > 16-bit:- TCNT{MTU1,MTU2}, TGRA{MTU1,MTU2},  and TGRB{MTU1,MTU2},
> > > 32-bit:- MTU1.TCNT_1_LW, MTU1.TGRA_1_LW and MTU1.TGRB_1_LW
> > >
> > > Counter in MTU1   MTU1.TCNT Word MTU1.TCNT_1_LW Longword
> > > Counter in MTU2   MTU2.TCNT Word
> > >
> > > General register A in MTU1 MTU1.TGRA Word MTU1.TGRA_1_LW Longword
> > > General register A in MTU2 MTU2.TGRA Word
> > >
> > > General register B in MTU1 MTU1.TGRB Word MTU1.TGRB_1_LW Longword
> > > General register B in MTU2 MTU2.TGRB Word
> > >
> > > Cheers,
> > > Biju
Biju Das Oct. 1, 2022, 7:21 p.m. UTC | #12
> Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver
> 
> On Sat, Oct 01, 2022 at 06:51:48PM +0000, Biju Das wrote:
> > > On Sat, Oct 01, 2022 at 06:03:37PM +0000, Biju Das wrote:
> > > > > What is the configuration when 32-bit phase counting mode is
> > > selected?
> > > >
> > > > LWA Bit (MTU1/MTU2 Combination Longword Access Control) needs to
> > > > set
> > > for 32-bit phase counting mode.
> > > >
> > > > b0 LWA 0 R/W MTU1/MTU2 Combination Longword Access Control
> > > > 0: 16-bit access is enabled.
> > > > 1: 32-bit access is enabled.
> > > >
> > > > > Does MTCLKA and MTCLKB serve as the counting signals in this
> > > > > case,
> > > >
> > > > For 16-bit and 32-bit counting signals same. We can set
> > > >
> > > > 1) MTU 1 and MTU2 signals as MTCLKA and MTCLKB
> > > >
> > > > Or
> > > >
> > > > 2) MTU 1 signal as MTCLKA and MTCLKB and MTU2 signals as MTCLKC
> > > > and MTCLKD
> > >
> > > I'm having trouble understanding this case. If 32-bit access is
> > > enabled by setting the LWA bit, and the MTU1 signals are
> configured
> > > as MTCLKA and MTCLKB while at the same time the MTU2 signals are
> > > configured as MTCLKC and MTCLKD, how is the 32-bit count value
> > > determined -- wouldn't
> > > MTU1 and MTU2 be counting independently if they each had separate
> > > input clocks fed to them?
> >
> > It is taken care by the HW. We just configure the register as
> > mentioned below and hardware provide counter values once feeding the
> > signals to either {MTCLKA and MTCLKB} for both MTU1 and  MTU2
> >
> > or
> >
> > MTU1{MTCLKA and MTCLKB} and MTU2{MTCLKC and MTCLKD}
> >
> > The signal feeding is same for 16-bit and 32-bit phase modes.
> >
> > Note:- I haven't tested 32-bit mode yet.
> >
> > Cheers,
> > Biju
> 
> I'm not quite grokking it yet, but I'll trust that you're right for
> now.
> I suspect it'll make more sense to me once your next revision is
> submitted and I've had time to evaluate the code more closely.

OK, softwarewise we don't need to anything for 16-bit and 32-bit 
as HW provide separate registers for accessing 32-bit counter values
eventhoug internally, it is formed by combining 2 16-bit counters on
MTU1 and MTU2.

OK, we will have more idea when I submit patches for 32-bit mode.

Cheers,
Biju


> 
> Thanks,
> 
> William Breathitt Gray
> 
> >
> > >
> > > >
> > > >
> > > > b1 PHCKSEL 1 R/W External Input Phase Clock Select Selects the
> > > > external clock pin for phase counting mode.
> > > > 0: MTCLKA and MTCLKB are selected for the external phase clock.
> > > > 1: MTCLKC and MTCLKD are selected for the external phase clock
> > > >
> > > > > with overflows on the MTU1 register incrementing the MTU2
> > > register?
> > > >
> > > > No. that won't happen as we need to use different register for
> > > > Long word access
> > > >
> > > > These are the regiters used
> > > > 16-bit:- TCNT{MTU1,MTU2}, TGRA{MTU1,MTU2},  and TGRB{MTU1,MTU2},
> > > > 32-bit:- MTU1.TCNT_1_LW, MTU1.TGRA_1_LW and MTU1.TGRB_1_LW
> > > >
> > > > Counter in MTU1   MTU1.TCNT Word MTU1.TCNT_1_LW Longword
> > > > Counter in MTU2   MTU2.TCNT Word
> > > >
> > > > General register A in MTU1 MTU1.TGRA Word MTU1.TGRA_1_LW
> Longword
> > > > General register A in MTU2 MTU2.TGRA Word
> > > >
> > > > General register B in MTU1 MTU1.TGRB Word MTU1.TGRB_1_LW
> Longword
> > > > General register B in MTU2 MTU2.TGRB Word
> > > >
> > > > Cheers,
> > > > Biju