On 03/16/2017 11:11 PM, Rush, Jason A. wrote: > 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 <jason.r...@gd-ms.com> >>>>>>>> >>>>>>>> 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.
Why don't you just gain that experience ? You can ask questions on the list ... > 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. I think that's still an improvement ... -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot