[U-Boot] am33xx config_ddr api change needed to support mDDR

[U-Boot] am33xx config_ddr api change needed to support mDDR

[Markus Brunner]

Hi,
for mDDR to work mddr_sel (bit 28) of ddrctrl->ddrioctrl has to be
set. There is no way of telling config_ddr(unsigned int pll, unsigned
int ioctrl, ...) my desired value, it won't be set atm.

[U-Boot] am33xx config_ddr api change needed to support mDDR

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/05/2013 03:48 AM, Markus Brunner wrote:
> Hi, for mDDR to work mddr_sel (bit 28) of ddrctrl->ddrioctrl has to
> be set. There is no way of telling config_ddr(unsigned int pll,
> unsigned int ioctrl, ...) my desired value, it won't be set atm.
> 
> From the 1.3kB ddr_ctrl struct only 2 registers are defined, which
> is fine if it's used memory mapped, however it makes it  unsuitable
> for passing values in the SPL. So at first glance I would extend 
> config_ddr() with an additional register value, because guessing
> the ddr type from the values of the other parameters is, in
> addition to being hackish, not guaranteed to work with every weired
> ram configuration. However might it be possible, that the second
> value of config_ddr() should not hold the value for
> config_io_ctrl(), but the value of my desired ddrctrl->ddrioctrl? 
> The wiki [1] from TI says that the first should _always_ be 0x18B, 
> whereas the second needs to be set depending on the mem type. We
> are talking about a set of registers named DDR_*_IOCTRL and a 
> single register called DDR_IO_CTRL, so easy to mix up.
> 
> Ragards, Markus
> 
> [1]
> http://processors.wiki.ti.com/index.php/AM335x_EMIF_Configuration_tips

Hadn't
> 
seen that wiki page before, thanks.  So a problem right now is
that yes, I hadn't seen "in the wild" anything but DDR2 or DDR3
designs.  I'm going to assume that on your custom hardware you are not
doing the EEPROM-based board detection method that the TI boards do.
We should try and lay things out so that boards where we can run-time
say "we are on board X (rev Y) and must have ..." can all out the same
functions for boards where we simply have to set that at build time
(until we get to the some-day of using a device tree or something like
that perhaps).

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJREokiAAoJENk4IS6UOR1Wd90P/2hgjjH1e9E749n2PJLQhIrt
r3So+ylQd+q4hzLieuwqRmi2sEtm6af+A5GPH6xoGSNiSPO+ps3mJAAaWZ04CxqC
W/ZKPB3mgDkwGCIyqRAJd4EuMxKqwyTkv+NGoElRvyN3G/OAunMBq2cTMAo6s6St
zo2/j8d2pbHlKQByteBN/3QWgALQ/6emqKSqhD6O5b7htinp5c8lPiRBeHNBAaRa
aeTorHIA8cuSzzXrmZE7lZUlSRQxe0CyApzcXfHfdZ2QD8fAA0REAxqD3r4XR4wp
5iobESR3wl+7FEXU0hp6lI/PjCCckKbubH3s8eVwxNGRyP0BOPvVDWyVTChXaHfe
jmNG10b60CLLGr4tnJSpimT26UIeE/uxCUumG8BkWQQIyPl9SYUObOlzqXSQmMIh
KquiF2m9zvPNEMSLdhpKqpdt4zO55bE46PSd13FFOhiyBgpIYP/+Jk4I3RgmCg4p
pXiFnK83MRhwzSeRZd9klPsN2gd8pmnLZee1EbpL4/LpOFhFGMfQl8a/BEJLiCoj
GDawN4OPWUQWX6pYuhWtKpRFUiPiOWi5/EvNkfW4JYUrP+8/ycGgk14p3sUkvPIE
9kjZoZ1W2XMSqnJA8jz4oPBNPqluNadabjX7N9OB52jYSApU2qJxa9RJlD3zGrxy
Daq4me3pz1dNDVUmtRVl
=u++P
-----END PGP SIGNATURE-----

[U-Boot] am33xx config_ddr api change needed to support mDDR

