[PATCH] dt-bindings: clock: ti,dm816-fapll-clock: Convert to DT schema

[PATCH] dt-bindings: clock: ti,dm816-fapll-clock: Convert to DT schema

[Bhargav Joshi]

Convert the Texas Instruments DM816 FAPLL clock binding from plain text
to YAML DT schema.

The text documented #clock-cells to be set as 0, while the example and
existing dt and driver require it to be 1. The schema now strictly
enforces #clock-cells = <1>.

Properties clock-indices and clock-output-names are documented in the
binding and added as required as driver strictly requires it and existing
dt already use this property.

Signed-off-by: Bhargav Joshi <j.bhargav.u@gmail.com>
---
 .../devicetree/bindings/clock/ti/fapll.txt         | 31 ----------
 .../bindings/clock/ti/ti,dm816-fapll-clock.yaml    | 72 ++++++++++++++++++++++
 2 files changed, 72 insertions(+), 31 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/ti/fapll.txt b/Documentation/devicetree/bindings/clock/ti/fapll.txt
deleted file mode 100644
index 88986ef39ddd..000000000000
--- a/Documentation/devicetree/bindings/clock/ti/fapll.txt
+++ /dev/null
@@ -1,31 +0,0 @@
-Binding for Texas Instruments FAPLL clock.
-
-This binding uses the common clock binding[1]. It assumes a
-register-mapped FAPLL with usually two selectable input clocks
-(reference clock and bypass clock), and one or more child
-syntesizers.
-
-[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-Required properties:
-- compatible : shall be "ti,dm816-fapll-clock"
-- #clock-cells : from common clock binding; shall be set to 0.
-- clocks : link phandles of parent clocks (clk-ref and clk-bypass)
-- reg : address and length of the register set for controlling the FAPLL.
-
-Examples:
-	main_fapll: main_fapll {
-		#clock-cells = <1>;
-		compatible = "ti,dm816-fapll-clock";
-		reg = <0x400 0x40>;
-		clocks = <&sys_clkin_ck &sys_clkin_ck>;
-		clock-indices = <1>, <2>, <3>, <4>, <5>,
-				<6>, <7>;
-		clock-output-names = "main_pll_clk1",
-				     "main_pll_clk2",
-				     "main_pll_clk3",
-				     "main_pll_clk4",
-				     "main_pll_clk5",
-				     "main_pll_clk6",
-				     "main_pll_clk7";
-	};
diff --git a/Documentation/devicetree/bindings/clock/ti/ti,dm816-fapll-clock.yaml b/Documentation/devicetree/bindings/clock/ti/ti,dm816-fapll-clock.yaml
new file mode 100644
index 000000000000..9b6b824faa30
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ti/ti,dm816-fapll-clock.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/ti/ti,dm816-fapll-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments FAPLL clock
+
+maintainers:
+  - Tony Lindgren <tony@atomide.com>
+  - Tero Kristo <kristo@kernel.org>
+
+description:
+  This binding uses the common clock binding. It assumes a register-mapped
+  FAPLL with usually two selectable input clocks (reference clock and bypass
+  clock), and one or more child synthesizers.
+
+properties:
+  compatible:
+    const: ti,dm816-fapll-clock
+
+  "#clock-cells":
+    const: 1
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: phandle for parent clock clk-ref
+      - description: phandle for parent clock clk-bypass
+
+  clock-indices:
+    description:
+      Maps child synthesizers to physical indices 1-7. Index 0 is physically
+      reserved for the Master PLL.
+    minItems: 1
+    maxItems: 7
+    items:
+      minimum: 1
+      maximum: 7
+
+  clock-output-names:
+    description:
+      Names of the child synthesizer outputs. The length of this array must
+      match clock-indices.
+    minItems: 1
+    maxItems: 7
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - "#clock-cells"
+  - clock-indices
+  - clock-output-names
+
+additionalProperties: false
+
+examples:
+  - |
+    clock-controller@400 {
+        #clock-cells = <1>;
+        compatible = "ti,dm816-fapll-clock";
+        reg = <0x400 0x40>;
+        clocks = <&sys_clkin_ck &sys_clkin_ck>;
+        clock-indices = <1>, <2>, <3>, <4>, <5>, <6>, <7>;
+        clock-output-names = "main_pll_clk1", "main_pll_clk2", "main_pll_clk3",
+                             "main_pll_clk4", "main_pll_clk5", "main_pll_clk6",
+                             "main_pll_clk7";
+
+    };

---
base-commit: 0716f9b9338a86dd27796e00ed0fd560c653323a
change-id: 20260629-ti-fapll-clock-57a02c264e60

Best regards,
-- 
Bhargav

Re: [PATCH] dt-bindings: clock: ti,dm816-fapll-clock: Convert to DT schema

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Operator precedence bug causes an undersized array allocation leading to an out-of-bounds write
- [High] Incorrect array bounds tracking breaks clock lookup for sparse clock configurations
--

--- Patch [1]: [PATCH] dt-bindings: clock: ti,dm816-fapll-clock: Convert to DT schema ---
commit 87daba9f6927df76d909d0fe50168fefb82110f5
Author: Bhargav Joshi <j.bhargav.u@gmail.com>

dt-bindings: clock: ti,dm816-fapll-clock: Convert to DT schema

Convert the Texas Instruments DM816 FAPLL clock binding from plain text
to YAML DT schema. The schema now strictly enforces #clock-cells = <1>,
and adds clock-indices and clock-output-names as required properties.

> diff --git a/Documentation/devicetree/bindings/clock/ti/ti,dm816-fapll-clock.yaml b/Documentation/devicetree/bindings/clock/ti/ti,dm816-fapll-clock.yaml
> new file mode 100644
> index 0000000000000..9b6b824faa305
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ti/ti,dm816-fapll-clock.yaml
[ ... ]
> +  clock-indices:
> +    description:
> +      Maps child synthesizers to physical indices 1-7. Index 0 is physically
> +      reserved for the Master PLL.
> +    minItems: 1
> +    maxItems: 7
> +    items:
> +      minimum: 1
> +      maximum: 7

[Severity: Critical]
This is a pre-existing issue, but by officially documenting 7 as the maximum
index, this binding exposes an out-of-bounds write in the driver.

In the driver code:

drivers/clk/ti/fapll.c:ti_fapll_setup() {
    ...
        fd->outputs.clks = kzalloc(sizeof(struct clk *) *
                                   MAX_FAPLL_OUTPUTS + 1,
                                   GFP_KERNEL);
    ...
}

Due to C operator precedence, this evaluates to (8 * 7) + 1 = 57 bytes on
64-bit architectures, rather than the intended 8 * (7 + 1) = 64 bytes.

If a device tree uses a clock index of 7 (the maximum allowed, as demonstrated
in the DT bindings example), the driver writes to fd->outputs.clks[7]. This
pointer write covers bytes 56 through 63, overflowing the 57-byte allocation by
7 bytes, which can cause memory corruption and a KASAN splat.

Could we fix the operator precedence in the driver by adding parentheses around
MAX_FAPLL_OUTPUTS + 1?

[Severity: High]
This isn't a bug introduced by this patch, but there's a pre-existing issue in
how the driver handles the sparse indices allowed by this binding.

drivers/clk/ti/fapll.c:ti_fapll_setup() {
    ...
        fd->outputs.clks[output_instance] = synth_clk;
        fd->outputs.clk_num++;
    ...
}

If the device tree specifies sparse indices (e.g., <1> and <5>), clk_num is
incremented to 3 instead of tracking the maximum index + 1.

When of_clk_src_onecell_get() performs a lookup for index 5, it checks
if (idx >= clk_data->clk_num) (i.e., 5 >= 3), which incorrectly rejects the
valid lookup and returns -EINVAL.

Can the driver be updated to set clk_num to the maximum index + 1 instead of
just counting the number of registered clocks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-ti-fapll-clock-v1-1-1b684cb2543b@gmail.com?part=1