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

[email protected]
Newsgroups dev.linux.lists.mfd,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The 'clock-names' property uses an 'enum' for array items without enforcing uniqueness or order, allowing invalid configurations to pass schema validation.

Pre-existing issues:
- [High] The MFD driver mutates globally shared `static` resource arrays during `probe()`, creating a concurrency data race.
--

--- Patch [1]: [PATCH v3] dt-bindings: mfd: Convert TI TWL6040 to DT schema ---
commit 217f75902b2af0561cd52b8ef59fbfb6ab482cc0
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.

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

> diff --git a/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml b/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml
> new file mode 100644
> index 0000000000000..0d4e620d25a51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml
> @@ -0,0 +1,151 @@
[ ... ]
> +  clocks:
> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      enum:
> +        - clk32k
> +        - mclk

[Severity: Low]
Does this clock-names definition allow duplicate items or arbitrary ordering?

Using enum for array items without enforcing uniqueness or order might allow
invalid configurations like ["clk32k", "clk32k"] to pass schema validation.

[Severity: High]
This isn't a bug introduced by this patch, but looking at twl6040_probe()
in drivers/mfd/twl6040.c, it modifies a globally shared static array:

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);
    ...
}

Does this mutate a globally shared static resource array during probe?

If multiple instances of the TWL6040 device probe concurrently on different
I2C buses, could they simultaneously modify the start and end fields of the
global twl6040_codec_rsrc array, creating a concurrency data race?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.