Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 0/4] Allwinner H6 watchdog support
@ 2019-05-18 15:23 Clément Péron
  2019-05-18 15:23 ` [PATCH v3 1/4] dt-bindings: watchdog: add Allwinner H6 watchdog Clément Péron
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Clément Péron @ 2019-05-18 15:23 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Maxime Ripard, Chen-Yu Tsai
  Cc: linux-watchdog, devicetree, linux-arm-kernel, linux-kernel,
	Clément Péron

Hi,

Allwinner H6 SoC has two watchdogs.

As we are not sure that both A64 and H6 are stricly identical, I have
introduced the H6 bindings.

After investigation it seems that on some boards the first watchdog doesn't
make it properly reboot. Please see details in the commit log.

I think it's proper to add it with a comment anyway.

The r_watchdog is still available and usable on all the H6 boards.

Maybe it would be proper to introduce a "allwinner,sun50i-h6-r-wdt" bindings?

Thanks,
Clément

Changes since v2:
 - Reintroduce H6 bindings
 - Add watchdog Maintainters / ML
 - Add Martin Ayotte test results

Changes since v1:
 - Use A64 compatible instead of H6
 - Remove dt-bindings patch
 - Change watchdog status to disabled
 - Add r_watchdog node patch
 - Add enable sunxi watchdog patch

Clément Péron (4):
  dt-bindings: watchdog: add Allwinner H6 watchdog
  arm64: dts: allwinner: h6: add watchdog node
  arm64: dts: allwinner: h6: add r_watchog node
  arm64: defconfig: enable sunxi watchdog

 .../devicetree/bindings/watchdog/sunxi-wdt.txt | 10 ++++++----
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi   | 18 ++++++++++++++++++
 arch/arm64/configs/defconfig                   |  1 +
 3 files changed, 25 insertions(+), 4 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/4] dt-bindings: watchdog: add Allwinner H6 watchdog
  2019-05-18 15:23 [PATCH v3 0/4] Allwinner H6 watchdog support Clément Péron
@ 2019-05-18 15:23 ` Clément Péron
  2019-05-20  7:35   ` Maxime Ripard
  2019-05-18 15:23 ` [PATCH v3 2/4] arm64: dts: allwinner: h6: add watchdog node Clément Péron
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Clément Péron @ 2019-05-18 15:23 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Maxime Ripard, Chen-Yu Tsai
  Cc: linux-watchdog, devicetree, linux-arm-kernel, linux-kernel,
	Clément Péron

Allwinner H6 has a similar watchdog as the A64 which is already
a compatible of the A31.

This commit sort the lines and add the H6 compatible.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 .../devicetree/bindings/watchdog/sunxi-wdt.txt         | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
index 46055254e8dd..f4810f8ad1c5 100644
--- a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
@@ -3,10 +3,12 @@ Allwinner SoCs Watchdog timer
 Required properties:
 
 - compatible : should be one of
-	"allwinner,sun4i-a10-wdt"
-	"allwinner,sun6i-a31-wdt"
-	"allwinner,sun50i-a64-wdt","allwinner,sun6i-a31-wdt"
-	"allwinner,suniv-f1c100s-wdt", "allwinner,sun4i-a10-wdt"
+	- "allwinner,sun4i-a10-wdt"
+	- "allwinner,sun50i-a64-wdt","allwinner,sun6i-a31-wdt"
+	- "allwinner,sun50i-h6-wdt","allwinner,sun50i-a64-wdt",
+	  "allwinner,sun6i-a31-wdt"
+	- "allwinner,sun6i-a31-wdt"
+	- "allwinner,suniv-f1c100s-wdt", "allwinner,sun4i-a10-wdt"
 - reg : Specifies base physical address and size of the registers.
 
 Optional properties:
