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