Re: [PATCH v2 08/11] hw/intc/sh_intc: Use existing macro instead of local one

[Philippe Mathieu-Daudé]

On 10/27/21 15:46, BALATON Zoltan wrote:
> The INTC_A7 local macro does the same as the A7ADDR from
> include/sh/sh.h so use the latter and drop the local macro definiion.

Typo "definition".

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/intc/sh_intc.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/intc/sh_intc.c b/hw/intc/sh_intc.c
> index c1058d97c0..0bd27aaf4f 100644
> --- a/hw/intc/sh_intc.c
> +++ b/hw/intc/sh_intc.c
> @@ -16,8 +16,6 @@
>  #include "hw/sh4/sh.h"
>  #include "trace.h"
>  
> -#define INTC_A7(x) ((x) & 0x1fffffff)
> -
>  void sh_intc_toggle_source(struct intc_source *source,
>                             int enable_adj, int assert_adj)
>  {
> @@ -112,12 +110,12 @@ int sh_intc_get_pending_vector(struct intc_desc *desc, 
> int imask)
>  static unsigned int sh_intc_mode(unsigned long address,
>                                   unsigned long set_reg, unsigned long 
> clr_reg)
>  {
> -    if ((address != INTC_A7(set_reg)) &&
> -        (address != INTC_A7(clr_reg)))
> +    if ((address != A7ADDR(set_reg)) &&
> +        (address != A7ADDR(clr_reg)))
>          return INTC_MODE_NONE;
>  
>      if (set_reg && clr_reg) {
> -        if (address == INTC_A7(set_reg)) {
> +        if (address == A7ADDR(set_reg)) {
>              return INTC_MODE_DUAL_SET;
>          } else {
>              return INTC_MODE_DUAL_CLR;
> @@ -297,11 +295,11 @@ static unsigned int sh_intc_register(MemoryRegion 
> *sysmem,
>  
>  #define SH_INTC_IOMEM_FORMAT "interrupt-controller-%s-%s-%s"
>      snprintf(name, sizeof(name), SH_INTC_IOMEM_FORMAT, type, action, "p4");
> -    memory_region_init_alias(iomem_p4, NULL, name, iomem, INTC_A7(address), 
> 4);
> +    memory_region_init_alias(iomem_p4, NULL, name, iomem, A7ADDR(address), 
> 4);
>      memory_region_add_subregion(sysmem, P4ADDR(address), iomem_p4);
>  
>      snprintf(name, sizeof(name), SH_INTC_IOMEM_FORMAT, type, action, "a7");
> -    memory_region_init_alias(iomem_a7, NULL, name, iomem, INTC_A7(address), 
> 4);
> +    memory_region_init_alias(iomem_a7, NULL, name, iomem, A7ADDR(address), 
> 4);

I wonder why the address is masked out... It looks there is a mismatch
in the memory region mapping. Anyway this predates this cleanup, so:

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>