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 3/5] sun6i: dram: Do not try to initialize a second dram chan on A31s
Date: Sat, 13 Dec 2014 12:00:44 +0100	[thread overview]
Message-ID: <548C1C5C.7000706@redhat.com> (raw)
In-Reply-To: <20141212222554.085a5fdc@t61>

Hi,

On 12-12-14 21:25, Siarhei Siamashka wrote:
> On Sun, 23 Nov 2014 14:43:13 +0100
> Hans de Goede <hdegoede@redhat.com> wrote:
>
>> The A31s only has one dram channel, so do not bother with trying to initialize
>> a second channel.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> Ian Campbell has already noticed in the earlier review. In the case *if*
> the automatic detection works correctly,

It does work correctly in my testing.

> then why bother adding this
> *extra* code?

I'm just mimicking what the boot0 code does here, and the boot0 code does
detect the SoC type and not bother with the second channel on A31s, and that
seems sensible to do, nothing good will come out of trying to use the second
channel on A31s.

> Based on your description, it looks like a superfluous
> band-aid and increases the number of "moving parts". Which generally
> makes the code less maintainable.
>
> There was a talk about calling it a "performance optimization" earlier,
> but the v2 commit message does not mention this. Neither does it present
> any benchmark numbers.
>
> Basically, it boils down to whether we can trust the automatic detection
> code to do a proper job or not. Normally, if the automatic detection
> does not work correctly in all cases, then it needs to be fixed in the
> long run. But I can clearly understand if you are not willing to take
> any risks and just want to deploy something that somehow works to the
> end users sooner.

In my testing the auto-detect code worked fine for automatically disabling
the 2nd channel on on A31s, but as said it seems sensible to just not bother
with the 2nd channel at all on A31s.

Regards,

Hans

  reply	other threads:[~2014-12-13 11:00 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-23 13:43 [U-Boot] [PATCH v2 0/5] sun6i: A31s / CSQ_CS908 board support Hans de Goede
2014-11-23 13:43 ` [U-Boot] [PATCH v2 1/5] sun6i: Make dram clk and zq value Kconfig options Hans de Goede
2014-11-25  8:55   ` Ian Campbell
2014-11-25  9:08     ` Hans de Goede
2014-12-12 20:29   ` Siarhei Siamashka
2014-11-23 13:43 ` [U-Boot] [PATCH v2 2/5] sun6i: Add sunxi_get_ss_bonding_id() function Hans de Goede
2014-11-23 13:43 ` [U-Boot] [PATCH v2 3/5] sun6i: dram: Do not try to initialize a second dram chan on A31s Hans de Goede
2014-11-25  8:56   ` Ian Campbell
2014-12-12 20:25   ` Siarhei Siamashka
2014-12-13 11:00     ` Hans de Goede [this message]
2014-12-19 10:02       ` Siarhei Siamashka
2014-11-23 13:43 ` [U-Boot] [PATCH v2 4/5] sun6i: Drop some "unknown magic" from dram init Hans de Goede
2014-12-12 20:24   ` Siarhei Siamashka
2014-12-13 10:57     ` Hans de Goede
2014-11-23 13:43 ` [U-Boot] [PATCH v2 5/5] sun6i: Add new CSQ_CS908 board Hans de Goede
2014-12-14 14:57   ` Ian Campbell
2014-12-18 10:39     ` Hans de Goede
2014-12-18 18:56       ` Ian Campbell

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=548C1C5C.7000706@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