On 26 November 2017 at 23:16, Francisco Iglesias
<frasse.igles...@gmail.com> wrote:
> Add support for the RX discard and RX drain functionality. Also transmit
> one byte per dummy cycle (to the flash memories) with commands that require
> these.
>
> Signed-off-by: Francisco Iglesias <frasse.igles...@gmail.com>
> Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> Tested-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
Hi. Coverity (CID 1383841) complains that this change introduces
a use of an uninitialized local variable:
> static void xilinx_spips_flush_txfifo(XilinxSPIPS *s)
> {
> int debug_level = 0;
> + XilinxQSPIPS *q = (XilinxQSPIPS *) object_dynamic_cast(OBJECT(s),
> +
> TYPE_XILINX_QSPIPS);
>
> for (;;) {
> int i;
> uint8_t tx = 0;
> uint8_t tx_rx[num_effective_busses(s)];
tx_rx[] is the variable in question.
> + uint8_t dummy_cycles = 0;
> + uint8_t addr_length;
>
> if (fifo8_is_empty(&s->tx_fifo)) {
> if (!(s->regs[R_LQSPI_CFG] & LQSPI_CFG_LQ_MODE)) {
> @@ -258,54 +336,102 @@ static void xilinx_spips_flush_txfifo(XilinxSPIPS *s)
> tx_rx[i] = fifo8_pop(&s->tx_fifo);
> }
> stripe8(tx_rx, num_effective_busses(s), false);
> - } else {
> + } else if (s->snoop_state >= SNOOP_ADDR) {
> tx = fifo8_pop(&s->tx_fifo);
> for (i = 0; i < num_effective_busses(s); ++i) {
> tx_rx[i] = tx;
> }
> + } else {
> + /* Extract a dummy byte and generate dummy cycles according to
> the
> + * link state */
> + tx = fifo8_pop(&s->tx_fifo);
> + dummy_cycles = 8 / s->link_state;
> }
Before this if...else ladder was changed, each branch of it
filled in the whole tx_rx[] array. But the new else {} case
does not do that...
> for (i = 0; i < num_effective_busses(s); ++i) {
> int bus = num_effective_busses(s) - 1 - i;
> - DB_PRINT_L(debug_level, "tx = %02x\n", tx_rx[i]);
> - tx_rx[i] = ssi_transfer(s->spi[bus], (uint32_t)tx_rx[i]);
> - DB_PRINT_L(debug_level, "rx = %02x\n", tx_rx[i]);
> + if (dummy_cycles) {
> + int d;
> + for (d = 0; d < dummy_cycles; ++d) {
> + tx_rx[0] = ssi_transfer(s->spi[bus], (uint32_t)tx_rx[0]);
> + }
> + } else {
> + DB_PRINT_L(debug_level, "tx = %02x\n", tx_rx[i]);
> + tx_rx[i] = ssi_transfer(s->spi[bus], (uint32_t)tx_rx[i]);
> + DB_PRINT_L(debug_level, "rx = %02x\n", tx_rx[i]);
> + }
...and subsequent code, both introduced in this patch and code
already present, will read the tx_rx[] array.
Could one of you have a look at this and determine what the right
fix is, please?
thanks
-- PMM
