From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B8D26C38145 for ; Wed, 7 Sep 2022 14:35:57 +0000 (UTC) Received: from localhost ([::1]:54018 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oVw9k-0007c6-RY for qemu-devel@archiver.kernel.org; Wed, 07 Sep 2022 10:35:56 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:36776) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oVw8T-0006Dk-0j; Wed, 07 Sep 2022 10:34:37 -0400 Received: from zero.eik.bme.hu ([2001:738:2001:2001::2001]:32316) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oVw8N-0008W8-86; Wed, 07 Sep 2022 10:34:36 -0400 Received: from zero.eik.bme.hu (blah.eik.bme.hu [152.66.115.182]) by localhost (Postfix) with SMTP id EB3E874632B; Wed, 7 Sep 2022 16:34:26 +0200 (CEST) Received: by zero.eik.bme.hu (Postfix, from userid 432) id 7C9E17461AE; Wed, 7 Sep 2022 16:34:26 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by zero.eik.bme.hu (Postfix) with ESMTP id 7AE86745702; Wed, 7 Sep 2022 16:34:26 +0200 (CEST) Date: Wed, 7 Sep 2022 16:34:26 +0200 (CEST) From: BALATON Zoltan To: =?ISO-8859-15?Q?C=E9dric_Le_Goater?= cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Daniel Henrique Barboza , Peter Maydell Subject: Re: [PATCH 09/20] ppc440_sdram: Split off map/unmap of sdram banks for later reuse In-Reply-To: <2fd70fa7-3620-2206-0d9a-2287e94e4a90@kaod.org> Message-ID: <7be73bd-601a-46a5-4c71-f5689f30b1c4@eik.bme.hu> References: <2fd70fa7-3620-2206-0d9a-2287e94e4a90@kaod.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="3866299591-914918268-1662561266=:35947" Received-SPF: pass client-ip=2001:738:2001:2001::2001; envelope-from=balaton@eik.bme.hu; helo=zero.eik.bme.hu X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, T_SCC_BODY_TEXT_LINE=-0.01, T_SPF_TEMPERROR=0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --3866299591-914918268-1662561266=:35947 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8BIT On Wed, 7 Sep 2022, Cédric Le Goater wrote: > On 8/19/22 18:55, BALATON Zoltan wrote: >> Signed-off-by: BALATON Zoltan >> --- >> hw/ppc/ppc440_uc.c | 31 +++++++++++++++++++------------ >> 1 file changed, 19 insertions(+), 12 deletions(-) >> >> diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c >> index 3507c35b63..c33f91e134 100644 >> --- a/hw/ppc/ppc440_uc.c >> +++ b/hw/ppc/ppc440_uc.c >> @@ -561,26 +561,33 @@ static uint64_t sdram_size(uint32_t bcr) >> return size; >> } >> +static void sdram_bank_map(Ppc4xxSdramBank *bank) >> +{ >> + memory_region_init(&bank->container, NULL, "sdram-container", >> bank->size); > > I don't think we need to init the ->container memory region each time. > This could be done once and for all in the realize handler. > >> + memory_region_add_subregion(&bank->container, 0, &bank->ram); >> + memory_region_add_subregion(get_system_memory(), bank->base, >> + &bank->container); >> +} >> + >> +static void sdram_bank_unmap(Ppc4xxSdramBank *bank) >> +{ >> + memory_region_del_subregion(get_system_memory(), &bank->container); >> + memory_region_del_subregion(&bank->container, &bank->ram); >> + object_unparent(OBJECT(&bank->container)); > > object_unparent could be dropped if the memory_region_init was called in > realize. > > Also, memory_region_set_enabled() might be a better alternative. I think these could be considered as a follow up later, I don't want to change it now to avoid breaking it more as I've already managed to break ref405ep as you found (this will be fixed in v2) so I'd not try to change this in this series. Regards, BALATON Zoltan > Thanks, > > C. > > >> +} >> + >> static void sdram_set_bcr(ppc440_sdram_t *sdram, int i, >> uint32_t bcr, int enabled) >> { >> if (sdram->bank[i].bcr & 1) { >> /* First unmap RAM if enabled */ >> - memory_region_del_subregion(get_system_memory(), >> - &sdram->bank[i].container); >> - memory_region_del_subregion(&sdram->bank[i].container, >> - &sdram->bank[i].ram); >> - object_unparent(OBJECT(&sdram->bank[i].container)); >> + 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); >> if (enabled && (bcr & 1)) { >> - memory_region_init(&sdram->bank[i].container, NULL, >> "sdram-container", >> - sdram_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->bank[i].container); >> + sdram_bank_map(&sdram->bank[i]); >> } >> } >> > > > --3866299591-914918268-1662561266=:35947--