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@xxxxxxxxxx> > > --- > > .../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@xxxxxxxxxx> > > + - Rob Clark <robdclark@xxxxxxxxx> > > + > > +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. Rob