-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 2/4] arm64: dts: allwinner: h6: add watchdog node
  2019-05-18 15:23 [PATCH v3 0/4] Allwinner H6 watchdog support Clément Péron
  2019-05-18 15:23 ` [PATCH v3 1/4] dt-bindings: watchdog: add Allwinner H6 watchdog Clément Péron
@ 2019-05-18 15:23 ` Clément Péron
  2019-05-20  7:36   ` Maxime Ripard
  2019-05-18 15:23 ` [PATCH v3 3/4] arm64: dts: allwinner: h6: add r_watchog node Clément Péron
  2019-05-18 15:23 ` [PATCH v3 4/4] arm64: defconfig: enable sunxi watchdog Clément Péron
  3 siblings, 1 reply; 14+ messages in thread
From: Clément Péron @ 2019-05-18 15:23 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Maxime Ripard, Chen-Yu Tsai
  Cc: linux-watchdog, devicetree, linux-arm-kernel, linux-kernel,
	Clément Péron

Allwinner H6 has a watchog node which seems broken
on some boards.

Test has been performed on several boards.

Chen-Yu Tsai boards:
Pine H64 - H6448BA 7782 => OK
OrangePi Lite 2 - H8068BA 61C2 => KO

Martin Ayotte boards:
Pine H64 - H8069BA 6892 => OK
OrangePi 3 - HA047BA 69W2 => KO
OrangePi One Plus - H7310BA 6842 => KO
OrangePi Lite2 - H6448BA 6662 => KO

Clément Péron board:
Beelink GS1 - H7309BA 6842 => KO

As it seems not fixable for now, declare the node
but leave it disable with a comment.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 16c5c3d0fd81..60b47f23b2f5 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -208,6 +208,16 @@
 			reg = <0x03006000 0x400>;
 		};
 
+		watchdog: watchdog@30090a0 {
+			compatible = "allwinner,sun50i-h6-wdt",
+				     "allwinner,sun50i-a64-wdt",
+				     "allwinner,sun6i-a31-wdt";
+			reg = <0x030090a0 0x20>;
+			interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
+			/* Broken on some H6 boards */
+			status = "disabled";
+		};
+
 		pio: pinctrl@300b000 {
 			compatible = "allwinner,sun50i-h6-pinctrl";
 			reg = <0x0300b000 0x400>;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 3/4] arm64: dts: allwinner: h6: add r_watchog node
  2019-05-18 15:23 [PATCH v3 0/4] Allwinner H6 watchdog support Clément Péron
  2019-05-18 15:23 ` [PATCH v3 1/4] dt-bindings: watchdog: add Allwinner H6 watchdog Clément Péron
  2019-05-18 15:23 ` [PATCH v3 2/4] arm64: dts: allwinner: h6: add watchdog node Clément Péron
@ 2019-05-18 15:23 ` Clément Péron
  2019-05-20 15:19   ` Clément Péron
  2019-05-18 15:23 ` [PATCH v3 4/4] arm64: defconfig: enable sunxi watchdog Clément Péron
  3 siblings, 1 reply; 14+ messages in thread
From: Clément Péron @ 2019-05-18 15:23 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Maxime Ripard, Chen-Yu Tsai
  Cc: linux-watchdog, devicetree, linux-arm-kernel, linux-kernel,
	Clément Péron

Allwinner H6 has a r_watchdog similar to A64.

Declare it in the device-tree.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 60b47f23b2f5..27647e496269 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -632,6 +632,14 @@
 			#reset-cells = <1>;
 		};
 
+		r_watchdog: watchdog@7020400 {
+			compatible = "allwinner,sun50i-h6-wdt",
+				     "allwinner,sun50i-a64-wdt",
+				     "allwinner,sun6i-a31-wdt";
+			reg = <0x07020400 0x20>;
+			interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
 		r_intc: interrupt-controller@7021000 {
 			compatible = "allwinner,sun50i-h6-r-intc",
 				     "allwinner,sun6i-a31-r-intc";
-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 4/4] arm64: defconfig: enable sunxi watchdog
  2019-05-18 15:23 [PATCH v3 0/4] Allwinner H6 watchdog support Clément Péron
                   ` (2 preceding siblings ...)
  2019-05-18 15:23 ` [PATCH v3 3/4] arm64: dts: allwinner: h6: add r_watchog node Clément Péron
@ 2019-05-18 15:23 ` Clément Péron
  3 siblings, 0 replies; 14+ messages in thread
From: Clément Péron @ 2019-05-18 15:23 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Maxime Ripard, Chen-Yu Tsai
  Cc: linux-watchdog, devicetree, linux-arm-kernel, linux-kernel,
	Clément Péron

The SUNXI_WATCHDOG option is required to make the
watchdog available on Allwinner H6.

Enable this option as a module.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 4d583514258c..fc51dd4decb1 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -420,6 +420,7 @@ CONFIG_UNIPHIER_THERMAL=y
 CONFIG_WATCHDOG=y
 CONFIG_ARM_SP805_WATCHDOG=y
 CONFIG_S3C2410_WATCHDOG=y
+CONFIG_SUNXI_WATCHDOG=m
 CONFIG_IMX2_WDT=y
 CONFIG_MESON_GXBB_WATCHDOG=m
 CONFIG_MESON_WATCHDOG=m
-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] dt-bindings: watchdog: add Allwinner H6 watchdog
  2019-05-18 15:23 ` [PATCH v3 1/4] dt-bindings: watchdog: add Allwinner H6 watchdog Clément Péron
@ 2019-05-20  7:35   ` Maxime Ripard
  2019-05-20  8:14     ` Clément Péron
  0 siblings, 1 reply; 14+ messages in thread
From: Maxime Ripard @ 2019-05-20  7:35 UTC (permalink / raw)
  To: Clément Péron
  Cc: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, linux-watchdog, devicetree, linux-arm-kernel,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]

