diff mbox series

[v4,09/12] watchdog: s3c2410: Cleanup PMU related code

Message ID 20211121165647.26706-10-semen.protsenko@linaro.org (mailing list archive)
State Not Applicable
Headers show
Series watchdog: s3c2410: Add Exynos850 support | expand

Commit Message

Sam Protsenko Nov. 21, 2021, 4:56 p.m. UTC
Now that PMU enablement code was extended for new Exynos SoCs, it
doesn't look very cohesive and consistent anymore. Do a bit of renaming,
grouping and style changes, to make it look good again. While at it, add
quirks documentation as well.

No functional change, just a refactoring commit.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
Changes in v4:
  - Added R-b tag by Guenter Roeck

Changes in v3:
  - Added quirks documentation
  - Added R-b tag by Krzysztof Kozlowski

Changes in v2:
  - (none): it's a new patch

 drivers/watchdog/s3c2410_wdt.c | 83 ++++++++++++++++++++++++----------
 1 file changed, 58 insertions(+), 25 deletions(-)

Comments

Guenter Roeck Nov. 23, 2021, 4:06 p.m. UTC | #1
On Sun, Nov 21, 2021 at 06:56:44PM +0200, Sam Protsenko wrote:
> Now that PMU enablement code was extended for new Exynos SoCs, it
> doesn't look very cohesive and consistent anymore. Do a bit of renaming,
> grouping and style changes, to make it look good again. While at it, add
> quirks documentation as well.
> 
> No functional change, just a refactoring commit.
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Changes in v4:
>   - Added R-b tag by Guenter Roeck
> 
> Changes in v3:
>   - Added quirks documentation
>   - Added R-b tag by Krzysztof Kozlowski
> 
> Changes in v2:
>   - (none): it's a new patch
> 
>  drivers/watchdog/s3c2410_wdt.c | 83 ++++++++++++++++++++++++----------
>  1 file changed, 58 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index ec341c876225..f211be8bf976 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -56,17 +56,51 @@
>  #define EXYNOS5_RST_STAT_REG_OFFSET		0x0404
>  #define EXYNOS5_WDT_DISABLE_REG_OFFSET		0x0408
>  #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET	0x040c
> -#define QUIRK_HAS_PMU_CONFIG			(1 << 0)
> -#define QUIRK_HAS_RST_STAT			(1 << 1)
> -#define QUIRK_HAS_WTCLRINT_REG			(1 << 2)
> +
> +/**

0-day complains:

drivers/watchdog/s3c2410_wdt.c:94: warning: expecting prototype for Quirk flags for different Samsung watchdog IP(). Prototype was for QUIRK_HAS_WTCLRINT_REG() instead

It doesn't seem to like the idea of documented bit masks. Not really sure
what to do here. I am inclined to ignore it, but I don't want to get flooded
by 0-day complaints until I retire either. Any idea ?

Guenter

> + * Quirk flags for different Samsung watchdog IP-cores.
> + *
> + * This driver supports multiple Samsung SoCs, each of which might have
> + * different set of registers and features supported. As watchdog block
> + * sometimes requires modifying PMU registers for proper functioning, register
> + * differences in both watchdog and PMU IP-cores should be accounted for. Quirk
> + * flags described below serve the purpose of telling the driver about mentioned
> + * SoC traits, and can be specified in driver data for each particular supported
> + * device.
> + *
> + * %QUIRK_HAS_WTCLRINT_REG: Watchdog block has WTCLRINT register. It's used to
> + * clear the interrupt once the interrupt service routine is complete. It's
> + * write-only, writing any values to this register clears the interrupt, but
> + * reading is not permitted.
> + *
> + * %QUIRK_HAS_PMU_MASK_RESET: PMU block has the register for disabling/enabling
> + * WDT reset request. On old SoCs it's usually called MASK_WDT_RESET_REQUEST,
> + * new SoCs have CLUSTERx_NONCPU_INT_EN register, which 'mask_bit' value is
> + * inverted compared to the former one.
> + *
> + * %QUIRK_HAS_PMU_RST_STAT: PMU block has RST_STAT (reset status) register,
> + * which contains bits indicating the reason for most recent CPU reset. If
> + * present, driver will use this register to check if previous reboot was due to
> + * watchdog timer reset.
> + *
> + * %QUIRK_HAS_PMU_AUTO_DISABLE: PMU block has AUTOMATIC_WDT_RESET_DISABLE
> + * register. If 'mask_bit' bit is set, PMU will disable WDT reset when
> + * corresponding processor is in reset state.
> + *
> + * %QUIRK_HAS_PMU_CNT_EN: PMU block has some register (e.g. CLUSTERx_NONCPU_OUT)
> + * with "watchdog counter enable" bit. That bit should be set to make watchdog
> + * counter running.
> + */
> +#define QUIRK_HAS_WTCLRINT_REG			(1 << 0)
> +#define QUIRK_HAS_PMU_MASK_RESET		(1 << 1)
> +#define QUIRK_HAS_PMU_RST_STAT			(1 << 2)
>  #define QUIRK_HAS_PMU_AUTO_DISABLE		(1 << 3)
>  #define QUIRK_HAS_PMU_CNT_EN			(1 << 4)
>  
>  /* These quirks require that we have a PMU register map */
> -#define QUIRKS_HAVE_PMUREG			(QUIRK_HAS_PMU_CONFIG | \
> -						 QUIRK_HAS_RST_STAT | \
> -						 QUIRK_HAS_PMU_AUTO_DISABLE | \
> -						 QUIRK_HAS_PMU_CNT_EN)
> +#define QUIRKS_HAVE_PMUREG \
> +	(QUIRK_HAS_PMU_MASK_RESET | QUIRK_HAS_PMU_RST_STAT | \
> +	 QUIRK_HAS_PMU_AUTO_DISABLE | QUIRK_HAS_PMU_CNT_EN)
>  
>  static bool nowayout	= WATCHDOG_NOWAYOUT;
>  static int tmr_margin;
> @@ -146,8 +180,8 @@ static const struct s3c2410_wdt_variant drv_data_exynos5250  = {
>  	.mask_bit = 20,
>  	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
>  	.rst_stat_bit = 20,
> -	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
> -		  | QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_AUTO_DISABLE,
> +	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
> +		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
>  };
>  
>  static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
> @@ -156,8 +190,8 @@ static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
>  	.mask_bit = 0,
>  	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
>  	.rst_stat_bit = 9,
> -	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
> -		  | QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_AUTO_DISABLE,
> +	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
> +		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
>  };
>  
>  static const struct s3c2410_wdt_variant drv_data_exynos7 = {
> @@ -166,8 +200,8 @@ static const struct s3c2410_wdt_variant drv_data_exynos7 = {
>  	.mask_bit = 23,
>  	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
>  	.rst_stat_bit = 23,	/* A57 WDTRESET */
> -	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
> -		  | QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_AUTO_DISABLE,
> +	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
> +		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
>  };
>  
>  static const struct of_device_id s3c2410_wdt_match[] = {
> @@ -253,24 +287,24 @@ static int s3c2410wdt_enable_counter(struct s3c2410_wdt *wdt, bool en)
>  	return ret;
>  }
>  
> -static int s3c2410wdt_mask_and_disable_reset(struct s3c2410_wdt *wdt, bool mask)
> +static int s3c2410wdt_enable(struct s3c2410_wdt *wdt, bool en)
>  {
>  	int ret;
>  
>  	if (wdt->drv_data->quirks & QUIRK_HAS_PMU_AUTO_DISABLE) {
> -		ret = s3c2410wdt_disable_wdt_reset(wdt, mask);
> +		ret = s3c2410wdt_disable_wdt_reset(wdt, !en);
>  		if (ret < 0)
>  			return ret;
>  	}
>  
> -	if (wdt->drv_data->quirks & QUIRK_HAS_PMU_CONFIG) {
> -		ret = s3c2410wdt_mask_wdt_reset(wdt, mask);
> +	if (wdt->drv_data->quirks & QUIRK_HAS_PMU_MASK_RESET) {
> +		ret = s3c2410wdt_mask_wdt_reset(wdt, !en);
>  		if (ret < 0)
>  			return ret;
>  	}
>  
>  	if (wdt->drv_data->quirks & QUIRK_HAS_PMU_CNT_EN) {
> -		ret = s3c2410wdt_enable_counter(wdt, !mask);
> +		ret = s3c2410wdt_enable_counter(wdt, en);
>  		if (ret < 0)
>  			return ret;
>  	}
> @@ -531,7 +565,7 @@ static inline unsigned int s3c2410wdt_get_bootstatus(struct s3c2410_wdt *wdt)
>  	unsigned int rst_stat;
>  	int ret;
>  
> -	if (!(wdt->drv_data->quirks & QUIRK_HAS_RST_STAT))
> +	if (!(wdt->drv_data->quirks & QUIRK_HAS_PMU_RST_STAT))
>  		return 0;
>  
>  	ret = regmap_read(wdt->pmureg, wdt->drv_data->rst_stat_reg, &rst_stat);
> @@ -672,7 +706,7 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_cpufreq;
>  
> -	ret = s3c2410wdt_mask_and_disable_reset(wdt, false);
> +	ret = s3c2410wdt_enable(wdt, true);
>  	if (ret < 0)
>  		goto err_unregister;
>  
> @@ -707,7 +741,7 @@ static int s3c2410wdt_remove(struct platform_device *dev)
>  	int ret;
>  	struct s3c2410_wdt *wdt = platform_get_drvdata(dev);
>  
> -	ret = s3c2410wdt_mask_and_disable_reset(wdt, true);
> +	ret = s3c2410wdt_enable(wdt, false);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -724,8 +758,7 @@ static void s3c2410wdt_shutdown(struct platform_device *dev)
>  {
>  	struct s3c2410_wdt *wdt = platform_get_drvdata(dev);
>  
> -	s3c2410wdt_mask_and_disable_reset(wdt, true);
> -
> +	s3c2410wdt_enable(wdt, false);
>  	s3c2410wdt_stop(&wdt->wdt_device);
>  }
>  
> @@ -740,7 +773,7 @@ static int s3c2410wdt_suspend(struct device *dev)
>  	wdt->wtcon_save = readl(wdt->reg_base + S3C2410_WTCON);
>  	wdt->wtdat_save = readl(wdt->reg_base + S3C2410_WTDAT);
>  
> -	ret = s3c2410wdt_mask_and_disable_reset(wdt, true);
> +	ret = s3c2410wdt_enable(wdt, false);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -760,7 +793,7 @@ static int s3c2410wdt_resume(struct device *dev)
>  	writel(wdt->wtdat_save, wdt->reg_base + S3C2410_WTCNT);/* Reset count */
>  	writel(wdt->wtcon_save, wdt->reg_base + S3C2410_WTCON);
>  
> -	ret = s3c2410wdt_mask_and_disable_reset(wdt, false);
> +	ret = s3c2410wdt_enable(wdt, true);
>  	if (ret < 0)
>  		return ret;
>  
> -- 
> 2.30.2
>
Sam Protsenko Nov. 23, 2021, 4:17 p.m. UTC | #2
On Tue, 23 Nov 2021 at 18:06, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Sun, Nov 21, 2021 at 06:56:44PM +0200, Sam Protsenko wrote:
> > Now that PMU enablement code was extended for new Exynos SoCs, it
> > doesn't look very cohesive and consistent anymore. Do a bit of renaming,
> > grouping and style changes, to make it look good again. While at it, add
> > quirks documentation as well.
> >
> > No functional change, just a refactoring commit.
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > Changes in v4:
> >   - Added R-b tag by Guenter Roeck
> >
> > Changes in v3:
> >   - Added quirks documentation
> >   - Added R-b tag by Krzysztof Kozlowski
> >
> > Changes in v2:
> >   - (none): it's a new patch
> >
> >  drivers/watchdog/s3c2410_wdt.c | 83 ++++++++++++++++++++++++----------
> >  1 file changed, 58 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> > index ec341c876225..f211be8bf976 100644
> > --- a/drivers/watchdog/s3c2410_wdt.c
> > +++ b/drivers/watchdog/s3c2410_wdt.c
> > @@ -56,17 +56,51 @@
> >  #define EXYNOS5_RST_STAT_REG_OFFSET          0x0404
> >  #define EXYNOS5_WDT_DISABLE_REG_OFFSET               0x0408
> >  #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET    0x040c
> > -#define QUIRK_HAS_PMU_CONFIG                 (1 << 0)
> > -#define QUIRK_HAS_RST_STAT                   (1 << 1)
> > -#define QUIRK_HAS_WTCLRINT_REG                       (1 << 2)
> > +
> > +/**
>
> 0-day complains:
>
> drivers/watchdog/s3c2410_wdt.c:94: warning: expecting prototype for Quirk flags for different Samsung watchdog IP(). Prototype was for QUIRK_HAS_WTCLRINT_REG() instead
>
> It doesn't seem to like the idea of documented bit masks. Not really sure
> what to do here. I am inclined to ignore it, but I don't want to get flooded
> by 0-day complaints until I retire either. Any idea ?
>

Seems like 0-day thinks this kernel-doc comment is for the first
define only, and thus the comment has wrong format, or something like
that. I tried to follow the same style as GFP_KERNEL and others are
documented.

Anyway, if you don't like 0-day complaints, can you please just
replace kernel-doc comment (/**) with regular comment (/*), by
removing one asterisk in the patch? Or I can re-send the patch
correspondingly -- then just let me know.

> Guenter
>
> > + * Quirk flags for different Samsung watchdog IP-cores.
> > + *
> > + * This driver supports multiple Samsung SoCs, each of which might have
> > + * different set of registers and features supported. As watchdog block
> > + * sometimes requires modifying PMU registers for proper functioning, register
> > + * differences in both watchdog and PMU IP-cores should be accounted for. Quirk
> > + * flags described below serve the purpose of telling the driver about mentioned
> > + * SoC traits, and can be specified in driver data for each particular supported
> > + * device.
> > + *
> > + * %QUIRK_HAS_WTCLRINT_REG: Watchdog block has WTCLRINT register. It's used to
> > + * clear the interrupt once the interrupt service routine is complete. It's
> > + * write-only, writing any values to this register clears the interrupt, but
> > + * reading is not permitted.
> > + *
> > + * %QUIRK_HAS_PMU_MASK_RESET: PMU block has the register for disabling/enabling
> > + * WDT reset request. On old SoCs it's usually called MASK_WDT_RESET_REQUEST,
> > + * new SoCs have CLUSTERx_NONCPU_INT_EN register, which 'mask_bit' value is
> > + * inverted compared to the former one.
> > + *
> > + * %QUIRK_HAS_PMU_RST_STAT: PMU block has RST_STAT (reset status) register,
> > + * which contains bits indicating the reason for most recent CPU reset. If
> > + * present, driver will use this register to check if previous reboot was due to
> > + * watchdog timer reset.
> > + *
> > + * %QUIRK_HAS_PMU_AUTO_DISABLE: PMU block has AUTOMATIC_WDT_RESET_DISABLE
> > + * register. If 'mask_bit' bit is set, PMU will disable WDT reset when
> > + * corresponding processor is in reset state.
> > + *
> > + * %QUIRK_HAS_PMU_CNT_EN: PMU block has some register (e.g. CLUSTERx_NONCPU_OUT)
> > + * with "watchdog counter enable" bit. That bit should be set to make watchdog
> > + * counter running.
> > + */
> > +#define QUIRK_HAS_WTCLRINT_REG                       (1 << 0)
> > +#define QUIRK_HAS_PMU_MASK_RESET             (1 << 1)
> > +#define QUIRK_HAS_PMU_RST_STAT                       (1 << 2)
> >  #define QUIRK_HAS_PMU_AUTO_DISABLE           (1 << 3)
> >  #define QUIRK_HAS_PMU_CNT_EN                 (1 << 4)
> >
> >  /* These quirks require that we have a PMU register map */
> > -#define QUIRKS_HAVE_PMUREG                   (QUIRK_HAS_PMU_CONFIG | \
> > -                                              QUIRK_HAS_RST_STAT | \
> > -                                              QUIRK_HAS_PMU_AUTO_DISABLE | \
> > -                                              QUIRK_HAS_PMU_CNT_EN)
> > +#define QUIRKS_HAVE_PMUREG \
> > +     (QUIRK_HAS_PMU_MASK_RESET | QUIRK_HAS_PMU_RST_STAT | \
> > +      QUIRK_HAS_PMU_AUTO_DISABLE | QUIRK_HAS_PMU_CNT_EN)
> >
> >  static bool nowayout = WATCHDOG_NOWAYOUT;
> >  static int tmr_margin;
> > @@ -146,8 +180,8 @@ static const struct s3c2410_wdt_variant drv_data_exynos5250  = {
> >       .mask_bit = 20,
> >       .rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
> >       .rst_stat_bit = 20,
> > -     .quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
> > -               | QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_AUTO_DISABLE,
> > +     .quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
> > +               QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
> >  };
> >
> >  static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
> > @@ -156,8 +190,8 @@ static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
> >       .mask_bit = 0,
> >       .rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
> >       .rst_stat_bit = 9,
> > -     .quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
> > -               | QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_AUTO_DISABLE,
> > +     .quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
> > +               QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
> >  };
> >
> >  static const struct s3c2410_wdt_variant drv_data_exynos7 = {
> > @@ -166,8 +200,8 @@ static const struct s3c2410_wdt_variant drv_data_exynos7 = {
> >       .mask_bit = 23,
> >       .rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
> >       .rst_stat_bit = 23,     /* A57 WDTRESET */
> > -     .quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
> > -               | QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_AUTO_DISABLE,
> > +     .quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
> > +               QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
> >  };
> >
> >  static const struct of_device_id s3c2410_wdt_match[] = {
> > @@ -253,24 +287,24 @@ static int s3c2410wdt_enable_counter(struct s3c2410_wdt *wdt, bool en)
> >       return ret;
> >  }
> >
> > -static int s3c2410wdt_mask_and_disable_reset(struct s3c2410_wdt *wdt, bool mask)
> > +static int s3c2410wdt_enable(struct s3c2410_wdt *wdt, bool en)
> >  {
> >       int ret;
> >
> >       if (wdt->drv_data->quirks & QUIRK_HAS_PMU_AUTO_DISABLE) {
> > -             ret = s3c2410wdt_disable_wdt_reset(wdt, mask);
> > +             ret = s3c2410wdt_disable_wdt_reset(wdt, !en);
> >               if (ret < 0)
> >                       return ret;
> >       }
> >
> > -     if (wdt->drv_data->quirks & QUIRK_HAS_PMU_CONFIG) {
> > -             ret = s3c2410wdt_mask_wdt_reset(wdt, mask);
> > +     if (wdt->drv_data->quirks & QUIRK_HAS_PMU_MASK_RESET) {
> > +             ret = s3c2410wdt_mask_wdt_reset(wdt, !en);
> >               if (ret < 0)
> >                       return ret;
> >       }
> >
> >       if (wdt->drv_data->quirks & QUIRK_HAS_PMU_CNT_EN) {
> > -             ret = s3c2410wdt_enable_counter(wdt, !mask);
> > +             ret = s3c2410wdt_enable_counter(wdt, en);
> >               if (ret < 0)
> >                       return ret;
> >       }
> > @@ -531,7 +565,7 @@ static inline unsigned int s3c2410wdt_get_bootstatus(struct s3c2410_wdt *wdt)
> >       unsigned int rst_stat;
> >       int ret;
> >
> > -     if (!(wdt->drv_data->quirks & QUIRK_HAS_RST_STAT))
> > +     if (!(wdt->drv_data->quirks & QUIRK_HAS_PMU_RST_STAT))
> >               return 0;
> >
> >       ret = regmap_read(wdt->pmureg, wdt->drv_data->rst_stat_reg, &rst_stat);
> > @@ -672,7 +706,7 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
> >       if (ret)
> >               goto err_cpufreq;
> >
> > -     ret = s3c2410wdt_mask_and_disable_reset(wdt, false);
> > +     ret = s3c2410wdt_enable(wdt, true);
> >       if (ret < 0)
> >               goto err_unregister;
> >
> > @@ -707,7 +741,7 @@ static int s3c2410wdt_remove(struct platform_device *dev)
> >       int ret;
> >       struct s3c2410_wdt *wdt = platform_get_drvdata(dev);
> >
> > -     ret = s3c2410wdt_mask_and_disable_reset(wdt, true);
> > +     ret = s3c2410wdt_enable(wdt, false);
> >       if (ret < 0)
> >               return ret;
> >
> > @@ -724,8 +758,7 @@ static void s3c2410wdt_shutdown(struct platform_device *dev)
> >  {
> >       struct s3c2410_wdt *wdt = platform_get_drvdata(dev);
> >
> > -     s3c2410wdt_mask_and_disable_reset(wdt, true);
> > -
> > +     s3c2410wdt_enable(wdt, false);
> >       s3c2410wdt_stop(&wdt->wdt_device);
> >  }
> >
> > @@ -740,7 +773,7 @@ static int s3c2410wdt_suspend(struct device *dev)
> >       wdt->wtcon_save = readl(wdt->reg_base + S3C2410_WTCON);
> >       wdt->wtdat_save = readl(wdt->reg_base + S3C2410_WTDAT);
> >
> > -     ret = s3c2410wdt_mask_and_disable_reset(wdt, true);
> > +     ret = s3c2410wdt_enable(wdt, false);
> >       if (ret < 0)
> >               return ret;
> >
> > @@ -760,7 +793,7 @@ static int s3c2410wdt_resume(struct device *dev)
> >       writel(wdt->wtdat_save, wdt->reg_base + S3C2410_WTCNT);/* Reset count */
> >       writel(wdt->wtcon_save, wdt->reg_base + S3C2410_WTCON);
> >
> > -     ret = s3c2410wdt_mask_and_disable_reset(wdt, false);
> > +     ret = s3c2410wdt_enable(wdt, true);
> >       if (ret < 0)
> >               return ret;
> >
> > --
> > 2.30.2
> >
Guenter Roeck Nov. 23, 2021, 10:33 p.m. UTC | #3
On 11/23/21 8:17 AM, Sam Protsenko wrote:
> On Tue, 23 Nov 2021 at 18:06, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On Sun, Nov 21, 2021 at 06:56:44PM +0200, Sam Protsenko wrote:
>>> Now that PMU enablement code was extended for new Exynos SoCs, it
>>> doesn't look very cohesive and consistent anymore. Do a bit of renaming,
>>> grouping and style changes, to make it look good again. While at it, add
>>> quirks documentation as well.
>>>
>>> No functional change, just a refactoring commit.
>>>
>>> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>> ---
>>> Changes in v4:
>>>    - Added R-b tag by Guenter Roeck
>>>
>>> Changes in v3:
>>>    - Added quirks documentation
>>>    - Added R-b tag by Krzysztof Kozlowski
>>>
>>> Changes in v2:
>>>    - (none): it's a new patch
>>>
>>>   drivers/watchdog/s3c2410_wdt.c | 83 ++++++++++++++++++++++++----------
>>>   1 file changed, 58 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
>>> index ec341c876225..f211be8bf976 100644
>>> --- a/drivers/watchdog/s3c2410_wdt.c
>>> +++ b/drivers/watchdog/s3c2410_wdt.c
>>> @@ -56,17 +56,51 @@
>>>   #define EXYNOS5_RST_STAT_REG_OFFSET          0x0404
>>>   #define EXYNOS5_WDT_DISABLE_REG_OFFSET               0x0408
>>>   #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET    0x040c
>>> -#define QUIRK_HAS_PMU_CONFIG                 (1 << 0)
>>> -#define QUIRK_HAS_RST_STAT                   (1 << 1)
>>> -#define QUIRK_HAS_WTCLRINT_REG                       (1 << 2)
>>> +
>>> +/**
>>
>> 0-day complains:
>>
>> drivers/watchdog/s3c2410_wdt.c:94: warning: expecting prototype for Quirk flags for different Samsung watchdog IP(). Prototype was for QUIRK_HAS_WTCLRINT_REG() instead
>>
>> It doesn't seem to like the idea of documented bit masks. Not really sure
>> what to do here. I am inclined to ignore it, but I don't want to get flooded
>> by 0-day complaints until I retire either. Any idea ?
>>
> 
> Seems like 0-day thinks this kernel-doc comment is for the first
> define only, and thus the comment has wrong format, or something like
> that. I tried to follow the same style as GFP_KERNEL and others are
> documented.
> 
> Anyway, if you don't like 0-day complaints, can you please just
> replace kernel-doc comment (/**) with regular comment (/*), by
> removing one asterisk in the patch? Or I can re-send the patch
> correspondingly -- then just let me know.
> 

Oh, never mind. Let's just hope that 0-day stops complaining at some point.

Guenter
Sam Protsenko Nov. 23, 2021, 11:30 p.m. UTC | #4
On Wed, 24 Nov 2021 at 00:33, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 11/23/21 8:17 AM, Sam Protsenko wrote:
> > On Tue, 23 Nov 2021 at 18:06, Guenter Roeck <linux@roeck-us.net> wrote:
> >>
> >> On Sun, Nov 21, 2021 at 06:56:44PM +0200, Sam Protsenko wrote:
> >>> Now that PMU enablement code was extended for new Exynos SoCs, it
> >>> doesn't look very cohesive and consistent anymore. Do a bit of renaming,
> >>> grouping and style changes, to make it look good again. While at it, add
> >>> quirks documentation as well.
> >>>
> >>> No functional change, just a refactoring commit.
> >>>
> >>> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> >>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> >>> ---
> >>> Changes in v4:
> >>>    - Added R-b tag by Guenter Roeck
> >>>
> >>> Changes in v3:
> >>>    - Added quirks documentation
> >>>    - Added R-b tag by Krzysztof Kozlowski
> >>>
> >>> Changes in v2:
> >>>    - (none): it's a new patch
> >>>
> >>>   drivers/watchdog/s3c2410_wdt.c | 83 ++++++++++++++++++++++++----------
> >>>   1 file changed, 58 insertions(+), 25 deletions(-)
> >>>
> >>> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> >>> index ec341c876225..f211be8bf976 100644
> >>> --- a/drivers/watchdog/s3c2410_wdt.c
> >>> +++ b/drivers/watchdog/s3c2410_wdt.c
> >>> @@ -56,17 +56,51 @@
> >>>   #define EXYNOS5_RST_STAT_REG_OFFSET          0x0404
> >>>   #define EXYNOS5_WDT_DISABLE_REG_OFFSET               0x0408
> >>>   #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET    0x040c
> >>> -#define QUIRK_HAS_PMU_CONFIG                 (1 << 0)
> >>> -#define QUIRK_HAS_RST_STAT                   (1 << 1)
> >>> -#define QUIRK_HAS_WTCLRINT_REG                       (1 << 2)
> >>> +
> >>> +/**
> >>
> >> 0-day complains:
> >>
> >> drivers/watchdog/s3c2410_wdt.c:94: warning: expecting prototype for Quirk flags for different Samsung watchdog IP(). Prototype was for QUIRK_HAS_WTCLRINT_REG() instead
> >>
> >> It doesn't seem to like the idea of documented bit masks. Not really sure
> >> what to do here. I am inclined to ignore it, but I don't want to get flooded
> >> by 0-day complaints until I retire either. Any idea ?
> >>
> >
> > Seems like 0-day thinks this kernel-doc comment is for the first
> > define only, and thus the comment has wrong format, or something like
> > that. I tried to follow the same style as GFP_KERNEL and others are
> > documented.
> >
> > Anyway, if you don't like 0-day complaints, can you please just
> > replace kernel-doc comment (/**) with regular comment (/*), by
> > removing one asterisk in the patch? Or I can re-send the patch
> > correspondingly -- then just let me know.
> >
>
> Oh, never mind. Let's just hope that 0-day stops complaining at some point.
>

Just sent v5 for this patch, fixing that 0-day warning properly. Found
info about it here: [1]. So to check that warning, apparently it's
enough to run "make W=n" build, or dry-run for kernel-doc script like
this:

    $ scripts/kernel-doc -v -none drivers/watchdog/s3c2410_wdt.c

Anyway, please take v4 series + v5 for this patch. Hope that'll be all
for 0-day swearing :)

[1] https://github.com/torvalds/linux/blob/master/Documentation/doc-guide/kernel-doc.rst

> Guenter
Sam Protsenko Nov. 27, 2021, 10:52 p.m. UTC | #5
On Wed, 24 Nov 2021 at 01:30, Sam Protsenko <semen.protsenko@linaro.org> wrote:
>
> On Wed, 24 Nov 2021 at 00:33, Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > On 11/23/21 8:17 AM, Sam Protsenko wrote:
> > > On Tue, 23 Nov 2021 at 18:06, Guenter Roeck <linux@roeck-us.net> wrote:
> > >>
> > >> On Sun, Nov 21, 2021 at 06:56:44PM +0200, Sam Protsenko wrote:
> > >>> Now that PMU enablement code was extended for new Exynos SoCs, it
> > >>> doesn't look very cohesive and consistent anymore. Do a bit of renaming,
> > >>> grouping and style changes, to make it look good again. While at it, add
> > >>> quirks documentation as well.
> > >>>
> > >>> No functional change, just a refactoring commit.
> > >>>
> > >>> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> > >>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > >>> ---
> > >>> Changes in v4:
> > >>>    - Added R-b tag by Guenter Roeck
> > >>>
> > >>> Changes in v3:
> > >>>    - Added quirks documentation
> > >>>    - Added R-b tag by Krzysztof Kozlowski
> > >>>
> > >>> Changes in v2:
> > >>>    - (none): it's a new patch
> > >>>
> > >>>   drivers/watchdog/s3c2410_wdt.c | 83 ++++++++++++++++++++++++----------
> > >>>   1 file changed, 58 insertions(+), 25 deletions(-)
> > >>>
> > >>> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> > >>> index ec341c876225..f211be8bf976 100644
> > >>> --- a/drivers/watchdog/s3c2410_wdt.c
> > >>> +++ b/drivers/watchdog/s3c2410_wdt.c
> > >>> @@ -56,17 +56,51 @@
> > >>>   #define EXYNOS5_RST_STAT_REG_OFFSET          0x0404
> > >>>   #define EXYNOS5_WDT_DISABLE_REG_OFFSET               0x0408
> > >>>   #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET    0x040c
> > >>> -#define QUIRK_HAS_PMU_CONFIG                 (1 << 0)
> > >>> -#define QUIRK_HAS_RST_STAT                   (1 << 1)
> > >>> -#define QUIRK_HAS_WTCLRINT_REG                       (1 << 2)
> > >>> +
> > >>> +/**
> > >>
> > >> 0-day complains:
> > >>
> > >> drivers/watchdog/s3c2410_wdt.c:94: warning: expecting prototype for Quirk flags for different Samsung watchdog IP(). Prototype was for QUIRK_HAS_WTCLRINT_REG() instead
> > >>
> > >> It doesn't seem to like the idea of documented bit masks. Not really sure
> > >> what to do here. I am inclined to ignore it, but I don't want to get flooded
> > >> by 0-day complaints until I retire either. Any idea ?
> > >>
> > >
> > > Seems like 0-day thinks this kernel-doc comment is for the first
> > > define only, and thus the comment has wrong format, or something like
> > > that. I tried to follow the same style as GFP_KERNEL and others are
> > > documented.
> > >
> > > Anyway, if you don't like 0-day complaints, can you please just
> > > replace kernel-doc comment (/**) with regular comment (/*), by
> > > removing one asterisk in the patch? Or I can re-send the patch
> > > correspondingly -- then just let me know.
> > >
> >
> > Oh, never mind. Let's just hope that 0-day stops complaining at some point.
> >
>
> Just sent v5 for this patch, fixing that 0-day warning properly. Found
> info about it here: [1]. So to check that warning, apparently it's
> enough to run "make W=n" build, or dry-run for kernel-doc script like
> this:
>
>     $ scripts/kernel-doc -v -none drivers/watchdog/s3c2410_wdt.c
>
> Anyway, please take v4 series + v5 for this patch. Hope that'll be all
> for 0-day swearing :)
>
> [1] https://github.com/torvalds/linux/blob/master/Documentation/doc-guide/kernel-doc.rst
>

Hi Guenter,

Can you please take this patch:

    [PATCH v4 12/12] watchdog: s3c2410: Add Exynos850 support

and replace "Cleanup PMU related code" patch you already applied with this one:

    [PATCH v5] watchdog: s3c2410: Cleanup PMU related code

I can see you already took most of WDT patches I sent, but those two
seem to be missing.

Also, I can't see my patches (which are already present in your
"watchdog-next" branch) in linux-next/master. Is that expected, or I'm
missing something?

Thanks!

> > Guenter
Guenter Roeck Nov. 28, 2021, 5:56 p.m. UTC | #6
On 11/27/21 2:52 PM, Sam Protsenko wrote:
> On Wed, 24 Nov 2021 at 01:30, Sam Protsenko <semen.protsenko@linaro.org> wrote:
>>
>> On Wed, 24 Nov 2021 at 00:33, Guenter Roeck <linux@roeck-us.net> wrote:
>>>
>>> On 11/23/21 8:17 AM, Sam Protsenko wrote:
>>>> On Tue, 23 Nov 2021 at 18:06, Guenter Roeck <linux@roeck-us.net> wrote:
>>>>>
>>>>> On Sun, Nov 21, 2021 at 06:56:44PM +0200, Sam Protsenko wrote:
>>>>>> Now that PMU enablement code was extended for new Exynos SoCs, it
>>>>>> doesn't look very cohesive and consistent anymore. Do a bit of renaming,
>>>>>> grouping and style changes, to make it look good again. While at it, add
>>>>>> quirks documentation as well.
>>>>>>
>>>>>> No functional change, just a refactoring commit.
>>>>>>
>>>>>> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
>>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>>>>>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>>>>> ---
>>>>>> Changes in v4:
>>>>>>     - Added R-b tag by Guenter Roeck
>>>>>>
>>>>>> Changes in v3:
>>>>>>     - Added quirks documentation
>>>>>>     - Added R-b tag by Krzysztof Kozlowski
>>>>>>
>>>>>> Changes in v2:
>>>>>>     - (none): it's a new patch
>>>>>>
>>>>>>    drivers/watchdog/s3c2410_wdt.c | 83 ++++++++++++++++++++++++----------
>>>>>>    1 file changed, 58 insertions(+), 25 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
>>>>>> index ec341c876225..f211be8bf976 100644
>>>>>> --- a/drivers/watchdog/s3c2410_wdt.c
>>>>>> +++ b/drivers/watchdog/s3c2410_wdt.c
>>>>>> @@ -56,17 +56,51 @@
>>>>>>    #define EXYNOS5_RST_STAT_REG_OFFSET          0x0404
>>>>>>    #define EXYNOS5_WDT_DISABLE_REG_OFFSET               0x0408
>>>>>>    #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET    0x040c
>>>>>> -#define QUIRK_HAS_PMU_CONFIG                 (1 << 0)
>>>>>> -#define QUIRK_HAS_RST_STAT                   (1 << 1)
>>>>>> -#define QUIRK_HAS_WTCLRINT_REG                       (1 << 2)
>>>>>> +
>>>>>> +/**
>>>>>
>>>>> 0-day complains:
>>>>>
>>>>> drivers/watchdog/s3c2410_wdt.c:94: warning: expecting prototype for Quirk flags for different Samsung watchdog IP(). Prototype was for QUIRK_HAS_WTCLRINT_REG() instead
>>>>>
>>>>> It doesn't seem to like the idea of documented bit masks. Not really sure
>>>>> what to do here. I am inclined to ignore it, but I don't want to get flooded
>>>>> by 0-day complaints until I retire either. Any idea ?
>>>>>
>>>>
>>>> Seems like 0-day thinks this kernel-doc comment is for the first
>>>> define only, and thus the comment has wrong format, or something like
>>>> that. I tried to follow the same style as GFP_KERNEL and others are
>>>> documented.
>>>>
>>>> Anyway, if you don't like 0-day complaints, can you please just
>>>> replace kernel-doc comment (/**) with regular comment (/*), by
>>>> removing one asterisk in the patch? Or I can re-send the patch
>>>> correspondingly -- then just let me know.
>>>>
>>>
>>> Oh, never mind. Let's just hope that 0-day stops complaining at some point.
>>>
>>
>> Just sent v5 for this patch, fixing that 0-day warning properly. Found
>> info about it here: [1]. So to check that warning, apparently it's
>> enough to run "make W=n" build, or dry-run for kernel-doc script like
>> this:
>>
>>      $ scripts/kernel-doc -v -none drivers/watchdog/s3c2410_wdt.c
>>
>> Anyway, please take v4 series + v5 for this patch. Hope that'll be all
>> for 0-day swearing :)
>>
>> [1] https://github.com/torvalds/linux/blob/master/Documentation/doc-guide/kernel-doc.rst
>>
> 
> Hi Guenter,
> 
> Can you please take this patch:
> 
>      [PATCH v4 12/12] watchdog: s3c2410: Add Exynos850 support
> 
> and replace "Cleanup PMU related code" patch you already applied with this one:
> 
>      [PATCH v5] watchdog: s3c2410: Cleanup PMU related code
> 
> I can see you already took most of WDT patches I sent, but those two
> seem to be missing.
> 

Upstream work is always "time permitting". Done now.

> Also, I can't see my patches (which are already present in your
> "watchdog-next" branch) in linux-next/master. Is that expected, or I'm
> missing something?
> 
My watchdog-next branch is for 0-day coverage only. It is not made
available in linux-next. linux-next pulls watchdog related changes
from the official watchdog repository at
git://www.linux-watchdog.org/linux-watchdog-next.git#master

Guenter
Sam Protsenko Nov. 28, 2021, 7:18 p.m. UTC | #7
On Sun, 28 Nov 2021 at 19:56, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 11/27/21 2:52 PM, Sam Protsenko wrote:
> > On Wed, 24 Nov 2021 at 01:30, Sam Protsenko <semen.protsenko@linaro.org> wrote:
> >>
> >> On Wed, 24 Nov 2021 at 00:33, Guenter Roeck <linux@roeck-us.net> wrote:
> >>>
> >>> On 11/23/21 8:17 AM, Sam Protsenko wrote:
> >>>> On Tue, 23 Nov 2021 at 18:06, Guenter Roeck <linux@roeck-us.net> wrote:
> >>>>>
> >>>>> On Sun, Nov 21, 2021 at 06:56:44PM +0200, Sam Protsenko wrote:
> >>>>>> Now that PMU enablement code was extended for new Exynos SoCs, it
> >>>>>> doesn't look very cohesive and consistent anymore. Do a bit of renaming,
> >>>>>> grouping and style changes, to make it look good again. While at it, add
> >>>>>> quirks documentation as well.
> >>>>>>
> >>>>>> No functional change, just a refactoring commit.
> >>>>>>
> >>>>>> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> >>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> >>>>>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> >>>>>> ---
> >>>>>> Changes in v4:
> >>>>>>     - Added R-b tag by Guenter Roeck
> >>>>>>
> >>>>>> Changes in v3:
> >>>>>>     - Added quirks documentation
> >>>>>>     - Added R-b tag by Krzysztof Kozlowski
> >>>>>>
> >>>>>> Changes in v2:
> >>>>>>     - (none): it's a new patch
> >>>>>>
> >>>>>>    drivers/watchdog/s3c2410_wdt.c | 83 ++++++++++++++++++++++++----------
> >>>>>>    1 file changed, 58 insertions(+), 25 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> >>>>>> index ec341c876225..f211be8bf976 100644
> >>>>>> --- a/drivers/watchdog/s3c2410_wdt.c
> >>>>>> +++ b/drivers/watchdog/s3c2410_wdt.c
> >>>>>> @@ -56,17 +56,51 @@
> >>>>>>    #define EXYNOS5_RST_STAT_REG_OFFSET          0x0404
> >>>>>>    #define EXYNOS5_WDT_DISABLE_REG_OFFSET               0x0408
> >>>>>>    #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET    0x040c
> >>>>>> -#define QUIRK_HAS_PMU_CONFIG                 (1 << 0)
> >>>>>> -#define QUIRK_HAS_RST_STAT                   (1 << 1)
> >>>>>> -#define QUIRK_HAS_WTCLRINT_REG                       (1 << 2)
> >>>>>> +
> >>>>>> +/**
> >>>>>
> >>>>> 0-day complains:
> >>>>>
> >>>>> drivers/watchdog/s3c2410_wdt.c:94: warning: expecting prototype for Quirk flags for different Samsung watchdog IP(). Prototype was for QUIRK_HAS_WTCLRINT_REG() instead
> >>>>>
> >>>>> It doesn't seem to like the idea of documented bit masks. Not really sure
> >>>>> what to do here. I am inclined to ignore it, but I don't want to get flooded
> >>>>> by 0-day complaints until I retire either. Any idea ?
> >>>>>
> >>>>
> >>>> Seems like 0-day thinks this kernel-doc comment is for the first
> >>>> define only, and thus the comment has wrong format, or something like
> >>>> that. I tried to follow the same style as GFP_KERNEL and others are
> >>>> documented.
> >>>>
> >>>> Anyway, if you don't like 0-day complaints, can you please just
> >>>> replace kernel-doc comment (/**) with regular comment (/*), by
> >>>> removing one asterisk in the patch? Or I can re-send the patch
> >>>> correspondingly -- then just let me know.
> >>>>
> >>>
> >>> Oh, never mind. Let's just hope that 0-day stops complaining at some point.
> >>>
> >>
> >> Just sent v5 for this patch, fixing that 0-day warning properly. Found
> >> info about it here: [1]. So to check that warning, apparently it's
> >> enough to run "make W=n" build, or dry-run for kernel-doc script like
> >> this:
> >>
> >>      $ scripts/kernel-doc -v -none drivers/watchdog/s3c2410_wdt.c
> >>
> >> Anyway, please take v4 series + v5 for this patch. Hope that'll be all
> >> for 0-day swearing :)
> >>
> >> [1] https://github.com/torvalds/linux/blob/master/Documentation/doc-guide/kernel-doc.rst
> >>
> >
> > Hi Guenter,
> >
> > Can you please take this patch:
> >
> >      [PATCH v4 12/12] watchdog: s3c2410: Add Exynos850 support
> >
> > and replace "Cleanup PMU related code" patch you already applied with this one:
> >
> >      [PATCH v5] watchdog: s3c2410: Cleanup PMU related code
> >
> > I can see you already took most of WDT patches I sent, but those two
> > seem to be missing.
> >
>
> Upstream work is always "time permitting". Done now.
>

Thank you, Guenter!

> > Also, I can't see my patches (which are already present in your
> > "watchdog-next" branch) in linux-next/master. Is that expected, or I'm
> > missing something?
> >
> My watchdog-next branch is for 0-day coverage only. It is not made
> available in linux-next. linux-next pulls watchdog related changes
> from the official watchdog repository at
> git://www.linux-watchdog.org/linux-watchdog-next.git#master
>
> Guenter
diff mbox series

Patch

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index ec341c876225..f211be8bf976 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -56,17 +56,51 @@ 
 #define EXYNOS5_RST_STAT_REG_OFFSET		0x0404
 #define EXYNOS5_WDT_DISABLE_REG_OFFSET		0x0408
 #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET	0x040c
-#define QUIRK_HAS_PMU_CONFIG			(1 << 0)
-#define QUIRK_HAS_RST_STAT			(1 << 1)
-#define QUIRK_HAS_WTCLRINT_REG			(1 << 2)
+
+/**
+ * Quirk flags for different Samsung watchdog IP-cores.
+ *
+ * This driver supports multiple Samsung SoCs, each of which might have
+ * different set of registers and features supported. As watchdog block
+ * sometimes requires modifying PMU registers for proper functioning, register
+ * differences in both watchdog and PMU IP-cores should be accounted for. Quirk
+ * flags described below serve the purpose of telling the driver about mentioned
+ * SoC traits, and can be specified in driver data for each particular supported
+ * device.
+ *
+ * %QUIRK_HAS_WTCLRINT_REG: Watchdog block has WTCLRINT register. It's used to
+ * clear the interrupt once the interrupt service routine is complete. It's
+ * write-only, writing any values to this register clears the interrupt, but
+ * reading is not permitted.
+ *
+ * %QUIRK_HAS_PMU_MASK_RESET: PMU block has the register for disabling/enabling
+ * WDT reset request. On old SoCs it's usually called MASK_WDT_RESET_REQUEST,
+ * new SoCs have CLUSTERx_NONCPU_INT_EN register, which 'mask_bit' value is
+ * inverted compared to the former one.
+ *
+ * %QUIRK_HAS_PMU_RST_STAT: PMU block has RST_STAT (reset status) register,
+ * which contains bits indicating the reason for most recent CPU reset. If
+ * present, driver will use this register to check if previous reboot was due to
+ * watchdog timer reset.
+ *
+ * %QUIRK_HAS_PMU_AUTO_DISABLE: PMU block has AUTOMATIC_WDT_RESET_DISABLE
+ * register. If 'mask_bit' bit is set, PMU will disable WDT reset when
+ * corresponding processor is in reset state.
+ *
+ * %QUIRK_HAS_PMU_CNT_EN: PMU block has some register (e.g. CLUSTERx_NONCPU_OUT)
+ * with "watchdog counter enable" bit. That bit should be set to make watchdog
+ * counter running.
+ */
+#define QUIRK_HAS_WTCLRINT_REG			(1 << 0)
+#define QUIRK_HAS_PMU_MASK_RESET		(1 << 1)
+#define QUIRK_HAS_PMU_RST_STAT			(1 << 2)
 #define QUIRK_HAS_PMU_AUTO_DISABLE		(1 << 3)
 #define QUIRK_HAS_PMU_CNT_EN			(1 << 4)
 
 /* These quirks require that we have a PMU register map */
-#define QUIRKS_HAVE_PMUREG			(QUIRK_HAS_PMU_CONFIG | \
-						 QUIRK_HAS_RST_STAT | \
-						 QUIRK_HAS_PMU_AUTO_DISABLE | \
-						 QUIRK_HAS_PMU_CNT_EN)
+#define QUIRKS_HAVE_PMUREG \
+	(QUIRK_HAS_PMU_MASK_RESET | QUIRK_HAS_PMU_RST_STAT | \
+	 QUIRK_HAS_PMU_AUTO_DISABLE | QUIRK_HAS_PMU_CNT_EN)
 
 static bool nowayout	= WATCHDOG_NOWAYOUT;
 static int tmr_margin;
@@ -146,8 +180,8 @@  static const struct s3c2410_wdt_variant drv_data_exynos5250  = {
 	.mask_bit = 20,
 	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
 	.rst_stat_bit = 20,
-	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
-		  | QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_AUTO_DISABLE,
+	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
+		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
 };
 
 static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
@@ -156,8 +190,8 @@  static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
 	.mask_bit = 0,
 	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
 	.rst_stat_bit = 9,
-	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
-		  | QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_AUTO_DISABLE,
+	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
+		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
 };
 
 static const struct s3c2410_wdt_variant drv_data_exynos7 = {
@@ -166,8 +200,8 @@  static const struct s3c2410_wdt_variant drv_data_exynos7 = {
 	.mask_bit = 23,
 	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
 	.rst_stat_bit = 23,	/* A57 WDTRESET */
-	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
-		  | QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_AUTO_DISABLE,
+	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
+		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
 };
 
 static const struct of_device_id s3c2410_wdt_match[] = {
@@ -253,24 +287,24 @@  static int s3c2410wdt_enable_counter(struct s3c2410_wdt *wdt, bool en)
 	return ret;
 }
 
-static int s3c2410wdt_mask_and_disable_reset(struct s3c2410_wdt *wdt, bool mask)
+static int s3c2410wdt_enable(struct s3c2410_wdt *wdt, bool en)
 {
 	int ret;
 
 	if (wdt->drv_data->quirks & QUIRK_HAS_PMU_AUTO_DISABLE) {
-		ret = s3c2410wdt_disable_wdt_reset(wdt, mask);
+		ret = s3c2410wdt_disable_wdt_reset(wdt, !en);
 		if (ret < 0)
 			return ret;
 	}
 
-	if (wdt->drv_data->quirks & QUIRK_HAS_PMU_CONFIG) {
-		ret = s3c2410wdt_mask_wdt_reset(wdt, mask);
+	if (wdt->drv_data->quirks & QUIRK_HAS_PMU_MASK_RESET) {
+		ret = s3c2410wdt_mask_wdt_reset(wdt, !en);
 		if (ret < 0)
 			return ret;
 	}
 
 	if (wdt->drv_data->quirks & QUIRK_HAS_PMU_CNT_EN) {
-		ret = s3c2410wdt_enable_counter(wdt, !mask);
+		ret = s3c2410wdt_enable_counter(wdt, en);
 		if (ret < 0)
 			return ret;
 	}
@@ -531,7 +565,7 @@  static inline unsigned int s3c2410wdt_get_bootstatus(struct s3c2410_wdt *wdt)
 	unsigned int rst_stat;
 	int ret;
 
-	if (!(wdt->drv_data->quirks & QUIRK_HAS_RST_STAT))
+	if (!(wdt->drv_data->quirks & QUIRK_HAS_PMU_RST_STAT))
 		return 0;
 
 	ret = regmap_read(wdt->pmureg, wdt->drv_data->rst_stat_reg, &rst_stat);
@@ -672,7 +706,7 @@  static int s3c2410wdt_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_cpufreq;
 
-	ret = s3c2410wdt_mask_and_disable_reset(wdt, false);
+	ret = s3c2410wdt_enable(wdt, true);
 	if (ret < 0)
 		goto err_unregister;
 
@@ -707,7 +741,7 @@  static int s3c2410wdt_remove(struct platform_device *dev)
 	int ret;
 	struct s3c2410_wdt *wdt = platform_get_drvdata(dev);
 
-	ret = s3c2410wdt_mask_and_disable_reset(wdt, true);
+	ret = s3c2410wdt_enable(wdt, false);
 	if (ret < 0)
 		return ret;
 
@@ -724,8 +758,7 @@  static void s3c2410wdt_shutdown(struct platform_device *dev)
 {
 	struct s3c2410_wdt *wdt = platform_get_drvdata(dev);
 
-	s3c2410wdt_mask_and_disable_reset(wdt, true);
-
+	s3c2410wdt_enable(wdt, false);
 	s3c2410wdt_stop(&wdt->wdt_device);
 }
 
@@ -740,7 +773,7 @@  static int s3c2410wdt_suspend(struct device *dev)
 	wdt->wtcon_save = readl(wdt->reg_base + S3C2410_WTCON);
 	wdt->wtdat_save = readl(wdt->reg_base + S3C2410_WTDAT);
 
-	ret = s3c2410wdt_mask_and_disable_reset(wdt, true);
+	ret = s3c2410wdt_enable(wdt, false);
 	if (ret < 0)
 		return ret;
 
@@ -760,7 +793,7 @@  static int s3c2410wdt_resume(struct device *dev)
 	writel(wdt->wtdat_save, wdt->reg_base + S3C2410_WTCNT);/* Reset count */
 	writel(wdt->wtcon_save, wdt->reg_base + S3C2410_WTCON);
 
-	ret = s3c2410wdt_mask_and_disable_reset(wdt, false);
+	ret = s3c2410wdt_enable(wdt, true);
 	if (ret < 0)
 		return ret;