diff mbox series

[v3,15/20] watchdog: s3c2410_wdt: Add support for Google tensor SoCs

Message ID 20231011184823.443959-16-peter.griffin@linaro.org
State New
Headers show
Series Add minimal Tensor/GS101 SoC support and Oriole/Pixel6 board | expand

Commit Message

Peter Griffin Oct. 11, 2023, 6:48 p.m. UTC
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 <peter.griffin@linaro.org>
---
 drivers/watchdog/s3c2410_wdt.c | 127 ++++++++++++++++++++++++++++++---
 1 file changed, 116 insertions(+), 11 deletions(-)

Comments

Sam Protsenko Oct. 12, 2023, 2:32 a.m. UTC | #1
On Wed, Oct 11, 2023 at 1:49 PM Peter Griffin <peter.griffin@linaro.org> 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.
>

A couple of thoughts in addition to what Guenter said.
Krzysztof Kozlowski Oct. 12, 2023, 6:22 a.m. UTC | #2
On 11/10/2023 20:48, 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 <peter.griffin@linaro.org>
> ---
>  drivers/watchdog/s3c2410_wdt.c | 127 ++++++++++++++++++++++++++++++---
>  1 file changed, 116 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 0b4bd883ff28..36c170047180 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)
> @@ -51,6 +53,7 @@
>  
>  #define S3C2410_WATCHDOG_ATBOOT		(0)
>  #define S3C2410_WATCHDOG_DEFAULT_TIME	(15)
> +#define S3C2410_WINDOW_MULTIPLIER	2
>  
>  #define EXYNOS5_RST_STAT_REG_OFFSET		0x0404
>  #define EXYNOS5_WDT_DISABLE_REG_OFFSET		0x0408
> @@ -67,6 +70,13 @@
>  #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 +116,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 +275,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 = {

I do not see the difference between this and drv_data_gs101_cl0. Same
for cluster 1. Looks like these are compatible, so make them compatible.

Also same concerns as Guenter's has.

Best regards,
Krzysztof
Peter Griffin Oct. 17, 2023, 9:26 p.m. UTC | #3
Hi Guenter,

Thanks for your review.

On Wed, 11 Oct 2023 at 22:20, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Wed, Oct 11, 2023 at 07:48:18PM +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.
>
> Sorry, I don't understand what the code is doing here.

No need to be sorry, I appreciate the review feedback and questions :)

>
> It looks like it enables window mode unconditionally (?). If so,
> what is the impact ? Does it mean that any code requesting multiple
> keepalives in a row on the affected hardware will now cause an
> immediate reset ? If so, what is the rationale ?

Essentially yes, it stops continual keepalives being issued as a
keepalive is only considered valid at certain times in the windowed
cycle.

By way of example, if the watchdog interval is 30s and wtmincnt is set
to *half* of wtcnt, then we would have a "closed window" for the first
15s whereby issuing a keepalive will reset the system, and a "open
window" for the second half of the interval where issuing a keepalive
would reset wtcnt (and be considered a valid service of the watchdog).

The rationale is to stop the watchdog being re-armed "too much" and
it should enable detection of abnormally early as well as abnormally
late servicing of the watchdog.

> Alternatively, if it enables window mode and configures it such
> that WTMINCNT is always equal or larger than WTCNT, what is the
> point of enabling window mode in the first place ?

I looked again, and you are indeed correct that the code currently is
essentially turning windowed mode into a no-op. This appears to be a
bug in the downstream driver as well. Setting WTMINCNT to half of
WTCNT results in a 50% closed, 50% open window, and works on the
hardware as I would expect. I guess this is a good example of why
upstreaming your code, and public code review results in better quality
drivers.

Thanks,

Peter
Peter Griffin Oct. 17, 2023, 9:39 p.m. UTC | #4
Hi Sam,

Thanks for your review.

On Thu, 12 Oct 2023 at 03:32, Sam Protsenko <semen.protsenko@linaro.org> wrote:
>
> On Wed, Oct 11, 2023 at 1:49 PM Peter Griffin <peter.griffin@linaro.org> 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.
> >
>
> A couple of thoughts in addition to what Guenter said.
>
> From the description it looks like this patch should be split into 3 patches:
>   1. Add "dbgack" feature
>   2. Add "windowed mode" feature
>   3. Enable gsX01 support

Sure I can split it up like that in v4 if that is preferable. The most important
part atm is SoC support, as the watchdog is left enabled by the bootloader
so without this, the system resets after ~ 1 minute.

> Also, it's not clear if those features are mandatory for gsX01 wdt to
> function properly, or optional?

The features aren't mandatory, the watchdog works fine with windowed
mode disabled and the "dback" feature just affects the watchdog reset
behaviour when an external debug agent is used.

> From the code it looks like both
> dbgack and windowed mode will only affect gsX01 variants (because of
> quirk flags), but maybe the commit message should be more clear about
> that.

