Hi, On Thu, Jul 18, 2013 at 05:15:45PM +0530, Sourav Poddar wrote: > >>+ list_for_each_entry(t,&m->transfers, transfer_list) { > >>+ qspi->cmd |= QSPI_WLEN(t->bits_per_word); > >>+ qspi->cmd |= QSPI_WC_CMD_INT_EN; > >>+ > >>+ ret = qspi_transfer_msg(qspi, t); > >>+ if (ret) { > >>+ dev_dbg(qspi->dev, "transfer message failed\n"); > >>+ return -EINVAL; > >>+ } > >>+ > >>+ m->actual_length += t->len; > >>+ > >>+ if (list_is_last(&t->transfer_list,&m->transfers)) > >>+ goto out; > >>+ } > >The use of list_is_last() here is *realy* strange - what's going on > >there? > > > We are checking if there is any transfer left, if no we are signalling the > flash device about the end of transfer. I'll quote your diff below because I wanted to show where your 'out' label lies: + list_for_each_entry(t, &m->transfers, transfer_list) { + qspi->cmd |= QSPI_WLEN(t->bits_per_word); + qspi->cmd |= QSPI_WC_CMD_INT_EN; + + ret = qspi_transfer_msg(qspi, t); + if (ret) { + dev_dbg(qspi->dev, "transfer message failed\n"); + return -EINVAL; + } + + m->actual_length += t->len; + + if (list_is_last(&t->transfer_list, &m->transfers)) + goto out; + } + +out: + m->status = status; + spi_finalize_current_message(master); see how it's place right after the closing curly brackets ? In that case, why do you need it ? list_for_each_entry() will do exactly what it says: iterate over each entry on the entry. When it reaches the last one, the loop will end. This renders your list_is_last() check useless. In fact, you could've figured that out if you just spent the time to read list_for_each_entry() carefully. -- balbi