Hi Frieder, > -----Original Message----- > From: Schrempf Frieder <frieder.schre...@kontron.de> > Sent: Tuesday, December 3, 2019 6:17 PM > To: Kuldeep Singh <kuldeep.si...@nxp.com>; u-boot@lists.denx.de > Cc: Priyanka Jain <priyanka.j...@nxp.com>; ja...@amarulasolutions.com; > s...@denx.de; Ashish Kumar <ashish.ku...@nxp.com>; Ye Li <ye...@nxp.com> > Subject: Re: [EXT] Re: [PATCH 0/8] Transition of fsl qspi driver to spi-mem > framework > > Caution: EXT Email > > On 03.12.19 11:56, Kuldeep Singh wrote: > > Hi Frieder, > > > >> -----Original Message----- > >> From: Schrempf Frieder <frieder.schre...@kontron.de> > >> Sent: Tuesday, December 3, 2019 2:27 PM > >> To: Kuldeep Singh <kuldeep.si...@nxp.com>; u-boot@lists.denx.de > >> Cc: Priyanka Jain <priyanka.j...@nxp.com>; > >> ja...@amarulasolutions.com; s...@denx.de; Ashish Kumar > >> <ashish.ku...@nxp.com>; Ye Li <ye...@nxp.com> > >> Subject: Re: [EXT] Re: [PATCH 0/8] Transition of fsl qspi driver to > >> spi-mem framework > >> > >> Caution: EXT Email > >> > >> On 03.12.19 07:30, Kuldeep Singh wrote: > >>> Hi Frieder, > >>> > >>>> -----Original Message----- > >>>> From: Schrempf Frieder <frieder.schre...@kontron.de> > >>>> Sent: Monday, December 2, 2019 5:35 PM > >>>> To: Kuldeep Singh <kuldeep.si...@nxp.com>; u-boot@lists.denx.de > >>>> Cc: Priyanka Jain <priyanka.j...@nxp.com>; > >>>> ja...@amarulasolutions.com; s...@denx.de; Ashish Kumar > >>>> <ashish.ku...@nxp.com> > >>>> Subject: [EXT] Re: [PATCH 0/8] Transition of fsl qspi driver to > >>>> spi-mem framework > >>>> > >>>> Caution: EXT Email > >>>> > >>>> + Ashish > >>>> > >>>> Hi Kuldeep, > >>>> > >>>> On 29.11.19 06:54, Kuldeep Singh wrote: > >>>>> This entire patch series migrate freescale qspi driver to spi-mem > >> framework. > >>>> > >>>> First, thanks for working on this. I have this on my list for quite > >>>> a long time, but struggled to find enough time to actually get it done. > >>>> > >>>>> > >>>>> Patch 1 removes the old fsl qspi driver > >>>> > >>>> You shouldn't remove the old driver before the new one is in place, > >>>> as this breaks the build in between the two patches. > >>> > >>> I first merged the patch1 and patch2 and found that the diff output > >>> was very > >> much less readable. > >>> That's why I split them into 2 patches so as to make new driver > >>> changes > >> legible. > >>> Please let me know how shall I proceed. Shall I merge the two patches? > >> > >> Yes, the merged patch will not be very good to read, but as far as I > >> know there is no other option. We must not break the build to keep > bisectability. > > > > Alright I will merge the two patches. > > > >> > >>> > >>>> > >>>>> > >>>>> Patch 2 adds new qspi driver incorporating spi-mem framework which > >>>>> is ported version of linux qspi driver. Initial port was done by > >>>>> Frieder. > >>>>> Now, no more direct access to spi-nor memory is possible. Few > >>>>> changes were introduced to prevent uboot crash such as to add > >>>>> complete flash size to SFA1AD, SFA2AD, SFB1AD, SFB2AD based on > >>>>> chip select number instead of 1k. Immediate effect was observed on > >>>>> pfe while using 1k size as it access spi-nor memory directly. > >>>>> Using complete flash size resolves > >>>> the crash but data read will not be valid. > >>>> > >>>> Can you provide more information about the problem/crash you > >>>> experience and the platform you are working on? > >>> > >>> I observed crash on LS1012. Also, any access to flash direct memory > >>> above > >> 1k will crash without this change. > >> > >> As I already told Ashish in the conversation referenced in my last mail: > >> I can't see any good reason why the direct memory access is something > >> that we need or should support. We should always use the APIs > >> provided by U-Boot to access the flash and that is mtd. > >> > >>> By adding this, crash will be resolved but data is invalid as > >>> mentioned in > >> patch-set. > >> > >> So what's the purpose of your changes at all, if they do not solve > >> the problem you're trying to solve? > > > > I observed booting crash on all ls1012 platforms. Control does not reach > even end of uboot prompt. > > I dig in deeper, and found that "pfe (packet forwarding engine)" was using > spi-nor memory directly. > > With this change, booting crash was resolved. Now, at least other network > interfaces can be used. > > Without this changes, I have to disable pfe on adhoc basis so as to get > > uboot > prompt. > > This is to make sure all intended qspi targets are booting. > > Ok, thanks for pointing out the PFE driver. I didn't know about such a > peripheral. So this seems to be the actual problem here. > > I don't really understand, why Ashish didn't mention this when we were talking > about this issue some weeks ago. > > > > >> Why don't you just use sf/mtd to access the flash? > > > > Pfe framework have to bring in changes to access flash using sf in uboot. > > Yes and that's something that should be done first instead of hacking the QSPI > controller driver. It shouldn't be too hard to modify the PFE driver so that > it > uses the serial flash API (spi_flash_read()) to access the SPI NOR. > Can you try to come up with a patch for the PFE driver?
I have sent out PFE driver patch upstream[1] and booting crash is now resolved. Moreover, After using 1k size, I faced a random crash in environment which was resolved after enabling SYS_RELOC_GD_ENV_ADDR in defconfig. I am not sure why this needed when setting 1k size? Note that, same is not required if I use my previous implementation. Now, I found a new bug while testing read functionality in LS1012A, LS1046A on commit "4b19b89ca4a8". I cannot access memory above 16MB. For example, when I try to access 16M, data read is actually from 0x0 offset. Could you please share your views on this behavior. --Kuldeep > > > > > Thanks > > Kuldeep > > > >> > >>> > >>>> Are you referring to the same issue as Ashish in this discussion here > >>>> [1]? > >>> > >>> Yes, I had a discussion with him. > >>> > >>>> There are two reasons why I'd like to avoid using the whole memory > >>>> mapped area for AHB access. > >>>> First, I'd like to keep the U-Boot driver as close as possible to > >>>> the Linux > >> driver. > >>>> Second, the intention of the spi-mem layer is to abstract the flash > >>>> layer and therefore this driver should work independently of flash > >>>> type or > >> size. > >>> > >>> Boot from QSPI-NAND will still not be possible. Code in bootROM is > >>> only to > >> access QSPI-NOR. > >> > >> It will not be possible to use SPI NAND directly from the BootROM, > >> but you can just load the bootloader from a different device like SPI > >> NOR and then fetch the rest of the system (Kernel, rootfs, etc.) from > >> a SPI NAND device. Actually that's exactly the use case, that led to > >> the development of the SPI MEM layer and the migration of the QSPI driver. > >> > >>> > >>>> With your version this wouldn't be the case if you connect a flash > >>>> that is bigger than the memory map for example. > >>> > >>> I agree such use case can be valid for Linux but in case of Uboot, I > >>> believe > >> access to flash size greater than 256M will not be required. > >>> If in case there is a requirement, there is another region in CCSR > >>> space to > >> map flash memories up to 4G. > >>> Random crashes can be avoided by adding these changes. Please let us > >>> know > >> your views as well. > >> > >> We don't even need to consider these cases, if we would just stick to > >> the SPI MEM API and use it as intended. Apart from some possible > >> performance penalty (that shouldn't matter too much and could be > >> resolved by implementing the direct mapping API as in Linux), I can't > >> see the reason for not doing so. [1] https://patchwork.ozlabs.org/patch/1207610/