Re: [PATCH v3 1/2] dt-bindings: clock: qcom: Add Hawi CAMCC compatible strings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt-bindings: undocumented interconnects requirement for Hawi
- [Medium] dt-bindings: incorrect commit message claim about ICP instances
--

commit cd229f4ac046e3883d582722f42537a831b1a1f1
Author: Vivek Aknurwar <vivek.aknurwar@oss.qualcomm.com>

dt-bindings: clock: qcom: Add Hawi CAMCC compatible strings

This commit adds the qcom,hawi-camcc and qcom,hawi-cambistmclkcc compatible
strings for the Hawi SoC. It also introduces the corresponding clock ID header
with definitions for the updated camera clock controller.

>     The Hawi SoC is a derivative of Kaanapali  with an updated camera
>     clock controller (CAMCC). While the overall block structure is
>     similar, Hawi's CAMCC differs from Kaanapali in the following ways:
>     
>       - Uses Taycan EHA-T PLLs instead of Taycan EKO-T PLLs, with an
>         additional ninth PLL (PLL8)
>       - Adds a SQPF block with its own GDSC, AHB clock, and BCR reset
>       - Adds a CX GDSC (CAM_CC_TITAN_TOP_CX_GDSC) for the camera subsystem
>       - Expands ICP from one to two instances (ICP_0 and ICP_1)

[Severity: Medium]
Is this description accurate regarding the ICP instances? The commit
message claims that Hawi differs from Kaanapali by expanding from one
to two instances, but looking at include/dt-bindings/clock/qcom,kaanapali-camcc.h,
Kaanapali already defines clock IDs for both ICP_0 and ICP_1.

Could the commit message be updated to reflect the actual differences?

> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> index f653807472201..e2a126925e9d5 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml

[ ... ]

> @@ -80,12 +91,22 @@ allOf:
>      then:
>        required:
>          - required-opps
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,hawi-camcc
> +    then:
> +      required:
> +        - interconnects

[Severity: Medium]
Is there a missing explanation in the commit message for this new
requirement? The schema conditionally enforces the interconnects property
for the new qcom,hawi-camcc variant here in allOf, but the commit message
does not mention that interconnects are now mandatory for this hardware.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-camcc-hawi-v3-0-5b57f45477f1@oss.qualcomm.com?part=1