Re: [PATCH v2 1/3] dt-bindings: clock: qcom: Add SM8750 GPU clocks

[Krzysztof Kozlowski <krzk@kernel.org>]

On Wed, Jul 23, 2025 at 10:38:48PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> The SM8750 features a "traditional" GPU_CC block, much of which is
> controlled through the GMU microcontroller. Additionally, there's
> an separate GX_CC block, where the GX GDSC is moved.
> 
> Add bindings to accommodate for that.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
>  .../bindings/clock/qcom,sm8450-gpucc.yaml          |  5 ++
>  .../bindings/clock/qcom,sm8750-gxcc.yaml           | 61 ++++++++++++++++++++++
>  include/dt-bindings/clock/qcom,sm8750-gpucc.h      | 53 +++++++++++++++++++
>  3 files changed, 119 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
> index 02968632fb3af34d6b3983a6a24aa742db1d59b1..d1b3557ab344b071d16dba4d5c6a267b7ab70573 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
> @@ -20,6 +20,7 @@ description: |
>      include/dt-bindings/clock/qcom,sm8550-gpucc.h
>      include/dt-bindings/reset/qcom,sm8450-gpucc.h
>      include/dt-bindings/reset/qcom,sm8650-gpucc.h
> +    include/dt-bindings/reset/qcom,sm8750-gpucc.h
>      include/dt-bindings/reset/qcom,x1e80100-gpucc.h
>  
>  properties:
> @@ -31,6 +32,7 @@ properties:
>        - qcom,sm8475-gpucc
>        - qcom,sm8550-gpucc
>        - qcom,sm8650-gpucc
> +      - qcom,sm8750-gpucc
>        - qcom,x1e80100-gpucc
>        - qcom,x1p42100-gpucc
>  
> @@ -40,6 +42,9 @@ properties:
>        - description: GPLL0 main branch source
>        - description: GPLL0 div branch source
>  
> +  power-domains:
> +    maxItems: 1

This should be a different binding or you need to restrict other
variants here.

> +
>  required:
>    - compatible
>    - clocks
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8750-gxcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8750-gxcc.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..b900c19156f5a2ba4e0f7c95276c771f615fdf23
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8750-gxcc.yaml

There is nothing for clocks in the binding. Place power domain providers
in their directory.

> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,sm8750-gxcc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Graphics Clock & Reset Controller on SM8750

There is no clocks nor resets here. Only power domains.

> +
> +maintainers:
> +  - Konrad Dybcio <konradybcio@kernel.org>
> +
> +description: |
> +  Qualcomm graphics clock control module provides the clocks, resets and power

Also confusing.

> +  domains on Qualcomm SoCs.
> +
> +  See also:
> +    include/dt-bindings/reset/qcom,sm8750-gpucc.h

reset or clock path?

> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sm8750-gxcc
> +
> +  reg:
> +    maxItems: 1
> +
> +  power-domains:
> +    items:
> +      - description: GFX voltage rail
> +      - description: MX_COLLAPSIBLE voltage rail
> +      - description: GPU_CC_CX GDSC
> +
> +  '#power-domain-cells':
> +    const: 1
> +
> +required:
> +  - compatible
> +  - power-domains
> +  - '#power-domain-cells'
> +

You miss ref... or this is a bit confusing.

> +unevaluatedProperties: false

additionalProperties instead

> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,sm8750-gpucc.h>
> +    #include <dt-bindings/power/qcom,rpmhpd.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        clock-controller@3d64000 {

No, clock controllers have clock-cells. This cannot be a clock
controller if it does not have any clocks for anyone to use.

Best regards,
Krzysztof