qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: BALATON Zoltan <balaton@eik.bme.hu>
To: "Cédric Le Goater" <clg@kaod.org>
Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org,
	 Daniel Henrique Barboza <danielhb413@gmail.com>,
	 Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [PATCH 13/20] ppc4xx_sdram: Rename functions to prevent name clashes
Date: Wed, 7 Sep 2022 16:38:05 +0200 (CEST)	[thread overview]
Message-ID: <f3f4e37-8b3c-c193-2ba8-fa4fea9b8b9d@eik.bme.hu> (raw)
In-Reply-To: <156ed3a5-1b22-2892-fff9-93110d91e318@kaod.org>

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

On Wed, 7 Sep 2022, Cédric Le Goater wrote:
> On 8/19/22 18:55, BALATON Zoltan wrote:
>> Rename functions to avoid name clashes when moving the DDR2 controller
>> model currently called ppc440_sdram to ppc4xx_devs. This also more
>> clearly shows which function belongs to which model.
>
> Shouldn't we introduce class handlers instead  ?

What do you mean? Could you please explain more?

Regards,
BALATON Zoltan

> Thanks,
>
> C.
>
>
>> 
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/ppc/ppc440_uc.c   | 69 ++++++++++++++++++++++----------------------
>>   hw/ppc/ppc4xx_devs.c | 44 ++++++++++++++--------------
>>   2 files changed, 57 insertions(+), 56 deletions(-)
>> 
>> diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
>> index 72eb75d3d2..b39c6dbbd2 100644
>> --- a/hw/ppc/ppc440_uc.c
>> +++ b/hw/ppc/ppc440_uc.c
>> @@ -502,7 +502,7 @@ enum {
>>       SDRAM_PLBADDUHB = 0x50,
>>   };
>>   -static uint32_t sdram_bcr(hwaddr ram_base, hwaddr ram_size)
>> +static uint32_t sdram_ddr2_bcr(hwaddr ram_base, hwaddr ram_size)
>>   {
>>       uint32_t bcr;
>>   @@ -547,12 +547,12 @@ static uint32_t sdram_bcr(hwaddr ram_base, hwaddr 
>> ram_size)
>>       return bcr;
>>   }
>>   -static inline hwaddr sdram_base(uint32_t bcr)
>> +static inline hwaddr sdram_ddr2_base(uint32_t bcr)
>>   {
>>       return (bcr & 0xffe00000) << 2;
>>   }
>>   -static uint64_t sdram_size(uint32_t bcr)
>> +static uint64_t sdram_ddr2_size(uint32_t bcr)
>>   {
>>       uint64_t size;
>>       int sh;
>> @@ -578,50 +578,51 @@ static void sdram_bank_unmap(Ppc4xxSdramBank *bank)
>>       object_unparent(OBJECT(&bank->container));
>>   }
>>   -static void sdram_set_bcr(ppc440_sdram_t *sdram, int i,
>> -                          uint32_t bcr, int enabled)
>> +static void sdram_ddr2_set_bcr(ppc440_sdram_t *sdram, int i,
>> +                               uint32_t bcr, int enabled)
>>   {
>>       if (sdram->bank[i].bcr & 1) {
>>           /* First unmap RAM if enabled */
>> -        trace_ppc4xx_sdram_unmap(sdram_base(sdram->bank[i].bcr),
>> -                                 sdram_size(sdram->bank[i].bcr));
>> +        trace_ppc4xx_sdram_unmap(sdram_ddr2_base(sdram->bank[i].bcr),
>> +                                 sdram_ddr2_size(sdram->bank[i].bcr));
>>           sdram_bank_unmap(&sdram->bank[i]);
>>       }
>>       sdram->bank[i].bcr = bcr & 0xffe0ffc1;
>> -    sdram->bank[i].base = sdram_base(bcr);
>> -    sdram->bank[i].size = sdram_size(bcr);
>> +    sdram->bank[i].base = sdram_ddr2_base(bcr);
>> +    sdram->bank[i].size = sdram_ddr2_size(bcr);
>>       if (enabled && (bcr & 1)) {
>> -        trace_ppc4xx_sdram_map(sdram_base(bcr), sdram_size(bcr));
>> +        trace_ppc4xx_sdram_map(sdram_ddr2_base(bcr), 
>> sdram_ddr2_size(bcr));
>>           sdram_bank_map(&sdram->bank[i]);
>>       }
>>   }
>>   -static void sdram_map_bcr(ppc440_sdram_t *sdram)
>> +static void sdram_ddr2_map_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_bcr(sdram->bank[i].base,
>> +            sdram_ddr2_set_bcr(sdram, i,
>> +                               sdram_ddr2_bcr(sdram->bank[i].base,
>>                                                 sdram->bank[i].size), 1);
>>           } else {
>> -            sdram_set_bcr(sdram, i, 0, 0);
>> +            sdram_ddr2_set_bcr(sdram, i, 0, 0);
>>           }
>>       }
>>   }
>>   -static void sdram_unmap_bcr(ppc440_sdram_t *sdram)
>> +static void sdram_ddr2_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);
>> +            sdram_ddr2_set_bcr(sdram, i, sdram->bank[i].bcr & ~1, 0);
>>           }
>>       }
>>   }
>>   -static uint32_t dcr_read_sdram(void *opaque, int dcrn)
>> +static uint32_t sdram_ddr2_dcr_read(void *opaque, int dcrn)
>>   {
>>       ppc440_sdram_t *sdram = opaque;
>>       uint32_t ret = 0;
>> @@ -632,8 +633,8 @@ static uint32_t dcr_read_sdram(void *opaque, int dcrn)
>>       case SDRAM_R2BAS:
>>       case SDRAM_R3BAS:
>>           if (sdram->bank[dcrn - SDRAM_R0BAS].size) {
>> -            ret = sdram_bcr(sdram->bank[dcrn - SDRAM_R0BAS].base,
>> -                            sdram->bank[dcrn - SDRAM_R0BAS].size);
>> +            ret = sdram_ddr2_bcr(sdram->bank[dcrn - SDRAM_R0BAS].base,
>> +                                 sdram->bank[dcrn - SDRAM_R0BAS].size);
>>           }
>>           break;
>>       case SDRAM_CONF1HB:
>> @@ -674,7 +675,7 @@ static uint32_t dcr_read_sdram(void *opaque, int dcrn)
>>       return ret;
>>   }
>>   -static void dcr_write_sdram(void *opaque, int dcrn, uint32_t val)
>> +static void sdram_ddr2_dcr_write(void *opaque, int dcrn, uint32_t val)
>>   {
>>       ppc440_sdram_t *sdram = opaque;
>>   @@ -700,12 +701,12 @@ static void dcr_write_sdram(void *opaque, int dcrn, 
>> uint32_t val)
>>               if (!(sdram->mcopt2 & 0x08000000) && (val & 0x08000000)) {
>>                   trace_ppc4xx_sdram_enable("enable");
>>                   /* validate all RAM mappings */
>> -                sdram_map_bcr(sdram);
>> +                sdram_ddr2_map_bcr(sdram);
>>                   sdram->mcopt2 |= 0x08000000;
>>               } else if ((sdram->mcopt2 & 0x08000000) && !(val & 
>> 0x08000000)) {
>>                   trace_ppc4xx_sdram_enable("disable");
>>                   /* invalidate all RAM mappings */
>> -                sdram_unmap_bcr(sdram);
>> +                sdram_ddr2_unmap_bcr(sdram);
>>                   sdram->mcopt2 &= ~0x08000000;
>>               }
>>               break;
>> @@ -718,7 +719,7 @@ static void dcr_write_sdram(void *opaque, int dcrn, 
>> uint32_t val)
>>       }
>>   }
>>   -static void sdram_reset(void *opaque)
>> +static void sdram_ddr2_reset(void *opaque)
>>   {
>>       ppc440_sdram_t *sdram = opaque;
>>   @@ -739,30 +740,30 @@ void ppc440_sdram_init(CPUPPCState *env, int 
>> nbanks,
>>           s->bank[i].base = ram_banks[i].base;
>>           s->bank[i].size = ram_banks[i].size;
>>       }
>> -    qemu_register_reset(&sdram_reset, s);
>> +    qemu_register_reset(&sdram_ddr2_reset, s);
>>       ppc_dcr_register(env, SDRAM0_CFGADDR,
>> -                     s, &dcr_read_sdram, &dcr_write_sdram);
>> +                     s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write);
>>       ppc_dcr_register(env, SDRAM0_CFGDATA,
>> -                     s, &dcr_read_sdram, &dcr_write_sdram);
>> +                     s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write);
>>         ppc_dcr_register(env, SDRAM_R0BAS,
>> -                     s, &dcr_read_sdram, &dcr_write_sdram);
>> +                     s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write);
>>       ppc_dcr_register(env, SDRAM_R1BAS,
>> -                     s, &dcr_read_sdram, &dcr_write_sdram);
>> +                     s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write);
>>       ppc_dcr_register(env, SDRAM_R2BAS,
>> -                     s, &dcr_read_sdram, &dcr_write_sdram);
>> +                     s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write);
>>       ppc_dcr_register(env, SDRAM_R3BAS,
>> -                     s, &dcr_read_sdram, &dcr_write_sdram);
>> +                     s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write);
>>       ppc_dcr_register(env, SDRAM_CONF1HB,
>> -                     s, &dcr_read_sdram, &dcr_write_sdram);
>> +                     s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write);
>>       ppc_dcr_register(env, SDRAM_PLBADDULL,
>> -                     s, &dcr_read_sdram, &dcr_write_sdram);
>> +                     s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write);
>>       ppc_dcr_register(env, SDRAM_CONF1LL,
>> -                     s, &dcr_read_sdram, &dcr_write_sdram);
>> +                     s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write);
>>       ppc_dcr_register(env, SDRAM_CONFPATHB,
>> -                     s, &dcr_read_sdram, &dcr_write_sdram);
>> +                     s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write);
>>       ppc_dcr_register(env, SDRAM_PLBADDUHB,
>> -                     s, &dcr_read_sdram, &dcr_write_sdram);
>> +                     s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write);
>>   }
>>     /*****************************************************************************/
>> diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
>> index bfe7b2d3a6..7655967351 100644
>> --- a/hw/ppc/ppc4xx_devs.c
>> +++ b/hw/ppc/ppc4xx_devs.c
>> @@ -81,12 +81,12 @@ static uint32_t sdram_ddr_bcr(hwaddr ram_base, hwaddr 
>> ram_size)
>>       return bcr;
>>   }
>>   -static inline hwaddr sdram_base(uint32_t bcr)
>> +static inline hwaddr sdram_ddr_base(uint32_t bcr)
>>   {
>>       return bcr & 0xFF800000;
>>   }
>>   -static target_ulong sdram_size(uint32_t bcr)
>> +static target_ulong sdram_ddr_size(uint32_t bcr)
>>   {
>>       target_ulong size;
>>       int sh;
>> @@ -101,13 +101,13 @@ static target_ulong sdram_size(uint32_t bcr)
>>       return size;
>>   }
>>   -static void sdram_set_bcr(Ppc4xxSdramDdrState *sdram, int i,
>> -                          uint32_t bcr, int enabled)
>> +static void sdram_ddr_set_bcr(Ppc4xxSdramDdrState *sdram, int i,
>> +                              uint32_t bcr, int enabled)
>>   {
>>       if (sdram->bank[i].bcr & 1) {
>>           /* Unmap RAM */
>> -        trace_ppc4xx_sdram_unmap(sdram_base(sdram->bank[i].bcr),
>> -                                 sdram_size(sdram->bank[i].bcr));
>> +        trace_ppc4xx_sdram_unmap(sdram_ddr_base(sdram->bank[i].bcr),
>> +                                 sdram_ddr_size(sdram->bank[i].bcr));
>>           memory_region_del_subregion(get_system_memory(),
>>                                       &sdram->bank[i].container);
>>           memory_region_del_subregion(&sdram->bank[i].container,
>> @@ -116,38 +116,38 @@ static void sdram_set_bcr(Ppc4xxSdramDdrState *sdram, 
>> int i,
>>       }
>>       sdram->bank[i].bcr = bcr & 0xFFDEE001;
>>       if (enabled && (bcr & 1)) {
>> -        trace_ppc4xx_sdram_map(sdram_base(bcr), sdram_size(bcr));
>> +        trace_ppc4xx_sdram_map(sdram_ddr_base(bcr), sdram_ddr_size(bcr));
>>           memory_region_init(&sdram->bank[i].container, NULL, 
>> "sdram-container",
>> -                           sdram_size(bcr));
>> +                           sdram_ddr_size(bcr));
>>           memory_region_add_subregion(&sdram->bank[i].container, 0,
>>                                       &sdram->bank[i].ram);
>>           memory_region_add_subregion(get_system_memory(),
>> -                                    sdram_base(bcr),
>> +                                    sdram_ddr_base(bcr),
>>                                       &sdram->bank[i].container);
>>       }
>>   }
>>   -static void sdram_map_bcr(Ppc4xxSdramDdrState *sdram)
>> +static void sdram_ddr_map_bcr(Ppc4xxSdramDdrState *sdram)
>>   {
>>       int i;
>>         for (i = 0; i < sdram->nbanks; i++) {
>>           if (sdram->bank[i].size != 0) {
>> -            sdram_set_bcr(sdram, i, sdram_ddr_bcr(sdram->bank[i].base,
>> -                                                  sdram->bank[i].size), 
>> 1);
>> +            sdram_ddr_set_bcr(sdram, i, sdram_ddr_bcr(sdram->bank[i].base,
>> + 
>> sdram->bank[i].size), 1);
>>           } else {
>> -            sdram_set_bcr(sdram, i, 0, 0);
>> +            sdram_ddr_set_bcr(sdram, i, 0, 0);
>>           }
>>       }
>>   }
>>   -static void sdram_unmap_bcr(Ppc4xxSdramDdrState *sdram)
>> +static void sdram_ddr_unmap_bcr(Ppc4xxSdramDdrState *sdram)
>>   {
>>       int i;
>>         for (i = 0; i < sdram->nbanks; i++) {
>> -        trace_ppc4xx_sdram_unmap(sdram_base(sdram->bank[i].bcr),
>> -                                 sdram_size(sdram->bank[i].bcr));
>> +        trace_ppc4xx_sdram_unmap(sdram_ddr_base(sdram->bank[i].bcr),
>> +                                 sdram_ddr_size(sdram->bank[i].bcr));
>>           memory_region_del_subregion(get_system_memory(),
>>                                       &sdram->bank[i].ram);
>>       }
>> @@ -244,12 +244,12 @@ static void sdram_ddr_dcr_write(void *opaque, int 
>> dcrn, uint32_t val)
>>               if (!(sdram->cfg & 0x80000000) && (val & 0x80000000)) {
>>                   trace_ppc4xx_sdram_enable("enable");
>>                   /* validate all RAM mappings */
>> -                sdram_map_bcr(sdram);
>> +                sdram_ddr_map_bcr(sdram);
>>                   sdram->status &= ~0x80000000;
>>               } else if ((sdram->cfg & 0x80000000) && !(val & 0x80000000)) 
>> {
>>                   trace_ppc4xx_sdram_enable("disable");
>>                   /* invalidate all RAM mappings */
>> -                sdram_unmap_bcr(sdram);
>> +                sdram_ddr_unmap_bcr(sdram);
>>                   sdram->status |= 0x80000000;
>>               }
>>               if (!(sdram->cfg & 0x40000000) && (val & 0x40000000)) {
>> @@ -269,16 +269,16 @@ static void sdram_ddr_dcr_write(void *opaque, int 
>> dcrn, uint32_t val)
>>               sdram->pmit = (val & 0xF8000000) | 0x07C00000;
>>               break;
>>           case 0x40: /* SDRAM_B0CR */
>> -            sdram_set_bcr(sdram, 0, val, sdram->cfg & 0x80000000);
>> +            sdram_ddr_set_bcr(sdram, 0, val, sdram->cfg & 0x80000000);
>>               break;
>>           case 0x44: /* SDRAM_B1CR */
>> -            sdram_set_bcr(sdram, 1, val, sdram->cfg & 0x80000000);
>> +            sdram_ddr_set_bcr(sdram, 1, val, sdram->cfg & 0x80000000);
>>               break;
>>           case 0x48: /* SDRAM_B2CR */
>> -            sdram_set_bcr(sdram, 2, val, sdram->cfg & 0x80000000);
>> +            sdram_ddr_set_bcr(sdram, 2, val, sdram->cfg & 0x80000000);
>>               break;
>>           case 0x4C: /* SDRAM_B3CR */
>> -            sdram_set_bcr(sdram, 3, val, sdram->cfg & 0x80000000);
>> +            sdram_ddr_set_bcr(sdram, 3, val, sdram->cfg & 0x80000000);
>>               break;
>>           case 0x80: /* SDRAM_TR */
>>               sdram->tr = val & 0x018FC01F;
>
>
>

  reply	other threads:[~2022-09-07 14:40 UTC|newest]

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

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=f3f4e37-8b3c-c193-2ba8-fa4fea9b8b9d@eik.bme.hu \
    --to=balaton@eik.bme.hu \
    --cc=clg@kaod.org \
    --cc=danielhb413@gmail.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).