2015-11-16 11:14:18

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH RESEND 0/8] {arm|arm64}: berlin: add watchdog support

In Marvell berlin SoCs, there are watchdogs which are compatible with the
snps,dw-wdt driver. This series try to add watchdog support for them.

Jisheng Zhang (8):
arm: dts: berlin2q: add watchdog nodes
arm: dts: berlin2cd: add watchdog nodes
arm: dts: berlin2: add watchdog nodes
arm64: dts: berlin4ct: add watchdog nodes
arm: dts: berlin: enable wdt0 on Sony NSZ-GS7
ARM: dts: berlin: enable wdt0 on the Google Chromecast
arm: dts: berlin: enable wdt0 on the Marvell BG2Q DMP
arm64: dts: berlin: enable wdt0 on the Marvell BG4CT STB board

arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts | 2 ++
arch/arm/boot/dts/berlin2.dtsi | 24 +++++++++++++++++++++++
arch/arm/boot/dts/berlin2cd-google-chromecast.dts | 2 ++
arch/arm/boot/dts/berlin2cd.dtsi | 24 +++++++++++++++++++++++
arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 4 ++++
arch/arm/boot/dts/berlin2q.dtsi | 24 +++++++++++++++++++++++
arch/arm64/boot/dts/marvell/berlin4ct-stb.dts | 4 ++++
arch/arm64/boot/dts/marvell/berlin4ct.dtsi | 24 +++++++++++++++++++++++
8 files changed, 108 insertions(+)

--
2.6.1


2015-11-16 11:16:19

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH RESEND 1/8] arm: dts: berlin2q: add watchdog nodes

The Marvell Berlin BG2Q has 3 watchdogs which are compatible with the
snps,dw-wdt driver sit in the sysmgr domain. This patch adds the
corresponding device tree nodes.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/arm/boot/dts/berlin2q.dtsi | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index a3ecde5..fac4315 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -483,6 +483,30 @@
ranges = <0 0xfc0000 0x10000>;
interrupt-parent = <&sic>;

+ wdt0: watchdog@1000 {
+ compatible = "snps,dw-wdt";
+ reg = <0x1000 0x100>;
+ clocks = <&refclk>;
+ interrupts = <0>;
+ status = "disabled";
+ };
+
+ wdt1: watchdog@2000 {
+ compatible = "snps,dw-wdt";
+ reg = <0x2000 0x100>;
+ clocks = <&refclk>;
+ interrupts = <1>;
+ status = "disabled";
+ };
+
+ wdt2: watchdog@3000 {
+ compatible = "snps,dw-wdt";
+ reg = <0x3000 0x100>;
+ clocks = <&refclk>;
+ interrupts = <2>;
+ status = "disabled";
+ };
+
sm_gpio1: gpio@5000 {
compatible = "snps,dw-apb-gpio";
reg = <0x5000 0x400>;
--
2.6.1

2015-11-16 11:14:27

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH RESEND 2/8] arm: dts: berlin2cd: add watchdog nodes

The Marvell Berlin BG2CD has 3 watchdogs which are compatible with the
snps,dw-wdt driver sit in the sysmgr domain. This patch adds the
corresponding device tree nodes.

NOTE: although BG2CD doesn't have a HW sysmgr, but the sysmgr domain
exists.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/arm/boot/dts/berlin2cd.dtsi | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2cd.dtsi b/arch/arm/boot/dts/berlin2cd.dtsi
index 900213d..9108180 100644
--- a/arch/arm/boot/dts/berlin2cd.dtsi
+++ b/arch/arm/boot/dts/berlin2cd.dtsi
@@ -376,6 +376,30 @@
ranges = <0 0xfc0000 0x10000>;
interrupt-parent = <&sic>;

+ wdt0: watchdog@1000 {
+ compatible = "snps,dw-wdt";
+ reg = <0x1000 0x100>;
+ clocks = <&refclk>;
+ interrupts = <0>;
+ status = "disabled";
+ };
+
+ wdt1: watchdog@2000 {
+ compatible = "snps,dw-wdt";
+ reg = <0x2000 0x100>;
+ clocks = <&refclk>;
+ interrupts = <1>;
+ status = "disabled";
+ };
+
+ wdt2: watchdog@3000 {
+ compatible = "snps,dw-wdt";
+ reg = <0x3000 0x100>;
+ clocks = <&refclk>;
+ interrupts = <2>;
+ status = "disabled";
+ };
+
sm_gpio1: gpio@5000 {
compatible = "snps,dw-apb-gpio";
reg = <0x5000 0x400>;
--
2.6.1

2015-11-16 11:14:24

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH RESEND 3/8] arm: dts: berlin2: add watchdog nodes

