[PATCH v6 01/12] dt-bindings: display/msm: split qcom,mdss bindings

[Dmitry Baryshkov]

On 08/09/2022 22:37, Rob Herring wrote:
> On Thu, Sep 08, 2022 at 03:37:38PM +0200, Krzysztof Kozlowski wrote:
>> On 01/09/2022 12:23, Dmitry Baryshkov wrote:
>>> Split Mobile Display SubSystem (MDSS) root node bindings to the separate
>>> yaml file. Changes to the existing (txt) schema:
>>>   - Added optional "vbif_nrt_phys" region used by msm8996
>>>   - Made "bus" and "vsync" clocks optional (they are not used by some
>>>     platforms)
>>>   - Added (optional) "core" clock added recently to the mdss driver
>>>   - Added optional resets property referencing MDSS reset
>>>   - Defined child nodes pointing to corresponding reference schema.
>>>   - Dropped the "lut" clock. It was added to the schema by mistake (it is
>>>     a part of mdp4 schema, not the mdss).
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>> ---
>>>   .../devicetree/bindings/display/msm/mdp5.txt  |  30 +---
>>>   .../devicetree/bindings/display/msm/mdss.yaml | 166 ++++++++++++++++++
>>>   2 files changed, 167 insertions(+), 29 deletions(-)
>>>   create mode 100644 Documentation/devicetree/bindings/display/msm/mdss.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/msm/mdp5.txt b/Documentation/devicetree/bindings/display/msm/mdp5.txt
>>> index 43d11279c925..65d03c58dee6 100644
>>> --- a/Documentation/devicetree/bindings/display/msm/mdp5.txt
>>> +++ b/Documentation/devicetree/bindings/display/msm/mdp5.txt
>>> @@ -2,37 +2,9 @@ Qualcomm adreno/snapdragon MDP5 display controller
>>>   
>>>   Description:
>>>   
>>> -This is the bindings documentation for the Mobile Display Subsytem(MDSS) that
>>> -encapsulates sub-blocks like MDP5, DSI, HDMI, eDP etc, and the MDP5 display
>>> +This is the bindings documentation for the MDP5 display
>>>   controller found in SoCs like MSM8974, APQ8084, MSM8916, MSM8994 and MSM8996.
>>>   
>>> -MDSS:
>>> -Required properties:
>>> -- compatible:
>>> -  * "qcom,mdss" - MDSS
>>> -- reg: Physical base address and length of the controller's registers.
>>> -- reg-names: The names of register regions. The following regions are required:
>>> -  * "mdss_phys"
>>> -  * "vbif_phys"
>>> -- interrupts: The interrupt signal from MDSS.
>>> -- interrupt-controller: identifies the node as an interrupt controller.
>>> -- #interrupt-cells: specifies the number of cells needed to encode an interrupt
>>> -  source, should be 1.
>>> -- power-domains: a power domain consumer specifier according to
>>> -  Documentation/devicetree/bindings/power/power_domain.txt
>>> -- clocks: device clocks. See ../clocks/clock-bindings.txt for details.
>>> -- clock-names: the following clocks are required.
>>> -  * "iface"
>>> -  * "bus"
>>> -  * "vsync"
>>> -- #address-cells: number of address cells for the MDSS children. Should be 1.
>>> -- #size-cells: Should be 1.
>>> -- ranges: parent bus address space is the same as the child bus address space.
>>> -
>>> -Optional properties:
>>> -- clock-names: the following clocks are optional:
>>> -  * "lut"
>>> -
>>>   MDP5:
>>>   Required properties:
>>>   - compatible:
>>> diff --git a/Documentation/devicetree/bindings/display/msm/mdss.yaml b/Documentation/devicetree/bindings/display/msm/mdss.yaml
>>> new file mode 100644
>>> index 000000000000..8860fc55cca5
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/display/msm/mdss.yaml
>>> @@ -0,0 +1,166 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/display/msm/mdss.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm Mobile Display SubSystem (MDSS)
>>> +
>>> +maintainers:
>>> +  - Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>> +  - Rob Clark <robdclark at gmail.com>
>>> +
>>> +description:
>>> +  This is the bindings documentation for the Mobile Display Subsytem(MDSS) that
>>> +  encapsulates sub-blocks like MDP5, DSI, HDMI, eDP, etc.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - qcom,mdss
>>> +
>>> +  reg:
>>> +    minItems: 2
>>> +    maxItems: 3
>>> +
>>> +  reg-names:
>>> +    minItems: 2
>>> +    items:
>>> +      - const: mdss_phys
>>> +      - const: vbif_phys
>>> +      - const: vbif_nrt_phys
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  interrupt-controller:
>>> +    true
>>
>> If there is going to be v7 - please make it one line.
>>
>>> +
>>> +  "#interrupt-cells":
>>> +    const: 1
>>> +
>>> +  power-domains:
>>> +    maxItems: 1
>>> +    description: |
>>> +      The MDSS power domain provided by GCC
>>> +
>>> +  clocks:
>>> +    minItems: 1
>>> +    items:
>>> +      - description: Display abh clock
>>> +      - description: Display axi clock
>>> +      - description: Display vsync clock
>>> +
>>> +  clock-names:
>>> +    minItems: 1
>>> +    items:
>>> +      - const: iface
>>> +      - const: bus
>>> +      - const: vsync
>>> +
>>> +  "#address-cells":
>>> +    const: 1
>>> +
>>> +  "#size-cells":
>>> +    const: 1
>>> +
>>> +  ranges:
>>> +    true
>>
>> Ditto.
>>
>>> +
>>> +  resets:
>>> +    items:
>>> +      - description: MDSS_CORE reset
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - reg-names
>>> +  - interrupts
>>> +  - interrupt-controller
>>> +  - "#interrupt-cells"
>>> +  - power-domains
>>> +  - clocks
>>> +  - clock-names
>>> +  - "#address-cells"
>>> +  - "#size-cells"
>>> +  - ranges
>>> +
>>> +patternProperties:
>>> +  "^mdp@[1-9a-f][0-9a-f]*$":
>>> +    type: object
>>> +    properties:
>>> +      compatible:
>>> +        const: qcom,mdp5
>>> +
>>> +  "^dsi@[1-9a-f][0-9a-f]*$":
>>> +    type: object
>>> +    properties:
>>> +      compatible:
>>> +        const: qcom,mdss-dsi-ctrl
>>
>> This should be ref to dsi-controller-main.yaml... or based on previous
>> Rob's feedback you dropped it everywhere in children?
> 
> I don't think I said. I thought about it some, as yes, we normally have
> done as you suggested. The downside is with a ref we'll do the whole
> validation of the child node twice (unless the referenced schema has a
> 'select: false') whereas here only 'compatible' is validated twice. This
> way also complicates checking for unevaluatedProperties/additionalProperties.
> 
> So maybe better to keep with the 'normal' way and make this a ref.

Well.. I tried using $ref in the previous iteration, but then I faced 
the fact that I can not use it. Using the $ref the node gets validated 
even if it is disabled, and we do not want to do this. The nodes are 
usually split in way that regulators are specified in the board DT file. 
Thus disabled dsi/dsi-phy nodes do not get all the required regulators. 
And dt-validate happily dumps tons of warnings for disabled nodes. Thus 
I ended up with the compatible rather than $ref.


-- 
With best wishes
Dmitry