Re: [PATCH v2] watchdog: renesas_wdt: Add a few cycles delay

From: Wolfram Sang <wsa@the-dreams.de>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: wim@linux-watchdog.org, linux@roeck-us.net,
	linux-watchdog@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH v2] watchdog: renesas_wdt: Add a few cycles delay
Date: Mon, 3 Jun 2019 15:04:15 +0200	[thread overview]
Message-ID: <20190603130415.wfqwyya4etbmmser@ninjato> (raw)
In-Reply-To: <1559558161-31244-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>

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

On Mon, Jun 03, 2019 at 07:36:01PM +0900, Yoshihiro Shimoda wrote:
> According to the hardware manual of R-Car Gen2 and Gen3,
> software should wait a few RLCK cycles as following:
>  - Delay 2 cycles before setting watchdog counter.
>  - Delay 3 cycles before disabling module clock.
> 
> So, this patch adds such delays.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Despite minor nits:

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

> +static void rwdt_wait(struct rwdt_priv *priv, unsigned int cycles)

I'd think 'rwdt_wait_cycles' would be a more precise name for this
function.

> +	unsigned long delays;

Why not just 'delay'?

> +	/* Delay 3 cycles before disabling module clock */

I like the comments explaining why it is needed.

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