diff mbox series

[06/11] aspeed/smc: fix default read value

Message ID 20180831103816.13479-7-clg@kaod.org
State New
Headers show
Series aspeed: misc fixes and enhancements (SMC) | expand

Commit Message

Cédric Le Goater Aug. 31, 2018, 10:38 a.m. UTC 
0xFFFFFFFF should be returned for non implemented registers.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Philippe Mathieu-Daudé Sept. 7, 2018, 11:06 p.m. UTC | #1 
Hi Cédric,

On 8/31/18 7:38 AM, Cédric Le Goater wrote:
> 0xFFFFFFFF should be returned for non implemented registers.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ssi/aspeed_smc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 1270842dcf0c..6045ca11b969 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -665,12 +665,12 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
>          addr == s->r_ce_ctrl ||
>          addr == R_INTR_CTRL ||
>          (addr >= R_SEG_ADDR0 && addr < R_SEG_ADDR0 + s->ctrl->max_slaves) ||
> -        (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs)) {
> +        (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->ctrl->max_slaves)) {

This change seems unrelated to this commit.

>          return s->regs[addr];
>      } else {
>          qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
>                        __func__, addr);
> -        return 0;
> +        return -1;
>      }
>  }
>  
>
Cédric Le Goater Sept. 10, 2018, 6:20 a.m. UTC | #2 
On 09/08/2018 01:06 AM, Philippe Mathieu-Daudé wrote:
> Hi Cédric,
> 
> On 8/31/18 7:38 AM, Cédric Le Goater wrote:
>> 0xFFFFFFFF should be returned for non implemented registers.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  hw/ssi/aspeed_smc.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>> index 1270842dcf0c..6045ca11b969 100644
>> --- a/hw/ssi/aspeed_smc.c
>> +++ b/hw/ssi/aspeed_smc.c
>> @@ -665,12 +665,12 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
>>          addr == s->r_ce_ctrl ||
>>          addr == R_INTR_CTRL ||
>>          (addr >= R_SEG_ADDR0 && addr < R_SEG_ADDR0 + s->ctrl->max_slaves) ||
>> -        (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs)) {
>> +        (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->ctrl->max_slaves)) {
>
> This change seems unrelated to this commit.

The controller should have one control register for each flash device
if there is no CS available.

The problem is that s->num_cs is a QEMU software concept representing 
the number of flash devices handled by a board where as 'ctrl->max_slaves' 
is a HW limit representing the maximum number of flash devices a 
controller can handle.

Thanks,

C.
 
>>          return s->regs[addr];
>>      } else {
>>          qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
>>                        __func__, addr);
>> -        return 0;
>> +        return -1;
>>      }
>>  }
>>  
>>
diff mbox series

Patch

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 1270842dcf0c..6045ca11b969 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -665,12 +665,12 @@  static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
         addr == s->r_ce_ctrl ||
         addr == R_INTR_CTRL ||
         (addr >= R_SEG_ADDR0 && addr < R_SEG_ADDR0 + s->ctrl->max_slaves) ||
-        (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs)) {
+        (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->ctrl->max_slaves)) {
         return s->regs[addr];
     } else {
         qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
                       __func__, addr);
-        return 0;
+        return -1;
     }
 }