Re: [RFC PATCH v6 04/11] hw/ssi: imx_spi: Reduce 'change_mask' variable scope

[Juan Quintela <quintela@redhat.com>]

Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

I think this one is wrong.


> ---
>  hw/ssi/imx_spi.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
> index 35ab33c0511..bcc535f2893 100644
> --- a/hw/ssi/imx_spi.c
> +++ b/hw/ssi/imx_spi.c
> @@ -303,7 +303,6 @@ static void imx_spi_write(void *opaque, hwaddr offset, uint64_t value,
>  {
>      IMXSPIState *s = opaque;
>      uint32_t index = offset >> 2;
> -    uint32_t change_mask;
>  
>      if (index >=  ECSPI_MAX) {
>          qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad register at offset 0x%"
> @@ -313,7 +312,6 @@ static void imx_spi_write(void *opaque, hwaddr offset, uint64_t value,
>  
>      trace_imx_spi_write(index, imx_spi_reg_name(index), value);
>  
> -    change_mask = s->regs[index] ^ value;
>  
>      switch (index) {
>      case ECSPI_RXDATA:
> @@ -357,6 +355,7 @@ static void imx_spi_write(void *opaque, hwaddr offset, uint64_t value,
>          }
>  
>          if (imx_spi_channel_is_master(s)) {
> +            uint32_t change_mask = s->regs[index] ^ value;
>              int i;
>  
>              /* We are in master mode */

The code does:

    change_mask = s->regs[index] ^ value;

    switch (index) {

    ...

    case ECSPI_CONREG:
        s->regs[ECSPI_CONREG] = value;  <<---- here

        if (!imx_spi_is_enabled(s)) {
            /* device is disabled, so this is a reset */
            imx_spi_reset(DEVICE(s));
            return;
        }

        if (imx_spi_channel_is_master(s)) {
            int i;
       >>>>>  You are setting change_mask here.

At this point, s->regs[index] has a new value in "here".

Later, Juan.