Re: [PATCH v2 1/4] dt-bindings: display/msm: add core clock to the mdss bindings

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 09/01/2023 01:52, Dmitry Baryshkov wrote:
> Add (optional) core clock to the mdss bindings to let the MDSS driver
> access harware registers before MDP driver probes.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../bindings/display/msm/qcom,mdss.yaml       | 34 ++++++++++++++-----
>  1 file changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml
> index ba0460268731..0647fc5a7d94 100644
> --- a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml
> @@ -45,17 +45,11 @@ properties:
>  
>    clocks:
>      minItems: 1
> -    items:
> -      - description: Display abh clock
> -      - description: Display axi clock

Not related to this patch, but it is a bit surprising to see AXI clock
optional.




> -      - description: Display vsync clock
> +    maxItems: 4
>  
>    clock-names:
>      minItems: 1
> -    items:
> -      - const: iface
> -      - const: bus
> -      - const: vsync
> +    maxItems: 4
>  
>    "#address-cells":
>      const: 1
> @@ -69,6 +63,30 @@ properties:
>      items:
>        - description: MDSS_CORE reset
>  
> +oneOf:
> +  - properties:
> +      clocks:
> +        minItems: 3
> +        maxItems: 4
> +
> +      clock-names:
> +        minItems: 3
> +        items:
> +          - const: iface
> +          - const: bus

BTW, sc7180-mdss uses here ahb name and calls it "AHB clock from dispcc".

SM8250 won't match here either. Maybe this should be reworked to specify
limits here but not the names and actual clocks? IOW, drop entire oneOf?

There were a lot, a lot of changes to MDSS/DPU bindings recently, so I
am really loosing track what is done where and when.

There are also few separate patchsets from you on the lists. Could they
be combined into one cleanup?

I understand that sometimes new cleanup is needed after old cleanup
finished (I had the same with pinctrl), so it is not a complain.

Another problem (and this time I complain) is that several of your
patchsets were sent, discussed and then without any notice applied. No
message that a patchset was applied to some tree. Look:

https://lore.kernel.org/all/20221124001708.25720-2-a39.skl@gmail.com/
https://lore.kernel.org/all/09ed16e1-4af2-8fce-dab4-f6c0f09e688c@linaro.org/

Nothing. Silent application. If you are the maintainer which picks up
the patch, please always, always send message that they are applied.
Patchwork does it automatically, b4 can do it easily as well. If you use
other tools - use other tools for sending it. Otherwise things are
discussed on mailing lists, receive several comments and there is never
a resubmit but instead they show in the tree.



Best regards,
Krzysztof