Marek Vasut wrote: > 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. >
I agree a workaround is no good. I have no knowledge or experience with fixing a cache problem though, so I think someone else is going to have to tackle this bug. If you'd like, I can submit a separate patch for adopting the Linux DT trigger-address property, but the CQSPI will still be broken for the socfpga until someone fixes the cache problem with the CQSPI. -- Regards, Jason _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
