On 03/14/2017 03:23 PM, Rush, Jason A. wrote: > Marek Vasut wrote: >> On 03/07/2017 04:18 PM, Rush, Jason A. wrote: >>> Marek Vasut wrote: >>>> On 03/03/2017 04:17 PM, Rush, Jason A. wrote: >>>>> Marek Vasut wrote: >>>>>> On 03/01/2017 05:36 PM, Rush, Jason A. wrote: >>>>>>> This reverts commit b63b46313ed29e9b0c36b3d6b9407f6eade40c8f. >>>>>>> >>>>>>> The Cadence QSPI device does not work with caching (introduced with >>>>>>> the bounce buffer in this commit) on the Altera SoC platform. >>>>>>> >>>>>>> Signed-off-by: Jason A. Rush <[email protected]> >>>>>> >>>>>> Do we really need the reverts or can we just fix the commit(s) up >>>>>> somehow ? >>>>>> >>>>> >>>>> Would you prefer I squash the 2 reverts and the subsequent patch together >>>>> as a single commit? >>>> >>>> I would prefer if you answered my question :) So let me re-iterate, can >>>> we incrementally fix the driver instead of doing the revert(s) ? >>> >>> I think I misunderstood your question. Could you clarify what you mean by >>> incrementally fix the driver? >> >> That means change it to the final form without reverting patches. > > Yes, I can definitely change this so it does not revert the patches.
OK >>> Are you asking if there is a way to fix the cache issue with the CQSPI on >>> the >>> Altera SoC platform? If so, I don't know if I have the knowledge to answer >>> that. >>> Do you have any suggestions on where one would start looking to fix the >>> caching problem? >> >> Uh, there is a cache problem on socfpga ? Is this series working around >> it then ? >> > > Yes, there seems to be a cache issue with the CQSPI on the socfpga. And yes, > this patch series is working around this cache issue. OK, that means I'm not accepting this series. Let's fix the cache issue properly. > In addition, this series > introduces the trigger-address DT that Linux introduced and is required by > the CQSPI on socfpga. Can we split this part from the patchset, so it can go in separately ? > Here's a quick recap of the cache issue and this patch series... The original > commits used a bounce buffer implementation to fix 32-bit read alignment > issues with the CQSPI on a TI SoC device. The bounce buffer implementation > was a clean solution to the 32-bit alignment issue, but this implementation > returned random data from the QSPI flash on socfpga. You suggested I try > disabling the dcache, as you recalled some caching problem on the CQSPI in > the past. Running 'dcache off' solved the random data issue on socfpga > while using the bounce buffer implementation. > > That's when Vignesh and I decided to work around the cache problem by > reverting the two original commits and using a previous patch Vignesh had > written to fix the alignment problem. > > If I clean the series up to change it to the final form and not revert the > patches, is this an acceptable approach? No, if we know there is a bug and we only pile workarounds, we will end up with shitty code. Let's fix this properly please. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
