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
Attachment:
signature.asc
Description: Digital signature
