[U-Boot] QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA

[Jason Rush]

On 1/7/2018 5:39 AM, Marek Vasut wrote:
> On 01/06/2018 10:09 PM, Jason Rush wrote:
>> On 1/6/2018 1:29 PM, Marek Vasut wrote:
>>> On 01/06/2018 07:46 PM, Jason Rush wrote:
>>>> On 1/6/2018 9:42 AM, Marek Vasut wrote:
>> <snip>
>>
>>>> There was a minor upstream change to one of the files since I submitted v4 of my
>>>> cadence device-tree patchset, so I rebased and resent the patchset as a v5.  I
>>>> included everyone that was originally involved with the patches and added a CC
>>>> for Marek.
>>>>
>>>> This is only the patchset for the device tree changes for the cadence qspi driver,
>>>> Simon will still have to add the patch that fixes the cache invalidation bug in the
>>>> cadence driver.
>>> Sigh, can we get a single patchset out which fixes the problem ?
>>>
>>> I mean, if I understand this correctly, you're all addressing one single
>>> problem, but with two patchsets, yes ?
>>>
>> Well... The one issue we're trying to fix is that the cadence QSPI hasn't worked on
>> the socfpga arch since late 2016.  However, it's two different issues that have caused
>> this bigger problem:
>>
>> 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.  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.

-- Jason