All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] aspeed/scu: Implement chip id register
@ 2020-01-21  1:33 Joel Stanley
  2020-01-21  1:33 ` [PATCH 1/2] aspeed/scu: Create separate write callbacks Joel Stanley
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Joel Stanley @ 2020-01-21  1:33 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell; +Cc: Andrew Jeffery, qemu-arm, qemu-devel

This implements the chip id register in the SCU for the ast2500 and
ast2600. The first patch is a cleanup to separate out ast2400 and
ast2500 functionality.

Joel Stanley (2):
  aspeed/scu: Create separate write callbacks
  aspeed/scu: Implement chip ID register

 hw/misc/aspeed_scu.c | 93 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 70 insertions(+), 23 deletions(-)

-- 
2.24.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] aspeed/scu: Create separate write callbacks
  2020-01-21  1:33 [PATCH 0/2] aspeed/scu: Implement chip id register Joel Stanley
@ 2020-01-21  1:33 ` Joel Stanley
  2020-01-21  4:20   ` Andrew Jeffery
                     ` (2 more replies)
  2020-01-21  1:33 ` [PATCH 2/2] aspeed/scu: Implement chip ID register Joel Stanley
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 12+ messages in thread
From: Joel Stanley @ 2020-01-21  1:33 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell; +Cc: Andrew Jeffery, qemu-arm, qemu-devel

This splits the common write callback into separate ast2400 and ast2500
implementations. This makes it clearer when implementing differing
behaviour.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 hw/misc/aspeed_scu.c | 80 +++++++++++++++++++++++++++++++-------------
 1 file changed, 57 insertions(+), 23 deletions(-)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index f62fa25e3474..7108cad8c6a7 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -232,8 +232,47 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
     return s->regs[reg];
 }
 
-static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
-                             unsigned size)
+static void aspeed_ast2400_scu_write(void *opaque, hwaddr offset,
+                                     uint64_t data, unsigned size)
+{
+    AspeedSCUState *s = ASPEED_SCU(opaque);
+    int reg = TO_REG(offset);
+
+    if (reg >= ASPEED_SCU_NR_REGS) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n",
+                      __func__, offset);
+        return;
+    }
+
+    if (reg > PROT_KEY && reg < CPU2_BASE_SEG1 &&
+            !s->regs[PROT_KEY]) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__);
+    }
+
+    trace_aspeed_scu_write(offset, size, data);
+
+    switch (reg) {
+    case PROT_KEY:
+        s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
+        return;
+    case SILICON_REV:
+    case FREQ_CNTR_EVAL:
+    case VGA_SCRATCH1 ... VGA_SCRATCH8:
+    case RNG_DATA:
+    case FREE_CNTR4:
+    case FREE_CNTR4_EXT:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
+                      __func__, offset);
+        return;
+    }
+
+    s->regs[reg] = data;
+}
+
+static void aspeed_ast2500_scu_write(void *opaque, hwaddr offset,
+                                     uint64_t data, unsigned size)
 {
     AspeedSCUState *s = ASPEED_SCU(opaque);
     int reg = TO_REG(offset);
@@ -257,25 +296,11 @@ static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
     case PROT_KEY:
         s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
         return;
-    case CLK_SEL:
-        s->regs[reg] = data;
-        break;
     case HW_STRAP1:
-        if (ASPEED_IS_AST2500(s->regs[SILICON_REV])) {
-            s->regs[HW_STRAP1] |= data;
-            return;
-        }
-        /* Jump to assignment below */
-        break;
+        s->regs[HW_STRAP1] |= data;
+        return;
     case SILICON_REV:
-        if (ASPEED_IS_AST2500(s->regs[SILICON_REV])) {
-            s->regs[HW_STRAP1] &= ~data;
-        } else {
-            qemu_log_mask(LOG_GUEST_ERROR,
-                          "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
-                          __func__, offset);
-        }
-        /* Avoid assignment below, we've handled everything */
+        s->regs[HW_STRAP1] &= ~data;
         return;
     case FREQ_CNTR_EVAL:
     case VGA_SCRATCH1 ... VGA_SCRATCH8:
@@ -291,9 +316,18 @@ static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
     s->regs[reg] = data;
 }
 
-static const MemoryRegionOps aspeed_scu_ops = {
+static const MemoryRegionOps aspeed_ast2400_scu_ops = {
+    .read = aspeed_scu_read,
+    .write = aspeed_ast2400_scu_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+    .valid.unaligned = false,
+};
+
+static const MemoryRegionOps aspeed_ast2500_scu_ops = {
     .read = aspeed_scu_read,
-    .write = aspeed_scu_write,
+    .write = aspeed_ast2500_scu_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .valid.min_access_size = 4,
     .valid.max_access_size = 4,
@@ -469,7 +503,7 @@ static void aspeed_2400_scu_class_init(ObjectClass *klass, void *data)
     asc->calc_hpll = aspeed_2400_scu_calc_hpll;
     asc->apb_divider = 2;
     asc->nr_regs = ASPEED_SCU_NR_REGS;
-    asc->ops = &aspeed_scu_ops;
+    asc->ops = &aspeed_ast2400_scu_ops;
 }
 
 static const TypeInfo aspeed_2400_scu_info = {
@@ -489,7 +523,7 @@ static void aspeed_2500_scu_class_init(ObjectClass *klass, void *data)
     asc->calc_hpll = aspeed_2500_scu_calc_hpll;
     asc->apb_divider = 4;
     asc->nr_regs = ASPEED_SCU_NR_REGS;
-    asc->ops = &aspeed_scu_ops;
+    asc->ops = &aspeed_ast2500_scu_ops;
 }
 
 static const TypeInfo aspeed_2500_scu_info = {
-- 
2.24.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] aspeed/scu: Implement chip ID register
  2020-01-21  1:33 [PATCH 0/2] aspeed/scu: Implement chip id register Joel Stanley
  2020-01-21  1:33 ` [PATCH 1/2] aspeed/scu: Create separate write callbacks Joel Stanley
