diff mbox series

[PATCHv2,2/2] dt-bindings: watchdog: Add compatible for QCS404, SC7180, SDM845, SM8150

Message ID ff71077aa09c489b2b072c6f5605dccb96f60051.1580570160.git.saiprakash.ranjan@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series Convert QCOM watchdog timer bindings to YAML | expand

Commit Message

Sai Prakash Ranjan Feb. 1, 2020, 3:29 p.m. UTC
Add missing compatible for watchdog timer on QCS404,
SC7180, SDM845 and SM8150 SoCs.

Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 .../bindings/watchdog/qcom-wdt.yaml           | 21 ++++++++++++-------
 1 file changed, 13 insertions(+), 8 deletions(-)

Comments

Stephen Boyd Feb. 2, 2020, 4:41 a.m. UTC | #1
Quoting Sai Prakash Ranjan (2020-02-01 07:29:49)
> Add missing compatible for watchdog timer on QCS404,
> SC7180, SDM845 and SM8150 SoCs.
> 
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Rob Herring Feb. 6, 2020, 6:38 p.m. UTC | #2
On Sat, Feb 01, 2020 at 08:59:49PM +0530, Sai Prakash Ranjan wrote:
> Add missing compatible for watchdog timer on QCS404,
> SC7180, SDM845 and SM8150 SoCs.

That's not what the commit does. You are changing what's valid.

One string was valid, now 2 are required.

> 
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
>  .../bindings/watchdog/qcom-wdt.yaml           | 21 ++++++++++++-------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> index 5448cc537a03..7180c64f54fb 100644
> --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> @@ -14,14 +14,19 @@ allOf:
>  
>  properties:
>    compatible:
> -    enum:
> -      - qcom,kpss-timer
> -      - qcom,kpss-wdt
> -      - qcom,kpss-wdt-apq8064
> -      - qcom,kpss-wdt-ipq4019
> -      - qcom,kpss-wdt-ipq8064
> -      - qcom,kpss-wdt-msm8960
> -      - qcom,scss-timer
> +    items:
> +      - enum:
> +          - qcom,apss-wdt-qcs404
> +          - qcom,apss-wdt-sc7180
> +          - qcom,apss-wdt-sdm845
> +          - qcom,apss-wdt-sm8150
> +          - qcom,kpss-timer
> +          - qcom,kpss-wdt-apq8064
> +          - qcom,kpss-wdt-ipq4019
> +          - qcom,kpss-wdt-ipq8064
> +          - qcom,kpss-wdt-msm8960
> +          - qcom,scss-timer
> +      - const: qcom,kpss-wdt
>  
>    reg:
>      maxItems: 1
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
Sai Prakash Ranjan Feb. 7, 2020, 6:10 a.m. UTC | #3
Hi Rob,

On 2020-02-07 00:08, Rob Herring wrote:
> On Sat, Feb 01, 2020 at 08:59:49PM +0530, Sai Prakash Ranjan wrote:
>> Add missing compatible for watchdog timer on QCS404,
>> SC7180, SDM845 and SM8150 SoCs.
> 
> That's not what the commit does. You are changing what's valid.
> 
> One string was valid, now 2 are required.
> 

Does this look good?

diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml 
b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
index 46d6aad5786a..3378244b67cd 100644
--- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
@@ -14,19 +14,22 @@ allOf:

  properties:
    compatible:
-    items:
+    oneOf:
        - enum:
            - qcom,apss-wdt-qcs404
            - qcom,apss-wdt-sc7180
            - qcom,apss-wdt-sdm845
            - qcom,apss-wdt-sm8150
-          - qcom,kpss-timer
-          - qcom,kpss-wdt
            - qcom,kpss-wdt-apq8064
            - qcom,kpss-wdt-ipq4019
            - qcom,kpss-wdt-ipq8064
            - qcom,kpss-wdt-msm8960
+          - qcom,kpss-timer
+          - qcom,kpss-wdt
            - qcom,scss-timer
+      - const: qcom,kpss-timer
+      - const: qcom,kpss-wdt
+      - const: qcom,scss-timer

    reg:
      maxItems: 1

Thanks,
Sai
Rob Herring Feb. 11, 2020, 6:24 p.m. UTC | #4
On Fri, Feb 7, 2020 at 12:10 AM Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> Hi Rob,
>
> On 2020-02-07 00:08, Rob Herring wrote:
> > On Sat, Feb 01, 2020 at 08:59:49PM +0530, Sai Prakash Ranjan wrote:
> >> Add missing compatible for watchdog timer on QCS404,
> >> SC7180, SDM845 and SM8150 SoCs.
> >
> > That's not what the commit does. You are changing what's valid.
> >
> > One string was valid, now 2 are required.
> >
>
> Does this look good?

No. First of all, what's the base for the diff? It's not what you
originally had nor incremental on top of this patch.

Second, a value of 'qcom,kpss-timer' or 'qcom,kpss-wdt' or
'qcom,scss-timer' will fail validation because 2 clauses of 'oneOf'
will be true.

>
> diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> index 46d6aad5786a..3378244b67cd 100644
> --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> @@ -14,19 +14,22 @@ allOf:
>
>   properties:
>     compatible:
> -    items:
> +    oneOf:
>         - enum:
>             - qcom,apss-wdt-qcs404
>             - qcom,apss-wdt-sc7180
>             - qcom,apss-wdt-sdm845
>             - qcom,apss-wdt-sm8150
> -          - qcom,kpss-timer
> -          - qcom,kpss-wdt
>             - qcom,kpss-wdt-apq8064
>             - qcom,kpss-wdt-ipq4019
>             - qcom,kpss-wdt-ipq8064
>             - qcom,kpss-wdt-msm8960
> +          - qcom,kpss-timer
> +          - qcom,kpss-wdt
>             - qcom,scss-timer
> +      - const: qcom,kpss-timer
> +      - const: qcom,kpss-wdt
> +      - const: qcom,scss-timer
>
>     reg:
>       maxItems: 1
>
> Thanks,
> Sai
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> member
> of Code Aurora Forum, hosted by The Linux Foundation
Sai Prakash Ranjan Feb. 11, 2020, 9:57 p.m. UTC | #5
On 2020-02-11 23:54, Rob Herring wrote:
> On Fri, Feb 7, 2020 at 12:10 AM Sai Prakash Ranjan
> <saiprakash.ranjan@codeaurora.org> wrote:
>> 
>> Hi Rob,
>> 
>> On 2020-02-07 00:08, Rob Herring wrote:
>> > On Sat, Feb 01, 2020 at 08:59:49PM +0530, Sai Prakash Ranjan wrote:
>> >> Add missing compatible for watchdog timer on QCS404,
>> >> SC7180, SDM845 and SM8150 SoCs.
>> >
>> > That's not what the commit does. You are changing what's valid.
>> >
>> > One string was valid, now 2 are required.
>> >
>> 
>> Does this look good?
> 
> No. First of all, what's the base for the diff? It's not what you
> originally had nor incremental on top of this patch.
> 

It was an incremental on top of this patch.

> Second, a value of 'qcom,kpss-timer' or 'qcom,kpss-wdt' or
> 'qcom,scss-timer' will fail validation because 2 clauses of 'oneOf'
> will be true.
> 

I will just remove oneOf and add the missing compatibles to the enum.

Thanks,
Sai
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
index 5448cc537a03..7180c64f54fb 100644
--- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
@@ -14,14 +14,19 @@  allOf:
 
 properties:
   compatible:
-    enum:
-      - qcom,kpss-timer
-      - qcom,kpss-wdt
-      - qcom,kpss-wdt-apq8064
-      - qcom,kpss-wdt-ipq4019
-      - qcom,kpss-wdt-ipq8064
-      - qcom,kpss-wdt-msm8960
-      - qcom,scss-timer
+    items:
+      - enum:
+          - qcom,apss-wdt-qcs404
+          - qcom,apss-wdt-sc7180
+          - qcom,apss-wdt-sdm845
+          - qcom,apss-wdt-sm8150
+          - qcom,kpss-timer
+          - qcom,kpss-wdt-apq8064
+          - qcom,kpss-wdt-ipq4019
+          - qcom,kpss-wdt-ipq8064
+          - qcom,kpss-wdt-msm8960
+          - qcom,scss-timer
+      - const: qcom,kpss-wdt
 
   reg:
     maxItems: 1