Re: [PATCH v9 1/3] dt-bindings: clock: imx95-blk-ctl: Allow child nodes

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The dt-binding fails to allow the `ranges` (and potentially `dma-ranges`) property, causing schema validation to reject valid device tree nodes that map child device registers.
- [Low] Child node `patternProperties` definition lacks the required `additionalProperties` or `unevaluatedProperties` directive.

Pre-existing issues:
- [Critical] The imx95_bc driver completely lacks a `.remove` callback and fails to clean up non-managed resources, leading to critical resource leaks, use-after-free bugs on unbind, and an APB clock leak.
--

commit 726604ca3bd356c1af7395b3121524ccf63624b9
Author: Guoniu Zhou <guoniu.zhou@oss.nxp.com>

dt-bindings: clock: imx95-blk-ctl: Allow child nodes

This commit adds #address-cells, #size-cells, and a patternProperties block
to the imx95-blk-ctl binding. This allows child device nodes to have their
own bindings, matching the driver's use of devm_of_platform_populate().

> Add support for child nodes in the imx95-blk-ctl binding to match
> the driver implementation which calls devm_of_platform_populate().

[Severity: Critical]
This is a pre-existing issue, but does the imx95_bc driver completely lack a
.remove callback?

While looking at the driver implementation mentioned here, it appears it
relies on non-managed APIs (clk_hw_register_mux, clk_hw_register_divider,
of_clk_add_hw_provider) to register clocks but lacks a way to unregister them
on unbind.

Since the clk_hw_data and associated strings are allocated via devm_kzalloc(),
they would be freed on unbind, potentially leaving the clock framework with
pointers to freed structures. Can this result in a use-after-free?

Additionally, when !bc->pdata is true, the probe function leaves bc->clk_apb
enabled without any cleanup mechanism. Does this cause a clock leak on unbind?

> diff --git a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> index 27403b4c52d62..33ad1c15cc9c8 100644
> --- a/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> +++ b/Documentation/devicetree/bindings/clock/nxp,imx95-blk-ctl.yaml
> @@ -39,6 +39,17 @@ properties:
>        ID in its "clocks" phandle cell. See
>        include/dt-bindings/clock/nxp,imx95-clock.h
>  
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1

[Severity: Medium]
Since this binding now allows child nodes with #address-cells and #size-cells,
does the parent node also need to allow the ranges property?

Without ranges: true in the properties block, device trees that correctly
define ranges to translate memory-mapped child registers into the CPU address
space might be rejected by dt_binding_check due to the top-level
additionalProperties: false restriction.

> +
> +patternProperties:
> +  "^.*@[0-9a-f]+$":
> +    type: object
> +    description: Child nodes with their own bindings

[Severity: Low]
Does this child node definition need an additionalProperties or
unevaluatedProperties directive?

Device tree schema guidelines typically require all type: object nodes to
explicitly declare one of these. Omitting it might trigger meta-schema
warnings during make dt_binding_check.

> +
>  required:
>    - compatible
>    - reg

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-csi_formatter-v9-0-ca3d8c334c39@oss.nxp.com?part=1