Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1659520

Re: [PATCH v2 linux-next 3/3] spi: imx: Add support for SPI Slave mode

Show all headers | View raw

On Fri, Jun 02, 2017 at 06:13:07AM -0700, Jiada Wang wrote:
> Hi Sascha
> 
> On 05/31/2017 05:10 AM, Sascha Hauer wrote:
> >> +
> >> +static void mx53_ecspi_tx_slave(struct spi_imx_data *spi_imx)
> >> +{
> >> +	u32 val = 0;
> >> +	int shift = spi_imx->count % sizeof(val);
> >> +
> >> +	if (spi_imx->tx_buf) {
> >> +		if (shift) {
> >> +			memcpy(((u8 *)&val) + sizeof(val) - shift,
> >> +			       spi_imx->tx_buf, shift);
> >> +		} else {
> >> +			val = *((u32 *)spi_imx->tx_buf);
> >> +			shift = sizeof(val);
> >> +		}
> >> +		val = cpu_to_be32(val);
> >> +		spi_imx->tx_buf += shift;
> >> +	}
> >> +
> >> +	if (!shift)
> >> +		shift = sizeof(val);
> > A better name for 'shift' is n_bytes. Its value should be calculated
> > before the if (spi_imx->tx_buf) block. Then you can use memcpy for the
> > four byte case aswell.
> >
> >
> > But anyway, have you tested this function for anything with
> > spi_imx->count % sizeof(val) != 0? In this case you have to put the fill
> > the FIFO only partly somewhere. I would assume you have to do this at
> > the end of the transfer, but you do this at the beginning. Can this
> > work?
> Yes, I have tested the case when "spi_imx->count % sizeof(val) != 0", it
> works.

For the record: Yes, this seems to be right. I misread the datasheet.
When the controller is programmed to transfer 5 bytes, then it seemed
fairly logical to me that it first expects four bytes in its 32bit FIFO
and then the remaining single byte. In fact it's the other way round,
how the example of the BURST_LENGTH register description says:

0x021 A SPI burst contains the 2 LSB in first word and all 32 bits in
second word.

So the part that doesn't fill a whole FIFO width goes out first, not
last.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH v2 linux-next 0/3] i.MX ECSPI controller slave mode support <jiada_wang@mentor.com> - 2017-05-31 11:30 +0200
  [PATCH v2 linux-next 1/3] spi: imx: add selection for iMX53 and iMX6 controller <jiada_wang@mentor.com> - 2017-05-31 11:30 +0200
    Re: [PATCH v2 linux-next 1/3] spi: imx: add selection for iMX53 and  iMX6 controller Sascha Hauer <s.hauer@pengutronix.de> - 2017-05-31 12:10 +0200
  [PATCH v2 linux-next 3/3] spi: imx: Add support for SPI Slave mode <jiada_wang@mentor.com> - 2017-05-31 11:30 +0200
    Re: [PATCH v2 linux-next 3/3] spi: imx: Add support for SPI Slave  mode Sascha Hauer <s.hauer@pengutronix.de> - 2017-05-31 14:20 +0200
      Re: [PATCH v2 linux-next 3/3] spi: imx: Add support for SPI Slave  mode Jiada Wang <jiada_wang@mentor.com> - 2017-06-02 15:20 +0200
        Re: [PATCH v2 linux-next 3/3] spi: imx: Add support for SPI Slave  mode Sascha Hauer <s.hauer@pengutronix.de> - 2017-06-07 10:40 +0200

csiph-web