[Markus Brunner]

Hi Tom,
I'm not using an eeprom, but I can't see how this would change
anything. The problem is, that the generic config_ddr from
am3xx/emif4.c isn't capable of configuring mDDR and all that's needed
is setting an additional register.
I will post a patch.

Regards,
Markus

On Wed, Feb 6, 2013 at 5:47 PM, Tom Rini <trini@ti.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 02/05/2013 03:48 AM, Markus Brunner wrote:
>> Hi, for mDDR to work mddr_sel (bit 28) of ddrctrl->ddrioctrl has to
>> be set. There is no way of telling config_ddr(unsigned int pll,
>> unsigned int ioctrl, ...) my desired value, it won't be set atm.
>>
>> From the 1.3kB ddr_ctrl struct only 2 registers are defined, which
>> is fine if it's used memory mapped, however it makes it  unsuitable
>> for passing values in the SPL. So at first glance I would extend
>> config_ddr() with an additional register value, because guessing
>> the ddr type from the values of the other parameters is, in
>> addition to being hackish, not guaranteed to work with every weired
>> ram configuration. However might it be possible, that the second
>> value of config_ddr() should not hold the value for
>> config_io_ctrl(), but the value of my desired ddrctrl->ddrioctrl?
>> The wiki [1] from TI says that the first should _always_ be 0x18B,
>> whereas the second needs to be set depending on the mem type. We
>> are talking about a set of registers named DDR_*_IOCTRL and a
>> single register called DDR_IO_CTRL, so easy to mix up.
>>
>> Ragards, Markus
>>
>> [1]
>> http://processors.wiki.ti.com/index.php/AM335x_EMIF_Configuration_tips
>
> Hadn't
>>
> seen that wiki page before, thanks.  So a problem right now is
> that yes, I hadn't seen "in the wild" anything but DDR2 or DDR3
> designs.  I'm going to assume that on your custom hardware you are not
> doing the EEPROM-based board detection method that the TI boards do.
> We should try and lay things out so that boards where we can run-time
> say "we are on board X (rev Y) and must have ..." can all out the same
> functions for boards where we simply have to set that at build time
> (until we get to the some-day of using a device tree or something like
> that perhaps).
>
> - --
> Tom
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iQIcBAEBAgAGBQJREokiAAoJENk4IS6UOR1Wd90P/2hgjjH1e9E749n2PJLQhIrt
> r3So+ylQd+q4hzLieuwqRmi2sEtm6af+A5GPH6xoGSNiSPO+ps3mJAAaWZ04CxqC
> W/ZKPB3mgDkwGCIyqRAJd4EuMxKqwyTkv+NGoElRvyN3G/OAunMBq2cTMAo6s6St
> zo2/j8d2pbHlKQByteBN/3QWgALQ/6emqKSqhD6O5b7htinp5c8lPiRBeHNBAaRa
> aeTorHIA8cuSzzXrmZE7lZUlSRQxe0CyApzcXfHfdZ2QD8fAA0REAxqD3r4XR4wp
> 5iobESR3wl+7FEXU0hp6lI/PjCCckKbubH3s8eVwxNGRyP0BOPvVDWyVTChXaHfe
> jmNG10b60CLLGr4tnJSpimT26UIeE/uxCUumG8BkWQQIyPl9SYUObOlzqXSQmMIh
> KquiF2m9zvPNEMSLdhpKqpdt4zO55bE46PSd13FFOhiyBgpIYP/+Jk4I3RgmCg4p
> pXiFnK83MRhwzSeRZd9klPsN2gd8pmnLZee1EbpL4/LpOFhFGMfQl8a/BEJLiCoj
> GDawN4OPWUQWX6pYuhWtKpRFUiPiOWi5/EvNkfW4JYUrP+8/ycGgk14p3sUkvPIE
> 9kjZoZ1W2XMSqnJA8jz4oPBNPqluNadabjX7N9OB52jYSApU2qJxa9RJlD3zGrxy
> Daq4me3pz1dNDVUmtRVl
> =u++P
> -----END PGP SIGNATURE-----