[PATCH 1/2] mfd: da9063: fix TWDSCALE debug message

[PATCH 1/2] mfd: da9063: fix TWDSCALE debug message

[Marco Felsch]

The TWDSCALE is the found scale + 1 as described in the datasheets
for the DA9062/3 devices. The driver logic is correct just the debug
message is wrong.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/mfd/da9063.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c
index f0381944d9..4d459c7f18 100644
--- a/drivers/mfd/da9063.c
+++ b/drivers/mfd/da9063.c
@@ -270,7 +270,7 @@ static int da9063_watchdog_set_timeout(struct watchdog *wd, unsigned timeout)
 		while (timeout > (2048 << scale) && scale <= 6)
 			scale++;
 		dev_dbg(dev, "calculated TWDSCALE=%u (req=%ims calc=%ims)\n",
-				scale, timeout, 2048 << scale);
+			scale + 1, timeout, 2048 << scale);
 		scale++; /* scale 0 disables the WD */
 	}
 
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] mfd: da9063: fix watchdog ping execution

[Marco Felsch]

The watchdog resets the system if the watchdog gets pinged to fast.
Between each watchdog ping must be a pause of at least 200ms.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/mfd/da9063.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c
index 4d459c7f18..ab57885240 100644
--- a/drivers/mfd/da9063.c
+++ b/drivers/mfd/da9063.c
@@ -14,6 +14,7 @@
  */
 
 #include <common.h>
+#include <clock.h>
 #include <driver.h>
 #include <gpio.h>
 #include <restart.h>
@@ -33,6 +34,7 @@ struct da9063 {
 	struct i2c_client	*client1;
 	struct device_d		*dev;
 	unsigned int		timeout;
+	uint64_t		last_ping;
 };
 
 /* forbidden/impossible value; timeout will be set to this value initially to
@@ -237,6 +239,13 @@ static int da9063_watchdog_ping(struct da9063 *priv)
 	int ret;
 	u8 val;
 
+	/* We need to wait at least 200ms till we can resend a ping */
+	if (!is_timeout_non_interruptible(priv->last_ping, 200 * MSECOND)) {
+		dev_dbg(priv->dev, "active ping delay\n");
+		mdelay(50);
+		return da9063_watchdog_ping(priv);
+	}
+
 	dev_dbg(priv->dev, "ping\n");
 
 	/* reset watchdog timer; register is self clearing */
@@ -245,6 +254,8 @@ static int da9063_watchdog_ping(struct da9063 *priv)
 	if (ret < 0)
 		return ret;
 
+	priv->last_ping = get_time_ns();
+
 	return 0;
 }
 
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mfd: da9063: fix watchdog ping execution

[Sascha Hauer]

On Wed, Oct 30, 2019 at 06:06:53PM +0100, Marco Felsch wrote:
> The watchdog resets the system if the watchdog gets pinged to fast.
> Between each watchdog ping must be a pause of at least 200ms.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/mfd/da9063.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c
> index 4d459c7f18..ab57885240 100644
> --- a/drivers/mfd/da9063.c
> +++ b/drivers/mfd/da9063.c
> @@ -14,6 +14,7 @@
>   */
>  
>  #include <common.h>
> +#include <clock.h>
>  #include <driver.h>
>  #include <gpio.h>
>  #include <restart.h>
> @@ -33,6 +34,7 @@ struct da9063 {
>  	struct i2c_client	*client1;
>  	struct device_d		*dev;
>  	unsigned int		timeout;
> +	uint64_t		last_ping;
>  };
>  
>  /* forbidden/impossible value; timeout will be set to this value initially to
> @@ -237,6 +239,13 @@ static int da9063_watchdog_ping(struct da9063 *priv)
>  	int ret;
>  	u8 val;
>  
> +	/* We need to wait at least 200ms till we can resend a ping */
> +	if (!is_timeout_non_interruptible(priv->last_ping, 200 * MSECOND)) {
> +		dev_dbg(priv->dev, "active ping delay\n");
> +		mdelay(50);

I would expect to wait the missing time to 200ms here. Maybe doing
nothing in this case would be more appropriate here. I mean, why should
you slow down barebox here when some code triggers the watchdog too
often?

> +		return da9063_watchdog_ping(priv);

Drop this, just fall through.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mfd: da9063: fix watchdog ping execution

[Marco Felsch]

On 19-11-04 09:27, Sascha Hauer wrote:
> On Wed, Oct 30, 2019 at 06:06:53PM +0100, Marco Felsch wrote:
> > The watchdog resets the system if the watchdog gets pinged to fast.
> > Between each watchdog ping must be a pause of at least 200ms.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  drivers/mfd/da9063.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c
> > index 4d459c7f18..ab57885240 100644
> > --- a/drivers/mfd/da9063.c
> > +++ b/drivers/mfd/da9063.c
> > @@ -14,6 +14,7 @@
> >   */
> >  
> >  #include <common.h>
> > +#include <clock.h>
> >  #include <driver.h>
> >  #include <gpio.h>
> >  #include <restart.h>
> > @@ -33,6 +34,7 @@ struct da9063 {
> >  	struct i2c_client	*client1;
> >  	struct device_d		*dev;
> >  	unsigned int		timeout;
> > +	uint64_t		last_ping;
> >  };
> >  
> >  /* forbidden/impossible value; timeout will be set to this value initially to
> > @@ -237,6 +239,13 @@ static int da9063_watchdog_ping(struct da9063 *priv)
> >  	int ret;
> >  	u8 val;
> >  
> > +	/* We need to wait at least 200ms till we can resend a ping */
> > +	if (!is_timeout_non_interruptible(priv->last_ping, 200 * MSECOND)) {
> > +		dev_dbg(priv->dev, "active ping delay\n");
> > +		mdelay(50);
> 
> I would expect to wait the missing time to 200ms here. Maybe doing
> nothing in this case would be more appropriate here. I mean, why should
> you slow down barebox here when some code triggers the watchdog too
> often?
> 
> > +		return da9063_watchdog_ping(priv);
> 
> Drop this, just fall through.

Just prepared a v2 with a busy wait after discussed it with Lucas.
Thanks for your input too :)

Regards,
  Marco

> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mfd: da9063: fix watchdog ping execution

[Ahmad Fatoum]

Hello Marco,

On 11/4/19 9:34 AM, Marco Felsch wrote:
> On 19-11-04 09:27, Sascha Hauer wrote:
>> On Wed, Oct 30, 2019 at 06:06:53PM +0100, Marco Felsch wrote:
>>> The watchdog resets the system if the watchdog gets pinged to fast.
>>> Between each watchdog ping must be a pause of at least 200ms.

I assume you're using the boot.watchdog_timeout parameter? The time contained
in this parameter is communicated to the watchdog in boot_entry, which is called
twice in a normal bootchooser boot, once from the boot command and once more
from bootchooser.

This means that your boot time would increase by 200 ms. If this matter to you,
you might want to change this, so watchdog_set_timeout is called only once.

And if you do so, you could drop this patch. The only other places that feed
the watchdog are the watchdog poller and the wd command. The watchdog poller
already waits 500 ms between pings and the command is meant for debugging/testing.
If someone wants to feed the watchdog that fast while testing, why prevent them?

(I assume you don't need to wait 200 ms between ping and disabling WDT, if you do,
 one more place is the .priority watchdog device parameter in barebox-next)

Cheers
Ahmad

>>>
>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>>> ---
>>>  drivers/mfd/da9063.c | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c
>>> index 4d459c7f18..ab57885240 100644
>>> --- a/drivers/mfd/da9063.c
>>> +++ b/drivers/mfd/da9063.c
>>> @@ -14,6 +14,7 @@
>>>   */
>>>  
>>>  #include <common.h>
>>> +#include <clock.h>
>>>  #include <driver.h>
>>>  #include <gpio.h>
>>>  #include <restart.h>
>>> @@ -33,6 +34,7 @@ struct da9063 {
>>>  	struct i2c_client	*client1;
>>>  	struct device_d		*dev;
>>>  	unsigned int		timeout;
>>> +	uint64_t		last_ping;
>>>  };
>>>  
>>>  /* forbidden/impossible value; timeout will be set to this value initially to
>>> @@ -237,6 +239,13 @@ static int da9063_watchdog_ping(struct da9063 *priv)
>>>  	int ret;
>>>  	u8 val;
>>>  
>>> +	/* We need to wait at least 200ms till we can resend a ping */
>>> +	if (!is_timeout_non_interruptible(priv->last_ping, 200 * MSECOND)) {
>>> +		dev_dbg(priv->dev, "active ping delay\n");
>>> +		mdelay(50);
>>
>> I would expect to wait the missing time to 200ms here. Maybe doing
>> nothing in this case would be more appropriate here. I mean, why should
>> you slow down barebox here when some code triggers the watchdog too
>> often?
>>
>>> +		return da9063_watchdog_ping(priv);
>>
>> Drop this, just fall through.
> 
> Just prepared a v2 with a busy wait after discussed it with Lucas.
> Thanks for your input too :)
> 
> Regards,
>   Marco
> 
>> Sascha
>>
>> -- 
>> Pengutronix e.K.                           |                             |
>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mfd: da9063: fix watchdog ping execution

[Marco Felsch]

Hi Ahmad,

On 19-11-04 09:51, Ahmad Fatoum wrote:
> Hello Marco,
> 
> On 11/4/19 9:34 AM, Marco Felsch wrote:
> > On 19-11-04 09:27, Sascha Hauer wrote:
> >> On Wed, Oct 30, 2019 at 06:06:53PM +0100, Marco Felsch wrote:
> >>> The watchdog resets the system if the watchdog gets pinged to fast.
> >>> Between each watchdog ping must be a pause of at least 200ms.
> 
> I assume you're using the boot.watchdog_timeout parameter? The time contained
> in this parameter is communicated to the watchdog in boot_entry, which is called
> twice in a normal bootchooser boot, once from the boot command and once more
> from bootchooser.

Thats correct. I saw this bug during testing my boot.default param so
didn't noticed that bootchooser uses boot_entry too.

> This means that your boot time would increase by 200 ms. If this matter to you,
> you might want to change this, so watchdog_set_timeout is called only once.

Increasing the delay isn't a big deal. But after we discussed it again I
will send a v2 which handles the to fast pings by dropping those.

> And if you do so, you could drop this patch. The only other places that feed
> the watchdog are the watchdog poller and the wd command. The watchdog poller
> already waits 500 ms between pings and the command is meant for debugging/testing.
> If someone wants to feed the watchdog that fast while testing, why prevent them?

Becuase if the watchdog gets feeded to fast then the system gets
reseted. So dropping the patch isn't a option.

> (I assume you don't need to wait 200 ms between ping and disabling WDT, if you do,
>  one more place is the .priority watchdog device parameter in barebox-next

Sorry, I don't get this. You don't need to wait 200ms between ping and
disabling.

Regards,
  Marco

> Cheers
> Ahmad
> 
> >>>
> >>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> >>> ---
> >>>  drivers/mfd/da9063.c | 11 +++++++++++
> >>>  1 file changed, 11 insertions(+)
> >>>
> >>> diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c
> >>> index 4d459c7f18..ab57885240 100644
> >>> --- a/drivers/mfd/da9063.c
> >>> +++ b/drivers/mfd/da9063.c
> >>> @@ -14,6 +14,7 @@
> >>>   */
> >>>  
> >>>  #include <common.h>
> >>> +#include <clock.h>
> >>>  #include <driver.h>
> >>>  #include <gpio.h>
> >>>  #include <restart.h>
> >>> @@ -33,6 +34,7 @@ struct da9063 {
> >>>  	struct i2c_client	*client1;
> >>>  	struct device_d		*dev;
> >>>  	unsigned int		timeout;
> >>> +	uint64_t		last_ping;
> >>>  };
> >>>  
> >>>  /* forbidden/impossible value; timeout will be set to this value initially to
> >>> @@ -237,6 +239,13 @@ static int da9063_watchdog_ping(struct da9063 *priv)
> >>>  	int ret;
> >>>  	u8 val;
> >>>  
> >>> +	/* We need to wait at least 200ms till we can resend a ping */
> >>> +	if (!is_timeout_non_interruptible(priv->last_ping, 200 * MSECOND)) {
> >>> +		dev_dbg(priv->dev, "active ping delay\n");
> >>> +		mdelay(50);
> >>
> >> I would expect to wait the missing time to 200ms here. Maybe doing
> >> nothing in this case would be more appropriate here. I mean, why should
> >> you slow down barebox here when some code triggers the watchdog too
> >> often?
> >>
> >>> +		return da9063_watchdog_ping(priv);
> >>
> >> Drop this, just fall through.
> > 
> > Just prepared a v2 with a busy wait after discussed it with Lucas.
> > Thanks for your input too :)
> > 
> > Regards,
> >   Marco
> > 
> >> Sascha
> >>
> >> -- 
> >> Pengutronix e.K.                           |                             |
> >> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> >> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >>
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mfd: da9063: fix watchdog ping execution

[Ahmad Fatoum]

Hello Marco,

On 11/4/19 10:44 AM, Marco Felsch wrote:
>> This means that your boot time would increase by 200 ms. If this matter to you,
>> you might want to change this, so watchdog_set_timeout is called only once.
> 
> Increasing the delay isn't a big deal. But after we discussed it again I
> will send a v2 which handles the to fast pings by dropping those.

That would be an option too, but moving watchdog_set_timeout out of boot_entry
would benefit other platforms too.

> 
>> And if you do so, you could drop this patch. The only other places that feed
>> the watchdog are the watchdog poller and the wd command. The watchdog poller
>> already waits 500 ms between pings and the command is meant for debugging/testing.
>> If someone wants to feed the watchdog that fast while testing, why prevent them?
> 
> Becuase if the watchdog gets feeded to fast then the system gets
> reseted. So dropping the patch isn't a option.

If you move watchdog_set_timeout out of boot_entry, you'll only be able to feed the
watchdog too fast if you manually type wd 1; wd 1;, which I argue isn't really an issue
IMHO, but I am fine with what you implement either way.

> 
>> (I assume you don't need to wait 200 ms between ping and disabling WDT, if you do,
>>  one more place is the .priority watchdog device parameter in barebox-next
> 
> Sorry, I don't get this. You don't need to wait 200ms between ping and
> disabling.

Just wanted to make sure that disabling the watchdog twice in rapdi succession doesn't
trigger the issue as well. All good.

Cheers
Ahmad

> 
> Regards,
>   Marco
> 
>> Cheers
>> Ahmad
>>
>>>>>
>>>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>>>>> ---
>>>>>  drivers/mfd/da9063.c | 11 +++++++++++
>>>>>  1 file changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c
>>>>> index 4d459c7f18..ab57885240 100644
>>>>> --- a/drivers/mfd/da9063.c
>>>>> +++ b/drivers/mfd/da9063.c
>>>>> @@ -14,6 +14,7 @@
>>>>>   */
>>>>>  
>>>>>  #include <common.h>
>>>>> +#include <clock.h>
>>>>>  #include <driver.h>
>>>>>  #include <gpio.h>
>>>>>  #include <restart.h>
>>>>> @@ -33,6 +34,7 @@ struct da9063 {
>>>>>  	struct i2c_client	*client1;
>>>>>  	struct device_d		*dev;
>>>>>  	unsigned int		timeout;
>>>>> +	uint64_t		last_ping;
>>>>>  };
>>>>>  
>>>>>  /* forbidden/impossible value; timeout will be set to this value initially to
>>>>> @@ -237,6 +239,13 @@ static int da9063_watchdog_ping(struct da9063 *priv)
>>>>>  	int ret;
>>>>>  	u8 val;
>>>>>  
>>>>> +	/* We need to wait at least 200ms till we can resend a ping */
>>>>> +	if (!is_timeout_non_interruptible(priv->last_ping, 200 * MSECOND)) {
>>>>> +		dev_dbg(priv->dev, "active ping delay\n");
>>>>> +		mdelay(50);
>>>>
>>>> I would expect to wait the missing time to 200ms here. Maybe doing
>>>> nothing in this case would be more appropriate here. I mean, why should
>>>> you slow down barebox here when some code triggers the watchdog too
>>>> often?
>>>>
>>>>> +		return da9063_watchdog_ping(priv);
>>>>
>>>> Drop this, just fall through.
>>>
>>> Just prepared a v2 with a busy wait after discussed it with Lucas.
>>> Thanks for your input too :)
>>>
>>> Regards,
>>>   Marco
>>>
>>>> Sascha
>>>>
>>>> -- 
>>>> Pengutronix e.K.                           |                             |
>>>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>>>> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
>>>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>>>
>>>
>>
>> -- 
>> Pengutronix e.K.                           |                             |
>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>
> 


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mfd: da9063: fix watchdog ping execution

[Marco Felsch]

Hi Ahmad,

On 19-11-04 10:57, Ahmad Fatoum wrote:
> Hello Marco,
> 
> On 11/4/19 10:44 AM, Marco Felsch wrote:
> >> This means that your boot time would increase by 200 ms. If this matter to you,
> >> you might want to change this, so watchdog_set_timeout is called only once.
> > 
> > Increasing the delay isn't a big deal. But after we discussed it again I
> > will send a v2 which handles the to fast pings by dropping those.
> 
> That would be an option too, but moving watchdog_set_timeout out of boot_entry
> would benefit other platforms too.

Hm.. I don't have a strong opinion about that. To me it seems correct to
handle the boot-watchdog within the boot_entry() function and the
callers don't need to setup it. Anyway v2 is coming ;)

Regards,
  Marco

> >> And if you do so, you could drop this patch. The only other places that feed
> >> the watchdog are the watchdog poller and the wd command. The watchdog poller
> >> already waits 500 ms between pings and the command is meant for debugging/testing.
> >> If someone wants to feed the watchdog that fast while testing, why prevent them?
> > 
> > Becuase if the watchdog gets feeded to fast then the system gets
> > reseted. So dropping the patch isn't a option.
> 
> If you move watchdog_set_timeout out of boot_entry, you'll only be able to feed the
> watchdog too fast if you manually type wd 1; wd 1;, which I argue isn't really an issue
> IMHO, but I am fine with what you implement either way.
> 
> > 
> >> (I assume you don't need to wait 200 ms between ping and disabling WDT, if you do,
> >>  one more place is the .priority watchdog device parameter in barebox-next
> > 
> > Sorry, I don't get this. You don't need to wait 200ms between ping and
> > disabling.
> 
> Just wanted to make sure that disabling the watchdog twice in rapdi succession doesn't
> trigger the issue as well. All good.
> 
> Cheers
> Ahmad
> 
> > 
> > Regards,
> >   Marco
> > 
> >> Cheers
> >> Ahmad
> >>
> >>>>>
> >>>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> >>>>> ---
> >>>>>  drivers/mfd/da9063.c | 11 +++++++++++
> >>>>>  1 file changed, 11 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c
> >>>>> index 4d459c7f18..ab57885240 100644
> >>>>> --- a/drivers/mfd/da9063.c
> >>>>> +++ b/drivers/mfd/da9063.c
> >>>>> @@ -14,6 +14,7 @@
> >>>>>   */
> >>>>>  
> >>>>>  #include <common.h>
> >>>>> +#include <clock.h>
> >>>>>  #include <driver.h>
> >>>>>  #include <gpio.h>
> >>>>>  #include <restart.h>
> >>>>> @@ -33,6 +34,7 @@ struct da9063 {
> >>>>>  	struct i2c_client	*client1;
> >>>>>  	struct device_d		*dev;
> >>>>>  	unsigned int		timeout;
> >>>>> +	uint64_t		last_ping;
> >>>>>  };
> >>>>>  
> >>>>>  /* forbidden/impossible value; timeout will be set to this value initially to
> >>>>> @@ -237,6 +239,13 @@ static int da9063_watchdog_ping(struct da9063 *priv)
> >>>>>  	int ret;
> >>>>>  	u8 val;
> >>>>>  
> >>>>> +	/* We need to wait at least 200ms till we can resend a ping */
> >>>>> +	if (!is_timeout_non_interruptible(priv->last_ping, 200 * MSECOND)) {
> >>>>> +		dev_dbg(priv->dev, "active ping delay\n");
> >>>>> +		mdelay(50);
> >>>>
> >>>> I would expect to wait the missing time to 200ms here. Maybe doing
> >>>> nothing in this case would be more appropriate here. I mean, why should
> >>>> you slow down barebox here when some code triggers the watchdog too
> >>>> often?
> >>>>
> >>>>> +		return da9063_watchdog_ping(priv);
> >>>>
> >>>> Drop this, just fall through.
> >>>
> >>> Just prepared a v2 with a busy wait after discussed it with Lucas.
> >>> Thanks for your input too :)
> >>>
> >>> Regards,
> >>>   Marco
> >>>
> >>>> Sascha
> >>>>
> >>>> -- 
> >>>> Pengutronix e.K.                           |                             |
> >>>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> >>>> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> >>>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >>>>
> >>>
> >>
> >> -- 
> >> Pengutronix e.K.                           |                             |
> >> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> >> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >>
> > 
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox