* [U-Boot] [PATCH 0/3] fix bcm6345 watchdog on broadcom board
@ 2019-05-03 17:43 Philippe Reynes
2019-05-03 17:43 ` [U-Boot] [PATCH 1/3] watchdog: bcm6345: callback start use tick instead of ms Philippe Reynes
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Philippe Reynes @ 2019-05-03 17:43 UTC (permalink / raw
To: u-boot
Since the commit: commit 06985289d452 ("watchdog: Implement generic
watchdog_reset() version"), the watchdog is always started and a default
timeout of 60000 ms is used. But the driver for the bcm6345 watchdog use
this timeout in ms as tick. So a board using this driver reboot
immediately.
The first commit in this serie fix the driver of the bcm6345 watchdog by
converting the timeout in ms to tick before writing the register. The two
others commits fix the clock used by boards bcm96858xref and bcm963158.
This serie was tested on:
- bcm6838 (mips)
- bcm96858xref (arm)
- bcm963158 (arm)
Philippe Reynes (3):
watchdog: bcm6345: callback start use tick instead of ms
dt: bcm6858: watchdog should use a 50Mhz clock
dt: bcm63158: watchdog should use a 50Mhz clock
arch/arm/dts/bcm63158.dtsi | 10 ++++++++--
arch/arm/dts/bcm6858.dtsi | 10 ++++++++--
drivers/watchdog/bcm6345_wdt.c | 21 ++++++++++++++++-----
3 files changed, 32 insertions(+), 9 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 1/3] watchdog: bcm6345: callback start use tick instead of ms
2019-05-03 17:43 [U-Boot] [PATCH 0/3] fix bcm6345 watchdog on broadcom board Philippe Reynes
@ 2019-05-03 17:43 ` Philippe Reynes
2019-05-06 7:33 ` Stefan Roese
2019-05-19 20:45 ` Tom Rini
2019-05-03 17:43 ` [U-Boot] [PATCH 2/3] dt: bcm6858: watchdog should use a 50Mhz clock Philippe Reynes
2019-05-03 17:43 ` [U-Boot] [PATCH 3/3] dt: bcm63158: " Philippe Reynes
2 siblings, 2 replies; 13+ messages in thread
From: Philippe Reynes @ 2019-05-03 17:43 UTC (permalink / raw
To: u-boot
The function bcm6345_wdt_start use the argument timeout
as tick but it should be used as milliseconds.
A clock is added as requirement for this driver.
The frequency of the clock is then used to convert the
millisecond to ticks in the function bcm6345_wdt_start.
Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
drivers/watchdog/bcm6345_wdt.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/watchdog/bcm6345_wdt.c b/drivers/watchdog/bcm6345_wdt.c
index 44f5662..9f14e7d 100644
--- a/drivers/watchdog/bcm6345_wdt.c
+++ b/drivers/watchdog/bcm6345_wdt.c
@@ -10,6 +10,7 @@
#include <common.h>
#include <dm.h>
#include <wdt.h>
+#include <clk.h>
#include <asm/io.h>
/* WDT Value register */
@@ -26,6 +27,7 @@
struct bcm6345_wdt_priv {
void __iomem *regs;
+ unsigned long clk_rate;
};
static int bcm6345_wdt_reset(struct udevice *dev)
@@ -41,16 +43,17 @@ static int bcm6345_wdt_reset(struct udevice *dev)
static int bcm6345_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
{
struct bcm6345_wdt_priv *priv = dev_get_priv(dev);
+ u32 val = priv->clk_rate / 1000 * timeout;
- if (timeout < WDT_VAL_MIN) {
+ if (val < WDT_VAL_MIN) {
debug("watchdog won't fire with less than 2 ticks\n");
- timeout = WDT_VAL_MIN;
- } else if (timeout > WDT_VAL_MAX) {
+ val = WDT_VAL_MIN;
+ } else if (val > WDT_VAL_MAX) {
debug("maximum watchdog timeout exceeded\n");
- timeout = WDT_VAL_MAX;
+ val = WDT_VAL_MAX;
}
- writel(timeout, priv->regs + WDT_VAL_REG);
+ writel(val, priv->regs + WDT_VAL_REG);
return bcm6345_wdt_reset(dev);
}
@@ -85,11 +88,19 @@ static const struct udevice_id bcm6345_wdt_ids[] = {
static int bcm6345_wdt_probe(struct udevice *dev)
{
struct bcm6345_wdt_priv *priv = dev_get_priv(dev);
+ struct clk clk;
+ int ret;
priv->regs = dev_remap_addr(dev);
if (!priv->regs)
return -EINVAL;
+ ret = clk_get_by_index(dev, 0, &clk);
+ if (!ret)
+ priv->clk_rate = clk_get_rate(&clk);
+ else
+ return -EINVAL;
+
bcm6345_wdt_stop(dev);
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 2/3] dt: bcm6858: watchdog should use a 50Mhz clock
2019-05-03 17:43 [U-Boot] [PATCH 0/3] fix bcm6345 watchdog on broadcom board Philippe Reynes
2019-05-03 17:43 ` [U-Boot] [PATCH 1/3] watchdog: bcm6345: callback start use tick instead of ms Philippe Reynes
@ 2019-05-03 17:43 ` Philippe Reynes
2019-05-06 7:44 ` Stefan Roese
2019-05-19 20:45 ` Tom Rini
2019-05-03 17:43 ` [U-Boot] [PATCH 3/3] dt: bcm63158: " Philippe Reynes
2 siblings, 2 replies; 13+ messages in thread
From: Philippe Reynes @ 2019-05-03 17:43 UTC (permalink / raw
To: u-boot
The watchdog should use a clock at 50 Mhz, so
instead of using the clock osc (200 Mhz), we
define a reference clock at 50Mhz and use it
for both watchdog.
Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
arch/arm/dts/bcm6858.dtsi | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/dts/bcm6858.dtsi b/arch/arm/dts/bcm6858.dtsi
index 76ba0ea..91f7787 100644
--- a/arch/arm/dts/bcm6858.dtsi
+++ b/arch/arm/dts/bcm6858.dtsi
@@ -66,6 +66,12 @@
clock-frequency = <200000000>;
u-boot,dm-pre-reloc;
};
+
+ refclk50mhz: refclk50mhz {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <50000000>;
+ };
};
ubus {
@@ -92,13 +98,13 @@
wdt1: watchdog at ff802780 {
compatible = "brcm,bcm6345-wdt";
reg = <0x0 0xff802780 0x0 0x14>;
- clocks = <&periph_osc>;
+ clocks = <&refclk50mhz>;
};
wdt2: watchdog at ff8027c0 {
compatible = "brcm,bcm6345-wdt";
reg = <0x0 0xff8027c0 0x0 0x14>;
- clocks = <&periph_osc>;
+ clocks = <&refclk50mhz>;
};
wdt-reboot {
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 3/3] dt: bcm63158: watchdog should use a 50Mhz clock
2019-05-03 17:43 [U-Boot] [PATCH 0/3] fix bcm6345 watchdog on broadcom board Philippe Reynes
2019-05-03 17:43 ` [U-Boot] [PATCH 1/3] watchdog: bcm6345: callback start use tick instead of ms Philippe Reynes
2019-05-03 17:43 ` [U-Boot] [PATCH 2/3] dt: bcm6858: watchdog should use a 50Mhz clock Philippe Reynes
@ 2019-05-03 17:43 ` Philippe Reynes
2019-05-06 7:45 ` Stefan Roese
2019-05-19 20:45 ` Tom Rini
2 siblings, 2 replies; 13+ messages in thread
From: Philippe Reynes @ 2019-05-03 17:43 UTC (permalink / raw
To: u-boot
The watchdog should use a clock at 50 Mhz, so
instead of using the clock osc (200 Mhz), we
define a reference clock at 50Mhz and use it
for both watchdog.
Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
arch/arm/dts/bcm63158.dtsi | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/dts/bcm63158.dtsi b/arch/arm/dts/bcm63158.dtsi
index 4b2eaee..175af38 100644
--- a/arch/arm/dts/bcm63158.dtsi
+++ b/arch/arm/dts/bcm63158.dtsi
@@ -66,6 +66,12 @@
clock-frequency = <0xbebc200>;
u-boot,dm-pre-reloc;
};
+
+ refclk50mhz: refclk50mhz {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <50000000>;
+ };
};
ubus {
@@ -92,13 +98,13 @@
wdt1: watchdog at ff800480 {
compatible = "brcm,bcm6345-wdt";
reg = <0x0 0xff800480 0x0 0x14>;
- clocks = <&periph_osc>;
+ clocks = <&refclk50mhz>;
};
wdt2: watchdog at ff8004c0 {
compatible = "brcm,bcm6345-wdt";
reg = <0x0 0xff8004c0 0x0 0x14>;
- clocks = <&periph_osc>;
+ clocks = <&refclk50mhz>;
};
wdt-reboot {
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 1/3] watchdog: bcm6345: callback start use tick instead of ms
2019-05-03 17:43 ` [U-Boot] [PATCH 1/3] watchdog: bcm6345: callback start use tick instead of ms Philippe Reynes
@ 2019-05-06 7:33 ` Stefan Roese
2019-05-19 20:45 ` Tom Rini
1 sibling, 0 replies; 13+ messages in thread
From: Stefan Roese @ 2019-05-06 7:33 UTC (permalink / raw
To: u-boot
On 03.05.19 19:43, Philippe Reynes wrote:
> The function bcm6345_wdt_start use the argument timeout
> as tick but it should be used as milliseconds.
>
> A clock is added as requirement for this driver.
> The frequency of the clock is then used to convert the
> millisecond to ticks in the function bcm6345_wdt_start.
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
You might want to emphasize, that this is a pretty severe fix to
this driver, as without this patch, boards using this driver will
end in a endless reboot loop. So it should be applied to this
release.
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
> ---
> drivers/watchdog/bcm6345_wdt.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/watchdog/bcm6345_wdt.c b/drivers/watchdog/bcm6345_wdt.c
> index 44f5662..9f14e7d 100644
> --- a/drivers/watchdog/bcm6345_wdt.c
> +++ b/drivers/watchdog/bcm6345_wdt.c
> @@ -10,6 +10,7 @@
> #include <common.h>
> #include <dm.h>
> #include <wdt.h>
> +#include <clk.h>
> #include <asm/io.h>
>
> /* WDT Value register */
> @@ -26,6 +27,7 @@
>
> struct bcm6345_wdt_priv {
> void __iomem *regs;
> + unsigned long clk_rate;
> };
>
> static int bcm6345_wdt_reset(struct udevice *dev)
> @@ -41,16 +43,17 @@ static int bcm6345_wdt_reset(struct udevice *dev)
> static int bcm6345_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
> {
> struct bcm6345_wdt_priv *priv = dev_get_priv(dev);
> + u32 val = priv->clk_rate / 1000 * timeout;
>
> - if (timeout < WDT_VAL_MIN) {
> + if (val < WDT_VAL_MIN) {
> debug("watchdog won't fire with less than 2 ticks\n");
> - timeout = WDT_VAL_MIN;
> - } else if (timeout > WDT_VAL_MAX) {
> + val = WDT_VAL_MIN;
> + } else if (val > WDT_VAL_MAX) {
> debug("maximum watchdog timeout exceeded\n");
> - timeout = WDT_VAL_MAX;
> + val = WDT_VAL_MAX;
> }
>
> - writel(timeout, priv->regs + WDT_VAL_REG);
> + writel(val, priv->regs + WDT_VAL_REG);
>
> return bcm6345_wdt_reset(dev);
> }
> @@ -85,11 +88,19 @@ static const struct udevice_id bcm6345_wdt_ids[] = {
> static int bcm6345_wdt_probe(struct udevice *dev)
> {
> struct bcm6345_wdt_priv *priv = dev_get_priv(dev);
> + struct clk clk;
> + int ret;
>
> priv->regs = dev_remap_addr(dev);
> if (!priv->regs)
> return -EINVAL;
>
> + ret = clk_get_by_index(dev, 0, &clk);
> + if (!ret)
> + priv->clk_rate = clk_get_rate(&clk);
> + else
> + return -EINVAL;
> +
> bcm6345_wdt_stop(dev);
>
> return 0;
>
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 2/3] dt: bcm6858: watchdog should use a 50Mhz clock
2019-05-03 17:43 ` [U-Boot] [PATCH 2/3] dt: bcm6858: watchdog should use a 50Mhz clock Philippe Reynes
@ 2019-05-06 7:44 ` Stefan Roese
2019-05-19 20:45 ` Tom Rini
1 sibling, 0 replies; 13+ messages in thread
From: Stefan Roese @ 2019-05-06 7:44 UTC (permalink / raw
To: u-boot
On 03.05.19 19:43, Philippe Reynes wrote:
> The watchdog should use a clock at 50 Mhz, so
> instead of using the clock osc (200 Mhz), we
> define a reference clock at 50Mhz and use it
> for both watchdog.
Just curious: Why is this the case? Is this also what's done in
the Linux DT version?
Other than that:
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
> arch/arm/dts/bcm6858.dtsi | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/dts/bcm6858.dtsi b/arch/arm/dts/bcm6858.dtsi
> index 76ba0ea..91f7787 100644
> --- a/arch/arm/dts/bcm6858.dtsi
> +++ b/arch/arm/dts/bcm6858.dtsi
> @@ -66,6 +66,12 @@
> clock-frequency = <200000000>;
> u-boot,dm-pre-reloc;
> };
> +
> + refclk50mhz: refclk50mhz {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <50000000>;
> + };
> };
>
> ubus {
> @@ -92,13 +98,13 @@
> wdt1: watchdog at ff802780 {
> compatible = "brcm,bcm6345-wdt";
> reg = <0x0 0xff802780 0x0 0x14>;
> - clocks = <&periph_osc>;
> + clocks = <&refclk50mhz>;
> };
>
> wdt2: watchdog at ff8027c0 {
> compatible = "brcm,bcm6345-wdt";
> reg = <0x0 0xff8027c0 0x0 0x14>;
> - clocks = <&periph_osc>;
> + clocks = <&refclk50mhz>;
> };
>
> wdt-reboot {
>
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 3/3] dt: bcm63158: watchdog should use a 50Mhz clock
2019-05-03 17:43 ` [U-Boot] [PATCH 3/3] dt: bcm63158: " Philippe Reynes
@ 2019-05-06 7:45 ` Stefan Roese
2019-05-06 12:38 ` Philippe REYNES
2019-05-19 20:45 ` Tom Rini
1 sibling, 1 reply; 13+ messages in thread
From: Stefan Roese @ 2019-05-06 7:45 UTC (permalink / raw
To: u-boot
On 03.05.19 19:43, Philippe Reynes wrote:
> The watchdog should use a clock at 50 Mhz, so
> instead of using the clock osc (200 Mhz), we
> define a reference clock at 50Mhz and use it
> for both watchdog.
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
Just curious: Why is this the case? Is this also what's done in
the Linux DT version?
Other than that:
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
> ---
> arch/arm/dts/bcm63158.dtsi | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/dts/bcm63158.dtsi b/arch/arm/dts/bcm63158.dtsi
> index 4b2eaee..175af38 100644
> --- a/arch/arm/dts/bcm63158.dtsi
> +++ b/arch/arm/dts/bcm63158.dtsi
> @@ -66,6 +66,12 @@
> clock-frequency = <0xbebc200>;
> u-boot,dm-pre-reloc;
> };
> +
> + refclk50mhz: refclk50mhz {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <50000000>;
> + };
> };
>
> ubus {
> @@ -92,13 +98,13 @@
> wdt1: watchdog at ff800480 {
> compatible = "brcm,bcm6345-wdt";
> reg = <0x0 0xff800480 0x0 0x14>;
> - clocks = <&periph_osc>;
> + clocks = <&refclk50mhz>;
> };
>
> wdt2: watchdog at ff8004c0 {
> compatible = "brcm,bcm6345-wdt";
> reg = <0x0 0xff8004c0 0x0 0x14>;
> - clocks = <&periph_osc>;
> + clocks = <&refclk50mhz>;
> };
>
> wdt-reboot {
>
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 3/3] dt: bcm63158: watchdog should use a 50Mhz clock
2019-05-06 7:45 ` Stefan Roese
@ 2019-05-06 12:38 ` Philippe REYNES
2019-05-06 12:50 ` Stefan Roese
0 siblings, 1 reply; 13+ messages in thread
From: Philippe REYNES @ 2019-05-06 12:38 UTC (permalink / raw
To: u-boot
Hi Stefan,
> On 03.05.19 19:43, Philippe Reynes wrote:
>> The watchdog should use a clock at 50 Mhz, so
>> instead of using the clock osc (200 Mhz), we
>> define a reference clock at 50Mhz and use it
>> for both watchdog.
>>
>> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
>
> Just curious: Why is this the case? Is this also what's done in
> the Linux DT version?
From my understanding, in the linux kernel, the driver doesn't compute
the timeout for the watchdog counter register. Every second, the driver
set the maximum value in the watchdog counter register and compute a
logical tick. If this tick decrease below zero, the watchdog isn't
restarted, so when the watchdog counter reach zero, the board is resetted.
In u-boot, the driver compute the expected timeout and set it
in the watchdog register.
> Other than that:
>
> Reviewed-by: Stefan Roese <sr@denx.de>
Thanks
> Thanks,
> Stefan
Regards,
Philippe
>
>> ---
>> arch/arm/dts/bcm63158.dtsi | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/dts/bcm63158.dtsi b/arch/arm/dts/bcm63158.dtsi
>> index 4b2eaee..175af38 100644
>> --- a/arch/arm/dts/bcm63158.dtsi
>> +++ b/arch/arm/dts/bcm63158.dtsi
>> @@ -66,6 +66,12 @@
>> clock-frequency = <0xbebc200>;
>> u-boot,dm-pre-reloc;
>> };
>> +
>> + refclk50mhz: refclk50mhz {
>> + compatible = "fixed-clock";
>> + #clock-cells = <0>;
>> + clock-frequency = <50000000>;
>> + };
>> };
>>
>> ubus {
>> @@ -92,13 +98,13 @@
>> wdt1: watchdog at ff800480 {
>> compatible = "brcm,bcm6345-wdt";
>> reg = <0x0 0xff800480 0x0 0x14>;
>> - clocks = <&periph_osc>;
>> + clocks = <&refclk50mhz>;
>> };
>>
>> wdt2: watchdog at ff8004c0 {
>> compatible = "brcm,bcm6345-wdt";
>> reg = <0x0 0xff8004c0 0x0 0x14>;
>> - clocks = <&periph_osc>;
>> + clocks = <&refclk50mhz>;
>> };
>>
>> wdt-reboot {
>>
>
> Viele Grüße,
> Stefan
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 3/3] dt: bcm63158: watchdog should use a 50Mhz clock
2019-05-06 12:38 ` Philippe REYNES
@ 2019-05-06 12:50 ` Stefan Roese
2019-05-06 13:59 ` Philippe REYNES
0 siblings, 1 reply; 13+ messages in thread
From: Stefan Roese @ 2019-05-06 12:50 UTC (permalink / raw
To: u-boot
Hi Philippe,
On 06.05.19 14:38, Philippe REYNES wrote:
> Hi Stefan,
>
>> On 03.05.19 19:43, Philippe Reynes wrote:
>>> The watchdog should use a clock at 50 Mhz, so
>>> instead of using the clock osc (200 Mhz), we
>>> define a reference clock at 50Mhz and use it
>>> for both watchdog.
>>>
>>> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
>>
>> Just curious: Why is this the case? Is this also what's done in
>> the Linux DT version?
>
> From my understanding, in the linux kernel, the driver doesn't compute
> the timeout for the watchdog counter register. Every second, the driver
> set the maximum value in the watchdog counter register and compute a
> logical tick. If this tick decrease below zero, the watchdog isn't
> restarted, so when the watchdog counter reach zero, the board is resetted.
>
> In u-boot, the driver compute the expected timeout and set it
> in the watchdog register.
I see. But why "should the watchdog use a clock at 50MHz" instead of
the default 200MHz (from your commit text)?
I'm checking as this change most likely results in a DT difference in
the U-Boot vs the Linux version, which should be avoided.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 3/3] dt: bcm63158: watchdog should use a 50Mhz clock
2019-05-06 12:50 ` Stefan Roese
@ 2019-05-06 13:59 ` Philippe REYNES
0 siblings, 0 replies; 13+ messages in thread
From: Philippe REYNES @ 2019-05-06 13:59 UTC (permalink / raw
To: u-boot
Hi Stefan,
> Hi Philippe,
>
> On 06.05.19 14:38, Philippe REYNES wrote:
>> Hi Stefan,
>>
>>> On 03.05.19 19:43, Philippe Reynes wrote:
>>>> The watchdog should use a clock at 50 Mhz, so
>>>> instead of using the clock osc (200 Mhz), we
>>>> define a reference clock at 50Mhz and use it
>>>> for both watchdog.
>>>>
>>>> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
>>>
>>> Just curious: Why is this the case? Is this also what's done in
>>> the Linux DT version?
>>
>> From my understanding, in the linux kernel, the driver doesn't compute
>> the timeout for the watchdog counter register. Every second, the driver
>> set the maximum value in the watchdog counter register and compute a
>> logical tick. If this tick decrease below zero, the watchdog isn't
>> restarted, so when the watchdog counter reach zero, the board is resetted.
>>
>> In u-boot, the driver compute the expected timeout and set it
>> in the watchdog register.
>
> I see. But why "should the watchdog use a clock at 50MHz" instead of
> the default 200MHz (from your commit text)?
In the patch "fix bcm6345 watchdog on broadcom board", I've updated the
watchdog driver for bcm6345 to compute the number of tick for the counter
in the watchdog. For this computation, I need the frequency of the clock
used by the watchdog. As this IP is used on many SoC (mips, arm), I've
added a clock in the device tree so it's could be more generic.
> I'm checking as this change most likely results in a DT difference in
> the U-Boot vs the Linux version, which should be avoided.
> Thanks,
> Stefan
Regards,
Philippe
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 1/3] watchdog: bcm6345: callback start use tick instead of ms
2019-05-03 17:43 ` [U-Boot] [PATCH 1/3] watchdog: bcm6345: callback start use tick instead of ms Philippe Reynes
2019-05-06 7:33 ` Stefan Roese
@ 2019-05-19 20:45 ` Tom Rini
1 sibling, 0 replies; 13+ messages in thread
From: Tom Rini @ 2019-05-19 20:45 UTC (permalink / raw
To: u-boot
On Fri, May 03, 2019 at 07:43:06PM +0200, Philippe Reynes wrote:
> The function bcm6345_wdt_start use the argument timeout
> as tick but it should be used as milliseconds.
>
> A clock is added as requirement for this driver.
> The frequency of the clock is then used to convert the
> millisecond to ticks in the function bcm6345_wdt_start.
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> Reviewed-by: Stefan Roese <sr@denx.de>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190519/a2ddd695/attachment.sig>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 2/3] dt: bcm6858: watchdog should use a 50Mhz clock
2019-05-03 17:43 ` [U-Boot] [PATCH 2/3] dt: bcm6858: watchdog should use a 50Mhz clock Philippe Reynes
2019-05-06 7:44 ` Stefan Roese
@ 2019-05-19 20:45 ` Tom Rini
1 sibling, 0 replies; 13+ messages in thread
From: Tom Rini @ 2019-05-19 20:45 UTC (permalink / raw
To: u-boot
On Fri, May 03, 2019 at 07:43:07PM +0200, Philippe Reynes wrote:
> The watchdog should use a clock at 50 Mhz, so
> instead of using the clock osc (200 Mhz), we
> define a reference clock at 50Mhz and use it
> for both watchdog.
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> Reviewed-by: Stefan Roese <sr@denx.de>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190519/86239ff5/attachment.sig>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 3/3] dt: bcm63158: watchdog should use a 50Mhz clock
2019-05-03 17:43 ` [U-Boot] [PATCH 3/3] dt: bcm63158: " Philippe Reynes
2019-05-06 7:45 ` Stefan Roese
@ 2019-05-19 20:45 ` Tom Rini
1 sibling, 0 replies; 13+ messages in thread
From: Tom Rini @ 2019-05-19 20:45 UTC (permalink / raw
To: u-boot
On Fri, May 03, 2019 at 07:43:08PM +0200, Philippe Reynes wrote:
> The watchdog should use a clock at 50 Mhz, so
> instead of using the clock osc (200 Mhz), we
> define a reference clock at 50Mhz and use it
> for both watchdog.
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> Reviewed-by: Stefan Roese <sr@denx.de>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190519/39435eb8/attachment.sig>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-05-19 20:45 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-03 17:43 [U-Boot] [PATCH 0/3] fix bcm6345 watchdog on broadcom board Philippe Reynes
2019-05-03 17:43 ` [U-Boot] [PATCH 1/3] watchdog: bcm6345: callback start use tick instead of ms Philippe Reynes
2019-05-06 7:33 ` Stefan Roese
2019-05-19 20:45 ` Tom Rini
2019-05-03 17:43 ` [U-Boot] [PATCH 2/3] dt: bcm6858: watchdog should use a 50Mhz clock Philippe Reynes
2019-05-06 7:44 ` Stefan Roese
2019-05-19 20:45 ` Tom Rini
2019-05-03 17:43 ` [U-Boot] [PATCH 3/3] dt: bcm63158: " Philippe Reynes
2019-05-06 7:45 ` Stefan Roese
2019-05-06 12:38 ` Philippe REYNES
2019-05-06 12:50 ` Stefan Roese
2019-05-06 13:59 ` Philippe REYNES
2019-05-19 20:45 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.