The Marvell Berlin BG2 has 3 watchdogs which are compatible with the
snps,dw-wdt driver sit in the sysmgr domain. This patch adds the
corresponding device tree nodes.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/arm/boot/dts/berlin2.dtsi | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi
index ef811de..7e35de2 100644
--- a/arch/arm/boot/dts/berlin2.dtsi
+++ b/arch/arm/boot/dts/berlin2.dtsi
@@ -412,6 +412,30 @@
ranges = <0 0xfc0000 0x10000>;
interrupt-parent = <&sic>;

+ wdt0: watchdog@1000 {
+ compatible = "snps,dw-wdt";
+ reg = <0x1000 0x100>;
+ clocks = <&refclk>;
+ interrupts = <0>;
+ status = "disabled";
+ };
+
+ wdt1: watchdog@2000 {
+ compatible = "snps,dw-wdt";
+ reg = <0x2000 0x100>;
+ clocks = <&refclk>;
+ interrupts = <1>;
+ status = "disabled";
+ };
+
+ wdt2: watchdog@3000 {
+ compatible = "snps,dw-wdt";
+ reg = <0x3000 0x100>;
+ clocks = <&refclk>;
+ interrupts = <2>;
+ status = "disabled";
+ };
+
sm_gpio1: gpio@5000 {
compatible = "snps,dw-apb-gpio";
reg = <0x5000 0x400>;
--
2.6.1

2015-11-16 11:15:52

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH RESEND 4/8] arm64: dts: berlin4ct: add watchdog nodes

The Marvell Berlin BG4CT has 3 watchdogs which are compatible with the
snps,dw-wdt driver sit in the sysmgr domain. This patch adds the
corresponding device tree nodes.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/arm64/boot/dts/marvell/berlin4ct.dtsi | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/berlin4ct.dtsi b/arch/arm64/boot/dts/marvell/berlin4ct.dtsi
index a3b5f1d..8eddf10 100644
--- a/arch/arm64/boot/dts/marvell/berlin4ct.dtsi
+++ b/arch/arm64/boot/dts/marvell/berlin4ct.dtsi
@@ -241,6 +241,30 @@
interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
};

+ wdt0: watchdog@3000 {
+ compatible = "snps,dw-wdt";
+ reg = <0x3000 0x100>;
+ clocks = <&osc>;
+ interrupts = <0>;
+ status = "disabled";
+ };
+
+ wdt1: watchdog@4000 {
+ compatible = "snps,dw-wdt";
+ reg = <0x4000 0x100>;
+ clocks = <&osc>;
+ interrupts = <1>;
+ status = "disabled";
+ };
+
+ wdt2: watchdog@5000 {
+ compatible = "snps,dw-wdt";
+ reg = <0x5000 0x100>;
+ clocks = <&osc>;
+ interrupts = <2>;
+ status = "disabled";
+ };
+
sm_gpio0: gpio@8000 {
compatible = "snps,dw-apb-gpio";
reg = <0x8000 0x400>;
--
2.6.1

2015-11-16 11:14:30

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH RESEND 5/8] arm: dts: berlin: enable wdt0 on Sony NSZ-GS7

Enable wdt0 on Marvell Berlin BG2 based Sony NSZ-GS7 to make use of the
watchdog functionality.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts b/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts
index 5c99fb3..f3c53b3 100644
--- a/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts
+++ b/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts
@@ -71,3 +71,5 @@
};

&uart0 { status = "okay"; };
+
+&wdt0 { status = "okay"; };
--
2.6.1

2015-11-16 11:14:38

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH RESEND 6/8] ARM: dts: berlin: enable wdt0 on the Google Chromecast