Sure I will be more verbose in the commit messages when I split it out into
separate patches

>
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  drivers/watchdog/s3c2410_wdt.c | 127 ++++++++++++++++++++++++++++++---
> >  1 file changed, 116 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> > index 0b4bd883ff28..36c170047180 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)
>
> Maybe use BIT() macro here?

I didn't use the BIT macro for my changes, as the rest of the driver
isn't currently using the BIT macro.

>
> >
> >  #define S3C2410_WTCON_DIV16    (0 << 3)
> >  #define S3C2410_WTCON_DIV32    (1 << 3)
> > @@ -51,6 +53,7 @@
> >
> >  #define S3C2410_WATCHDOG_ATBOOT                (0)
> >  #define S3C2410_WATCHDOG_DEFAULT_TIME  (15)
> > +#define S3C2410_WINDOW_MULTIPLIER      2
> >
> >  #define EXYNOS5_RST_STAT_REG_OFFSET            0x0404
> >  #define EXYNOS5_WDT_DISABLE_REG_OFFSET         0x0408
> > @@ -67,6 +70,13 @@
> >  #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
>
> Please move those to the section above, where similar registers are
> described for other SoCs.

Will fix.

>
> > +
> >  /**
> >   * DOC: Quirk flags for different Samsung watchdog IP-cores
> >   *
> > @@ -106,6 +116,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)
>
> Please also document those two quirks in the kernel-doc comment above.
> Btw, the comment correctness can be checked like this:
>
>     $ scripts/kernel-doc -v -none drivers/watchdog/s3c2410_wdt.c
>
> or without "-none" option to see how the comment is parsed by kernel-doc.

Will do, and thanks for the kernel-doc hint!

>
> >
> >  /* These quirks require that we have a PMU register map */
> >  #define QUIRKS_HAVE_PMUREG \
> > @@ -263,6 +275,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 |
>
> Here and further: please stick to 80 characters per line when possible.

Will fix