On Sat, May 18, 2019 at 05:23:52PM +0200, Clément Péron wrote:
> Allwinner H6 has a similar watchdog as the A64 which is already
> a compatible of the A31.
>
> This commit sort the lines and add the H6 compatible.
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  .../devicetree/bindings/watchdog/sunxi-wdt.txt         | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
> index 46055254e8dd..f4810f8ad1c5 100644
> --- a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
> @@ -3,10 +3,12 @@ Allwinner SoCs Watchdog timer
>  Required properties:
>
>  - compatible : should be one of
> -	"allwinner,sun4i-a10-wdt"
> -	"allwinner,sun6i-a31-wdt"
> -	"allwinner,sun50i-a64-wdt","allwinner,sun6i-a31-wdt"
> -	"allwinner,suniv-f1c100s-wdt", "allwinner,sun4i-a10-wdt"

That sorting was kind of intentional

> +	- "allwinner,sun4i-a10-wdt"
> +	- "allwinner,sun50i-a64-wdt","allwinner,sun6i-a31-wdt"
> +	- "allwinner,sun50i-h6-wdt","allwinner,sun50i-a64-wdt",
> +	  "allwinner,sun6i-a31-wdt"

Is there a reason to keep the A64 compatible?

Thanks,
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] arm64: dts: allwinner: h6: add watchdog node
  2019-05-18 15:23 ` [PATCH v3 2/4] arm64: dts: allwinner: h6: add watchdog node Clément Péron
@ 2019-05-20  7:36   ` Maxime Ripard
  2019-05-20  8:21     ` Clément Péron
  0 siblings, 1 reply; 14+ messages in thread
From: Maxime Ripard @ 2019-05-20  7:36 UTC (permalink / raw)
  To: Clément Péron
  Cc: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, linux-watchdog, devicetree, linux-arm-kernel,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 876 bytes --]

On Sat, May 18, 2019 at 05:23:53PM +0200, Clément Péron wrote:
> Allwinner H6 has a watchog node which seems broken
> on some boards.
>
> Test has been performed on several boards.
>
> Chen-Yu Tsai boards:
> Pine H64 - H6448BA 7782 => OK
> OrangePi Lite 2 - H8068BA 61C2 => KO
>
> Martin Ayotte boards:
> Pine H64 - H8069BA 6892 => OK
> OrangePi 3 - HA047BA 69W2 => KO
> OrangePi One Plus - H7310BA 6842 => KO
> OrangePi Lite2 - H6448BA 6662 => KO
>
> Clément Péron board:
> Beelink GS1 - H7309BA 6842 => KO
>
> As it seems not fixable for now, declare the node
> but leave it disable with a comment.
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>

If it doesn't work most boards, then why do we need to merge that
patch in the first place?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] dt-bindings: watchdog: add Allwinner H6 watchdog
  2019-05-20  7:35   ` Maxime Ripard
@ 2019-05-20  8:14     ` Clément Péron
  2019-05-20 14:42       ` Maxime Ripard
  0 siblings, 1 reply; 14+ messages in thread
From: Clément Péron @ 2019-05-20  8:14 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, linux-watchdog, devicetree, linux-arm-kernel,
	linux-kernel

Hi Maxime,

On Mon, 20 May 2019 at 09:35, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Sat, May 18, 2019 at 05:23:52PM +0200, Clément Péron wrote:
> > Allwinner H6 has a similar watchdog as the A64 which is already
> > a compatible of the A31.
> >
> > This commit sort the lines and add the H6 compatible.
> >
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> >  .../devicetree/bindings/watchdog/sunxi-wdt.txt         | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
> > index 46055254e8dd..f4810f8ad1c5 100644
> > --- a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
> > +++ b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
> > @@ -3,10 +3,12 @@ Allwinner SoCs Watchdog timer
> >  Required properties:
> >
> >  - compatible : should be one of
> > -     "allwinner,sun4i-a10-wdt"
> > -     "allwinner,sun6i-a31-wdt"
> > -     "allwinner,sun50i-a64-wdt","allwinner,sun6i-a31-wdt"
> > -     "allwinner,suniv-f1c100s-wdt", "allwinner,sun4i-a10-wdt"
>
> That sorting was kind of intentional
Arg indeed, I will remove it.


