Re: [PATCH 2/4] aspeed: Clean up local variable shadowing

2023-09-23 Thread Cédric Le Goater

On 9/23/23 08:34, Markus Armbruster wrote:

Cédric Le Goater  writes:


Remove superfluous local 'irq' variables and use the one define at the
top of the routine. This fixes warnings in aspeed_soc_ast2600_realize()
such as :

   ../hw/arm/aspeed_ast2600.c: In function ‘aspeed_soc_ast2600_realize’:
   ../hw/arm/aspeed_ast2600.c:420:18: warning: declaration of ‘irq’ shadows a 
previous local [-Wshadow=compatible-local]
 420 | qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
 |  ^~~
   ../hw/arm/aspeed_ast2600.c:312:14: note: shadowed declaration is here
 312 | qemu_irq irq;
 |  ^~~

Signed-off-by: Cédric Le Goater 
---
  hw/arm/aspeed_ast2600.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index a8b3a8065a11..e122e1c32d42 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -388,7 +388,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
  aspeed_mmio_map(s, SYS_BUS_DEVICE(>timerctrl), 0,
  sc->memmap[ASPEED_DEV_TIMER1]);
  for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
-qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
+irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
  sysbus_connect_irq(SYS_BUS_DEVICE(>timerctrl), i, irq);
  }
  
@@ -413,8 +413,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)

  }
  aspeed_mmio_map(s, SYS_BUS_DEVICE(>i2c), 0, 
sc->memmap[ASPEED_DEV_I2C]);
  for (i = 0; i < ASPEED_I2C_GET_CLASS(>i2c)->num_busses; i++) {
-qemu_irq irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
-sc->irqmap[ASPEED_DEV_I2C] + i);
+irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
+   sc->irqmap[ASPEED_DEV_I2C] + i);
  /* The AST2600 I2C controller has one IRQ per bus. */
  sysbus_connect_irq(SYS_BUS_DEVICE(>i2c.busses[i]), 0, irq);
  }
@@ -611,8 +611,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
  }
  aspeed_mmio_map(s, SYS_BUS_DEVICE(>i3c), 0, 
sc->memmap[ASPEED_DEV_I3C]);
  for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
-qemu_irq irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
-sc->irqmap[ASPEED_DEV_I3C] + i);
+irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
+   sc->irqmap[ASPEED_DEV_I3C] + i);
  /* The AST2600 I3C controller has one IRQ per bus. */
  sysbus_connect_irq(SYS_BUS_DEVICE(>i3c.devices[i]), 0, irq);
  }


Clashes with Philippe's

 [PATCH v2 10/22] hw/arm/aspeed: Clean up local variable shadowing
 Message-ID: <20230904161235.84651-11-phi...@linaro.org>

The difference is a matter of taste: one @irq in function scope vs. four
in nested scopes.  I'd prefer the former, i.e. this patch, but
maintainers' preference matter more than mine.  If you want me to merge
together with other shadowing patches, tell me your preference, if any.


You are the one gathering patches. Please choose. I don't mind.

Thanks,

C.




Re: [PATCH 2/4] aspeed: Clean up local variable shadowing

2023-09-23 Thread Markus Armbruster
Cédric Le Goater  writes:

> Remove superfluous local 'irq' variables and use the one define at the
> top of the routine. This fixes warnings in aspeed_soc_ast2600_realize()
> such as :
>
>   ../hw/arm/aspeed_ast2600.c: In function ‘aspeed_soc_ast2600_realize’:
>   ../hw/arm/aspeed_ast2600.c:420:18: warning: declaration of ‘irq’ shadows a 
> previous local [-Wshadow=compatible-local]
> 420 | qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
> |  ^~~
>   ../hw/arm/aspeed_ast2600.c:312:14: note: shadowed declaration is here
> 312 | qemu_irq irq;
> |  ^~~
>
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/arm/aspeed_ast2600.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index a8b3a8065a11..e122e1c32d42 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -388,7 +388,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
> Error **errp)
>  aspeed_mmio_map(s, SYS_BUS_DEVICE(>timerctrl), 0,
>  sc->memmap[ASPEED_DEV_TIMER1]);
>  for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
> -qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
> +irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
>  sysbus_connect_irq(SYS_BUS_DEVICE(>timerctrl), i, irq);
>  }
>  
> @@ -413,8 +413,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
> Error **errp)
>  }
>  aspeed_mmio_map(s, SYS_BUS_DEVICE(>i2c), 0, 
> sc->memmap[ASPEED_DEV_I2C]);
>  for (i = 0; i < ASPEED_I2C_GET_CLASS(>i2c)->num_busses; i++) {
> -qemu_irq irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
> -sc->irqmap[ASPEED_DEV_I2C] + i);
> +irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
> +   sc->irqmap[ASPEED_DEV_I2C] + i);
>  /* The AST2600 I2C controller has one IRQ per bus. */
>  sysbus_connect_irq(SYS_BUS_DEVICE(>i2c.busses[i]), 0, irq);
>  }
> @@ -611,8 +611,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
> Error **errp)
>  }
>  aspeed_mmio_map(s, SYS_BUS_DEVICE(>i3c), 0, 
> sc->memmap[ASPEED_DEV_I3C]);
>  for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
> -qemu_irq irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
> -sc->irqmap[ASPEED_DEV_I3C] + i);
> +irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
> +   sc->irqmap[ASPEED_DEV_I3C] + i);
>  /* The AST2600 I3C controller has one IRQ per bus. */
>  sysbus_connect_irq(SYS_BUS_DEVICE(>i3c.devices[i]), 0, irq);
>  }

