Hi Biju,
Thanks for your patch!
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?
Or should it wait 42µs unconditionally, i.e. without checking CMCNT?
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)?
> + 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?
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