>
> > +     - "allwinner,sun4i-a10-wdt"
> > +     - "allwinner,sun50i-a64-wdt","allwinner,sun6i-a31-wdt"
> > +     - "allwinner,sun50i-h6-wdt","allwinner,sun50i-a64-wdt",
> > +       "allwinner,sun6i-a31-wdt"
>
> Is there a reason to keep the A64 compatible?
Yes, A64 and H6 has the exact same memory mapping looking at the datasheet.
So if there is an errata or a new feature for the A64, it should be
also compatible with the H6.
Which is not the case with A31 (WDT_KEY_FIELD is not preset)

Thanks,
Clement

>
> Thanks,
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] arm64: dts: allwinner: h6: add watchdog node
  2019-05-20  7:36   ` Maxime Ripard
@ 2019-05-20  8:21     ` Clément Péron
  2019-05-20 14:44       ` Maxime Ripard
  0 siblings, 1 reply; 14+ messages in thread
From: Clément Péron @ 2019-05-20  8:21 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, linux-watchdog, devicetree, linux-arm-kernel,
	linux-kernel

Hi,

On Mon, 20 May 2019 at 09:36, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Sat, May 18, 2019 at 05:23:53PM +0200, Clément Péron wrote:
> > Allwinner H6 has a watchog node which seems broken
> > on some boards.
> >
> > Test has been performed on several boards.
> >
> > Chen-Yu Tsai boards:
> > Pine H64 - H6448BA 7782 => OK
> > OrangePi Lite 2 - H8068BA 61C2 => KO
> >
> > Martin Ayotte boards:
> > Pine H64 - H8069BA 6892 => OK
> > OrangePi 3 - HA047BA 69W2 => KO
> > OrangePi One Plus - H7310BA 6842 => KO
> > OrangePi Lite2 - H6448BA 6662 => KO
> >
> > Clément Péron board:
> > Beelink GS1 - H7309BA 6842 => KO
> >
> > As it seems not fixable for now, declare the node
> > but leave it disable with a comment.
> >
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
>
> If it doesn't work most boards, then why do we need to merge that
> patch in the first place?

My personnal opinion, is that having the IP declared and disabled with
a comment saying "it's broken on some boards" in the device-tree is
better than not having at all.

This will explicit say "the IP exist but don't use it!".
Maybe some people with a functionnal board would like to explicitly
use it on their dts.

Again just my personnal opinion,
Thanks for the review,
Clément

>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] dt-bindings: watchdog: add Allwinner H6 watchdog
  2019-05-20  8:14     ` Clément Péron
@ 2019-05-20 14:42       ` Maxime Ripard
  2019-05-20 15:16         ` Clément Péron
  0 siblings, 1 reply; 14+ messages in thread
From: Maxime Ripard @ 2019-05-20 14:42 UTC (permalink / raw)
  To: Clément Péron
  Cc: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, linux-watchdog, devicetree, linux-arm-kernel,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 985 bytes --]

On Mon, May 20, 2019 at 10:14:10AM +0200, Clément Péron wrote:
> >
> > > +     - "allwinner,sun4i-a10-wdt"
> > > +     - "allwinner,sun50i-a64-wdt","allwinner,sun6i-a31-wdt"
> > > +     - "allwinner,sun50i-h6-wdt","allwinner,sun50i-a64-wdt",
> > > +       "allwinner,sun6i-a31-wdt"
> >
> > Is there a reason to keep the A64 compatible?
>
> Yes, A64 and H6 has the exact same memory mapping looking at the datasheet.
> So if there is an errata or a new feature for the A64, it should be
> also compatible with the H6.
> Which is not the case with A31 (WDT_KEY_FIELD is not preset)

The thing is, if you use those three compatibles, then you're saying
that it's ok for the OS to use first the H6 driver, then the A64
driver, and then the A31 driver.

