From: Simon Schwarz <simonschwarzcor@googlemail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/2] nand_spl_simple: Add omap3 DMA usage to SPL
Date: Mon, 31 Oct 2011 09:56:54 +0100 [thread overview]
Message-ID: <4EAE62D6.5030109@gmail.com> (raw)
In-Reply-To: <4EA6FEF9.6070208@freescale.com>
Dear Scott,
On 10/25/2011 08:24 PM, Scott Wood wrote:
> On 10/16/2011 05:10 AM, Simon Schwarz wrote:
>> This adds DMA copy for the nand spl implementation. If CONFIG_SPL_DMA_SUPPORT
>> is defined the DMA is used.
>>
>> Based on DMA driver patch:
>> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/109744/focus=109747
>
> As Wolfgang pointed out, this doesn't belong here. Create your own
> alternate SPL driver if your hardware doesn't work with the simple one
> (similar to the not-yet-migrated nand_spl/nand_boot_fsl_elbc.c,
> nand_spl/nand_boot_fsl_ifc.c, etc).
>
Hm. The naming of the functions was a fault. Will rename the calls in
nand_spl_simple to remove omap parts. So
omap3_dma_wait_for_transfer
will become
dma_wait_for_transfer
etc.
So a board which intents to use DMA in SPL can implement these
functions. Would this be ok?
A whole new driver is IMHO not the right thing as there is too much
duplicated code then.
>> @@ -46,11 +59,11 @@ static int nand_command(int block, int page, uint32_t offs,
>> this->cmd_ctrl(&mtd, offs, NAND_CTRL_ALE | NAND_CTRL_CHANGE);
>> this->cmd_ctrl(&mtd, page_addr& 0xff, NAND_CTRL_ALE); /* A[16:9] */
>> this->cmd_ctrl(&mtd, (page_addr>> 8)& 0xff,
>> - NAND_CTRL_ALE); /* A[24:17] */
>> + NAND_CTRL_ALE); /* A[24:17] */
>> #ifdef CONFIG_SYS_NAND_4_ADDR_CYCLE
>> /* One more address cycle for devices> 32MiB */
>> this->cmd_ctrl(&mtd, (page_addr>> 16)& 0x0f,
>> - NAND_CTRL_ALE); /* A[28:25] */
>> + NAND_CTRL_ALE); /* A[28:25] */
>> #endif
>
> Please refrain from making random unrelated whitespace changes in a
> patch that also makes functional changes, particularly when they are
> extensive enough to make it hard to spot the functional changes.
>
> In this particular case, I think the whitespace was fine the way it was;
> the continuation lines were nicely aligned.
If I remember right I changed these because of checkpatch errors.
Regards
Simon
next prev parent reply other threads:[~2011-10-31 8:56 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-16 10:10 [U-Boot] [PATCH 0/2] Use DMA in SPL Simon Schwarz
2011-10-16 10:10 ` [U-Boot] [PATCH 1/2] nand_spl_simple: Add omap3 DMA usage to SPL Simon Schwarz
2011-10-23 18:40 ` Wolfgang Denk
2011-10-25 18:24 ` Scott Wood
2011-10-31 8:56 ` Simon Schwarz [this message]
2011-10-31 21:22 ` Scott Wood
2011-11-02 9:57 ` Simon Schwarz
2011-11-02 17:29 ` Scott Wood
2011-10-16 10:10 ` [U-Boot] [PATCH 2/2] devkit8000: Activate DMA support in SPL Simon Schwarz
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=4EAE62D6.5030109@gmail.com \
--to=simonschwarzcor@googlemail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.