mbox series

[v3,00/12] watchdog: s3c2410: Add Exynos850 support

Message ID 20211107202943.8859-1-semen.protsenko@linaro.org
Headers show
Series watchdog: s3c2410: Add Exynos850 support | expand

Message

Sam Protsenko Nov. 7, 2021, 8:29 p.m. UTC
Exynos850 WDT IP-core differs a bit from existing platforms:
  - Another set of PMU registers
  - Separate WDT instance for each CPU cluster, having different PMU
    registers/bits
  - Source clock is different from PCLK

Implement all missing features above and enable Exynos850 WDT support.
While at it, do a bit of cleaning up.

Changes in v3:
  - Renamed "samsung,index" property to more descriptive
    "samsung,cluster-index"
  - bindings: Disabled "samsung,cluster-index" property for SoCs other
    than Exynos850
  - Added quirks documentation
  - Removed has_src_clk field: clk framework can handle NULL clk; added
    s3c2410wdt_get_freq() function instead, to figure out which clock to
    use for getting the rate
  - Used pre-defined and completely set driver data for cluster0 and
    cluster1
  - Coding style changes
  - Added R-b tags

Changes in v2:
  - Added patch to fail probe if no valid timeout was found, as
    suggested by Guenter Roeck (03/12)
  - Extracted code for separating disable/mask functions into separate
    patch (06/12)
  - Added patch for inverting mask register value (07/12)
  - Extracted PMU cleanup code to separate patch, to ease the review and
    porting (09/12)
  - Added patch for removing not needed 'err' label in probe function
    (11/12)
  - Addressed all outstanding review comments on mailing list

Sam Protsenko (12):
  dt-bindings: watchdog: Require samsung,syscon-phandle for Exynos7
  dt-bindings: watchdog: Document Exynos850 watchdog bindings
  watchdog: s3c2410: Fail probe if can't find valid timeout
  watchdog: s3c2410: Let kernel kick watchdog
  watchdog: s3c2410: Make reset disable register optional
  watchdog: s3c2410: Extract disable and mask code into separate
    functions
  watchdog: s3c2410: Implement a way to invert mask reg value
  watchdog: s3c2410: Add support for WDT counter enable register
  watchdog: s3c2410: Cleanup PMU related code
  watchdog: s3c2410: Support separate source clock
  watchdog: s3c2410: Remove superfluous err label
  watchdog: s3c2410: Add Exynos850 support

 .../bindings/watchdog/samsung-wdt.yaml        |  48 ++-
 drivers/watchdog/s3c2410_wdt.c                | 322 +++++++++++++-----
 2 files changed, 287 insertions(+), 83 deletions(-)

Comments

