From mboxrd@z Thu Jan  1 00:00:00 1970
From: Hans de Goede <hdegoede@redhat.com>
Date: Tue, 19 May 2015 14:56:10 +0200
Subject: [U-Boot] [PATCH v2] sunxi: Make dram odt-en configurable
 through Kconfig for A33 based boards
In-Reply-To: <1431858342.5748.55.camel@hellion.org.uk>
References: <1431715428-23385-1-git-send-email-hdegoede@redhat.com>
 <1431858342.5748.55.camel@hellion.org.uk>
Message-ID: <555B32EA.8000306@redhat.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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