diff mbox series

[4/7] aspeed_sdmc: Init status alwlays idle

Message ID 20180807075757.7242-5-joel@jms.id.au (mailing list archive)
State New, archived
Headers show
Series arm: aspeed: Extend SDRAM controller | expand

Commit Message

Joel Stanley Aug. 7, 2018, 7:57 a.m. UTC
The ast2500 SDRAM training routine busy waits on the 'init cycle busy
state' bit in DDR PHY Control/Status register #1 (MCR60).

This ensures the bit always reads zero, and allows training to
complete with upstream u-boot on the ast2500-evb.

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

Comments

Cédric Le Goater Aug. 7, 2018, 10:27 a.m. UTC | #1
On 08/07/2018 09:57 AM, Joel Stanley wrote:
> The ast2500 SDRAM training routine busy waits on the 'init cycle busy
> state' bit in DDR PHY Control/Status register #1 (MCR60).
> 
> This ensures the bit always reads zero, and allows training to
> complete with upstream u-boot on the ast2500-evb.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>



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

One comment below, no need to resend for that.

> ---
>  hw/misc/aspeed_sdmc.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
> index 9ece545c4ffa..522e01ef8c0d 100644
> --- a/hw/misc/aspeed_sdmc.c
> +++ b/hw/misc/aspeed_sdmc.c
> @@ -23,6 +23,10 @@
>  /* Configuration Register */
>  #define R_CONF            (0x04 / 4)
>  
> +/* Control/Status Register #1 (ast2500) */
> +#define R_STATUS1         (0x60 / 4)
> +#define   PHY_BUSY_STATE      BIT(0)
> +
>  /*
>   * Configuration register Ox4 (for Aspeed AST2400 SOC)
>   *
> @@ -137,6 +141,17 @@ static void aspeed_sdmc_write(void *opaque, hwaddr addr, uint64_t data,
>              g_assert_not_reached();
>          }
>      }
> +    if (s->silicon_rev == AST2500_A0_SILICON_REV ||
> +            s->silicon_rev == AST2500_A1_SILICON_REV) {

Maybe use the ASPEED_IS_AST2500() ? 

It would be nice to have a set of macros we could use in the different
models testing the SoC silicon revision. How about :

#define ASPEED_AST2400 0x2
#define ASPEED_AST2500 0x4
#define ASPEED_AST2600 0x6

#define ASPEED_REVISION_MAJOR(si_rev)     (((si_rev) >> 24) & 0xff)

#define ASPEED_IS_AST2400(si_rev)  (ASPEED_REVISION_MAJOR(si_rev) == ASPEED_AST2400)
#define ASPEED_IS_AST2500(si_rev)  (ASPEED_REVISION_MAJOR(si_rev) == ASPEED_AST2500)
#define ASPEED_IS_AST2600(si_rev)  (ASPEED_REVISION_MAJOR(si_rev) == ASPEED_AST2600)


Thanks,

C.


> +        switch (addr) {
> +        case R_STATUS1:
> +            /* Will never return 'busy' */
> +            data &= ~PHY_BUSY_STATE;
> +            break;
> +        default:
> +            break;
> +        }
> +    }
>  
>      s->regs[addr] = data;
>  }
>
diff mbox series

Patch

diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index 9ece545c4ffa..522e01ef8c0d 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -23,6 +23,10 @@ 
 /* Configuration Register */
 #define R_CONF            (0x04 / 4)
 
+/* Control/Status Register #1 (ast2500) */
+#define R_STATUS1         (0x60 / 4)
+#define   PHY_BUSY_STATE      BIT(0)
+
 /*
  * Configuration register Ox4 (for Aspeed AST2400 SOC)
  *
@@ -137,6 +141,17 @@  static void aspeed_sdmc_write(void *opaque, hwaddr addr, uint64_t data,
             g_assert_not_reached();
         }
     }
+    if (s->silicon_rev == AST2500_A0_SILICON_REV ||
+            s->silicon_rev == AST2500_A1_SILICON_REV) {
+        switch (addr) {
+        case R_STATUS1:
+            /* Will never return 'busy' */
+            data &= ~PHY_BUSY_STATE;
+            break;
+        default:
+            break;
+        }
+    }
 
     s->regs[addr] = data;
 }