Re: [PATCHv2] drivers: spi: Add qspi flash controller

From: Mark Brown
Date: Tue Jul 02 2013 - 07:04:46 EST


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
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

Attachment: signature.asc
Description: Digital signature