Apparently l2sram_update_mappings() bit-rotted over time,
when defining MAP_L2SRAM we get:
hw/ppc/ppc440_uc.c:83:17: error: no member named 'isarc' in 'struct ppc4xx_l2sram_t'
if (l2sram->isarc != isarc ||
~~~~~~ ^
hw/ppc/ppc440_uc.c:84:18: error: no member named 'isacntl' in 'struct ppc4xx_l2sram_t'
(l2sram->isacntl & 0x80000000) != (isacntl & 0x80000000)) {
~~~~~~ ^
hw/ppc/ppc440_uc.c:85:21: error: no member named 'isacntl' in 'struct ppc4xx_l2sram_t'
if (l2sram->isacntl & 0x80000000) {
~~~~~~ ^
hw/ppc/ppc440_uc.c:88:50: error: no member named 'isarc_ram' in 'struct ppc4xx_l2sram_t'
&l2sram->isarc_ram);
~~~~~~ ^
hw/ppc/ppc440_uc.c:93:50: error: no member named 'isarc_ram' in 'struct ppc4xx_l2sram_t'
&l2sram->isarc_ram);
~~~~~~ ^
hw/ppc/ppc440_uc.c:96:17: error: no member named 'dsarc' in 'struct ppc4xx_l2sram_t'
if (l2sram->dsarc != dsarc ||
~~~~~~ ^
hw/ppc/ppc440_uc.c:97:18: error: no member named 'dsacntl' in 'struct ppc4xx_l2sram_t'
(l2sram->dsacntl & 0x80000000) != (dsacntl & 0x80000000)) {
~~~~~~ ^
hw/ppc/ppc440_uc.c:98:21: error: no member named 'dsacntl' in 'struct ppc4xx_l2sram_t'
if (l2sram->dsacntl & 0x80000000) {
~~~~~~ ^
hw/ppc/ppc440_uc.c:100:52: error: no member named 'dsarc' in 'struct ppc4xx_l2sram_t'
if (!(isacntl & 0x80000000) || l2sram->dsarc != isarc) {
~~~~~~ ^
hw/ppc/ppc440_uc.c:103:54: error: no member named 'dsarc_ram' in 'struct ppc4xx_l2sram_t'
&l2sram->dsarc_ram);
~~~~~~ ^
hw/ppc/ppc440_uc.c:111:54: error: no member named 'dsarc_ram' in 'struct ppc4xx_l2sram_t'
&l2sram->dsarc_ram);
~~~~~~ ^
Remove that dead code.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/ppc/ppc440_uc.c | 40 ----------------------------------------
1 file changed, 40 deletions(-)
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index 4181c843a8..643a79e330 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -73,46 +73,6 @@ typedef struct ppc4xx_l2sram_t {
uint32_t isram0[11];
} ppc4xx_l2sram_t;
-#ifdef MAP_L2SRAM
-static void l2sram_update_mappings(ppc4xx_l2sram_t *l2sram,
- uint32_t isarc, uint32_t isacntl,
- uint32_t dsarc, uint32_t dsacntl)
-{
- if (l2sram->isarc != isarc ||
- (l2sram->isacntl & 0x80000000) != (isacntl & 0x80000000)) {
- if (l2sram->isacntl & 0x80000000) {
- /* Unmap previously assigned memory region */
- memory_region_del_subregion(get_system_memory(),
- &l2sram->isarc_ram);
- }
- if (isacntl & 0x80000000) {
- /* Map new instruction memory region */
- memory_region_add_subregion(get_system_memory(), isarc,
- &l2sram->isarc_ram);
- }
- }
- if (l2sram->dsarc != dsarc ||
- (l2sram->dsacntl & 0x80000000) != (dsacntl & 0x80000000)) {
- if (l2sram->dsacntl & 0x80000000) {
- /* Beware not to unmap the region we just mapped */
- if (!(isacntl & 0x80000000) || l2sram->dsarc != isarc) {
- /* Unmap previously assigned memory region */
- memory_region_del_subregion(get_system_memory(),
- &l2sram->dsarc_ram);
- }
- }
- if (dsacntl & 0x80000000) {
- /* Beware not to remap the region we just mapped */
- if (!(isacntl & 0x80000000) || dsarc != isarc) {
- /* Map new data memory region */
- memory_region_add_subregion(get_system_memory(), dsarc,
- &l2sram->dsarc_ram);
- }
- }
- }
-}
-#endif
-
static uint32_t dcr_read_l2sram(void *opaque, int dcrn)
{
ppc4xx_l2sram_t *l2sram = opaque;
--
2.41.0
On Wed, 11 Oct 2023, Philippe Mathieu-Daudé wrote:
> Apparently l2sram_update_mappings() bit-rotted over time,
> when defining MAP_L2SRAM we get:
>
> hw/ppc/ppc440_uc.c:83:17: error: no member named 'isarc' in 'struct ppc4xx_l2sram_t'
> if (l2sram->isarc != isarc ||
> ~~~~~~ ^
> hw/ppc/ppc440_uc.c:84:18: error: no member named 'isacntl' in 'struct ppc4xx_l2sram_t'
> (l2sram->isacntl & 0x80000000) != (isacntl & 0x80000000)) {
> ~~~~~~ ^
> hw/ppc/ppc440_uc.c:85:21: error: no member named 'isacntl' in 'struct ppc4xx_l2sram_t'
> if (l2sram->isacntl & 0x80000000) {
> ~~~~~~ ^
> hw/ppc/ppc440_uc.c:88:50: error: no member named 'isarc_ram' in 'struct ppc4xx_l2sram_t'
> &l2sram->isarc_ram);
> ~~~~~~ ^
> hw/ppc/ppc440_uc.c:93:50: error: no member named 'isarc_ram' in 'struct ppc4xx_l2sram_t'
> &l2sram->isarc_ram);
> ~~~~~~ ^
> hw/ppc/ppc440_uc.c:96:17: error: no member named 'dsarc' in 'struct ppc4xx_l2sram_t'
> if (l2sram->dsarc != dsarc ||
> ~~~~~~ ^
> hw/ppc/ppc440_uc.c:97:18: error: no member named 'dsacntl' in 'struct ppc4xx_l2sram_t'
> (l2sram->dsacntl & 0x80000000) != (dsacntl & 0x80000000)) {
> ~~~~~~ ^
> hw/ppc/ppc440_uc.c:98:21: error: no member named 'dsacntl' in 'struct ppc4xx_l2sram_t'
> if (l2sram->dsacntl & 0x80000000) {
> ~~~~~~ ^
> hw/ppc/ppc440_uc.c:100:52: error: no member named 'dsarc' in 'struct ppc4xx_l2sram_t'
> if (!(isacntl & 0x80000000) || l2sram->dsarc != isarc) {
> ~~~~~~ ^
> hw/ppc/ppc440_uc.c:103:54: error: no member named 'dsarc_ram' in 'struct ppc4xx_l2sram_t'
> &l2sram->dsarc_ram);
> ~~~~~~ ^
> hw/ppc/ppc440_uc.c:111:54: error: no member named 'dsarc_ram' in 'struct ppc4xx_l2sram_t'
> &l2sram->dsarc_ram);
> ~~~~~~ ^
>
> Remove that dead code.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/ppc/ppc440_uc.c | 40 ----------------------------------------
> 1 file changed, 40 deletions(-)
>
> diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
> index 4181c843a8..643a79e330 100644
> --- a/hw/ppc/ppc440_uc.c
> +++ b/hw/ppc/ppc440_uc.c
> @@ -73,46 +73,6 @@ typedef struct ppc4xx_l2sram_t {
> uint32_t isram0[11];
> } ppc4xx_l2sram_t;
>
> -#ifdef MAP_L2SRAM
> -static void l2sram_update_mappings(ppc4xx_l2sram_t *l2sram,
> - uint32_t isarc, uint32_t isacntl,
> - uint32_t dsarc, uint32_t dsacntl)
If you remove this then nobody will remember this could be modelled or may
be fixed so maybe leave it as a reminder for now.
Regards,
BALATON Zoltan
> -{
> - if (l2sram->isarc != isarc ||
> - (l2sram->isacntl & 0x80000000) != (isacntl & 0x80000000)) {
> - if (l2sram->isacntl & 0x80000000) {
> - /* Unmap previously assigned memory region */
> - memory_region_del_subregion(get_system_memory(),
> - &l2sram->isarc_ram);
> - }
> - if (isacntl & 0x80000000) {
> - /* Map new instruction memory region */
> - memory_region_add_subregion(get_system_memory(), isarc,
> - &l2sram->isarc_ram);
> - }
> - }
> - if (l2sram->dsarc != dsarc ||
> - (l2sram->dsacntl & 0x80000000) != (dsacntl & 0x80000000)) {
> - if (l2sram->dsacntl & 0x80000000) {
> - /* Beware not to unmap the region we just mapped */
> - if (!(isacntl & 0x80000000) || l2sram->dsarc != isarc) {
> - /* Unmap previously assigned memory region */
> - memory_region_del_subregion(get_system_memory(),
> - &l2sram->dsarc_ram);
> - }
> - }
> - if (dsacntl & 0x80000000) {
> - /* Beware not to remap the region we just mapped */
> - if (!(isacntl & 0x80000000) || dsarc != isarc) {
> - /* Map new data memory region */
> - memory_region_add_subregion(get_system_memory(), dsarc,
> - &l2sram->dsarc_ram);
> - }
> - }
> - }
> -}
> -#endif
> -
> static uint32_t dcr_read_l2sram(void *opaque, int dcrn)
> {
> ppc4xx_l2sram_t *l2sram = opaque;
>
Hi Zoltan,
On 11/10/23 15:31, BALATON Zoltan wrote:
> On Wed, 11 Oct 2023, Philippe Mathieu-Daudé wrote:
>> Apparently l2sram_update_mappings() bit-rotted over time,
>> when defining MAP_L2SRAM we get:
>>
>> hw/ppc/ppc440_uc.c:83:17: error: no member named 'isarc' in 'struct
>> ppc4xx_l2sram_t'
>> if (l2sram->isarc != isarc ||
>> ~~~~~~ ^
>> hw/ppc/ppc440_uc.c:84:18: error: no member named 'isacntl' in 'struct
>> ppc4xx_l2sram_t'
>> (l2sram->isacntl & 0x80000000) != (isacntl & 0x80000000)) {
>> ~~~~~~ ^
>> hw/ppc/ppc440_uc.c:85:21: error: no member named 'isacntl' in 'struct
>> ppc4xx_l2sram_t'
>> if (l2sram->isacntl & 0x80000000) {
>> ~~~~~~ ^
>> hw/ppc/ppc440_uc.c:88:50: error: no member named 'isarc_ram' in
>> 'struct ppc4xx_l2sram_t'
>> &l2sram->isarc_ram);
>> ~~~~~~ ^
>> hw/ppc/ppc440_uc.c:93:50: error: no member named 'isarc_ram' in
>> 'struct ppc4xx_l2sram_t'
>> &l2sram->isarc_ram);
>> ~~~~~~ ^
>> hw/ppc/ppc440_uc.c:96:17: error: no member named 'dsarc' in 'struct
>> ppc4xx_l2sram_t'
>> if (l2sram->dsarc != dsarc ||
>> ~~~~~~ ^
>> hw/ppc/ppc440_uc.c:97:18: error: no member named 'dsacntl' in 'struct
>> ppc4xx_l2sram_t'
>> (l2sram->dsacntl & 0x80000000) != (dsacntl & 0x80000000)) {
>> ~~~~~~ ^
>> hw/ppc/ppc440_uc.c:98:21: error: no member named 'dsacntl' in 'struct
>> ppc4xx_l2sram_t'
>> if (l2sram->dsacntl & 0x80000000) {
>> ~~~~~~ ^
>> hw/ppc/ppc440_uc.c:100:52: error: no member named 'dsarc' in 'struct
>> ppc4xx_l2sram_t'
>> if (!(isacntl & 0x80000000) || l2sram->dsarc != isarc) {
>> ~~~~~~ ^
>> hw/ppc/ppc440_uc.c:103:54: error: no member named 'dsarc_ram' in
>> 'struct ppc4xx_l2sram_t'
>> &l2sram->dsarc_ram);
>> ~~~~~~ ^
>> hw/ppc/ppc440_uc.c:111:54: error: no member named 'dsarc_ram' in
>> 'struct ppc4xx_l2sram_t'
>> &l2sram->dsarc_ram);
>> ~~~~~~ ^
>>
>> Remove that dead code.
I missed to remove:
-- >8 --
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index 3a66b0c7f7..1312aa2080 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -154,7 +154,6 @@ static void dcr_write_l2sram(void *opaque, int dcrn,
uint32_t val)
/*l2sram->isram1[dcrn - DCR_L2CACHE_BASE] = val;*/
break;
}
- /*l2sram_update_mappings(l2sram, isarc, isacntl, dsarc, dsacntl);*/
}
static void l2sram_reset(void *opaque)
@@ -164,7 +163,6 @@ static void l2sram_reset(void *opaque)
memset(l2sram->l2cache, 0, sizeof(l2sram->l2cache));
l2sram->l2cache[DCR_L2CACHE_STAT - DCR_L2CACHE_BASE] = 0x80000000;
memset(l2sram->isram0, 0, sizeof(l2sram->isram0));
- /*l2sram_update_mappings(l2sram, isarc, isacntl, dsarc, dsacntl);*/
}
---
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/ppc/ppc440_uc.c | 40 ----------------------------------------
>> 1 file changed, 40 deletions(-)
>>
>> diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
>> index 4181c843a8..643a79e330 100644
>> --- a/hw/ppc/ppc440_uc.c
>> +++ b/hw/ppc/ppc440_uc.c
>> @@ -73,46 +73,6 @@ typedef struct ppc4xx_l2sram_t {
>> uint32_t isram0[11];
>> } ppc4xx_l2sram_t;
>>
>> -#ifdef MAP_L2SRAM
>> -static void l2sram_update_mappings(ppc4xx_l2sram_t *l2sram,
>> - uint32_t isarc, uint32_t isacntl,
>> - uint32_t dsarc, uint32_t dsacntl)
>
> If you remove this then nobody will remember this could be modelled or
> may be fixed so maybe leave it as a reminder for now.
We can keep this code if someone fix it and enable it (convert the
definition to a static boolean). Some APIs are being modified, we can
not test modifications in such dead code. Even converting to a comment
doesn't seem useful.
Maybe you can add a comment "If you are interested in ..., see
l2sram_update_mappings() draft implementation in
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04261.html"?
> Regards,
> BALATON Zoltan
On Wed, 11 Oct 2023, Philippe Mathieu-Daudé wrote:
> Hi Zoltan,
>
> On 11/10/23 15:31, BALATON Zoltan wrote:
>> On Wed, 11 Oct 2023, Philippe Mathieu-Daudé wrote:
>>> Apparently l2sram_update_mappings() bit-rotted over time,
>>> when defining MAP_L2SRAM we get:
>>>
>>> hw/ppc/ppc440_uc.c:83:17: error: no member named 'isarc' in 'struct
>>> ppc4xx_l2sram_t'
>>> if (l2sram->isarc != isarc ||
>>> ~~~~~~ ^
>>> hw/ppc/ppc440_uc.c:84:18: error: no member named 'isacntl' in 'struct
>>> ppc4xx_l2sram_t'
>>> (l2sram->isacntl & 0x80000000) != (isacntl & 0x80000000)) {
>>> ~~~~~~ ^
>>> hw/ppc/ppc440_uc.c:85:21: error: no member named 'isacntl' in 'struct
>>> ppc4xx_l2sram_t'
>>> if (l2sram->isacntl & 0x80000000) {
>>> ~~~~~~ ^
>>> hw/ppc/ppc440_uc.c:88:50: error: no member named 'isarc_ram' in 'struct
>>> ppc4xx_l2sram_t'
>>> &l2sram->isarc_ram);
>>> ~~~~~~ ^
>>> hw/ppc/ppc440_uc.c:93:50: error: no member named 'isarc_ram' in 'struct
>>> ppc4xx_l2sram_t'
>>> &l2sram->isarc_ram);
>>> ~~~~~~ ^
>>> hw/ppc/ppc440_uc.c:96:17: error: no member named 'dsarc' in 'struct
>>> ppc4xx_l2sram_t'
>>> if (l2sram->dsarc != dsarc ||
>>> ~~~~~~ ^
>>> hw/ppc/ppc440_uc.c:97:18: error: no member named 'dsacntl' in 'struct
>>> ppc4xx_l2sram_t'
>>> (l2sram->dsacntl & 0x80000000) != (dsacntl & 0x80000000)) {
>>> ~~~~~~ ^
>>> hw/ppc/ppc440_uc.c:98:21: error: no member named 'dsacntl' in 'struct
>>> ppc4xx_l2sram_t'
>>> if (l2sram->dsacntl & 0x80000000) {
>>> ~~~~~~ ^
>>> hw/ppc/ppc440_uc.c:100:52: error: no member named 'dsarc' in 'struct
>>> ppc4xx_l2sram_t'
>>> if (!(isacntl & 0x80000000) || l2sram->dsarc != isarc) {
>>> ~~~~~~ ^
>>> hw/ppc/ppc440_uc.c:103:54: error: no member named 'dsarc_ram' in 'struct
>>> ppc4xx_l2sram_t'
>>> &l2sram->dsarc_ram);
>>> ~~~~~~ ^
>>> hw/ppc/ppc440_uc.c:111:54: error: no member named 'dsarc_ram' in 'struct
>>> ppc4xx_l2sram_t'
>>> &l2sram->dsarc_ram);
>>> ~~~~~~ ^
>>>
>>> Remove that dead code.
>
> I missed to remove:
>
> -- >8 --
> diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
> index 3a66b0c7f7..1312aa2080 100644
> --- a/hw/ppc/ppc440_uc.c
> +++ b/hw/ppc/ppc440_uc.c
> @@ -154,7 +154,6 @@ static void dcr_write_l2sram(void *opaque, int dcrn,
> uint32_t val)
> /*l2sram->isram1[dcrn - DCR_L2CACHE_BASE] = val;*/
> break;
> }
> - /*l2sram_update_mappings(l2sram, isarc, isacntl, dsarc, dsacntl);*/
> }
Well, all of this func does nothing and just here so accessing these DCRs
won't crash but it already has a FIXME comment at the beginning noting
that, so in that case it's probably OK to remove the unfinished func as we
still have a reminder here. So then:
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Regards,
BALATON Zoltan
> static void l2sram_reset(void *opaque)
> @@ -164,7 +163,6 @@ static void l2sram_reset(void *opaque)
> memset(l2sram->l2cache, 0, sizeof(l2sram->l2cache));
> l2sram->l2cache[DCR_L2CACHE_STAT - DCR_L2CACHE_BASE] = 0x80000000;
> memset(l2sram->isram0, 0, sizeof(l2sram->isram0));
> - /*l2sram_update_mappings(l2sram, isarc, isacntl, dsarc, dsacntl);*/
> }
> ---
>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> hw/ppc/ppc440_uc.c | 40 ----------------------------------------
>>> 1 file changed, 40 deletions(-)
>>>
>>> diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
>>> index 4181c843a8..643a79e330 100644
>>> --- a/hw/ppc/ppc440_uc.c
>>> +++ b/hw/ppc/ppc440_uc.c
>>> @@ -73,46 +73,6 @@ typedef struct ppc4xx_l2sram_t {
>>> uint32_t isram0[11];
>>> } ppc4xx_l2sram_t;
>>>
>>> -#ifdef MAP_L2SRAM
>>> -static void l2sram_update_mappings(ppc4xx_l2sram_t *l2sram,
>>> - uint32_t isarc, uint32_t isacntl,
>>> - uint32_t dsarc, uint32_t dsacntl)
>>
>> If you remove this then nobody will remember this could be modelled or may
>> be fixed so maybe leave it as a reminder for now.
>
> We can keep this code if someone fix it and enable it (convert the
> definition to a static boolean). Some APIs are being modified, we can
> not test modifications in such dead code. Even converting to a comment
> doesn't seem useful.
>
> Maybe you can add a comment "If you are interested in ..., see
> l2sram_update_mappings() draft implementation in
> https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04261.html"?
>
>> Regards,
>> BALATON Zoltan
>
>
>
© 2016 - 2026 Red Hat, Inc.