If the A31 isn't compatible, then it shouldn't be listed there. And if
it is, then you can skip the A64 compatible.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] arm64: dts: allwinner: h6: add watchdog node
  2019-05-20  8:21     ` Clément Péron
@ 2019-05-20 14:44       ` Maxime Ripard
  2019-05-20 14:57         ` Chen-Yu Tsai
  0 siblings, 1 reply; 14+ messages in thread
From: Maxime Ripard @ 2019-05-20 14:44 UTC (permalink / raw)
  To: Clément Péron
  Cc: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, linux-watchdog, devicetree, linux-arm-kernel,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1543 bytes --]

On Mon, May 20, 2019 at 10:21:40AM +0200, Clément Péron wrote:
> Hi,
>
> On Mon, 20 May 2019 at 09:36, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Sat, May 18, 2019 at 05:23:53PM +0200, Clément Péron wrote:
> > > Allwinner H6 has a watchog node which seems broken
> > > on some boards.
> > >
> > > Test has been performed on several boards.
> > >
> > > Chen-Yu Tsai boards:
> > > Pine H64 - H6448BA 7782 => OK
> > > OrangePi Lite 2 - H8068BA 61C2 => KO
> > >
> > > Martin Ayotte boards:
> > > Pine H64 - H8069BA 6892 => OK
> > > OrangePi 3 - HA047BA 69W2 => KO
> > > OrangePi One Plus - H7310BA 6842 => KO
> > > OrangePi Lite2 - H6448BA 6662 => KO
> > >
> > > Clément Péron board:
> > > Beelink GS1 - H7309BA 6842 => KO
> > >
> > > As it seems not fixable for now, declare the node
> > > but leave it disable with a comment.
> > >
> > > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> >
> > If it doesn't work most boards, then why do we need to merge that
> > patch in the first place?
>
> My personnal opinion, is that having the IP declared and disabled with
> a comment saying "it's broken on some boards" in the device-tree is
> better than not having at all.
>
> This will explicit say "the IP exist but don't use it!".
> Maybe some people with a functionnal board would like to explicitly
> use it on their dts.

Yeah, that makes sense. Chen-Yu, any opinion on the matter?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] arm64: dts: allwinner: h6: add watchdog node
  2019-05-20 14:44       ` Maxime Ripard
@ 2019-05-20 14:57         ` Chen-Yu Tsai
  0 siblings, 0 replies; 14+ messages in thread
From: Chen-Yu Tsai @ 2019-05-20 14:57 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Clément Péron, Wim Van Sebroeck, Guenter Roeck,
	Rob Herring, Mark Rutland, linux-watchdog, devicetree,
	linux-arm-kernel, linux-kernel

On Mon, May 20, 2019 at 10:44 PM Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
>
> On Mon, May 20, 2019 at 10:21:40AM +0200, Clément Péron wrote:
> > Hi,
> >
> > On Mon, 20 May 2019 at 09:36, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Sat, May 18, 2019 at 05:23:53PM +0200, Clément Péron wrote:
> > > > Allwinner H6 has a watchog node which seems broken
> > > > on some boards.
> > > >
> > > > Test has been performed on several boards.
> > > >
> > > > Chen-Yu Tsai boards:
> > > > Pine H64 - H6448BA 7782 => OK
> > > > OrangePi Lite 2 - H8068BA 61C2 => KO
> > > >
> > > > Martin Ayotte boards:
> > > > Pine H64 - H8069BA 6892 => OK
> > > > OrangePi 3 - HA047BA 69W2 => KO
> > > > OrangePi One Plus - H7310BA 6842 => KO
> > > > OrangePi Lite2 - H6448BA 6662 => KO
> > > >
> > > > Clément Péron board:
> > > > Beelink GS1 - H7309BA 6842 => KO
> > > >
> > > > As it seems not fixable for now, declare the node
> > > > but leave it disable with a comment.
> > > >
> > > > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > >
> > > If it doesn't work most boards, then why do we need to merge that
> > > patch in the first place?
> >
> > My personnal opinion, is that having the IP declared and disabled with
> > a comment saying "it's broken on some boards" in the device-tree is
> > better than not having at all.
> >
> > This will explicit say "the IP exist but don't use it!".
> > Maybe some people with a functionnal board would like to explicitly
> > use it on their dts.
>
> Yeah, that makes sense. Chen-Yu, any opinion on the matter?

Works for me.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] dt-bindings: watchdog: add Allwinner H6 watchdog
  2019-05-20 14:42       ` Maxime Ripard
