All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.aribaud@3adev.fr>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/5] vf610: refactor DDRMC code
Date: Fri, 19 Jun 2015 18:50:12 +0200	[thread overview]
Message-ID: <20150619185012.782d4ae6.albert.aribaud@3adev.fr> (raw)
In-Reply-To: <55843188.2020205@toradex.com>

Bonjour Stefan,

Le Fri, 19 Jun 2015 17:13:12 +0200, Stefan Agner
<stefan.agner@toradex.com> a ?crit :

> Hi Albert,
> 
> On 19.06.2015 14:18, Albert ARIBAUD (3ADEV) wrote:
> > The VF610 DDRMC driver code contains settings which are
> > board-specific. Move these out to boards so that new boards
> > can define their own without having to modify the driver.
> >
> > Signed-off-by: Albert ARIBAUD (3ADEV) <albert.aribaud@3adev.fr>
> > ---
> >
> >  arch/arm/imx-common/ddrmc-vf610.c             | 239 ++++++--------------------
> >  arch/arm/include/asm/arch-vf610/ddrmc-vf610.h |  60 +------
> >  board/freescale/vf610twr/vf610twr.c           | 181 +++++++++++++------
> >  board/toradex/colibri_vf/colibri_vf.c         | 169 +++++++++++++-----
> >  4 files changed, 312 insertions(+), 337 deletions(-)
> 
> So this goes basically back to setting all DDR registers directly from
> boards? What we tried to do here is to factor out the memory chip
> specific data (JEDEC). The idea behind this is it would make it simpler
> to add new RAM timings if a new RAM vendor is used on a different board
> revision...  With your changes it will be unnecessarily hard to add just
> a new RAM timing for a new board revision...

I could probably factor back out the JEDEC settings, but there are
still differences in the lists of registers to write between the
existing vf610twr/colibri_vf and the new pcm052, especially the PHY
regs but elsewhere too, and there are some writes in the driver that
the PCM052 does not have.

As I wanted to leave the existing boards strictly unaffected, and as I
did not want to start sprinkling '#if defined(some-board)' over the
driver code, I went for a fully board-controlled design so that no
board could possibly be affected by any future change to the driver.

How about a mix? I could keep the JEDEC and lvl pointers in the DDR
controller init call arguments and append "per-boards" CR and PHY
arrays. The driver would do the JEDEC writes (thus keeping JEDEC DDR3
additions simple), the LVL writes if not NULL, then the "per-board" CR
writes if not NULL, then the current common PHY writes, then the
"per-board" PHY writes if not null.

This would keep common parts (JEDEC and minimal settings) in the driver
while allowing board their own specific settings -- even overriding the
driver settings, since the per-board writes would come last before
CR000 is rewritten.

Would that be ok ?

> --
> Stefan

Cordialement,
Albert ARIBAUD
3ADEV

  reply	other threads:[~2015-06-19 16:50 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-19 12:18 [U-Boot] [PATCH 0/5] Add support for Vybrid VF610-based PCM052 Albert ARIBAUD
2015-06-19 12:18 ` [U-Boot] [PATCH 1/5] net: fec_mxc: remove useless struct nbuf Albert ARIBAUD
2015-06-19 12:18   ` [U-Boot] [PATCH 2/5] vf610: refactor DDRMC code Albert ARIBAUD
2015-06-19 12:18     ` [U-Boot] [PATCH 3/5] i2c: fix vf610 support Albert ARIBAUD
2015-06-19 12:18       ` [U-Boot] [PATCH 4/5] tools: mkimage: fix imximage header size Albert ARIBAUD
2015-06-19 12:18         ` [U-Boot] [PATCH 5/5] vf610: add support for Phytec PCM052 Albert ARIBAUD
2015-07-10  8:14         ` [U-Boot] [PATCH 4/5] tools: mkimage: fix imximage header size Stefano Babic
2015-07-14 10:29           ` Stefan Agner
2015-07-15  7:19             ` Stefano Babic
2015-07-15  7:54               ` Albert ARIBAUD
2015-07-15 10:41                 ` Stefan Agner
2015-07-15 11:44                   ` Albert ARIBAUD
2015-07-15 12:36                     ` Stefan Agner
2015-07-15 12:54                       ` Albert ARIBAUD
2015-07-15  7:37             ` Albert ARIBAUD
2015-07-10  8:11       ` [U-Boot] [PATCH 3/5] i2c: fix vf610 support Stefano Babic
2015-06-19 15:13     ` [U-Boot] [PATCH 2/5] vf610: refactor DDRMC code Stefan Agner
2015-06-19 16:50       ` Albert ARIBAUD [this message]
2015-06-19 17:33       ` Albert ARIBAUD
2015-07-10  8:09         ` Stefano Babic
2015-07-13 19:01           ` Stefan Agner
2015-07-14  7:16             ` [U-Boot] (rather [LONG], sorry) " Albert ARIBAUD
2015-06-19 15:38   ` [U-Boot] [PATCH 1/5] net: fec_mxc: remove useless struct nbuf Joe Hershberger
2015-07-10  8:03   ` Stefano Babic

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=20150619185012.782d4ae6.albert.aribaud@3adev.fr \
    --to=albert.aribaud@3adev.fr \
    --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.