Am 4. Oktober 2026 15:05:57 UTC schrieb Bin Meng <[email protected]>:
>Hi Bernhard,
>
>On Sun, Oct 4, 2026 at 8:29 PM Bernhard Beschow <[email protected]> wrote:
>>
>> Guests such as u-boot may experience inadvertant reboots when configuring
>> the WCR register. Apparently there is a typo in the WCR locked bits
>> handling: The locked values for s->wcr must come from itself rather than
>> from s->wicr, so fix that.
>>
>> Fixes: daca13d495a1 ("hw/watchdog: Implement full i.MX watchdog support")
>> cc: qemu-stable
>> Signed-off-by: Bernhard Beschow <[email protected]>
>> ---
>> hw/watchdog/wdt_imx2.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/watchdog/wdt_imx2.c b/hw/watchdog/wdt_imx2.c
>> index c83d4107c1..3283990bbf 100644
>> --- a/hw/watchdog/wdt_imx2.c
>> +++ b/hw/watchdog/wdt_imx2.c
>> @@ -156,18 +156,18 @@ static void imx2_wdt_write(void *opaque, hwaddr addr,
>> case IMX2_WDT_WCR:
>> if (s->wcr_locked) {
>> value &= ~IMX2_WDT_WCR_LOCK_MASK;
>> - value |= (s->wicr & IMX2_WDT_WCR_LOCK_MASK);
>> + value |= (s->wcr & IMX2_WDT_WCR_LOCK_MASK);
>
>I see this patch has qemu-stable cc'ed, so it shall be a complete fix
>instead of a partial one that relies one another patch (in this case,
>patch 03 in this series).
>
>So patch 1 corrects the source register from s->wicr to s->wcr, but
>the WDE and WDT branches still use inverted masks:
>
>> s->wcr & ~IMX2_WDT_WCR_WDE
>> s->wcr & ~IMX2_WDT_WCR_WDT
>
>These preserve all the other bits rather than the locked bit, which is
>apparently wrong.
>
>Patch 3 fixes this as part of removing the redundant lock flags, but
>not marked as qemu-stable intentionally. When applying patch 1 & 3
>together on QEMU master, this issue is fixed correctly, but not for
>stable.
>
>Could you fix the WDE/WDT masks in patch 1, leaving patch 3 to remove
>the redundant flags and migration fields? That would make the stable
>fixes work correctly without depending on patch 3.
Hi Bin,
Thanks for spotting this! Dedicated patch added to v3 series.
And thanks for the other comments and reviews! Everything should be
incorporated in v3.
Best regards,
Bernhard
>
>> }
>> s->wcr_locked = true;
>> if (s->wcr_wde_locked) {
>> value &= ~IMX2_WDT_WCR_WDE;
>> - value |= (s->wicr & ~IMX2_WDT_WCR_WDE);
>> + value |= (s->wcr & ~IMX2_WDT_WCR_WDE);
>> } else if (value & IMX2_WDT_WCR_WDE) {
>> s->wcr_wde_locked = true;
>> }
>> if (s->wcr_wdt_locked) {
>> value &= ~IMX2_WDT_WCR_WDT;
>> - value |= (s->wicr & ~IMX2_WDT_WCR_WDT);
>> + value |= (s->wcr & ~IMX2_WDT_WCR_WDT);
>> } else if (value & IMX2_WDT_WCR_WDT) {
>> s->wcr_wdt_locked = true;
>> }
>> --
>
>Regards,
>Bin
