Hi,

On 12-12-14 21:25, Siarhei Siamashka wrote:
On Sun, 23 Nov 2014 14:43:13 +0100
Hans de Goede <hdego...@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 <hdego...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to