Enable wdt0 on the Google Chromecast to make use of the watchdog
functionality.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/arm/boot/dts/berlin2cd-google-chromecast.dts | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2cd-google-chromecast.dts b/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
index 772165a..a17c850 100644
--- a/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
+++ b/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
@@ -85,3 +85,5 @@
&usb_phy1 { status = "okay"; };

&usb1 { status = "okay"; };
+
+&wdt0 { status = "okay"; };
--
2.6.1

2015-11-16 11:14:35

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH RESEND 7/8] arm: dts: berlin: enable wdt0 on the Marvell BG2Q DMP

Enable wdt0 on the Marvell BG2Q DMP board to make use of the watchdog
functionality.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
index 0fa8f1e..e60f53a 100644
--- a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
+++ b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
@@ -139,3 +139,7 @@
&sata_phy {
status = "okay";
};
+
+&wdt0 {
+ status = "okay";
+};
--
2.6.1

2015-11-16 11:14:43

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH RESEND 8/8] arm64: dts: berlin: enable wdt0 on the Marvell BG4CT STB board

Enable wdt0 on the Marvell BG4CT STB board to make use of the watchdog
functionality.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/arm64/boot/dts/marvell/berlin4ct-stb.dts | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/berlin4ct-stb.dts b/arch/arm64/boot/dts/marvell/berlin4ct-stb.dts
index 348c37e..4f0a873 100644
--- a/arch/arm64/boot/dts/marvell/berlin4ct-stb.dts
+++ b/arch/arm64/boot/dts/marvell/berlin4ct-stb.dts
@@ -64,3 +64,7 @@
&uart0 {
status = "okay";
};
+
+&wdt0 {
+ status = "okay";
+};
--
2.6.1

2015-11-19 20:47:11

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH RESEND 1/8] arm: dts: berlin2q: add watchdog nodes

On 16.11.2015 12:09, Jisheng Zhang wrote:
> The Marvell Berlin BG2Q has 3 watchdogs which are compatible with the
> snps,dw-wdt driver sit in the sysmgr domain. This patch adds the
> corresponding device tree nodes.
>
> Signed-off-by: Jisheng Zhang <[email protected]>
> ---
> arch/arm/boot/dts/berlin2q.dtsi | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
> index a3ecde5..fac4315 100644
> --- a/arch/arm/boot/dts/berlin2q.dtsi
> +++ b/arch/arm/boot/dts/berlin2q.dtsi
> @@ -483,6 +483,30 @@
> ranges = <0 0xfc0000 0x10000>;
> interrupt-parent = <&sic>;
>
> + wdt0: watchdog@1000 {
> + compatible = "snps,dw-wdt";
> + reg = <0x1000 0x100>;
> + clocks = <&refclk>;
> + interrupts = <0>;
> + status = "disabled";

Jisheng,

as the watchdogs are internal and cannot be clock gated
at all, how about we remove the status = "disabled" and
make them always available?

I have applied patches 1-4 with the status property removed.
This also renders patches 5-8 useless.

So, for now tentatively

Appled to berlin/dt and berlin64/dt respectivly

with status property removed.

Sebastian

2015-11-20 03:38:43

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH RESEND 1/8] arm: dts: berlin2q: add watchdog nodes

On Thu, 19 Nov 2015 21:47:05 +0100
Sebastian Hesselbarth wrote:

> On 16.11.2015 12:09, Jisheng Zhang wrote:
> > The Marvell Berlin BG2Q has 3 watchdogs which are compatible with the
> > snps,dw-wdt driver sit in the sysmgr domain. This patch adds the
> > corresponding device tree nodes.
> >
> > Signed-off-by: Jisheng Zhang <[email protected]>
> > ---
> > arch/arm/boot/dts/berlin2q.dtsi | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
> > index a3ecde5..fac4315 100644
> > --- a/arch/arm/boot/dts/berlin2q.dtsi
> > +++ b/arch/arm/boot/dts/berlin2q.dtsi
> > @@ -483,6 +483,30 @@
> > ranges = <0 0xfc0000 0x10000>;
> > interrupt-parent = <&sic>;
> >
> > + wdt0: watchdog@1000 {
> > + compatible = "snps,dw-wdt";
> > + reg = <0x1000 0x100>;
> > + clocks = <&refclk>;
> > + interrupts = <0>;
> > + status = "disabled";
>
> Jisheng,
>
> as the watchdogs are internal and cannot be clock gated
> at all, how about we remove the status = "disabled" and
> make them always available?

there are two issues here:

1. the dw-wdt can't support multiple variants now. I have rewrite the driver
with watchdog core supplied framework, but the patch isn't sent out and
may be need time to clean up and review.

2. not all dw-wdt devices are available and functional. This depends on
board design and configuration.

So IMHO status=disabled and patch5-8 is necessary, what do you think?

Thanks a lot for review,
Jisheng

>
> I have applied patches 1-4 with the status property removed.
> This also renders patches 5-8 useless.
>
> So, for now tentatively
>
> Appled to berlin/dt and berlin64/dt respectivly
>
> with status property removed.
>
> Sebastian
>

2015-11-20 20:19:52

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH RESEND 1/8] arm: dts: berlin2q: add watchdog nodes

