From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Fri, 19 Jun 2015 18:50:12 +0200 Subject: [U-Boot] [PATCH 2/5] vf610: refactor DDRMC code In-Reply-To: <55843188.2020205@toradex.com> References: <1434716311-26189-1-git-send-email-albert.aribaud@3adev.fr> <1434716311-26189-2-git-send-email-albert.aribaud@3adev.fr> <1434716311-26189-3-git-send-email-albert.aribaud@3adev.fr> <55843188.2020205@toradex.com> Message-ID: <20150619185012.782d4ae6.albert.aribaud@3adev.fr> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Bonjour Stefan, Le Fri, 19 Jun 2015 17:13:12 +0200, Stefan Agner 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) > > --- > > > > 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