Re: [PATCH v6 10/25] ppc440_sdram: Implement enable bit in the DDR2 SDRAM controller

[Daniel Henrique Barboza]

On 9/24/22 09:27, BALATON Zoltan wrote:
> To allow removing the do_init hack we need to improve the DDR2 SDRAM
> controller model to handle the enable/disable bit that it ignored so
> far.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   hw/ppc/ppc440_uc.c | 34 ++++++++++++++++++++++++++++++++--
>   1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
> index 900b7ab998..3fbfe4ad13 100644
> --- a/hw/ppc/ppc440_uc.c
> +++ b/hw/ppc/ppc440_uc.c
> @@ -485,6 +485,7 @@ void ppc4xx_sdr_init(CPUPPCState *env)
>   /* SDRAM controller */
>   typedef struct ppc440_sdram_t {
>       uint32_t addr;
> +    uint32_t mcopt2;
>       int nbanks;
>       Ppc4xxSdramBank bank[4];
>   } ppc440_sdram_t;
> @@ -600,7 +601,7 @@ static void sdram_map_bcr(ppc440_sdram_t *sdram)
>       int i;
>   
>       for (i = 0; i < sdram->nbanks; i++) {
> -        if (sdram->bank[i].size != 0) {
> +        if (sdram->bank[i].size) {
>               sdram_set_bcr(sdram, i, sdram_bcr(sdram->bank[i].base,
>                                                 sdram->bank[i].size), 1);
>           } else {
> @@ -609,6 +610,17 @@ static void sdram_map_bcr(ppc440_sdram_t *sdram)
>       }
>   }
>   
> +static void sdram_unmap_bcr(ppc440_sdram_t *sdram)
> +{
> +    int i;
> +
> +    for (i = 0; i < sdram->nbanks; i++) {
> +        if (sdram->bank[i].size) {
> +            sdram_set_bcr(sdram, i, sdram->bank[i].bcr & ~1, 0);
> +        }
> +    }
> +}
> +
>   static uint32_t dcr_read_sdram(void *opaque, int dcrn)
>   {
>       ppc440_sdram_t *sdram = opaque;
> @@ -640,7 +652,7 @@ static uint32_t dcr_read_sdram(void *opaque, int dcrn)
>               ret = 0x80000000;
>               break;
>           case 0x21: /* SDRAM_MCOPT2 */
> -            ret = 0x08000000;
> +            ret = sdram->mcopt2;
>               break;
>           case 0x40: /* SDRAM_MB0CF */
>               ret = 0x00008001;
> @@ -662,6 +674,8 @@ static uint32_t dcr_read_sdram(void *opaque, int dcrn)
>       return ret;
>   }
>   
> +#define SDRAM_DDR2_MCOPT2_DCEN BIT(27)
> +
>   static void dcr_write_sdram(void *opaque, int dcrn, uint32_t val)
>   {
>       ppc440_sdram_t *sdram = opaque;
> @@ -684,6 +698,21 @@ static void dcr_write_sdram(void *opaque, int dcrn, 
> uint32_t val)
>           switch (sdram->addr) {
>           case 0x00: /* B0CR */
>               break;
> +        case 0x21: /* SDRAM_MCOPT2 */
> +            if (!(sdram->mcopt2 & SDRAM_DDR2_MCOPT2_DCEN) &&
> +                (val & SDRAM_DDR2_MCOPT2_DCEN)) {
> +                trace_ppc4xx_sdram_enable("enable");
> +                /* validate all RAM mappings */
> +                sdram_map_bcr(sdram);
> +                sdram->mcopt2 |= SDRAM_DDR2_MCOPT2_DCEN;
> +            } else if ((sdram->mcopt2 & SDRAM_DDR2_MCOPT2_DCEN) &&
> +                       !(val & SDRAM_DDR2_MCOPT2_DCEN)) {
> +                trace_ppc4xx_sdram_enable("disable");
> +                /* invalidate all RAM mappings */
> +                sdram_unmap_bcr(sdram);
> +                sdram->mcopt2 &= ~SDRAM_DDR2_MCOPT2_DCEN;
> +            }
> +            break;
>           default:
>               break;
>           }
> @@ -698,6 +727,7 @@ static void sdram_reset(void *opaque)
>       ppc440_sdram_t *sdram = opaque;
>   
>       sdram->addr = 0;
> +    sdram->mcopt2 = SDRAM_DDR2_MCOPT2_DCEN;
>   }
>   
>   void ppc440_sdram_init(CPUPPCState *env, int nbanks,