lkml.org 
[lkml]   [2022]   [May]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v13 2/3] fpga: microchip-spi: add Microchip MPF FPGA manager
    On Mon, May 30, 2022 at 11:22:26AM +0000, Conor.Dooley@microchip.com wrote:
    > On 26/05/2022 19:13, Ivan Bornyakov wrote:
    > > +static int mpf_read_status(struct spi_device *spi)
    > > +{
    > > + u8 status = 0, status_command = MPF_SPI_READ_STATUS;
    > > + /*
    > > + * Two identical SPI transfers are used for status reading.
    > > + * The reason is that the first one can be inadequate.
    > > + * We ignore it completely and use the second one.
    > > + */
    > > + struct spi_transfer xfers[] = {
    > > + [0 ... 1] = {
    > > + .tx_buf = &status_command,
    > > + .rx_buf = &status,
    > > + .len = 1,
    > > + .cs_change = 1,
    > > + }
    > > + };
    >
    > Hmm, I don't think that this is correct, or at least it is not
    > correct from the polarfire /soc/ perspective. I was told that
    > there was nothing different other than the envm between the
    > programming for both devices - but this is another situation
    > where I start to question that.
    >
    > When I run this code, ISC enable /never/ passes - failing due
    > to timing out. I see something like this picture here:
    > https://i.imgur.com/EKhd1S3.png
    > You can see the 0x0B ISC enable coming through & then a status
    > check after it.
    >
    > With the current code, the value of the "status" variable will
    > be 0x0, given you are overwriting the first MISO value with the
    > second. According to the hw guys, the spi hw status *should*
    > only be returned on MISO in the first byte after SS goes low.
    >
    > If this is not the case for a non -soc part, which, as I said
    > before, I don't have a board with the SPI programmer exposed
    > for & I have been told is not the case then my comments can
    > just be ignored entirely & I'll have some head scratching to
    > do...
    >
    > Thanks,
    > Conor.
    >

    If I understood correctly, SS doesn't alter between two status reading
    transactions despite .cs_change = 1. May be adding some .cs_change_delay
    to spi_transfer struct can help with that?

    > > + int ret = spi_sync_transfer(spi, xfers, 2);
    > > +
    > > + if ((status & MPF_STATUS_SPI_VIOLATION) ||
    > > + (status & MPF_STATUS_SPI_ERROR))
    > > + ret = -EIO;
    > > +
    > > + return ret ? : status;
    > > +}

    \
     
     \ /
      Last update: 2022-05-30 14:30    [W:6.442 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site