Messages in this thread | | | Date | Thu, 18 Jul 2013 14:56:07 +0300 | From | Felipe Balbi <> | Subject | Re: [PATCHv4 2/3] drivers: spi: Add qspi flash controller |
| |
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 [unhandled content-type:application/pgp-signature] | |