linux-kernel - Re: [PATCHv2] drivers: spi: Add qspi flash controller

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: <20130702151947.GD7013@arwen.pp.htv.fi>
Date:	Tue, 2 Jul 2013 18:19:47 +0300
From:	Felipe Balbi <balbi@...com>
To:	Mark Brown <broonie@...nel.org>
CC:	Felipe Balbi <balbi@...com>, Sourav Poddar <sourav.poddar@...com>,
	<spi-devel-general@...ts.sourceforge.net>,
	<grant.likely@...aro.org>, <rnayak@...com>,
	<linux-omap@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv2] drivers: spi: Add qspi flash controller

On Tue, Jul 02, 2013 at 12:04:32PM +0100, Mark Brown wrote:
> On Tue, Jul 02, 2013 at 01:43:38PM +0300, Felipe Balbi wrote:
> > On Tue, Jul 02, 2013 at 11:17:18AM +0100, Mark Brown wrote:
> 
> > > +	/* setup command reg */
> > > +	qspi->cmd = 0;
> > > +	qspi->cmd |= QSPI_WLEN(8);
> 
> > Sourav hardcodes wordlenght to 8-bits, and yet he enables 8, 16 and
> > 32-bits per word.
> 
> Yeah, that's what I noticed (well first off I noticed that there were no
> constraints on bits per word at all).
> 
> > > +	qspi->cmd |= QSPI_EN_CS(0);
> 
> > he's also hardcoding the chipselect line which should be take from
> > m->spi->chip_select
> 
> This one *can* be OK if the driver only accepts one chip select (though
> obviously supporting more is better).  I'd really only done a fairly

this controller has 6 chip selects IIRC

> high level review for framework stuff so hadn't got that far yet.
> 
> One thing I really want to get round to doing with the SPI core is
> providing an easy to pick up GPIO chip select as standard 

that should be fairly simple I guess. Just lack of time, I'm assuming ?

Complex will be to support up to 128-bits per word as this particular
controller supports. In fact, this controller is, IMO, overly flexible.
You can do 1, 2, 3, 4, 5, 6, ... , 128 bits per word.

The only problem is that the DATA registers which give you access to the
shift register, aren't mapped to consecutive offsets, but that should
still be ok since we will, anyway, be using readl/writel for each
register. Still, for platforms which can provide writeq/readq (not sure
if that's already supported on ARM) we could save a few cycles, no ?

anyway... it is what it is.

-- 
balbi

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