@ 2019-05-20 15:16         ` Clément Péron
  0 siblings, 0 replies; 14+ messages in thread
From: Clément Péron @ 2019-05-20 15:16 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, linux-watchdog, devicetree, linux-arm-kernel,
	linux-kernel

On Mon, 20 May 2019 at 16:43, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Mon, May 20, 2019 at 10:14:10AM +0200, Clément Péron wrote:
> > >
> > > > +     - "allwinner,sun4i-a10-wdt"
> > > > +     - "allwinner,sun50i-a64-wdt","allwinner,sun6i-a31-wdt"
> > > > +     - "allwinner,sun50i-h6-wdt","allwinner,sun50i-a64-wdt",
> > > > +       "allwinner,sun6i-a31-wdt"
> > >
> > > Is there a reason to keep the A64 compatible?
> >
> > Yes, A64 and H6 has the exact same memory mapping looking at the datasheet.
> > So if there is an errata or a new feature for the A64, it should be
> > also compatible with the H6.
> > Which is not the case with A31 (WDT_KEY_FIELD is not preset)
>
> The thing is, if you use those three compatibles, then you're saying
> that it's ok for the OS to use first the H6 driver, then the A64
> driver, and then the A31 driver.
>
> If the A31 isn't compatible, then it shouldn't be listed there. And if
> it is, then you can skip the A64 compatible.

Hi Maxime,

I'm just supposing that A31 is the version 1.0 of the IP, A64 is the
version 1.1 and H6 is 1.2.
And if an issue is found for A64 there is more chance that we will
have to fix it also for H6.
But bindings the driver with :
"allwinner,sun50i-h6-wdt","allwinner,sun50i-a31-wdt" is also fine for
me.

Regards,
Clement
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/4] arm64: dts: allwinner: h6: add r_watchog node
  2019-05-18 15:23 ` [PATCH v3 3/4] arm64: dts: allwinner: h6: add r_watchog node Clément Péron
@ 2019-05-20 15:19   ` Clément Péron
  0 siblings, 0 replies; 14+ messages in thread
From: Clément Péron @ 2019-05-20 15:19 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
	Maxime Ripard, Chen-Yu Tsai
  Cc: linux-watchdog, devicetree, linux-arm-kernel, linux-kernel

Hi Maxime,

On Sat, 18 May 2019 at 17:24, Clément Péron <peron.clem@gmail.com> wrote:
>
> Allwinner H6 has a r_watchdog similar to A64.
>
> Declare it in the device-tree.
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index 60b47f23b2f5..27647e496269 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -632,6 +632,14 @@
>                         #reset-cells = <1>;
>                 };
>
> +               r_watchdog: watchdog@7020400 {
> +                       compatible = "allwinner,sun50i-h6-wdt",
> +                                    "allwinner,sun50i-a64-wdt",
> +                                    "allwinner,sun6i-a31-wdt";
> +                       reg = <0x07020400 0x20>;
> +                       interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;

Just want to point out that i have used the same bindings for WDT and R_WDT.
I think it would be better to also introduce a bindings like
"allwinner,sun50i-h6-r-wdt", "allwinner,sun6i-a31-wdt";

We don't have access to the datasheet of this IP but we can strongly
suppose that wdt and r-wdt are the same.

What do you think?

Regards,
Clement


> +               };
> +
>                 r_intc: interrupt-controller@7021000 {
>                         compatible = "allwinner,sun50i-h6-r-intc",
>                                      "allwinner,sun6i-a31-r-intc";
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-18 15:23 [PATCH v3 0/4] Allwinner H6 watchdog support Clément Péron
2019-05-18 15:23 ` [PATCH v3 1/4] dt-bindings: watchdog: add Allwinner H6 watchdog Clément Péron
2019-05-20  7:35   ` Maxime Ripard
2019-05-20  8:14     ` Clément Péron
2019-05-20 14:42       ` Maxime Ripard
2019-05-20 15:16         ` Clément Péron
2019-05-18 15:23 ` [PATCH v3 2/4] arm64: dts: allwinner: h6: add watchdog node Clément Péron
2019-05-20  7:36   ` Maxime Ripard
2019-05-20  8:21     ` Clément Péron
2019-05-20 14:44       ` Maxime Ripard
2019-05-20 14:57         ` Chen-Yu Tsai
2019-05-18 15:23 ` [PATCH v3 3/4] arm64: dts: allwinner: h6: add r_watchog node Clément Péron
2019-05-20 15:19   ` Clément Péron
2019-05-18 15:23 ` [PATCH v3 4/4] arm64: defconfig: enable sunxi watchdog Clément Péron

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org linux-watchdog@archiver.kernel.org
	public-inbox-index linux-watchdog


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/ public-inbox