public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2] sunxi: Make dram odt-en configurable through Kconfig for A33 based boards
Date: Tue, 19 May 2015 14:56:10 +0200	[thread overview]
Message-ID: <555B32EA.8000306@redhat.com> (raw)
In-Reply-To: <1431858342.5748.55.camel@hellion.org.uk>

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

      parent reply	other threads:[~2015-05-19 12:56 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-15 18:43 [U-Boot] [PATCH v2] sunxi: Make dram odt-en configurable through Kconfig for A33 based boards Hans de Goede
2015-05-17 10:25 ` Ian Campbell
2015-05-18 10:58   ` Siarhei Siamashka
2015-05-19 12:56   ` Hans de Goede [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=555B32EA.8000306@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox