[PATCH 1/2] platform: sifive_fu740: do not use a global in da9063_reset/shutdown

[PATCH 1/2] platform: sifive_fu740: do not use a global in da9063_reset/shutdown

[Aurelien Jarno]

da9063_reset() and da9063_shutdown() take the chip address in argument
(like similar functions), but in practice use the da9063 global struct
instead. Fix that.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 platform/generic/sifive_fu740.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/platform/generic/sifive_fu740.c b/platform/generic/sifive_fu740.c
index 333b3fb..866e924 100644
--- a/platform/generic/sifive_fu740.c
+++ b/platform/generic/sifive_fu740.c
@@ -81,32 +81,32 @@ static inline int da9063_sanity_check(struct i2c_adapter *adap, uint32_t reg)
 
 static inline int da9063_shutdown(struct i2c_adapter *adap, uint32_t reg)
 {
-	int rc = i2c_adapter_reg_write(adap, da9063.reg,
+	int rc = i2c_adapter_reg_write(adap, reg,
 					DA9063_REG_PAGE_CON, 0x00);
 
 	if (rc)
 		return rc;
 
-	return i2c_adapter_reg_write(adap, da9063.reg,
+	return i2c_adapter_reg_write(adap, reg,
 				     DA9063_REG_CONTROL_F,
 				     DA9063_CONTROL_F_SHUTDOWN);
 }
 
 static inline int da9063_reset(struct i2c_adapter *adap, uint32_t reg)
 {
-	int rc = i2c_adapter_reg_write(adap, da9063.reg,
+	int rc = i2c_adapter_reg_write(adap, reg,
 					DA9063_REG_PAGE_CON, 0x00);
 
 	if (rc)
 		return rc;
 
-	rc = i2c_adapter_reg_write(adap, da9063.reg,
+	rc = i2c_adapter_reg_write(adap, reg,
 				   DA9063_REG_CONTROL_F,
 				   DA9063_CONTROL_F_WAKEUP);
 	if (rc)
 		return rc;
 
-	return i2c_adapter_reg_write(adap, da9063.reg,
+	return i2c_adapter_reg_write(adap, reg,
 				DA9063_REG_CONTROL_A,
 				DA9063_CONTROL_A_M_POWER1_EN |
 				DA9063_CONTROL_A_M_POWER_EN |
-- 
2.34.1

[PATCH 2/2] platform: sifive_fu740: fix reset when watchdog is running

[Aurelien Jarno]

When the watchdog is running the HiFive Unmatched board does not reboot
properly and shuts down itself a few seconds after reboot, in the early
stages of the u-boot loading. On a Linux kernel this happens when the
da9063_wdt module is loaded. This does not happen if the module is
unloaded before reboot or if the watchdog module is loaded with
"stop_on_reboot=1".

Fix that by stopping the watchdog before attempting to reset the board.
This is done by zeroing the TWDSCALE field of CONTROL_D register, unless
it was already set to 0.

Reported-by: Tianon Gravi <tianon@debian.org>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 platform/generic/sifive_fu740.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/platform/generic/sifive_fu740.c b/platform/generic/sifive_fu740.c
index 866e924..f595c04 100644
--- a/platform/generic/sifive_fu740.c
+++ b/platform/generic/sifive_fu740.c
@@ -22,6 +22,7 @@
 
 #define DA9063_REG_PAGE_CON		0x00
 #define DA9063_REG_CONTROL_A		0x0e
+#define DA9063_REG_CONTROL_D		0x11
 #define DA9063_REG_CONTROL_F		0x13
 #define DA9063_REG_DEVICE_ID		0x81
 
@@ -29,6 +30,8 @@
 #define DA9063_CONTROL_A_M_POWER_EN	(1 << 5)
 #define DA9063_CONTROL_A_STANDBY	(1 << 3)
 
+#define DA9063_CONTROL_D_TWDSCALE_MASK	0x07
+
 #define DA9063_CONTROL_F_WAKEUP	(1 << 2)
 #define DA9063_CONTROL_F_SHUTDOWN	(1 << 1)
 
@@ -79,6 +82,27 @@ static inline int da9063_sanity_check(struct i2c_adapter *adap, uint32_t reg)
 	return 0;
 }
 
+static inline int da9063_stop_watchdog(struct i2c_adapter *adap, uint32_t reg)
+{
+	uint8_t val;
+	int rc = i2c_adapter_reg_write(adap, reg,
+					DA9063_REG_PAGE_CON, 0x00);
+
+	if (rc)
+		return rc;
+
+	rc = i2c_adapter_reg_read(adap, reg, DA9063_REG_CONTROL_D, &val);
+	if (rc)
+		return rc;
+
+	if ((val & DA9063_CONTROL_D_TWDSCALE_MASK) == 0)
+		return 0;
+
+	val &= ~DA9063_CONTROL_D_TWDSCALE_MASK;
+
+	return i2c_adapter_reg_write(adap, reg, DA9063_REG_CONTROL_D, val);
+}
+
 static inline int da9063_shutdown(struct i2c_adapter *adap, uint32_t reg)
 {
 	int rc = i2c_adapter_reg_write(adap, reg,
@@ -133,6 +157,7 @@ static void da9063_system_reset(u32 type, u32 reason)
 			break;
 		case SBI_SRST_RESET_TYPE_COLD_REBOOT:
 		case SBI_SRST_RESET_TYPE_WARM_REBOOT:
+			da9063_stop_watchdog(adap, reg);
 			da9063_reset(adap, reg);
 			break;
 		}
-- 
2.34.1

[PATCH 2/2] platform: sifive_fu740: fix reset when watchdog is running

[Nikita Shubin]

Hello Aurelien!

Adding David from SiFive...

On Wed,  5 Jan 2022 08:20:39 +0100
Aurelien Jarno <aurelien@aurel32.net> wrote:

> When the watchdog is running the HiFive Unmatched board does not
> reboot properly and shuts down itself a few seconds after reboot, in
> the early stages of the u-boot loading. On a Linux kernel this
> happens when the da9063_wdt module is loaded. This does not happen if
> the module is unloaded before reboot or if the watchdog module is
> loaded with "stop_on_reboot=1".

| A running application is typically in ACTIVE mode. The DA9063
| transitions to ACTIVE mode after the host processor performs at least
| one initial ?alive? watchdog write (or alternatively an initial
| assertion of the KEEP_ACT port) inside the target time window. If the
| WATCHDOG function is disabled by setting TWDSCALE to zero, the DA9063
| transitions to ACTIVE mode when all of the sequencer IDs in the POWER
| domain are complete.

Is this that's case mentioned ? What if we press a reset key when
watchdog is enabled ? Or if it was reseted by thermal sensor ?

Can we disable watchdog on start instead of disabling it before a reset
?

Could you please give us a link to actual report ?

> 
> Fix that by stopping the watchdog before attempting to reset the
> board. This is done by zeroing the TWDSCALE field of CONTROL_D
> register, unless it was already set to 0.
> 
> Reported-by: Tianon Gravi <tianon@debian.org>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  platform/generic/sifive_fu740.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/platform/generic/sifive_fu740.c
> b/platform/generic/sifive_fu740.c index 866e924..f595c04 100644
> --- a/platform/generic/sifive_fu740.c
> +++ b/platform/generic/sifive_fu740.c
> @@ -22,6 +22,7 @@
>  
>  #define DA9063_REG_PAGE_CON		0x00
>  #define DA9063_REG_CONTROL_A		0x0e
> +#define DA9063_REG_CONTROL_D		0x11
>  #define DA9063_REG_CONTROL_F		0x13
>  #define DA9063_REG_DEVICE_ID		0x81
>  
> @@ -29,6 +30,8 @@
>  #define DA9063_CONTROL_A_M_POWER_EN	(1 << 5)
>  #define DA9063_CONTROL_A_STANDBY	(1 << 3)
>  
> +#define DA9063_CONTROL_D_TWDSCALE_MASK	0x07
> +
>  #define DA9063_CONTROL_F_WAKEUP	(1 << 2)
>  #define DA9063_CONTROL_F_SHUTDOWN	(1 << 1)
>  
> @@ -79,6 +82,27 @@ static inline int da9063_sanity_check(struct
> i2c_adapter *adap, uint32_t reg) return 0;
>  }
>  
> +static inline int da9063_stop_watchdog(struct i2c_adapter *adap,
> uint32_t reg) +{
> +	uint8_t val;
> +	int rc = i2c_adapter_reg_write(adap, reg,
> +					DA9063_REG_PAGE_CON, 0x00);
> +
> +	if (rc)
> +		return rc;
> +
> +	rc = i2c_adapter_reg_read(adap, reg, DA9063_REG_CONTROL_D,
> &val);
> +	if (rc)
> +		return rc;
> +
> +	if ((val & DA9063_CONTROL_D_TWDSCALE_MASK) == 0)
> +		return 0;
> +
> +	val &= ~DA9063_CONTROL_D_TWDSCALE_MASK;
> +
> +	return i2c_adapter_reg_write(adap, reg,
> DA9063_REG_CONTROL_D, val); +}
> +
>  static inline int da9063_shutdown(struct i2c_adapter *adap, uint32_t
> reg) {
>  	int rc = i2c_adapter_reg_write(adap, reg,
> @@ -133,6 +157,7 @@ static void da9063_system_reset(u32 type, u32
> reason) break;
>  		case SBI_SRST_RESET_TYPE_COLD_REBOOT:
>  		case SBI_SRST_RESET_TYPE_WARM_REBOOT:
> +			da9063_stop_watchdog(adap, reg);
>  			da9063_reset(adap, reg);
>  			break;
>  		}

[PATCH 2/2] platform: sifive_fu740: fix reset when watchdog is running

[David Abdurachmanov]

On Wed, Jan 5, 2022 at 3:40 PM Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>
> Hello Aurelien!
>
> Adding David from SiFive...
>
> On Wed,  5 Jan 2022 08:20:39 +0100
> Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> > When the watchdog is running the HiFive Unmatched board does not
> > reboot properly and shuts down itself a few seconds after reboot, in
> > the early stages of the u-boot loading. On a Linux kernel this
> > happens when the da9063_wdt module is loaded. This does not happen if
> > the module is unloaded before reboot or if the watchdog module is
> > loaded with "stop_on_reboot=1".
>
> | A running application is typically in ACTIVE mode. The DA9063
> | transitions to ACTIVE mode after the host processor performs at least
> | one initial ?alive? watchdog write (or alternatively an initial
> | assertion of the KEEP_ACT port) inside the target time window. If the
> | WATCHDOG function is disabled by setting TWDSCALE to zero, the DA9063
> | transitions to ACTIVE mode when all of the sequencer IDs in the POWER
> | domain are complete.
>
> Is this that's case mentioned ? What if we press a reset key when
> watchdog is enabled ? Or if it was reseted by thermal sensor ?
>
> Can we disable watchdog on start instead of disabling it before a reset
> ?

I would suggest doing this too. Disable watchdog on the start.

IIRC watchdog DA9063 can do 131 seconds (max). Alternative would be to
reconfigure the timer for 131 seconds, kick it in OpenSBI to give the
most possible time and reboot. Hopefully that would be enough time to
reboot and take control of the watchdog again. I don't know if U-Boot
has a DA9063 watchdog driver.

Disabling it is probably a better option :) System boot with watchdog
disabled and then it can be enabled again.

david

>
> Could you please give us a link to actual report ?
>
> >
> > Fix that by stopping the watchdog before attempting to reset the
> > board. This is done by zeroing the TWDSCALE field of CONTROL_D
> > register, unless it was already set to 0.
> >
> > Reported-by: Tianon Gravi <tianon@debian.org>
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> >  platform/generic/sifive_fu740.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/platform/generic/sifive_fu740.c
> > b/platform/generic/sifive_fu740.c index 866e924..f595c04 100644
> > --- a/platform/generic/sifive_fu740.c
> > +++ b/platform/generic/sifive_fu740.c
> > @@ -22,6 +22,7 @@
> >
> >  #define DA9063_REG_PAGE_CON          0x00
> >  #define DA9063_REG_CONTROL_A         0x0e
> > +#define DA9063_REG_CONTROL_D         0x11
> >  #define DA9063_REG_CONTROL_F         0x13
> >  #define DA9063_REG_DEVICE_ID         0x81
> >
> > @@ -29,6 +30,8 @@
> >  #define DA9063_CONTROL_A_M_POWER_EN  (1 << 5)
> >  #define DA9063_CONTROL_A_STANDBY     (1 << 3)
> >
> > +#define DA9063_CONTROL_D_TWDSCALE_MASK       0x07
> > +
> >  #define DA9063_CONTROL_F_WAKEUP      (1 << 2)
> >  #define DA9063_CONTROL_F_SHUTDOWN    (1 << 1)
> >
> > @@ -79,6 +82,27 @@ static inline int da9063_sanity_check(struct
> > i2c_adapter *adap, uint32_t reg) return 0;
> >  }
> >
> > +static inline int da9063_stop_watchdog(struct i2c_adapter *adap,
> > uint32_t reg) +{
> > +     uint8_t val;
> > +     int rc = i2c_adapter_reg_write(adap, reg,
> > +                                     DA9063_REG_PAGE_CON, 0x00);
> > +
> > +     if (rc)
> > +             return rc;
> > +
> > +     rc = i2c_adapter_reg_read(adap, reg, DA9063_REG_CONTROL_D,
> > &val);
> > +     if (rc)
> > +             return rc;
> > +
> > +     if ((val & DA9063_CONTROL_D_TWDSCALE_MASK) == 0)
> > +             return 0;
> > +
> > +     val &= ~DA9063_CONTROL_D_TWDSCALE_MASK;
> > +
> > +     return i2c_adapter_reg_write(adap, reg,
> > DA9063_REG_CONTROL_D, val); +}
> > +
> >  static inline int da9063_shutdown(struct i2c_adapter *adap, uint32_t
> > reg) {
> >       int rc = i2c_adapter_reg_write(adap, reg,
> > @@ -133,6 +157,7 @@ static void da9063_system_reset(u32 type, u32
> > reason) break;
> >               case SBI_SRST_RESET_TYPE_COLD_REBOOT:
> >               case SBI_SRST_RESET_TYPE_WARM_REBOOT:
> > +                     da9063_stop_watchdog(adap, reg);
> >                       da9063_reset(adap, reg);
> >                       break;
> >               }
>

Fwd: [PATCH 2/2] platform: sifive_fu740: fix reset when watchdog is running

[Heinrich Schuchardt]

David Abdurachmanov <david.abdurachmanov@sifive.com> wrote:
> On Wed, Jan 5, 2022 at 3:40 PM Nikita Shubin <nikita.shubin@maquefel.me> 
> wrote:
>> Hello Aurelien!
>>
>> Adding David from SiFive...
>>
>> On Wed,  5 Jan 2022 08:20:39 +0100
>> Aurelien Jarno<aurelien@aurel32.net>  wrote:
>>
>>> When the watchdog is running the HiFive Unmatched board does not
>>> reboot properly and shuts down itself a few seconds after reboot, in
>>> the early stages of the u-boot loading. On a Linux kernel this
>>> happens when the da9063_wdt module is loaded. This does not happen if
>>> the module is unloaded before reboot or if the watchdog module is
>>> loaded with "stop_on_reboot=1".
>>
>> | A running application is typically in ACTIVE mode. The DA9063
>> | transitions to ACTIVE mode after the host processor performs at least
>> | one initial ?alive? watchdog write (or alternatively an initial
>> | assertion of the KEEP_ACT port) inside the target time window. If the
>> | WATCHDOG function is disabled by setting TWDSCALE to zero, the DA9063
>> | transitions to ACTIVE mode when all of the sequencer IDs in the POWER
>> | domain are complete.
>>
>> Is this that's case mentioned ? What if we press a reset key when
>> watchdog is enabled ? Or if it was reseted by thermal sensor ?
>>
>> Can we disable watchdog on start instead of disabling it before a reset
>> ?
> 
> I would suggest doing this too. Disable watchdog on the start.


An operating system or a firmware (EDK II, U-Boot) may enable the 
watchdog for its internal use. When the SRST extension is called we 
should not assume any specific state of the watchdog. Instead put the 
watchdog into a known state.

Additionally putting the watchdog into a known state when OpenSBI is 
initialized is a sound idea.

After Linux crashes (missing root file system) of the Unmatched board I 
have reproducibly experienced a state where pressing the reset button 
did not lead to a reboot.

> 
> IIRC watchdog DA9063 can do 131 seconds (max). Alternative would be to
> reconfigure the timer for 131 seconds, kick it in OpenSBI to give the
> most possible time and reboot. Hopefully that would be enough time to
> reboot and take control of the watchdog again. I don't know if U-Boot
> has a DA9063 watchdog driver.

We should not make any assumptions about the next boot stage. Our 
solution must both work payloads with and without a driver for the 
watchdog. Hence the watchdog should be inactive when calling the payload.

Best regards

Heinrich

> 
> Disabling it is probably a better option :) System boot with watchdog
> disabled and then it can be enabled again.
> 
> david
> 
>> Could you please give us a link to actual report ?
>>
>>> Fix that by stopping the watchdog before attempting to reset the
>>> board. This is done by zeroing the TWDSCALE field of CONTROL_D
>>> register, unless it was already set to 0.
>>>
>>> Reported-by: Tianon Gravi<tianon@debian.org>
>>> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
>>> ---
>>>   platform/generic/sifive_fu740.c | 25 +++++++++++++++++++++++++
>>>   1 file changed, 25 insertions(+)
>>>
>>> diff --git a/platform/generic/sifive_fu740.c
>>> b/platform/generic/sifive_fu740.c index 866e924..f595c04 100644
>>> --- a/platform/generic/sifive_fu740.c
>>> +++ b/platform/generic/sifive_fu740.c
>>> @@ -22,6 +22,7 @@
>>>
>>>   #define DA9063_REG_PAGE_CON          0x00
>>>   #define DA9063_REG_CONTROL_A         0x0e
>>> +#define DA9063_REG_CONTROL_D         0x11
>>>   #define DA9063_REG_CONTROL_F         0x13
>>>   #define DA9063_REG_DEVICE_ID         0x81
>>>
>>> @@ -29,6 +30,8 @@
>>>   #define DA9063_CONTROL_A_M_POWER_EN  (1 << 5)
>>>   #define DA9063_CONTROL_A_STANDBY     (1 << 3)
>>>
>>> +#define DA9063_CONTROL_D_TWDSCALE_MASK       0x07
>>> +
>>>   #define DA9063_CONTROL_F_WAKEUP      (1 << 2)
>>>   #define DA9063_CONTROL_F_SHUTDOWN    (1 << 1)
>>>
>>> @@ -79,6 +82,27 @@ static inline int da9063_sanity_check(struct
>>> i2c_adapter *adap, uint32_t reg) return 0;
>>>   }
>>>
>>> +static inline int da9063_stop_watchdog(struct i2c_adapter *adap,
>>> uint32_t reg) +{
>>> +     uint8_t val;
>>> +     int rc = i2c_adapter_reg_write(adap, reg,
>>> +                                     DA9063_REG_PAGE_CON, 0x00);
>>> +
>>> +     if (rc)
>>> +             return rc;
>>> +
>>> +     rc = i2c_adapter_reg_read(adap, reg, DA9063_REG_CONTROL_D,
>>> &val);
>>> +     if (rc)
>>> +             return rc;
>>> +
>>> +     if ((val & DA9063_CONTROL_D_TWDSCALE_MASK) == 0)
>>> +             return 0;
>>> +
>>> +     val &= ~DA9063_CONTROL_D_TWDSCALE_MASK;
>>> +
>>> +     return i2c_adapter_reg_write(adap, reg,
>>> DA9063_REG_CONTROL_D, val); +}
>>> +
>>>   static inline int da9063_shutdown(struct i2c_adapter *adap, uint32_t
>>> reg) {
>>>        int rc = i2c_adapter_reg_write(adap, reg,
>>> @@ -133,6 +157,7 @@ static void da9063_system_reset(u32 type, u32
>>> reason) break;
>>>                case SBI_SRST_RESET_TYPE_COLD_REBOOT:
>>>                case SBI_SRST_RESET_TYPE_WARM_REBOOT:
>>> +                     da9063_stop_watchdog(adap, reg);
>>>                        da9063_reset(adap, reg);
>>>                        break;
>>>                }
>>
> 
> -- 
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
> 

[PATCH 2/2] platform: sifive_fu740: fix reset when watchdog is running

[Aurelien Jarno]

On 2022-01-05 18:16, David Abdurachmanov wrote:
> On Wed, Jan 5, 2022 at 3:40 PM Nikita Shubin <nikita.shubin@maquefel.me> wrote:
> >
> > Hello Aurelien!
> >
> > Adding David from SiFive...
> >
> > On Wed,  5 Jan 2022 08:20:39 +0100
> > Aurelien Jarno <aurelien@aurel32.net> wrote:
> >
> > > When the watchdog is running the HiFive Unmatched board does not
> > > reboot properly and shuts down itself a few seconds after reboot, in
> > > the early stages of the u-boot loading. On a Linux kernel this
> > > happens when the da9063_wdt module is loaded. This does not happen if
> > > the module is unloaded before reboot or if the watchdog module is
> > > loaded with "stop_on_reboot=1".
> >
> > | A running application is typically in ACTIVE mode. The DA9063
> > | transitions to ACTIVE mode after the host processor performs at least
> > | one initial ?alive? watchdog write (or alternatively an initial
> > | assertion of the KEEP_ACT port) inside the target time window. If the
> > | WATCHDOG function is disabled by setting TWDSCALE to zero, the DA9063
> > | transitions to ACTIVE mode when all of the sequencer IDs in the POWER
> > | domain are complete.

Yes, this seems to match that. The da9063_wdt module is responsible for
doing the ?alive? watchdog writes.

> > Is this that's case mentioned ? What if we press a reset key when
> > watchdog is enabled ? Or if it was reseted by thermal sensor ?

I am not sure what happens in that case and unfortunately I do not have
a board when I can try that.

> > Can we disable watchdog on start instead of disabling it before a reset
> > ?
> 
> I would suggest doing this too. Disable watchdog on the start.

What do you mean by on the start? At the beginning of OpenSBI, as soon
as it boots?

In that case it might be necessary to do an ?alive? watchdog write just
before the reset, as we do not know when the OS did the last write, and
we have no way of knowing that. Also in that case we might want to
increase the timeout, as currently the board gets powered-off in U-Boot
SPL, before displaying the OpenSBI banner, so the execution of OpenSBI
might not have started yet.

All the above looks complex compared to disabling the watchdog before
the reset sequence. And leaving the watchdog running does not seem to
bring any advantage, as it just powers off the board. I would have
expected it resets it again.

> IIRC watchdog DA9063 can do 131 seconds (max). Alternative would be to
> reconfigure the timer for 131 seconds, kick it in OpenSBI to give the
> most possible time and reboot. Hopefully that would be enough time to
> reboot and take control of the watchdog again. I don't know if U-Boot
> has a DA9063 watchdog driver.
> 
> Disabling it is probably a better option :) System boot with watchdog
> disabled and then it can be enabled again.

u-boot has support for watchdog, but there is no support for the DA9063
driver, so it's probably a good idea to just disable it.

> > Could you please give us a link to actual report ?

Unfortunately I do not have a link to share. This was discussed on IRC.
I built a Debian kernel with the SRST [1] patch and asked people to test
it on an Unmatched board against OpenSBI 1.0 to check if it was working.
Tianon reported me quickly that it was not working, the board starts to
reboot properly with:

| [  372.508832] reboot: Restarting system
|
| U-Boot SPL 2022.01-rc4+dfsg-1 (Dec 21 2021 - 02:38:20 +0000)
| Trying to boot from MMC1

But then the board powers off ("all the lights and even the fan are off
too").

We tracked that the issue was a bad interaction with the da9063 kernel
modules and then narrowed it down to the watchdog module.

Note that it is using kernel 5.16-rc7 with the DA9063 support enabled in
both the config and the DT [2].

If you need more details, Tianon (Cc:ed) can probably provide them.


[1] https://lists.infradead.org/pipermail/linux-riscv/2021-June/006998.html
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.16-rc8&id=cd29cc8ad2540a4f9a0a3e174394d39e648ef941

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien at aurel32.net                 http://www.aurel32.net

[PATCH 2/2] platform: sifive_fu740: fix reset when watchdog is running

[Nikita Shubin]

Hello Aurelien!

If it happens during U-Boot SPL that's a bad thing to happen, the ZSBL
reading from SD card is slow, so we can catch up only in SPL.

First i blamed our non-conforming reset theme, but since Heinrich
reported that he has the same behavior even for reset button, i should
assume that:

- It looks like TWDSCALE setting are not being read from OTP
- We end up in feeding/disabling watchdog early in SPL, and no way to
  boot if something happened with U-Boot SPL (removing the
  battery/unplugging the cord ?).

The official manual states following:

| If required, the application supervision by the WATCHDOG timer can be
|  continued in POWERDOWN mode via WATCHDOG_PD. If the host will not
|  communicate with the DA9063 during POWERDOWN mode, then the control
|  interfaces may also be temporarily disabled (see controls
|  PMIF_DIS/HS2WIRE_DIS)

We don't have WATCHDOG_PD set anywhere, so that's not the reason.

| Once in the ACTIVE state, the DA9063 continues to monitor the system
| unless it is disabled by setting TWDSCALE to zero. When powering
| down from ACTIVE mode, the watchdog monitor is stopped unless it
| enters POWERDOWN mode via WATCHDOG_PD.

| After powering up domain POWER, the DA9063 can initiate a watchdog
| monitor function. The host processor must write a 1 within a
| configured tWD_MAX time into control WATCHDOG, thereby indicating
| that the host is alive. If the host does not write 1 to this watchdog
| bit within the tWD_MAX time, the DA9063 asserts TWD_ERROR in the
| FAULT_LOG register and powers down to RESET mode.

Looks like we are experiencing that exact behaviour, through "can
initiate a watchdog monitor function" bugs me.

On boot: 
da9063_check_watchdog_pd: DA9063_REG_CONTROL_B=0x9
da9063_check_watchdog_pd: DA9063_REG_CONTROL_C=0xd3

On reset:
da9063_check_watchdog_pd: DA9063_REG_CONTROL_B=0x9
da9063_check_watchdog_pd: DA9063_REG_CONTROL_C=0xd3

We can do something (if really can catch up with WDG time window) only
in U-Boot SPL.

Since this affects only Unmatched and is located in platform code, i
think that at least we can have a way of "normal" reboot, but it looks
like da9063 WDG should be avoided.

Anup, what do you think about all this ?

Anyway, i've tested you patch and it works as expected.

Reviewed-by: Nikita Shubin <n.shubin@yadro.com>
Tested-by: Nikita Shubin <n.shubin@yadro.com>

On Wed, 5 Jan 2022 17:51:19 +0100
Aurelien Jarno <aurelien@aurel32.net> wrote:

> On 2022-01-05 18:16, David Abdurachmanov wrote:
> > On Wed, Jan 5, 2022 at 3:40 PM Nikita Shubin
> > <nikita.shubin@maquefel.me> wrote:  
> > >
> > > Hello Aurelien!
> > >
> > > Adding David from SiFive...
> > >
> > > On Wed,  5 Jan 2022 08:20:39 +0100
> > > Aurelien Jarno <aurelien@aurel32.net> wrote:
> > >  
> > > > When the watchdog is running the HiFive Unmatched board does not
> > > > reboot properly and shuts down itself a few seconds after
> > > > reboot, in the early stages of the u-boot loading. On a Linux
> > > > kernel this happens when the da9063_wdt module is loaded. This
> > > > does not happen if the module is unloaded before reboot or if
> > > > the watchdog module is loaded with "stop_on_reboot=1".  
> > >
> > > | A running application is typically in ACTIVE mode. The DA9063
> > > | transitions to ACTIVE mode after the host processor performs at
> > > least | one initial ?alive? watchdog write (or alternatively an
> > > initial | assertion of the KEEP_ACT port) inside the target time
> > > window. If the | WATCHDOG function is disabled by setting
> > > TWDSCALE to zero, the DA9063 | transitions to ACTIVE mode when
> > > all of the sequencer IDs in the POWER | domain are complete.  
> 
> Yes, this seems to match that. The da9063_wdt module is responsible
> for doing the ?alive? watchdog writes.
> 
> > > Is this that's case mentioned ? What if we press a reset key when
> > > watchdog is enabled ? Or if it was reseted by thermal sensor ?  
> 
> I am not sure what happens in that case and unfortunately I do not
> have a board when I can try that.
> 
> > > Can we disable watchdog on start instead of disabling it before a
> > > reset ?  
> > 
> > I would suggest doing this too. Disable watchdog on the start.  
> 
> What do you mean by on the start? At the beginning of OpenSBI, as soon
> as it boots?
> 
> In that case it might be necessary to do an ?alive? watchdog write
> just before the reset, as we do not know when the OS did the last
> write, and we have no way of knowing that. Also in that case we might
> want to increase the timeout, as currently the board gets powered-off
> in U-Boot SPL, before displaying the OpenSBI banner, so the execution
> of OpenSBI might not have started yet.
> 
> All the above looks complex compared to disabling the watchdog before
> the reset sequence. And leaving the watchdog running does not seem to
> bring any advantage, as it just powers off the board. I would have
> expected it resets it again.
> 
> > IIRC watchdog DA9063 can do 131 seconds (max). Alternative would be
> > to reconfigure the timer for 131 seconds, kick it in OpenSBI to
> > give the most possible time and reboot. Hopefully that would be
> > enough time to reboot and take control of the watchdog again. I
> > don't know if U-Boot has a DA9063 watchdog driver.
> > 
> > Disabling it is probably a better option :) System boot with
> > watchdog disabled and then it can be enabled again.  
> 
> u-boot has support for watchdog, but there is no support for the
> DA9063 driver, so it's probably a good idea to just disable it.
> 
> > > Could you please give us a link to actual report ?  
> 
> Unfortunately I do not have a link to share. This was discussed on
> IRC. I built a Debian kernel with the SRST [1] patch and asked people
> to test it on an Unmatched board against OpenSBI 1.0 to check if it
> was working. Tianon reported me quickly that it was not working, the
> board starts to reboot properly with:
> 
> | [  372.508832] reboot: Restarting system
> |
> | U-Boot SPL 2022.01-rc4+dfsg-1 (Dec 21 2021 - 02:38:20 +0000)
> | Trying to boot from MMC1
> 
> But then the board powers off ("all the lights and even the fan are
> off too").
> 
> We tracked that the issue was a bad interaction with the da9063 kernel
> modules and then narrowed it down to the watchdog module.
> 
> Note that it is using kernel 5.16-rc7 with the DA9063 support enabled
> in both the config and the DT [2].
> 
> If you need more details, Tianon (Cc:ed) can probably provide them.
> 
> 
> [1]
> https://lists.infradead.org/pipermail/linux-riscv/2021-June/006998.html
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.16-rc8&id=cd29cc8ad2540a4f9a0a3e174394d39e648ef941
> 

[PATCH 2/2] platform: sifive_fu740: fix reset when watchdog is running

[David Abdurachmanov]

On Thu, Jan 6, 2022 at 3:10 PM Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>
> Hello Aurelien!
>
> If it happens during U-Boot SPL that's a bad thing to happen, the ZSBL
> reading from SD card is slow, so we can catch up only in SPL.
>
> First i blamed our non-conforming reset theme, but since Heinrich
> reported that he has the same behavior even for reset button, i should
> assume that:
>
> - It looks like TWDSCALE setting are not being read from OTP
> - We end up in feeding/disabling watchdog early in SPL, and no way to
>   boot if something happened with U-Boot SPL (removing the
>   battery/unplugging the cord ?).
>
> The official manual states following:
>
> | If required, the application supervision by the WATCHDOG timer can be
> |  continued in POWERDOWN mode via WATCHDOG_PD. If the host will not
> |  communicate with the DA9063 during POWERDOWN mode, then the control
> |  interfaces may also be temporarily disabled (see controls
> |  PMIF_DIS/HS2WIRE_DIS)
>
> We don't have WATCHDOG_PD set anywhere, so that's not the reason.
>
> | Once in the ACTIVE state, the DA9063 continues to monitor the system
> | unless it is disabled by setting TWDSCALE to zero. When powering
> | down from ACTIVE mode, the watchdog monitor is stopped unless it
> | enters POWERDOWN mode via WATCHDOG_PD.
>
> | After powering up domain POWER, the DA9063 can initiate a watchdog
> | monitor function. The host processor must write a 1 within a
> | configured tWD_MAX time into control WATCHDOG, thereby indicating
> | that the host is alive. If the host does not write 1 to this watchdog
> | bit within the tWD_MAX time, the DA9063 asserts TWD_ERROR in the
> | FAULT_LOG register and powers down to RESET mode.
>
> Looks like we are experiencing that exact behaviour, through "can
> initiate a watchdog monitor function" bugs me.
>
> On boot:
> da9063_check_watchdog_pd: DA9063_REG_CONTROL_B=0x9
> da9063_check_watchdog_pd: DA9063_REG_CONTROL_C=0xd3
>
> On reset:
> da9063_check_watchdog_pd: DA9063_REG_CONTROL_B=0x9
> da9063_check_watchdog_pd: DA9063_REG_CONTROL_C=0xd3
>
> We can do something (if really can catch up with WDG time window) only
> in U-Boot SPL.
>
> Since this affects only Unmatched and is located in platform code, i
> think that at least we can have a way of "normal" reboot, but it looks
> like da9063 WDG should be avoided.
>
> Anup, what do you think about all this ?

Ping.

I don't think I have seen a comment from Anup, and I don't see the
patch merged in opensbi.

david

>
> Anyway, i've tested you patch and it works as expected.
>
> Reviewed-by: Nikita Shubin <n.shubin@yadro.com>
> Tested-by: Nikita Shubin <n.shubin@yadro.com>
>
> On Wed, 5 Jan 2022 17:51:19 +0100
> Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> > On 2022-01-05 18:16, David Abdurachmanov wrote:
> > > On Wed, Jan 5, 2022 at 3:40 PM Nikita Shubin
> > > <nikita.shubin@maquefel.me> wrote:
> > > >
> > > > Hello Aurelien!
> > > >
> > > > Adding David from SiFive...
> > > >
> > > > On Wed,  5 Jan 2022 08:20:39 +0100
> > > > Aurelien Jarno <aurelien@aurel32.net> wrote:
> > > >
> > > > > When the watchdog is running the HiFive Unmatched board does not
> > > > > reboot properly and shuts down itself a few seconds after
> > > > > reboot, in the early stages of the u-boot loading. On a Linux
> > > > > kernel this happens when the da9063_wdt module is loaded. This
> > > > > does not happen if the module is unloaded before reboot or if
> > > > > the watchdog module is loaded with "stop_on_reboot=1".
> > > >
> > > > | A running application is typically in ACTIVE mode. The DA9063
> > > > | transitions to ACTIVE mode after the host processor performs at
> > > > least | one initial ?alive? watchdog write (or alternatively an
> > > > initial | assertion of the KEEP_ACT port) inside the target time
> > > > window. If the | WATCHDOG function is disabled by setting
> > > > TWDSCALE to zero, the DA9063 | transitions to ACTIVE mode when
> > > > all of the sequencer IDs in the POWER | domain are complete.
> >
> > Yes, this seems to match that. The da9063_wdt module is responsible
> > for doing the ?alive? watchdog writes.
> >
> > > > Is this that's case mentioned ? What if we press a reset key when
> > > > watchdog is enabled ? Or if it was reseted by thermal sensor ?
> >
> > I am not sure what happens in that case and unfortunately I do not
> > have a board when I can try that.
> >
> > > > Can we disable watchdog on start instead of disabling it before a
> > > > reset ?
> > >
> > > I would suggest doing this too. Disable watchdog on the start.
> >
> > What do you mean by on the start? At the beginning of OpenSBI, as soon
> > as it boots?
> >
> > In that case it might be necessary to do an ?alive? watchdog write
> > just before the reset, as we do not know when the OS did the last
> > write, and we have no way of knowing that. Also in that case we might
> > want to increase the timeout, as currently the board gets powered-off
> > in U-Boot SPL, before displaying the OpenSBI banner, so the execution
> > of OpenSBI might not have started yet.
> >
> > All the above looks complex compared to disabling the watchdog before
> > the reset sequence. And leaving the watchdog running does not seem to
> > bring any advantage, as it just powers off the board. I would have
> > expected it resets it again.
> >
> > > IIRC watchdog DA9063 can do 131 seconds (max). Alternative would be
> > > to reconfigure the timer for 131 seconds, kick it in OpenSBI to
> > > give the most possible time and reboot. Hopefully that would be
> > > enough time to reboot and take control of the watchdog again. I
> > > don't know if U-Boot has a DA9063 watchdog driver.
> > >
> > > Disabling it is probably a better option :) System boot with
> > > watchdog disabled and then it can be enabled again.
> >
> > u-boot has support for watchdog, but there is no support for the
> > DA9063 driver, so it's probably a good idea to just disable it.
> >
> > > > Could you please give us a link to actual report ?
> >
> > Unfortunately I do not have a link to share. This was discussed on
> > IRC. I built a Debian kernel with the SRST [1] patch and asked people
> > to test it on an Unmatched board against OpenSBI 1.0 to check if it
> > was working. Tianon reported me quickly that it was not working, the
> > board starts to reboot properly with:
> >
> > | [  372.508832] reboot: Restarting system
> > |
> > | U-Boot SPL 2022.01-rc4+dfsg-1 (Dec 21 2021 - 02:38:20 +0000)
> > | Trying to boot from MMC1
> >
> > But then the board powers off ("all the lights and even the fan are
> > off too").
> >
> > We tracked that the issue was a bad interaction with the da9063 kernel
> > modules and then narrowed it down to the watchdog module.
> >
> > Note that it is using kernel 5.16-rc7 with the DA9063 support enabled
> > in both the config and the DT [2].
> >
> > If you need more details, Tianon (Cc:ed) can probably provide them.
> >
> >
> > [1]
> > https://lists.infradead.org/pipermail/linux-riscv/2021-June/006998.html
> > [2]
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.16-rc8&id=cd29cc8ad2540a4f9a0a3e174394d39e648ef941
> >
>

[PATCH 2/2] platform: sifive_fu740: fix reset when watchdog is running

[Anup Patel]

On Wed, Jan 5, 2022 at 12:52 PM Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> When the watchdog is running the HiFive Unmatched board does not reboot
> properly and shuts down itself a few seconds after reboot, in the early
> stages of the u-boot loading. On a Linux kernel this happens when the
> da9063_wdt module is loaded. This does not happen if the module is
> unloaded before reboot or if the watchdog module is loaded with
> "stop_on_reboot=1".
>
> Fix that by stopping the watchdog before attempting to reset the board.
> This is done by zeroing the TWDSCALE field of CONTROL_D register, unless
> it was already set to 0.
>
> Reported-by: Tianon Gravi <tianon@debian.org>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Apologies for being slow on this patch.

Reviewed-by: Anup Patel <anup@brainfault.org>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup

> ---
>  platform/generic/sifive_fu740.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/platform/generic/sifive_fu740.c b/platform/generic/sifive_fu740.c
> index 866e924..f595c04 100644
> --- a/platform/generic/sifive_fu740.c
> +++ b/platform/generic/sifive_fu740.c
> @@ -22,6 +22,7 @@
>
>  #define DA9063_REG_PAGE_CON            0x00
>  #define DA9063_REG_CONTROL_A           0x0e
> +#define DA9063_REG_CONTROL_D           0x11
>  #define DA9063_REG_CONTROL_F           0x13
>  #define DA9063_REG_DEVICE_ID           0x81
>
> @@ -29,6 +30,8 @@
>  #define DA9063_CONTROL_A_M_POWER_EN    (1 << 5)
>  #define DA9063_CONTROL_A_STANDBY       (1 << 3)
>
> +#define DA9063_CONTROL_D_TWDSCALE_MASK 0x07
> +
>  #define DA9063_CONTROL_F_WAKEUP        (1 << 2)
>  #define DA9063_CONTROL_F_SHUTDOWN      (1 << 1)
>
> @@ -79,6 +82,27 @@ static inline int da9063_sanity_check(struct i2c_adapter *adap, uint32_t reg)
>         return 0;
>  }
>
> +static inline int da9063_stop_watchdog(struct i2c_adapter *adap, uint32_t reg)
> +{
> +       uint8_t val;
> +       int rc = i2c_adapter_reg_write(adap, reg,
> +                                       DA9063_REG_PAGE_CON, 0x00);
> +
> +       if (rc)
> +               return rc;
> +
> +       rc = i2c_adapter_reg_read(adap, reg, DA9063_REG_CONTROL_D, &val);
> +       if (rc)
> +               return rc;
> +
> +       if ((val & DA9063_CONTROL_D_TWDSCALE_MASK) == 0)
> +               return 0;
> +
> +       val &= ~DA9063_CONTROL_D_TWDSCALE_MASK;
> +
> +       return i2c_adapter_reg_write(adap, reg, DA9063_REG_CONTROL_D, val);
> +}
> +
>  static inline int da9063_shutdown(struct i2c_adapter *adap, uint32_t reg)
>  {
>         int rc = i2c_adapter_reg_write(adap, reg,
> @@ -133,6 +157,7 @@ static void da9063_system_reset(u32 type, u32 reason)
>                         break;
>                 case SBI_SRST_RESET_TYPE_COLD_REBOOT:
>                 case SBI_SRST_RESET_TYPE_WARM_REBOOT:
> +                       da9063_stop_watchdog(adap, reg);
>                         da9063_reset(adap, reg);
>                         break;
>                 }
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH 1/2] platform: sifive_fu740: do not use a global in da9063_reset/shutdown

[Nikita Shubin]

Hello Aurelien!

On Wed,  5 Jan 2022 08:20:38 +0100
Aurelien Jarno <aurelien@aurel32.net> wrote:

Seems good to me.

Reviewed-by: Nikita Shubin <n.shubin@yadro.com>

> da9063_reset() and da9063_shutdown() take the chip address in argument
> (like similar functions), but in practice use the da9063 global struct
> instead. Fix that.
> 
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  platform/generic/sifive_fu740.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/platform/generic/sifive_fu740.c
> b/platform/generic/sifive_fu740.c index 333b3fb..866e924 100644
> --- a/platform/generic/sifive_fu740.c
> +++ b/platform/generic/sifive_fu740.c
> @@ -81,32 +81,32 @@ static inline int da9063_sanity_check(struct
> i2c_adapter *adap, uint32_t reg) 
>  static inline int da9063_shutdown(struct i2c_adapter *adap, uint32_t
> reg) {
> -	int rc = i2c_adapter_reg_write(adap, da9063.reg,
> +	int rc = i2c_adapter_reg_write(adap, reg,
>  					DA9063_REG_PAGE_CON, 0x00);
>  
>  	if (rc)
>  		return rc;
>  
> -	return i2c_adapter_reg_write(adap, da9063.reg,
> +	return i2c_adapter_reg_write(adap, reg,
>  				     DA9063_REG_CONTROL_F,
>  				     DA9063_CONTROL_F_SHUTDOWN);
>  }
>  
>  static inline int da9063_reset(struct i2c_adapter *adap, uint32_t
> reg) {
> -	int rc = i2c_adapter_reg_write(adap, da9063.reg,
> +	int rc = i2c_adapter_reg_write(adap, reg,
>  					DA9063_REG_PAGE_CON, 0x00);
>  
>  	if (rc)
>  		return rc;
>  
> -	rc = i2c_adapter_reg_write(adap, da9063.reg,
> +	rc = i2c_adapter_reg_write(adap, reg,
>  				   DA9063_REG_CONTROL_F,
>  				   DA9063_CONTROL_F_WAKEUP);
>  	if (rc)
>  		return rc;
>  
> -	return i2c_adapter_reg_write(adap, da9063.reg,
> +	return i2c_adapter_reg_write(adap, reg,
>  				DA9063_REG_CONTROL_A,
>  				DA9063_CONTROL_A_M_POWER1_EN |
>  				DA9063_CONTROL_A_M_POWER_EN |

[PATCH 1/2] platform: sifive_fu740: do not use a global in da9063_reset/shutdown

[Xiang W]

? 2022-01-05???? 08:20 +0100?Aurelien Jarno???
> da9063_reset() and da9063_shutdown() take the chip address in argument
> (like similar functions), but in practice use the da9063 global struct
> instead. Fix that.
> 
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
look good to me.

Reviewed-by: Xiang W <wxjstz@126.com>
> ---
> ?platform/generic/sifive_fu740.c | 10 +++++-----
> ?1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/platform/generic/sifive_fu740.c
> b/platform/generic/sifive_fu740.c
> index 333b3fb..866e924 100644
> --- a/platform/generic/sifive_fu740.c
> +++ b/platform/generic/sifive_fu740.c
> @@ -81,32 +81,32 @@ static inline int da9063_sanity_check(struct
> i2c_adapter *adap, uint32_t reg)
> ?
> ?static inline int da9063_shutdown(struct i2c_adapter *adap, uint32_t
> reg)
> ?{
> -???????int rc = i2c_adapter_reg_write(adap, da9063.reg,
> +???????int rc = i2c_adapter_reg_write(adap, reg,
> ????????????????????????????????????????DA9063_REG_PAGE_CON, 0x00);
> ?
> ????????if (rc)
> ????????????????return rc;
> ?
> -???????return i2c_adapter_reg_write(adap, da9063.reg,
> +???????return i2c_adapter_reg_write(adap, reg,
> ???????????????????????????????????? DA9063_REG_CONTROL_F,
> ???????????????????????????????????? DA9063_CONTROL_F_SHUTDOWN);
> ?}
> ?
> ?static inline int da9063_reset(struct i2c_adapter *adap, uint32_t
> reg)
> ?{
> -???????int rc = i2c_adapter_reg_write(adap, da9063.reg,
> +???????int rc = i2c_adapter_reg_write(adap, reg,
> ????????????????????????????????????????DA9063_REG_PAGE_CON, 0x00);
> ?
> ????????if (rc)
> ????????????????return rc;
> ?
> -???????rc = i2c_adapter_reg_write(adap, da9063.reg,
> +???????rc = i2c_adapter_reg_write(adap, reg,
> ?????????????????????????????????? DA9063_REG_CONTROL_F,
> ?????????????????????????????????? DA9063_CONTROL_F_WAKEUP);
> ????????if (rc)
> ????????????????return rc;
> ?
> -???????return i2c_adapter_reg_write(adap, da9063.reg,
> +???????return i2c_adapter_reg_write(adap, reg,
> ????????????????????????????????DA9063_REG_CONTROL_A,
> ????????????????????????????????DA9063_CONTROL_A_M_POWER1_EN |
> ????????????????????????????????DA9063_CONTROL_A_M_POWER_EN |
> -- 
> 2.34.1
> 
> 

[PATCH 1/2] platform: sifive_fu740: do not use a global in da9063_reset/shutdown

[Anup Patel]

On Wed, Jan 5, 2022 at 12:52 PM Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> da9063_reset() and da9063_shutdown() take the chip address in argument
> (like similar functions), but in practice use the da9063 global struct
> instead. Fix that.
>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Apologies for being slow on this patch.

Reviewed-by: Anup Patel <anup@brainfault.org>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup

> ---
>  platform/generic/sifive_fu740.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/platform/generic/sifive_fu740.c b/platform/generic/sifive_fu740.c
> index 333b3fb..866e924 100644
> --- a/platform/generic/sifive_fu740.c
> +++ b/platform/generic/sifive_fu740.c
> @@ -81,32 +81,32 @@ static inline int da9063_sanity_check(struct i2c_adapter *adap, uint32_t reg)
>
>  static inline int da9063_shutdown(struct i2c_adapter *adap, uint32_t reg)
>  {
> -       int rc = i2c_adapter_reg_write(adap, da9063.reg,
> +       int rc = i2c_adapter_reg_write(adap, reg,
>                                         DA9063_REG_PAGE_CON, 0x00);
>
>         if (rc)
>                 return rc;
>
> -       return i2c_adapter_reg_write(adap, da9063.reg,
> +       return i2c_adapter_reg_write(adap, reg,
>                                      DA9063_REG_CONTROL_F,
>                                      DA9063_CONTROL_F_SHUTDOWN);
>  }
>
>  static inline int da9063_reset(struct i2c_adapter *adap, uint32_t reg)
>  {
> -       int rc = i2c_adapter_reg_write(adap, da9063.reg,
> +       int rc = i2c_adapter_reg_write(adap, reg,
>                                         DA9063_REG_PAGE_CON, 0x00);
>
>         if (rc)
>                 return rc;
>
> -       rc = i2c_adapter_reg_write(adap, da9063.reg,
> +       rc = i2c_adapter_reg_write(adap, reg,
>                                    DA9063_REG_CONTROL_F,
>                                    DA9063_CONTROL_F_WAKEUP);
>         if (rc)
>                 return rc;
>
> -       return i2c_adapter_reg_write(adap, da9063.reg,
> +       return i2c_adapter_reg_write(adap, reg,
>                                 DA9063_REG_CONTROL_A,
>                                 DA9063_CONTROL_A_M_POWER1_EN |
>                                 DA9063_CONTROL_A_M_POWER_EN |
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi