[Qemu-devel] [PATCH] Check correct register for clock source

Amithash Prasad posted 1 patch 4 years, 6 months ago
Test docker-clang@ubuntu failed
Test FreeBSD passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190906011010.1135084-1-amithash@fb.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>, "Cédric Le Goater" <clg@kaod.org>
hw/watchdog/wdt_aspeed.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[Qemu-devel] [PATCH] Check correct register for clock source
Posted by Amithash Prasad 4 years, 6 months ago
When WDT_RESTART is written, the data is not the contents
of the WDT_CTRL register. Hence ensure we are looking at
WDT_CTRL to check if bit WDT_CTRL_1MHZ_CLK is set or not.

Signed-off-by: Amithash Prasad <amithash@fb.com>
---
 hw/watchdog/wdt_aspeed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index 9b93213417..f710036535 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -140,7 +140,7 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,
     case WDT_RESTART:
         if ((data & 0xFFFF) == WDT_RESTART_MAGIC) {
             s->regs[WDT_STATUS] = s->regs[WDT_RELOAD_VALUE];
-            aspeed_wdt_reload(s, !(data & WDT_CTRL_1MHZ_CLK));
+            aspeed_wdt_reload(s, !(s->regs[WDT_CTRL] & WDT_CTRL_1MHZ_CLK));
         }
         break;
     case WDT_CTRL:
-- 
2.21.0


Re: [Qemu-devel] [PATCH] Check correct register for clock source
Posted by Joel Stanley 4 years, 6 months ago
On Fri, 6 Sep 2019 at 01:10, Amithash Prasad <amithash@fb.com> wrote:
>
> When WDT_RESTART is written, the data is not the contents
> of the WDT_CTRL register. Hence ensure we are looking at
> WDT_CTRL to check if bit WDT_CTRL_1MHZ_CLK is set or not.
>
> Signed-off-by: Amithash Prasad <amithash@fb.com>

Reviewed-by: Joel Stanley <joel@jms.id.au>

Thanks for the patch!

> ---
>  hw/watchdog/wdt_aspeed.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
> index 9b93213417..f710036535 100644
> --- a/hw/watchdog/wdt_aspeed.c
> +++ b/hw/watchdog/wdt_aspeed.c
> @@ -140,7 +140,7 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,
>      case WDT_RESTART:
>          if ((data & 0xFFFF) == WDT_RESTART_MAGIC) {
>              s->regs[WDT_STATUS] = s->regs[WDT_RELOAD_VALUE];
> -            aspeed_wdt_reload(s, !(data & WDT_CTRL_1MHZ_CLK));
> +            aspeed_wdt_reload(s, !(s->regs[WDT_CTRL] & WDT_CTRL_1MHZ_CLK));
>          }
>          break;
>      case WDT_CTRL:
> --
> 2.21.0
>

Re: [Qemu-devel] [PATCH] Check correct register for clock source
Posted by Cédric Le Goater 4 years, 6 months ago
On 06/09/2019 03:16, Joel Stanley wrote:
> On Fri, 6 Sep 2019 at 01:10, Amithash Prasad <amithash@fb.com> wrote:
>>
>> When WDT_RESTART is written, the data is not the contents
>> of the WDT_CTRL register. Hence ensure we are looking at
>> WDT_CTRL to check if bit WDT_CTRL_1MHZ_CLK is set or not.
>>
>> Signed-off-by: Amithash Prasad <amithash@fb.com>
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> 
> Thanks for the patch!

yes. I have pushed it on the aspeed-4.2 branch but it can go independently
as there are no conflicts. I changed the title slightly to reflect the
area being changed.

On the clock topic, we should consider looking at the "Clock framework API" 
from  Damien Hedde :

	http://patchwork.ozlabs.org/cover/1157671/

Thanks,

C. 

> 
>> ---
>>  hw/watchdog/wdt_aspeed.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
>> index 9b93213417..f710036535 100644
>> --- a/hw/watchdog/wdt_aspeed.c
>> +++ b/hw/watchdog/wdt_aspeed.c
>> @@ -140,7 +140,7 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,
>>      case WDT_RESTART:
>>          if ((data & 0xFFFF) == WDT_RESTART_MAGIC) {
>>              s->regs[WDT_STATUS] = s->regs[WDT_RELOAD_VALUE];
>> -            aspeed_wdt_reload(s, !(data & WDT_CTRL_1MHZ_CLK));
>> +            aspeed_wdt_reload(s, !(s->regs[WDT_CTRL] & WDT_CTRL_1MHZ_CLK));
>>          }
>>          break;
>>      case WDT_CTRL:
>> --
>> 2.21.0
>>


Re: [Qemu-devel] [PATCH] Check correct register for clock source
Posted by Amithash Prasad 4 years, 6 months ago
>>  yes. I have pushed it on the aspeed-4.2 branch but it can go independently
>>as there are no conflicts. I changed the title slightly to reflect the
>> area being changed.
Thanks! If required, I can change the patch title and resubmit.