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