@ 2020-01-21  1:33 ` Joel Stanley
  2020-01-21  4:23   ` Andrew Jeffery
                     ` (2 more replies)
  2020-01-21  7:23 ` [PATCH 0/2] aspeed/scu: Implement chip id register Cédric Le Goater
  2020-02-17  9:29 ` Cédric Le Goater
  3 siblings, 3 replies; 12+ messages in thread
From: Joel Stanley @ 2020-01-21  1:33 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell; +Cc: Andrew Jeffery, qemu-arm, qemu-devel

This returns a fixed but non-zero value for the chip id.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 hw/misc/aspeed_scu.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 7108cad8c6a7..19d1780a40da 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -77,6 +77,8 @@
 #define CPU2_BASE_SEG4       TO_REG(0x110)
 #define CPU2_BASE_SEG5       TO_REG(0x114)
 #define CPU2_CACHE_CTRL      TO_REG(0x118)
+#define CHIP_ID0             TO_REG(0x150)
+#define CHIP_ID1             TO_REG(0x154)
 #define UART_HPLL_CLK        TO_REG(0x160)
 #define PCIE_CTRL            TO_REG(0x180)
 #define BMC_MMIO_CTRL        TO_REG(0x184)
@@ -115,6 +117,8 @@
 #define AST2600_HW_STRAP2_PROT    TO_REG(0x518)
 #define AST2600_RNG_CTRL          TO_REG(0x524)
 #define AST2600_RNG_DATA          TO_REG(0x540)
+#define AST2600_CHIP_ID0          TO_REG(0x5B0)
+#define AST2600_CHIP_ID1          TO_REG(0x5B4)
 
 #define AST2600_CLK TO_REG(0x40)
 
@@ -182,6 +186,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
      [CPU2_BASE_SEG1]  = 0x80000000U,
      [CPU2_BASE_SEG4]  = 0x1E600000U,
      [CPU2_BASE_SEG5]  = 0xC0000000U,
+     [CHIP_ID0]        = 0x1234ABCDU,
+     [CHIP_ID1]        = 0x88884444U,
      [UART_HPLL_CLK]   = 0x00001903U,
      [PCIE_CTRL]       = 0x0000007BU,
      [BMC_DEV_ID]      = 0x00002402U
@@ -307,6 +313,8 @@ static void aspeed_ast2500_scu_write(void *opaque, hwaddr offset,
     case RNG_DATA:
     case FREE_CNTR4:
     case FREE_CNTR4_EXT:
+    case CHIP_ID0:
+    case CHIP_ID1:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
                       __func__, offset);
@@ -620,6 +628,8 @@ static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset,
     case AST2600_RNG_DATA:
     case AST2600_SILICON_REV:
     case AST2600_SILICON_REV2:
+    case AST2600_CHIP_ID0:
+    case AST2600_CHIP_ID1:
         /* Add read only registers here */
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
@@ -648,6 +658,9 @@ static const uint32_t ast2600_a0_resets[ASPEED_AST2600_SCU_NR_REGS] = {
     [AST2600_CLK_STOP_CTRL2]    = 0xFFF0FFF0,
     [AST2600_SDRAM_HANDSHAKE]   = 0x00000040,  /* SoC completed DRAM init */
     [AST2600_HPLL_PARAM]        = 0x1000405F,
+    [AST2600_CHIP_ID0]          = 0x1234ABCD,
+    [AST2600_CHIP_ID1]          = 0x88884444,
+
 };
 
 static void aspeed_ast2600_scu_reset(DeviceState *dev)
-- 
2.24.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] aspeed/scu: Create separate write callbacks
  2020-01-21  1:33 ` [PATCH 1/2] aspeed/scu: Create separate write callbacks Joel Stanley
