From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1bAkB5-0007hx-KF for linux-mtd@lists.infradead.org; Wed, 08 Jun 2016 20:34:16 +0000 Received: by mail-wm0-x244.google.com with SMTP id k184so5740772wme.2 for ; Wed, 08 Jun 2016 13:33:53 -0700 (PDT) Subject: Re: [PATCH 2/2] mtd: m25p80: consider max_transfer_size when reading To: Mark Brown References: <56D22823.7090005@gmail.com> <20160405193952.GA5243@localhost> <57041B43.2000109@gmail.com> <20160405210727.GB5243@localhost> <5706B084.2070909@gmail.com> <20160505235700.GA99474@google.com> <20160506121431.GQ6292@sirena.org.uk> <77a22258-95ca-031a-825d-a9e98e30a162@gmail.com> <20160606174003.GE7510@sirena.org.uk> <20160606230752.GM7510@sirena.org.uk> Cc: Brian Norris , Michal Suchanek , MTD Maling List , Cyrille Pitchen , Marek Vasut , Han Xu , linux-spi@vger.kernel.org From: Heiner Kallweit Message-ID: <20977e07-6354-2753-8286-e992c32454d1@gmail.com> Date: Wed, 8 Jun 2016 21:51:15 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Am 07.06.2016 um 08:03 schrieb Heiner Kallweit: > Am 07.06.2016 um 01:07 schrieb Mark Brown: >> On Mon, Jun 06, 2016 at 08:53:10PM +0200, Heiner Kallweit wrote: >>> Am 06.06.2016 um 19:40 schrieb Mark Brown: >> >>>> That doesn't make any sense, the controller hardware doesn't >>>> magically change based on what is connected to it. >> >>> The issue with fsl-espi is that the controller deactivates CS after >>> each physical transfer. And a lot of HW designs use the hardware CS, >>> therefore the advise to use a GPIO instead doesn't really help. >> >> There's no pinmux to fix the broken behaviour? The Freescale SPI >> controllers really are terrible :( >> >>> In case of SPI NOR CS needs to stay active between command and data >> >> In the case of *all* SPI transactions this needs to happen. This is >> *not* optional, it's not some weird requirement of NOR, it's one of the >> most basic requirements for a SPI driver on Linux. >> >>> Well, we could reduce the max_transfer_size exposed by the driver by >>> the max length of a m25p80 read command in general. >> >> No! Apart from anything else this would be a complete and total >> abstraction failure. This is a driver for a SPI controller, not a >> driver for a flash controller, which means that it should not have flash >> specific hacks in it. >> >>> Then we might be too strict for other SPI devices, but this may >>> be acceptable as the current fsl-espi driver is usable with SPI NOR >>> devices only anyway. >> >> What makes you say this? I'm guessing that the driver is just broken >> but it would be good to understand the specifics. >> > See fsl_espi_rw_trans(): > In case of transfers > SPCOM_TRANLEN_MAX it loops and reads in > SPCOM_TRANLEN_MAX chunks whilst replacing bytes 2-4 of the > message buffer with the new start address to read from. > Having said that it implicitely assumes that the first transfer in > the message is a m25p80 read command with a 3 byte address. > > And in fsl_espi_do_one_msg() it assumes that each message includes > max one r/o transfer (else rx_buf is overwritten). > > I'm not 100% sure whether any arbitrary combination of r/o, r/w, w/o > transfers in a SPI message is acceptable. But this controller driver > supports just the transfer combinations generated by the m25p80 > protocol driver. > I think in this discussion there was the idea already to introduce a message size limit complementing the transfer size limit. Could this be an acceptable way to go? If yes I'd prepare a patch as basis for further discussion.