Re: [PATCH 08/11] dt-bindings: display/msm: add mdp-opp-table to dpu-sdm845

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 27 June 2022 21:05:06 GMT+03:00, Rob Herring <robh@kernel.org> wrote:
>On Sun, Jun 26, 2022 at 02:25:10AM +0300, Dmitry Baryshkov wrote:
>> On SDM845 platforms DPU device tree node contains child object
>> mdp-opp-table providing OPP table for the DPU. Add it to the list of
>> properties to let sdm845.dtsi to validate.
>> 
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>  .../devicetree/bindings/display/msm/dpu-sdm845.yaml      | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml
>> index 0dc16326bf8e..cc95adcf8f11 100644
>> --- a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml
>> +++ b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml
>> @@ -50,6 +50,10 @@ properties:
>>      maxItems: 1
>>  
>>    operating-points-v2: true
>> +
>> +  mdp-opp-table:
>
>Is there another kind of opp-table besides mdp? Node names should be 
>generic.

Ack. Now if we rename the node to opp-table, should we define the old name with deprecated: true?

>
>> +    $ref: /schemas/opp/opp-v2.yaml#
>> +
>>    ports:
>>      $ref: /schemas/graph.yaml#/properties/ports
>>      description: |
>> @@ -116,11 +120,12 @@ examples:
>>                            <0x0aeb0000 0x2008>;
>>                      reg-names = "mdp", "vbif";
>>  
>> -                    clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
>> +                    clocks = <&gcc GCC_DISP_AXI_CLK>,
>> +                             <&dispcc DISP_CC_MDSS_AHB_CLK>,
>
>What does the OPP table have to do with clocks? Adding a clock anywhere 
>but the end is an ABI break.

I should split this to a separate patch. And, I must admit, this clock change has already landed. We did not think that it is an ABI break since we have clock-names here.

>
>>                               <&dispcc DISP_CC_MDSS_AXI_CLK>,
>>                               <&dispcc DISP_CC_MDSS_MDP_CLK>,
>>                               <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
>> -                    clock-names = "iface", "bus", "core", "vsync";
>> +                    clock-names = "gcc-bus", "iface", "bus", "core", "vsync";
>>  
>>                      interrupt-parent = <&mdss>;
>>                      interrupts = <0>;