Re: [PATCH v2] dt-bindings: mfd: Convert TI TWL6040 to DT schema

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Missing dependency enforcement for the 'vibra' node power supplies ('vddvibl-supply' and 'vddvibr-supply').

Pre-existing issues:
- [High] The twl6040 driver constructs its mfd_cell array dynamically at runtime and mutates a global static resource array, violating MFD static reference rules.
- [High] Hardcoded cell indexing via PLATFORM_DEVID_NONE creates sysfs naming collisions.
--

--- Patch [1]: [PATCH v2] dt-bindings: mfd: Convert TI TWL6040 to DT schema ---
commit 940b493f232639b743b8f6ddfb4feba28f32a8ab
Author: Eduard Bostina <[email protected]>

dt-bindings: mfd: Convert TI TWL6040 to DT schema

Convert the Texas Instruments TWL6040 audio codecs family bindings to DT
schema. During the conversion, ti,audpwron-gpio was renamed and made
optional along with gpio-controller. The vibra voltage properties were also
renamed to match what the driver actually reads.

Link: https://lore.kernel.org/all/[email protected]/

While reviewing this binding conversion, I noticed a couple of pre-existing
issues in the corresponding driver.

[Severity: High]
This isn't a bug introduced by this patch, but does the dynamic mutation of
mfd_cell resources in twl6040_probe() cause a data race?

drivers/mfd/twl6040.c:twl6040_probe() {
    ...
    twl6040_codec_rsrc[0].start = irq;
    twl6040_codec_rsrc[0].end = irq;
    cell->resources = twl6040_codec_rsrc;
    cell->num_resources = ARRAY_SIZE(twl6040_codec_rsrc);
    ...
}

If two TWL6040 devices probe concurrently, could they race to overwrite these
globally shared arrays, resulting in incorrect IRQ assignments for the child
devices?

[Severity: High]
This is a pre-existing issue, but does using the hardcoded value of -1
instead of PLATFORM_DEVID_AUTO prevent multiple TWL6040 chips from being
used in the same system?

drivers/mfd/twl6040.c:twl6040_probe() {
    ...
    ret = mfd_add_devices(&client->dev, -1, twl6040->cells, children,
                          NULL, 0, NULL);
    ...
}

If a system integrates more than one TWL6040 device, would the second
instance fail to probe its child devices due to identical sysfs device names?

> diff --git a/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml b/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml
> new file mode 100644
> index 0000000000000..011a1a58615a6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml
[ ... ]
> +  vibra:
> +    type: object
> +    additionalProperties: false
> +    properties:
> +      ti,vibldrv-res:

[Severity: Medium]
Should the vibra configuration object include a dependencies constraint to
ensure vddvibl-supply and vddvibr-supply are present?

The previous twl6040.txt binding explicitly marked vddvibl-supply and
vddvibr-supply as required when the vibra functionality is present.

Without this constraint, would an incomplete device tree missing these
regulators silently pass dt_binding_check validation but fail in
twl6040-vibra.c at runtime?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1