Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1597 bytes --]

On Sat, Apr 12, 2014 at 11:48:36AM -0700, Jane Wan wrote:
> Make FSL eSPI CSnBEF and CSnAFT in ESPI_SPMODEn registers (n=0,1,2,3)
> configurable through device tree.  FSL eSPI driver hardcodes them to 0.
> Some device requires different value.

What do these do?

> Allow FSL eSPI driver configurable whether to send all received data
> form slave devices to user.  When user wants to send n_tx bytes and
> receives n_rx bytes, FSL eSPI driver sends (n_tx + n_rx) bytes on MOSI.
> For the received (n_tx + n_rx) bytes from MISO, current FSL eSPI driver
> drops the first n_tx bytes, only passes the last n_rx bytes to user.
> Some device driver has problem with this.  It requires to know all bytes
> that the slave device puts on MISO.

This sounds like a separate patch to the first one, the described
behaviour is definitely buggy and any correctly implemented Linux driver
that does bidirectional I/O will have trouble with it.  It should be
split out from the new DT bindings which are a new feature.

> ---
>  drivers/spi/spi-fsl-espi.c       |   68 ++++++++++++++++++++++++++++++++++----
>  1 files changed, 61 insertions(+), 7 deletions(-)

All DT binding changes need to be documented in the binding document.

> +/* whether to send all rx data to user per chip select */
> +static u8 *spi_raw_rxdata_to_user;
> +

No, any data needs to be part of the driver data structure not a global
variable.

> +		if (spi_raw_rxdata_to_user[m->spi->chip_select])
> +			espi_trans->len = n_tx;
> +		else
> +			espi_trans->len = trans_len + n_tx;

Why is there even an option for the buggy behaviour?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]