All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] dt-bindings: mfd: Convert TI TWL6040 to DT schema
@ 2026-09-05 21:52 Eduard Bostina
  2026-09-05 22:06 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Eduard Bostina @ 2026-09-05 21:52 UTC (permalink / raw)
  To: Conor Dooley, devicetree, Eduard Bostina, Krzysztof Kozlowski,
	Lee Jones, linux-kernel, mfd, Rob Herring

Convert the Texas Instruments TWL6040 audio codecs family bindings to DT
schema.

During the conversion, the following updates were made:
- Changed 'twl6040,audpwron-gpio' to 'ti,audpwron-gpio', which was
  misnamed in the old txt binding.
- Made 'gpio-controller', '#gpio-cells', and 'ti,audpwron-gpio' optional
  because modern platforms do not require them.
- Renamed the vibra 'vddvibl_uV'/'vddvibr_uV' properties to
  'ti,vddvibl-uV'/'ti,vddvibr-uV', the names actually read by the
  twl6040-vibra driver.
- Required 'vddvibl-supply' and 'vddvibr-supply' when the 'vibra' child
  node is present, since the twl6040-vibra driver unconditionally
  requests both.

Signed-off-by: Eduard Bostina <egbostina@gmail.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
Changes in v4:
- Add blank lines between the vibra child node's properties for
  readability, as requested in review. No functional change.

Link to v3: https://lore.kernel.org/all/20260817100207.2970303-1-egbostina@gmail.com/
Link to v2: https://lore.kernel.org/all/20260816092847.2522994-1-egbostina@gmail.com/
Link to v1: https://lore.kernel.org/all/20260815083451.2147129-1-egbostina@gmail.com/

 .../devicetree/bindings/mfd/ti,twl6040.yaml   | 156 ++++++++++++++++++
 .../devicetree/bindings/mfd/twl6040.txt       |  67 --------
 2 files changed, 156 insertions(+), 67 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/ti,twl6040.yaml
 delete mode 100644 Documentation/devicetree/bindings/mfd/twl6040.txt

diff --git a/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml b/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml
new file mode 100644
index 000000000000..5efcd79b527c
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml
@@ -0,0 +1,156 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/ti,twl6040.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments TWL6040 family audio codec
+
+maintainers:
+  - Eduard Bostina <egbostina@gmail.com>
+
+description:
+  The TWL6040s are 8-channel high quality low-power audio codecs providing
+  audio, vibra and GPO functionality on OMAP4+ platforms. They are connected
+  to the host processor via i2c for commands, McPDM for audio data and
+  commands.
+
+properties:
+  compatible:
+    enum:
+      - ti,twl6040
+      - ti,twl6041
+
+  reg:
+    const: 0x4b
+
+  interrupts:
+    maxItems: 1
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 1
+
+  "#clock-cells":
+    const: 0
+
+  ti,audpwron-gpio:
+    maxItems: 1
+    description: Power on GPIO line for the twl6040
+
+  vio-supply:
+    description: Regulator for the twl6040 VIO supply
+
+  v2v1-supply:
+    description: Regulator for the twl6040 V2V1 supply
+
+  enable-active-high:
+    type: boolean
+    description: To power on the twl6040 during boot.
+
+  clocks:
+    minItems: 1
+    maxItems: 2
+
+  clock-names:
+    minItems: 1
+    maxItems: 2
+    items:
+      enum:
+        - clk32k
+        - mclk
+
+  vddvibl-supply:
+    description: Regulator for the left vibra motor
+
+  vddvibr-supply:
+    description: Regulator for the right vibra motor
+
+  vibra:
+    type: object
+    additionalProperties: false
+    properties:
+      ti,vibldrv-res:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Resistance parameter for left driver
+
+      ti,vibrdrv-res:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Resistance parameter for right driver
+
+      ti,viblmotor-res:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Resistance parameter for left motor
+
+      ti,vibrmotor-res:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Resistance parameter for right motor
+
+      ti,vddvibl-uV:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: vddvibl default voltage if it needs to be changed
+
+      ti,vddvibr-uV:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: vddvibr default voltage if it needs to be changed
+
+    required:
+      - ti,vibldrv-res
+      - ti,vibrdrv-res
+      - ti,viblmotor-res
+      - ti,vibrmotor-res
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - "#clock-cells"
+  - vio-supply
+  - v2v1-supply
+
+allOf:
+  - if:
+      required:
+        - vibra
+    then:
+      required:
+        - vddvibl-supply
+        - vddvibr-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        audio-codec@4b {
+            compatible = "ti,twl6040";
+            reg = <0x4b>;
+            interrupts = <0 119 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-parent = <&gic>;
+            gpio-controller;
+            #gpio-cells = <1>;
+            #clock-cells = <0>;
+            ti,audpwron-gpio = <&gpio4 31 0>;
+
+            vio-supply = <&v1v8>;
+            v2v1-supply = <&v2v1>;
+            enable-active-high;
+
+            /* regulators for vibra motor */
+            vddvibl-supply = <&vbat>;
+            vddvibr-supply = <&vbat>;
+
+            vibra {
+                ti,vibldrv-res = <8>;
+                ti,vibrdrv-res = <3>;
+                ti,viblmotor-res = <10>;
+                ti,vibrmotor-res = <10>;
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/mfd/twl6040.txt b/Documentation/devicetree/bindings/mfd/twl6040.txt
deleted file mode 100644
index dfd8683ede0c..000000000000
--- a/Documentation/devicetree/bindings/mfd/twl6040.txt
+++ /dev/null
@@ -1,67 +0,0 @@
-Texas Instruments TWL6040 family
-
-The TWL6040s are 8-channel high quality low-power audio codecs providing audio,
-vibra and GPO functionality on OMAP4+ platforms.
-They are connected to the host processor via i2c for commands, McPDM for audio
-data and commands.
-
-Required properties:
-- compatible : "ti,twl6040" for twl6040, "ti,twl6041" for twl6041
-- reg: must be 0x4b for i2c address
-- interrupts: twl6040 has one interrupt line connecteded to the main SoC
-- gpio-controller:
-- #gpio-cells = <1>: twl6040 provides GPO lines.
-- #clock-cells = <0>; twl6040 is a provider of pdmclk which is used by McPDM
-- twl6040,audpwron-gpio: Power on GPIO line for the twl6040
-
-- vio-supply: Regulator for the twl6040 VIO supply
-- v2v1-supply: Regulator for the twl6040 V2V1 supply
-
-Optional properties, nodes:
-- enable-active-high: To power on the twl6040 during boot.
-- clocks: phandle to the clk32k and/or to mclk clock provider
-- clock-names: Must be "clk32k" for the 32K clock and "mclk" for the MCLK.
-
-Vibra functionality
-Required properties:
-- vddvibl-supply: Regulator for the left vibra motor
-- vddvibr-supply: Regulator for the right vibra motor
-- vibra { }: Configuration section for vibra parameters containing the following
-	     properties:
-- ti,vibldrv-res: Resistance parameter for left driver
-- ti,vibrdrv-res: Resistance parameter for right driver
-- ti,viblmotor-res: Resistance parameter for left motor
-- ti,viblmotor-res: Resistance parameter for right motor
-
-Optional properties within vibra { } section:
-- vddvibl_uV: If the vddvibl default voltage need to be changed
-- vddvibr_uV: If the vddvibr default voltage need to be changed
-
-Example:
-&i2c1 {
-	twl6040: twl@4b {
-		compatible = "ti,twl6040";
-
-		interrupts = <0 119 4>;
-		interrupt-parent = <&gic>;
-		twl6040,audpwron-gpio = <&gpio4 31 0>;
-
-		vio-supply = <&v1v8>;
-		v2v1-supply = <&v2v1>;
-		enable-active-high;
-
-		/* regulators for vibra motor */
-		vddvibl-supply = <&vbat>;
-		vddvibr-supply = <&vbat>;
-
-		vibra {
-			/* Vibra driver, motor resistance parameters */
-			ti,vibldrv-res = <8>;
-			ti,vibrdrv-res = <3>;
-			ti,viblmotor-res = <10>;
-			ti,vibrmotor-res = <10>;
-		};
-	};
-};
-
-/include/ "twl6040.dtsi"
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v4] dt-bindings: mfd: Convert TI TWL6040 to DT schema
  2026-09-05 21:52 [PATCH v4] dt-bindings: mfd: Convert TI TWL6040 to DT schema Eduard Bostina
@ 2026-09-05 22:06 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-09-05 22:06 UTC (permalink / raw)
  To: Eduard Bostina; +Cc: robh, conor+dt, devicetree, lee, mfd

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

Pre-existing issues:
- [High] IRQ domain destroyed before devm-managed threaded IRQs are freed and before child devices are removed, causing Use-After-Free.
- [High] The child driver twl6040-vibra allocates regulator devres actions on the parent device using a pointer allocated on the child, leading to a Use-After-Free.
--

commit 0404bd7a83f494396c387e5f75dbf3991d0ac035
Author: Eduard Bostina <egbostina@gmail.com>

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

Convert the Texas Instruments TWL6040 audio codecs family bindings to DT
schema. It updates naming and optional parameters for modern platforms while
maintaining hardware requirements for the vibra child node.

Link to v3: https://lore.kernel.org/all/20260817100207.2970303-1-egbostina@gmail.com/
Link to v2: https://lore.kernel.org/all/20260816092847.2522994-1-egbostina@gmail.com/
Link to v1: https://lore.kernel.org/all/20260815083451.2147129-1-egbostina@gmail.com/

> diff --git a/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml b/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml
> new file mode 100644
> index 0000000000000..5efcd79b527c3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml
[ ... ]

While reviewing this dt-bindings conversion, I noticed a few pre-existing
issues in the related C drivers for this hardware family.

In drivers/mfd/twl6040.c:

> static void twl6040_remove(struct i2c_client *client)
> {
> 	struct twl6040 *twl6040 = i2c_get_clientdata(client);
> 
> 	if (twl6040->power_count)
> 		twl6040_power(twl6040, 0);
> 
> 	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);
> 
> 	mfd_remove_devices(&client->dev);

[Severity: High]
This isn't a bug introduced by this patch, but does this sequence lead to a
use-after-free during teardown?

It looks like twl6040_remove() destroys the IRQ domain via
regmap_del_irq_chip() before the child devices are removed via
mfd_remove_devices(). When the children unbind and attempt to free their
IRQs, won't they access the already-destroyed IRQ domain and descriptors?

A similar sequence seems to occur in the twl6040_probe() error path:

drivers/mfd/twl6040.c:twl6040_probe() {
[ ... ]
> 	ret = mfd_add_devices(&client->dev, -1, twl6040->cells, children,
> 			      NULL, 0, NULL);
> 	if (ret)
> 		goto readyirq_err;
> 
> 	return 0;
> 
> readyirq_err:
> 	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);

[Severity: High]
This is a pre-existing issue, but does this error path similarly destroy the
IRQ domain before the devm-managed threaded IRQs are automatically freed by
devres? This appears to cause a use-after-free when probe fails.


In drivers/input/misc/twl6040-vibra.c:

> 	info->supplies[0].supply = "vddvibl";
> 	info->supplies[1].supply = "vddvibr";
> 	/*
> 	 * When booted with Device tree the regulators are attached to the
> 	 * parent device (twl6040 MFD core)
> 	 */
> 	error = devm_regulator_bulk_get(twl6040_core_dev,
> 					ARRAY_SIZE(info->supplies),
> 					info->supplies);

[Severity: High]
This is a pre-existing issue, but does this devres attachment cause a
use-after-free if the parent device unbinds after the child device?

The info structure is allocated via devm_kzalloc() on the child device. By
passing info->supplies to devm_regulator_bulk_get() for the parent device
(twl6040_core_dev), the parent's devres context stores this child-managed
pointer. If the child unbinds, info is freed, and when the parent later
unbinds, won't its devres call regulator_bulk_free() on the now-dangling
info->supplies pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905215234.454266-1-egbostina@gmail.com?part=1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-05 22:06 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-05 21:52 [PATCH v4] dt-bindings: mfd: Convert TI TWL6040 to DT schema Eduard Bostina
2026-09-05 22:06 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.