Re: [PATCH v4 1/5] dt-bindings: mfd: x-powers,axp152: Document AXP318W

[Jerome Brunet <jbrunet@baylibre.com>]

On ven. 17 juil. 2026 at 14:43, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The `x-powers,axp318w` compatible string was omitted from the conditional block that exempts similar PMIC generations from strictly requiring the `interrupts` property.
> - [Medium] The regex pattern for regulator nodes allows non-existent switched output nodes (`swout3` through `swout9`) to silently pass validation.
> --
>
> --- Patch [1]: [PATCH v4 1/5] dt-bindings: mfd: x-powers,axp152: Document AXP318W ---
> commit 08577b06937ca6f13b4a7f204e7f0abb6042c44d
> Author: Andre Przywara <andre.przywara@arm.com>
>
> dt-bindings: mfd: x-powers,axp152: Document AXP318W
>
> The X-Powers AXP318W is a PMIC used on some newer Allwinner devices.
> This patch adds the new compatible string, updates the PWM frequency 
> property list, adds input supply properties, and updates the pattern
> for matching switched outputs.
>
>> diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
>> index 45f015d63df16..95b3984fe0f96 100644
>> --- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
>> @@ -83,6 +83,7 @@ allOf:
>>            contains:
>>              enum:
>>                - x-powers,axp313a
>> +              - x-powers,axp318w
>>                - x-powers,axp323
>
> [Severity: Medium]
> Should x-powers,axp318w also be added to the earlier conditional block that
> exempts PMICs from requiring the interrupts property?
>
> There is another allOf block in this binding that forces interrupts to be 
> required for all devices except a few specific generations:
>
>   - if:
>       not:
>         properties:
>           compatible:
>             contains:
>               enum:
>                 - x-powers,axp15060
>                 - x-powers,axp305
>                 - x-powers,axp313a
>                 - x-powers,axp323
>     then:
>       required:
>         - interrupts
>
> Since the AXP318W shares constraints with siblings like axp313a and axp323,
> will omitting it from that exemption block cause valid device trees without 
> an interrupts property to erroneously fail validation?

No, irq should not be omitted

>
> [ ... ]
>
>> @@ -277,7 +303,7 @@ properties:
>>            Defines the work frequency of DC-DC in kHz.
>>  
>>      patternProperties:
>> -      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|cpusldo|drivevbus|dc5ldo|boost)$":
>> +      "^(([a-f])?ldo[0-9]|dcdc[0-9a-e]|ldo(_|-)io(0|1)|(dc1)?sw|swout[1-9]|rtc(_|-)ldo|cpusldo|drivevbus|dc5ldo|boost)$":
>
> [Severity: Medium]
> Could the swout[1-9] pattern allow invalid switched output nodes to silently 
> pass validation?
>
> The commit message notes that the PMIC features exactly two switched outputs,
> swout1 and swout2. Allowing swout[1-9] might let device trees with invalid
> nodes or typos like swout3 pass schema validation incorrectly. Would a tighter
> pattern like swout[1-2] be more appropriate here?

It is like that since v1 and has been acked.
It has been loose since forever so if it is an issue, it is defenitely
nothing introduced by this change. Making it strict would require a
different (crazy) regex for each chip.

-- 
Jerome