QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA
hey, i have looked at this thread:
http://u-boot.10912.n7.nabble.com/QSPI-quot-sf-probe-quot-quot-sf-read-quot-on-Altera-SoC-FPGA-td304882.html
i'm having the same problem with Arria 5,
when i try to use the "sf probe" command on the nor flash, the result i'm getting0 is:
SF: Detected n25q512 with page size 256 Bytes, erase size 64 KiB, total 64 MiB
### ERROR ### Please RESET the board ###
i have already tried to use the patches for the uboot code and also to add
more defines before making the uboot, but it still doesn't work for me.
any ideas? thanks ahead.
--
Sent from: http://u-boot.10912.n7.nabble.com/
Quoted 9 lines · Jagan Teki
Quoted 5 lines
There were already patches posted on this list by me and others, but unfortunately they haven't made it into the repository, yet.
Jagan, could you comment on the status of these fixes? I can search for the patchwork items related if you want me to.
2 out of 1 of this[1] have some discussion still going is it?
No, that series should be dropped. I don't know if I can do anything about that in patchwork though?
Let me check the patches from my upstreaming queue when I'm back at work tomorrow. I'll send a list of patchwork items I needed to get QSPI running on mach-socfpga.
Thanks,
Simon
yeah, those patches might help me a lot, i can't get it to work right now.
thanks a lot.
--
Sent from: http://u-boot.10912.n7.nabble.com/
Quoted 46 lines · Vignesh R
On Monday 08 January 2018 09:10 AM, Jason Rush wrote:
[...]Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
1. The indaddrtrig register was being programmed with an incorrect value for socfpga as the result of assuming it should be programmed with the same address as the ahbbase address. This issue is resolved by adopting the Linux DT bindings, which has an independent setting for the indaddrtrig register so the register can be set correctly on all
architectures. Plus it aligns the DT between u-boot and Linux.
Quoted 10 lines
Quoted 2 lines
That should be an easy patch, so this is the patchset 0/5..5/5 that you just submitted ?
Yes. I saw you Acked it, thank you.
Quoted 7 lines
Quoted 7 lines
2. The cadence driver was modified at one point to use the bouncebuf functions to fix an issue on a TI architecture that expected, where if I recall correctly all reads except the last have to be 32-bit reads. However, since the bouncebuf was designed for DMA transfers, it invalidates the data cache after reading, but since the cadence is using cpu transfers the newly read data is thrown away when the cache is invalidated. This issue is resolved by reverting the commit that
introduced using the bounce buffer for read operations, which according to Vignesh don't cause any issues to the TI architecture.
Quoted 9 lines
Quoted 4 lines
Hmmmmm, I wonder why you need bounce buffer at all here. The CQSPI literally reads/writes a register space (or some FIFO in register space), there is no DMA involved at all. I also wonder why we have to manipulate with cache at all here.
I agree, I don't believe this needs a bounce buffer at all. This isn't a DMA, there is no need for cache manipulation. Vignesh understands the problem better than I do on the TI platform, but I believe it was used since it was an easy way to ensure the register read/writes
were all 32-bits wide up until the last read/write.
Yes, that was the intention. Unfortunately, I chose to use common bounce buffer implementation which was doing cache manipulations.
Quoted 4 lines
I believe the bounce buffer should be removed from the CQSPI driver and a different solution should be implemented, but Vignesh should weigh in on that since it effects his architecture.
CQSPI on TI K2G has problems with non 32 bit aligned write operations. But read operations are unaffected. Therefore I have Ack'ed Simon's patch reverting bouncebuf for read. For writes, I have patches to revert common bouncebuf usage and use a local pagesize buffer for overcome alignment issue. I am waiting for current patch backlogs to be merged so that its easy for testing w/o specifying bunch of dependent patches.
Quoted 2 lines
Or if Simon agrees, I can add his patch to my series post it to mailing list (rebased on top of Jason's series)?
Well, it's not really "my" patch, anyway. It reverts a commit of yours, so sure, as long as this does not stand in the way of getting qspi running on 2018.03, go ahead and itegrate it in your patchset.
I'd be happy to have this sent now so I can test both patchsets on top of 2018.01(-rc).
Thanks,
Simon
hey, i downloaded the september and november versions and i applied the patches on both of them, re-compiled the u boot,
and still, it gives me the same error when trying the command "sf probe" i'm not sure what to do, is it possible that i missed something? like a define or something in the make menuconfig?
i did some debugging and it seems that the crash happens on this line:
div = DIV_ROUND_UP(ref_clk_hz, sclk-hz * 2) -1;
thanks.
2018-01-08 11:17 GMT+02:00 Goldschmidt Simon <
[email protected]>:
Quoted 68 lines · Vignesh R
Quoted 7 lines · Jason Rush
[...]
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
1. The indaddrtrig register was being programmed with an incorrect value for socfpga as the result of assuming it should be programmed with the same address as the ahbbase address. This issue is resolved by adopting the Linux DT bindings, which has an independent setting for the indaddrtrig register so the register can be set
correctly on all
Quoted 11 lines
architectures. Plus it aligns the DT between u-boot and Linux.
Quoted 10 lines
Quoted 2 lines
That should be an easy patch, so this is the patchset 0/5..5/5 that you just submitted ?
Yes. I saw you Acked it, thank you.
Quoted 7 lines
Quoted 7 lines
2. The cadence driver was modified at one point to use the bouncebuf functions to fix an issue on a TI architecture that expected, where if I recall correctly all reads except the last have to be 32-bit reads. However, since the bouncebuf was designed for DMA transfers, it invalidates the data cache after reading, but since the cadence is using cpu transfers the newly read data is thrown away when the cache is invalidated. This issue is resolved by reverting the
commit that
Quoted 1 line
introduced using the bounce buffer for read operations, which according
to
Quoted 10 lines
Vignesh don't cause any issues to the TI architecture.
Quoted 9 lines
Quoted 4 lines
Hmmmmm, I wonder why you need bounce buffer at all here. The CQSPI literally reads/writes a register space (or some FIFO in register space), there is no DMA involved at all. I also wonder why we have to manipulate with cache at all here.
I agree, I don't believe this needs a bounce buffer at all. This isn't a DMA, there is no need for cache manipulation. Vignesh understands the problem better than I do on the TI platform, but I believe it was used since it was an easy way to ensure the register
read/writes
Quoted 3 lines
were all 32-bits wide up until the last read/write.
Yes, that was the intention. Unfortunately, I chose to use common bounce
buffer
Quoted 11 lines
implementation which was doing cache manipulations.
Quoted 4 lines
I believe the bounce buffer should be removed from the CQSPI driver and a different solution should be implemented, but Vignesh should weigh in on that since it effects his architecture.
CQSPI on TI K2G has problems with non 32 bit aligned write operations. But read operations are unaffected. Therefore I have Ack'ed Simon's patch reverting bouncebuf for read. For writes, I have patches to revert common bouncebuf usage and use a local pagesize buffer for overcome alignment
issue. I
Quoted 1 line
am waiting for current patch backlogs to be merged so that its easy for
testing w/o
Quoted 3 lines
specifying bunch of dependent patches.
Or if Simon agrees, I can add his patch to my series post it to mailing
list (rebased
Quoted 1 line
on top of Jason's series)?
Well, it's not really "my" patch, anyway. It reverts a commit of yours, so sure, as long as this does not stand in the way of getting qspi running on 2018.03, go ahead and itegrate it in your patchset.
I'd be happy to have this sent now so I can test both patchsets on top of 2018.01(-rc).
Thanks,
Simon
Quoted 9 lines · On 17.01.2018 14:01, RB23 wrote:
hey, i downloaded the september and november versions and i applied the patches on both of them, re-compiled the u boot,
and still, it gives me the same error when trying the command "sf probe" i'm not sure what to do, is it possible that i missed something? like a define or something in the make menuconfig?
i did some debugging and it seems that the crash happens on this line:
div =DIV_ROUND_UP(ref_clk_hz, sclk-hz * 2)-1;
thanks.
I don't know exactly which patches you are talking about, but the divide-by-zero problem is still present with all patches I applied. To fix it, you need to set up your device tree correctly so that the speed is initialized.
If I remember correctly, you need need to have a flash chip below the qspi in the device tree. Check Jason's changes to the various socfpga dts files.
Regards,
Simon
Quoted 89 lines
2018-01-08 11:17 GMT+02:00 Goldschmidt Simon
<[email protected]
<mailto:[email protected]>>:On Mon, 08/012018 06:27, Vignesh R wrote:
> On Monday 08 January 2018 09:10 AM, Jason Rush wrote:
> [...]
> >>> 1. The indaddrtrig register was being programmed with an incorrect
> >>> value for socfpga as the result of assuming it should be programmed
> >>> with the same address as the ahbbase address. This issue is > >>> resolved by adopting the Linux DT bindings, which has an independent
> >>> setting for the indaddrtrig register so the register can be set correctly on all
> architectures. Plus it aligns the DT between u-boot and Linux. > >> That should be an easy patch, so this is the patchset 0/5..5/5 that
> >> you just submitted ?
> > Yes. I saw you Acked it, thank you.
> >>> 2. The cadence driver was modified at one point to use the bouncebuf
> >>> functions to fix an issue on a TI architecture that expected, where
> >>> if I recall correctly all reads except the last have to be 32-bit
> >>> reads. However, since the bouncebuf was designed for DMA transfers,
> >>> it invalidates the data cache after reading, but since the cadence
> >>> is using cpu transfers the newly read data is thrown away when the
> >>> cache is invalidated. This issue is resolved by reverting the commit that
> introduced using the bounce buffer for read operations, which according to
> Vignesh don't cause any issues to the TI architecture.
> >> Hmmmmm, I wonder why you need bounce buffer at all here. The CQSPI
> >> literally reads/writes a register space (or some FIFO in register > >> space), there is no DMA involved at all. I also wonder why we have to
> >> manipulate with cache at all here.
> >
> > I agree, I don't believe this needs a bounce buffer at all. This > > isn't a DMA, there is no need for cache manipulation. Vignesh > > understands the problem better than I do on the TI platform, but I > > believe it was used since it was an easy way to ensure the register read/writes
> were all 32-bits wide up until the last read/write.
>
> Yes, that was the intention. Unfortunately, I chose to use common bounce buffer
> implementation which was doing cache manipulations.
>
> > I believe the bounce buffer should be removed from the CQSPI driver
> > and a different solution should be implemented, but Vignesh should > > weigh in on that since it effects his architecture.
> >
>
> CQSPI on TI K2G has problems with non 32 bit aligned write operations.
> But read operations are unaffected. Therefore I have Ack'ed Simon's patch
> reverting bouncebuf for read. For writes, I have patches to revert common
> bouncebuf usage and use a local pagesize buffer for overcome alignment issue. I
> am waiting for current patch backlogs to be merged so that its easy for testing w/o
> specifying bunch of dependent patches.
>
> Or if Simon agrees, I can add his patch to my series post it to mailing list (rebased
> on top of Jason's series)?Well, it's not really "my" patch, anyway. It reverts a commit of yours, so sure, as long as this does not stand in the way of getting qspi running on 2018.03, go ahead and itegrate it in your patchset.
I'd be happy to have this sent now so I can test both patchsets on top of 2018.01(-rc).
Thanks,
Simon
Quoted 11 lines · Simon Goldschmidt
Quoted 9 lines · On 17.01.2018 14:01, RB23 wrote:
hey, i downloaded the september and november versions and i applied the patches on both of them, re-compiled the u boot,
and still, it gives me the same error when trying the command "sf probe" i'm not sure what to do, is it possible that i missed something? like a define or something in the make menuconfig?
i did some debugging and it seems that the crash happens on this line:
div =DIV_ROUND_UP(ref_clk_hz, sclk-hz * 2)-1;
thanks.
Quoted 9 lines
I don't know exactly which patches you are talking about, but the divide-by-zero problem is still present with all patches I applied. To fix it, you need to set up your device tree correctly so that the speed is initialized.
If I remember correctly, you need need to have a flash chip below the qspi in the device tree. Check Jason's changes to the various socfpga dts files.
Er, maybe a patch which checks this condition wouldn't hurt ?
Signature · 2 lines
Best regards, Marek Vasut
i checked, and if you mean these patches:
https://patchwork.ozlabs.org/patch/765992/
https://patchwork.ozlabs.org/patch/765996/
https://patchwork.ozlabs.org/patch/765997/
https://patchwork.ozlabs.org/patch/765998/
i already applied them, and they didn't work as well.
i also found these patches which i didn't try yet:
https://lists.denx.de/pipermail/u-boot/2017-May/292230.html
should i try those instead?
2018-01-17 15:09 GMT+02:00 Marek Vasut <[email protected]>:
Quoted 28 lines · Simon Goldschmidt
Quoted 11 lines · On 17.01.2018 14:01, RB23 wrote:
Quoted 9 lines
hey, i downloaded the september and november versions and i applied the patches on both of them, re-compiled the u boot,
and still, it gives me the same error when trying the command "sf probe" i'm not sure what to do, is it possible that i missed something? like a define or something in the make menuconfig?
i did some debugging and it seems that the crash happens on this line:
div =DIV_ROUND_UP(ref_clk_hz, sclk-hz * 2)-1;
thanks.Quoted 9 lines
I don't know exactly which patches you are talking about, but the divide-by-zero problem is still present with all patches I applied. To fix it, you need to set up your device tree correctly so that the speed is initialized.
If I remember correctly, you need need to have a flash chip below the qspi in the device tree. Check Jason's changes to the various socfpga dts files.
Er, maybe a patch which checks this condition wouldn't hurt ?
--
Best regards,
Marek Vasut
Quoted 6 lines · On 1/17/2018 7:46 AM, RB23 wrote:
i checked, and if you mean these patches:
https://patchwork.ozlabs.org/patch/765992/
https://patchwork.ozlabs.org/patch/765996/
https://patchwork.ozlabs.org/patch/765997/
https://patchwork.ozlabs.org/patch/765998/
Quoted 35 lines
i already applied them, and they didn't work as well.
i also found these patches which i didn't try yet:
https://lists.denx.de/pipermail/u-boot/2017-May/292230.htmlshould i try those instead?
2018-01-17 15:09 GMT+02:00 Marek Vasut <[email protected]>:
Quoted 26 lines · Simon Goldschmidt
Quoted 10 lines · On 17.01.2018 14:01, RB23 wrote:
Quoted 9 lines
hey, i downloaded the september and november versions and i applied the patches on both of them, re-compiled the u boot,
and still, it gives me the same error when trying the command "sf probe" i'm not sure what to do, is it possible that i missed something? like a define or something in the make menuconfig?
i did some debugging and it seems that the crash happens on this line:
div =DIV_ROUND_UP(ref_clk_hz, sclk-hz * 2)-1;
thanks.Quoted 9 lines
I don't know exactly which patches you are talking about, but the divide-by-zero problem is still present with all patches I applied. To fix it, you need to set up your device tree correctly so that the speed is initialized.
If I remember correctly, you need need to have a flash chip below the qspi in the device tree. Check Jason's changes to the various socfpga dts files.
Er, maybe a patch which checks this condition wouldn't hurt ?
--
Best regards,
Marek Vasut
I re-tested on a Cyclone V SoCKit devboard and a custom Arria V board using the DTS patches I submitted (the 4 you mentioned above) and Vignesh's patch to fix the cache invalidation problem, and I don't get the divide-by-zero problem. This looks like the clock rate for the flash part isn't getting set and is defaulting to 0 for some reason. I would look at your device tree. Are you using a stock device tree, or is this a custom board? Make sure the 'spi-max-frequency' is being set for the flash part that is a child to the cadence qspi node in the device tree.
-- Jason
Quoted 32 lines · Jason Rush
Quoted 31 lines · On 1/17/2018 7:46 AM, RB23 wrote:
i checked, and if you mean these patches:
https://patchwork.ozlabs.org/patch/765992/
https://patchwork.ozlabs.org/patch/765996/
https://patchwork.ozlabs.org/patch/765997/
https://patchwork.ozlabs.org/patch/765998/i already applied them, and they didn't work as well.
i also found these patches which i didn't try yet:
https://lists.denx.de/pipermail/u-boot/2017-May/292230.htmlshould i try those instead?
2018-01-17 15:09 GMT+02:00 Marek Vasut <[email protected]>:
Quoted 16 lines · Simon Goldschmidt
Quoted 15 lines · On 17.01.2018 14:01, RB23 wrote:
Quoted 9 lines
hey, i downloaded the september and november versions and i applied the patches on both of them, re-compiled the u boot,
and still, it gives me the same error when trying the command "sf probe" i'm not sure what to do, is it possible that i missed something? like a define or something in the make menuconfig?
i did some debugging and it seems that the crash happens on this line:
div =DIV_ROUND_UP(ref_clk_hz, sclk-hz * 2)-1;
thanks.I don't know exactly which patches you are talking about, but the divide-by-zero problem is still present with all patches I applied. To fix it, you need to set up your device tree correctly so that the speed is initialized.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
If I remember correctly, you need need to have a flash chip below the qspi in the device tree. Check Jason's changes to the various socfpga dts files.
Er, maybe a patch which checks this condition wouldn't hurt ?
Hmm, the problem here is not specific to cadence_qspi, but to the clock rate calculation in an upper layer (as Jason wrote below). From the ML, I got the impression it's OK like that (which I don't think it is, it should at least give a hint what's going wrong instead of a data abort). However, I'll try to prepare a patch for that as soon as I find the time.
Regards,
Simon
Quoted 15 lines
Quoted 5 lines
Quoted 5 lines
--
Best regards,
Marek VasutI re-tested on a Cyclone V SoCKit devboard and a custom Arria V board using the DTS patches I submitted (the 4 you mentioned above) and Vignesh's patch to fix the cache invalidation problem, and I don't get the divide-by-zero problem. This looks like the clock rate for the flash part isn't getting set and is defaulting to 0 for some reason. I would look at your device tree. Are you using a stock device tree, or is this a custom board? Make sure the 'spi-max-frequency' is being set for the flash part that is a child to the cadence qspi node in the device tree.
-- Jason
Pepperl+Fuchs GmbH, Mannheim
Geschaeftsfuehrer/Managing Directors: Dr.-Ing. Gunther Kegel (Vors./CEO), Werner Guthier, Mehmet Hatiboglu Vorsitzender des Aufsichtsrats/Chairman of the supervisory board: Claus Michael Registergericht/Register Court: AG Mannheim HRB 4713
Quoted 42 lines · Jason Rush
Quoted 6 lines · On 1/17/2018 7:46 AM, RB23 wrote:
i checked, and if you mean these patches:
https://patchwork.ozlabs.org/patch/765992/
https://patchwork.ozlabs.org/patch/765996/
https://patchwork.ozlabs.org/patch/765997/
https://patchwork.ozlabs.org/patch/765998/Quoted 35 lines
i already applied them, and they didn't work as well.
i also found these patches which i didn't try yet:
https://lists.denx.de/pipermail/u-boot/2017-May/292230.htmlshould i try those instead?
2018-01-17 15:09 GMT+02:00 Marek Vasut <[email protected]>:
Quoted 26 lines · Simon Goldschmidt
Quoted 10 lines · On 17.01.2018 14:01, RB23 wrote:
Quoted 9 lines
hey, i downloaded the september and november versions and i applied the patches on both of them, re-compiled the u boot,
and still, it gives me the same error when trying the command "sf probe" i'm not sure what to do, is it possible that i missed something? like a define or something in the make menuconfig?
i did some debugging and it seems that the crash happens on this line:
div =DIV_ROUND_UP(ref_clk_hz, sclk-hz * 2)-1;
thanks.Quoted 9 lines
I don't know exactly which patches you are talking about, but the divide-by-zero problem is still present with all patches I applied. To fix it, you need to set up your device tree correctly so that the speed is initialized.
If I remember correctly, you need need to have a flash chip below the qspi in the device tree. Check Jason's changes to the various socfpga dts files.
Er, maybe a patch which checks this condition wouldn't hurt ?
--
Best regards,
Marek Vasut
Quoted 7 lines
I re-tested on a Cyclone V SoCKit devboard and a custom Arria V board using the DTS patches I submitted (the 4 you mentioned above) and Vignesh's patch to fix the cache invalidation problem, and I don't get the divide-by-zero problem. This looks like the clock rate for the flash part isn't getting set and is defaulting to 0 for some reason. I would look at your device tree. Are you using a stock device tree, or is this a custom board? Make sure the 'spi-max-frequency' is being set for the flash part that is a child to the cadence qspi node in the device tree.
Note that in addition to the 'spi-max-frequency' property, the flash part (child of the cadence qspi node) also must have 'compatible = "spi-flash";'.
This only seems to fail in U-Boot when loading the environment or executing 'sf probe' without speed argument. SPL succeeds to load U-Boot from qspi without a 'spi-max-frequency' because the cadence qspi driver has a fallback to 500 kHz which gets overridden by the code from U-Boot.
Regards,
Simon
Wichtiger Hinweis:
Diese E-Mail einschliesslich ihrer Anhaenge enthaelt vertrauliche und rechtlich geschuetzte Informationen, die nur fuer den Adressaten bestimmt sind.
Sollten Sie nicht der bezeichnete Adressat sein, so teilen Sie dies bitte dem Absender umgehend mit und loeschen Sie diese Nachricht und ihre Anhaenge. Die unbefugte Weitergabe, das Anfertigen von Kopien und jede Veraenderung der E-Mail ist untersagt. Der Absender haftet nicht fuer Inhalte von veraenderten E-Mails.
Important Information:
This e-mail message including its attachments contains confidential and legally protected information solely intended for the addressee. If you are not the intended addressee of this message, please contact the addresser immediately and delete this message including its attachments. The unauthorized dissemination, copying and change of this e-mail are strictly forbidden. The addresser shall not be liable for the content of such changed e-mails.
it worked!
i applied the patches alone and it didn't work, but then i noticed simon's note to add compatible = "spi-flash" so i added it on the arria 5 dts file, and it worked.
i think it should be added to the patches.
thanks for the help!
--
Sent from: http://u-boot.10912.n7.nabble.com/