Re: [PATCH v4 3/7] dt-bindings: clock: qcom: Add support for CAMCC for Eliza

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The schema incorrectly enforces the `#reset-cells` requirement for the newly added `qcom,eliza-cambistmclkcc` variant, which is not a reset provider.
--

commit a9d0f97a6a58cdea9469da3167d31dd8b62fa02f
Author: Taniya Das <taniya.das@oss.qualcomm.com>

dt-bindings: clock: qcom: Add support for CAMCC for Eliza

This commit adds compatible strings for the Eliza camera and camera BIST
clock controllers to the existing Milos camcc binding. It also introduces
the corresponding dt-binding headers.

> diff --git a/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml
> index f63149ecf3e1b..fe94dbeb80c60 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml
> @@ -8,16 +8,23 @@ title: Qualcomm Camera Clock & Reset Controller on Milos
[ ... ]
>  properties:
>    compatible:
> -    const: qcom,milos-camcc
> +    enum:
> +      - qcom,eliza-cambistmclkcc
> +      - qcom,eliza-camcc
> +      - qcom,milos-camcc

Since qcom,milos-camcc.yaml already has an unconditional inheritance from
qcom,gcc.yaml:

Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml:
    allOf:
      - $ref: qcom,gcc.yaml#

the base schema enforces #reset-cells in its required list.

> diff --git a/include/dt-bindings/clock/qcom,eliza-cambistmclkcc.h b/include/dt-bindings/clock/qcom,eliza-cambistmclkcc.h
> new file mode 100644
> index 0000000000000..7b8b285f18d27
> --- /dev/null
> +++ b/include/dt-bindings/clock/qcom,eliza-cambistmclkcc.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_QCOM_CAM_BIST_MCLK_CC_ELIZA_H
> +#define _DT_BINDINGS_CLK_QCOM_CAM_BIST_MCLK_CC_ELIZA_H
> +
> +/* CAM_BIST_MCLK_CC clocks */
> +#define CAM_BIST_MCLK_CC_MCLK0_CLK				0
[ ... ]
> +#define CAM_BIST_MCLK_CC_SLEEP_CLK				19
> +#define CAM_BIST_MCLK_CC_SLEEP_CLK_SRC				20
> +
> +#endif

Looking at this newly added header, the BIST MCLK controller only provides
clocks and defines zero resets.

Forcing a pure clock provider to declare #reset-cells to pass schema
validation seems semantically incorrect. It might force developers to add
bogus properties to their device trees to circumvent validation errors,
which happened in the subsequent commit "arm64: dts: qcom: eliza: Add
support for MM clock controllers".

Would it make sense to use an if-then conditional block in the schema to
restrict the #reset-cells requirement to actual reset providers, or move
the BIST controller to a separate schema file?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-eliza_mm_cc_v2-v4-0-e61b5434e8d9@oss.qualcomm.com?part=3