Guenter Roeck Nov. 17, 2021, 1:33 p.m. UTC | #1
On Sun, Nov 07, 2021 at 10:29:32PM +0200, Sam Protsenko wrote:
> Exynos7 watchdog driver is clearly indicating that its dts node must
> define syscon phandle property. That was probably forgotten, so add it.
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> Fixes: 2b9366b66967 ("watchdog: s3c2410_wdt: Add support for Watchdog device on Exynos7")
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes in v3:
>   - Added R-b tag by Rob Herring
> 
> Changes in v2:
>   - Added R-b tag by Krzysztof Kozlowski
>   - Added "Fixes" tag
> 
>  Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
> index 76cb9586ee00..93cd77a6e92c 100644
> --- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
> @@ -39,8 +39,8 @@ properties:
>    samsung,syscon-phandle:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description:
> -      Phandle to the PMU system controller node (in case of Exynos5250
> -      and Exynos5420).
> +      Phandle to the PMU system controller node (in case of Exynos5250,
> +      Exynos5420 and Exynos7).
>  
>  required:
>    - compatible
> @@ -58,6 +58,7 @@ allOf:
>              enum:
>                - samsung,exynos5250-wdt
>                - samsung,exynos5420-wdt
> +              - samsung,exynos7-wdt
>      then:
>        required:
>          - samsung,syscon-phandle
> -- 
> 2.30.2
>
Guenter Roeck Nov. 17, 2021, 1:35 p.m. UTC | #2
On Sun, Nov 07, 2021 at 10:29:36PM +0200, Sam Protsenko wrote:
> On new Exynos chips (e.g. Exynos850 and Exynos9) the
> AUTOMATIC_WDT_RESET_DISABLE register was removed, and its value can be
> thought of as "always 0x0". Add correspondig quirk bit, so that the
> driver can omit accessing it if it's not present.
> 
> This commit doesn't bring any functional change to existing devices, but
> merely provides an infrastructure for upcoming chips support.
> 
> 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 v3:
>   - Aligned arguments with opening parentheses
>   - Added R-b tag by Krzysztof Kozlowski
> 
> Changes in v2:
>   - Used quirks instead of callbacks for all added PMU registers
>   - Used BIT() macro
>   - Extracted splitting the s3c2410wdt_mask_and_disable_reset() function
>     to separate patch
>   - Extracted cleanup code to separate patch to minimize changes and
>     ease the review and porting
> 
>  drivers/watchdog/s3c2410_wdt.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 0845c05034a1..2cc4923a98a5 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -59,10 +59,12 @@
>  #define QUIRK_HAS_PMU_CONFIG			(1 << 0)
>  #define QUIRK_HAS_RST_STAT			(1 << 1)
>  #define QUIRK_HAS_WTCLRINT_REG			(1 << 2)
> +#define QUIRK_HAS_PMU_AUTO_DISABLE		(1 << 3)
>  
>  /* These quirks require that we have a PMU register map */
>  #define QUIRKS_HAVE_PMUREG			(QUIRK_HAS_PMU_CONFIG | \
> -						 QUIRK_HAS_RST_STAT)
> +						 QUIRK_HAS_RST_STAT | \
> +						 QUIRK_HAS_PMU_AUTO_DISABLE)
>  
>  static bool nowayout	= WATCHDOG_NOWAYOUT;
>  static int tmr_margin;
> @@ -137,7 +139,7 @@ static const struct s3c2410_wdt_variant drv_data_exynos5250  = {
>  	.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_WTCLRINT_REG | QUIRK_HAS_PMU_AUTO_DISABLE,
>  };
>  
>  static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
> @@ -147,7 +149,7 @@ static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
>  	.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_WTCLRINT_REG | QUIRK_HAS_PMU_AUTO_DISABLE,
>  };
>  
>  static const struct s3c2410_wdt_variant drv_data_exynos7 = {
> @@ -157,7 +159,7 @@ static const struct s3c2410_wdt_variant drv_data_exynos7 = {
>  	.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_WTCLRINT_REG | QUIRK_HAS_PMU_AUTO_DISABLE,
>  };
>  
>  static const struct of_device_id s3c2410_wdt_match[] = {
> @@ -213,11 +215,13 @@ static int s3c2410wdt_mask_and_disable_reset(struct s3c2410_wdt *wdt, bool mask)
>  	if (mask)
>  		val = mask_val;
>  
> -	ret = regmap_update_bits(wdt->pmureg,
> -			wdt->drv_data->disable_reg,
> -			mask_val, val);
> -	if (ret < 0)
> -		goto error;
> +	if (wdt->drv_data->quirks & QUIRK_HAS_PMU_AUTO_DISABLE) {
> +		ret = regmap_update_bits(wdt->pmureg,
> +					 wdt->drv_data->disable_reg, mask_val,
> +					 val);
> +		if (ret < 0)
> +			goto error;
> +	}
>  
>  	ret = regmap_update_bits(wdt->pmureg,
>  			wdt->drv_data->mask_reset_reg,
> -- 
> 2.30.2
>
Guenter Roeck Nov. 17, 2021, 1:35 p.m. UTC | #3
On Sun, Nov 07, 2021 at 10:29:38PM +0200, Sam Protsenko wrote:
> On new Exynos chips (like Exynos850) the MASK_WDT_RESET_REQUEST register
> is replaced with CLUSTERx_NONCPU_INT_EN, and its mask bit value meaning
> was reversed: for new register the bit value "1" means "Interrupt
> enabled", while for MASK_WDT_RESET_REQUEST register "1" means "Mask the
> interrupt" (i.e. "Interrupt disabled").
> 
> Introduce "mask_reset_inv" boolean field in driver data structure; when
> that field is "true", mask register handling function will invert the
> value before setting it to the register.
> 
> This commit doesn't bring any functional change to existing devices, but
> merely provides an infrastructure for upcoming chips support.
> 
> 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 v3:
>   - Added R-b tag by Krzysztof Kozlowski
> 
> Changes in v2:
>   - (none): it's a new patch
> 
>  drivers/watchdog/s3c2410_wdt.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 4ac0a30e835e..2a61b6ea5602 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -92,6 +92,7 @@ MODULE_PARM_DESC(soft_noboot, "Watchdog action, set to 1 to ignore reboots, 0 to
>   * timer reset functionality.
>   * @mask_reset_reg: Offset in pmureg for the register that masks the watchdog
>   * timer reset functionality.
> + * @mask_reset_inv: If set, mask_reset_reg value will have inverted meaning.
>   * @mask_bit: Bit number for the watchdog timer in the disable register and the
>   * mask reset register.
>   * @rst_stat_reg: Offset in pmureg for the register that has the reset status.
> @@ -103,6 +104,7 @@ MODULE_PARM_DESC(soft_noboot, "Watchdog action, set to 1 to ignore reboots, 0 to
>  struct s3c2410_wdt_variant {
>  	int disable_reg;
>  	int mask_reset_reg;
> +	bool mask_reset_inv;
>  	int mask_bit;
>  	int rst_stat_reg;
>  	int rst_stat_bit;
> @@ -219,7 +221,8 @@ static int s3c2410wdt_disable_wdt_reset(struct s3c2410_wdt *wdt, bool mask)
>  static int s3c2410wdt_mask_wdt_reset(struct s3c2410_wdt *wdt, bool mask)
>  {
>  	const u32 mask_val = BIT(wdt->drv_data->mask_bit);
> -	const u32 val = mask ? mask_val : 0;
> +	const bool val_inv = wdt->drv_data->mask_reset_inv;
> +	const u32 val = (mask ^ val_inv) ? mask_val : 0;
>  	int ret;
>  
>  	ret = regmap_update_bits(wdt->pmureg, wdt->drv_data->mask_reset_reg,
> -- 
> 2.30.2
>
Guenter Roeck Nov. 17, 2021, 1:36 p.m. UTC | #4
On Sun, Nov 07, 2021 at 10:29:40PM +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 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)
> +
> +/**
> + * 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. 17, 2021, 1:37 p.m. UTC | #5
On Sun, Nov 07, 2021 at 10:29:42PM +0200, Sam Protsenko wrote:
> 'err' label in probe function is not really need, it just returns.
> Remove it and replace all 'goto' statements with actual returns in
> place.
> 
> No functional change here, just a cleanup patch.
> 
> 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 v3:
>   - Added R-b tag by Krzysztof Kozlowski
> 
> Changes in v2:
>   - (none): it's a new patch
> 
>  drivers/watchdog/s3c2410_wdt.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index f31bc765a8a5..96aa5d9c6ed4 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -627,22 +627,18 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
>  	wdt_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>  	if (wdt_irq == NULL) {
>  		dev_err(dev, "no irq resource specified\n");
> -		ret = -ENOENT;
> -		goto err;
> +		return -ENOENT;
>  	}
>  
>  	/* get the memory region for the watchdog timer */
>  	wdt->reg_base = devm_platform_ioremap_resource(pdev, 0);
> -	if (IS_ERR(wdt->reg_base)) {
> -		ret = PTR_ERR(wdt->reg_base);
> -		goto err;
> -	}
> +	if (IS_ERR(wdt->reg_base))
> +		return PTR_ERR(wdt->reg_base);
>  
>  	wdt->bus_clk = devm_clk_get(dev, "watchdog");
>  	if (IS_ERR(wdt->bus_clk)) {
>  		dev_err(dev, "failed to find bus clock\n");
> -		ret = PTR_ERR(wdt->bus_clk);
> -		goto err;
> +		return PTR_ERR(wdt->bus_clk);
>  	}
>  
>  	ret = clk_prepare_enable(wdt->bus_clk);
> @@ -757,7 +753,6 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
>   err_bus_clk:
>  	clk_disable_unprepare(wdt->bus_clk);
>  
> - err:
>  	return ret;
>  }
>  
> -- 
> 2.30.2
>