linux-kernel - Re: [PATCH 3/4] dt-bindings: display/msm: add stream 1 pixel clock binding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f7941d74-3856-4bd9-95db-0b7f09eb07fd@kernel.org>
Date: Thu, 8 May 2025 08:18:30 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Abhinav Kumar <quic_abhinavk@...cinc.com>, Rob Clark
 <robdclark@...il.com>, Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
 Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Kuogee Hsieh <quic_khsieh@...cinc.com>,
 Mahadevan <quic_mahap@...cinc.com>
Cc: linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 freedreno@...ts.freedesktop.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] dt-bindings: display/msm: add stream 1 pixel clock
 binding

On 23/04/2025 04:46, Abhinav Kumar wrote:
> Hi Krzysztof
> 
> On 12/3/2024 12:04 AM, Krzysztof Kozlowski wrote:
>> On 03/12/2024 04:31, Abhinav Kumar wrote:
>>> On some chipsets the display port controller can support more
>>
>> Which chipsets?
>>
> 
>  From the current list of chipsets which support DP, the following can 
> support more than one stream.
> 
> qcom,sa8775p-dp
> qcom,sc7280-dp
> qcom,sc8180x-dp
> qcom,sc8280xp-dp
> qcom,sm8350-dp
> qcom,sm8650-dp
> qcom,sm8550-dp
> qcom,sm8450-dp
> qcom,sm8250-dp
> qcom,sm8150-dp
> 
> So do you also want all of these to be added in the same if block as
> qcom,sa8775p-dp?

That was talk in 2024. Entire context is gone if you reply after three
months. I do not have even that emails in my inbox anymore.

Probably I expected commit msg to mention at least some, so everyone
knows which chipsets are affected here and one can verify the statements
from commit msg.

> 
>>> than one pixel stream (multi-stream transport). To support MST
>>> on such chipsets, add the binding for stream 1 pixel clock for
>>> display port controller. Since this mode is not supported on all
>>> chipsets, add exception rules and min/max items to clearly mark
>>> which chipsets support only SST mode (single stream) and which ones
>>> support MST.
>>>
>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
>>> ---
>>>   .../bindings/display/msm/dp-controller.yaml        | 32 ++++++++++++++++++++++
>>>   .../bindings/display/msm/qcom,sa8775p-mdss.yaml    |  9 ++++--
>>>   2 files changed, 38 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>>> index 9fe2bf0484d8..650d19e58277 100644
>>> --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>>> +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>>> @@ -50,30 +50,38 @@ properties:
>>>       maxItems: 1
>>>   
>>>     clocks:
>>> +    minItems: 5
>>>       items:
>>>         - description: AHB clock to enable register access
>>>         - description: Display Port AUX clock
>>>         - description: Display Port Link clock
>>>         - description: Link interface clock between DP and PHY
>>>         - description: Display Port stream 0 Pixel clock
>>> +      - description: Display Port stream 1 Pixel clock
>>>   
>>>     clock-names:
>>> +    minItems: 5
>>>       items:
>>>         - const: core_iface
>>>         - const: core_aux
>>>         - const: ctrl_link
>>>         - const: ctrl_link_iface
>>>         - const: stream_pixel
>>> +      - const: stream_1_pixel
>>>   
>>>     assigned-clocks:
>>> +    minItems: 2
>>>       items:
>>>         - description: link clock source
>>>         - description: stream 0 pixel clock source
>>> +      - description: stream 1 pixel clock source
>>>   
>>>     assigned-clock-parents:
>>> +    minItems: 2
>>>       items:
>>>         - description: Link clock PLL output provided by PHY block
>>>         - description: Stream 0 pixel clock PLL output provided by PHY block
>>> +      - description: Stream 1 pixel clock PLL output provided by PHY block
>>>   
>>>     phys:
>>>       maxItems: 1
>>> @@ -175,6 +183,30 @@ allOf:
>>>         required:
>>>           - "#sound-dai-cells"
>>>   
>>
>> Missing if: narrowing this to 5 items for other devices.
>>
> 
> OR would an else be better?

Usually not, although depends how this binding is written.


> 
> +    else:
> +      properties:
> +        clocks:
> +          maxItems: 5
> +        clock-names:
> +          items:
> +            - const: core_iface
> +            - const: core_aux
> +            - const: ctrl_link
> +            - const: ctrl_link_iface
> +            - const: stream_pixel
> 
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - qcom,sa8775p-dp
>>> +
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>
>> Missing minItems, otherwise it is pointless.
>>
> 
> I thought that since I have already specified the minItems as 5
> in the clocks in the section above, I need to specify only the maxItems 
> here?

No, you need explicit constraints here.



Best regards,
Krzysztof