Juan Quintela <quint...@redhat.com> wrote:
> Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>
> I think this one is wrong.
Wrong is a strong word. I mean that it changes behaviour and the commit
message don't talk about changing behaviour.
Later, Juan.
>
>
>> ---
>> 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.