Clashes with Philippe's

[PATCH v2 10/22] hw/arm/aspeed: Clean up local variable shadowing
Message-ID: <20230904161235.84651-11-phi...@linaro.org>

The difference is a matter of taste: one @irq in function scope vs. four
in nested scopes.  I'd prefer the former, i.e. this patch, but
maintainers' preference matter more than mine.  If you want me to merge
together with other shadowing patches, tell me your preference, if any.




Re: [PATCH 2/4] aspeed: Clean up local variable shadowing

2023-09-22 Thread Philippe Mathieu-Daudé

On 22/9/23 17:59, Cédric Le Goater wrote:

Remove superfluous local 'irq' variables and use the one define at the
top of the routine. This fixes warnings in aspeed_soc_ast2600_realize()
such as :

   ../hw/arm/aspeed_ast2600.c: In function ‘aspeed_soc_ast2600_realize’:
   ../hw/arm/aspeed_ast2600.c:420:18: warning: declaration of ‘irq’ shadows a 
previous local [-Wshadow=compatible-local]
 420 | qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
 |  ^~~
   ../hw/arm/aspeed_ast2600.c:312:14: note: shadowed declaration is here
 312 | qemu_irq irq;
 |  ^~~

Signed-off-by: Cédric Le Goater 
---
  hw/arm/aspeed_ast2600.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 




[PATCH 2/4] aspeed: Clean up local variable shadowing

2023-09-22 Thread Cédric Le Goater
Remove superfluous local 'irq' variables and use the one define at the
top of the routine. This fixes warnings in aspeed_soc_ast2600_realize()
such as :

  ../hw/arm/aspeed_ast2600.c: In function ‘aspeed_soc_ast2600_realize’:
  ../hw/arm/aspeed_ast2600.c:420:18: warning: declaration of ‘irq’ shadows a 
previous local [-Wshadow=compatible-local]
420 | qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
|  ^~~
  ../hw/arm/aspeed_ast2600.c:312:14: note: shadowed declaration is here
312 | qemu_irq irq;
|  ^~~

Signed-off-by: Cédric Le Goater 
---
 hw/arm/aspeed_ast2600.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index a8b3a8065a11..e122e1c32d42 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -388,7 +388,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 aspeed_mmio_map(s, SYS_BUS_DEVICE(>timerctrl), 0,
 sc->memmap[ASPEED_DEV_TIMER1]);
 for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
-qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
+irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
 sysbus_connect_irq(SYS_BUS_DEVICE(>timerctrl), i, irq);
 }
 
@@ -413,8 +413,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 }
 aspeed_mmio_map(s, SYS_BUS_DEVICE(>i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
 for (i = 0; i < ASPEED_I2C_GET_CLASS(>i2c)->num_busses; i++) {
-qemu_irq irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
-sc->irqmap[ASPEED_DEV_I2C] + i);
+irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
+   sc->irqmap[ASPEED_DEV_I2C] + i);
 /* The AST2600 I2C controller has one IRQ per bus. */
 sysbus_connect_irq(SYS_BUS_DEVICE(>i2c.busses[i]), 0, irq);
 }
@@ -611,8 +611,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 }
 aspeed_mmio_map(s, SYS_BUS_DEVICE(>i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
 for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
-qemu_irq irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
-sc->irqmap[ASPEED_DEV_I3C] + i);
+irq = qdev_get_gpio_in(DEVICE(>a7mpcore),
+   sc->irqmap[ASPEED_DEV_I3C] + i);
 /* The AST2600 I3C controller has one IRQ per bus. */
 sysbus_connect_irq(SYS_BUS_DEVICE(>i3c.devices[i]), 0, irq);
 }
-- 
2.41.0