On 20.11.2015 04:34, Jisheng Zhang wrote:
> On Thu, 19 Nov 2015 21:47:05 +0100
> Sebastian Hesselbarth wrote:
>> On 16.11.2015 12:09, Jisheng Zhang wrote:
>>> The Marvell Berlin BG2Q has 3 watchdogs which are compatible with the
>>> snps,dw-wdt driver sit in the sysmgr domain. This patch adds the
>>> corresponding device tree nodes.
>>>
>>> Signed-off-by: Jisheng Zhang <[email protected]>
>>> ---
>>> arch/arm/boot/dts/berlin2q.dtsi | 24 ++++++++++++++++++++++++
>>> 1 file changed, 24 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
>>> index a3ecde5..fac4315 100644
>>> --- a/arch/arm/boot/dts/berlin2q.dtsi
>>> +++ b/arch/arm/boot/dts/berlin2q.dtsi
>>> @@ -483,6 +483,30 @@
>>> ranges = <0 0xfc0000 0x10000>;
>>> interrupt-parent = <&sic>;
>>>
>>> + wdt0: watchdog@1000 {
>>> + compatible = "snps,dw-wdt";
>>> + reg = <0x1000 0x100>;
>>> + clocks = <&refclk>;
>>> + interrupts = <0>;
>>> + status = "disabled";
>>
>> as the watchdogs are internal and cannot be clock gated
>> at all, how about we remove the status = "disabled" and
>> make them always available?
>
> there are two issues here:
>
> 1. the dw-wdt can't support multiple variants now. I have rewrite the driver
> with watchdog core supplied framework, but the patch isn't sent out and
> may be need time to clean up and review.

Ok.

> 2. not all dw-wdt devices are available and functional. This depends on
> board design and configuration.

I understand that "board design and configuration" may hinder the wdt
to issue a hard reset. But all others are able to issue a soft reset
or just an interrupt, right?

So, I still don't see why we should disable wdt nodes by default
except for the driver issue above.

> So IMHO status=disabled and patch5-8 is necessary, what do you think?

No. I'd agree to enable wdt0 by default and leave wdt[1,2] disabled
because of the driver issue. Patches 5-8 only enable wdt0 anyway.

As soon as the driver issue is resolved, we enable all wdt nodes
unconditionally.

Sebastian

>> I have applied patches 1-4 with the status property removed.
>> This also renders patches 5-8 useless.
>>
>> So, for now tentatively
>>
>> Appled to berlin/dt and berlin64/dt respectivly
>>
>> with status property removed.
>>
>> Sebastian
>>
>

2015-11-23 05:04:11

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH RESEND 1/8] arm: dts: berlin2q: add watchdog nodes

On Fri, 20 Nov 2015 21:19:46 +0100
Sebastian Hesselbarth wrote:

> On 20.11.2015 04:34, Jisheng Zhang wrote:
> > On Thu, 19 Nov 2015 21:47:05 +0100
> > Sebastian Hesselbarth wrote:
> >> On 16.11.2015 12:09, Jisheng Zhang wrote:
> >>> The Marvell Berlin BG2Q has 3 watchdogs which are compatible with the
> >>> snps,dw-wdt driver sit in the sysmgr domain. This patch adds the
> >>> corresponding device tree nodes.
> >>>
> >>> Signed-off-by: Jisheng Zhang <[email protected]>
> >>> ---
> >>> arch/arm/boot/dts/berlin2q.dtsi | 24 ++++++++++++++++++++++++
> >>> 1 file changed, 24 insertions(+)
> >>>
> >>> diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
> >>> index a3ecde5..fac4315 100644
> >>> --- a/arch/arm/boot/dts/berlin2q.dtsi
> >>> +++ b/arch/arm/boot/dts/berlin2q.dtsi
> >>> @@ -483,6 +483,30 @@
> >>> ranges = <0 0xfc0000 0x10000>;
> >>> interrupt-parent = <&sic>;
> >>>
> >>> + wdt0: watchdog@1000 {
> >>> + compatible = "snps,dw-wdt";
> >>> + reg = <0x1000 0x100>;
> >>> + clocks = <&refclk>;
> >>> + interrupts = <0>;
> >>> + status = "disabled";
> >>
> >> as the watchdogs are internal and cannot be clock gated
> >> at all, how about we remove the status = "disabled" and
> >> make them always available?
> >
> > there are two issues here:
> >
> > 1. the dw-wdt can't support multiple variants now. I have rewrite the driver
> > with watchdog core supplied framework, but the patch isn't sent out and
> > may be need time to clean up and review.
>
> Ok.
>
> > 2. not all dw-wdt devices are available and functional. This depends on
> > board design and configuration.
>
> I understand that "board design and configuration" may hinder the wdt
> to issue a hard reset. But all others are able to issue a soft reset
> or just an interrupt, right?
>
> So, I still don't see why we should disable wdt nodes by default
> except for the driver issue above.
>
> > So IMHO status=disabled and patch5-8 is necessary, what do you think?
>
> No. I'd agree to enable wdt0 by default and leave wdt[1,2] disabled
> because of the driver issue. Patches 5-8 only enable wdt0 anyway.

That's fine.

>
> As soon as the driver issue is resolved, we enable all wdt nodes
> unconditionally.

I will submit patch for the wdt driver and hope it will be merged
in v4.5.

Thanks,
Jisheng

>
> Sebastian
>
> >> I have applied patches 1-4 with the status property removed.
> >> This also renders patches 5-8 useless.
> >>
> >> So, for now tentatively
> >>
> >> Appled to berlin/dt and berlin64/dt respectivly
> >>
> >> with status property removed.
> >>
> >> Sebastian
> >>
> >
>

2015-11-28 11:36:25

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH RESEND 1/8] arm: dts: berlin2q: add watchdog nodes

On 23.11.2015 05:59, Jisheng Zhang wrote:
> On Fri, 20 Nov 2015 21:19:46 +0100
> Sebastian Hesselbarth wrote:
>> On 20.11.2015 04:34, Jisheng Zhang wrote:
>>> On Thu, 19 Nov 2015 21:47:05 +0100
>>> Sebastian Hesselbarth wrote:
>>>> On 16.11.2015 12:09, Jisheng Zhang wrote:
>>>>> The Marvell Berlin BG2Q has 3 watchdogs which are compatible with the
>>>>> snps,dw-wdt driver sit in the sysmgr domain. This patch adds the
>>>>> corresponding device tree nodes.
>>>>>
>>>>> Signed-off-by: Jisheng Zhang <[email protected]>
>>>>> ---
[...]
>>>>> + wdt0: watchdog@1000 {
>>>>> + compatible = "snps,dw-wdt";
>>>>> + reg = <0x1000 0x100>;
>>>>> + clocks = <&refclk>;
>>>>> + interrupts = <0>;
>>>>> + status = "disabled";
>>>>
>>>> as the watchdogs are internal and cannot be clock gated
>>>> at all, how about we remove the status = "disabled" and
>>>> make them always available?
>>>
>>> there are two issues here:
>>>
>>> 1. the dw-wdt can't support multiple variants now. I have rewrite the driver
>>> with watchdog core supplied framework, but the patch isn't sent out and
>>> may be need time to clean up and review.
>>
>> Ok.
>>
>>> 2. not all dw-wdt devices are available and functional. This depends on
>>> board design and configuration.
>>
>> I understand that "board design and configuration" may hinder the wdt
>> to issue a hard reset. But all others are able to issue a soft reset
>> or just an interrupt, right?
>>
>> So, I still don't see why we should disable wdt nodes by default
>> except for the driver issue above.
>>
>>> So IMHO status=disabled and patch5-8 is necessary, what do you think?
>>
>> No. I'd agree to enable wdt0 by default and leave wdt[1,2] disabled
>> because of the driver issue. Patches 5-8 only enable wdt0 anyway.
>
> That's fine.

Jisheng,

I amended your SoC dtsi watchdog patches accordingly. wdt0 is
now always enabled, while the others are disabled.

So, with the changes Patches 1-4 applied to berlin/dt and berlin64/dt
respectively. Patches 5-8 dropped.

>> As soon as the driver issue is resolved, we enable all wdt nodes
>> unconditionally.
>
> I will submit patch for the wdt driver and hope it will be merged
> in v4.5.

Ok. Feel free to add a patch that removes the status disabled properties
again if berlin[64]/dt has already hit mainline in the meantime.

If not, keep me posted on the DW wdt patch outcome.

Thanks,

Sebastian

2015-11-30 13:04:17

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH RESEND 1/8] arm: dts: berlin2q: add watchdog nodes

