From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 31CA5E95A9A for ; Mon, 9 Oct 2023 11:57:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346380AbjJIL5B (ORCPT ); Mon, 9 Oct 2023 07:57:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48838 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346409AbjJIL5A (ORCPT ); Mon, 9 Oct 2023 07:57:00 -0400 Received: from mail-qk1-x72b.google.com (mail-qk1-x72b.google.com [IPv6:2607:f8b0:4864:20::72b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6B955EB for ; Mon, 9 Oct 2023 04:56:56 -0700 (PDT) Received: by mail-qk1-x72b.google.com with SMTP id af79cd13be357-77421a47db6so257391385a.0 for ; Mon, 09 Oct 2023 04:56:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1696852615; x=1697457415; darn=vger.kernel.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=/f3Y8yieoU6v7s5C4WV6Y0O5J9cEkuy8nWCpTmPUkgw=; b=zs0edn9m3Ie8+EHgtBGtkUnjAvFU2XV3dNUHVJ7HxaIgp+LDUAhrsBtFu7GIpGLyV1 vI1dG3jiULfQvm6YkCpeCQsh3JSjAeNHXJ0RCqWxOAkRy0QmuaHCPrAnk/hQeEaSxRz6 PHvfrw4uwjQ6P/D33cXXcNjEHUy+nOcsRkCWIKwru4niLir82dRsIx0diifo6M7LZFW0 dYEX8i7UA7Y3Ze0unK1qXpOK3YRQFwBUpBQMC0yieltfpUZ3FXRtZX9Mp2NtChVRjffI JDFKvQbvwc/WYthdNY1r858xchc50LJATQrMjq5voLEuRgDi6CKHF+WksWXEfotT/LWE 8o7A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696852615; x=1697457415; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=/f3Y8yieoU6v7s5C4WV6Y0O5J9cEkuy8nWCpTmPUkgw=; b=eTuRdKehTyGQxpUHsnWjl2brwic9XJt2bS81KgDstjGipT8t4OPyMQ67iDpkKHEBTc bPGwm2Edg4J6hVI2RZi8YdGABhVLs6CCt9YMQnlcLOJ8UHOCwXsTqlWvrxrckatjVVsC Ous8R+9bsZudgBBQR22A2aNgiUHoke0t0dwZl6thBEYf3OhudMm+HtmfGNPV40f1H+HR CApbyZSJzRNHe6h8oOdNpfQ2oae34sKXmixpDex1F0+tUDfyQuby0bvQjjtKxZVfnb0b pw4me4NSJg2AyuwTogGiWrJJDw5ZO09YeUSuTENI6ZzXfZMEXl91hcsVb3elQYVTKp4i BlZw== X-Gm-Message-State: AOJu0YyKfHd2Fh906gejAVUMWB5kRGtj/JPkn5pO6wAWqEZIJM9mv2XD IS1392VQwydyHbiyOqZ1aio+livtXxfhwkHcXI4ITA== X-Google-Smtp-Source: AGHT+IHBi7CCnQITPDjc/LN91Jc+xo4HezSa4fpbHKYILisr+wnl+MGmZR0V7UsfYuHDEB90aPuGsjJquCL3jJkS+tU= X-Received: by 2002:a0c:f4cf:0:b0:65b:11b3:6ef5 with SMTP id o15-20020a0cf4cf000000b0065b11b36ef5mr12663046qvm.17.1696852615374; Mon, 09 Oct 2023 04:56:55 -0700 (PDT) MIME-Version: 1.0 References: <20231005155618.700312-1-peter.griffin@linaro.org> <20231005155618.700312-18-peter.griffin@linaro.org> <734eb901-84cc-4a47-a3f6-2313273f79b2@roeck-us.net> In-Reply-To: <734eb901-84cc-4a47-a3f6-2313273f79b2@roeck-us.net> From: Peter Griffin Date: Mon, 9 Oct 2023 12:56:44 +0100 Message-ID: Subject: Re: [PATCH 17/21] watchdog: s3c2410_wdt: Add support for Google tensor SoCs To: Guenter Roeck Cc: robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, mturquette@baylibre.com, conor+dt@kernel.org, sboyd@kernel.org, tomasz.figa@gmail.com, s.nawrocki@samsung.com, linus.walleij@linaro.org, wim@linux-watchdog.org, catalin.marinas@arm.com, will@kernel.org, arnd@arndb.de, olof@lixom.net, cw00.choi@samsung.com, tudor.ambarus@linaro.org, andre.draszik@linaro.org, semen.protsenko@linaro.org, soc@kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-clk@vger.kernel.org, linux-gpio@vger.kernel.org, linux-watchdog@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org Hi Guenter, On Thu, 5 Oct 2023 at 19:58, Guenter Roeck wrote: > > On Thu, Oct 05, 2023 at 04:56:14PM +0100, Peter Griffin wrote: > > This patch adds the compatibles and drvdata for the Google > > gs101 & gs201 SoCs found in Pixel 6 and Pixel 7 phones. Similar > > to Exynos850 it has two watchdog instances, one for each cluster > > and has some control bits in PMU registers. > > > > The watchdog IP found in gs101 SoCs also supports a few > > additional bits/features in the WTCON register which we add > > support for and an additional register detailed below. > > > > dbgack-mask - Enables masking WDT interrupt and reset request > > according to asserted DBGACK input > > > > windowed-mode - Enabled Windowed watchdog mode > > > > Windowed watchdog mode also has an additional register WTMINCNT. > > If windowed watchdog is enabled and you reload WTCNT when the > > value is greater than WTMINCNT, it prompts interrupt or reset > > request as if the watchdog time has expired. > > > > Signed-off-by: Peter Griffin > > --- > > drivers/watchdog/s3c2410_wdt.c | 116 +++++++++++++++++++++++++++++---- > > 1 file changed, 105 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c > > index 0b4bd883ff28..4c23c7e6a3f1 100644 > > --- a/drivers/watchdog/s3c2410_wdt.c > > +++ b/drivers/watchdog/s3c2410_wdt.c > > @@ -31,12 +31,14 @@ > > #define S3C2410_WTDAT 0x04 > > #define S3C2410_WTCNT 0x08 > > #define S3C2410_WTCLRINT 0x0c > > - > > +#define S3C2410_WTMINCNT 0x10 > > #define S3C2410_WTCNT_MAXCNT 0xffff > > > > -#define S3C2410_WTCON_RSTEN (1 << 0) > > -#define S3C2410_WTCON_INTEN (1 << 2) > > -#define S3C2410_WTCON_ENABLE (1 << 5) > > +#define S3C2410_WTCON_RSTEN (1 << 0) > > +#define S3C2410_WTCON_INTEN (1 << 2) > > +#define S3C2410_WTCON_ENABLE (1 << 5) > > +#define S3C2410_WTCON_DBGACK_MASK (1 << 16) > > +#define S3C2410_WTCON_WINDOWED_WD (1 << 20) > > > > #define S3C2410_WTCON_DIV16 (0 << 3) > > #define S3C2410_WTCON_DIV32 (1 << 3) > > @@ -61,12 +63,16 @@ > > #define EXYNOS850_CLUSTER1_NONCPU_INT_EN 0x1644 > > #define EXYNOSAUTOV9_CLUSTER1_NONCPU_OUT 0x1520 > > #define EXYNOSAUTOV9_CLUSTER1_NONCPU_INT_EN 0x1544 > > - > > #define EXYNOS850_CLUSTER0_WDTRESET_BIT 24 > > #define EXYNOS850_CLUSTER1_WDTRESET_BIT 23 > > #define EXYNOSAUTOV9_CLUSTER0_WDTRESET_BIT 25 > > #define EXYNOSAUTOV9_CLUSTER1_WDTRESET_BIT 24 > > - > > +#define GS_CLUSTER0_NONCPU_OUT 0x1220 > > +#define GS_CLUSTER1_NONCPU_OUT 0x1420 > > +#define GS_CLUSTER0_NONCPU_INT_EN 0x1244 > > +#define GS_CLUSTER1_NONCPU_INT_EN 0x1444 > > +#define GS_CLUSTER2_NONCPU_INT_EN 0x1644 > > +#define GS_RST_STAT_REG_OFFSET 0x3B44 > > /** > > * DOC: Quirk flags for different Samsung watchdog IP-cores > > * > > @@ -106,6 +112,8 @@ > > #define QUIRK_HAS_PMU_RST_STAT (1 << 2) > > #define QUIRK_HAS_PMU_AUTO_DISABLE (1 << 3) > > #define QUIRK_HAS_PMU_CNT_EN (1 << 4) > > +#define QUIRK_HAS_DBGACK_BIT (1 << 5) > > +#define QUIRK_HAS_WTMINCNT_REG (1 << 6) > > > > /* These quirks require that we have a PMU register map */ > > #define QUIRKS_HAVE_PMUREG \ > > @@ -263,6 +271,54 @@ static const struct s3c2410_wdt_variant drv_data_exynosautov9_cl1 = { > > QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN, > > }; > > > > +static const struct s3c2410_wdt_variant drv_data_gs101_cl0 = { > > + .mask_reset_reg = GS_CLUSTER0_NONCPU_INT_EN, > > + .mask_bit = 2, > > + .mask_reset_inv = true, > > + .rst_stat_reg = GS_RST_STAT_REG_OFFSET, > > + .rst_stat_bit = 0, > > + .cnt_en_reg = GS_CLUSTER0_NONCPU_OUT, > > + .cnt_en_bit = 8, > > + .quirks = QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_MASK_RESET | QUIRK_HAS_PMU_CNT_EN | > > + QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG, > > +}; > > + > > +static const struct s3c2410_wdt_variant drv_data_gs101_cl1 = { > > + .mask_reset_reg = GS_CLUSTER1_NONCPU_INT_EN, > > + .mask_bit = 2, > > + .mask_reset_inv = true, > > + .rst_stat_reg = GS_RST_STAT_REG_OFFSET, > > + .rst_stat_bit = 1, > > + .cnt_en_reg = GS_CLUSTER1_NONCPU_OUT, > > + .cnt_en_bit = 7, > > + .quirks = QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_MASK_RESET | QUIRK_HAS_PMU_CNT_EN | > > + QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG, > > +}; > > + > > +static const struct s3c2410_wdt_variant drv_data_gs201_cl0 = { > > + .mask_reset_reg = GS_CLUSTER0_NONCPU_INT_EN, > > + .mask_bit = 2, > > + .mask_reset_inv = true, > > + .rst_stat_reg = GS_RST_STAT_REG_OFFSET, > > + .rst_stat_bit = 0, > > + .cnt_en_reg = GS_CLUSTER0_NONCPU_OUT, > > + .cnt_en_bit = 8, > > + .quirks = QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_MASK_RESET | QUIRK_HAS_PMU_CNT_EN | > > + QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG, > > +}; > > + > > +static const struct s3c2410_wdt_variant drv_data_gs201_cl1 = { > > + .mask_reset_reg = GS_CLUSTER1_NONCPU_INT_EN, > > + .mask_bit = 2, > > + .mask_reset_inv = true, > > + .rst_stat_reg = GS_RST_STAT_REG_OFFSET, > > + .rst_stat_bit = 1, > > + .cnt_en_reg = GS_CLUSTER1_NONCPU_OUT, > > + .cnt_en_bit = 7, > > + .quirks = QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_MASK_RESET | QUIRK_HAS_PMU_CNT_EN | > > + QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG, > > +}; > > + > > static const struct of_device_id s3c2410_wdt_match[] = { > > { .compatible = "samsung,s3c2410-wdt", > > .data = &drv_data_s3c2410 }, > > @@ -278,6 +334,10 @@ static const struct of_device_id s3c2410_wdt_match[] = { > > .data = &drv_data_exynos850_cl0 }, > > { .compatible = "samsung,exynosautov9-wdt", > > .data = &drv_data_exynosautov9_cl0 }, > > + { .compatible = "google,gs101-wdt", > > + .data = &drv_data_gs101_cl0 }, > > + { .compatible = "google,gs201-wdt", > > + .data = &drv_data_gs201_cl0 }, > > {}, > > }; > > MODULE_DEVICE_TABLE(of, s3c2410_wdt_match); > > @@ -375,6 +435,21 @@ static int s3c2410wdt_enable(struct s3c2410_wdt *wdt, bool en) > > return 0; > > } > > > > +static void s3c2410wdt_mask_dbgack(struct s3c2410_wdt *wdt, bool mask) > > +{ > > + unsigned long wtcon; > > + > > + if (!(wdt->drv_data->quirks & QUIRK_HAS_DBGACK_BIT)) > > + return; > > + > > + wtcon = readl(wdt->reg_base + S3C2410_WTCON); > > + if (mask) > > + wtcon |= S3C2410_WTCON_DBGACK_MASK; > > + else > > + wtcon &= ~S3C2410_WTCON_DBGACK_MASK; > > + writel(wtcon, wdt->reg_base + S3C2410_WTCON); > > +} > > + > > static int s3c2410wdt_keepalive(struct watchdog_device *wdd) > > { > > struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd); > > @@ -585,9 +660,11 @@ s3c2410_get_wdt_drv_data(struct platform_device *pdev, struct s3c2410_wdt *wdt) > > } > > > > #ifdef CONFIG_OF > > - /* Choose Exynos850/ExynosAutov9 driver data w.r.t. cluster index */ > > + /* Choose Exynos850/ExynosAutov9/gsx01 driver data w.r.t. cluster index */ > > if (variant == &drv_data_exynos850_cl0 || > > - variant == &drv_data_exynosautov9_cl0) { > > + variant == &drv_data_exynosautov9_cl0 || > > + variant == &drv_data_gs101_cl0 || > > + variant == &drv_data_gs201_cl0) { > > u32 index; > > int err; > > > > @@ -600,9 +677,14 @@ s3c2410_get_wdt_drv_data(struct platform_device *pdev, struct s3c2410_wdt *wdt) > > case 0: > > break; > > case 1: > > - variant = (variant == &drv_data_exynos850_cl0) ? > > - &drv_data_exynos850_cl1 : > > - &drv_data_exynosautov9_cl1; > > + if (variant == &drv_data_exynos850_cl0) > > + variant = &drv_data_exynos850_cl1; > > + else if (variant == &drv_data_exynosautov9_cl0) > > + variant = &drv_data_exynosautov9_cl1; > > + else if (variant == &drv_data_gs101_cl0) > > + variant = &drv_data_gs101_cl1; > > + else if (variant == &drv_data_gs201_cl0) > > + variant = &drv_data_gs201_cl1; > > break; > > default: > > return dev_err_probe(dev, -EINVAL, "wrong cluster index: %u\n", index); > > @@ -700,6 +782,8 @@ static int s3c2410wdt_probe(struct platform_device *pdev) > > wdt->wdt_device.bootstatus = s3c2410wdt_get_bootstatus(wdt); > > wdt->wdt_device.parent = dev; > > > > + s3c2410wdt_mask_dbgack(wdt, true); > > + > > /* > > * If "tmr_atboot" param is non-zero, start the watchdog right now. Also > > * set WDOG_HW_RUNNING bit, so that watchdog core can kick the watchdog. > > @@ -712,6 +796,7 @@ static int s3c2410wdt_probe(struct platform_device *pdev) > > s3c2410wdt_start(&wdt->wdt_device); > > set_bit(WDOG_HW_RUNNING, &wdt->wdt_device.status); > > } else { > > + dev_info(dev, "stopping watchdog timer\n"); > > I am not inclined to accept patches adding such noise. > > > s3c2410wdt_stop(&wdt->wdt_device); > > } > > > > @@ -738,6 +823,15 @@ static int s3c2410wdt_probe(struct platform_device *pdev) > > (wtcon & S3C2410_WTCON_RSTEN) ? "en" : "dis", > > (wtcon & S3C2410_WTCON_INTEN) ? "en" : "dis"); > > > > + if (wdt->drv_data->quirks & QUIRK_HAS_DBGACK_BIT) > > + dev_info(dev, "DBGACK %sabled\n", > > + (wtcon & S3C2410_WTCON_DBGACK_MASK) ? "en" : "dis"); > > + > > + if (wdt->drv_data->quirks & QUIRK_HAS_WTMINCNT_REG) > > + dev_info(dev, "windowed watchdog %sabled, wtmincnt=%x\n", > > + (wtcon & S3C2410_WTCON_WINDOWED_WD) ? "en" : "dis", > > + readl(wdt->reg_base + S3C2410_WTMINCNT)); > > ... and I really don't see its value. Thanks for your review feedback. I will remove these dev_info prints in v2. regards, Peter. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 42B4DE95A8E for ; Mon, 9 Oct 2023 11:57:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Vl6gcM0BDwNOK3gGm+6Jzc5UorJVLXmrfwQB+KoAVkw=; b=jVITQEURHhgZqw PTXik6Idmn5ozqn6iQGwT1JybGk6y63mk34gzvTfzuz1xWVR0/mmQ/DKBf4IB2Ws6xDjRAbeoN9Xr 3EXjKR4vvG3jE4gqNvFvpuh6ydCJhaYxzFGUHqybaDJAvfJhDfgEnbXxedbH0azj6OtDoz8A22dkz Ra1Ak9/JHQW9k1AL/KG85jphMDq1XVwV1zxPXy6/OM/h6fFfBHtsNQFlMS8PmgsqXWutw0GIX1jaH UoRHJkJ9Yrk0fPyTIefMJ+C5rO0DaPNOrjCZHpFB0fGsaQW3JUjRcyIUdSoza0VaCEE5tJhSoUhpK YU0uhnxYPeIWnnbEgedg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qposj-00AXhH-0M; Mon, 09 Oct 2023 11:57:05 +0000 Received: from mail-qk1-x72a.google.com ([2607:f8b0:4864:20::72a]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qposf-00AXfq-2D for linux-arm-kernel@lists.infradead.org; Mon, 09 Oct 2023 11:57:03 +0000 Received: by mail-qk1-x72a.google.com with SMTP id af79cd13be357-7740cf93901so256032485a.2 for ; Mon, 09 Oct 2023 04:56:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1696852615; x=1697457415; darn=lists.infradead.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=/f3Y8yieoU6v7s5C4WV6Y0O5J9cEkuy8nWCpTmPUkgw=; b=jQu3Br/3Hj6BAQ3IoAD5L4ekh3RSulSI2zSeT7SBLQ1lKPuGeO9u6iXA41TXC34pan 5i5sMquZnBiC3HUgATm7u+gmWYpSHO+Oo8MHXH42ZUR6CthWx3bUK8mRY1+HluJJDmNP zP6Nox5jLcwkQo11pmxNKN9TDxONYzvoH6mhMLA3KjaQw9G9Bkiiov7t0e9natGqQv6B 3zhFMOojIgGJWEpQmO/2/okDh++yU+9PtXSp+4eY2+oQVf/BuZU9mkv3t6qs2gNyRorh fbHM0nMxrQBZA6ioO/3ydQRVZ+i5P9Vo8YyeEVVBFpeHCp2kkK3Aiw24CJsTimDdQAlo Qqmw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696852615; x=1697457415; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=/f3Y8yieoU6v7s5C4WV6Y0O5J9cEkuy8nWCpTmPUkgw=; b=VngHqOWoKpYrCPBkb0l3WEbZj1FRxPkeugDAxHjZwt2/zLEVZejXLcI9IitvC6q6J5 hPaCq9udvVYC7VTUkd6I1minNQ6yK+pGDiOYqK3lCT2cYzlD40uPxNpsaxVGnw3xTx1N 1B2qis7rJvi06LFo7UlbOJOA0RDAtYzDzU2Dx0FsdaG2v/Dedr8Dgt9rx0k5xXnBn8gM oC2yny+Zop7bmV1KDVj1a9OIT/wJPqbd9Sf6wEhLUfQ5T9uCQP8UfN7wBVb3/CkYFZ9e oq/Fh4/p2lzHZUHCDB9f6JGFZNTOOdGUaoXHPSw9yOfWQbB0Hv0hU2pBpqkT5gDxep6D Iscg== X-Gm-Message-State: AOJu0YyzWdCZXl4/RA07MH6lE5qID9Jg5r4vMDMmFilokVaw4r9AktXN 6NZXqWUiJvr+phANG6k1WLgEy8iZoLM416oF/0LkVA== X-Google-Smtp-Source: AGHT+IHBi7CCnQITPDjc/LN91Jc+xo4HezSa4fpbHKYILisr+wnl+MGmZR0V7UsfYuHDEB90aPuGsjJquCL3jJkS+tU= X-Received: by 2002:a0c:f4cf:0:b0:65b:11b3:6ef5 with SMTP id o15-20020a0cf4cf000000b0065b11b36ef5mr12663046qvm.17.1696852615374; Mon, 09 Oct 2023 04:56:55 -0700 (PDT) MIME-Version: 1.0 References: <20231005155618.700312-1-peter.griffin@linaro.org> <20231005155618.700312-18-peter.griffin@linaro.org> <734eb901-84cc-4a47-a3f6-2313273f79b2@roeck-us.net> In-Reply-To: <734eb901-84cc-4a47-a3f6-2313273f79b2@roeck-us.net> From: Peter Griffin Date: Mon, 9 Oct 2023 12:56:44 +0100 Message-ID: Subject: Re: [PATCH 17/21] watchdog: s3c2410_wdt: Add support for Google tensor SoCs To: Guenter Roeck Cc: robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, mturquette@baylibre.com, conor+dt@kernel.org, sboyd@kernel.org, tomasz.figa@gmail.com, s.nawrocki@samsung.com, linus.walleij@linaro.org, wim@linux-watchdog.org, catalin.marinas@arm.com, will@kernel.org, arnd@arndb.de, olof@lixom.net, cw00.choi@samsung.com, tudor.ambarus@linaro.org, andre.draszik@linaro.org, semen.protsenko@linaro.org, soc@kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-clk@vger.kernel.org, linux-gpio@vger.kernel.org, linux-watchdog@vger.kernel.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231009_045701_750261_0C503F3A X-CRM114-Status: GOOD ( 38.51 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Guenter, On Thu, 5 Oct 2023 at 19:58, Guenter Roeck wrote: > > On Thu, Oct 05, 2023 at 04:56:14PM +0100, Peter Griffin wrote: > > This patch adds the compatibles and drvdata for the Google > > gs101 & gs201 SoCs found in Pixel 6 and Pixel 7 phones. Similar > > to Exynos850 it has two watchdog instances, one for each cluster > > and has some control bits in PMU registers. > > > > The watchdog IP found in gs101 SoCs also supports a few > > additional bits/features in the WTCON register which we add > > support for and an additional register detailed below. > > > > dbgack-mask - Enables masking WDT interrupt and reset request > > according to asserted DBGACK input > > > > windowed-mode - Enabled Windowed watchdog mode > > > > Windowed watchdog mode also has an additional register WTMINCNT. > > If windowed watchdog is enabled and you reload WTCNT when the > > value is greater than WTMINCNT, it prompts interrupt or reset > > request as if the watchdog time has expired. > > > > Signed-off-by: Peter Griffin > > --- > > drivers/watchdog/s3c2410_wdt.c | 116 +++++++++++++++++++++++++++++---- > > 1 file changed, 105 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c > > index 0b4bd883ff28..4c23c7e6a3f1 100644 > > --- a/drivers/watchdog/s3c2410_wdt.c > > +++ b/drivers/watchdog/s3c2410_wdt.c > > @@ -31,12 +31,14 @@ > > #define S3C2410_WTDAT 0x04 > > #define S3C2410_WTCNT 0x08 > > #define S3C2410_WTCLRINT 0x0c > > - > > +#define S3C2410_WTMINCNT 0x10 > > #define S3C2410_WTCNT_MAXCNT 0xffff > > > > -#define S3C2410_WTCON_RSTEN (1 << 0) > > -#define S3C2410_WTCON_INTEN (1 << 2) > > -#define S3C2410_WTCON_ENABLE (1 << 5) > > +#define S3C2410_WTCON_RSTEN (1 << 0) > > +#define S3C2410_WTCON_INTEN (1 << 2) > > +#define S3C2410_WTCON_ENABLE (1 << 5) > > +#define S3C2410_WTCON_DBGACK_MASK (1 << 16) > > +#define S3C2410_WTCON_WINDOWED_WD (1 << 20) > > > > #define S3C2410_WTCON_DIV16 (0 << 3) > > #define S3C2410_WTCON_DIV32 (1 << 3) > > @@ -61,12 +63,16 @@ > > #define EXYNOS850_CLUSTER1_NONCPU_INT_EN 0x1644 > > #define EXYNOSAUTOV9_CLUSTER1_NONCPU_OUT 0x1520 > > #define EXYNOSAUTOV9_CLUSTER1_NONCPU_INT_EN 0x1544 > > - > > #define EXYNOS850_CLUSTER0_WDTRESET_BIT 24 > > #define EXYNOS850_CLUSTER1_WDTRESET_BIT 23 > > #define EXYNOSAUTOV9_CLUSTER0_WDTRESET_BIT 25 > > #define EXYNOSAUTOV9_CLUSTER1_WDTRESET_BIT 24 > > - > > +#define GS_CLUSTER0_NONCPU_OUT 0x1220 > > +#define GS_CLUSTER1_NONCPU_OUT 0x1420 > > +#define GS_CLUSTER0_NONCPU_INT_EN 0x1244 > > +#define GS_CLUSTER1_NONCPU_INT_EN 0x1444 > > +#define GS_CLUSTER2_NONCPU_INT_EN 0x1644 > > +#define GS_RST_STAT_REG_OFFSET 0x3B44 > > /** > > * DOC: Quirk flags for different Samsung watchdog IP-cores > > * > > @@ -106,6 +112,8 @@ > > #define QUIRK_HAS_PMU_RST_STAT (1 << 2) > > #define QUIRK_HAS_PMU_AUTO_DISABLE (1 << 3) > > #define QUIRK_HAS_PMU_CNT_EN (1 << 4) > > +#define QUIRK_HAS_DBGACK_BIT (1 << 5) > > +#define QUIRK_HAS_WTMINCNT_REG (1 << 6) > > > > /* These quirks require that we have a PMU register map */ > > #define QUIRKS_HAVE_PMUREG \ > > @@ -263,6 +271,54 @@ static const struct s3c2410_wdt_variant drv_data_exynosautov9_cl1 = { > > QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN, > > }; > > > > +static const struct s3c2410_wdt_variant drv_data_gs101_cl0 = { > > + .mask_reset_reg = GS_CLUSTER0_NONCPU_INT_EN, > > + .mask_bit = 2, > > + .mask_reset_inv = true, > > + .rst_stat_reg = GS_RST_STAT_REG_OFFSET, > > + .rst_stat_bit = 0, > > + .cnt_en_reg = GS_CLUSTER0_NONCPU_OUT, > > + .cnt_en_bit = 8, > > + .quirks = QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_MASK_RESET | QUIRK_HAS_PMU_CNT_EN | > > + QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG, > > +}; > > + > > +static const struct s3c2410_wdt_variant drv_data_gs101_cl1 = { > > + .mask_reset_reg = GS_CLUSTER1_NONCPU_INT_EN, > > + .mask_bit = 2, > > + .mask_reset_inv = true, > > + .rst_stat_reg = GS_RST_STAT_REG_OFFSET, > > + .rst_stat_bit = 1, > > + .cnt_en_reg = GS_CLUSTER1_NONCPU_OUT, > > + .cnt_en_bit = 7, > > + .quirks = QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_MASK_RESET | QUIRK_HAS_PMU_CNT_EN | > > + QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG, > > +}; > > + > > +static const struct s3c2410_wdt_variant drv_data_gs201_cl0 = { > > + .mask_reset_reg = GS_CLUSTER0_NONCPU_INT_EN, > > + .mask_bit = 2, > > + .mask_reset_inv = true, > > + .rst_stat_reg = GS_RST_STAT_REG_OFFSET, > > + .rst_stat_bit = 0, > > + .cnt_en_reg = GS_CLUSTER0_NONCPU_OUT, > > + .cnt_en_bit = 8, > > + .quirks = QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_MASK_RESET | QUIRK_HAS_PMU_CNT_EN | > > + QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG, > > +}; > > + > > +static const struct s3c2410_wdt_variant drv_data_gs201_cl1 = { > > + .mask_reset_reg = GS_CLUSTER1_NONCPU_INT_EN, > > + .mask_bit = 2, > > + .mask_reset_inv = true, > > + .rst_stat_reg = GS_RST_STAT_REG_OFFSET, > > + .rst_stat_bit = 1, > > + .cnt_en_reg = GS_CLUSTER1_NONCPU_OUT, > > + .cnt_en_bit = 7, > > + .quirks = QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_MASK_RESET | QUIRK_HAS_PMU_CNT_EN | > > + QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG, > > +}; > > + > > static const struct of_device_id s3c2410_wdt_match[] = { > > { .compatible = "samsung,s3c2410-wdt", > > .data = &drv_data_s3c2410 }, > > @@ -278,6 +334,10 @@ static const struct of_device_id s3c2410_wdt_match[] = { > > .data = &drv_data_exynos850_cl0 }, > > { .compatible = "samsung,exynosautov9-wdt", > > .data = &drv_data_exynosautov9_cl0 }, > > + { .compatible = "google,gs101-wdt", > > + .data = &drv_data_gs101_cl0 }, > > + { .compatible = "google,gs201-wdt", > > + .data = &drv_data_gs201_cl0 }, > > {}, > > }; > > MODULE_DEVICE_TABLE(of, s3c2410_wdt_match); > > @@ -375,6 +435,21 @@ static int s3c2410wdt_enable(struct s3c2410_wdt *wdt, bool en) > > return 0; > > } > > > > +static void s3c2410wdt_mask_dbgack(struct s3c2410_wdt *wdt, bool mask) > > +{ > > + unsigned long wtcon; > > + > > + if (!(wdt->drv_data->quirks & QUIRK_HAS_DBGACK_BIT)) > > + return; > > + > > + wtcon = readl(wdt->reg_base + S3C2410_WTCON); > > + if (mask) > > + wtcon |= S3C2410_WTCON_DBGACK_MASK; > > + else > > + wtcon &= ~S3C2410_WTCON_DBGACK_MASK; > > + writel(wtcon, wdt->reg_base + S3C2410_WTCON); > > +} > > + > > static int s3c2410wdt_keepalive(struct watchdog_device *wdd) > > { > > struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd); > > @@ -585,9 +660,11 @@ s3c2410_get_wdt_drv_data(struct platform_device *pdev, struct s3c2410_wdt *wdt) > > } > > > > #ifdef CONFIG_OF > > - /* Choose Exynos850/ExynosAutov9 driver data w.r.t. cluster index */ > > + /* Choose Exynos850/ExynosAutov9/gsx01 driver data w.r.t. cluster index */ > > if (variant == &drv_data_exynos850_cl0 || > > - variant == &drv_data_exynosautov9_cl0) { > > + variant == &drv_data_exynosautov9_cl0 || > > + variant == &drv_data_gs101_cl0 || > > + variant == &drv_data_gs201_cl0) { > > u32 index; > > int err; > > > > @@ -600,9 +677,14 @@ s3c2410_get_wdt_drv_data(struct platform_device *pdev, struct s3c2410_wdt *wdt) > > case 0: > > break; > > case 1: > > - variant = (variant == &drv_data_exynos850_cl0) ? > > - &drv_data_exynos850_cl1 : > > - &drv_data_exynosautov9_cl1; > > + if (variant == &drv_data_exynos850_cl0) > > + variant = &drv_data_exynos850_cl1; > > + else if (variant == &drv_data_exynosautov9_cl0) > > + variant = &drv_data_exynosautov9_cl1; > > + else if (variant == &drv_data_gs101_cl0) > > + variant = &drv_data_gs101_cl1; > > + else if (variant == &drv_data_gs201_cl0) > > + variant = &drv_data_gs201_cl1; > > break; > > default: > > return dev_err_probe(dev, -EINVAL, "wrong cluster index: %u\n", index); > > @@ -700,6 +782,8 @@ static int s3c2410wdt_probe(struct platform_device *pdev) > > wdt->wdt_device.bootstatus = s3c2410wdt_get_bootstatus(wdt); > > wdt->wdt_device.parent = dev; > > > > + s3c2410wdt_mask_dbgack(wdt, true); > > + > > /* > > * If "tmr_atboot" param is non-zero, start the watchdog right now. Also > > * set WDOG_HW_RUNNING bit, so that watchdog core can kick the watchdog. > > @@ -712,6 +796,7 @@ static int s3c2410wdt_probe(struct platform_device *pdev) > > s3c2410wdt_start(&wdt->wdt_device); > > set_bit(WDOG_HW_RUNNING, &wdt->wdt_device.status); > > } else { > > + dev_info(dev, "stopping watchdog timer\n"); > > I am not inclined to accept patches adding such noise. > > > s3c2410wdt_stop(&wdt->wdt_device); > > } > > > > @@ -738,6 +823,15 @@ static int s3c2410wdt_probe(struct platform_device *pdev) > > (wtcon & S3C2410_WTCON_RSTEN) ? "en" : "dis", > > (wtcon & S3C2410_WTCON_INTEN) ? "en" : "dis"); > > > > + if (wdt->drv_data->quirks & QUIRK_HAS_DBGACK_BIT) > > + dev_info(dev, "DBGACK %sabled\n", > > + (wtcon & S3C2410_WTCON_DBGACK_MASK) ? "en" : "dis"); > > + > > + if (wdt->drv_data->quirks & QUIRK_HAS_WTMINCNT_REG) > > + dev_info(dev, "windowed watchdog %sabled, wtmincnt=%x\n", > > + (wtcon & S3C2410_WTCON_WINDOWED_WD) ? "en" : "dis", > > + readl(wdt->reg_base + S3C2410_WTMINCNT)); > > ... and I really don't see its value. Thanks for your review feedback. I will remove these dev_info prints in v2. regards, Peter. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel