Re: [RFC PATCH] mtd: spi-nor: add DDR quad read support

[Huang Shijie <shijie8@gmail.com>]

On Thu, Apr 17, 2014 at 04:24:36PM +0200, Marek Vasut wrote:
> On Wednesday, April 16, 2014 at 11:47:21 AM, Huang Shijie wrote:
> [...]
> 
> > @@ -74,6 +74,11 @@ static int read_cr(struct spi_nor *nor)
> >  static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
> >  {
> >  	switch (nor->flash_read) {
> > +	case SPI_NOR_DDR_QUAD:
> > +		/*
> > +		 * We set 8 for the DDR Quad read, the SPI NOR controller
> > +		 * can change it to 6 or 4 with DeviceTree property.
> > +		 */
> 
> Isn't the number of dummy cycles a property of the chip ?
I want to add DT node, such as "ddr_quad_read_dummy".
But I do not know which Document file to put the DT node to. 

the spi-bus.txt ? or the spi-nor.txt? 


> [...]
> 
> > +static int set_ddr_quad_mode(struct spi_nor *nor, u32 jedec_id)
> > +{
> > +	int status;
> > +
> > +	switch (JEDEC_MFR(jedec_id)) {
> > +	default: /* Spansion */
> 
> This should really _check_ the jedec ID here and only enable the quad mode for 
> spansion on spansion flashes. The default case should just return -ENOTSUP or 
> something.
> [...]
ok. change it in next version.


> 
> > @@ -1016,8 +1038,15 @@ int spi_nor_scan(struct spi_nor *nor, const struct
> > spi_device_id *id, if (info->flags & SPI_NOR_NO_FR)
> >  		nor->flash_read = SPI_NOR_NORMAL;
> > 
> > -	/* Quad/Dual-read mode takes precedence over fast/normal */
> > -	if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
> > +	/* DDR Quad/Quad/Dual-read mode takes precedence over fast/normal */
> > +	if (mode == SPI_NOR_DDR_QUAD && info->flags & SPI_NOR_DDR_QUAD_READ) {
> > +		ret = set_ddr_quad_mode(nor, info->jedec_id);
> > +		if (ret) {
> > +			dev_err(dev, "DDR quad mode not supported\n");
> > +			return ret;
> 
> Is it really necessary to fail here? Can we not fall back to some "lower" mode ?
I perfer to fail here.
If we fail to enable the quad read, we also do not fall back to fast
read mode.

thanks
Huang Shijie