Re: [PATCH v8 01/10] dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock for RK3588

[Damon Ding <damon.ding@rock-chips.com>]

Hi Conor,

On 5/26/2026 8:54 AM, Damon Ding wrote:
> Hi Conor,
> 
> On 5/26/2026 12:54 AM, Conor Dooley wrote:
>> On Mon, May 25, 2026 at 08:53:22PM +0800, Damon Ding wrote:
>>> RK3588 eDP controller requires HCLK_VO1 to access the VO1 GRF
>>> registers and enable the video datapath.
>>>
>>> Previously, the clock was enabled implicitly via the 'rockchip,vo-grf'
>>> phandle reference, which allowed the eDP to work without explicitly
>>> managing the hclk_vo1 clock. However, this is not safe or explicit.
>>>
>>> To make the clock dependency explicit, enforce per-SoC clock-names
>>> requirements:
>>>   - RK3288: 2 clocks (dp, pclk)
>>>   - RK3399: 3 clocks (dp, pclk, grf)
>>>   - RK3588: 3 clocks (dp, pclk, hclk)
>>>
>>> Do not reuse the 'grf' clock name for RK3588 because it represents
>>> a different clock with distinct control logic:
>>> - The 'grf' clock is only for GRF register access and is toggled
>>>    dynamically during register access.
>>> - The 'hclk' clock controls both GRF access and video datapath
>>>    gating, and must remain enabled during probe.
>>>
>>> Fixes: f855146263b1 ("dt-bindings: display: rockchip: analogix-dp: 
>>> Add support for RK3588")
>>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>>
>>> ---
>>>
>>> Changes in v4:
>>> - Modify the commit msg.
>>>
>>> Changes in v5:
>>> - Enforce the correct third clock name on a per-compatible basis.
>>> - Modify the commit msg simultaneously.
>>>
>>> Changes in v6:
>>> - Expand more detail commit msg about using hclk instead of grf clock.
>>>
>>> Changes in v7:
>>> - List all valid clock names at the top level, and constrain the clock
>>>    count for each platform with minItems/maxItems in allOf.
>>>
>>> Changes in v8:
>>> - Fix indentation to 10 for enum in clock-names property.
>>> ---
>>>   .../rockchip/rockchip,analogix-dp.yaml        | 34 ++++++++++++++++++-
>>>   1 file changed, 33 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/rockchip/ 
>>> rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/ 
>>> display/rockchip/rockchip,analogix-dp.yaml
>>> index d99b23b88cc5..a1ab7a77bdd3 100644
>>> --- a/Documentation/devicetree/bindings/display/rockchip/ 
>>> rockchip,analogix-dp.yaml
>>> +++ b/Documentation/devicetree/bindings/display/rockchip/ 
>>> rockchip,analogix-dp.yaml
>>> @@ -26,7 +26,9 @@ properties:
>>>       items:
>>>         - const: dp
>>>         - const: pclk
>>> -      - const: grf
>>> +      - enum:
>>> +          - grf
>>> +          - hclk
>>>     power-domains:
>>>       maxItems: 1
>>> @@ -60,6 +62,32 @@ required:
>>>   allOf:
>>>     - $ref: /schemas/display/bridge/analogix,dp.yaml#
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - rockchip,rk3288-dp
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          maxItems: 2
>>> +        clock-names:
>>> +          maxItems: 2
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - rockchip,rk3399-edp
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          minItems: 3
>>> +        clock-names:
>>> +          minItems: 3
>>
>> If you go back to v6, you'll see that I never asked you to remove the
>> explict clock-names from here or below. Only the one from the 3288
>> section. The minItems was an addition, not a replacement.
>>
>> pw-bot: changes-requested
>>
> 
> Sorry for the misunderstanding. I will restore the explicit clock-names 
> definitions and fix this in next version.
> 

Sorry to bother you. I attempted to place the explicit clock-names under 
minItems:

diff --git 
a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml 
b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
index a1ab7a77bdd3..ef03edf52de8 100644
--- 
a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ 
b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -87,6 +87,10 @@ allOf:
            minItems: 3
          clock-names:
            minItems: 3
+          items:
+            - const: dp
+            - const: pclk
+            - const: grf

    - if:
        properties:
@@ -100,6 +104,10 @@ allOf:
            minItems: 3
          clock-names:
            minItems: 3
+          items:
+            - const: dp
+            - const: pclk
+            - const: hclk
          resets:
            minItems: 2
          reset-names:

make dt_binding_check 
DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml

Then the following errors occurred:

   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
   CHKDT   ./Documentation/devicetree/bindings
/home/ding/drm-misc/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml: 
allOf:2:then:properties:clock-names: 'oneOf' conditional failed, one 
must be fixed:
         False schema does not allow 3
         [{'const': 'dp'}, {'const': 'pclk'}, {'const': 'grf'}] is too long
         [{'const': 'dp'}, {'const': 'pclk'}, {'const': 'grf'}] is too short
         1 was expected
         3 is greater than the maximum of 2
         hint: "minItems" is only needed if less than the "items" list 
length
         from schema $id: http://devicetree.org/meta-schemas/items.yaml
/home/ding/drm-misc/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml: 
allOf:3:then:properties:clock-names: 'oneOf' conditional failed, one 
must be fixed:
         False schema does not allow 3
         [{'const': 'dp'}, {'const': 'pclk'}, {'const': 'hclk'}] is too long
         [{'const': 'dp'}, {'const': 'pclk'}, {'const': 'hclk'}] is too 
short
         1 was expected
         3 is greater than the maximum of 2
         hint: "minItems" is only needed if less than the "items" list 
length
         from schema $id: http://devicetree.org/meta-schemas/items.yaml
   LINT    ./Documentation/devicetree/bindings
   DTEX 
Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.example.dts
   DTC [C] 
Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.example.dtb

Neither keeping only minItems nor only the explicit clock-names causes 
any errors. Would it be a better idea to keep just the explicit 
clock-names here?

Best regards,
Damon

> 
>>
>>> +
>>>     - if:
>>>         properties:
>>>           compatible:
>>> @@ -68,6 +96,10 @@ allOf:
>>>                 - rockchip,rk3588-edp
>>>       then:
>>>         properties:
>>> +        clocks:
>>> +          minItems: 3
>>> +        clock-names:
>>> +          minItems: 3
>>>           resets:
>>>             minItems: 2
>>>           reset-names:
>>> -- 
>>> 2.34.1
>>>
>>>
> 
> 
>