From:   George Cherian <gcherian@marvell.com>
To:     Guenter Roeck <linux@roeck-us.net>
CC:     "wim@linux-watchdog.org" <wim@linux-watchdog.org>,
        "zhangshaokun@hisilicon.com" <zhangshaokun@hisilicon.com>,
        "linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] watchdog: sbsa_wdog: Make sure the timeout programming
 is within the limits
Thread-Topic: [PATCH v2] watchdog: sbsa_wdog: Make sure the timeout
 programming is within the limits
Thread-Index: AQHZPCt93Xej8F93+EuTg7ZQ9cdYSA==
Date:   Thu, 9 Feb 2023 02:09:07 +0000
Message-ID: <BYAPR18MB26792B48DF03593B3CFAF608C5D99@BYAPR18MB2679.namprd18.prod.outlook.com>
References: <20230208083736.1237256-1-george.cherian@marvell.com>
 <2f2dcb7c-e5bc-7e97-71e0-eebdc72fbd03@roeck-us.net>
 <BYAPR18MB267995FB55301778894B3334C5D89@BYAPR18MB2679.namprd18.prod.outlook.com>
 <20230208175547.GA3065515@roeck-us.net>
In-Reply-To: <20230208175547.GA3065515@roeck-us.net>
Accept-Language: en-IN, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?tTe07zI8bKPmPzxemi/WCi9BwwRus2tGtUuZO3Cw7F/Ta+dxk4qvsBQhL23L?=
 =?us-ascii?Q?yhGWJfr3YwQ/NIDDdXHPccYFTmBbKOQW04sdMuJ24cUpyYJh4D+JVySx9S6k?=
 =?us-ascii?Q?sXJAjS1io1pPydNeUMxzBaH1XPFoGOplxfrjfP8BlcVODtB7i3frIaMRBwnu?=
 =?us-ascii?Q?kOkdx1Spt/dHZUboqVdpi/vhA8aLMVjzyCJm0vJ8Lmf3+iiNw9v7JLXNLAWY?=
 =?us-ascii?Q?ycBFfO9fdrZpFnFGbWU6ECRbFpKMOYvGRsw2ybII7v2jU4n2zGe92T8k62Bo?=
 =?us-ascii?Q?34hHhEbf70914IdNTiZPCakBQHZhYeT40nblmM77p7q92WjDRzr1a+BP9p/6?=
 =?us-ascii?Q?NTLJIJIR86HCfuorxylIH7NBqLsTdNjz/ZEmkpfpbY8cNOTvyBF1o6Ss6A7R?=
 =?us-ascii?Q?TOsJcIxMjfq3DYPS4Vk+lJGZg51CWLlD9D1MH+vXsWYutZTb8xBZqGeh7dXG?=
 =?us-ascii?Q?D8tjibb2qLtG3EHZEd77MpfPlbLPCIyxDJyBvwxx4gbvbo1L1SvNcUK3AmhM?=
 =?us-ascii?Q?QHIXafxeLmddu3QYbWtLm9K0je8DHUB3eO11NsLPEbgRg3mFQc9Yyraxdd7h?=
 =?us-ascii?Q?jmy2zd2KJwiAkLKf/zwKC/KUVnWb89S5idddESDsG1wvhMAZ0U2oLgvq+d0l?=
 =?us-ascii?Q?JhHIt0yzchd0gL9tF9SuCQW5EMPBnmiG+W6/j8hilbaJDMGOkO/5/Xp59pdF?=
 =?us-ascii?Q?pHR5z39o1/jDHWzpLmrWubivQ8jaMIgPXkB1N8w7wKSdI0MR6A3OBhgE1YUi?=
 =?us-ascii?Q?eOU+LObDYhm6lzOV3WwttdP9EFqzTZgWwZAZ7rffheEPzISUa7+0Plw1ftxb?=
 =?us-ascii?Q?SotyozDIzsxgqKbS23PCKMLgtsOg2zTY3E2OM8GlnSdFAxKqtHPq2yaK0eY5?=
 =?us-ascii?Q?yVu3uhHZdG261keT4DhIYb57MDxNjRYUwgA/ple9eNRK5uypuSSYwTR8sk/q?=
 =?us-ascii?Q?4UicAddDwROj/Pb821/6yIbjmJNDvPQEQhM8jOJzi4t07sb3JYD4G/pKxU/r?=
 =?us-ascii?Q?/mpoV+lRBeGmIFEus0PNVl6QapXZ3IXQ8T7D6g/iqvfpAql1qL4OQyWQGood?=
 =?us-ascii?Q?loBERhPH9gmVX9S6n+4mUucrlO6DSSVAY1VqxRMZ4CmbbdN254fkDTtlSJ7r?=
 =?us-ascii?Q?+MFxvb71/mkVhVI7txc3HTQRdchyLNn0nAdHmBue5lj32R/5OzbA1yD860Sc?=
 =?us-ascii?Q?98AfCP24hX+xvlGgFC0WZ3EbiXIEopTkNbA+zHKyKHhZfR70D4voXtSjzHBG?=
 =?us-ascii?Q?apsx/V9QaWAT5nEvz3h7YoQG/2WACYcJx3Ny+/x7WeKZQZqqcm5YSKNSxLm4?=
 =?us-ascii?Q?bNDenPJBj4SD9iChHsBPAVgdBa8CIRPK1W8YUFj0UJVwlQoipa5dOcPQwug3?=
 =?us-ascii?Q?hoefj91p3pXa2/6IIBz2RVM1fRPLRiSZ3GVHQxmCOp90k/f+JIHGbwDbTNy8?=
 =?us-ascii?Q?ToWkVWGa9JGTPeBiNB+m6w9Am65klWjPyqtGue03gleopYLtHrWSXV0Iw0nm?=
 =?us-ascii?Q?D5u+HCHKaj1/DWEaKz8MFNaTiXZo0WHpaG0PRso47a006BPyEAzVJUU2O3Kv?=
 =?us-ascii?Q?VvPpKzU31W+ANyPas+eGfoEI/WfYQKeFigv8D+99?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BYAPR18MB2679.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: da3bdb44-1d15-4d19-24ba-08db0a42a017
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Feb 2023 02:09:07.2422
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: HPQvZ3/ORC7kxJ5rl3PwVb3PQnaJh1L007VH971MHifcB1XQkFshzJElKqRKCdVgaDwGtRQIY/JQ0SJoV8DH+g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR18MB4058
X-Proofpoint-GUID: WonbXGvU_WFMhtO-2fTFrhd6nx_BSXfF
X-Proofpoint-ORIG-GUID: WonbXGvU_WFMhtO-2fTFrhd6nx_BSXfF
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.122.1
 definitions=2023-02-08_11,2023-02-08_02,2022-06-22_01
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Wednesday, February 8, 2023 11:26 PM
> To: George Cherian <gcherian@marvell.com>
> Cc: wim@linux-watchdog.org; zhangshaokun@hisilicon.com; linux-
> watchdog@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2] watchdog: sbsa_wdog: Make sure the timeout
> programming is within the limits
>=20
> ----------------------------------------------------------------------
> On Wed, Feb 08, 2023 at 05:20:34PM +0000, George Cherian wrote:
> >
> >
> > > -----Original Message-----
> > > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter
> Roeck
> > > Sent: Wednesday, February 8, 2023 8:41 PM
> > > To: George Cherian <gcherian@marvell.com>; wim@linux-watchdog.org;
> > > zhangshaokun@hisilicon.com
> > > Cc: linux-watchdog@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v2] watchdog: sbsa_wdog: Make sure the timeout
> > > programming is within the limits
> > >
> > >
> > > --------------------------------------------------------------------
> > > -- On 2/8/23 00:37, George Cherian wrote:
> > > > Make sure to honour the max_hw_heartbeat_ms while programming
> the
> > > > timeout value to WOR. Clamp the timeout passed to
> > > > sbsa_gwdt_set_timeout() to make sure the programmed value is
> > > > within
> > > the permissible range.
> > > >
> > > > Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version
> > > > 1")
> > > >
> > > > Signed-off-by: George Cherian <george.cherian@marvell.com>
> > > > ---
> > > >   drivers/watchdog/sbsa_gwdt.c | 1 +
> > > >   1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/watchdog/sbsa_gwdt.c
> > > > b/drivers/watchdog/sbsa_gwdt.c index 9791c74aebd4..ee1039a652f1
> > > 100644
> > > > --- a/drivers/watchdog/sbsa_gwdt.c
> > > > +++ b/drivers/watchdog/sbsa_gwdt.c
> > > > @@ -149,6 +149,7 @@ static int sbsa_gwdt_set_timeout(struct
> > > watchdog_device *wdd,
> > > >   {
> > > >   	struct sbsa_gwdt *gwdt =3D watchdog_get_drvdata(wdd);
> > > >
> > > > +	timeout =3D clamp_t(unsigned int, timeout, 1, wdd-
> > > >max_hw_heartbeat_ms
> > > > +/ 1000);
> > > >   	wdd->timeout =3D timeout;
> > > >
> > >
> > > Wrong order. wdd->timeout should still contain the requested timeout.
> > But then the requested timeout is not being set in HW.
> > Isn't that a disconnect?
>=20
> No. The driver is supposed to set a timeout no larger than
> max_hw_heartbeat_ms. The watchdog core then takes care of differences
> between ->timeout (as seen by the user) and max_hw_heartbeat_ms and
> pings the watchdog as needed.
Thanks for the explanation. Will post v3.

-George
>=20
> Guenter