On Mon, May 09, 2022 at 11:41:18AM +0000, Conor.Dooley@xxxxxxxxxxxxx wrote:
> Hey Ivan, one comment below.
> Thanks,
> Conor.
>
> On 07/05/2022 08:43, Ivan Bornyakov wrote:
> > ... snip ...
> > +static int mpf_read_status(struct spi_device *spi)
> > +{
> > + u8 status, status_command = MPF_SPI_READ_STATUS;
> > + struct spi_transfer xfer = {
> > + .tx_buf = &status_command,
> > + .rx_buf = &status,
> > + .len = 1,
> > + };
> > + int ret = spi_sync_transfer(spi, &xfer, 1);
> > +
> > + if ((status & MPF_STATUS_SPI_VIOLATION) ||
> > + (status & MPF_STATUS_SPI_ERROR))
> > + ret = -EIO;
> > +
> > + return ret ? : status;
> > +}
> > +
> > ... snip ...
> > +
> > +static int poll_status_not_busy(struct spi_device *spi, u8 mask)
> > +{
> > + int status, timeout = MPF_STATUS_POLL_TIMEOUT;
> > +
> > + while (timeout--) {
> > + status = mpf_read_status(spi);
> > + if (status < 0 ||
> > + (!(status & MPF_STATUS_BUSY) && (!mask || (status & mask))))
> > + return status;
> > +
> > + usleep_range(1000, 2000);
> > + }
> > +
> > + return -EBUSY;
> > +}
>
> Is there a reason you changed this from the snippet you sent me
> in the responses to version 8:
> static int poll_status_not_busy(struct spi_device *spi, u8 mask)
> {
> u8 status, status_command = MPF_SPI_READ_STATUS;
> int ret, timeout = MPF_STATUS_POLL_TIMEOUT;
> struct spi_transfer xfer = {
> .tx_buf = &status_command,
> .rx_buf = &status,
> .len = 1,
> };
>
> while (timeout--) {
> ret = spi_sync_transfer(spi, &xfer, 1);
> if (ret < 0)
> return ret;
>
> if (!(status & MPF_STATUS_BUSY) && (!mask || (status & mask)))
> return status;
>
> usleep_range(1000, 2000);
> }
>
> return -EBUSY;
> }
>
> With the current version, I hit the "Failed to write bitstream
> frame" check in mpf_ops_write at random points in the transfer.
> Replacing poll_status_not_busy with the above allows it to run
> to completion.
In my eyes they are equivalent, aren't they?
