mbox series

[0/7] msm8916/msm8939 DSI controller and DSI PHY bindings fixes

Message ID 20220630120845.3356144-1-bryan.odonoghue@linaro.org
Headers show
Series msm8916/msm8939 DSI controller and DSI PHY bindings fixes | expand

Message

Bryan O'Donoghue June 30, 2022, 12:08 p.m. UTC
This series fixes up a number of dtbs checks which are being flagged adding
in the msm8939 dtsi.

When converting from .txt to .yaml a number of the parameters for the older
msm8916 silicon were not transmitted into the yaml.

Adding in the msm8939 which is a near 1:1 copy of the msm8916 in terms of
dtsi triggers a rake of dtbs checks as a result.

Bryan O'Donoghue (7):
  dt-bindings: msm: dsi: Fix phy-name binding
  dt-bindings: msm: dsi: Fix operating-points-v2 constraint
  dt-bindings: msm: dsi: Fix power-domains constraint
  dt-bindings: msm: dsi: Add vdd* descriptions back in
  dt-bindings: msm: dsi: Fix clock declarations
  dt-bindings: msm: dsi: Add missing clocks to 28nm DSI PHY YAML
  dt-bindings: msm: dsi: Add missing qcom,dsi-phy-regulator-ldo-mode

 .../display/msm/dsi-controller-main.yaml      | 39 ++++++++++++++-----
 .../bindings/display/msm/dsi-phy-28nm.yaml    | 20 ++++++++++
 2 files changed, 49 insertions(+), 10 deletions(-)

Comments

Dmitry Baryshkov June 30, 2022, 5:06 p.m. UTC | #1
On 30 June 2022 15:08:39 GMT+03:00, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
>The driver doesn't care about the name of the phy in DTS. msm8916 has since
>2016 named the phy "dsi-phy" with later SoCs taking the name "dsi".
>
>There's no reason to have a constraint on the name here, simply to have a
>name. Remove the redundant const in favour of maxItems: 1.
>
>Fixes: 4dbe55c97741 ("dt-bindings: msm: dsi: add yaml schemas for DSI bindings")
>Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

>---
> .../devicetree/bindings/display/msm/dsi-controller-main.yaml    | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>index 880bfe930830c..717a5d255ffe4 100644
>--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>@@ -49,7 +49,7 @@ properties:
>     maxItems: 1
> 
>   phy-names:
>-    const: dsi
>+    maxItems: 1
> 
>   "#address-cells": true
>
Dmitry Baryshkov June 30, 2022, 5:16 p.m. UTC | #2
On 30 June 2022 15:08:42 GMT+03:00, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
>When converting from .txt to .yaml we didn't include descriptions for the
>existing regulator supplies.
>
>- vdd
>- vdda
>- vddio
>
>Add those descriptions into the yaml now as they were prior to the
>conversion.
>
>Fixes: 4dbe55c97741 ("dt-bindings: msm: dsi: add yaml schemas for DSI bindings")
>Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>---
> .../bindings/display/msm/dsi-controller-main.yaml    | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
>diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>index 91324f0828448..fd9d472437853 100644
>--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>@@ -125,6 +125,18 @@ properties:
>       - port@0
>       - port@1
> 
>+  vdd-supply:
>+    description:
>+      Phandle to vdd regulator device node
>+
>+  vddio-supply:
>+    description:
>+      Phandle to vdd regulator device node
>+
>+  vdda-supply:
>+    description:
>+      Phandle to vdd regulator device node


All three descriptions are the same. This looks like a c&p issue

>+
> required:
>   - compatible
>   - reg
Dmitry Baryshkov June 30, 2022, 5:20 p.m. UTC | #3
On 30 June 2022 15:08:44 GMT+03:00, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
>Add in missing clock and clock-names declarations to the 28nm DSI PHY.
>When converting from .txt to .yaml we missed these.
>
>Fixes: 4dbe55c97741 ("dt-bindings: msm: dsi: add yaml schemas for DSI bindings")
>Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>---
> .../bindings/display/msm/dsi-phy-28nm.yaml      | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
>diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml
>index 3d8540a06fe22..9ecd513d93661 100644
>--- a/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml
>+++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml
>@@ -19,6 +19,21 @@ properties:
>       - qcom,dsi-phy-28nm-lp
>       - qcom,dsi-phy-28nm-8960
> 
>+  clocks:
>+    minItems: 1
>+    maxItems: 2
>+
>+  clock-names:
>+    items:
>+      - const: iface
>+        description: AHB clock
>+      - const: iface_clk
>+        description: AHB clock
>+      - const: ref
>+        description: Block reference clock
>+    minItems: 1
>+    maxItems: 2

This looks incorrect. You have 3 items, but maxItems in 2

Also I'd put description to the clocks node. Clock-names are self-descriptive enough.

>+
>   reg:
>     items:
>       - description: dsi pll register set
>@@ -36,6 +51,8 @@ properties:
> 
> required:
>   - compatible
>+  - clocks
>+  - clock-names
>   - reg
>   - reg-names
>   - vddio-supply
Bryan O'Donoghue June 30, 2022, 5:30 p.m. UTC | #4
On 30/06/2022 18:16, Dmitry Baryshkov wrote:
> 
> All three descriptions are the same. This looks like a c&p issue

Those are what the previous values were.

I'll come up with something less robotic though.
Krzysztof Kozlowski June 30, 2022, 7 p.m. UTC | #5
On 30/06/2022 14:08, Bryan O'Donoghue wrote:
> The existing msm8916.dtsi does not depend on nor require operating points.
> 
> Fixes: 4dbe55c97741 ("dt-bindings: msm: dsi: add yaml schemas for DSI bindings")
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof
Krzysztof Kozlowski June 30, 2022, 7:02 p.m. UTC | #6
On 30/06/2022 14:08, Bryan O'Donoghue wrote:
> When converting from .txt to .yaml we didn't include descriptions for the
> existing regulator supplies.
> 
> - vdd
> - vdda
> - vddio
> 
> Add those descriptions into the yaml now as they were prior to the
> conversion.
> 
> Fixes: 4dbe55c97741 ("dt-bindings: msm: dsi: add yaml schemas for DSI bindings")
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../bindings/display/msm/dsi-controller-main.yaml    | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index 91324f0828448..fd9d472437853 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -125,6 +125,18 @@ properties:
>        - port@0
>        - port@1
>  
> +  vdd-supply:
> +    description:
> +      Phandle to vdd regulator device node
> +
> +  vddio-supply:
> +    description:
> +      Phandle to vdd regulator device node
> +
> +  vdda-supply:
> +    description:
> +      Phandle to vdd regulator device node
> +

They should be also added to required fields, because before they were
required.


Best regards,
Krzysztof
Rob Herring July 1, 2022, 7:57 p.m. UTC | #7
On Thu, Jun 30, 2022 at 08:59:03PM +0200, Krzysztof Kozlowski wrote:
> On 30/06/2022 14:08, Bryan O'Donoghue wrote:
> > The driver doesn't care about the name of the phy in DTS. msm8916 has since
> > 2016 named the phy "dsi-phy" with later SoCs taking the name "dsi".
> > 
> > There's no reason to have a constraint on the name here, simply to have a
> > name. Remove the redundant const in favour of maxItems: 1.
> 
> For single entry the names field could be as well removed. But if it is
> there, let's make it specific, not generic. Therefore instead add
> allOf:if:then restricting it per variants.

If we really don't care, an if/then seems excessive. Just mark as 
deprecated and remove it in all the dts files. 

Rob
Rob Herring July 1, 2022, 8:08 p.m. UTC | #8
On Thu, Jun 30, 2022 at 01:08:39PM +0100, Bryan O'Donoghue wrote:
> The driver doesn't care about the name of the phy in DTS. msm8916 has since
> 2016 named the phy "dsi-phy" with later SoCs taking the name "dsi".

Also, for the subject, it's 'phy-names' not 'phy-name'.

> There's no reason to have a constraint on the name here, simply to have a
> name. Remove the redundant const in favour of maxItems: 1.
> 
> Fixes: 4dbe55c97741 ("dt-bindings: msm: dsi: add yaml schemas for DSI bindings")
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../devicetree/bindings/display/msm/dsi-controller-main.yaml    | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index 880bfe930830c..717a5d255ffe4 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -49,7 +49,7 @@ properties:
>      maxItems: 1
>  
>    phy-names:
> -    const: dsi
> +    maxItems: 1
>  
>    "#address-cells": true
>  
> -- 
> 2.36.1
> 
>