[U-Boot] [PATCH 6/9] CACHE: nand read/write: Test if start address is aligned

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/25/2012 02:17 PM, Scott Wood wrote:
> On 06/25/2012 03:48 PM, Tom Rini wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>> 
>> On 06/25/2012 01:08 PM, Scott Wood wrote:
>>> On 06/25/2012 01:43 PM, Tom Rini wrote:
>>>> On Mon, Jun 25, 2012 at 11:58:10AM -0500, Scott Wood wrote:
>>>>> On 06/24/2012 07:17 PM, Marek Vasut wrote:
>>>>>> This prevents the scenario where data cache is on and the
>>>>>>  device uses DMA to deploy data. In that case, it might
>>>>>> not be possible to flush/invalidate data to RAM properly.
>>>>>> The other option is to use bounce buffer,
>>>>> 
>>>>> Or get cache coherent hardware. :-)
>>>>> 
>>>>>> but that involves a lot of copying and therefore
>>>>>> degrades performance rapidly. Therefore disallow this
>>>>>> possibility of unaligned load address altogether if data
>>>>>> cache is on.
>>>>> 
>>>>> How about use the bounce buffer only if the address is 
>>>>> misaligned?  The corrective action a user has to take is
>>>>> the same as with this patch, except for an additional
>>>>> option of living with the slight performance penalty.  How
>>>>> often does this actually happen?  How much does it actually
>>>>> slow things down compared to the speed of the NAND chip?
>>>> 
>>>> We would need to architect things such that any 'load'
>>>> command would be routed through this logic.
>>> 
>>> It's something the driver backend should handle (possibly via
>>> a common helper library).  The fact that you can't do a DMA
>>> transfer to an unaligned buffer is a hardware-specific detail,
>>> just as is the fact that you're setting up a DMA buffer in the
>>> first place.
>> 
>> Right.  What I'm trying to say is it's not a NAND problem it's
>> an unaligned addresses problem so the solution needs to be easily
>> used everywhere.
> 
> OK, so fix it in each driver that has this issue.  A lot of drivers
> are probably not so performance critical that you can't just always
> use a bounce buffer.  A static buffer plus memcpy isn't that
> burdensome -- it's close to what the drivers for non-DMA hardware
> do.  For higher performance peripherals, throw in an if-statement
> or two.  It doesn't seem like something that needs a U-Boot-wide
> change.
> 
> In the specific case of NAND, how many NAND drivers use DMA at
> all?

Off hand I'm not sure.  I know there've been patches for OMAP parts
before but they had some unaddressed feedback and aren't in mainline.

>>> I'm not sure what bootm has to do with nand (and the fact that
>>> some ppc is cache coherent actually doesn't matter, since we
>>> don't do DMA for NAND), but I was able to bootm from an odd RAM
>>> address, and "nand read" to an odd RAM address, on p5020ds.
>> 
>> On ARM-land we have a lot of problems with unaligned addresses,
>> even with cache off.  I went to reproduce the original bootm
>> problem and ran into fatload hanging.  tftp didn't fail but bootm
>> hangs.
> 
> Maybe you can't take alignment exceptions during bootm?  PPC
> doesn't normally take alignment checks, but we would have trouble
> with this scenario if it did, since bootm clobbers the exception
> vectors.

Could be it.  But again, fatload via mmc also chokes, so there's a few
and quite long-standing problems.  What Marek and I are arguing for I
believe is to say it's not worth the effort to enable this corner
case.  Since it works on other arches, we shouldn't make this a silent
change that slips in, certainly, but at the end of the day it's highly
unlikely someone is using this in the wild and won't be able to work
around it in their next design or field upgrade.  I would hope at least.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJP6NaYAAoJENk4IS6UOR1WSogP/RX/VzrN0NzhApkVg38Bal7M
zgbjYS/Mub12oJmzU+iDosVC/HS6RqZb+LoLRlXROLAUka6SxUub/DtJATQ5ivRd
Y+x17Z7rdu4Q3dIcYV27RkJOCiXCRkUVFvulZEfzXlsLpoTC3PbiAA06xDuxHePJ
85liIhxMfTvwUGH0b4umtvy5vrlgop/K6Vdba9SPJwiS/pfW6hIj9ZnnFmJYY6aN
e1IqYjcQtDpxBttdy4PPZr4l0Qx+cYonHs9oFpgnibMsgr7FilqM8fVMq7iHGPYU
acX1YpvOZCWQsv3HcQdq/SrjnRJSDdcySV5q1xEZbpgw0iYbkT9lBb5GX4QcXr43
3zuvLlcvdPz1NzFp4v5gRWUnGaKvCUN5rQHT4UFQDfUErAHuA4q6xjnCe6bYD/EF
kETrAc8pN3sKwJTMJyE1LU2fFfaUirtggT3gWljfgKkfWmbbLnPRTUSkTFWRwu3S
QEcsGD8X+XUsiW9h/mZOlAuspzd6oxOOUCy54NfxkMQZy6YDGFFkPrzme6ztxIlz
O2oSSUSmsWFZbWHVxX/YejZta/RNp+3R9c37J8qFhDku19gMDo+RVn49vKW3dBxp
yb62WkxgO20990kAi7yDfy0XUlJR0WYdiUFiG273TFxNLcanHyct1JELf6zckxju
gBKoz+Ddlqtc7v5AWsuK
=Pyx6
-----END PGP SIGNATURE-----