Re: [PATCH v3 4/6] dt-bindings: pinctrl: samsung: Add exynos8855-wakeup-eint compatible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Mismatched interrupt initialization callback for the Exynos8855 CMGP pin controller instance.
- [Medium] Missing schema constraint for 'samsung,exynos8855-wakeup-eint' in conditional blocks.
--

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

dt-bindings: pinctrl: samsung: Add exynos8855-wakeup-eint compatible

Add a dedicated compatible for the exynos8855-wakeup-eint node, which
is compatible with Exynos7 implementation.

> diff --git a/Documentation/devicetree/bindings/pinctrl/samsung,pinctrl-wakeup-interrupt.yaml b/Documentation/devicetree/bindings/pinctrl/samsung,pinctrl-wakeup-interrupt.yaml
> index 2b88f25e80a6d..802911e23aff5 100644
> --- a/Documentation/devicetree/bindings/pinctrl/samsung,pinctrl-wakeup-interrupt.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/samsung,pinctrl-wakeup-interrupt.yaml
> @@ -41,6 +41,7 @@ properties:
>                - samsung,exynos7870-wakeup-eint
>                - samsung,exynos7885-wakeup-eint
>                - samsung,exynos850-wakeup-eint
> +              - samsung,exynos8855-wakeup-eint
>                - samsung,exynos8890-wakeup-eint
>                - samsung,exynos8895-wakeup-eint
>            - const: samsung,exynos7-wakeup-eint

[Severity: Medium]
Should samsung,exynos8855-wakeup-eint also be added to the allOf conditional
blocks to enforce the correct interrupt constraints?

Looking later in this file, this conditional block restricts the 'interrupts'
property to a single item for Exynos7 variants:

  - if:
      properties:
        compatible:
          oneOf:
            - enum:
[ ... ]
            - contains:
                enum:
                  - samsung,exynos5433-wakeup-eint
                  - samsung,exynos7870-wakeup-eint
[ ... ]

Is the new Exynos8855 variant missing from this list, which would allow
invalid hardware descriptions to silently pass validation?

[Severity: High]
While looking at the related driver code for Exynos8855, is there a mismatched
interrupt initialization callback for the CMGP instance?

The exynos8855_pin_banks1 definition configures the banks as wake-up type
interrupts (EINTW), which require the wake-up initialization routine:

drivers/pinctrl/samsung/pinctrl-exynos-arm64.c:exynos8855_pin_banks1[] {
	GS101_PIN_BANK_EINTW(1, 0x00,  "gpm0",  0x00, 0x00),
[ ... ]
};

But the CMGP instance exynos8855_pin_ctrl incorrectly assigns the GPIO init
callback instead of the required wake-up init callback:

drivers/pinctrl/samsung/pinctrl-exynos-arm64.c:exynos8855_pin_ctrl[] {
[ ... ]
	}, {
		/* pin-controller instance 1 CMGP data */
		.pin_banks	= exynos8855_pin_banks1,
		.nr_banks	= ARRAY_SIZE(exynos8855_pin_banks1),
		.eint_gpio_init = exynos_eint_gpio_init,
		.suspend	= gs101_pinctrl_suspend,
[ ... ]
}

Does this cause the wake-up interrupt banks to be ignored?

During the driver probe process, the GPIO init function explicitly skips banks
that are not of type EINT_TYPE_GPIO. Because the wake-up init callback
(.eint_wkup_init) is missing, it seems no initialization runs for these banks
at all, leaving the external wake-up interrupts completely dead and their IRQ
domains uncreated.

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