>
> > +                 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 +338,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 +439,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);
> > @@ -410,7 +489,7 @@ static int s3c2410wdt_stop(struct watchdog_device *wdd)
> >
> >  static int s3c2410wdt_start(struct watchdog_device *wdd)
> >  {
> > -       unsigned long wtcon;
> > +       unsigned long wtcon, wtmincnt;
> >         struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd);
> >         unsigned long flags;
> >
> > @@ -432,6 +511,12 @@ static int s3c2410wdt_start(struct watchdog_device *wdd)
> >         dev_dbg(wdt->dev, "Starting watchdog: count=0x%08x, wtcon=%08lx\n",
> >                 wdt->count, wtcon);
> >
> > +       if (wdt->drv_data->quirks & QUIRK_HAS_WTMINCNT_REG) {
> > +               wtcon |= S3C2410_WTCON_WINDOWED_WD;
> > +               wtmincnt = wdt->count * S3C2410_WINDOW_MULTIPLIER;
> > +               writel(wtmincnt, wdt->reg_base + S3C2410_WTMINCNT);
> > +       }
> > +
> >         writel(wdt->count, wdt->reg_base + S3C2410_WTDAT);
> >         writel(wdt->count, wdt->reg_base + S3C2410_WTCNT);
> >         writel(wtcon, wdt->reg_base + S3C2410_WTCON);
> > @@ -447,7 +532,7 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
> >         unsigned long freq = s3c2410wdt_get_freq(wdt);
> >         unsigned int count;
> >         unsigned int divisor = 1;
> > -       unsigned long wtcon;
> > +       unsigned long wtcon, wtmincnt;
> >
> >         if (timeout < 1)
> >                 return -EINVAL;
> > @@ -478,6 +563,11 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
> >         count = DIV_ROUND_UP(count, divisor);
> >         wdt->count = count;
> >
> > +       if (wdt->drv_data->quirks & QUIRK_HAS_WTMINCNT_REG) {
> > +               wtmincnt = count * S3C2410_WINDOW_MULTIPLIER;
> > +               writel(wtmincnt, wdt->reg_base + S3C2410_WTMINCNT);
> > +       }
> > +
> >         /* update the pre-scaler */
> >         wtcon = readl(wdt->reg_base + S3C2410_WTCON);
> >         wtcon &= ~S3C2410_WTCON_PRESCALE_MASK;
> > @@ -496,14 +586,20 @@ static int s3c2410wdt_restart(struct watchdog_device *wdd, unsigned long action,
> >  {
> >         struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd);
> >         void __iomem *wdt_base = wdt->reg_base;
> > +       unsigned long wtcon;
> >
> >         /* disable watchdog, to be safe  */
> >         writel(0, wdt_base + S3C2410_WTCON);
> >
> >         /* put initial values into count and data */
> > +       if (wdt->drv_data->quirks & QUIRK_HAS_WTMINCNT_REG)
> > +               writel(0x100, wdt_base + S3C2410_WTMINCNT);
> >         writel(0x80, wdt_base + S3C2410_WTCNT);
> >         writel(0x80, wdt_base + S3C2410_WTDAT);
> >
> > +       if (wdt->drv_data->quirks & QUIRK_HAS_WTMINCNT_REG)
> > +               wtcon |= S3C2410_WTCON_WINDOWED_WD;
> > +
> >         /* set the watchdog to go and reset... */
> >         writel(S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV16 |
> >                 S3C2410_WTCON_RSTEN | S3C2410_WTCON_PRESCALE(0x20),
> > @@ -585,9 +681,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 */
>
> Please keep 80 characters per line.

Will fix

regards,

Peter
>
> >         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 +698,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 +803,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.
> > --
> > 2.42.0.655.g421f12c284-goog
> >
diff mbox series

Patch

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 0b4bd883ff28..36c170047180 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)
@@ -51,6 +53,7 @@ 
 
 #define S3C2410_WATCHDOG_ATBOOT		(0)
 #define S3C2410_WATCHDOG_DEFAULT_TIME	(15)
+#define S3C2410_WINDOW_MULTIPLIER	2
 
 #define EXYNOS5_RST_STAT_REG_OFFSET		0x0404
 #define EXYNOS5_WDT_DISABLE_REG_OFFSET		0x0408
@@ -67,6 +70,13 @@ 
 #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 +116,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 +275,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 +338,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 +439,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);
@@ -410,7 +489,7 @@  static int s3c2410wdt_stop(struct watchdog_device *wdd)
 
 static int s3c2410wdt_start(struct watchdog_device *wdd)
 {
-	unsigned long wtcon;
+	unsigned long wtcon, wtmincnt;
 	struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd);
 	unsigned long flags;
 
@@ -432,6 +511,12 @@  static int s3c2410wdt_start(struct watchdog_device *wdd)
 	dev_dbg(wdt->dev, "Starting watchdog: count=0x%08x, wtcon=%08lx\n",
 		wdt->count, wtcon);
 
+	if (wdt->drv_data->quirks & QUIRK_HAS_WTMINCNT_REG) {
+		wtcon |= S3C2410_WTCON_WINDOWED_WD;
+		wtmincnt = wdt->count * S3C2410_WINDOW_MULTIPLIER;
+		writel(wtmincnt, wdt->reg_base + S3C2410_WTMINCNT);
+	}
+
 	writel(wdt->count, wdt->reg_base + S3C2410_WTDAT);
 	writel(wdt->count, wdt->reg_base + S3C2410_WTCNT);
 	writel(wtcon, wdt->reg_base + S3C2410_WTCON);
@@ -447,7 +532,7 @@  static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
 	unsigned long freq = s3c2410wdt_get_freq(wdt);
 	unsigned int count;
 	unsigned int divisor = 1;
-	unsigned long wtcon;
+	unsigned long wtcon, wtmincnt;
 
 	if (timeout < 1)
 		return -EINVAL;
@@ -478,6 +563,11 @@  static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
 	count = DIV_ROUND_UP(count, divisor);
 	wdt->count = count;
 
+	if (wdt->drv_data->quirks & QUIRK_HAS_WTMINCNT_REG) {
+		wtmincnt = count * S3C2410_WINDOW_MULTIPLIER;
+		writel(wtmincnt, wdt->reg_base + S3C2410_WTMINCNT);
+	}
+
 	/* update the pre-scaler */
 	wtcon = readl(wdt->reg_base + S3C2410_WTCON);
 	wtcon &= ~S3C2410_WTCON_PRESCALE_MASK;
@@ -496,14 +586,20 @@  static int s3c2410wdt_restart(struct watchdog_device *wdd, unsigned long action,
 {
 	struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd);
 	void __iomem *wdt_base = wdt->reg_base;
+	unsigned long wtcon;
 
 	/* disable watchdog, to be safe  */
 	writel(0, wdt_base + S3C2410_WTCON);
 
 	/* put initial values into count and data */
+	if (wdt->drv_data->quirks & QUIRK_HAS_WTMINCNT_REG)
+		writel(0x100, wdt_base + S3C2410_WTMINCNT);
 	writel(0x80, wdt_base + S3C2410_WTCNT);
 	writel(0x80, wdt_base + S3C2410_WTDAT);
 
+	if (wdt->drv_data->quirks & QUIRK_HAS_WTMINCNT_REG)
+		wtcon |= S3C2410_WTCON_WINDOWED_WD;
+
 	/* set the watchdog to go and reset... */
 	writel(S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV16 |
 		S3C2410_WTCON_RSTEN | S3C2410_WTCON_PRESCALE(0x20),
@@ -585,9 +681,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 +698,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 +803,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.