| Message ID | 20231216014850.1011344-2-jisheng.teoh@starfivetech.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add StarFive JH8100 watchdog | expand |
On 16/12/2023 02:48, Ji Sheng Teoh wrote: > Add "starfive,jh8100-wdt" compatible string for StarFive's JH8100 > watchdog. > Since JH8100 watchdog only has 1 reset signal, update binding > document to support one reset for "starfive,jh8100-wdt" compatible. > > Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com> > Signed-off-by: Ji Sheng Teoh <jisheng.teoh@starfivetech.com> > --- > .../watchdog/starfive,jh7100-wdt.yaml | 29 +++++++++++++++---- > 1 file changed, 23 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/watchdog/starfive,jh7100-wdt.yaml b/Documentation/devicetree/bindings/watchdog/starfive,jh7100-wdt.yaml > index 68f3f6fd08a6..ab077f64a83e 100644 > --- a/Documentation/devicetree/bindings/watchdog/starfive,jh7100-wdt.yaml > +++ b/Documentation/devicetree/bindings/watchdog/starfive,jh7100-wdt.yaml > @@ -19,14 +19,12 @@ description: > isn't cleared, the watchdog will reset the system unless the watchdog > reset is disabled. > > -allOf: > - - $ref: watchdog.yaml# > - > properties: > compatible: > enum: > - starfive,jh7100-wdt > - starfive,jh7110-wdt > + - starfive,jh8100-wdt What is happening with this patchset? I asked about one specific items. you know - comment is written under specific inline quopte. You wrote in changelog "Drop items in compatible field.", but I see oneOf gone! I have real doubts that you ever tested your entire solution with this binding. Where is the DTS? Best regards, Krzysztof
On Mon, 18 Dec 2023 09:41:07 +0100 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 16/12/2023 02:48, Ji Sheng Teoh wrote: > > Add "starfive,jh8100-wdt" compatible string for StarFive's JH8100 > > watchdog. > > Since JH8100 watchdog only has 1 reset signal, update binding > > document to support one reset for "starfive,jh8100-wdt" compatible. > > > > Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com> > > Signed-off-by: Ji Sheng Teoh <jisheng.teoh@starfivetech.com> > > --- > > .../watchdog/starfive,jh7100-wdt.yaml | 29 > > +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/watchdog/starfive,jh7100-wdt.yaml > > b/Documentation/devicetree/bindings/watchdog/starfive,jh7100-wdt.yaml > > index 68f3f6fd08a6..ab077f64a83e 100644 --- > > a/Documentation/devicetree/bindings/watchdog/starfive,jh7100-wdt.yaml > > +++ > > b/Documentation/devicetree/bindings/watchdog/starfive,jh7100-wdt.yaml > > @@ -19,14 +19,12 @@ description: isn't cleared, the watchdog will > > reset the system unless the watchdog reset is disabled. -allOf: > > - - $ref: watchdog.yaml# > > - > > properties: > > compatible: > > enum: > > - starfive,jh7100-wdt > > - starfive,jh7110-wdt > > + - starfive,jh8100-wdt > > What is happening with this patchset? I asked about one specific > items. you know - comment is written under specific inline quopte. > You wrote in changelog "Drop items in compatible field.", but I see > oneOf gone! My bad, I've interpreted it wrongly. Will rework the compatible field to this instead: compatible: oneOf: - enum: - starfive,jh7100-wdt - starfive,jh7110-wdt - items: - enum: - starfive,jh8100-wdt - const: starfive,jh7110-wdt While reworking this, I've noticed reset field with maxItems alone will cause dt_binding_check to fail with following error: 'watchdog@12270000: resets: [[4294967295, 15]] is too short' This is fixed by defining minItems and maxItems as follow: resets: minItems: 1 maxItems: 2 > > I have real doubts that you ever tested your entire solution with this > binding. Where is the DTS? > Currently, the DTS is still in internal and yet to upstream as it depends on [1]. [1] https://lore.kernel.org/all/20231201121410.95298-1-jeeheng.sia@starfivetech.com/ > Best regards, > Krzysztof >
On 18/12/2023 15:27, Ji Sheng Teoh wrote: >> >> I have real doubts that you ever tested your entire solution with this >> binding. Where is the DTS? >> > > Currently, the DTS is still in internal and yet to upstream as it depends > on [1]. Yeah, so you send untested code which cannot work or pass tests. If you do not test your code, we need to be able to at least verify it, so send your DTS. Otherwise I cannot trust that this works at all. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/watchdog/starfive,jh7100-wdt.yaml b/Documentation/devicetree/bindings/watchdog/starfive,jh7100-wdt.yaml index 68f3f6fd08a6..ab077f64a83e 100644 --- a/Documentation/devicetree/bindings/watchdog/starfive,jh7100-wdt.yaml +++ b/Documentation/devicetree/bindings/watchdog/starfive,jh7100-wdt.yaml @@ -19,14 +19,12 @@ description: isn't cleared, the watchdog will reset the system unless the watchdog reset is disabled. -allOf: - - $ref: watchdog.yaml# - properties: compatible: enum: - starfive,jh7100-wdt - starfive,jh7110-wdt + - starfive,jh8100-wdt reg: maxItems: 1 @@ -45,9 +43,7 @@ properties: - const: core resets: - items: - - description: APB reset - - description: Core reset + maxItems: 2 required: - compatible @@ -56,6 +52,27 @@ required: - clock-names - resets +allOf: + - $ref: watchdog.yaml# + + - if: + properties: + compatible: + contains: + enum: + - starfive,jh8100-wdt + then: + properties: + resets: + items: + - description: Core reset + else: + properties: + resets: + items: + - description: APB reset + - description: Core reset + unevaluatedProperties: false examples: