Re: [PATCH 1/2] dt-bindings: watchdog: Add Exynos8855 watchdog compatible string

Re: [PATCH 1/2] dt-bindings: watchdog: Add Exynos8855 watchdog compatible string

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dt-bindings: watchdog: missing comma in samsung,syscon-phandle description
--

commit 8354ecfd36bd009fe5b5d0f2ef81e3977d3ba5fd
Author: Alim Akhtar <alim.akhtar@samsung.com>

dt-bindings: watchdog: Add Exynos8855 watchdog compatible string

This commit adds the samsung,exynos8855-wdt compatible string to the
watchdog device tree binding documentation. It updates the property
descriptions and constraint blocks to indicate that the Exynos8855 requires
the cluster-index and syscon-phandle properties.

> diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
> index a32c478315779..517c0e1cd78bf 100644
> --- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
[ ... ]
> @@ -52,13 +53,13 @@ properties:
>      enum: [0, 1, 2]
>      description:
>        Index of CPU cluster on which watchdog is running (in case of Exynos850,
> -      Exynos990 or Google gs101).
> +      Exynos8855, Exynos990 or Google gs101).
>  
>    samsung,syscon-phandle:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description:
>        Phandle to the PMU system controller node (in case of Exynos5250,
> -      Exynos5420, Exynos5515, Exynos7, Exynos850, Exynos990 and gs101).
> +      Exynos5420, Exynos5515, Exynos7, Exynos850, Exynos8855 Exynos990 and gs101).

[Severity: Low]
This isn't a functional issue, but is there a missing comma between
Exynos8855 and Exynos990 in this updated description string?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927143100.3576195-1-alim.akhtar@samsung.com?part=1

Re: [PATCH 2/2] watchdog: s3c2410_wdt: Add support for Exynos8855 SoC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] watchdog: confusing reuse of CLUSTER0 macro for CLUSTER1
- [Low] watchdog: space-based alignment for EXYNOS8855_RST_STAT_REG_OFFSET
--

commit fec6f94918dfbe88ae7146d760500426231edbc3
Author: Alim Akhtar <alim.akhtar@samsung.com>

watchdog: s3c2410_wdt: Add support for Exynos8855 SoC

Watchdog IP on this SoC is similar to Exynos850/ExynosAutov{9/920}.
This does not have CNT_EN_WDT bit in PMU and offset of RST_STAT is
different then the previously support SoC.
Reuse the offset as much as from already supported SoC

> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 7053963fb0ac6..8ce4ca3e2aa81 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -58,6 +58,7 @@
>  #define S3C2410_WATCHDOG_DEFAULT_TIME	(15)
>  
>  #define EXYNOS5_RST_STAT_REG_OFFSET		0x0404
> +#define EXYNOS8855_RST_STAT_REG_OFFSET          0x10
                                          ^^^^^^^^^^
[Severity: Low]
Does this new macro intentionally use space-based alignment here?

The surrounding macros seem to use tabs for alignment. Using spaces deviates
from the standard tab-based formatting.

>  #define EXYNOS5_WDT_DISABLE_REG_OFFSET		0x0408
>  #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET	0x040c

[ ... ]

> @@ -281,6 +282,26 @@ static const struct s3c2410_wdt_variant drv_data_exynos850_cl1 = {
>  		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN,
>  };
>  
> +static const struct s3c2410_wdt_variant drv_data_exynos8855_cl0 = {
> +	.mask_reset_reg = EXYNOS850_CLUSTER0_NONCPU_INT_EN,
> +	.mask_bit = 2,
> +	.mask_reset_inv = true,
> +	.rst_stat_reg = EXYNOS8855_RST_STAT_REG_OFFSET,
> +	.rst_stat_bit = EXYNOS850_CLUSTER0_WDTRESET_BIT,
> +	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET |
> +		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_DBGACK_BIT,
> +};
> +
> +static const struct s3c2410_wdt_variant drv_data_exynos8855_cl1 = {
> +	.mask_reset_reg = EXYNOSAUTOV920_CLUSTER1_NONCPU_INT_EN,
> +	.mask_bit = 2,
> +	.mask_reset_inv = true,
> +	.rst_stat_reg = EXYNOS8855_RST_STAT_REG_OFFSET,
> +	.rst_stat_bit = EXYNOSAUTOV9_CLUSTER0_WDTRESET_BIT,
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Low]
Is it intentional to use a CLUSTER0 macro from an unrelated SoC to define the
reset status bit for a cluster 1 configuration?

Even if the underlying hardware bit value is correct and allows reuse, applying
a CLUSTER0 macro here is misleading and looks like a potential copy-paste typo.
Could this result in a functional logic error if the status bit for cluster 1
is actually supposed to be different?

> +	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET |
> +		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_DBGACK_BIT,
> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927143100.3576195-1-alim.akhtar@samsung.com?part=2

[PATCH 1/2] dt-bindings: watchdog: Add Exynos8855 watchdog compatible string

[Alim Akhtar]

Add the 'samsung,exynos8855-wdt' compatible string to watchdog DT
binding documentation. The Exynos8855 watchdog nodes use two
clocks (bus clock and source clock), require samsung,cluster-index
and samsung,syscon-phandle properties, similar to Exynos850 watchdog.

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
---
 Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
index a32c47831577..517c0e1cd78b 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
@@ -25,6 +25,7 @@ properties:
           - samsung,exynos5515-wdt
           - samsung,exynos7-wdt                   # for Exynos7
           - samsung,exynos850-wdt                 # for Exynos850
+          - samsung,exynos8855-wdt                # for Exynos8855
           - samsung,exynos990-wdt                 # for Exynos990
           - samsung,exynosautov9-wdt              # for Exynosautov9
           - samsung,exynosautov920-wdt            # for Exynosautov920
@@ -52,13 +53,13 @@ properties:
     enum: [0, 1, 2]
     description:
       Index of CPU cluster on which watchdog is running (in case of Exynos850,
-      Exynos990 or Google gs101).
+      Exynos8855, Exynos990 or Google gs101).
 
   samsung,syscon-phandle:
     $ref: /schemas/types.yaml#/definitions/phandle
     description:
       Phandle to the PMU system controller node (in case of Exynos5250,
-      Exynos5420, Exynos5515, Exynos7, Exynos850, Exynos990 and gs101).
+      Exynos5420, Exynos5515, Exynos7, Exynos850, Exynos8855 Exynos990 and gs101).
 
 required:
   - compatible
@@ -76,6 +77,7 @@ allOf:
             enum:
               - google,gs101-wdt
               - samsung,exynos850-wdt
+              - samsung,exynos8855-wdt
               - samsung,exynosautov9-wdt
               - samsung,exynosautov920-wdt
     then:

base-commit: 4c253ac4b29b8c6cc6fdef8f92d4facde62e63b9
-- 
2.34.1

[PATCH 2/2] watchdog: s3c2410_wdt: Add support for Exynos8855 SoC

[Alim Akhtar]

Watchdog IP on this SoC is similar to Exynos850/ExynosAutov{9/920}.
This does not have CNT_EN_WDT bit in PMU and offset of RST_STAT is
different then the previously support SoC.
Reuse the offset as much as from already supported SoC

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
---
 drivers/watchdog/s3c2410_wdt.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 7053963fb0ac..8ce4ca3e2aa8 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -58,6 +58,7 @@
 #define S3C2410_WATCHDOG_DEFAULT_TIME	(15)
 
 #define EXYNOS5_RST_STAT_REG_OFFSET		0x0404
+#define EXYNOS8855_RST_STAT_REG_OFFSET          0x10
 #define EXYNOS5_WDT_DISABLE_REG_OFFSET		0x0408
 #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET	0x040c
 #define EXYNOS850_CLUSTER0_NONCPU_OUT		0x1220
@@ -281,6 +282,26 @@ static const struct s3c2410_wdt_variant drv_data_exynos850_cl1 = {
 		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN,
 };
 
+static const struct s3c2410_wdt_variant drv_data_exynos8855_cl0 = {
+	.mask_reset_reg = EXYNOS850_CLUSTER0_NONCPU_INT_EN,
+	.mask_bit = 2,
+	.mask_reset_inv = true,
+	.rst_stat_reg = EXYNOS8855_RST_STAT_REG_OFFSET,
+	.rst_stat_bit = EXYNOS850_CLUSTER0_WDTRESET_BIT,
+	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET |
+		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_DBGACK_BIT,
+};
+
+static const struct s3c2410_wdt_variant drv_data_exynos8855_cl1 = {
+	.mask_reset_reg = EXYNOSAUTOV920_CLUSTER1_NONCPU_INT_EN,
+	.mask_bit = 2,
+	.mask_reset_inv = true,
+	.rst_stat_reg = EXYNOS8855_RST_STAT_REG_OFFSET,
+	.rst_stat_bit = EXYNOSAUTOV9_CLUSTER0_WDTRESET_BIT,
+	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET |
+		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_DBGACK_BIT,
+};
+
 static const struct s3c2410_wdt_variant drv_data_exynos990_cl0 = {
 	.mask_reset_reg = GS_CLUSTER0_NONCPU_INT_EN,
 	.mask_bit = 2,
@@ -400,6 +421,8 @@ static const struct of_device_id s3c2410_wdt_match[] = {
 	  .data = &drv_data_exynos7 },
 	{ .compatible = "samsung,exynos850-wdt",
 	  .data = &drv_data_exynos850_cl0 },
+	{ .compatible = "samsung,exynos8855-wdt",
+	  .data = &drv_data_exynos8855_cl0 },
 	{ .compatible = "samsung,exynos990-wdt",
 	  .data = &drv_data_exynos990_cl0 },
 	{ .compatible = "samsung,exynosautov9-wdt",
@@ -723,6 +746,7 @@ s3c2410_get_wdt_drv_data(struct platform_device *pdev, struct s3c2410_wdt *wdt)
 	/* Choose Exynos850/ExynosAutov9 driver data w.r.t. cluster index */
 	if (variant == &drv_data_exynos850_cl0 ||
 	    variant == &drv_data_exynosautov9_cl0 ||
+	    variant == &drv_data_exynos8855_cl0 ||
 	    variant == &drv_data_gs101_cl0 ||
 	    variant == &drv_data_exynosautov920_cl0 ||
 	    variant == &drv_data_exynos990_cl0) {
@@ -740,6 +764,8 @@ s3c2410_get_wdt_drv_data(struct platform_device *pdev, struct s3c2410_wdt *wdt)
 		case 1:
 			if (variant == &drv_data_exynos850_cl0)
 				variant = &drv_data_exynos850_cl1;
+			else if (variant == &drv_data_exynos8855_cl0)
+				variant = &drv_data_exynos8855_cl1;
 			else if (variant == &drv_data_exynosautov9_cl0)
 				variant = &drv_data_exynosautov9_cl1;
 			else if (variant == &drv_data_gs101_cl0)
-- 
2.34.1