linux-kernel - Re: [PATCH] mtd: spi-nor: Only set Micron quad-read mode when controller in 4-lane TX mode

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] [day] [month] [year] [list]

Message-ID: <55937F74.1020801@topic.nl>
Date:	Wed, 1 Jul 2015 07:49:40 +0200
From:	Mike Looijmans <mike.looijmans@...ic.nl>
To:	"Bean Huo 霍斌斌 (beanhuo)" 
	<beanhuo@...ron.com>
CC:	"git@...inx.com" <git@...inx.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"dwmw2@...radead.org" <dwmw2@...radead.org>,
	"computersforpeace@...il.com" <computersforpeace@...il.com>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>
Subject: Re: [PATCH] mtd: spi-nor: Only set Micron quad-read mode when controller
 in 4-lane TX mode

On 01-07-15 04:19, Bean Huo 霍斌斌 (beanhuo) wrote:
>> drivers/mtd/spi-nor/spi-nor.c | 2 ++
>> 1 file changed, 2 insertions(+)
> 
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index e8f6131..10ba94f 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -1398,6 +1398,8 @@ static int set_quad_mode(struct spi_nor *nor, struct flash_info *info)
>   >		}
>   >		return status;
>   >	case CFI_MFR_ST:
>> +		if (!(nor->spi->mode & SPI_TX_QUAD))
>> +			return 0;
> I think, this action should be done before callback set_quad_mode,and add some codes in
> Spi controller driver.what is more,how do you get spi member in spi_nor?seems no this member.

Too many changes between the Xilinx tree and mainline. My patch isn't for
mainline, I'll make a new one.

> I have one concern that as long as spi nor be configed into quad mode,spi controller must be also
> immediately Transfer into quad mode, otherwise read status will be fail.

The original commit that added the micron Quad mode did not do this. I wonder
how this was tested, because it could never have worked on any setup I'm aware
of. After setting the "quad mode" bit in volatile config, the controller needs
to start sending commands on all 4 lanes.
Even if it succeeds in doing that, a soft reset (e.g. unloading/loading the
driver, or kexec to another kernel) will bring the system in a state where the
controller will use 1 lane for commands while the chip expects 4, thus
breaking all communication.

Having slept and pondered about this, I suggest plainly removing the Micron
Quad mode code, because it is just broken and can never work. There's no
actual performance advantage here, apart from just saving a few clocks on
commands, which would be less than 0.1% on a 1k data read.

> But setting spi controller layer(driver/spi/) and configure spi nor layer(driver/mtd/spi-nor)
>   are not the same mtd layer,I found that it's hard to do.
> But for new structure spi controller(such as driver/mtd/spi-nor/fsl-quadspi.c) is very reasonable.and
> it can be easy to set spi controller and spi nor into quad mode at the same time.

I'm sorry, but I didn't understand what you meant  here.



Kind regards,

Mike Looijmans
System Expert

TOPIC Embedded Products
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
Telefax: +31 (0) 499 33 69 70
E-mail: mike.looijmans@...icproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/