Hi Frieder, > -----Original Message----- > From: Schrempf Frieder <frieder.schre...@kontron.de> > Sent: Wednesday, December 11, 2019 6:56 PM > To: Kuldeep Singh <kuldeep.si...@nxp.com>; u-boot@lists.denx.de; > ja...@amarulasolutions.com > Cc: Priyanka Jain <priyanka.j...@nxp.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 > > Hi Kuldeep, > > On 11.12.19 13:23, Kuldeep Singh wrote: > > Hi Frieder, > [...] > >>>>>>> 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. > > Ok, good. > > > > > 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. > > SYS_RELOC_GD_ENV_ADDR was only introduced very recently and it seems > like it should be enabled for your boards (see [1]) when using something more > recent than 8d8ee47e03ef. > > My guess would be that you're missing the > "CONFIG_SYS_RELOC_GD_ENV_ADDR=y" because of some mistake while > rebasing or merging .
I checked and found that Tom had made changes in all LS1012A variants except LS1012ARDB. I will make the required changes for the same. > > > > > 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. > > This is usually a problem if the addressing mode for the SPI NOR is incorrect. > When using 2-bytes addresses, only the first 16MiB of the flash can be > accessed. For SPI NOR flashes with sizes bigger than 16MiB, 3-byte mode is > mandatory to access areas above 16MiB. > > What's the manufacturer and type of the SPI flash you are using? > Also please try to test on latest master with all the latest changes for MTD, > etc. Yes, I have rebased the patches and read functionality is now working. Actually, I had SPI_FLASH_BAR config enabled by mistake and that's why run into the bug. Thanks for providing the info. I will send v2 version of series after incorporating all the changes. Thanks Kuldeep > > Thanks, > Frieder > > [1] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.de > nx.de%2Fu-boot%2Fu- > boot%2Fcommit%2F8d8ee47e03ef23b0d0e842ea455a30bf0d2023b9&dat > a=02%7C01%7Ckuldeep.singh%40nxp.com%7C1764cecf960246f8a66808d77e3 > db32b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637116675774 > 853728&sdata=spAy%2B02oTno6IdUovwyZzRRqY5%2FqZJtg%2FLCo%2BU > 9TiLI%3D&reserved=0 > > > > > --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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwo > rk.ozlabs.org%2Fpatch%2F1207610%2F&data=02%7C01%7Ckuldeep.singh > %40nxp.com%7C1764cecf960246f8a66808d77e3db32b%7C686ea1d3bc2b4c6f > a92cd99c5c301635%7C0%7C0%7C637116675774853728&sdata=RKXrWE > sYgLFfJAACFSa%2BPFZ%2FzXTEH8SUDW6dv3prkWM%3D&reserved=0 > >