On Tue, May 26, 2026 at 07:27:55PM +0800, Damon Ding wrote: > 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 > > > > > > > > --- > > > > > > > > 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? Sure. This feels like a bug because I have no idea where the "maximum of 2" is coming from. I'll ask Rob about it.