On Sat, 28 Nov 2015 12:36:16 +0100
Sebastian Hesselbarth wrote:

> On 23.11.2015 05:59, Jisheng Zhang wrote:
> > On Fri, 20 Nov 2015 21:19:46 +0100
> > Sebastian Hesselbarth wrote:
> >> On 20.11.2015 04:34, Jisheng Zhang wrote:
> >>> On Thu, 19 Nov 2015 21:47:05 +0100
> >>> Sebastian Hesselbarth wrote:
> >>>> On 16.11.2015 12:09, Jisheng Zhang wrote:
> >>>>> The Marvell Berlin BG2Q has 3 watchdogs which are compatible with the
> >>>>> snps,dw-wdt driver sit in the sysmgr domain. This patch adds the
> >>>>> corresponding device tree nodes.
> >>>>>
> >>>>> Signed-off-by: Jisheng Zhang <[email protected]>
> >>>>> ---
> [...]
> >>>>> + wdt0: watchdog@1000 {
> >>>>> + compatible = "snps,dw-wdt";
> >>>>> + reg = <0x1000 0x100>;
> >>>>> + clocks = <&refclk>;
> >>>>> + interrupts = <0>;
> >>>>> + status = "disabled";
> >>>>
> >>>> as the watchdogs are internal and cannot be clock gated
> >>>> at all, how about we remove the status = "disabled" and
> >>>> make them always available?
> >>>
> >>> there are two issues here:
> >>>
> >>> 1. the dw-wdt can't support multiple variants now. I have rewrite the driver
> >>> with watchdog core supplied framework, but the patch isn't sent out and
> >>> may be need time to clean up and review.
> >>
> >> Ok.
> >>
> >>> 2. not all dw-wdt devices are available and functional. This depends on
> >>> board design and configuration.
> >>
> >> I understand that "board design and configuration" may hinder the wdt
> >> to issue a hard reset. But all others are able to issue a soft reset
> >> or just an interrupt, right?
> >>
> >> So, I still don't see why we should disable wdt nodes by default
> >> except for the driver issue above.
> >>
> >>> So IMHO status=disabled and patch5-8 is necessary, what do you think?
> >>
> >> No. I'd agree to enable wdt0 by default and leave wdt[1,2] disabled
> >> because of the driver issue. Patches 5-8 only enable wdt0 anyway.
> >
> > That's fine.
>
> Jisheng,
>
> I amended your SoC dtsi watchdog patches accordingly. wdt0 is
> now always enabled, while the others are disabled.
>
> So, with the changes Patches 1-4 applied to berlin/dt and berlin64/dt
> respectively. Patches 5-8 dropped.
>
> >> As soon as the driver issue is resolved, we enable all wdt nodes
> >> unconditionally.
> >
> > I will submit patch for the wdt driver and hope it will be merged
> > in v4.5.
>
> Ok. Feel free to add a patch that removes the status disabled properties
> again if berlin[64]/dt has already hit mainline in the meantime.

Got it. No problems.

>
> If not, keep me posted on the DW wdt patch outcome.

Will do. These days, I'm focusing on clk patches and some trivial dw-apb-timer
performance improvement patches. I will loop you in the mail when send out
dw wdt driver rewrite patch.

Thanks a lot,
Jisheng