@ 2020-01-21  4:20   ` Andrew Jeffery
  2020-01-21  7:23   ` Cédric Le Goater
  2020-01-21  8:50   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 12+ messages in thread
From: Andrew Jeffery @ 2020-01-21  4:20 UTC (permalink / raw)
  To: Joel Stanley, Cédric Le Goater, Peter Maydell; +Cc: qemu-arm, qemu-devel



On Tue, 21 Jan 2020, at 12:03, Joel Stanley wrote:
> This splits the common write callback into separate ast2400 and ast2500
> implementations. This makes it clearer when implementing differing
> behaviour.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] aspeed/scu: Implement chip ID register
  2020-01-21  1:33 ` [PATCH 2/2] aspeed/scu: Implement chip ID register Joel Stanley
@ 2020-01-21  4:23   ` Andrew Jeffery
  2020-01-21  7:23   ` Cédric Le Goater
  2020-01-21  8:52   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 12+ messages in thread
From: Andrew Jeffery @ 2020-01-21  4:23 UTC (permalink / raw)
  To: Joel Stanley, Cédric Le Goater, Peter Maydell; +Cc: qemu-arm, qemu-devel



On Tue, 21 Jan 2020, at 12:03, Joel Stanley wrote:
> This returns a fixed but non-zero value for the chip id.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>  hw/misc/aspeed_scu.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index 7108cad8c6a7..19d1780a40da 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -77,6 +77,8 @@
>  #define CPU2_BASE_SEG4       TO_REG(0x110)
>  #define CPU2_BASE_SEG5       TO_REG(0x114)
>  #define CPU2_CACHE_CTRL      TO_REG(0x118)
> +#define CHIP_ID0             TO_REG(0x150)
> +#define CHIP_ID1             TO_REG(0x154)
>  #define UART_HPLL_CLK        TO_REG(0x160)
>  #define PCIE_CTRL            TO_REG(0x180)
>  #define BMC_MMIO_CTRL        TO_REG(0x184)
> @@ -115,6 +117,8 @@
>  #define AST2600_HW_STRAP2_PROT    TO_REG(0x518)
>  #define AST2600_RNG_CTRL          TO_REG(0x524)
>  #define AST2600_RNG_DATA          TO_REG(0x540)
> +#define AST2600_CHIP_ID0          TO_REG(0x5B0)
> +#define AST2600_CHIP_ID1          TO_REG(0x5B4)
>  
>  #define AST2600_CLK TO_REG(0x40)
>  
> @@ -182,6 +186,8 @@ static const uint32_t 
> ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>       [CPU2_BASE_SEG1]  = 0x80000000U,
>       [CPU2_BASE_SEG4]  = 0x1E600000U,
>       [CPU2_BASE_SEG5]  = 0xC0000000U,
> +     [CHIP_ID0]        = 0x1234ABCDU,
> +     [CHIP_ID1]        = 0x88884444U,
>       [UART_HPLL_CLK]   = 0x00001903U,
>       [PCIE_CTRL]       = 0x0000007BU,
>       [BMC_DEV_ID]      = 0x00002402U
> @@ -307,6 +313,8 @@ static void aspeed_ast2500_scu_write(void *opaque, 
> hwaddr offset,
>      case RNG_DATA:
>      case FREE_CNTR4:
>      case FREE_CNTR4_EXT:
> +    case CHIP_ID0:
> +    case CHIP_ID1:
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s: Write to read-only offset 0x%" HWADDR_PRIx 
> "\n",
>                        __func__, offset);
> @@ -620,6 +628,8 @@ static void aspeed_ast2600_scu_write(void *opaque, 
> hwaddr offset,
>      case AST2600_RNG_DATA:
>      case AST2600_SILICON_REV:
>      case AST2600_SILICON_REV2:
> +    case AST2600_CHIP_ID0:
> +    case AST2600_CHIP_ID1:
>          /* Add read only registers here */
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s: Write to read-only offset 0x%" HWADDR_PRIx 
> "\n",
> @@ -648,6 +658,9 @@ static const uint32_t 
> ast2600_a0_resets[ASPEED_AST2600_SCU_NR_REGS] = {
>      [AST2600_CLK_STOP_CTRL2]    = 0xFFF0FFF0,
>      [AST2600_SDRAM_HANDSHAKE]   = 0x00000040,  /* SoC completed DRAM 
> init */
>      [AST2600_HPLL_PARAM]        = 0x1000405F,
> +    [AST2600_CHIP_ID0]          = 0x1234ABCD,
> +    [AST2600_CHIP_ID1]          = 0x88884444,

Probably should add the explicit trailing 'U' to the constants at some point.

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] aspeed/scu: Create separate write callbacks
  2020-01-21  1:33 ` [PATCH 1/2] aspeed/scu: Create separate write callbacks Joel Stanley
  2020-01-21  4:20   ` Andrew Jeffery
@ 2020-01-21  7:23   ` Cédric Le Goater
  2020-01-21  8:50   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 12+ messages in thread
