| Message ID | 20181204122659.14720-1-vigneshr@ti.com |
|---|---|
| Headers | show |
| Series | SF: Migrate to Linux SPI NOR framework | expand |
On Tue, 4 Dec 2018 17:56:48 +0530 Vignesh R <vigneshr@ti.com> wrote: > U-Boot SPI NOR support (sf layer) is quite outdated as it does not > support 4 byte addressing opcodes, SFDP table parsing and different types of > quad mode enable sequences. Many newer flashes no longer support BANK > registers used by sf layer to a access >16MB space. > Also, many SPI controllers have special MMIO interfaces which provide > accelerated read/write access but require knowledge of flash parameters > to make use of it. Recent spi-mem layer provides a way to support such > flashes but sf layer isn't using that. > This patch series syncs SPI NOR framework from Linux v4.19. It also adds > spi-mem support on top. > So, we gain 4byte addressing support and SFDP support. This makes > migrating to U-Boot MTD framework easier. Glad to see that happen sooner than I had expected. Looks like the patch series is in a good shape already, and I'm happy to see the u-boot spi-nor layer being based on the Linux one. Good job, and thanks again for working on that! Boris
Am 04.12.2018 um 13:55 schrieb Boris Brezillon: > On Tue, 4 Dec 2018 17:56:48 +0530 > Vignesh R <vigneshr@ti.com> wrote: > >> U-Boot SPI NOR support (sf layer) is quite outdated as it does not >> support 4 byte addressing opcodes, SFDP table parsing and different types of >> quad mode enable sequences. Many newer flashes no longer support BANK >> registers used by sf layer to a access >16MB space. >> Also, many SPI controllers have special MMIO interfaces which provide >> accelerated read/write access but require knowledge of flash parameters >> to make use of it. Recent spi-mem layer provides a way to support such >> flashes but sf layer isn't using that. >> This patch series syncs SPI NOR framework from Linux v4.19. It also adds >> spi-mem support on top. >> So, we gain 4byte addressing support and SFDP support. This makes >> migrating to U-Boot MTD framework easier. > > Glad to see that happen sooner than I had expected. Looks like the > patch series is in a good shape already, and I'm happy to see the > u-boot spi-nor layer being based on the Linux one. > > Good job, and thanks again for working on that! Right, I do appreciate this! I did some compilation tests first and I'm happy to say that with the SPL_SPI_FLASH_TINY option enabled, my SPL is about 1900 byte smaller than before :-) However, my socfpga socrates board does not boot. I'll have to investigate why. I just applied this series and compiled for socfpga_socrates_defconfig. Is there anything else I should have changed to make it work? Regards, Simon
On 05/12/18 1:41 AM, Simon Goldschmidt wrote: > Am 04.12.2018 um 13:55 schrieb Boris Brezillon: >> On Tue, 4 Dec 2018 17:56:48 +0530 >> Vignesh R <vigneshr@ti.com> wrote: >> >>> U-Boot SPI NOR support (sf layer) is quite outdated as it does not >>> support 4 byte addressing opcodes, SFDP table parsing and different types of >>> quad mode enable sequences. Many newer flashes no longer support BANK >>> registers used by sf layer to a access >16MB space. >>> Also, many SPI controllers have special MMIO interfaces which provide >>> accelerated read/write access but require knowledge of flash parameters >>> to make use of it. Recent spi-mem layer provides a way to support such >>> flashes but sf layer isn't using that. >>> This patch series syncs SPI NOR framework from Linux v4.19. It also adds >>> spi-mem support on top. >>> So, we gain 4byte addressing support and SFDP support. This makes >>> migrating to U-Boot MTD framework easier. >> >> Glad to see that happen sooner than I had expected. Looks like the >> patch series is in a good shape already, and I'm happy to see the >> u-boot spi-nor layer being based on the Linux one. >> >> Good job, and thanks again for working on that! > > Right, I do appreciate this! > Thanks! > I did some compilation tests first and I'm happy to say that with the > SPL_SPI_FLASH_TINY option enabled, my SPL is about 1900 byte smaller > than before :-) > > However, my socfpga socrates board does not boot. I'll have to > investigate why. I just applied this series and compiled for > socfpga_socrates_defconfig. Is there anything else I should have changed > to make it work? > Oops, that's unfortunate. Just to be sure, does SPL fail to come up or SPL fails to load U-Boot from flash? Is this with SPL_SPI_FLASH_TINY enabled? Could you enable debug prints at the end of spi_mem_exec_op() in drivers/spi/spi-mem.c to see what commands are sent and their responses? I have TI EVM with Cadence QSPI(like SoCFPGA) but with a Spansion flash and that seems to work fine with both full and tiny stack.
On Wed, Dec 5, 2018 at 7:51 AM Vignesh R <vigneshr@ti.com> wrote: > > On 05/12/18 1:41 AM, Simon Goldschmidt wrote: > > Am 04.12.2018 um 13:55 schrieb Boris Brezillon: > >> On Tue, 4 Dec 2018 17:56:48 +0530 > >> Vignesh R <vigneshr@ti.com> wrote: > >> > >>> U-Boot SPI NOR support (sf layer) is quite outdated as it does not > >>> support 4 byte addressing opcodes, SFDP table parsing and different types of > >>> quad mode enable sequences. Many newer flashes no longer support BANK > >>> registers used by sf layer to a access >16MB space. > >>> Also, many SPI controllers have special MMIO interfaces which provide > >>> accelerated read/write access but require knowledge of flash parameters > >>> to make use of it. Recent spi-mem layer provides a way to support such > >>> flashes but sf layer isn't using that. > >>> This patch series syncs SPI NOR framework from Linux v4.19. It also adds > >>> spi-mem support on top. > >>> So, we gain 4byte addressing support and SFDP support. This makes > >>> migrating to U-Boot MTD framework easier. > >> > >> Glad to see that happen sooner than I had expected. Looks like the > >> patch series is in a good shape already, and I'm happy to see the > >> u-boot spi-nor layer being based on the Linux one. > > >> > >> Good job, and thanks again for working on that! > > > > Right, I do appreciate this! > > > > Thanks! > > > I did some compilation tests first and I'm happy to say that with the > > SPL_SPI_FLASH_TINY option enabled, my SPL is about 1900 byte smaller > > than before :-) > > > > However, my socfpga socrates board does not boot. I'll have to > > investigate why. I just applied this series and compiled for > > socfpga_socrates_defconfig. Is there anything else I should have changed > > to make it work? > > > > Oops, that's unfortunate. > Just to be sure, does SPL fail to come up or SPL fails to load U-Boot > from flash? Is this with SPL_SPI_FLASH_TINY enabled? I tried both TINY and standard. Both are failing to load U-Boot (I get the standard error message that loading from SPI flash failed). > Could you enable debug prints at the end of spi_mem_exec_op() in > drivers/spi/spi-mem.c to see what commands are sent and their responses? OK, I'll do that. > I have TI EVM with Cadence QSPI(like SoCFPGA) but with a Spansion flash > and that seems to work fine with both full and tiny stack. That's why I thought it could somehow be defconfig-related...? Regards, Simon
Am 05.12.2018 um 07:55 schrieb Simon Goldschmidt: > On Wed, Dec 5, 2018 at 7:51 AM Vignesh R <vigneshr@ti.com> wrote: >> >> On 05/12/18 1:41 AM, Simon Goldschmidt wrote: >>> Am 04.12.2018 um 13:55 schrieb Boris Brezillon: >>>> On Tue, 4 Dec 2018 17:56:48 +0530 >>>> Vignesh R <vigneshr@ti.com> wrote: >>>> >>>>> U-Boot SPI NOR support (sf layer) is quite outdated as it does not >>>>> support 4 byte addressing opcodes, SFDP table parsing and different types of >>>>> quad mode enable sequences. Many newer flashes no longer support BANK >>>>> registers used by sf layer to a access >16MB space. >>>>> Also, many SPI controllers have special MMIO interfaces which provide >>>>> accelerated read/write access but require knowledge of flash parameters >>>>> to make use of it. Recent spi-mem layer provides a way to support such >>>>> flashes but sf layer isn't using that. >>>>> This patch series syncs SPI NOR framework from Linux v4.19. It also adds >>>>> spi-mem support on top. >>>>> So, we gain 4byte addressing support and SFDP support. This makes >>>>> migrating to U-Boot MTD framework easier. >>>> >>>> Glad to see that happen sooner than I had expected. Looks like the >>>> patch series is in a good shape already, and I'm happy to see the >>>> u-boot spi-nor layer being based on the Linux one. >> >>>> >>>> Good job, and thanks again for working on that! >>> >>> Right, I do appreciate this! >>> >> >> Thanks! >> >>> I did some compilation tests first and I'm happy to say that with the >>> SPL_SPI_FLASH_TINY option enabled, my SPL is about 1900 byte smaller >>> than before :-) >>> >>> However, my socfpga socrates board does not boot. I'll have to >>> investigate why. I just applied this series and compiled for >>> socfpga_socrates_defconfig. Is there anything else I should have changed >>> to make it work? >>> >> >> Oops, that's unfortunate. >> Just to be sure, does SPL fail to come up or SPL fails to load U-Boot >> from flash? Is this with SPL_SPI_FLASH_TINY enabled? > > I tried both TINY and standard. Both are failing to load U-Boot (I get > the standard error message that loading from SPI flash failed). > >> Could you enable debug prints at the end of spi_mem_exec_op() in >> drivers/spi/spi-mem.c to see what commands are sent and their responses? > > OK, I'll do that. > >> I have TI EVM with Cadence QSPI(like SoCFPGA) but with a Spansion flash >> and that seems to work fine with both full and tiny stack. OK, so I enabled some debugging and the first issue is that U-Boot has SNOR_MFR_MICRON and SNOR_MFR_ST while Linux only has SNOR_MFR_MICRON but defines it to CFI_MFR_ST (0x20, U-Boot ends up with 0x2c). While this might be correct, it leads to my n25q256a not being handled as "micron-like" and the code falls through to using Spansion-like opcode (0x17) that my chip does not understand. However, after changing this (and using opcode 0xb7 to bring the chip into 4 byte mode, including 0x06 for write-enable), it still does not work as it uses the 'READ' opcode 0x03 instead of the 'FAST READ' opcode 0x0b that U-Boot used until now. The problem here might be that 'READ' cannot do Quad-SPI (4-4-4) and I'm not sure the Cadence driver can handle Extended-SPI (1-1-1) that the 'READ' command is limited to... Using SFDP would probably help, but that doesn't compile (as I wrote in the other thread). On the other hand, enabling SFDP will increase the text/rodata size for SPL. We might need to remove the non-SFDP and write code as a counter measure to prevent increasing code size. Regards, Simon
Hi Simon, On 06/12/18 2:15 AM, Simon Goldschmidt wrote: > Am 05.12.2018 um 07:55 schrieb Simon Goldschmidt: >> On Wed, Dec 5, 2018 at 7:51 AM Vignesh R <vigneshr@ti.com> wrote: [...] >>>> I did some compilation tests first and I'm happy to say that with the >>>> SPL_SPI_FLASH_TINY option enabled, my SPL is about 1900 byte smaller >>>> than before :-) >>>> >>>> However, my socfpga socrates board does not boot. I'll have to >>>> investigate why. I just applied this series and compiled for >>>> socfpga_socrates_defconfig. Is there anything else I should have changed >>>> to make it work? >>>> >>> >>> Oops, that's unfortunate. >>> Just to be sure, does SPL fail to come up or SPL fails to load U-Boot >>> from flash? Is this with SPL_SPI_FLASH_TINY enabled? >> >> I tried both TINY and standard. Both are failing to load U-Boot (I get >> the standard error message that loading from SPI flash failed). >> >>> Could you enable debug prints at the end of spi_mem_exec_op() in >>> drivers/spi/spi-mem.c to see what commands are sent and their responses? >> >> OK, I'll do that. >> >>> I have TI EVM with Cadence QSPI(like SoCFPGA) but with a Spansion flash >>> and that seems to work fine with both full and tiny stack. > > OK, so I enabled some debugging and the first issue is that U-Boot has > SNOR_MFR_MICRON and SNOR_MFR_ST while Linux only has SNOR_MFR_MICRON but > defines it to CFI_MFR_ST (0x20, U-Boot ends up with 0x2c). While this > might be correct, it leads to my n25q256a not being handled as > "micron-like" and the code falls through to using Spansion-like opcode > (0x17) that my chip does not understand. > Oops, sorry, the micron chip I had has CFI MFR id of 0x2c, so I overlooked this. I have updated this now. > However, after changing this (and using opcode 0xb7 to bring the chip > into 4 byte mode, including 0x06 for write-enable), it still does not > work as it uses the 'READ' opcode 0x03 instead of the 'FAST READ' opcode > 0x0b that U-Boot used until now. The problem here might be that 'READ' > cannot do Quad-SPI (4-4-4) and I'm not sure the Cadence driver can > handle Extended-SPI (1-1-1) that the 'READ' command is limited to... > I had accidentally dropped Fast Read from SPI NOR layer and have sync'd it up now. I see that in socfpga dts, flash does not contain: spi-rx-bus-width = <4>; Therefore SPI NOR layer falls back to READ (even though its a QSPI flash). But nevertheless with updated branch[1] you should see falling back to FAST READ with Cadence QSPI on your board (and 1-1-4, if rx bus width is 4). > Using SFDP would probably help, but that doesn't compile (as I wrote in > the other thread). > Sorry for that. I have fixed that up and tested SFDP parsing logic with a Spansion flash. Note that Cadence QSPI wont support SFDP parsing as driver enables Quad mode for all reads and SFDP is only 1-1-1. > On the other hand, enabling SFDP will increase the text/rodata size for > SPL. We might need to remove the non-SFDP and write code as a counter > measure to prevent increasing code size. > That's not an option as Flash devices (even though have same JEDEC ID) manufactured before JESD216 std dont have SFDP populated. So, we will need to have both non-SFDP and SFDP support with option to disable SFDP (for size and boot time optimizations). Thanks for all the debugging! It would be great if you could try below branch and let me know if it fixes all the issues. If yes, I will remove RFC and post new series. [1] https://github.com/r-vignesh/u-boot.git spi-nor-mig-patch-v1
On Thu, Dec 6, 2018 at 2:45 PM Vignesh R <vigneshr@ti.com> wrote: > > Hi Simon, > > On 06/12/18 2:15 AM, Simon Goldschmidt wrote: > > Am 05.12.2018 um 07:55 schrieb Simon Goldschmidt: > >> On Wed, Dec 5, 2018 at 7:51 AM Vignesh R <vigneshr@ti.com> wrote: > [...] > >>>> I did some compilation tests first and I'm happy to say that with the > >>>> SPL_SPI_FLASH_TINY option enabled, my SPL is about 1900 byte smaller > >>>> than before :-) > >>>> > >>>> However, my socfpga socrates board does not boot. I'll have to > >>>> investigate why. I just applied this series and compiled for > >>>> socfpga_socrates_defconfig. Is there anything else I should have changed > >>>> to make it work? > >>>> > >>> > >>> Oops, that's unfortunate. > >>> Just to be sure, does SPL fail to come up or SPL fails to load U-Boot > >>> from flash? Is this with SPL_SPI_FLASH_TINY enabled? > >> > >> I tried both TINY and standard. Both are failing to load U-Boot (I get > >> the standard error message that loading from SPI flash failed). > >> > >>> Could you enable debug prints at the end of spi_mem_exec_op() in > >>> drivers/spi/spi-mem.c to see what commands are sent and their responses? > >> > >> OK, I'll do that. > >> > >>> I have TI EVM with Cadence QSPI(like SoCFPGA) but with a Spansion flash > >>> and that seems to work fine with both full and tiny stack. > > > > OK, so I enabled some debugging and the first issue is that U-Boot has > > SNOR_MFR_MICRON and SNOR_MFR_ST while Linux only has SNOR_MFR_MICRON but > > defines it to CFI_MFR_ST (0x20, U-Boot ends up with 0x2c). While this > > might be correct, it leads to my n25q256a not being handled as > > "micron-like" and the code falls through to using Spansion-like opcode > > (0x17) that my chip does not understand. > > > > Oops, sorry, the micron chip I had has CFI MFR id of 0x2c, so I > overlooked this. I have updated this now. > > > However, after changing this (and using opcode 0xb7 to bring the chip > > into 4 byte mode, including 0x06 for write-enable), it still does not > > work as it uses the 'READ' opcode 0x03 instead of the 'FAST READ' opcode > > 0x0b that U-Boot used until now. The problem here might be that 'READ' > > cannot do Quad-SPI (4-4-4) and I'm not sure the Cadence driver can > > handle Extended-SPI (1-1-1) that the 'READ' command is limited to... > > > > I had accidentally dropped Fast Read from SPI NOR layer and have sync'd > it up now. I see that in socfpga dts, flash does not contain: > spi-rx-bus-width = <4>; We can add that if it helps. I'll try to update the upstream Linux dts anyway to add "jeded,spi-nor" compatible once Neil's series is applied. > Therefore SPI NOR layer falls back to READ (even though its a QSPI > flash). But nevertheless with updated branch[1] you should see falling > back to FAST READ with Cadence QSPI on your board (and 1-1-4, if rx bus > width is 4). > > > Using SFDP would probably help, but that doesn't compile (as I wrote in > > the other thread). > > > > Sorry for that. I have fixed that up and tested SFDP parsing logic with > a Spansion flash. Note that Cadence QSPI wont support SFDP parsing as > driver enables Quad mode for all reads and SFDP is only 1-1-1. Oh, I haven't really looked into that yet. That's sad. > > > On the other hand, enabling SFDP will increase the text/rodata size for > > SPL. We might need to remove the non-SFDP and write code as a counter > > measure to prevent increasing code size. > > > > That's not an option as Flash devices (even though have same JEDEC ID) > manufactured before JESD216 std dont have SFDP populated. So, we will > need to have both non-SFDP and SFDP support with option to disable SFDP > (for size and boot time optimizations). Well, my idea was if you'd know that all the flash chips on your board supported it, you'd be safe. It would only be a Kconfig option to reduce binary size (get rid of the flash IDs table). But if SFDP doesn't work for QSPI anyway, that's not the best idea, I guess... > Thanks for all the debugging! It would be great if you could try below > branch and let me know if it fixes all the issues. > If yes, I will remove RFC and post new series. Sure, I'll try to find the time soon. Regards, Simon > > [1] https://github.com/r-vignesh/u-boot.git spi-nor-mig-patch-v1 > > -- > Regards > Vignesh
Am 06.12.2018 um 14:54 schrieb Simon Goldschmidt: > On Thu, Dec 6, 2018 at 2:45 PM Vignesh R <vigneshr@ti.com> wrote: >> >> Hi Simon, >> >> On 06/12/18 2:15 AM, Simon Goldschmidt wrote: >>> Am 05.12.2018 um 07:55 schrieb Simon Goldschmidt: >>>> On Wed, Dec 5, 2018 at 7:51 AM Vignesh R <vigneshr@ti.com> wrote: >> [...] >>>>>> I did some compilation tests first and I'm happy to say that with the >>>>>> SPL_SPI_FLASH_TINY option enabled, my SPL is about 1900 byte smaller >>>>>> than before :-) >>>>>> >>>>>> However, my socfpga socrates board does not boot. I'll have to >>>>>> investigate why. I just applied this series and compiled for >>>>>> socfpga_socrates_defconfig. Is there anything else I should have changed >>>>>> to make it work? >>>>>> >>>>> >>>>> Oops, that's unfortunate. >>>>> Just to be sure, does SPL fail to come up or SPL fails to load U-Boot >>>>> from flash? Is this with SPL_SPI_FLASH_TINY enabled? >>>> >>>> I tried both TINY and standard. Both are failing to load U-Boot (I get >>>> the standard error message that loading from SPI flash failed). >>>> >>>>> Could you enable debug prints at the end of spi_mem_exec_op() in >>>>> drivers/spi/spi-mem.c to see what commands are sent and their responses? >>>> >>>> OK, I'll do that. >>>> >>>>> I have TI EVM with Cadence QSPI(like SoCFPGA) but with a Spansion flash >>>>> and that seems to work fine with both full and tiny stack. >>> >>> OK, so I enabled some debugging and the first issue is that U-Boot has >>> SNOR_MFR_MICRON and SNOR_MFR_ST while Linux only has SNOR_MFR_MICRON but >>> defines it to CFI_MFR_ST (0x20, U-Boot ends up with 0x2c). While this >>> might be correct, it leads to my n25q256a not being handled as >>> "micron-like" and the code falls through to using Spansion-like opcode >>> (0x17) that my chip does not understand. >>> >> >> Oops, sorry, the micron chip I had has CFI MFR id of 0x2c, so I >> overlooked this. I have updated this now. >> >>> However, after changing this (and using opcode 0xb7 to bring the chip >>> into 4 byte mode, including 0x06 for write-enable), it still does not >>> work as it uses the 'READ' opcode 0x03 instead of the 'FAST READ' opcode >>> 0x0b that U-Boot used until now. The problem here might be that 'READ' >>> cannot do Quad-SPI (4-4-4) and I'm not sure the Cadence driver can >>> handle Extended-SPI (1-1-1) that the 'READ' command is limited to... >>> >> >> I had accidentally dropped Fast Read from SPI NOR layer and have sync'd >> it up now. I see that in socfpga dts, flash does not contain: >> spi-rx-bus-width = <4>; > > We can add that if it helps. I'll try to update the upstream Linux dts > anyway to add "jeded,spi-nor" compatible once Neil's series is > applied. > >> Therefore SPI NOR layer falls back to READ (even though its a QSPI >> flash). But nevertheless with updated branch[1] you should see falling >> back to FAST READ with Cadence QSPI on your board (and 1-1-4, if rx bus >> width is 4). >> >>> Using SFDP would probably help, but that doesn't compile (as I wrote in >>> the other thread). >>> >> >> Sorry for that. I have fixed that up and tested SFDP parsing logic with >> a Spansion flash. Note that Cadence QSPI wont support SFDP parsing as >> driver enables Quad mode for all reads and SFDP is only 1-1-1. > > Oh, I haven't really looked into that yet. That's sad. > >> >>> On the other hand, enabling SFDP will increase the text/rodata size for >>> SPL. We might need to remove the non-SFDP and write code as a counter >>> measure to prevent increasing code size. >>> >> >> That's not an option as Flash devices (even though have same JEDEC ID) >> manufactured before JESD216 std dont have SFDP populated. So, we will >> need to have both non-SFDP and SFDP support with option to disable SFDP >> (for size and boot time optimizations). > > Well, my idea was if you'd know that all the flash chips on your board > supported it, you'd be safe. It would only be a Kconfig option to > reduce binary size (get rid of the flash IDs table). > > But if SFDP doesn't work for QSPI anyway, that's not the best idea, I guess... > >> Thanks for all the debugging! It would be great if you could try below >> branch and let me know if it fixes all the issues. >> If yes, I will remove RFC and post new series. > > Sure, I'll try to find the time soon. Had a quick test with both standard and tiny in SPL and both configs now work on my board. I haven't tested SFDP as you said it does not works on Cadence QSPI. (I tested compiling it though, and that works now.) Oh, and I think it's important to say that with your spi-nor-tiny code, SPL code size is reduced by 1980 bytes for my config (socfpga_socrates_defconfig)! Regards, Simon > > Regards, > Simon > >> >> [1] https://github.com/r-vignesh/u-boot.git spi-nor-mig-patch-v1 >> >> -- >> Regards >> Vignesh
On Tue, Dec 4, 2018 at 5:56 PM Vignesh R <vigneshr@ti.com> wrote: > > U-Boot SPI NOR support (sf layer) is quite outdated as it does not > support 4 byte addressing opcodes, SFDP table parsing and different types of > quad mode enable sequences. Many newer flashes no longer support BANK > registers used by sf layer to a access >16MB space. > Also, many SPI controllers have special MMIO interfaces which provide > accelerated read/write access but require knowledge of flash parameters > to make use of it. Recent spi-mem layer provides a way to support such > flashes but sf layer isn't using that. > This patch series syncs SPI NOR framework from Linux v4.19. It also adds > spi-mem support on top. > So, we gain 4byte addressing support and SFDP support. This makes > migrating to U-Boot MTD framework easier. We(someone) has proposed this sync before, but we(at-least I) rely on implementing via DM not direct sync to Linux. ofcourse other subsystems might have doing this but I literally don't propose to do that, since it may fire the u-boot implementation in future. If you really get things up further, try to check this DM based spi-nor here[1] and lets discuss on u-boot point-of-view. I've paused this because of non-dm code, but I'm thinking we even re-change this to fit MTD driver-model (this is my TODO, once spi dm migration done). [1] http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/next-working
On 06/12/18 10:06 PM, Simon Goldschmidt wrote: > Am 06.12.2018 um 14:54 schrieb Simon Goldschmidt: >> On Thu, Dec 6, 2018 at 2:45 PM Vignesh R <vigneshr@ti.com> wrote: >>> >>> Hi Simon, >>> >>> On 06/12/18 2:15 AM, Simon Goldschmidt wrote: >>>> Am 05.12.2018 um 07:55 schrieb Simon Goldschmidt: >>>>> On Wed, Dec 5, 2018 at 7:51 AM Vignesh R <vigneshr@ti.com> wrote: >>> [...] >>>>>>> I did some compilation tests first and I'm happy to say that with the >>>>>>> SPL_SPI_FLASH_TINY option enabled, my SPL is about 1900 byte smaller >>>>>>> than before :-) >>>>>>> >>>>>>> However, my socfpga socrates board does not boot. I'll have to >>>>>>> investigate why. I just applied this series and compiled for >>>>>>> socfpga_socrates_defconfig. Is there anything else I should have changed >>>>>>> to make it work? >>>>>>> >>>>>> >>>>>> Oops, that's unfortunate. >>>>>> Just to be sure, does SPL fail to come up or SPL fails to load U-Boot >>>>>> from flash? Is this with SPL_SPI_FLASH_TINY enabled? >>>>> >>>>> I tried both TINY and standard. Both are failing to load U-Boot (I get >>>>> the standard error message that loading from SPI flash failed). >>>>> >>>>>> Could you enable debug prints at the end of spi_mem_exec_op() in >>>>>> drivers/spi/spi-mem.c to see what commands are sent and their responses? >>>>> >>>>> OK, I'll do that. >>>>> >>>>>> I have TI EVM with Cadence QSPI(like SoCFPGA) but with a Spansion flash >>>>>> and that seems to work fine with both full and tiny stack. >>>> >>>> OK, so I enabled some debugging and the first issue is that U-Boot has >>>> SNOR_MFR_MICRON and SNOR_MFR_ST while Linux only has SNOR_MFR_MICRON but >>>> defines it to CFI_MFR_ST (0x20, U-Boot ends up with 0x2c). While this >>>> might be correct, it leads to my n25q256a not being handled as >>>> "micron-like" and the code falls through to using Spansion-like opcode >>>> (0x17) that my chip does not understand. >>>> >>> >>> Oops, sorry, the micron chip I had has CFI MFR id of 0x2c, so I >>> overlooked this. I have updated this now. >>> >>>> However, after changing this (and using opcode 0xb7 to bring the chip >>>> into 4 byte mode, including 0x06 for write-enable), it still does not >>>> work as it uses the 'READ' opcode 0x03 instead of the 'FAST READ' opcode >>>> 0x0b that U-Boot used until now. The problem here might be that 'READ' >>>> cannot do Quad-SPI (4-4-4) and I'm not sure the Cadence driver can >>>> handle Extended-SPI (1-1-1) that the 'READ' command is limited to... >>>> >>> >>> I had accidentally dropped Fast Read from SPI NOR layer and have sync'd >>> it up now. I see that in socfpga dts, flash does not contain: >>> spi-rx-bus-width = <4>; >> >> We can add that if it helps. I'll try to update the upstream Linux dts >> anyway to add "jeded,spi-nor" compatible once Neil's series is >> applied. >> >>> Therefore SPI NOR layer falls back to READ (even though its a QSPI >>> flash). But nevertheless with updated branch[1] you should see falling >>> back to FAST READ with Cadence QSPI on your board (and 1-1-4, if rx bus >>> width is 4). >>> >>>> Using SFDP would probably help, but that doesn't compile (as I wrote in >>>> the other thread). >>>> >>> >>> Sorry for that. I have fixed that up and tested SFDP parsing logic with >>> a Spansion flash. Note that Cadence QSPI wont support SFDP parsing as >>> driver enables Quad mode for all reads and SFDP is only 1-1-1. >> >> Oh, I haven't really looked into that yet. That's sad. >> Yes, thats one of the motivation for this series. I wanted to use same driver to support Cadence Octal SPI IP but this simply wasn't possible due disconnect b/w SF and SPI layer. I plan to fix Cadence QSPI by moving to spi-mem layer once SF layer is in place >>> >>>> On the other hand, enabling SFDP will increase the text/rodata size for >>>> SPL. We might need to remove the non-SFDP and write code as a counter >>>> measure to prevent increasing code size. >>>> >>> >>> That's not an option as Flash devices (even though have same JEDEC ID) >>> manufactured before JESD216 std dont have SFDP populated. So, we will >>> need to have both non-SFDP and SFDP support with option to disable SFDP >>> (for size and boot time optimizations). >> >> Well, my idea was if you'd know that all the flash chips on your board >> supported it, you'd be safe. It would only be a Kconfig option to >> reduce binary size (get rid of the flash IDs table). >>>> But if SFDP doesn't work for QSPI anyway, that's not the best idea, I guess... >> Yeah, current U-Boot SPI layer does not allow SPI Flash layer to communicate bus width to SPI controller drivers. Either drivers capable of supporting Quad mode should move to spi-mem framework or spi_xfer() interface needs to be expanded. >>> Thanks for all the debugging! It would be great if you could try below >>> branch and let me know if it fixes all the issues. >>> If yes, I will remove RFC and post new series. >> >> Sure, I'll try to find the time soon. > > Had a quick test with both standard and tiny in SPL and both configs now > work on my board. I haven't tested SFDP as you said it does not works on > Cadence QSPI. (I tested compiling it though, and that works now.) > I used below hack to test SFDP with Cadence QSPI: diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index a8af35203035..9dbf9aa7d20c 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -559,7 +559,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, /* Configure the opcode */ rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB; - if (rx_width & SPI_RX_QUAD) + if (rx_width & SPI_RX_QUAD && (cmdbuf[0] != 0x5a)) /* Instruction and address at DQ0, data at DQ0-3. */ rd_reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB; > Oh, and I think it's important to say that with your spi-nor-tiny code, > SPL code size is reduced by 1980 bytes for my config > (socfpga_socrates_defconfig)! > Thats good to know. > Regards, > Simon > >> >> Regards, >> Simon >> >>> >>> [1] https://github.com/r-vignesh/u-boot.git spi-nor-mig-patch-v1 >>> >>> -- >>> Regards >>> Vignesh >
On Thu, Dec 06, 2018 at 10:44:00PM +0530, Jagan Teki wrote: > On Tue, Dec 4, 2018 at 5:56 PM Vignesh R <vigneshr@ti.com> wrote: > > > > U-Boot SPI NOR support (sf layer) is quite outdated as it does not > > support 4 byte addressing opcodes, SFDP table parsing and different types of > > quad mode enable sequences. Many newer flashes no longer support BANK > > registers used by sf layer to a access >16MB space. > > Also, many SPI controllers have special MMIO interfaces which provide > > accelerated read/write access but require knowledge of flash parameters > > to make use of it. Recent spi-mem layer provides a way to support such > > flashes but sf layer isn't using that. > > This patch series syncs SPI NOR framework from Linux v4.19. It also adds > > spi-mem support on top. > > So, we gain 4byte addressing support and SFDP support. This makes > > migrating to U-Boot MTD framework easier. > > We(someone) has proposed this sync before, but we(at-least I) rely on > implementing via DM not direct sync to Linux. ofcourse other > subsystems might have doing this but I literally don't propose to do > that, since it may fire the u-boot implementation in future. > > If you really get things up further, try to check this DM based > spi-nor here[1] and lets discuss on u-boot point-of-view. I've paused > this because of non-dm code, but I'm thinking we even re-change this > to fit MTD driver-model (this is my TODO, once spi dm migration done). We need to make sure things overall are a good fit and I know we've had some back and forth on this, but, "sync with Linux subsystem and make it work cleanly (as possible) in U-Boot" is the path I think we need to take and I like what Vignesh has done and to repeat myself from earlier, I want to see this series (or rather, it's overall evolution) come in for v2019.04. Progress over time on top of those changes is good but as you note, your series is from a year ago and that was a pretty long evolution in that direction too. We need to get this solved and since we have motivated developers right now, I want to see this resolved. Thanks!
Hi Jagan, On 06-Dec-18 10:44 PM, Jagan Teki wrote: > On Tue, Dec 4, 2018 at 5:56 PM Vignesh R <vigneshr@ti.com> wrote: >> >> U-Boot SPI NOR support (sf layer) is quite outdated as it does not >> support 4 byte addressing opcodes, SFDP table parsing and different types of >> quad mode enable sequences. Many newer flashes no longer support BANK >> registers used by sf layer to a access >16MB space. >> Also, many SPI controllers have special MMIO interfaces which provide >> accelerated read/write access but require knowledge of flash parameters >> to make use of it. Recent spi-mem layer provides a way to support such >> flashes but sf layer isn't using that. >> This patch series syncs SPI NOR framework from Linux v4.19. It also adds >> spi-mem support on top. >> So, we gain 4byte addressing support and SFDP support. This makes >> migrating to U-Boot MTD framework easier. > > We(someone) has proposed this sync before, but we(at-least I) rely on > implementing via DM not direct sync to Linux. As I said in my cover letter, U-Boot sf layer is unable to support newer flashes mainly due to lack of 4 byte addressing and proper support for MMIO capable SPI controllers. My idea of fixing this is to borrow _features_ from Linux SPI NOR "as is". All that's needed is stateless 4 byte addressing, SFDP parsing(optionally), Quad/Octal support and spi-mem like abstraction for MMIO capable Controllers. I see no point in re-coding them from ground up. Could you be more specific on what you would like to see here in DM way? I have no issues in adapting this code to any framework here in U-Boot. Linux has driver model and SPI NOR subsystem is a framework and therefore any code ported from Linux will inherently have those abstractions. The only difference I see wrt your code in branch below vs this series is SPI-NOR uclass. This can be easily achieved by moving nor->ops out of struct spi_nor into uclass abstraction. Upstream Linux is anyways merging m25p80 and spi-nor so I did not see a need for SPI NOR uclass. I am okay to change that if you insist on having it. > ofcourse other > subsystems might have doing this but I literally don't propose to do > that, since it may fire the u-boot implementation in future. > Could you explain what might be possible issues in future? I will work on it. > If you really get things up further, try to check this DM based > spi-nor here[1] and lets discuss on u-boot point-of-view. I've paused > this because of non-dm code, but I'm thinking we even re-change this > to fit MTD driver-model (this is my TODO, once spi dm migration done). > AFAICS, Non DM code is not going away anytime soon. Meanwhile, IMHO, lack of 4 byte addressing support is significantly hampering new development (as evident from number of people trying to add 4 byte addressing support.) > [1] http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/next-working > IIRC, last post of this branch was in January 2018. And looking at my inbox, its seems that 4 byte addressing was broken in that series. If you add 4 byte addressing + SFDP parsing logic + spi-mem support to above branch, I am pretty sure you spi-nor.c would look almost the same to whats there in this series. Regards Vignesh
Am 06.12.2018 um 18:39 schrieb Vignesh R: > > > On 06/12/18 10:06 PM, Simon Goldschmidt wrote: >> Am 06.12.2018 um 14:54 schrieb Simon Goldschmidt: >>> On Thu, Dec 6, 2018 at 2:45 PM Vignesh R <vigneshr@ti.com> wrote: >>>> >>>> Hi Simon, >>>> >>>> On 06/12/18 2:15 AM, Simon Goldschmidt wrote: >>>>> Am 05.12.2018 um 07:55 schrieb Simon Goldschmidt: >>>>>> On Wed, Dec 5, 2018 at 7:51 AM Vignesh R <vigneshr@ti.com> wrote: >>>> [...] >>>>>>>> I did some compilation tests first and I'm happy to say that with the >>>>>>>> SPL_SPI_FLASH_TINY option enabled, my SPL is about 1900 byte smaller >>>>>>>> than before :-) >>>>>>>> >>>>>>>> However, my socfpga socrates board does not boot. I'll have to >>>>>>>> investigate why. I just applied this series and compiled for >>>>>>>> socfpga_socrates_defconfig. Is there anything else I should have changed >>>>>>>> to make it work? >>>>>>>> >>>>>>> >>>>>>> Oops, that's unfortunate. >>>>>>> Just to be sure, does SPL fail to come up or SPL fails to load U-Boot >>>>>>> from flash? Is this with SPL_SPI_FLASH_TINY enabled? >>>>>> >>>>>> I tried both TINY and standard. Both are failing to load U-Boot (I get >>>>>> the standard error message that loading from SPI flash failed). >>>>>> >>>>>>> Could you enable debug prints at the end of spi_mem_exec_op() in >>>>>>> drivers/spi/spi-mem.c to see what commands are sent and their responses? >>>>>> >>>>>> OK, I'll do that. >>>>>> >>>>>>> I have TI EVM with Cadence QSPI(like SoCFPGA) but with a Spansion flash >>>>>>> and that seems to work fine with both full and tiny stack. >>>>> >>>>> OK, so I enabled some debugging and the first issue is that U-Boot has >>>>> SNOR_MFR_MICRON and SNOR_MFR_ST while Linux only has SNOR_MFR_MICRON but >>>>> defines it to CFI_MFR_ST (0x20, U-Boot ends up with 0x2c). While this >>>>> might be correct, it leads to my n25q256a not being handled as >>>>> "micron-like" and the code falls through to using Spansion-like opcode >>>>> (0x17) that my chip does not understand. >>>>> >>>> >>>> Oops, sorry, the micron chip I had has CFI MFR id of 0x2c, so I >>>> overlooked this. I have updated this now. >>>> >>>>> However, after changing this (and using opcode 0xb7 to bring the chip >>>>> into 4 byte mode, including 0x06 for write-enable), it still does not >>>>> work as it uses the 'READ' opcode 0x03 instead of the 'FAST READ' opcode >>>>> 0x0b that U-Boot used until now. The problem here might be that 'READ' >>>>> cannot do Quad-SPI (4-4-4) and I'm not sure the Cadence driver can >>>>> handle Extended-SPI (1-1-1) that the 'READ' command is limited to... >>>>> >>>> >>>> I had accidentally dropped Fast Read from SPI NOR layer and have sync'd >>>> it up now. I see that in socfpga dts, flash does not contain: >>>> spi-rx-bus-width = <4>; >>> >>> We can add that if it helps. I'll try to update the upstream Linux dts >>> anyway to add "jeded,spi-nor" compatible once Neil's series is >>> applied. >>> >>>> Therefore SPI NOR layer falls back to READ (even though its a QSPI >>>> flash). But nevertheless with updated branch[1] you should see falling >>>> back to FAST READ with Cadence QSPI on your board (and 1-1-4, if rx bus >>>> width is 4). >>>> >>>>> Using SFDP would probably help, but that doesn't compile (as I wrote in >>>>> the other thread). >>>>> >>>> >>>> Sorry for that. I have fixed that up and tested SFDP parsing logic with >>>> a Spansion flash. Note that Cadence QSPI wont support SFDP parsing as >>>> driver enables Quad mode for all reads and SFDP is only 1-1-1. >>> >>> Oh, I haven't really looked into that yet. That's sad. >>> > > Yes, thats one of the motivation for this series. I wanted to use same driver > to support Cadence Octal SPI IP but this simply wasn't possible due disconnect b/w SF and SPI layer. > I plan to fix Cadence QSPI by moving to spi-mem layer once SF layer is in place > >>>> >>>>> On the other hand, enabling SFDP will increase the text/rodata size for >>>>> SPL. We might need to remove the non-SFDP and write code as a counter >>>>> measure to prevent increasing code size. >>>>> >>>> >>>> That's not an option as Flash devices (even though have same JEDEC ID) >>>> manufactured before JESD216 std dont have SFDP populated. So, we will >>>> need to have both non-SFDP and SFDP support with option to disable SFDP >>>> (for size and boot time optimizations). >>> >>> Well, my idea was if you'd know that all the flash chips on your board >>> supported it, you'd be safe. It would only be a Kconfig option to >>> reduce binary size (get rid of the flash IDs table). >>>>> But if SFDP doesn't work for QSPI anyway, that's not the best idea, I guess... >>> > > Yeah, current U-Boot SPI layer does not allow SPI Flash layer to communicate > bus width to SPI controller drivers. Either drivers capable of supporting > Quad mode should move to spi-mem framework or spi_xfer() interface needs to be expanded. > >>>> Thanks for all the debugging! It would be great if you could try below >>>> branch and let me know if it fixes all the issues. >>>> If yes, I will remove RFC and post new series. >>> >>> Sure, I'll try to find the time soon. >> >> Had a quick test with both standard and tiny in SPL and both configs now >> work on my board. I haven't tested SFDP as you said it does not works on >> Cadence QSPI. (I tested compiling it though, and that works now.) >> After enabling DEBUG in spi-mem.c, I see that we're changing to 4-byte mode (via b7h) instead of using 4-byte opcodes. Unfortunately, this conflicts with the boot rom using the default read opcose in 3-byte mode on warm reboot. Do you plan to change this or would this be fixed by parsing SFDP? > > I used below hack to test SFDP with Cadence QSPI: Just by accident, I saw that enabling the SFDP config does not seem to break booting. But it still switches to 4-byte mode, so it probably just fails to parse and continues the non-SFDP way? > > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c > index a8af35203035..9dbf9aa7d20c 100644 > --- a/drivers/spi/cadence_qspi_apb.c > +++ b/drivers/spi/cadence_qspi_apb.c > @@ -559,7 +559,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > /* Configure the opcode */ > rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB; > > - if (rx_width & SPI_RX_QUAD) > + if (rx_width & SPI_RX_QUAD && (cmdbuf[0] != 0x5a)) But this is Spansion-specific, right? Regards, Simon > /* Instruction and address at DQ0, data at DQ0-3. */ > rd_reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB; > > > > >> Oh, and I think it's important to say that with your spi-nor-tiny code, >> SPL code size is reduced by 1980 bytes for my config >> (socfpga_socrates_defconfig)! >> > > Thats good to know. > >> Regards, >> Simon >> >>> >>> Regards, >>> Simon >>> >>>> >>>> [1] https://github.com/r-vignesh/u-boot.git spi-nor-mig-patch-v1 >>>> >>>> -- >>>> Regards >>>> Vignesh >> >
Am 06.12.2018 um 19:25 schrieb Vignesh R: > Hi Jagan, > > On 06-Dec-18 10:44 PM, Jagan Teki wrote: >> On Tue, Dec 4, 2018 at 5:56 PM Vignesh R <vigneshr@ti.com> wrote: >>> >>> U-Boot SPI NOR support (sf layer) is quite outdated as it does not >>> support 4 byte addressing opcodes, SFDP table parsing and different types of >>> quad mode enable sequences. Many newer flashes no longer support BANK >>> registers used by sf layer to a access >16MB space. >>> Also, many SPI controllers have special MMIO interfaces which provide >>> accelerated read/write access but require knowledge of flash parameters >>> to make use of it. Recent spi-mem layer provides a way to support such >>> flashes but sf layer isn't using that. >>> This patch series syncs SPI NOR framework from Linux v4.19. It also adds >>> spi-mem support on top. >>> So, we gain 4byte addressing support and SFDP support. This makes >>> migrating to U-Boot MTD framework easier. >> >> We(someone) has proposed this sync before, but we(at-least I) rely on >> implementing via DM not direct sync to Linux. > > As I said in my cover letter, U-Boot sf layer is unable to support newer > flashes mainly due to lack of 4 byte addressing and proper support for > MMIO capable SPI controllers. > My idea of fixing this is to borrow _features_ from Linux SPI NOR "as > is". All that's needed is stateless 4 byte addressing, SFDP > parsing(optionally), Quad/Octal support and spi-mem like abstraction for > MMIO capable Controllers. I see no point in re-coding them from ground up. And this is exactly why I'm testing this series: it's the only series I've seen in the last months that seems to properly handle 4-byte addressing (even if it does not fully do that for me, yet, but it's still RFC). I have a bug in my hardware that requires me to put the chip back to 3-byte mode manually up to now. I've sent a patch for that and this patch has been rejected. So now I'm testing this series to see if it allows me to run U-Boot mainline on my board without additional patches required. Simon > > Could you be more specific on what you would like to see here in DM way? > I have no issues in adapting this code to any framework here in U-Boot. > Linux has driver model and SPI NOR subsystem is a framework and > therefore any code ported from Linux will inherently have those > abstractions. The only difference I see wrt your code in branch below vs > this series is SPI-NOR uclass. This can be easily achieved by moving > nor->ops out of struct spi_nor into uclass abstraction. > Upstream Linux is anyways merging m25p80 and spi-nor so I did not see a > need for SPI NOR uclass. I am okay to change that if you insist on > having it. > > >> ofcourse other >> subsystems might have doing this but I literally don't propose to do >> that, since it may fire the u-boot implementation in future. >> > > Could you explain what might be possible issues in future? I will work > on it. > >> If you really get things up further, try to check this DM based >> spi-nor here[1] and lets discuss on u-boot point-of-view. I've paused >> this because of non-dm code, but I'm thinking we even re-change this >> to fit MTD driver-model (this is my TODO, once spi dm migration done). >> > AFAICS, Non DM code is not going away anytime soon. Meanwhile, IMHO, > lack of 4 byte addressing support is significantly hampering new > development (as evident from number of people trying to add 4 byte > addressing support.) > >> [1] http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/next-working >> > > IIRC, last post of this branch was in January 2018. And looking at my > inbox, its seems that 4 byte addressing was broken in that series. > If you add 4 byte addressing + SFDP parsing logic + spi-mem support to > above branch, I am pretty sure you spi-nor.c would look almost the same > to whats there in this series. > > Regards > Vignesh > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot >
On 07/12/18 12:47 AM, Simon Goldschmidt wrote: > Am 06.12.2018 um 18:39 schrieb Vignesh R: >> On 06/12/18 10:06 PM, Simon Goldschmidt wrote: >>> Am 06.12.2018 um 14:54 schrieb Simon Goldschmidt: >>>> On Thu, Dec 6, 2018 at 2:45 PM Vignesh R <vigneshr@ti.com> wrote: >>>>> On 06/12/18 2:15 AM, Simon Goldschmidt wrote: >>>>>> Am 05.12.2018 um 07:55 schrieb Simon Goldschmidt: >>>>>>> On Wed, Dec 5, 2018 at 7:51 AM Vignesh R <vigneshr@ti.com> wrote: >>>>> [...] [...] >>> Had a quick test with both standard and tiny in SPL and both configs now >>> work on my board. I haven't tested SFDP as you said it does not works on >>> Cadence QSPI. (I tested compiling it though, and that works now.) >>> > > After enabling DEBUG in spi-mem.c, I see that we're changing to 4-byte > mode (via b7h) instead of using 4-byte opcodes. Unfortunately, this > conflicts with the boot rom using the default read opcose in 3-byte mode > on warm reboot. > > Do you plan to change this or would this be fixed by parsing SFDP? > Oh, I see n25q256a entry is missing SPI_NOR_4B_OPCODES flag in the spi_nor_ids table. In non SFDP configuration adding SPI_NOR_4B_OPCODES flag should avoid sending 0xb7 and use 4-byte opcodes directly. May be you could give that a try? >> >> I used below hack to test SFDP with Cadence QSPI: > > Just by accident, I saw that enabling the SFDP config does not seem to > break booting. But it still switches to 4-byte mode, so it probably just > fails to parse and continues the non-SFDP way? > For stateless 4-byte addressing mode with SFDP, we need to parse JEDEC 4-byte Address Instruction Parameter Header and Table (part of JESD216B) to get 4 byte addressing opcode. We need to port this patch once accepted to Linux kernel: https://lore.kernel.org/patchwork/patch/1022200/ >> >> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c >> index a8af35203035..9dbf9aa7d20c 100644 >> --- a/drivers/spi/cadence_qspi_apb.c >> +++ b/drivers/spi/cadence_qspi_apb.c >> @@ -559,7 +559,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, >> /* Configure the opcode */ >> rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB; >> >> - if (rx_width & SPI_RX_QUAD) >> + if (rx_width & SPI_RX_QUAD && (cmdbuf[0] != 0x5a)) > > But this is Spansion-specific, right? > Nope, SFDP command operates in 1-1-1 mode for all flash devices. But if you don't have rx width set to 4 in DT, above hack is not needed and SFDP parsing would work fine with Cadence QSPI.
On Thu, 6 Dec 2018 23:55:30 +0530 Vignesh R <vigneshr@ti.com> wrote: > On 06-Dec-18 10:44 PM, Jagan Teki wrote: > > On Tue, Dec 4, 2018 at 5:56 PM Vignesh R <vigneshr@ti.com> wrote: > >> > >> U-Boot SPI NOR support (sf layer) is quite outdated as it does not > >> support 4 byte addressing opcodes, SFDP table parsing and different types of > >> quad mode enable sequences. Many newer flashes no longer support BANK > >> registers used by sf layer to a access >16MB space. > >> Also, many SPI controllers have special MMIO interfaces which provide > >> accelerated read/write access but require knowledge of flash parameters > >> to make use of it. Recent spi-mem layer provides a way to support such > >> flashes but sf layer isn't using that. > >> This patch series syncs SPI NOR framework from Linux v4.19. It also adds > >> spi-mem support on top. > >> So, we gain 4byte addressing support and SFDP support. This makes > >> migrating to U-Boot MTD framework easier. > > > > We(someone) has proposed this sync before, but we(at-least I) rely on > > implementing via DM not direct sync to Linux. > > As I said in my cover letter, U-Boot sf layer is unable to support newer > flashes mainly due to lack of 4 byte addressing and proper support for > MMIO capable SPI controllers. > My idea of fixing this is to borrow _features_ from Linux SPI NOR "as > is". All that's needed is stateless 4 byte addressing, SFDP > parsing(optionally), Quad/Octal support and spi-mem like abstraction for > MMIO capable Controllers. I see no point in re-coding them from ground up. > > Could you be more specific on what you would like to see here in DM way? > I have no issues in adapting this code to any framework here in U-Boot. > Linux has driver model and SPI NOR subsystem is a framework and > therefore any code ported from Linux will inherently have those > abstractions. The only difference I see wrt your code in branch below vs > this series is SPI-NOR uclass. This can be easily achieved by moving > nor->ops out of struct spi_nor into uclass abstraction. > Upstream Linux is anyways merging m25p80 and spi-nor so I did not see a > need for SPI NOR uclass. I am okay to change that if you insist on > having it. > No, please don't add a SPI_NOR_UCLASS. We already have the MTD_UCLASS which is barely used by drivers, so if anything, we should work on converting drivers to this UCLASS and maybe automate a few more things for MTD drivers (like MTD dev registration/deregistration [1]) instead of adding yet another UCLASS. If you need an example of how integration with the DM (and the MTD UCLASS) should be been done, you can look at the SPI NAND driver [2]. [1]http://code.bulix.org/9lart9-519406 [2]http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/mtd/nand/spi/core.c;h=cb8ffa3fa96a16dd40dda3553bbb171107e71df7;hb=HEAD#l1249
On Fri, Dec 7, 2018 at 6:58 AM Vignesh R <vigneshr@ti.com> wrote: > > On 07/12/18 12:47 AM, Simon Goldschmidt wrote: > > Am 06.12.2018 um 18:39 schrieb Vignesh R: > >> On 06/12/18 10:06 PM, Simon Goldschmidt wrote: > >>> Am 06.12.2018 um 14:54 schrieb Simon Goldschmidt: > >>>> On Thu, Dec 6, 2018 at 2:45 PM Vignesh R <vigneshr@ti.com> wrote: > >>>>> On 06/12/18 2:15 AM, Simon Goldschmidt wrote: > >>>>>> Am 05.12.2018 um 07:55 schrieb Simon Goldschmidt: > >>>>>>> On Wed, Dec 5, 2018 at 7:51 AM Vignesh R <vigneshr@ti.com> wrote: > >>>>> [...] > [...] > >>> Had a quick test with both standard and tiny in SPL and both configs now > >>> work on my board. I haven't tested SFDP as you said it does not works on > >>> Cadence QSPI. (I tested compiling it though, and that works now.) > >>> > > > > After enabling DEBUG in spi-mem.c, I see that we're changing to 4-byte > > mode (via b7h) instead of using 4-byte opcodes. Unfortunately, this > > conflicts with the boot rom using the default read opcose in 3-byte mode > > on warm reboot. > > > > Do you plan to change this or would this be fixed by parsing SFDP? > > > > Oh, I see n25q256a entry is missing SPI_NOR_4B_OPCODES flag in the > spi_nor_ids table. In non SFDP configuration adding SPI_NOR_4B_OPCODES > flag should avoid sending 0xb7 and use 4-byte opcodes directly. May be > you could give that a try? Yes, that did the trick. With SFDP disabled I now see 9Fh (read ID) followed directly by 0Ch (4-byte FAST READ). Just like it should be. With SFDP enabled, I get 9Fh, 5Ah, 5Ah and then 0Ch. Both configs do work fine for me, so: Tested-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> Regards, Simon > > >> > >> I used below hack to test SFDP with Cadence QSPI: > > > > Just by accident, I saw that enabling the SFDP config does not seem to > > break booting. But it still switches to 4-byte mode, so it probably just > > fails to parse and continues the non-SFDP way? > > > > For stateless 4-byte addressing mode with SFDP, we need to parse JEDEC > 4-byte Address Instruction Parameter Header and Table (part of JESD216B) > to get 4 byte addressing opcode. > We need to port this patch once accepted to Linux kernel: > https://lore.kernel.org/patchwork/patch/1022200/ > > >> > >> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c > >> index a8af35203035..9dbf9aa7d20c 100644 > >> --- a/drivers/spi/cadence_qspi_apb.c > >> +++ b/drivers/spi/cadence_qspi_apb.c > >> @@ -559,7 +559,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, > >> /* Configure the opcode */ > >> rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB; > >> > >> - if (rx_width & SPI_RX_QUAD) > >> + if (rx_width & SPI_RX_QUAD && (cmdbuf[0] != 0x5a)) > > > > But this is Spansion-specific, right? > > > > Nope, SFDP command operates in 1-1-1 mode for all flash devices. But if > you don't have rx width set to 4 in DT, above hack is not needed and > SFDP parsing would work fine with Cadence QSPI. > > > -- > Regards > Vignesh
On 07/12/18 3:21 PM, Boris Brezillon wrote: > On Thu, 6 Dec 2018 23:55:30 +0530 > Vignesh R <vigneshr@ti.com> wrote: > >> On 06-Dec-18 10:44 PM, Jagan Teki wrote: >>> On Tue, Dec 4, 2018 at 5:56 PM Vignesh R <vigneshr@ti.com> wrote: >>>> >>>> U-Boot SPI NOR support (sf layer) is quite outdated as it does not >>>> support 4 byte addressing opcodes, SFDP table parsing and different types of >>>> quad mode enable sequences. Many newer flashes no longer support BANK >>>> registers used by sf layer to a access >16MB space. >>>> Also, many SPI controllers have special MMIO interfaces which provide >>>> accelerated read/write access but require knowledge of flash parameters >>>> to make use of it. Recent spi-mem layer provides a way to support such >>>> flashes but sf layer isn't using that. >>>> This patch series syncs SPI NOR framework from Linux v4.19. It also adds >>>> spi-mem support on top. >>>> So, we gain 4byte addressing support and SFDP support. This makes >>>> migrating to U-Boot MTD framework easier. >>> >>> We(someone) has proposed this sync before, but we(at-least I) rely on >>> implementing via DM not direct sync to Linux. >> >> As I said in my cover letter, U-Boot sf layer is unable to support newer >> flashes mainly due to lack of 4 byte addressing and proper support for >> MMIO capable SPI controllers. >> My idea of fixing this is to borrow _features_ from Linux SPI NOR "as >> is". All that's needed is stateless 4 byte addressing, SFDP >> parsing(optionally), Quad/Octal support and spi-mem like abstraction for >> MMIO capable Controllers. I see no point in re-coding them from ground up. >> >> Could you be more specific on what you would like to see here in DM way? >> I have no issues in adapting this code to any framework here in U-Boot. >> Linux has driver model and SPI NOR subsystem is a framework and >> therefore any code ported from Linux will inherently have those >> abstractions. The only difference I see wrt your code in branch below vs >> this series is SPI-NOR uclass. This can be easily achieved by moving >> nor->ops out of struct spi_nor into uclass abstraction. >> Upstream Linux is anyways merging m25p80 and spi-nor so I did not see a >> need for SPI NOR uclass. I am okay to change that if you insist on >> having it. >> > > No, please don't add a SPI_NOR_UCLASS. We already have the MTD_UCLASS > which is barely used by drivers, so if anything, we should work on > converting drivers to this UCLASS and maybe automate a few more things > for MTD drivers (like MTD dev registration/deregistration [1]) instead > of adding yet another UCLASS. > I agree. > If you need an example of how integration with the DM (and the MTD > UCLASS) should be been done, you can look at the SPI NAND driver [2]. > > [1]http://code.bulix.org/9lart9-519406 > [2]http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/mtd/nand/spi/core.c;h=cb8ffa3fa96a16dd40dda3553bbb171107e71df7;hb=HEAD#l1249 > Yeah, that can be done once MTD Kconfig cleanup is done and we have dropped UCLASS_SPI_FLASH (would also require sf_dataflash.c to be moved to MTD)
On Thu, Dec 6, 2018 at 11:55 PM Vignesh R <vigneshr@ti.com> wrote: > > Hi Jagan, > > On 06-Dec-18 10:44 PM, Jagan Teki wrote: > > On Tue, Dec 4, 2018 at 5:56 PM Vignesh R <vigneshr@ti.com> wrote: > >> > >> U-Boot SPI NOR support (sf layer) is quite outdated as it does not > >> support 4 byte addressing opcodes, SFDP table parsing and different types of > >> quad mode enable sequences. Many newer flashes no longer support BANK > >> registers used by sf layer to a access >16MB space. > >> Also, many SPI controllers have special MMIO interfaces which provide > >> accelerated read/write access but require knowledge of flash parameters > >> to make use of it. Recent spi-mem layer provides a way to support such > >> flashes but sf layer isn't using that. > >> This patch series syncs SPI NOR framework from Linux v4.19. It also adds > >> spi-mem support on top. > >> So, we gain 4byte addressing support and SFDP support. This makes > >> migrating to U-Boot MTD framework easier. > > > > We(someone) has proposed this sync before, but we(at-least I) rely on > > implementing via DM not direct sync to Linux. > > As I said in my cover letter, U-Boot sf layer is unable to support newer > flashes mainly due to lack of 4 byte addressing and proper support for > MMIO capable SPI controllers. > My idea of fixing this is to borrow _features_ from Linux SPI NOR "as > is". All that's needed is stateless 4 byte addressing, SFDP > parsing(optionally), Quad/Octal support and spi-mem like abstraction for > MMIO capable Controllers. I see no point in re-coding them from ground up. > > Could you be more specific on what you would like to see here in DM way? > I have no issues in adapting this code to any framework here in U-Boot. > Linux has driver model and SPI NOR subsystem is a framework and > therefore any code ported from Linux will inherently have those > abstractions. The only difference I see wrt your code in branch below vs > this series is SPI-NOR uclass. This can be easily achieved by moving > nor->ops out of struct spi_nor into uclass abstraction. > Upstream Linux is anyways merging m25p80 and spi-nor so I did not see a > need for SPI NOR uclass. I am okay to change that if you insist on > having it. Merging or syncing spi-nor features stuff from Linux is good, I'm not stopping that. but this can be do by satisfying u-boot driver-model with proper architectural model. I know you take care but I'm not sure ie what can be manageable for long term. Let's discuss the proper architectural model, so-that we can move further to incorporate the changes accordingly. (thanks at last we have a thread to discuss) Linux m25p80 is moved to spi-nor right? so does controllers on spi-nor should be reside in same area like drivers/mtd/spi-nor or it should be part of spi-mem. The last mail with Boris, noted all spi-nor can't be fit into spi-mem(sorry I lost the thread) Example: we have zynq qspi it support BAR(with >16MB flashes), dual qspi ect so does it comes under spi-mem or spi-nor? So, if no driver should be part of spi-nor and all can be handle spi-mem even-though they have controller specific features, yes we can skip SPI_NOR_UCLASS otherwise we need spi-nor uclass that can be child uclass of MTD.
On Mon, 10 Dec 2018 18:32:09 +0530 Jagan Teki <jagan@amarulasolutions.com> wrote: > On Thu, Dec 6, 2018 at 11:55 PM Vignesh R <vigneshr@ti.com> wrote: > > > > Hi Jagan, > > > > On 06-Dec-18 10:44 PM, Jagan Teki wrote: > > > On Tue, Dec 4, 2018 at 5:56 PM Vignesh R <vigneshr@ti.com> wrote: > > >> > > >> U-Boot SPI NOR support (sf layer) is quite outdated as it does not > > >> support 4 byte addressing opcodes, SFDP table parsing and different types of > > >> quad mode enable sequences. Many newer flashes no longer support BANK > > >> registers used by sf layer to a access >16MB space. > > >> Also, many SPI controllers have special MMIO interfaces which provide > > >> accelerated read/write access but require knowledge of flash parameters > > >> to make use of it. Recent spi-mem layer provides a way to support such > > >> flashes but sf layer isn't using that. > > >> This patch series syncs SPI NOR framework from Linux v4.19. It also adds > > >> spi-mem support on top. > > >> So, we gain 4byte addressing support and SFDP support. This makes > > >> migrating to U-Boot MTD framework easier. > > > > > > We(someone) has proposed this sync before, but we(at-least I) rely on > > > implementing via DM not direct sync to Linux. > > > > As I said in my cover letter, U-Boot sf layer is unable to support newer > > flashes mainly due to lack of 4 byte addressing and proper support for > > MMIO capable SPI controllers. > > My idea of fixing this is to borrow _features_ from Linux SPI NOR "as > > is". All that's needed is stateless 4 byte addressing, SFDP > > parsing(optionally), Quad/Octal support and spi-mem like abstraction for > > MMIO capable Controllers. I see no point in re-coding them from ground up. > > > > Could you be more specific on what you would like to see here in DM way? > > I have no issues in adapting this code to any framework here in U-Boot. > > Linux has driver model and SPI NOR subsystem is a framework and > > therefore any code ported from Linux will inherently have those > > abstractions. The only difference I see wrt your code in branch below vs > > this series is SPI-NOR uclass. This can be easily achieved by moving > > nor->ops out of struct spi_nor into uclass abstraction. > > Upstream Linux is anyways merging m25p80 and spi-nor so I did not see a > > need for SPI NOR uclass. I am okay to change that if you insist on > > having it. > > Merging or syncing spi-nor features stuff from Linux is good, I'm not > stopping that. but this can be do by satisfying u-boot driver-model > with proper architectural model. I know you take care but I'm not sure > ie what can be manageable for long term. > > Let's discuss the proper architectural model, so-that we can move > further to incorporate the changes accordingly. (thanks at last we > have a thread to discuss) Having discussions about the long term plan is good, as long as it's not blocking support for new features for too long. Look, you've been discussing the spi-nor stuff for more than 1 year now, and people are still waiting it. Yogesh's attempt seems to go in the right direction, so let's not block that just because we don't know how to integrate things in the DM (that's not entirely BTW, I suggested an approach in one of my previous reply). > > Linux m25p80 is moved to spi-nor right? so does controllers on spi-nor > should be reside in same area like drivers/mtd/spi-nor or it should be > part of spi-mem. The last mail with Boris, noted all spi-nor can't be > fit into spi-mem(sorry I lost the thread) > > Example: we have zynq qspi it support BAR(with >16MB flashes), dual > qspi ect so does it comes under spi-mem or spi-nor? Everything should go in drivers/spi/ and be exposed as spi/spi-mem controllers. So yes, that's one aspect where Linux and u-boot should differ IMO, at least until we have all SPI NOR controller drivers converted to spi-mem in Linux. > > So, if no driver should be part of spi-nor and all can be handle > spi-mem even-though they have controller specific features, yes we can > skip SPI_NOR_UCLASS otherwise we need spi-nor uclass that can be child > uclass of MTD. That's the idea, yes. Note that I'm open to any discussion regarding the needed features and how we want expose that at the spi-mem level.
On Mon, Dec 10, 2018 at 02:15:05PM +0100, Boris Brezillon wrote: > On Mon, 10 Dec 2018 18:32:09 +0530 > Jagan Teki <jagan@amarulasolutions.com> wrote: > > > On Thu, Dec 6, 2018 at 11:55 PM Vignesh R <vigneshr@ti.com> wrote: > > > > > > Hi Jagan, > > > > > > On 06-Dec-18 10:44 PM, Jagan Teki wrote: > > > > On Tue, Dec 4, 2018 at 5:56 PM Vignesh R <vigneshr@ti.com> wrote: > > > >> > > > >> U-Boot SPI NOR support (sf layer) is quite outdated as it does not > > > >> support 4 byte addressing opcodes, SFDP table parsing and different types of > > > >> quad mode enable sequences. Many newer flashes no longer support BANK > > > >> registers used by sf layer to a access >16MB space. > > > >> Also, many SPI controllers have special MMIO interfaces which provide > > > >> accelerated read/write access but require knowledge of flash parameters > > > >> to make use of it. Recent spi-mem layer provides a way to support such > > > >> flashes but sf layer isn't using that. > > > >> This patch series syncs SPI NOR framework from Linux v4.19. It also adds > > > >> spi-mem support on top. > > > >> So, we gain 4byte addressing support and SFDP support. This makes > > > >> migrating to U-Boot MTD framework easier. > > > > > > > > We(someone) has proposed this sync before, but we(at-least I) rely on > > > > implementing via DM not direct sync to Linux. > > > > > > As I said in my cover letter, U-Boot sf layer is unable to support newer > > > flashes mainly due to lack of 4 byte addressing and proper support for > > > MMIO capable SPI controllers. > > > My idea of fixing this is to borrow _features_ from Linux SPI NOR "as > > > is". All that's needed is stateless 4 byte addressing, SFDP > > > parsing(optionally), Quad/Octal support and spi-mem like abstraction for > > > MMIO capable Controllers. I see no point in re-coding them from ground up. > > > > > > Could you be more specific on what you would like to see here in DM way? > > > I have no issues in adapting this code to any framework here in U-Boot. > > > Linux has driver model and SPI NOR subsystem is a framework and > > > therefore any code ported from Linux will inherently have those > > > abstractions. The only difference I see wrt your code in branch below vs > > > this series is SPI-NOR uclass. This can be easily achieved by moving > > > nor->ops out of struct spi_nor into uclass abstraction. > > > Upstream Linux is anyways merging m25p80 and spi-nor so I did not see a > > > need for SPI NOR uclass. I am okay to change that if you insist on > > > having it. > > > > Merging or syncing spi-nor features stuff from Linux is good, I'm not > > stopping that. but this can be do by satisfying u-boot driver-model > > with proper architectural model. I know you take care but I'm not sure > > ie what can be manageable for long term. > > > > Let's discuss the proper architectural model, so-that we can move > > further to incorporate the changes accordingly. (thanks at last we > > have a thread to discuss) > > Having discussions about the long term plan is good, as long as it's > not blocking support for new features for too long. Look, you've been > discussing the spi-nor stuff for more than 1 year now, and people are > still waiting it. Yogesh's attempt seems to go in the right direction, > so let's not block that just because we don't know how to integrate > things in the DM (that's not entirely BTW, I suggested an approach in > one of my previous reply). To be clear, we've been having this discussion for probably closer to two or three years now. Which is why at this point what I'm saying we need to do is get a logical evolution of To be clear, we've been having this discussion for probably closer to two or three years now. Which is why at this point what I'm saying we need to do is get a logical evolution of Vignesh's series in, for v2019.04 and improve it over time. The notion of "we'll block progress on new features in order to make sure we get conversion done" has failed. Lets go back to good old reliable incremental change over time.
On 10-Dec-18 6:32 PM, Jagan Teki wrote: > On Thu, Dec 6, 2018 at 11:55 PM Vignesh R <vigneshr@ti.com> wrote: >> >> Hi Jagan, >> >> On 06-Dec-18 10:44 PM, Jagan Teki wrote: >>> On Tue, Dec 4, 2018 at 5:56 PM Vignesh R <vigneshr@ti.com> wrote: >>>> >>>> U-Boot SPI NOR support (sf layer) is quite outdated as it does not >>>> support 4 byte addressing opcodes, SFDP table parsing and different types of >>>> quad mode enable sequences. Many newer flashes no longer support BANK >>>> registers used by sf layer to a access >16MB space. >>>> Also, many SPI controllers have special MMIO interfaces which provide >>>> accelerated read/write access but require knowledge of flash parameters >>>> to make use of it. Recent spi-mem layer provides a way to support such >>>> flashes but sf layer isn't using that. >>>> This patch series syncs SPI NOR framework from Linux v4.19. It also adds >>>> spi-mem support on top. >>>> So, we gain 4byte addressing support and SFDP support. This makes >>>> migrating to U-Boot MTD framework easier. >>> >>> We(someone) has proposed this sync before, but we(at-least I) rely on >>> implementing via DM not direct sync to Linux. >> >> As I said in my cover letter, U-Boot sf layer is unable to support newer >> flashes mainly due to lack of 4 byte addressing and proper support for >> MMIO capable SPI controllers. >> My idea of fixing this is to borrow _features_ from Linux SPI NOR "as >> is". All that's needed is stateless 4 byte addressing, SFDP >> parsing(optionally), Quad/Octal support and spi-mem like abstraction for >> MMIO capable Controllers. I see no point in re-coding them from ground up. >> >> Could you be more specific on what you would like to see here in DM way? >> I have no issues in adapting this code to any framework here in U-Boot. >> Linux has driver model and SPI NOR subsystem is a framework and >> therefore any code ported from Linux will inherently have those >> abstractions. The only difference I see wrt your code in branch below vs >> this series is SPI-NOR uclass. This can be easily achieved by moving >> nor->ops out of struct spi_nor into uclass abstraction. >> Upstream Linux is anyways merging m25p80 and spi-nor so I did not see a >> need for SPI NOR uclass. I am okay to change that if you insist on >> having it. > > Merging or syncing spi-nor features stuff from Linux is good, I'm not > stopping that. but this can be do by satisfying u-boot driver-model > with proper architectural model. I know you take care but I'm not sure > ie what can be manageable for long term. > > Let's discuss the proper architectural model, so-that we can move > further to incorporate the changes accordingly. (thanks at last we > have a thread to discuss) > > Linux m25p80 is moved to spi-nor right? so does controllers on spi-nor > should be reside in same area like drivers/mtd/spi-nor or it should be > part of spi-mem. The last mail with Boris, noted all spi-nor can't be > fit into spi-mem(sorry I lost the thread) > Yes, ATM all drivers fit into spi/spi-mem APIs and don't see any need for new spi-nor uclass > Example: we have zynq qspi it support BAR(with >16MB flashes), dual > qspi ect so does it comes under spi-mem or spi-nor? > In current mainline U-Boot, I see _no_ users of flags: SF_DUAL_STACKED_FLASH and SF_DUAL_PARALLEL_FLASH (I don't see flash->dual_flash set to any of the above enums). But if we do need to support such flashes in future, current address translation logic can be added to spi-nor.c (based on a DT flag), along with a way to pass this info via spi-mem ops. I would suggest to look at spi-mem ops (and in Linux mainline as well), if there are any shortcomings we can discuss here. > So, if no driver should be part of spi-nor and all can be handle > spi-mem even-though they have controller specific features, yes we can > skip SPI_NOR_UCLASS otherwise we need spi-nor uclass that can be child > uclass of MTD. > In fact, after this series is merged, UCLASS_SPI_FLASH can be dropped and we can move to spi-nor(and sf_dataflash.c) directly under UCLASS_MTD. But, mostly likely would need to provide a lightweight MTD for SPL (similar to spi-nor-tiny.c) before that can be done. Regards Vignesh
Hi Vignesh, Vignesh R <vigneshr@ti.com> wrote on Mon, 10 Dec 2018 23:08:38 +0530: > On 10-Dec-18 6:32 PM, Jagan Teki wrote: > > On Thu, Dec 6, 2018 at 11:55 PM Vignesh R <vigneshr@ti.com> wrote: > >> > >> Hi Jagan, > >> > >> On 06-Dec-18 10:44 PM, Jagan Teki wrote: > >>> On Tue, Dec 4, 2018 at 5:56 PM Vignesh R <vigneshr@ti.com> wrote: > >>>> > >>>> U-Boot SPI NOR support (sf layer) is quite outdated as it does not > >>>> support 4 byte addressing opcodes, SFDP table parsing and different types of > >>>> quad mode enable sequences. Many newer flashes no longer support BANK > >>>> registers used by sf layer to a access >16MB space. > >>>> Also, many SPI controllers have special MMIO interfaces which provide > >>>> accelerated read/write access but require knowledge of flash parameters > >>>> to make use of it. Recent spi-mem layer provides a way to support such > >>>> flashes but sf layer isn't using that. > >>>> This patch series syncs SPI NOR framework from Linux v4.19. It also adds > >>>> spi-mem support on top. > >>>> So, we gain 4byte addressing support and SFDP support. This makes > >>>> migrating to U-Boot MTD framework easier. > >>> > >>> We(someone) has proposed this sync before, but we(at-least I) rely on > >>> implementing via DM not direct sync to Linux. > >> > >> As I said in my cover letter, U-Boot sf layer is unable to support newer > >> flashes mainly due to lack of 4 byte addressing and proper support for > >> MMIO capable SPI controllers. > >> My idea of fixing this is to borrow _features_ from Linux SPI NOR "as > >> is". All that's needed is stateless 4 byte addressing, SFDP > >> parsing(optionally), Quad/Octal support and spi-mem like abstraction for > >> MMIO capable Controllers. I see no point in re-coding them from ground up. > >> > >> Could you be more specific on what you would like to see here in DM way? > >> I have no issues in adapting this code to any framework here in U-Boot. > >> Linux has driver model and SPI NOR subsystem is a framework and > >> therefore any code ported from Linux will inherently have those > >> abstractions. The only difference I see wrt your code in branch below vs > >> this series is SPI-NOR uclass. This can be easily achieved by moving > >> nor->ops out of struct spi_nor into uclass abstraction. > >> Upstream Linux is anyways merging m25p80 and spi-nor so I did not see a > >> need for SPI NOR uclass. I am okay to change that if you insist on > >> having it. > > > > Merging or syncing spi-nor features stuff from Linux is good, I'm not > > stopping that. but this can be do by satisfying u-boot driver-model > > with proper architectural model. I know you take care but I'm not sure > > ie what can be manageable for long term. > > > > Let's discuss the proper architectural model, so-that we can move > > further to incorporate the changes accordingly. (thanks at last we > > have a thread to discuss) > > > > Linux m25p80 is moved to spi-nor right? so does controllers on spi-nor > > should be reside in same area like drivers/mtd/spi-nor or it should be > > part of spi-mem. The last mail with Boris, noted all spi-nor can't be > > fit into spi-mem(sorry I lost the thread) > > > > Yes, ATM all drivers fit into spi/spi-mem APIs and don't see any need > for new spi-nor uclass > > > > Example: we have zynq qspi it support BAR(with >16MB flashes), dual > > qspi ect so does it comes under spi-mem or spi-nor? > > > > In current mainline U-Boot, I see _no_ users of flags: > SF_DUAL_STACKED_FLASH and SF_DUAL_PARALLEL_FLASH > (I don't see flash->dual_flash set to any of the above enums). > But if we do need to support such flashes in future, current address > translation logic can be added to spi-nor.c (based on a DT flag), along > with a way to pass this info via spi-mem ops. > I would suggest to look at spi-mem ops (and in Linux mainline as well), > if there are any shortcomings we can discuss here. > > > So, if no driver should be part of spi-nor and all can be handle > > spi-mem even-though they have controller specific features, yes we can > > skip SPI_NOR_UCLASS otherwise we need spi-nor uclass that can be child > > uclass of MTD. > > > > In fact, after this series is merged, UCLASS_SPI_FLASH can be dropped > and we can move to spi-nor(and sf_dataflash.c) directly under > UCLASS_MTD. But, mostly likely would need to provide a lightweight MTD > for SPL (similar to spi-nor-tiny.c) before that can be done. That would be great! Thanks, Miquèl