From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ashish Kumar Date: Fri, 21 Dec 2018 08:55:07 +0000 Subject: [U-Boot] [PATCH 00/16] SF: Migrate to Linux SPI NOR framework In-Reply-To: References: <20181212173228.12281-1-vigneshr@ti.com> <1cd66c4d-d37b-e48b-0a6a-6037e1d255f8@ti.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de > -----Original Message----- > From: U-Boot On Behalf Of Vignesh R > Sent: Tuesday, December 18, 2018 10:49 PM > To: Jagan Teki > Cc: Marek Vasut ; Tom Rini ; U-Boot- > Denx ; Michal Simek ; Siva > Durga Prasad Paladugu ; Boris Brezillon > ; Miquel Raynal ; > Stefan Roese ; Jagan Teki > Subject: Re: [U-Boot] [PATCH 00/16] SF: Migrate to Linux SPI NOR framework > > > On 18-Dec-18 6:02 PM, Jagan Teki wrote: > > On Sat, Dec 15, 2018 at 9:13 PM Vignesh R wrote: > >> > >>>>> 2) For BAR support, lets place it as it is and support via spi-nor > >>>> > >>>> Problem is, it not desirable to use BAR as default because its not > >>>> stateless and does not work with all flash parts. OTOH, it seems > >>>> like 4 byte addressing (stateless dedicated opcode or with > >>>> enter/exit 4 byte > >>>> mode) seems to be standard. > >>>> Also, Linux doesn't support BAR and haven't seen any request for > >>>> BAR support. Why support additional feature and burden of > >>>> maintaining when it may not be needed. > >>>> > >>>> But if you insist, I just have to add BAR support back. > >>> > >>> It's not about insistence, ie how we support since from long and > >>> u-boot bootloader project does in general. bootloader can be some > >>> certain boot difference functionalities unlike Linux, it's better > >>> not to compare u-boot with Linux in all cases. > >>> > >>> Example we have SPI_TX_BYTE which usually not supported in Linux. > >>> Since it's ich controller specific and it require for bootloader to > >>> do byte tx on that specific controller, so we supported. Same for > >>> the case with the BAR, this specific controller(or supported boards) > >>> has been in U-Boot since from long and they do upstream well. So we > >>> need to support BAR in any case or we can find any alternative to > >>> work the same functionalities. (we tried before but ended-up adding > >>> BAR) > >>> > >> How about this instead? > >> > >> Lets use 4 byte addressing opcodes as default for >16MB flashes. > >> But if CONFIG_SPI_FLASH_BAR is enabled then, spi-nor layer will use > >> BAR registers for >16MB access instead of 4 byte addressing. > > > > Yes, this can be normal approach. but what I'm trying to initiate here > > is if we have a generic kind of sanity check transfer for > 16MB > > flashes, then we can notify the controllers enable BAR to support > > > 16MiB. > > > > I guess you are saying to attempt 4 byte addressing transfers and if that fails then > try using BAR? AFAIK, there is _no way_ to detect whether or not > controller/flash supports 4 byte addressing and then do a fallback. > > > This way we can reduce the ifdefs on code, and if we handle BAR in > > better way to minimize the code eventually we even drop the CONFIG > > option. > > > >> I will remove SPI_FLASH_BAR from defconfigs from all boards expect > >> those controllers that really cannot handle 4 byte addressing. From a > >> quick glance it looks following controllers support only 3 byte addresses: > >> stm32_qspi.c > >> ich.c > >> fsl_qspi.c > >> renesas_rpc_spi.c > >> mtk_qspi.c > > > > I'm not sure whether all these support 4-byte addressing or not? > > Just to be clear, all SPI drivers _except_ those in above list can support 4 byte > addressing mode w/o any additional code change. fsl_qspi.c: should support 4 byte as well, since the current frame work had limitation, u-boot.denx.de version of this driver was only supporting until 16MB Regards Ashish > > > since we don't have 4-byte on spi-flash they do use BAR for accessing > > > 16MiB, zynq_qspi is the key taker for this initially. > > > > I am confused here, let me put down more details: > > Access >16MB of flash memory is possible by 3 ways: > 1. using dedicated 4 byte addressing opcodes 2. Send "enter 4 byte" command > and then use normal commands but with > 4 bytes of address > 3 Provide BA24 BA25 of address via BAR register > > AFAIK, all flashes support 1 or 2. But only some flashes support 3. > So, the idea is to use option 1 or 2 as default and use 3 only if SPI_FLASH_BAR is > enabled. > > Ideally, there is _nothing_ in SPI controllers that would stop us from using 4 byte > addressing because a SPI controller would have no idea of slave connected (ie. it > may or may not be a flash). SPI drivers should _blindly_ send/receive dout/din > buffers passed as part of spi_xfer() and not interpret these buffers as > cmd/addr/data separately So, the decision on which of the above 3 options to > use should be in the SPI NOR layer and has nothing to do with SPI controller > drivers. > > But since spi_flash.c was always used 3 byte addressing, _some_ of the SPI > controller drivers are _hard-coded_ to expect 3 byte addresses (eg.: > above 4 drivers that I mentioned) See [1] on how these drivers convert dout > buffer is back to flash offset. This is because there is no way to communicate > cmd/addr/data separately using spi_xfer(), but _intelligent_ SPI HW do need this > information. This is not a HW limitation but more of a framework limitation. > > So, thus we can classify SPI controllers into roughly two types: > 1. Controllers that need to know flash specific information like opcode, address > length, dummy bytes etc (controllers that supports memory mapped access to > flash(or such other acceleration interface). > Such controllers should move to spi-mem (but can optionally implement spi_xfer > to support non flash devices) > > 2. Traditional SPI controllers that are usually FIFO/shift register based and > provide direct access to SPI bus will continue to use traditional SPI framework. > > Moving to 4 byte addressing mode will not affect second type of SPI controllers, > but will break first type of controllers that do their own address conversion logic > like[1]. > So my suggestion is to enable CONFIG_SPI_FLASH_BAR only for controllers to > do[1] until moved to spi-mem. > > From my analysis: > >> stm32_qspi.c > >> ich.c > >> fsl_qspi.c > >> renesas_rpc_spi.c > >> mtk_qspi.c > >> ti_qspi.c > >> cadence_quadspi.c > > are candidates to move to spi-mem layer. Rest seems to be traditional SPI > controllers. Yes fsl_qspi.c should be moved to spi-mem layer. Similar work in being done in Linux as well. Regards Ashish > > AFAIU, zynq_qspi seems to be FIFO based SPI controller. Why would zynq_qspi > driverneed to know whether SPI NOR layer uses BAR or not? Or did I miss > something? > > >> > >> So, except for boards with above controllers, I will remove > >> SPI_FLASH_BAR for all other boards. If there is a regression, then > >> such boards can go back to enabling CONFIG_SPI_FLASH_BAR. > > > > I think we can go the BAR code in flash as you mentioned with CONFIG > > option, and don't remove any CONFIG items from board configs then we > > can add sanity check patch on top of spi-nor changes and give warning > > to boards which never need BAR(ask them to drop CONFIG_BAR). This way > > transition look promising to move BAR to 4-byte addressing. > > > > Are you suggesting board users should individually remove > CONFIG_SPI_FLASH_BAR from defconfigs based on their judgement? > How do you propose to do sanity check? > Why not use 4 byte addressing as default and enable CONFIG_SPI_FLASH_BAR > only for controllers that need them? > > >> > >> AFAIU, above controller HWs(except ich perhaps) can support 4 byte > >> addressing but would need to move to spi-mem. > >t > > Even if we move to spi-mem, but BAR need to handle it flash side. isn't it? > > > > Yes, presence/configuration of BAR registers is abstracted by SPI NOR and spi- > mem drivers need not have any knowledge. > > [1] > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.b > ootlin.com%2Fu- > boot%2Flatest%2Fsource%2Fdrivers%2Fspi%2Frenesas_rpc_spi.c%23L264&am > p;data=02%7C01%7CAshish.Kumar%40nxp.com%7Cd535d3bc013949ad15230 > 8d6650d0bfa%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63680 > 7504073361903&sdata=DpdMcJ%2F81jOOVE5MRNe6xSqgAlRGXVyvFb6C > dm3ot8Q%3D&reserved=0 > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.b > ootlin.com%2Fu-boot%2Fv2019.01- > rc2%2Fsource%2Fdrivers%2Fspi%2Fmtk_qspi.c%23L253&data=02%7C01 > %7CAshish.Kumar%40nxp.com%7Cd535d3bc013949ad152308d6650d0bfa%7 > C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636807504073361903 > &sdata=JprAHMDaR4DY2vZocYIP0Wfu1l9yuxrl4zveN69hVA0%3D&re > served=0 > > Regards > Vignesh > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.de > nx.de%2Flistinfo%2Fu- > boot&data=02%7C01%7CAshish.Kumar%40nxp.com%7Cd535d3bc013949 > ad152308d6650d0bfa%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0% > 7C636807504073361903&sdata=bJBeSCBf%2BtAcF3XyUr04jB1ba4UMTS > ZC%2FRSlr6qKkcA%3D&reserved=0