From: Cédric Le Goater @ 2020-01-21  7:23 UTC (permalink / raw)
  To: Joel Stanley, Peter Maydell; +Cc: Andrew Jeffery, qemu-arm, qemu-devel

On 1/21/20 2:33 AM, Joel Stanley wrote:
> This splits the common write callback into separate ast2400 and ast2500
> implementations. This makes it clearer when implementing differing
> behaviour.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

> ---
>  hw/misc/aspeed_scu.c | 80 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 57 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index f62fa25e3474..7108cad8c6a7 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -232,8 +232,47 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
>      return s->regs[reg];
>  }
>  
> -static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
> -                             unsigned size)
> +static void aspeed_ast2400_scu_write(void *opaque, hwaddr offset,
> +                                     uint64_t data, unsigned size)
> +{
> +    AspeedSCUState *s = ASPEED_SCU(opaque);
> +    int reg = TO_REG(offset);
> +
> +    if (reg >= ASPEED_SCU_NR_REGS) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n",
> +                      __func__, offset);
> +        return;
> +    }
> +
> +    if (reg > PROT_KEY && reg < CPU2_BASE_SEG1 &&
> +            !s->regs[PROT_KEY]) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__);
> +    }
> +
> +    trace_aspeed_scu_write(offset, size, data);
> +
> +    switch (reg) {
> +    case PROT_KEY:
> +        s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
> +        return;
> +    case SILICON_REV:
> +    case FREQ_CNTR_EVAL:
> +    case VGA_SCRATCH1 ... VGA_SCRATCH8:
> +    case RNG_DATA:
> +    case FREE_CNTR4:
> +    case FREE_CNTR4_EXT:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
> +                      __func__, offset);
> +        return;
> +    }
> +
> +    s->regs[reg] = data;
> +}
> +
> +static void aspeed_ast2500_scu_write(void *opaque, hwaddr offset,
> +                                     uint64_t data, unsigned size)
>  {
>      AspeedSCUState *s = ASPEED_SCU(opaque);
>      int reg = TO_REG(offset);
> @@ -257,25 +296,11 @@ static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
>      case PROT_KEY:
>          s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
>          return;
> -    case CLK_SEL:
> -        s->regs[reg] = data;
> -        break;
>      case HW_STRAP1:
> -        if (ASPEED_IS_AST2500(s->regs[SILICON_REV])) {
> -            s->regs[HW_STRAP1] |= data;
> -            return;
> -        }
> -        /* Jump to assignment below */
> -        break;
> +        s->regs[HW_STRAP1] |= data;
> +        return;
>      case SILICON_REV:
> -        if (ASPEED_IS_AST2500(s->regs[SILICON_REV])) {
> -            s->regs[HW_STRAP1] &= ~data;
> -        } else {
> -            qemu_log_mask(LOG_GUEST_ERROR,
> -                          "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
> -                          __func__, offset);
> -        }
> -        /* Avoid assignment below, we've handled everything */
> +        s->regs[HW_STRAP1] &= ~data;
>          return;
>      case FREQ_CNTR_EVAL:
>      case VGA_SCRATCH1 ... VGA_SCRATCH8:
> @@ -291,9 +316,18 @@ static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
>      s->regs[reg] = data;
>  }
>  
> -static const MemoryRegionOps aspeed_scu_ops = {
> +static const MemoryRegionOps aspeed_ast2400_scu_ops = {
> +    .read = aspeed_scu_read,
> +    .write = aspeed_ast2400_scu_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,
> +    .valid.unaligned = false,
> +};
> +
> +static const MemoryRegionOps aspeed_ast2500_scu_ops = {
>      .read = aspeed_scu_read,
> -    .write = aspeed_scu_write,
> +    .write = aspeed_ast2500_scu_write,
>      .endianness = DEVICE_LITTLE_ENDIAN,
>      .valid.min_access_size = 4,
>      .valid.max_access_size = 4,
> @@ -469,7 +503,7 @@ static void aspeed_2400_scu_class_init(ObjectClass *klass, void *data)
>      asc->calc_hpll = aspeed_2400_scu_calc_hpll;
>      asc->apb_divider = 2;
>      asc->nr_regs = ASPEED_SCU_NR_REGS;
> -    asc->ops = &aspeed_scu_ops;
> +    asc->ops = &aspeed_ast2400_scu_ops;
>  }
>  
>  static const TypeInfo aspeed_2400_scu_info = {
> @@ -489,7 +523,7 @@ static void aspeed_2500_scu_class_init(ObjectClass *klass, void *data)
>      asc->calc_hpll = aspeed_2500_scu_calc_hpll;
>      asc->apb_divider = 4;
>      asc->nr_regs = ASPEED_SCU_NR_REGS;
> -    asc->ops = &aspeed_scu_ops;
> +    asc->ops = &aspeed_ast2500_scu_ops;
>  }
>  
>  static const TypeInfo aspeed_2500_scu_info = {
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] aspeed/scu: Implement chip ID register
  2020-01-21  1:33 ` [PATCH 2/2] aspeed/scu: Implement chip ID register Joel Stanley
  2020-01-21  4:23   ` Andrew Jeffery
@ 2020-01-21  7:23   ` Cédric Le Goater
  2020-01-21  8:52   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 12+ messages in thread
From: Cédric Le Goater @ 2020-01-21  7:23 UTC (permalink / raw)
  To: Joel Stanley, Peter Maydell; +Cc: Andrew Jeffery, qemu-arm, qemu-devel

On 1/21/20 2:33 AM, Joel Stanley wrote:
> This returns a fixed but non-zero value for the chip id.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

> ---
>  hw/misc/aspeed_scu.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index 7108cad8c6a7..19d1780a40da 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -77,6 +77,8 @@
>  #define CPU2_BASE_SEG4       TO_REG(0x110)
>  #define CPU2_BASE_SEG5       TO_REG(0x114)
>  #define CPU2_CACHE_CTRL      TO_REG(0x118)
> +#define CHIP_ID0             TO_REG(0x150)
> +#define CHIP_ID1             TO_REG(0x154)
>  #define UART_HPLL_CLK        TO_REG(0x160)
>  #define PCIE_CTRL            TO_REG(0x180)
>  #define BMC_MMIO_CTRL        TO_REG(0x184)
> @@ -115,6 +117,8 @@
>  #define AST2600_HW_STRAP2_PROT    TO_REG(0x518)
>  #define AST2600_RNG_CTRL          TO_REG(0x524)
>  #define AST2600_RNG_DATA          TO_REG(0x540)
> +#define AST2600_CHIP_ID0          TO_REG(0x5B0)
> +#define AST2600_CHIP_ID1          TO_REG(0x5B4)
>  
>  #define AST2600_CLK TO_REG(0x40)
>  
> @@ -182,6 +186,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>       [CPU2_BASE_SEG1]  = 0x80000000U,
>       [CPU2_BASE_SEG4]  = 0x1E600000U,
>       [CPU2_BASE_SEG5]  = 0xC0000000U,
> +     [CHIP_ID0]        = 0x1234ABCDU,
> +     [CHIP_ID1]        = 0x88884444U,
>       [UART_HPLL_CLK]   = 0x00001903U,
>       [PCIE_CTRL]       = 0x0000007BU,
>       [BMC_DEV_ID]      = 0x00002402U
> @@ -307,6 +313,8 @@ static void aspeed_ast2500_scu_write(void *opaque, hwaddr offset,
>      case RNG_DATA:
>      case FREE_CNTR4:
>      case FREE_CNTR4_EXT:
> +    case CHIP_ID0:
> +    case CHIP_ID1:
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
>                        __func__, offset);
> @@ -620,6 +628,8 @@ static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset,
>      case AST2600_RNG_DATA:
>      case AST2600_SILICON_REV:
>      case AST2600_SILICON_REV2:
> +    case AST2600_CHIP_ID0:
> +    case AST2600_CHIP_ID1:
>          /* Add read only registers here */
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
> @@ -648,6 +658,9 @@ static const uint32_t ast2600_a0_resets[ASPEED_AST2600_SCU_NR_REGS] = {
>      [AST2600_CLK_STOP_CTRL2]    = 0xFFF0FFF0,
>      [AST2600_SDRAM_HANDSHAKE]   = 0x00000040,  /* SoC completed DRAM init */
>      [AST2600_HPLL_PARAM]        = 0x1000405F,
> +    [AST2600_CHIP_ID0]          = 0x1234ABCD,
> +    [AST2600_CHIP_ID1]          = 0x88884444,
> +
>  };
>  
>  static void aspeed_ast2600_scu_reset(DeviceState *dev)
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] aspeed/scu: Implement chip id register
  2020-01-21  1:33 [PATCH 0/2] aspeed/scu: Implement chip id register Joel Stanley
  2020-01-21  1:33 ` [PATCH 1/2] aspeed/scu: Create separate write callbacks Joel Stanley
  2020-01-21  1:33 ` [PATCH 2/2] aspeed/scu: Implement chip ID register Joel Stanley
@ 2020-01-21  7:23 ` Cédric Le Goater
  2020-02-17  9:29 ` Cédric Le Goater
  3 siblings, 0 replies; 12+ messages in thread
From: Cédric Le Goater @ 2020-01-21  7:23 UTC (permalink / raw)
  To: Joel Stanley, Peter Maydell; +Cc: Andrew Jeffery, qemu-arm, qemu-devel

On 1/21/20 2:33 AM, Joel Stanley wrote:
> This implements the chip id register in the SCU for the ast2500 and
> ast2600. The first patch is a cleanup to separate out ast2400 and
> ast2500 functionality.

These patches apply cleanly on top of :

	[v3,0/5] aspeed: extensions and fixes 
	http://patchwork.ozlabs.org/cover/1222667/

Thanks,

C.

> 
> Joel Stanley (2):
>   aspeed/scu: Create separate write callbacks
>   aspeed/scu: Implement chip ID register
> 
>  hw/misc/aspeed_scu.c | 93 +++++++++++++++++++++++++++++++++-----------
>  1 file changed, 70 insertions(+), 23 deletions(-)
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] aspeed/scu: Create separate write callbacks
  2020-01-21  1:33 ` [PATCH 1/2] aspeed/scu: Create separate write callbacks Joel Stanley
  2020-01-21  4:20   ` Andrew Jeffery
  2020-01-21  7:23   ` Cédric Le Goater
@ 2020-01-21  8:50   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21  8:50 UTC (permalink / raw)
  To: Joel Stanley, Cédric Le Goater, Peter Maydell
  Cc: Andrew Jeffery, qemu-arm, qemu-devel



On 1/21/20 2:33 AM, Joel Stanley wrote:
> This splits the common write callback into separate ast2400 and ast2500
> implementations. This makes it clearer when implementing differing
> behaviour.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   hw/misc/aspeed_scu.c | 80 +++++++++++++++++++++++++++++++-------------
>   1 file changed, 57 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index f62fa25e3474..7108cad8c6a7 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -232,8 +232,47 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
>       return s->regs[reg];
>   }
>   
> -static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
> -                             unsigned size)
> +static void aspeed_ast2400_scu_write(void *opaque, hwaddr offset,
> +                                     uint64_t data, unsigned size)
> +{
> +    AspeedSCUState *s = ASPEED_SCU(opaque);
> +    int reg = TO_REG(offset);
> +

I'd move the trace call here:

        trace_aspeed_scu_write(offset, size, data);

(we might be running with tracing enabled but not guest_errors).

Regardless:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +    if (reg >= ASPEED_SCU_NR_REGS) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n",
> +                      __func__, offset);
> +        return;
> +    }
> +
> +    if (reg > PROT_KEY && reg < CPU2_BASE_SEG1 &&
> +            !s->regs[PROT_KEY]) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__);
> +    }
> +
> +    trace_aspeed_scu_write(offset, size, data);
> +
> +    switch (reg) {
> +    case PROT_KEY:
> +        s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
> +        return;
> +    case SILICON_REV:
> +    case FREQ_CNTR_EVAL:
> +    case VGA_SCRATCH1 ... VGA_SCRATCH8:
> +    case RNG_DATA:
> +    case FREE_CNTR4:
> +    case FREE_CNTR4_EXT:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
> +                      __func__, offset);
> +        return;
> +    }
> +
> +    s->regs[reg] = data;
> +}
> +
> +static void aspeed_ast2500_scu_write(void *opaque, hwaddr offset,
> +                                     uint64_t data, unsigned size)
>   {
>       AspeedSCUState *s = ASPEED_SCU(opaque);
>       int reg = TO_REG(offset);
> @@ -257,25 +296,11 @@ static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
>       case PROT_KEY:
>           s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
>           return;
> -    case CLK_SEL:
> -        s->regs[reg] = data;
> -        break;
>       case HW_STRAP1:
> -        if (ASPEED_IS_AST2500(s->regs[SILICON_REV])) {
> -            s->regs[HW_STRAP1] |= data;
> -            return;
> -        }
> -        /* Jump to assignment below */
> -        break;
> +        s->regs[HW_STRAP1] |= data;
> +        return;
>       case SILICON_REV:
> -        if (ASPEED_IS_AST2500(s->regs[SILICON_REV])) {
> -            s->regs[HW_STRAP1] &= ~data;
> -        } else {
> -            qemu_log_mask(LOG_GUEST_ERROR,
> -                          "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
> -                          __func__, offset);
> -        }
> -        /* Avoid assignment below, we've handled everything */
> +        s->regs[HW_STRAP1] &= ~data;
>           return;
>       case FREQ_CNTR_EVAL:
>       case VGA_SCRATCH1 ... VGA_SCRATCH8:
> @@ -291,9 +316,18 @@ static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
>       s->regs[reg] = data;
>   }
>   
> -static const MemoryRegionOps aspeed_scu_ops = {
> +static const MemoryRegionOps aspeed_ast2400_scu_ops = {
> +    .read = aspeed_scu_read,
> +    .write = aspeed_ast2400_scu_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,
> +    .valid.unaligned = false,
> +};
> +
> +static const MemoryRegionOps aspeed_ast2500_scu_ops = {
>       .read = aspeed_scu_read,
> -    .write = aspeed_scu_write,
> +    .write = aspeed_ast2500_scu_write,
>       .endianness = DEVICE_LITTLE_ENDIAN,
>       .valid.min_access_size = 4,
>       .valid.max_access_size = 4,
> @@ -469,7 +503,7 @@ static void aspeed_2400_scu_class_init(ObjectClass *klass, void *data)
>       asc->calc_hpll = aspeed_2400_scu_calc_hpll;
>       asc->apb_divider = 2;
>       asc->nr_regs = ASPEED_SCU_NR_REGS;
> -    asc->ops = &aspeed_scu_ops;
> +    asc->ops = &aspeed_ast2400_scu_ops;
>   }
>   
>   static const TypeInfo aspeed_2400_scu_info = {
> @@ -489,7 +523,7 @@ static void aspeed_2500_scu_class_init(ObjectClass *klass, void *data)
>       asc->calc_hpll = aspeed_2500_scu_calc_hpll;
>       asc->apb_divider = 4;
>       asc->nr_regs = ASPEED_SCU_NR_REGS;
> -    asc->ops = &aspeed_scu_ops;
> +    asc->ops = &aspeed_ast2500_scu_ops;
>   }
>   
>   static const TypeInfo aspeed_2500_scu_info = {
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] aspeed/scu: Implement chip ID register
  2020-01-21  1:33 ` [PATCH 2/2] aspeed/scu: Implement chip ID register Joel Stanley
  2020-01-21  4:23   ` Andrew Jeffery
  2020-01-21  7:23   ` Cédric Le Goater
@ 2020-01-21  8:52   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21  8:52 UTC (permalink / raw)
  To: Joel Stanley, Cédric Le Goater, Peter Maydell
  Cc: Andrew Jeffery, qemu-arm, qemu-devel

On 1/21/20 2:33 AM, Joel Stanley wrote:
> This returns a fixed but non-zero value for the chip id.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   hw/misc/aspeed_scu.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index 7108cad8c6a7..19d1780a40da 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -77,6 +77,8 @@
>   #define CPU2_BASE_SEG4       TO_REG(0x110)
>   #define CPU2_BASE_SEG5       TO_REG(0x114)
>   #define CPU2_CACHE_CTRL      TO_REG(0x118)
> +#define CHIP_ID0             TO_REG(0x150)
> +#define CHIP_ID1             TO_REG(0x154)
>   #define UART_HPLL_CLK        TO_REG(0x160)
>   #define PCIE_CTRL            TO_REG(0x180)
>   #define BMC_MMIO_CTRL        TO_REG(0x184)
> @@ -115,6 +117,8 @@
>   #define AST2600_HW_STRAP2_PROT    TO_REG(0x518)
>   #define AST2600_RNG_CTRL          TO_REG(0x524)
>   #define AST2600_RNG_DATA          TO_REG(0x540)
> +#define AST2600_CHIP_ID0          TO_REG(0x5B0)
> +#define AST2600_CHIP_ID1          TO_REG(0x5B4)
>   
>   #define AST2600_CLK TO_REG(0x40)
>   
> @@ -182,6 +186,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>        [CPU2_BASE_SEG1]  = 0x80000000U,
>        [CPU2_BASE_SEG4]  = 0x1E600000U,
>        [CPU2_BASE_SEG5]  = 0xC0000000U,
> +     [CHIP_ID0]        = 0x1234ABCDU,
> +     [CHIP_ID1]        = 0x88884444U,
>        [UART_HPLL_CLK]   = 0x00001903U,
>        [PCIE_CTRL]       = 0x0000007BU,
>        [BMC_DEV_ID]      = 0x00002402U
> @@ -307,6 +313,8 @@ static void aspeed_ast2500_scu_write(void *opaque, hwaddr offset,
>       case RNG_DATA:
>       case FREE_CNTR4:
>       case FREE_CNTR4_EXT:
> +    case CHIP_ID0:
> +    case CHIP_ID1:
>           qemu_log_mask(LOG_GUEST_ERROR,
>                         "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
>                         __func__, offset);
> @@ -620,6 +628,8 @@ static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset,
>       case AST2600_RNG_DATA:
>       case AST2600_SILICON_REV:
>       case AST2600_SILICON_REV2:
> +    case AST2600_CHIP_ID0:
> +    case AST2600_CHIP_ID1:
>           /* Add read only registers here */
>           qemu_log_mask(LOG_GUEST_ERROR,
>                         "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
> @@ -648,6 +658,9 @@ static const uint32_t ast2600_a0_resets[ASPEED_AST2600_SCU_NR_REGS] = {
>       [AST2600_CLK_STOP_CTRL2]    = 0xFFF0FFF0,
>       [AST2600_SDRAM_HANDSHAKE]   = 0x00000040,  /* SoC completed DRAM init */
>       [AST2600_HPLL_PARAM]        = 0x1000405F,
> +    [AST2600_CHIP_ID0]          = 0x1234ABCD,
> +    [AST2600_CHIP_ID1]          = 0x88884444,

Since this doesn't match the datasheet, can you add a comment /* 
Arbitrary non-zero value */?

With it:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +
>   };
>   
>   static void aspeed_ast2600_scu_reset(DeviceState *dev)
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] aspeed/scu: Implement chip id register
  2020-01-21  1:33 [PATCH 0/2] aspeed/scu: Implement chip id register Joel Stanley
                   ` (2 preceding siblings ...)
  2020-01-21  7:23 ` [PATCH 0/2] aspeed/scu: Implement chip id register Cédric Le Goater
@ 2020-02-17  9:29 ` Cédric Le Goater
  2020-02-17 10:24   ` Peter Maydell
  3 siblings, 1 reply; 12+ messages in thread
From: Cédric Le Goater @ 2020-02-17  9:29 UTC (permalink / raw)
  To: Joel Stanley, Peter Maydell; +Cc: Andrew Jeffery, qemu-arm, qemu-devel

On 1/21/20 2:33 AM, Joel Stanley wrote:
> This implements the chip id register in the SCU for the ast2500 and
> ast2600. The first patch is a cleanup to separate out ast2400 and
> ast2500 functionality.
> 
> Joel Stanley (2):
>   aspeed/scu: Create separate write callbacks
>   aspeed/scu: Implement chip ID register
> 
>  hw/misc/aspeed_scu.c | 93 +++++++++++++++++++++++++++++++++-----------
>  1 file changed, 70 insertions(+), 23 deletions(-)
> 

Hello Peter,

I see you have merged patches more recent than this series. Do you
plan to take them later on ? They apply correctly on mainline.

Thanks,

C.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] aspeed/scu: Implement chip id register
  2020-02-17  9:29 ` Cédric Le Goater
@ 2020-02-17 10:24   ` Peter Maydell
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2020-02-17 10:24 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Andrew Jeffery, qemu-arm, Joel Stanley, QEMU Developers

On Mon, 17 Feb 2020 at 09:29, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 1/21/20 2:33 AM, Joel Stanley wrote:
> > This implements the chip id register in the SCU for the ast2500 and
> > ast2600. The first patch is a cleanup to separate out ast2400 and
> > ast2500 functionality.
> >
> > Joel Stanley (2):
> >   aspeed/scu: Create separate write callbacks
> >   aspeed/scu: Implement chip ID register
> >
> >  hw/misc/aspeed_scu.c | 93 +++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 70 insertions(+), 23 deletions(-)
> >
>
> Hello Peter,
>
> I see you have merged patches more recent than this series. Do you
> plan to take them later on ? They apply correctly on mainline.

Sorry, these didn't get onto my to-review queue for some reason;
I've just put them into target-arm.next.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-02-17 10:25 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-21  1:33 [PATCH 0/2] aspeed/scu: Implement chip id register Joel Stanley
2020-01-21  1:33 ` [PATCH 1/2] aspeed/scu: Create separate write callbacks Joel Stanley
2020-01-21  4:20   ` Andrew Jeffery
2020-01-21  7:23   ` Cédric Le Goater
2020-01-21  8:50   ` Philippe Mathieu-Daudé
2020-01-21  1:33 ` [PATCH 2/2] aspeed/scu: Implement chip ID register Joel Stanley
2020-01-21  4:23   ` Andrew Jeffery
2020-01-21  7:23   ` Cédric Le Goater
2020-01-21  8:52   ` Philippe Mathieu-Daudé
2020-01-21  7:23 ` [PATCH 0/2] aspeed/scu: Implement chip id register Cédric Le Goater
2020-02-17  9:29 ` Cédric Le Goater
2020-02-17 10:24   ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.