Re: [PATCH v3 1/2] dt-bindings: power: supply: ab8500: Add AB8505 charger

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dt-bindings: driver probe breakage due to 9 interrupt limit for AB8505
- [Medium] dt-bindings: missing schema constraint for autopower_cfg on AB8505
--

commit 56f862db85ac24639bf3e5234e563c4b067537a4
Author: Linus Walleij <linusw@kernel.org>

dt-bindings: power: supply: ab8500: Add AB8505 charger

This commit adds a unique AB8505 charger compatible string with the AB8500
compatible as its fallback. It requires 14 main and USB charger interrupts
on AB8500 and 9 USB charger interrupts on AB8505. It also notes that the
autopower_cfg describes the AB8500 facility, as the AB8505 register bit
is reserved.

> diff --git a/Documentation/devicetree/bindings/power/supply/stericsson,ab8500-charger.yaml b/Documentation/devicetree/bindings/power/supply/stericsson,ab8500-charger.yaml
> index 4f19744844e9c..dcffcecaea13b 100644
> --- a/Documentation/devicetree/bindings/power/supply/stericsson,ab8500-charger.yaml
> +++ b/Documentation/devicetree/bindings/power/supply/stericsson,ab8500-charger.yaml

[ ... ]

>  properties:
>    compatible:
> -    const: stericsson,ab8500-charger
> +    oneOf:
> +      - const: stericsson,ab8500-charger
> +      - items:
> +          - const: stericsson,ab8505-charger
> +          - const: stericsson,ab8500-charger

[ ... ]

> +allOf:
> +  - $ref: power-supply.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: stericsson,ab8505-charger
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 9
> +          maxItems: 9

[Severity: High]
Since stericsson,ab8505-charger uses stericsson,ab8500-charger as a fallback
above, does enforcing a strict limit of 9 interrupts here break driver probe?

Looking at drivers/power/supply/ab8500_charger.c:ab8500_charger_probe(), the
driver unconditionally requests all 14 interrupts:

	for (i = 0; i < ARRAY_SIZE(ab8500_charger_irq); i++) {
		irq = platform_get_irq_byname(pdev, ab8500_charger_irq[i].name);
		if (irq < 0)
			return irq;
	}

If the device tree strictly follows this schema and provides only 9
interrupts, platform_get_irq_byname() will return -ENXIO when requesting the
missing MAIN charger interrupts, aborting probe immediately.

[Severity: Medium]
Since the commit message notes that the corresponding AB8505 register bit
is reserved, should this conditional block explicitly disallow the
autopower_cfg property?

If a device tree includes autopower_cfg in an ab8505-charger node, the schema
will allow it. At runtime, the driver will act on it and invoke
ab8500_enable_disable_sw_fallback(), which unconditionally writes to the
reserved hardware register on AB8505 devices.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925-ab8500-charger-dt-v3-0-f80b8d295e2d@kernel.org?part=1