[PATCH v1] hw/ssi/aspeed_smc: Fix incorrect FMC_WDT2 register read on AST1030

Jamin Lin via posted 1 patch 6 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250804014633.512737-1-jamin._5Flin@aspeedtech.com
Maintainers: Alistair Francis <alistair@alistair23.me>, "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>
hw/ssi/aspeed_smc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH v1] hw/ssi/aspeed_smc: Fix incorrect FMC_WDT2 register read on AST1030
Posted by Jamin Lin via 6 months, 1 week ago
On AST1030, reading the FMC_WDT2 register always returns 0xFFFFFFFF.
This issue is due to the aspeed_smc_read function, which checks for the
ASPEED_SMC_FEATURE_WDT_CONTROL feature. Since AST1030 was missing this
feature flag, the read operation fails and returns -1.

To resolve this, add the WDT_CONTROL feature to AST1030's feature set
so that FMC_WDT2 can be correctly accessed by firmware.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/ssi/aspeed_smc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 614528b8ef..e33496f502 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -1857,7 +1857,8 @@ static void aspeed_1030_fmc_class_init(ObjectClass *klass, const void *data)
     asc->resets            = aspeed_1030_fmc_resets;
     asc->flash_window_base = 0x80000000;
     asc->flash_window_size = 0x10000000;
-    asc->features          = ASPEED_SMC_FEATURE_DMA;
+    asc->features          = ASPEED_SMC_FEATURE_DMA |
+                             ASPEED_SMC_FEATURE_WDT_CONTROL;
     asc->dma_flash_mask    = 0x0FFFFFFC;
     asc->dma_dram_mask     = 0x000BFFFC;
     asc->dma_start_length  = 1;
-- 
2.43.0
Re: [PATCH v1] hw/ssi/aspeed_smc: Fix incorrect FMC_WDT2 register read on AST1030
Posted by Cédric Le Goater 6 months, 1 week ago
Hello Jamin,

On 8/4/25 03:46, Jamin Lin wrote:
> On AST1030, reading the FMC_WDT2 register always returns 0xFFFFFFFF.
> This issue is due to the aspeed_smc_read function, which checks for the
> ASPEED_SMC_FEATURE_WDT_CONTROL feature. Since AST1030 was missing this
> feature flag, the read operation fails and returns -1.
> 
> To resolve this, add the WDT_CONTROL feature to AST1030's feature set
> so that FMC_WDT2 can be correctly accessed by firmware.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/ssi/aspeed_smc.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 614528b8ef..e33496f502 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -1857,7 +1857,8 @@ static void aspeed_1030_fmc_class_init(ObjectClass *klass, const void *data)
>       asc->resets            = aspeed_1030_fmc_resets;
>       asc->flash_window_base = 0x80000000;
>       asc->flash_window_size = 0x10000000;
> -    asc->features          = ASPEED_SMC_FEATURE_DMA;
> +    asc->features          = ASPEED_SMC_FEATURE_DMA |
> +                             ASPEED_SMC_FEATURE_WDT_CONTROL;
>       asc->dma_flash_mask    = 0x0FFFFFFC;
>       asc->dma_dram_mask     = 0x000BFFFC;
>       asc->dma_start_length  = 1;

Could you add a Fixes tag please ?

I plan to send a PR before -rc2 (Tuesday) with the vbootrom update.

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.



RE: [PATCH v1] hw/ssi/aspeed_smc: Fix incorrect FMC_WDT2 register read on AST1030
Posted by Jamin Lin 6 months, 1 week ago
Hi Cédric,

