Hi Biju,
On Mon, Jun 11, 2018 at 1:21 PM Biju Das <biju.das@xxxxxxxxxxxxxx> wrote:
> > Subject: Re: [PATCH] clocksource/drivers/sh_cmt: wait for CMCNT on init
> > On Fri, Jun 8, 2018 at 6:24 PM Biju Das <biju.das@xxxxxxxxxxxxxx> wrote:
> > > As per section 57A.3.5/69A.3.5/79.A.3.5 of rz/g/r-car gen2/3 hardware
> > > manual,it is mentioned that we need to provide 2 cycles in counter
> > > input clock (RCLK) for reflecting written data to counter behaviour.
> > > Adding sufficient wait to let the CMCNT register value settle before
> > > starting the timer channel.
> >
> > RCLK usually runs at ca. 32 kHz, so 32µs should be sufficient on R-Car
> > Gen2/3 and RZ/G1.
> > R-Mobile A1 is the exception: RCLK runs at 23 kHz, so you need 43µs to be
> > safe.
> >
> > > It fixes the error "sh_cmt ffca0000.timer: ch1: cannot clear CMCNT"
> > >
> > > Signed-off-by: Biju Das <biju.das@xxxxxxxxxxxxxx>
> > > Reviewed-by: Chris Paterson <chris.paterson2@xxxxxxxxxxx>
> > > ---
> > > Hello,
> > >
> > > During cmt testing, the tool
> > > (tools/testing/selftests/timers/clocksource-switch.c)
> > > is complaining about the error "sh_cmt ffca0000.timer: ch1: cannot clear
> > CMCNT".
> > > The above patch fixes this issue is by adding sufficient wait to let
> > > the CMCNT register value settle before starting the timer channel.
> > >
> > > This issue is reproduced on Koelsch/RZ/G1[ME] based iwave boards
> > > etc..., as I assume the same issue should be present on lager etc. as well?
> >
> > > --- a/drivers/clocksource/sh_cmt.c
> > > +++ b/drivers/clocksource/sh_cmt.c
> > > @@ -328,7 +328,7 @@ static void sh_cmt_start_stop_ch(struct
> > > sh_cmt_channel *ch, int start)
> > >
> > > static int sh_cmt_enable(struct sh_cmt_channel *ch) {
> > > - int k, ret;
> > > + int j, k, ret;
> > >
> > > pm_runtime_get_sync(&ch->cmt->pdev->dev);
> > > dev_pm_syscore_device(&ch->cmt->pdev->dev, true);
> > > @@ -368,11 +368,17 @@ static int sh_cmt_enable(struct sh_cmt_channel
> > *ch)
> > > * While at it, we're supposed to clear out the CMCNT as of this
> > > * moment, so make sure it's processed properly here. This will
> > > * take RCLKx2 at maximum.
> > > + *
> > > + * Similar register access usage for CMCNT is mentioned in R-Car
> > > + * Gen[2/3]/RZ/G1 user's manual, RCLKx2 for cmt0 and RCLKx2 or
> > > + * CPϕx2 (CPEXϕx2)) for cmt1.
> > > */
> > > - for (k = 0; k < 100; k++) {
> > > - if (!sh_cmt_read_cmcnt(ch))
> > > - break;
> > > - udelay(1);
> >
> > 100 * 1µs = 100µs, so the original code should be sufficient?
>
> From the test results, the original code is not sufficient. It needs 137 µs on koelsch platform to settle this value.
>
> > Or should it wait 42µs unconditionally, i.e. without checking CMCNT?
>
> Yes, Adding wait 42µs unconditionally, just before the for loop also fixes the issue, since worst case wait is 137 µs.
> What is your suggestion here?
>
> > The datasheet mentions some other registers 2 RCLK cycles, too.
> > Should there be a fixed 2 RCLK delay in sh_cmt_write_cmcsr() and
> > sh_cmt_write_cmcor(), which are both called just before
> > sh_cmt_write_cmcnt()
> > above (all out of diff context)?
>
> I am not sure on this.
>
> > > + for (j = 0; j < 2; j++) {
> > > + for (k = 0; k < 100; k++) {
> > > + if (!sh_cmt_read_cmcnt(ch))
> > > + break;
> > > + udelay(1);
> > > + }
> >
> > This is now two loops, with two checks for CMCNT, which looks strange to
> > me.
> > Do you have figures for the number of loops needed, for both the first (j=0)
> > and the 2nd (j=1) cycle?
>
> From the test results, first cycle j=0, loop count(max)=100
> Second cycle j=2, loop count(max)= 37.
137µs ~= 3 x 42µs, and there are 3 register writes above.
Is that a coincidence? ;-)
Does it work if you insert udelay(42) after the register writes in
sh_cmt_write_cmcsr(), sh_cmt_write_cmcor(), and sh_cmt_write_cmcnt()?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
