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

[PATCH v3] 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.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Link: https://lore.kernel.org/r/20260717-ti-fapll-clock-v1-1-1b684cb2543b@gmail.com
Signed-off-by: Bhargav Joshi <j.bhargav.u@gmail.com>
---
Changes in v3:
- Rebase to latest mainline rc.
- Collected tag.
- Link to v2: https://lore.kernel.org/r/20260722-ti-fapll-clock-v2-1-085054acf2dc@gmail.com

Changes in v2:
- Cleaned up binding descriptions by dropping the redundant common clock
  sentence and removing "phandle for" from the clock items
- Link to v1: https://lore.kernel.org/r/20260717-ti-fapll-clock-v1-1-1b684cb2543b@gmail.com
---
 .../devicetree/bindings/clock/ti/fapll.txt         | 31 ----------
 .../bindings/clock/ti/ti,dm816-fapll-clock.yaml    | 71 ++++++++++++++++++++++
 2 files changed, 71 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..423b7405e398
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ti/ti,dm816-fapll-clock.yaml
@@ -0,0 +1,71 @@
+# 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:
+  FAPLL is register-mapped 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: parent clock clk-ref
+      - description: 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: 9f0346dcbea363787186c94ef94dd01aaa215afa
change-id: 20260629-ti-fapll-clock-57a02c264e60

Best regards,
--  
Bhargav

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing parentheses around addition causes an under-allocation of the `clks` array, leading to a heap buffer overflow.
- [High] Missing bounds check on `output_instance` read from the device tree before using it as an array index.
- [High] Incorrect `clk_num` tracking breaks clock retrieval for sparse or failed outputs.
--

commit df6f3e72211e7dd6f2410f0f3ef1e36b9dec0028
Author: Bhargav Joshi <j.bhargav.u@gmail.com>
Subject: dt-bindings: clock: ti,dm816-fapll-clock: Convert to DT schema

This commit converts the Texas Instruments DM816 FAPLL clock binding from
plain text to YAML DT schema. It strictly enforces #clock-cells = <1> and
adds clock-indices and clock-output-names as required properties based on
driver requirements.

Link: https://lore.kernel.org/r/20260717-ti-fapll-clock-v1-1-1b684cb2543b@gmail.com

> 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..423b7405e398e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ti/ti,dm816-fapll-clock.yaml
> @@ -0,0 +1,71 @@
> +# 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#

[Severity: High]
This is a pre-existing issue, but I noticed a potential memory allocation
problem in the corresponding driver code when looking at the DT conversion.

In drivers/clk/ti/fapll.c:ti_fapll_setup(), there is an under-allocation
due to missing parentheses around the addition:

	fd->outputs.clks = kzalloc(sizeof(struct clk *) *
				   MAX_FAPLL_OUTPUTS + 1,
				   GFP_KERNEL);

The expression sizeof(struct clk *) * MAX_FAPLL_OUTPUTS + 1 evaluates to
57 bytes on a 64-bit architecture, but the code populates the array up to
index 7, which requires 64 bytes. Does this cause an out-of-bounds write
that corrupts adjacent heap memory when the driver probes?

[ ... ]

> +  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: High]
This isn't a bug introduced by this patch, but there is a lack of bounds
checking on the device tree values read for this property.

In drivers/clk/ti/fapll.c:ti_fapll_setup(), output_instance is read from the
clock-indices property and used immediately for memory offsets and array
indexing:

		if (of_property_read_u32_index(node, "clock-indices", i,
					       &output_instance))
			output_instance = i;

		freq = fd->base + (output_instance * 8);

and later:
		fd->outputs.clks[output_instance] = synth_clk;

If a malformed device tree provides an index larger than MAX_FAPLL_OUTPUTS,
could this cause an out-of-bounds heap read and write?

[Severity: High]
This is a pre-existing issue, but it could interact poorly with sparse
clock-indices from the device tree.

In drivers/clk/ti/fapll.c:ti_fapll_setup(), clk_num is incremented
sequentially for each successfully registered clock:

		fd->outputs.clks[output_instance] = synth_clk;
		fd->outputs.clk_num++;

Because of_clk_src_onecell_get() uses clk_num as the array size bound, if
the device tree defines sparse clock-indices or if a clock fails to register,
clk_num will be smaller than the highest registered output_instance index.
Will this cause valid clocks at higher indices to be rejected and become
inaccessible to consumers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-ti-fapll-clock-v3-1-b0ded340c62b@gmail.com?part=1