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

[Marek Vasut <marex@denx.de>]

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 ?
[...]

> +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.
[...]

> @@ -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 ?
[...]

Best regards,
Marek Vasut