public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 6/9] CACHE: nand read/write: Test if start address is aligned
Date: Mon, 25 Jun 2012 15:08:09 -0500	[thread overview]
Message-ID: <4FE8C529.30509@freescale.com> (raw)
In-Reply-To: <20120625184354.GA6377@bill-the-cat>

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.

>> I'm hesitant to break something -- even if it's odd (literally in this
>> case) -- that currently works on most hardware, just because one or two
>> drivers can't handle it.  It feels kind of like changing the read() and
>> write() system calls to require cacheline alignment. :-P
> 
> I don't want to get into an ARM vs PowerPC argument.  I think the best
> answer is that I'm not sure having things unaligned works totally right
> today as I did a silly test of loading a uImage to 0x82000001 and bootm
> hung inside of U-Boot not long ago.  Can you try that on some cache
> coherent hardware and see if that works?

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.

> [snip]
>>> @@ -609,6 +609,8 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
>>>  			goto usage;
>>>  
>>>  		addr = (ulong)simple_strtoul(argv[2], NULL, 16);
>>> +		if (!cacheline_aligned(addr))
>>> +			return 1;
>>
>> There's no way you can just return like this without printing an error
>> that lets the user know that the operation wasn't performed, and why.
> 
> Note that cacheline_aligned does the error print (patch 2 of 9).

Ah.  I saw this patch first since it was CCed to me. :-)

-Scott

  reply	other threads:[~2012-06-25 20:08 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-25  0:17 [U-Boot] [PATCH 0/9] CACHE: Finishing touches Marek Vasut
2012-06-25  0:17 ` [U-Boot] [PATCH 1/9] COMMON: Add __stringify() function Marek Vasut
2012-06-25  0:17 ` [U-Boot] [PATCH 2/9] CACHE: Add cache_aligned() macro Marek Vasut
2012-06-25 21:12   ` Scott Wood
2012-06-25 23:30     ` Marek Vasut
2012-07-07  3:00       ` Aneesh V
2012-06-25  0:17 ` [U-Boot] [PATCH 3/9] CACHE: ext2load: Test if start address is aligned Marek Vasut
2012-06-25  0:17 ` [U-Boot] [PATCH 4/9] CACHE: fatload: " Marek Vasut
2012-06-25  0:17 ` [U-Boot] [PATCH 5/9] CACHE: mmc read/write: " Marek Vasut
2012-06-25  0:17 ` [U-Boot] [PATCH 6/9] CACHE: nand " Marek Vasut
2012-06-25 16:58   ` Scott Wood
2012-06-25 18:43     ` Tom Rini
2012-06-25 20:08       ` Scott Wood [this message]
2012-06-25 20:48         ` Tom Rini
2012-06-25 21:17           ` Scott Wood
2012-06-25 21:22             ` Tom Rini
2012-06-25 23:42             ` Marek Vasut
2012-06-26  0:37               ` Scott Wood
2012-06-26  1:16                 ` Marek Vasut
2012-06-26 19:38                   ` Scott Wood
2012-06-25 23:38       ` Marek Vasut
2012-06-25 23:37     ` Marek Vasut
2012-06-25 23:57       ` Scott Wood
2012-06-26  1:33         ` Marek Vasut
2012-06-26 19:25           ` Scott Wood
2012-06-26 20:39             ` Marek Vasut
2012-07-07  3:05   ` Aneesh V
2012-06-25  0:17 ` [U-Boot] [PATCH 7/9] CACHE: net: " Marek Vasut
2012-06-25 18:05   ` Joe Hershberger
2012-06-25 23:16     ` Marek Vasut
2012-06-25  0:17 ` [U-Boot] [PATCH 8/9] CACHE: net: asix: Fix asix driver to work with data cache on Marek Vasut
2012-06-25 18:07   ` Joe Hershberger
2012-06-25 23:16     ` Marek Vasut
2012-07-06 23:09     ` Marek Vasut
2012-07-06 23:16       ` Marek Vasut
2012-06-25  0:17 ` [U-Boot] [PATCH 9/9] M28EVK: Enable instruction and data cache Marek Vasut

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FE8C529.30509@freescale.com \
    --to=scottwood@freescale.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox