linux-kernel - Re: [PATCH 1/3] spi: add xtfpga SPI controller driver

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 11 Mar 2014 19:49:59 +0000
From:	Mark Brown <broonie@...nel.org>
To:	Max Filippov <jcmvbkbc@...il.com>
Cc:	linux-xtensa@...ux-xtensa.org, linux-spi@...r.kernel.org,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	Chris Zankel <chris@...kel.net>,
	Marc Gauthier <marc@...ence.com>,
	Rob Herring <robh+dt@...nel.org>,
	Grant Likely <grant.likely@...aro.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 1/3] spi: add xtfpga SPI controller driver

On Tue, Mar 11, 2014 at 04:44:49PM +0400, Max Filippov wrote:

> +static inline int xtfpga_spi_wait_busy(struct xtfpga_spi *xspi)
> +{
> +	unsigned long timeout = jiffies + msecs_to_jiffies(100);
> +	while (xtfpga_spi_read32(xspi, XTFPGA_SPI_BUSY)) {
> +		if (!time_before(jiffies, timeout))
> +			return -EBUSY;
> +		else
> +			cpu_relax();
> +	}
> +	return 0;
> +}

So we'll busy wait for up to 100ms - that seems like an awfully long
time.  Perhaps fall back to msleep() if the delay is non-trivial (or
just reduce the timeout)?  If there are large enough FIFOs dead
reckoning a sleep for the expected transfer time might be helpful but it
seems like there's no meaningful FIFO.

> +/* Unused: this device controls its only CS automatically,
> + * deactivating it after every 16 bit transfer completion.
> + */

This is too limited to use with most SPI clients, they'll want to be
able to transmit more than one word (and the fact that only 16 bit words
are supported is also an issue, though that's easy enough to handle for
a bitbanging driver - I'd really strongly suggest supporting 8 bits per
word as well).  Clients are pretty much going to need to use GPIO based
chip select, you should make sure that's supported and covered in the
binding.

> +static void xtfpga_spi_chipselect(struct spi_device *spi, int is_on)
> +{
> +}

Omit this since it's empty.

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