From: Albert ARIBAUD <albert.u.boot@aribaud.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] nand: lpc32xx: add SLC NAND controller support
Date: Wed, 15 Jul 2015 11:21:19 +0200 [thread overview]
Message-ID: <20150715112119.10a2db4e@lilith> (raw)
In-Reply-To: <55A61E7D.8000001@mleia.com>
Hello Vladimir,
On Wed, 15 Jul 2015 11:49:01 +0300, Vladimir Zapolskiy <vz@mleia.com>
wrote:
> Hello Albert,
>
> On 15.07.2015 10:05, Albert ARIBAUD wrote:
> > Hello Vladimir,
> >
> > On Tue, 14 Jul 2015 23:23:57 +0300, Vladimir Zapolskiy <vz@mleia.com>
> > wrote:
> >> The change adds support of LPC32xx SLC NAND controller.
> >>
> >> LPC32xx SoC has two different mutually exclusive NAND controllers
> >> to communicate with single and multiple layer chips.
> >>
> >> This simple driver allows to specify NAND chip timings and defines
> >> custom read_buf()/write_buf() operations, because access to 8-bit
> >> data register must be 32-bit aligned.
> >>
> >> Support of hardware ECC calculation is not implemented (data
> >> correction is always done by software), since it requires a working
> >> DMA engine.
> >>
> >> The driver can be included to an SPL image.
> >
> > This is needed for an upcoming new board support patch, right?
>
> you are correct, I plan to extend current support of devkit3250 board.
>
> > If so, then I suggest you put together all patches for this new
> > board in a single series. This will make it clear(er) you're not
> > adding dead code here.
> >
>
> I got the point, I will be able to complete board specific changes today
> tonight and send them for review.
Thanks.
> Please let me ask you for one more advice, if I want to add peripherals
> support on the board (by the way thank you for your drivers) and SPL
> image building support, both changes touch defconfig and board config
> header files. Should I split these changes into separate ones or is one
> "board support extension" patch preferred?
A commit should ideally be a single, self-contained, logical change.
So I would say each driver addition should be one commit, and the SPL
support addition should be its own commit, even though each of these
commits touches the defconfig and header config files.
This has at least two benefits:
- each commit is simpler to review (and to design and test, too). If a
commit contains several logical changes, it is harder to sort out
which change(s) a given patch chunk is about.
- in case a change was applied to U-Boot and later proves to cause
an issue, then we can easily revert this change, and only
this change, by reverting its commit. If the commit contains
several change, then we cannot simply revert the commit, we need to
manually "patch out" the problematic change while keeping the others.
> --
> With best wishes,
> Vladimir
Amicalement,
--
Albert.
next prev parent reply other threads:[~2015-07-15 9:21 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-14 20:23 [U-Boot] [PATCH] nand: lpc32xx: add SLC NAND controller support Vladimir Zapolskiy
2015-07-15 7:05 ` Albert ARIBAUD
2015-07-15 8:49 ` Vladimir Zapolskiy
2015-07-15 9:21 ` Albert ARIBAUD [this message]
2015-07-15 19:23 ` LEMIEUX, SYLVAIN
2015-07-15 20:21 ` Albert ARIBAUD
2015-07-16 0:19 ` Vladimir Zapolskiy
2015-07-16 19:38 ` LEMIEUX, SYLVAIN
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=20150715112119.10a2db4e@lilith \
--to=albert.u.boot@aribaud.net \
--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