From:   Biju Das <biju.das.jz@bp.renesas.com>
To:     Fabrizio Castro <fabrizio.castro.jz@renesas.com>,
        Wim Van Sebroeck <wim@linux-watchdog.org>,
        Guenter Roeck <linux@roeck-us.net>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Geert Uytterhoeven <geert+renesas@glider.be>
CC:     Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
        "linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Chris Paterson <Chris.Paterson2@renesas.com>,
        Biju Das <biju.das@bp.renesas.com>,
        Fabrizio Castro <fabrizio.castro.jz@renesas.com>,
        "linux-renesas-soc@vger.kernel.org" 
        <linux-renesas-soc@vger.kernel.org>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Jacopo Mondi <jacopo@jmondi.org>
Subject: RE: [PATCH 1/2] watchdog: rzg2l_wdt: Issue a reset before we put the
 PM clocks
Thread-Topic: [PATCH 1/2] watchdog: rzg2l_wdt: Issue a reset before we put the
 PM clocks
Thread-Index: AQHY+nqlN8PVH6kQBUuXev2WjyIal67FLjeA
Date:   Wed, 8 Feb 2023 07:52:06 +0000
Message-ID: <OS0PR01MB5922D6BBEF769694B8846A7086D89@OS0PR01MB5922.jpnprd01.prod.outlook.com>
References: <20221117114907.138583-1-fabrizio.castro.jz@renesas.com>
 <20221117114907.138583-2-fabrizio.castro.jz@renesas.com>
In-Reply-To: <20221117114907.138583-2-fabrizio.castro.jz@renesas.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?DagTCLA9MxVYRjM0+bWv7fciEY2gCrv5Gdq4R2uzk7PZyw+qjwSbOXpjfDvo?=
 =?us-ascii?Q?6TWs/v+VHoRBJaG/AvyH01EEYhMBZjR5yMA4/gjrP3/GfZX9Mq/R+X+zAxiQ?=
 =?us-ascii?Q?R8ul3uLZz4XqRx5PQcPQ7HQfXqIDnoDgQvyHwZlhPrGz1UtRDNcGYf/q+TVq?=
 =?us-ascii?Q?Cr8qroFrZcgEs7jAkODCO6uA9Kn9/anqC4/0culLsWyYoqp5+Tm0uxtC+Mca?=
 =?us-ascii?Q?ewZ9aZKEnowIf/en1ZZWgwIvWTMczHkgp9VlxIsglxQURwmL9h8OiSjf8BaS?=
 =?us-ascii?Q?rXyBbO4Ej6dTQn7q1IIemval9TBBEZhP4Tydios1b18rGFz/68xXKh06iv9G?=
 =?us-ascii?Q?YupFgN/MjV8Pt3u4L9ptYp8q0iPYMB/K/VpVSwNsPZSegrloBtE2pQYmv9O4?=
 =?us-ascii?Q?akGv6d91cGcKnMvCLxMCFn483ni6D6sRtVNtU+rpD6nAdSvs/479VH/Xz6Ha?=
 =?us-ascii?Q?LPiwDgnoaJ5jecJ82v7AIrLL57ZYrBXn/nFnvY4UOY4qRHQH1/5J11AdTQrh?=
 =?us-ascii?Q?DfclTbs2OCsm2n/0Cpajw+GwXPhCVFYyFC3prNOcGNWK1iCadPJUzMbdoVSu?=
 =?us-ascii?Q?BD3c0H4uNHSMNxrCVXbUlLW5/n49DMo7zQS+lGIGV3FFq8Dcr61PG8NFACrj?=
 =?us-ascii?Q?MknmtKICVGTIvWxvaJlnVYloYOJZZ+U6HOwLl9jC/Qfa+7yjomt/Yp1bSuhv?=
 =?us-ascii?Q?CEc1jO0/jd9XeHIbXJ46pqG1BleLSE/Flir3S/wf/5h76lxv0ye/MwUrMTNu?=
 =?us-ascii?Q?3/biANFva/jwHBg6mc1HWheIC/c0nJUqcwvPJOLWdckMTgR49Pb/sF/v/et8?=
 =?us-ascii?Q?HIGWOIcBDM31Is2vGnpig9ALROIO0ljjw2JclDg0RuByYOwk0aC918JXWImm?=
 =?us-ascii?Q?7iDiSBV7w3ut+OcfAoCNaoMBT0PUSGXYhA3aSbRU4JCcP3xrvrUMETM9J3fs?=
 =?us-ascii?Q?nkBUs97NvEAawX+EoTHoGZLEY0EGsbVy6n6dkSIHIwcoGDb88x8QSbleg2Gh?=
 =?us-ascii?Q?1fYdTYOPmHG4Ou5/+G3p6BJkoYPnS0MV3wafDoCfercmAZFGjHcbLCuDKDgZ?=
 =?us-ascii?Q?5tyfpK/vu74nJCyTv96m6IEcwkLtGyWFVNgN+mb7rjBvRg1VEAvdabJyrKiO?=
 =?us-ascii?Q?0stylbVtnZjRAsYmmfXVFJogHxm6fW1NXss+FccMufcMBwcMHaLFzeweWJe3?=
 =?us-ascii?Q?l5FScOP1OeaFYYvHTXZLEVuoY9ubFKQpXRvkSwhGnf2jfgF+a+SugyDP7p6a?=
 =?us-ascii?Q?78SvZ61Iussyo7exb1tGUXdRROiF1zBHn2cu4wmHemdx32ixjfbCVa9RPBd+?=
 =?us-ascii?Q?Zf1Q0QzdAmWsv3Qrz6i50sMDX5oJmZQ3/3Kngpvys03M5YZe+DjAT8qAMkNP?=
 =?us-ascii?Q?qDpenktTHN+lUQ6W65Yr5oFOxVxk/iRPAo/J4QZuF6Cc8DiI9XV4jLFn5ibC?=
 =?us-ascii?Q?0zg6eLL84puL0sLpyPlQu8YhULl9vUXYLw+VwmQ4SZkEIlGrXf9+iE4Mlzis?=
 =?us-ascii?Q?mK9zSGDRtYw8opIpzPuPJUxcXyBPvE7mnnkvejuEdowrVckZ39Xh8WvEPSby?=
 =?us-ascii?Q?mgRlJ07FthrGmjh+gDmu8Y/GkEWBZXWqHOvmw0oPknW+EKY19LQyNY1lLwQQ?=
 =?us-ascii?Q?aA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: bp.renesas.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: OS0PR01MB5922.jpnprd01.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 86af9f97-9e8e-4cb9-6ac8-08db09a95fe2
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Feb 2023 07:52:06.5229
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 4e8suEt7U1cv7ETdpW8E+JD35ZcBnda1oRUnkwvRK8tGxPU/2UJKB8l5IadWH2icwUS6KkHzLbtY3hX72qU+E9h54SZD6qj1VMlSstVo4j4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: OS3PR01MB7850
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Fabrizio,

Thanks for the patch.

> Subject: [PATCH 1/2] watchdog: rzg2l_wdt: Issue a reset before we put the=
 PM
> clocks
>=20
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>=20
> On RZ/Five SoC it was observed that setting timeout (to say 1 sec) wouldn=
't
> reset the system.
>=20
> The procedure described in the HW manual (Procedure for Activating Module=
s)
> for activating the target module states we need to start supply of the cl=
ock
> module before applying the reset signal. This patch makes sure we follow =
the
> same procedure to clear the registers of the WDT module, fixing the issue=
s
> seen on RZ/Five SoC.
>=20
> While at it re-used rzg2l_wdt_stop() in rzg2l_wdt_set_timeout() as it has
> the same function calls.
>=20
> Fixes: 4055ee81009e ("watchdog: rzg2l_wdt: Add set_timeout callback")
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Cheers,
Biju

> ---
>  drivers/watchdog/rzg2l_wdt.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>=20
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 974a4194a8fd..ceca42db0837 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -115,25 +115,23 @@ static int rzg2l_wdt_stop(struct watchdog_device
> *wdev)  {
>  	struct rzg2l_wdt_priv *priv =3D watchdog_get_drvdata(wdev);
>=20
> -	pm_runtime_put(wdev->parent);
>  	reset_control_reset(priv->rstc);
> +	pm_runtime_put(wdev->parent);
>=20
>  	return 0;
>  }
>=20
>  static int rzg2l_wdt_set_timeout(struct watchdog_device *wdev, unsigned =
int
> timeout)  {
> -	struct rzg2l_wdt_priv *priv =3D watchdog_get_drvdata(wdev);
> -
>  	wdev->timeout =3D timeout;
>=20
>  	/*
>  	 * If the watchdog is active, reset the module for updating the WDTSET
> -	 * register so that it is updated with new timeout values.
> +	 * register by calling rzg2l_wdt_stop() (which internally calls
> reset_control_reset()
> +	 * to reset the module) so that it is updated with new timeout values.
>  	 */
>  	if (watchdog_active(wdev)) {
> -		pm_runtime_put(wdev->parent);
> -		reset_control_reset(priv->rstc);
> +		rzg2l_wdt_stop(wdev);
>  		rzg2l_wdt_start(wdev);
>  	}
>=20
> --
> 2.34.1