Hi,
On 17-05-15 12:25, Ian Campbell wrote:
On Fri, 2015-05-15 at 20:43 +0200, Hans de Goede wrote:
Some A33 based boards use odt, while others do not, so make odt_en
configurable for sun8i too by moving the existing Kconfig option for it out
of the #if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I block it was in.
I'm still not mad-keen on overloading an int Kconfig with boolean
semantics. _Especially_ if it has different semantics on different
generations of DRAM controller, that's even worse than before IMHO.
I think we should either:
Have two Kconfig options an int on SUN[457]I and a bool on SUN8I.
Whether or not they have the same name I don't know, consider adding
GENx to it perhaps?
-or-
Have a single option which is an int which has similar semantics on
both. This could be ODT_EN[0] == enable, and:
ODT_EN[31:1]==reserved on SUN8I
but
ODT_EN[7:1]==reserved,[15:8]==correction,[31:16]==reserved on
the others.
The main upshot here, apart from improved help text for the option,
would be adding &0x1 to the usage in SUN8I.
Is there any reason not to push odt_en through dram_para like on other
subarches? Or conversely any reason for those others not to use the
Kconfig directly?
The second option sounds like a simpler change from where the code is
today, but perhaps we prefer not to invent semantics in this way. FWIW
if you were to prefer the first option above then going via dram_param
would make the use of IS_ENABLED (to assign to a boolean field in the
param struct) much less ugly than with the existing structure of the
code.
+ Set the dram controller odt_en parameter. This can be used to
+ enable/disable the ODT feature.
This isn't strictly correct/true on SUN8I since we don't set odt_en in
the params.
Ok, I think I've a good solution for this now, which actually makes
it a bool, v3 of this patch is coming up.
Regards,
Hans
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
