All of lore.kernel.org
 help / color / mirror / Atom feed
From: BALATON Zoltan <balaton@eik.bme.hu>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, clg@kaod.org,
	 Daniel Henrique Barboza <danielhb413@gmail.com>,
	 Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [PATCH v4 14/21] ppc440_sdram: Move RAM size check to ppc440_sdram_init
Date: Sun, 18 Sep 2022 11:59:04 +0200 (CEST)	[thread overview]
Message-ID: <e2fc703c-5b2-3a4e-261b-ad6751abc37@eik.bme.hu> (raw)
In-Reply-To: <cb366d11-7abd-c70c-6392-d74f3fabf04b@amsat.org>

[-- Attachment #1: Type: text/plain, Size: 5933 bytes --]

On Sun, 18 Sep 2022, Philippe Mathieu-Daudé wrote:
> On 14/9/22 13:34, BALATON Zoltan wrote:
>> Move the check for valid memory sizes from board to sdram controller
>> init. Board now only checks for additional restrictions imposed by
>> firmware then sdram init checks for valid sizes for SoC.
>> 
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/ppc/ppc440.h    |  4 ++--
>>   hw/ppc/ppc440_uc.c | 15 +++++++--------
>>   hw/ppc/sam460ex.c  | 32 +++++++++++++++++---------------
>>   3 files changed, 26 insertions(+), 25 deletions(-)
>> 
>> diff --git a/hw/ppc/ppc440.h b/hw/ppc/ppc440.h
>> index 01d76b8000..29f6f14ed7 100644
>> --- a/hw/ppc/ppc440.h
>> +++ b/hw/ppc/ppc440.h
>> @@ -11,13 +11,13 @@
>>   #ifndef PPC440_H
>>   #define PPC440_H
>>   -#include "hw/ppc/ppc4xx.h"
>> +#include "hw/ppc/ppc.h"
>>     void ppc4xx_l2sram_init(CPUPPCState *env);
>>   void ppc4xx_cpr_init(CPUPPCState *env);
>>   void ppc4xx_sdr_init(CPUPPCState *env);
>>   void ppc440_sdram_init(CPUPPCState *env, int nbanks,
>> -                       Ppc4xxSdramBank *ram_banks);
>> +                       MemoryRegion *ram);
>>   void ppc4xx_ahb_init(CPUPPCState *env);
>>   void ppc4xx_dma_init(CPUPPCState *env, int dcr_base);
>>   void ppc460ex_pcie_init(CPUPPCState *env);
>> diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
>> index fa313f979d..bd2a489557 100644
>> --- a/hw/ppc/ppc440_uc.c
>> +++ b/hw/ppc/ppc440_uc.c
>> @@ -486,7 +486,7 @@ void ppc4xx_sdr_init(CPUPPCState *env)
>>   typedef struct ppc440_sdram_t {
>>       uint32_t addr;
>>       uint32_t mcopt2;
>> -    int nbanks;
>> +    int nbanks; /* Banks to use from the 4, e.g. when board has less slots 
>> */
>>       Ppc4xxSdramBank bank[4];
>>   } ppc440_sdram_t;
>>   @@ -732,18 +732,17 @@ static void sdram_ddr2_reset(void *opaque)
>>   }
>>     void ppc440_sdram_init(CPUPPCState *env, int nbanks,
>> -                       Ppc4xxSdramBank *ram_banks)
>> +                       MemoryRegion *ram)
>>   {
>>       ppc440_sdram_t *s;
>> -    int i;
>> +    const ram_addr_t valid_bank_sizes[] = {
>> +        4 * GiB, 2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB, 64 * 
>> MiB,
>> +        32 * MiB, 16 * MiB, 8 * MiB, 0
>
> Where 16/8 come from?

From sdram_ddr2_bcr(). This array now has the missing 4 GB, 16 and 8 MB 
the SoC can handle but the board can't and added separate checks for the 
board restrictions in the board code. Probably isn't worth splitting this 
out in a separate patch but I should more explicitly mention it in the 
commit message to make it clearer.

Regards,
BALATON Zoltan

>> +    };
>>         s = g_malloc0(sizeof(*s));
>>       s->nbanks = nbanks;
>> -    for (i = 0; i < nbanks; i++) {
>> -        s->bank[i].ram = ram_banks[i].ram;
>> -        s->bank[i].base = ram_banks[i].base;
>> -        s->bank[i].size = ram_banks[i].size;
>> -    }
>> +    ppc4xx_sdram_banks(ram, s->nbanks, s->bank, valid_bank_sizes);
>>       qemu_register_reset(&sdram_ddr2_reset, s);
>>       ppc_dcr_register(env, SDRAM0_CFGADDR,
>>                        s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write);
>> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
>> index dac329d482..9b850808a3 100644
>> --- a/hw/ppc/sam460ex.c
>> +++ b/hw/ppc/sam460ex.c
>> @@ -74,13 +74,6 @@
>>   #define EBC_FREQ 115000000
>>   #define UART_FREQ 11059200
>>   -/* The SoC could also handle 4 GiB but firmware does not work with that. 
>> */
>> -/* Maybe it overflows a signed 32 bit number somewhere? */
>> -static const ram_addr_t ppc460ex_sdram_bank_sizes[] = {
>> -    2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB, 64 * MiB,
>> -    32 * MiB, 0
>> -};
>> -
>>   struct boot_info {
>>       uint32_t dt_base;
>>       uint32_t dt_size;
>> @@ -273,7 +266,6 @@ static void sam460ex_init(MachineState *machine)
>>   {
>>       MemoryRegion *address_space_mem = get_system_memory();
>>       MemoryRegion *isa = g_new(MemoryRegion, 1);
>> -    Ppc4xxSdramBank *ram_banks = g_new0(Ppc4xxSdramBank, 1);
>>       MemoryRegion *l2cache_ram = g_new(MemoryRegion, 1);
>>       DeviceState *uic[4];
>>       int i;
>> @@ -340,12 +332,22 @@ static void sam460ex_init(MachineState *machine)
>>       }
>>         /* SDRAM controller */
>> -    /* put all RAM on first bank because board has one slot
>> -     * and firmware only checks that */
>> -    ppc4xx_sdram_banks(machine->ram, 1, ram_banks, 
>> ppc460ex_sdram_bank_sizes);
>> -
>> +    /* The SoC could also handle 4 GiB but firmware does not work with 
>> that. */
>> +    if (machine->ram_size > 2 * GiB) {
>> +        error_report("Memory over 2 GiB is not supported");
>> +        exit(1);
>> +    }
>> +    /* Firmware needs at least 64 MiB */
>> +    if (machine->ram_size < 64 * MiB) {
>> +        error_report("Memory below 64 MiB is not supported");
>> +        exit(1);
>> +    }
>> +    /*
>> +     * Put all RAM on first bank because board has one slot
>> +     * and firmware only checks that
>> +     */
>> +    ppc440_sdram_init(env, 1, machine->ram);
>>       /* FIXME: does 460EX have ECC interrupts? */
>> -    ppc440_sdram_init(env, 1, ram_banks);
>>       /* Enable SDRAM memory regions as we may boot without firmware */
>>       if (ppc_dcr_write(env->dcr_env, SDRAM0_CFGADDR, 0x21) ||
>>           ppc_dcr_write(env->dcr_env, SDRAM0_CFGDATA, 0x08000000)) {
>> @@ -358,8 +360,8 @@ static void sam460ex_init(MachineState *machine)
>>                                  qdev_get_gpio_in(uic[0], 2));
>>       i2c = PPC4xx_I2C(dev)->bus;
>>       /* SPD EEPROM on RAM module */
>> -    spd_data = spd_data_generate(ram_banks->size < 128 * MiB ? DDR : DDR2,
>> -                                 ram_banks->size);
>> +    spd_data = spd_data_generate(machine->ram_size < 128 * MiB ? DDR : 
>> DDR2,
>> +                                 machine->ram_size);
>>       spd_data[20] = 4; /* SO-DIMM module */
>>       smbus_eeprom_init_one(i2c, 0x50, spd_data);
>>       /* RTC */
>
>

  reply	other threads:[~2022-09-18 10:00 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-14 11:34 [PATCH v4 00/21] ppc4xx_sdram QOMify and clean ups BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 01/21] ppc440_bamboo: Remove unnecessary memsets BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 02/21] ppc4xx: Introduce Ppc4xxSdramBank struct BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 03/21] ppc4xx_sdram: Get rid of the init RAM hack BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 04/21] ppc4xx: Use Ppc4xxSdramBank in ppc4xx_sdram_banks() BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 05/21] ppc440_bamboo: Add missing 4 MiB valid memory size BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 06/21] ppc4xx_sdram: Move size check to ppc4xx_sdram_init() BALATON Zoltan
