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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140414205547.GI25182@sirena.org.uk>
Date:	Mon, 14 Apr 2014 21:55:47 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Jane Wan <Jane.Wan@...nspeed.com>
Cc:	grant.likely@...retlab.ca, rob.herring@...xeda.com,
	Emilian.Medve@...escale.com, kenth.eriksson@...nsmode.com,
	thomas.de.schampheleire@...il.com, b48286@...escale.com,
	jg1.han@...sung.com, sr@...x.de, insop.song@...nspeed.com,
	spi-devel-general@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org, devicetree-discuss@...ts.ozlabs.org
Subject: Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all
 received data to user

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?

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)