> From: Cédric Le Goater <clg@kaod.org>
> Sent: Monday, August 4, 2025 2:33 PM
> To: Jamin Lin <jamin_lin@aspeedtech.com>; Alistair Francis
> <alistair@alistair23.me>; Peter Maydell <peter.maydell@linaro.org>; Steven
> Lee <steven_lee@aspeedtech.com>; Troy Lee <leetroy@gmail.com>; Andrew
> Jeffery <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open
> list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_lee@aspeedtech.com>
> Subject: Re: [PATCH v1] hw/ssi/aspeed_smc: Fix incorrect FMC_WDT2 register
> read on AST1030
> 
> Hello Jamin,
> 
> On 8/4/25 03:46, Jamin Lin wrote:
> > On AST1030, reading the FMC_WDT2 register always returns 0xFFFFFFFF.
> > This issue is due to the aspeed_smc_read function, which checks for
> > the ASPEED_SMC_FEATURE_WDT_CONTROL feature. Since AST1030 was
> missing
> > this feature flag, the read operation fails and returns -1.
> >
> > To resolve this, add the WDT_CONTROL feature to AST1030's feature set
> > so that FMC_WDT2 can be correctly accessed by firmware.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >   hw/ssi/aspeed_smc.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index
> > 614528b8ef..e33496f502 100644
> > --- a/hw/ssi/aspeed_smc.c
> > +++ b/hw/ssi/aspeed_smc.c
> > @@ -1857,7 +1857,8 @@ static void
> aspeed_1030_fmc_class_init(ObjectClass *klass, const void *data)
> >       asc->resets            = aspeed_1030_fmc_resets;
> >       asc->flash_window_base = 0x80000000;
> >       asc->flash_window_size = 0x10000000;
> > -    asc->features          = ASPEED_SMC_FEATURE_DMA;
> > +    asc->features          = ASPEED_SMC_FEATURE_DMA |
> > +
> ASPEED_SMC_FEATURE_WDT_CONTROL;
> >       asc->dma_flash_mask    = 0x0FFFFFFC;
> >       asc->dma_dram_mask     = 0x000BFFFC;
> >       asc->dma_start_length  = 1;
> 
> Could you add a Fixes tag please ?
> 
Fixes: 2850df6 ("aspeed/smc: Add AST1030 support ")

> I plan to send a PR before -rc2 (Tuesday) with the vbootrom update.
> 
Okay. Thanks for review and help.
Jamin
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> 
> Thanks,
> 
> C.
> 

Re: [PATCH v1] hw/ssi/aspeed_smc: Fix incorrect FMC_WDT2 register read on AST1030
Posted by Cédric Le Goater 6 months, 1 week ago
On 8/4/25 08:41, Jamin Lin wrote:
> Hi Cédric,
> 
>> From: Cédric Le Goater <clg@kaod.org>
>> Sent: Monday, August 4, 2025 2:33 PM
>> To: Jamin Lin <jamin_lin@aspeedtech.com>; Alistair Francis
>> <alistair@alistair23.me>; Peter Maydell <peter.maydell@linaro.org>; Steven
>> Lee <steven_lee@aspeedtech.com>; Troy Lee <leetroy@gmail.com>; Andrew
>> Jeffery <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open
>> list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here
>> <qemu-devel@nongnu.org>
>> Cc: Troy Lee <troy_lee@aspeedtech.com>
>> Subject: Re: [PATCH v1] hw/ssi/aspeed_smc: Fix incorrect FMC_WDT2 register
>> read on AST1030
>>
>> Hello Jamin,
>>
>> On 8/4/25 03:46, Jamin Lin wrote:
>>> On AST1030, reading the FMC_WDT2 register always returns 0xFFFFFFFF.
>>> This issue is due to the aspeed_smc_read function, which checks for
>>> the ASPEED_SMC_FEATURE_WDT_CONTROL feature. Since AST1030 was
>> missing
>>> this feature flag, the read operation fails and returns -1.
>>>
>>> To resolve this, add the WDT_CONTROL feature to AST1030's feature set
>>> so that FMC_WDT2 can be correctly accessed by firmware.
>>>
>>> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>>> ---
>>>    hw/ssi/aspeed_smc.c | 3 ++-
>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index
>>> 614528b8ef..e33496f502 100644
>>> --- a/hw/ssi/aspeed_smc.c
>>> +++ b/hw/ssi/aspeed_smc.c
>>> @@ -1857,7 +1857,8 @@ static void
>> aspeed_1030_fmc_class_init(ObjectClass *klass, const void *data)
>>>        asc->resets            = aspeed_1030_fmc_resets;
>>>        asc->flash_window_base = 0x80000000;
>>>        asc->flash_window_size = 0x10000000;
>>> -    asc->features          = ASPEED_SMC_FEATURE_DMA;
>>> +    asc->features          = ASPEED_SMC_FEATURE_DMA |
>>> +
>> ASPEED_SMC_FEATURE_WDT_CONTROL;
>>>        asc->dma_flash_mask    = 0x0FFFFFFC;
>>>        asc->dma_dram_mask     = 0x000BFFFC;
>>>        asc->dma_start_length  = 1;
>>
>> Could you add a Fixes tag please ?
>>
> Fixes: 2850df6 ("aspeed/smc: Add AST1030 support ")

The commit SHA id is a bit short. 12 is a minimum now.

Thanks,

C.



RE: [PATCH v1] hw/ssi/aspeed_smc: Fix incorrect FMC_WDT2 register read on AST1030
Posted by Jamin Lin 6 months, 1 week ago
Hi Cédric,

> From: Cédric Le Goater <clg@kaod.org>
> Sent: Monday, August 4, 2025 2:52 PM
> To: Jamin Lin <jamin_lin@aspeedtech.com>; Alistair Francis
> <alistair@alistair23.me>; Peter Maydell <peter.maydell@linaro.org>; Steven
> Lee <steven_lee@aspeedtech.com>; Troy Lee <leetroy@gmail.com>; Andrew
> Jeffery <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open
> list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_lee@aspeedtech.com>
> Subject: Re: [PATCH v1] hw/ssi/aspeed_smc: Fix incorrect FMC_WDT2 register
> read on AST1030
> 
> On 8/4/25 08:41, Jamin Lin wrote:
> > Hi Cédric,
> >
> >> From: Cédric Le Goater <clg@kaod.org>
> >> Sent: Monday, August 4, 2025 2:33 PM
> >> To: Jamin Lin <jamin_lin@aspeedtech.com>; Alistair Francis
> >> <alistair@alistair23.me>; Peter Maydell <peter.maydell@linaro.org>;
> >> Steven Lee <steven_lee@aspeedtech.com>; Troy Lee
> <leetroy@gmail.com>;
> >> Andrew Jeffery <andrew@codeconstruct.com.au>; Joel Stanley
> >> <joel@jms.id.au>; open list:ASPEED BMCs <qemu-arm@nongnu.org>; open
> >> list:All patches CC here <qemu-devel@nongnu.org>
> >> Cc: Troy Lee <troy_lee@aspeedtech.com>
> >> Subject: Re: [PATCH v1] hw/ssi/aspeed_smc: Fix incorrect FMC_WDT2
> >> register read on AST1030
> >>
> >> Hello Jamin,
> >>
> >> On 8/4/25 03:46, Jamin Lin wrote:
> >>> On AST1030, reading the FMC_WDT2 register always returns 0xFFFFFFFF.
> >>> This issue is due to the aspeed_smc_read function, which checks for
> >>> the ASPEED_SMC_FEATURE_WDT_CONTROL feature. Since AST1030 was
> >> missing
> >>> this feature flag, the read operation fails and returns -1.
> >>>
> >>> To resolve this, add the WDT_CONTROL feature to AST1030's feature
> >>> set so that FMC_WDT2 can be correctly accessed by firmware.
> >>>
> >>> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> >>> ---
> >>>    hw/ssi/aspeed_smc.c | 3 ++-
> >>>    1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index
> >>> 614528b8ef..e33496f502 100644
> >>> --- a/hw/ssi/aspeed_smc.c
> >>> +++ b/hw/ssi/aspeed_smc.c
> >>> @@ -1857,7 +1857,8 @@ static void
> >> aspeed_1030_fmc_class_init(ObjectClass *klass, const void *data)
> >>>        asc->resets            = aspeed_1030_fmc_resets;
> >>>        asc->flash_window_base = 0x80000000;
> >>>        asc->flash_window_size = 0x10000000;
> >>> -    asc->features          = ASPEED_SMC_FEATURE_DMA;
> >>> +    asc->features          = ASPEED_SMC_FEATURE_DMA |
> >>> +
> >> ASPEED_SMC_FEATURE_WDT_CONTROL;
> >>>        asc->dma_flash_mask    = 0x0FFFFFFC;
> >>>        asc->dma_dram_mask     = 0x000BFFFC;
> >>>        asc->dma_start_length  = 1;
> >>
> >> Could you add a Fixes tag please ?
> >>
> > Fixes: 2850df6 ("aspeed/smc: Add AST1030 support ")
> 
> The commit SHA id is a bit short. 12 is a minimum now.
>
Update SHA id

Fixes: 2850df6a81bcdc2e063dfdd56751ee2d11c58030 ("aspeed/smc: Add AST1030 support ")

Thanks-Jamin
> Thanks,
> 
> C.
>