Changes since v3:
- Drop items in compatible field.
- Replace items with maxItems in reset field.
- Replace maxItems with items: -description in if else reset field
Changes since v2:
- Express JH8100 compatibility to JH7110 in dt-bindings.
- Rework min/maxItems constraint for JH8100 resets property.
Changes since v1:
- Drop "starfive,jh8100-wdt" compatible field in starfive-wdt.c,
and express them in dt-bindings.
- Use minItems in resets field to cater for single reset signal
in JH8100.
- Reword Watchdog reset to Core reset for JH8100.
StarFive's JH8100 watchdog reuses JH7100 register mapping.
DT-binding of JH7100 watchdog is extended to support JH8100.
Since JH8100 only uses 1 reset signal, update the binding to
support one reset for "starfive,jh8100-wdt" compatible.
Ji Sheng Teoh (1):
dt-bindings: watchdog: starfive,jh7100-wdt: Add compatible for JH8100
.../watchdog/starfive,jh7100-wdt.yaml | 29 +++++++++++++++----
1 file changed, 23 insertions(+), 6 deletions(-)
--
2.25.1
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 <[email protected]>
Signed-off-by: Ji Sheng Teoh <[email protected]>
---
.../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
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:
--
2.25.1
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 <[email protected]>
> Signed-off-by: Ji Sheng Teoh <[email protected]>
> ---
> .../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 <[email protected]> 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 <[email protected]>
> > Signed-off-by: Ji Sheng Teoh <[email protected]>
> > ---
> > .../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/[email protected]/
> 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
On Mon, 18 Dec 2023 15:41:37 +0100
Krzysztof Kozlowski <[email protected]> wrote:
> 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.
>
Will submit it with DTS once things have cleared up.
Thanks for the comment.
> Best regards,
> Krzysztof
>
On Mon, Dec 18, 2023 at 11:37:38PM +0800, Ji Sheng Teoh wrote:
> On Mon, 18 Dec 2023 15:41:37 +0100
> Krzysztof Kozlowski <[email protected]> wrote:
>
> > 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.
> >
> Will submit it with DTS once things have cleared up.
> Thanks for the comment.
[1] is not going to applied for a while since the SoC doesn't actually
exist yet and is pre-tapeout on an FPGA. I would just send the dts patch
adding the watchdog alongside the series, or else you'll be waiting for
quite a while. Or even link to the node on github or whatever.
On Tue, 19 Dec 2023 15:43:07 +0000
Conor Dooley <[email protected]> wrote:
> On Mon, Dec 18, 2023 at 11:37:38PM +0800, Ji Sheng Teoh wrote:
> > On Mon, 18 Dec 2023 15:41:37 +0100
> > Krzysztof Kozlowski <[email protected]> wrote:
> >
> > > 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.
> > Will submit it with DTS once things have cleared up.
> > Thanks for the comment.
>
> [1] is not going to applied for a while since the SoC doesn't actually
> exist yet and is pre-tapeout on an FPGA. I would just send the dts
> patch adding the watchdog alongside the series, or else you'll be
> waiting for quite a while. Or even link to the node on github or
> whatever.
>
Ok, will add the watchdog dts alongside this series and mention its
dependency on [1]. Thanks.