diff mbox

[v3,i.MX] fix CS handling during SPI access.

Message ID 20170103203502.25907-1-jcd@tribudubois.net
State New
Headers show

Commit Message

Jean-Christophe DUBOIS Jan. 3, 2017, 8:35 p.m. UTC
The i.MX SPI device was not de-asserting the CS line at the end of
memory access.

This triggered a SIGSEGV in Qemu when the sabrelite emulator was acessing
a SPI flash memory.

Whit this path the CS signal is correctly asserted and deasserted arround
memory access.

Assertion level is now based on SPI device configuration.

This was tested by:
* booting linux on Sabrelite Qemu emulator.
* booting xvisor on Sabrelite Qemu emultor.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---

Changes since v1:
* Fix coding style issue.

Changes since v2:
* get SS line polarity from config reg.

 hw/ssi/imx_spi.c         | 42 ++++++++++++++++++++++++++++++------------
 include/hw/ssi/imx_spi.h |  2 ++
 2 files changed, 32 insertions(+), 12 deletions(-)

Comments

mar.krzeminski Jan. 4, 2017, 8:56 p.m. UTC | #1
W dniu 03.01.2017 o 21:35, Jean-Christophe Dubois pisze:
> The i.MX SPI device was not de-asserting the CS line at the end of
> memory access.
>
> This triggered a SIGSEGV in Qemu when the sabrelite emulator was acessing
> a SPI flash memory.
>
> Whit this path the CS signal is correctly asserted and deasserted arround
> memory access.
>
> Assertion level is now based on SPI device configuration.
>
> This was tested by:
> * booting linux on Sabrelite Qemu emulator.
> * booting xvisor on Sabrelite Qemu emultor.
>
> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
> ---
>
> Changes since v1:
> * Fix coding style issue.
>
> Changes since v2:
> * get SS line polarity from config reg.
>
>   hw/ssi/imx_spi.c         | 42 ++++++++++++++++++++++++++++++------------
>   include/hw/ssi/imx_spi.h |  2 ++
>   2 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
> index e4e395f..3cbf279 100644
> --- a/hw/ssi/imx_spi.c
> +++ b/hw/ssi/imx_spi.c
> @@ -141,6 +141,18 @@ static bool imx_spi_channel_is_master(IMXSPIState *s)
>       return (mode & (1 << imx_spi_selected_channel(s))) ? true : false;
>   }
>   
> +static uint8_t imx_spi_channel_pol(IMXSPIState *s, uint8_t channel)
> +{
channel variable is unused. Should be instead of 
imx_spi_selected_channel() call?
> +    uint8_t pol = EXTRACT(s->regs[ECSPI_CONFIGREG], ECSPI_CONFIGREG_SS_POL);
> +
> +    return pol & (1 << imx_spi_selected_channel(s)) ? 1 : 0;
> +}
> +
> +static uint8_t imx_spi_current_channel_pol(IMXSPIState *s)
> +{
> +    return imx_spi_channel_pol(s, imx_spi_selected_channel(s));
> +}
> +
>   static bool imx_spi_is_multiple_master_burst(IMXSPIState *s)
>   {
>       uint8_t wave = EXTRACT(s->regs[ECSPI_CONFIGREG], ECSPI_CONFIGREG_SS_CTL);
> @@ -152,13 +164,16 @@ static bool imx_spi_is_multiple_master_burst(IMXSPIState *s)
>   
>   static void imx_spi_flush_txfifo(IMXSPIState *s)
>   {
> -    uint32_t tx;
> -    uint32_t rx;
> -
>       DPRINTF("Begin: TX Fifo Size = %d, RX Fifo Size = %d\n",
>               fifo32_num_used(&s->tx_fifo), fifo32_num_used(&s->rx_fifo));
>   
> +    /* Activate the requested CS line */
> +    qemu_set_irq(s->cs_lines[imx_spi_selected_channel(s)],
> +                 imx_spi_current_channel_pol(s));
> +
>       while (!fifo32_is_empty(&s->tx_fifo)) {
> +        uint32_t tx;
> +        uint32_t rx = 0;
>           int tx_burst = 0;
>           int index = 0;
>   
> @@ -178,8 +193,6 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
>   
>           tx_burst = MIN(s->burst_length, 32);
>   
> -        rx = 0;
> -
>           while (tx_burst) {
>               uint8_t byte = tx & 0xff;
>   
> @@ -221,6 +234,12 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
>           s->regs[ECSPI_STATREG] |= ECSPI_STATREG_TC;
>       }
>   
> +    /* Deselect all SS lines if transfert if completed */
> +    if (s->regs[ECSPI_STATREG] & ECSPI_STATREG_TC) {
> +        qemu_set_irq(s->cs_lines[imx_spi_selected_channel(s)],
> +                     !imx_spi_current_channel_pol(s));
> +    }
> +
>       /* TODO: We should also use TDR and RDR bits */
>   
>       DPRINTF("End: TX Fifo Size = %d, RX Fifo Size = %d\n",
> @@ -230,6 +249,7 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
>   static void imx_spi_reset(DeviceState *dev)
>   {
>       IMXSPIState *s = IMX_SPI(dev);
> +    uint32_t i;
>   
>       DPRINTF("\n");
>   
> @@ -243,6 +263,11 @@ static void imx_spi_reset(DeviceState *dev)
>       imx_spi_update_irq(s);
>   
>       s->burst_length = 0;
> +
> +    /* Disable all CS lines */
> +    for (i = 0; i < 4; i++) {
> +        qemu_set_irq(s->cs_lines[i], !imx_spi_channel_pol(s, i));
> +    }
Please make sure that in HW ECSPI_CONFIGREG_SS_POL bits are 0's after 
reset/power up (defaults).

Otherwise Acked-by: Marcin Krzemiński <mar.krzeminski@gmail.com>

Thanks,
Marcin
>   }
>   
>   static uint64_t imx_spi_read(void *opaque, hwaddr offset, unsigned size)
> @@ -359,15 +384,8 @@ static void imx_spi_write(void *opaque, hwaddr offset, uint64_t value,
>           }
>   
>           if (imx_spi_channel_is_master(s)) {
> -            int i;
> -
>               /* We are in master mode */
>   
> -            for (i = 0; i < 4; i++) {
> -                qemu_set_irq(s->cs_lines[i],
> -                             i == imx_spi_selected_channel(s) ? 0 : 1);
> -            }
> -
>               if ((value & change_mask & ECSPI_CONREG_SMC) &&
>                   !fifo32_is_empty(&s->tx_fifo)) {
>                   /* SMC bit is set and TX FIFO has some slots filled in */
> diff --git a/include/hw/ssi/imx_spi.h b/include/hw/ssi/imx_spi.h
> index 7103953..b9b9819 100644
> --- a/include/hw/ssi/imx_spi.h
> +++ b/include/hw/ssi/imx_spi.h
> @@ -46,6 +46,8 @@
>   /* ECSPI_CONFIGREG */
>   #define ECSPI_CONFIGREG_SS_CTL_SHIFT 8
>   #define ECSPI_CONFIGREG_SS_CTL_LENGTH 4
> +#define ECSPI_CONFIGREG_SS_POL_SHIFT 12
> +#define ECSPI_CONFIGREG_SS_POL_LENGTH 4
>   
>   /* ECSPI_INTREG */
>   #define ECSPI_INTREG_TEEN (1 << 0)
Jean-Christophe DUBOIS Jan. 4, 2017, 9:54 p.m. UTC | #2
Le 04/01/2017 à 21:56, mar.krzeminski a écrit :
>
>
> W dniu 03.01.2017 o 21:35, Jean-Christophe Dubois pisze:
>> The i.MX SPI device was not de-asserting the CS line at the end of
>> memory access.
>>
>> This triggered a SIGSEGV in Qemu when the sabrelite emulator was 
>> acessing
>> a SPI flash memory.
>>
>> Whit this path the CS signal is correctly asserted and deasserted 
>> arround
>> memory access.
>>
>> Assertion level is now based on SPI device configuration.
>>
>> This was tested by:
>> * booting linux on Sabrelite Qemu emulator.
>> * booting xvisor on Sabrelite Qemu emultor.
>>
>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>> ---
>>
>> Changes since v1:
>> * Fix coding style issue.
>>
>> Changes since v2:
>> * get SS line polarity from config reg.
>>
>>   hw/ssi/imx_spi.c         | 42 
>> ++++++++++++++++++++++++++++++------------
>>   include/hw/ssi/imx_spi.h |  2 ++
>>   2 files changed, 32 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
>> index e4e395f..3cbf279 100644
>> --- a/hw/ssi/imx_spi.c
>> +++ b/hw/ssi/imx_spi.c
>> @@ -141,6 +141,18 @@ static bool 
>> imx_spi_channel_is_master(IMXSPIState *s)
>>       return (mode & (1 << imx_spi_selected_channel(s))) ? true : false;
>>   }
>>   +static uint8_t imx_spi_channel_pol(IMXSPIState *s, uint8_t channel)
>> +{
> channel variable is unused. Should be instead of 
> imx_spi_selected_channel() call?

You are right. It should be used (instead of imx_spi_selected_channel(s) 
below). I'll fix it.

>> +    uint8_t pol = EXTRACT(s->regs[ECSPI_CONFIGREG], 
>> ECSPI_CONFIGREG_SS_POL);
>> +
>> +    return pol & (1 << ) ? 1 : 0;
>> +}
>> +
>> +static uint8_t imx_spi_current_channel_pol(IMXSPIState *s)
>> +{
>> +    return imx_spi_channel_pol(s, imx_spi_selected_channel(s));
>> +}
>> +
>>   static bool imx_spi_is_multiple_master_burst(IMXSPIState *s)
>>   {
>>       uint8_t wave = EXTRACT(s->regs[ECSPI_CONFIGREG], 
>> ECSPI_CONFIGREG_SS_CTL);
>> @@ -152,13 +164,16 @@ static bool 
>> imx_spi_is_multiple_master_burst(IMXSPIState *s)
>>     static void imx_spi_flush_txfifo(IMXSPIState *s)
>>   {
>> -    uint32_t tx;
>> -    uint32_t rx;
>> -
>>       DPRINTF("Begin: TX Fifo Size = %d, RX Fifo Size = %d\n",
>>               fifo32_num_used(&s->tx_fifo), 
>> fifo32_num_used(&s->rx_fifo));
>>   +    /* Activate the requested CS line */
>> +    qemu_set_irq(s->cs_lines[imx_spi_selected_channel(s)],
>> +                 imx_spi_current_channel_pol(s));
>> +
>>       while (!fifo32_is_empty(&s->tx_fifo)) {
>> +        uint32_t tx;
>> +        uint32_t rx = 0;
>>           int tx_burst = 0;
>>           int index = 0;
>>   @@ -178,8 +193,6 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
>>             tx_burst = MIN(s->burst_length, 32);
>>   -        rx = 0;
>> -
>>           while (tx_burst) {
>>               uint8_t byte = tx & 0xff;
>>   @@ -221,6 +234,12 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
>>           s->regs[ECSPI_STATREG] |= ECSPI_STATREG_TC;
>>       }
>>   +    /* Deselect all SS lines if transfert if completed */
>> +    if (s->regs[ECSPI_STATREG] & ECSPI_STATREG_TC) {
>> + qemu_set_irq(s->cs_lines[imx_spi_selected_channel(s)],
>> +                     !imx_spi_current_channel_pol(s));
>> +    }
>> +
>>       /* TODO: We should also use TDR and RDR bits */
>>         DPRINTF("End: TX Fifo Size = %d, RX Fifo Size = %d\n",
>> @@ -230,6 +249,7 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
>>   static void imx_spi_reset(DeviceState *dev)
>>   {
>>       IMXSPIState *s = IMX_SPI(dev);
>> +    uint32_t i;
>>         DPRINTF("\n");
>>   @@ -243,6 +263,11 @@ static void imx_spi_reset(DeviceState *dev)
>>       imx_spi_update_irq(s);
>>         s->burst_length = 0;
>> +
>> +    /* Disable all CS lines */
>> +    for (i = 0; i < 4; i++) {
>> +        qemu_set_irq(s->cs_lines[i], !imx_spi_channel_pol(s, i));
>> +    }
> Please make sure that in HW ECSPI_CONFIGREG_SS_POL bits are 0's after 
> reset/power up (defaults).

There is already a memset to 0 of all regs (including CONFIGREG) in the 
reset function.

>
> Otherwise Acked-by: Marcin Krzemiński <mar.krzeminski@gmail.com>

Thanks

JC

>
> Thanks,
> Marcin
>>   }
>>     static uint64_t imx_spi_read(void *opaque, hwaddr offset, 
>> unsigned size)
>> @@ -359,15 +384,8 @@ static void imx_spi_write(void *opaque, hwaddr 
>> offset, uint64_t value,
>>           }
>>             if (imx_spi_channel_is_master(s)) {
>> -            int i;
>> -
>>               /* We are in master mode */
>>   -            for (i = 0; i < 4; i++) {
>> -                qemu_set_irq(s->cs_lines[i],
>> -                             i == imx_spi_selected_channel(s) ? 0 : 1);
>> -            }
>> -
>>               if ((value & change_mask & ECSPI_CONREG_SMC) &&
>>                   !fifo32_is_empty(&s->tx_fifo)) {
>>                   /* SMC bit is set and TX FIFO has some slots filled 
>> in */
>> diff --git a/include/hw/ssi/imx_spi.h b/include/hw/ssi/imx_spi.h
>> index 7103953..b9b9819 100644
>> --- a/include/hw/ssi/imx_spi.h
>> +++ b/include/hw/ssi/imx_spi.h
>> @@ -46,6 +46,8 @@
>>   /* ECSPI_CONFIGREG */
>>   #define ECSPI_CONFIGREG_SS_CTL_SHIFT 8
>>   #define ECSPI_CONFIGREG_SS_CTL_LENGTH 4
>> +#define ECSPI_CONFIGREG_SS_POL_SHIFT 12
>> +#define ECSPI_CONFIGREG_SS_POL_LENGTH 4
>>     /* ECSPI_INTREG */
>>   #define ECSPI_INTREG_TEEN (1 << 0)
>
>
>
mar.krzeminski Jan. 6, 2017, 12:28 p.m. UTC | #3
W dniu 04.01.2017 o 22:54, Jean-Christophe DUBOIS pisze:
> Le 04/01/2017 à 21:56, mar.krzeminski a écrit :
>>
>>
>> W dniu 03.01.2017 o 21:35, Jean-Christophe Dubois pisze:
>>> The i.MX SPI device was not de-asserting the CS line at the end of
>>> memory access.
>>>
>>> This triggered a SIGSEGV in Qemu when the sabrelite emulator was 
>>> acessing
>>> a SPI flash memory.
>>>
>>> Whit this path the CS signal is correctly asserted and deasserted 
>>> arround
>>> memory access.
>>>
>>> Assertion level is now based on SPI device configuration.
>>>
>>> This was tested by:
>>> * booting linux on Sabrelite Qemu emulator.
>>> * booting xvisor on Sabrelite Qemu emultor.
>>>
>>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>>> ---
>>>
>>> Changes since v1:
>>> * Fix coding style issue.
>>>
>>> Changes since v2:
>>> * get SS line polarity from config reg.
>>>
>>>   hw/ssi/imx_spi.c         | 42 
>>> ++++++++++++++++++++++++++++++------------
>>>   include/hw/ssi/imx_spi.h |  2 ++
>>>   2 files changed, 32 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
>>> index e4e395f..3cbf279 100644
>>> --- a/hw/ssi/imx_spi.c
>>> +++ b/hw/ssi/imx_spi.c
>>> @@ -141,6 +141,18 @@ static bool 
>>> imx_spi_channel_is_master(IMXSPIState *s)
>>>       return (mode & (1 << imx_spi_selected_channel(s))) ? true : 
>>> false;
>>>   }
>>>   +static uint8_t imx_spi_channel_pol(IMXSPIState *s, uint8_t channel)
>>> +{
>> channel variable is unused. Should be instead of 
>> imx_spi_selected_channel() call?
>
> You are right. It should be used (instead of 
> imx_spi_selected_channel(s) below). I'll fix it.
>
>>> +    uint8_t pol = EXTRACT(s->regs[ECSPI_CONFIGREG], 
>>> ECSPI_CONFIGREG_SS_POL);
>>> +
>>> +    return pol & (1 << ) ? 1 : 0;
>>> +}
>>> +
>>> +static uint8_t imx_spi_current_channel_pol(IMXSPIState *s)
>>> +{
>>> +    return imx_spi_channel_pol(s, imx_spi_selected_channel(s));
>>> +}
>>> +
>>>   static bool imx_spi_is_multiple_master_burst(IMXSPIState *s)
>>>   {
>>>       uint8_t wave = EXTRACT(s->regs[ECSPI_CONFIGREG], 
>>> ECSPI_CONFIGREG_SS_CTL);
>>> @@ -152,13 +164,16 @@ static bool 
>>> imx_spi_is_multiple_master_burst(IMXSPIState *s)
>>>     static void imx_spi_flush_txfifo(IMXSPIState *s)
>>>   {
>>> -    uint32_t tx;
>>> -    uint32_t rx;
>>> -
>>>       DPRINTF("Begin: TX Fifo Size = %d, RX Fifo Size = %d\n",
>>>               fifo32_num_used(&s->tx_fifo), 
>>> fifo32_num_used(&s->rx_fifo));
>>>   +    /* Activate the requested CS line */
>>> +    qemu_set_irq(s->cs_lines[imx_spi_selected_channel(s)],
>>> +                 imx_spi_current_channel_pol(s));
>>> +
>>>       while (!fifo32_is_empty(&s->tx_fifo)) {
>>> +        uint32_t tx;
>>> +        uint32_t rx = 0;
>>>           int tx_burst = 0;
>>>           int index = 0;
>>>   @@ -178,8 +193,6 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
>>>             tx_burst = MIN(s->burst_length, 32);
>>>   -        rx = 0;
>>> -
>>>           while (tx_burst) {
>>>               uint8_t byte = tx & 0xff;
>>>   @@ -221,6 +234,12 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
>>>           s->regs[ECSPI_STATREG] |= ECSPI_STATREG_TC;
>>>       }
>>>   +    /* Deselect all SS lines if transfert if completed */
>>> +    if (s->regs[ECSPI_STATREG] & ECSPI_STATREG_TC) {
>>> + qemu_set_irq(s->cs_lines[imx_spi_selected_channel(s)],
>>> +                     !imx_spi_current_channel_pol(s));
>>> +    }
>>> +
>>>       /* TODO: We should also use TDR and RDR bits */
>>>         DPRINTF("End: TX Fifo Size = %d, RX Fifo Size = %d\n",
>>> @@ -230,6 +249,7 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
>>>   static void imx_spi_reset(DeviceState *dev)
>>>   {
>>>       IMXSPIState *s = IMX_SPI(dev);
>>> +    uint32_t i;
>>>         DPRINTF("\n");
>>>   @@ -243,6 +263,11 @@ static void imx_spi_reset(DeviceState *dev)
>>>       imx_spi_update_irq(s);
>>>         s->burst_length = 0;
>>> +
>>> +    /* Disable all CS lines */
>>> +    for (i = 0; i < 4; i++) {
>>> +        qemu_set_irq(s->cs_lines[i], !imx_spi_channel_pol(s, i));
>>> +    }
>> Please make sure that in HW ECSPI_CONFIGREG_SS_POL bits are 0's after 
>> reset/power up (defaults).
>
> There is already a memset to 0 of all regs (including CONFIGREG) in 
> the reset function.
I saw that memset, my question is rather real HW also have 0s after 
reset (could be important
in some cases).

Thanks,
Marcin
>
>>
>> Otherwise Acked-by: Marcin Krzemiński <mar.krzeminski@gmail.com>
>
> Thanks
>
> JC
>
>>
>> Thanks,
>> Marcin
>>>   }
>>>     static uint64_t imx_spi_read(void *opaque, hwaddr offset, 
>>> unsigned size)
>>> @@ -359,15 +384,8 @@ static void imx_spi_write(void *opaque, hwaddr 
>>> offset, uint64_t value,
>>>           }
>>>             if (imx_spi_channel_is_master(s)) {
>>> -            int i;
>>> -
>>>               /* We are in master mode */
>>>   -            for (i = 0; i < 4; i++) {
>>> -                qemu_set_irq(s->cs_lines[i],
>>> -                             i == imx_spi_selected_channel(s) ? 0 : 
>>> 1);
>>> -            }
>>> -
>>>               if ((value & change_mask & ECSPI_CONREG_SMC) &&
>>>                   !fifo32_is_empty(&s->tx_fifo)) {
>>>                   /* SMC bit is set and TX FIFO has some slots 
>>> filled in */
>>> diff --git a/include/hw/ssi/imx_spi.h b/include/hw/ssi/imx_spi.h
>>> index 7103953..b9b9819 100644
>>> --- a/include/hw/ssi/imx_spi.h
>>> +++ b/include/hw/ssi/imx_spi.h
>>> @@ -46,6 +46,8 @@
>>>   /* ECSPI_CONFIGREG */
>>>   #define ECSPI_CONFIGREG_SS_CTL_SHIFT 8
>>>   #define ECSPI_CONFIGREG_SS_CTL_LENGTH 4
>>> +#define ECSPI_CONFIGREG_SS_POL_SHIFT 12
>>> +#define ECSPI_CONFIGREG_SS_POL_LENGTH 4
>>>     /* ECSPI_INTREG */
>>>   #define ECSPI_INTREG_TEEN (1 << 0)
>>
>>
>>
>
>
Jean-Christophe DUBOIS Jan. 6, 2017, 6:18 p.m. UTC | #4
Le 06/01/2017 à 13:28, mar.krzeminski a écrit :
>>> Please make sure that in HW ECSPI_CONFIGREG_SS_POL bits are 0's 
>>> after reset/power up (defaults).
>>
>> There is already a memset to 0 of all regs (including CONFIGREG) in 
>> the reset function.
> I saw that memset, my question is rather real HW also have 0s after 
> reset (could be important
> in some cases).

Yes, all registers are set to 0 at reset except STATREG which is set to 
3 (according to reference manual).

>
> Thanks,
> Marcin
mar.krzeminski Jan. 6, 2017, 6:48 p.m. UTC | #5
W dniu 06.01.2017 o 19:18, Jean-Christophe DUBOIS pisze:
> Le 06/01/2017 à 13:28, mar.krzeminski a écrit :
>>>> Please make sure that in HW ECSPI_CONFIGREG_SS_POL bits are 0's 
>>>> after reset/power up (defaults).
>>>
>>> There is already a memset to 0 of all regs (including CONFIGREG) in 
>>> the reset function.
>> I saw that memset, my question is rather real HW also have 0s after 
>> reset (could be important
>> in some cases).
>
> Yes, all registers are set to 0 at reset except STATREG which is set 
> to 3 (according to reference manual).
>
Thanks!
>>
>> Thanks,
>> Marcin
>
>
diff mbox

Patch

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index e4e395f..3cbf279 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -141,6 +141,18 @@  static bool imx_spi_channel_is_master(IMXSPIState *s)
     return (mode & (1 << imx_spi_selected_channel(s))) ? true : false;
 }
 
+static uint8_t imx_spi_channel_pol(IMXSPIState *s, uint8_t channel)
+{
+    uint8_t pol = EXTRACT(s->regs[ECSPI_CONFIGREG], ECSPI_CONFIGREG_SS_POL);
+
+    return pol & (1 << imx_spi_selected_channel(s)) ? 1 : 0;
+}
+
+static uint8_t imx_spi_current_channel_pol(IMXSPIState *s)
+{
+    return imx_spi_channel_pol(s, imx_spi_selected_channel(s));
+}
+
 static bool imx_spi_is_multiple_master_burst(IMXSPIState *s)
 {
     uint8_t wave = EXTRACT(s->regs[ECSPI_CONFIGREG], ECSPI_CONFIGREG_SS_CTL);
@@ -152,13 +164,16 @@  static bool imx_spi_is_multiple_master_burst(IMXSPIState *s)
 
 static void imx_spi_flush_txfifo(IMXSPIState *s)
 {
-    uint32_t tx;
-    uint32_t rx;
-
     DPRINTF("Begin: TX Fifo Size = %d, RX Fifo Size = %d\n",
             fifo32_num_used(&s->tx_fifo), fifo32_num_used(&s->rx_fifo));
 
+    /* Activate the requested CS line */
+    qemu_set_irq(s->cs_lines[imx_spi_selected_channel(s)],
+                 imx_spi_current_channel_pol(s));
+
     while (!fifo32_is_empty(&s->tx_fifo)) {
+        uint32_t tx;
+        uint32_t rx = 0;
         int tx_burst = 0;
         int index = 0;
 
@@ -178,8 +193,6 @@  static void imx_spi_flush_txfifo(IMXSPIState *s)
 
         tx_burst = MIN(s->burst_length, 32);
 
-        rx = 0;
-
         while (tx_burst) {
             uint8_t byte = tx & 0xff;
 
@@ -221,6 +234,12 @@  static void imx_spi_flush_txfifo(IMXSPIState *s)
         s->regs[ECSPI_STATREG] |= ECSPI_STATREG_TC;
     }
 
+    /* Deselect all SS lines if transfert if completed */
+    if (s->regs[ECSPI_STATREG] & ECSPI_STATREG_TC) {
+        qemu_set_irq(s->cs_lines[imx_spi_selected_channel(s)],
+                     !imx_spi_current_channel_pol(s));
+    }
+
     /* TODO: We should also use TDR and RDR bits */
 
     DPRINTF("End: TX Fifo Size = %d, RX Fifo Size = %d\n",
@@ -230,6 +249,7 @@  static void imx_spi_flush_txfifo(IMXSPIState *s)
 static void imx_spi_reset(DeviceState *dev)
 {
     IMXSPIState *s = IMX_SPI(dev);
+    uint32_t i;
 
     DPRINTF("\n");
 
@@ -243,6 +263,11 @@  static void imx_spi_reset(DeviceState *dev)
     imx_spi_update_irq(s);
 
     s->burst_length = 0;
+
+    /* Disable all CS lines */
+    for (i = 0; i < 4; i++) {
+        qemu_set_irq(s->cs_lines[i], !imx_spi_channel_pol(s, i));
+    }
 }
 
 static uint64_t imx_spi_read(void *opaque, hwaddr offset, unsigned size)
@@ -359,15 +384,8 @@  static void imx_spi_write(void *opaque, hwaddr offset, uint64_t value,
         }
 
         if (imx_spi_channel_is_master(s)) {
-            int i;
-
             /* We are in master mode */
 
-            for (i = 0; i < 4; i++) {
-                qemu_set_irq(s->cs_lines[i],
-                             i == imx_spi_selected_channel(s) ? 0 : 1);
-            }
-
             if ((value & change_mask & ECSPI_CONREG_SMC) &&
                 !fifo32_is_empty(&s->tx_fifo)) {
                 /* SMC bit is set and TX FIFO has some slots filled in */
diff --git a/include/hw/ssi/imx_spi.h b/include/hw/ssi/imx_spi.h
index 7103953..b9b9819 100644
--- a/include/hw/ssi/imx_spi.h
+++ b/include/hw/ssi/imx_spi.h
@@ -46,6 +46,8 @@ 
 /* ECSPI_CONFIGREG */
 #define ECSPI_CONFIGREG_SS_CTL_SHIFT 8
 #define ECSPI_CONFIGREG_SS_CTL_LENGTH 4
+#define ECSPI_CONFIGREG_SS_POL_SHIFT 12
+#define ECSPI_CONFIGREG_SS_POL_LENGTH 4
 
 /* ECSPI_INTREG */
 #define ECSPI_INTREG_TEEN (1 << 0)