2022-09-17 21:54   ` Philippe Mathieu-Daudé via
2022-09-14 11:34 ` [PATCH v4 07/21] ppc4xx_sdram: QOM'ify BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 08/21] ppc4xx_sdram: Drop extra zeros for readability BALATON Zoltan
2022-09-17 22:12   ` Philippe Mathieu-Daudé via
2022-09-14 11:34 ` [PATCH v4 09/21] ppc440_sdram: Split off map/unmap of sdram banks for later reuse BALATON Zoltan
2022-09-17 22:14   ` Philippe Mathieu-Daudé via
2022-09-14 11:34 ` [PATCH v4 10/21] ppc440_sdram: Implement enable bit in the DDR2 SDRAM BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 11/21] ppc440_sdram: Get rid of the init RAM hack BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 12/21] ppc440_sdram: Rename local variable for readability BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 13/21] ppc4xx_sdram: Rename functions to prevent name clashes BALATON Zoltan
2022-09-17 22:15   ` Philippe Mathieu-Daudé via
2022-09-14 11:34 ` [PATCH v4 14/21] ppc440_sdram: Move RAM size check to ppc440_sdram_init BALATON Zoltan
2022-09-17 22:16   ` Philippe Mathieu-Daudé via
2022-09-18  9:59     ` BALATON Zoltan [this message]
2022-09-14 11:34 ` [PATCH v4 15/21] ppc440_sdram: QOM'ify BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 16/21] ppc4xx_sdram: Move ppc4xx DDR and DDR2 SDRAM controller models together BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 17/21] ppc4xx_sdram: Use hwaddr for memory bank size BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 18/21] ppc4xx_sdram: Rename local state variable for brevity BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 19/21] ppc4xx_sdram: Generalise bank setup BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 20/21] ppc4xx_sdram: Convert DDR SDRAM controller to new bank handling BALATON Zoltan
2022-09-14 11:34 ` [PATCH v4 21/21] ppc4xx_sdram: Add errp parameter to ppc4xx_sdram_banks() BALATON Zoltan
2022-09-17 22:18   ` Philippe Mathieu-Daudé via

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e2fc703c-5b2-3a4e-261b-ad6751abc37@eik.bme.hu \
    --to=balaton@eik.bme.hu \
    --cc=clg@kaod.org \
    --cc=danielhb413@gmail.com \
    --cc=f4bug@amsat.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.