Re: [PATCH v10 06/10] dt-bindings: clock: qcom: Add support for CAMCC for Eliza

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The binding imposes a strict 3-item `clocks` array constraint which conflicts with the typical hardware requirements of the newly added `qcom,eliza-cambistmclkcc` variant.
- [Medium] The schema incorrectly inherits a strict requirement for `#reset-cells` from `qcom,gcc.yaml#` and applies it to the `qcom,eliza-cambistmclkcc` variant, which does not provide resets.
--

--- Patch [6]: [PATCH v10 06/10] dt-bindings: clock: qcom: Add support for CAMCC for Eliza ---
commit edf279fa3bffce90685bc341e0addff553a7d4e2
Author: Taniya Das <taniya.das@oss.qualcomm.com>

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

This commit adds the compatible strings and dt-bindings headers for the
Eliza camera and camera BIST clock controllers to the existing Milos
camcc binding. It also enforces the '#power-domain-cells' requirement
for the camera clock controller.

> diff --git a/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml
> index c4aba40e316ca..0655cea72524e 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,milos-camcc.yaml
[ ... ]
>  properties:
>    compatible:
> -    const: qcom,milos-camcc
> +    enum:
> +      - qcom,eliza-cambistmclkcc
> +      - qcom,eliza-camcc
> +      - qcom,milos-camcc
>  
>    clocks:
>      items:

[Severity: Medium]
Is this global clocks array constraint too strict for the new BIST variant?

The base schema currently restricts the clocks property to exactly 3 specific
items. Do BIST clock controllers like qcom,eliza-cambistmclkcc use the exact
same 3 clocks, or do they require a different set and number (e.g., AHB, TCXO,
TCXO_AO, SLEEP)?

Should there be a conditional if-then block to override the clocks array
constraints for qcom,eliza-cambistmclkcc so validation doesn't fail when the
actual clocks used by the hardware are provided?

[ ... ]
> @@ -47,6 +54,15 @@ required:
>  
>  allOf:
>    - $ref: qcom,gcc.yaml#
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              const: qcom,eliza-cambistmclkcc

[Severity: Medium]
Does this inheritance from qcom,gcc.yaml# inadvertently force a
#reset-cells requirement on the qcom,eliza-cambistmclkcc variant?

The referenced qcom,gcc.yaml schema unconditionally requires the
#reset-cells property. By including it here globally, the new BIST controller
inherits this requirement. 

However, looking at the newly added header for this variant:

include/dt-bindings/clock/qcom,eliza-cambistmclkcc.h:
/* CAM_BIST_MCLK_CC clocks */
#define CAM_BIST_MCLK_CC_MCLK0_CLK                              0
#define CAM_BIST_MCLK_CC_MCLK0_CLK_SRC                          1
...

The controller only defines clocks and has no reset indices.

Could this cause device tree validation to fail for the new variant unless
authors add a dummy #reset-cells property?

Would it be better to make the $ref: qcom,gcc.yaml# inclusion conditional for
compatibles that actually provide resets, or perhaps split the BIST controller
into a separate YAML schema?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-eliza-mm-cc-v9-v10-0-6ba52dd14343@oss.qualcomm.com?part=6