diff mbox series

[4/7] aspeed_sdmc: Init status alwlays idle

Message ID 20180807075757.7242-5-joel@jms.id.au
State Superseded
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(+)

-- 
2.17.1

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;
 }