[PATCH 2/2] drm/ast: Support AST2500

[Emil Velikov]

Hi Ben,

On 16 February 2017 at 21:09, Benjamin Herrenschmidt
<benh at kernel.crashing.org> wrote:
> On Thu, 2017-02-16 at 17:33 +0000, Emil Velikov wrote:
>> On 16 February 2017 at 09:09, Benjamin Herrenschmidt
>> <benh at kernel.crashing.org> wrote:
>> > From: "Y.C. Chen" <yc_chen at aspeedtech.com>
>> >
>> > Add detection and POST code for AST2500 generation chip,
>> > code originally from Aspeed and slightly reworked for
>> > coding style mostly by Ben.
>> >
>> > Signed-off-by: Y.C. Chen <yc_chen at aspeedtech.com>
>> > Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
>> > ---
>> > v2. [BenH]
>> >   - Coding style fixes
>> >   - Add timeout to main DRAM init loop
>> >   - Improve boot time display of RAM info
>> >
>> > Y.C. Please check that I didn't break POST as I haven't manage to
>> > test
>> > that (we can't boot our machines if the BMC isn't already POSTed).
>> >
>>
>> Hi Ben,
>>
>> Barring the checkpatch.pl warnings/errors that you've spotted there's
>> a few other comments below.
>> I'm not familiar with the hardware, so it's just things that strike
>> me
>> as odd ;-)
>
> Heh ok. I don't want to change that POST code too much as I'm not
> equipped to test it, but I'll have a look later today.
>
Not sure why you opted for splitting each suggestion in separate
email, but it seems to have lead to a [serious] bugfix to go
unnoticed.
Namely:

>> > +static bool ddr_test_2500(struct ast_private *ast)
>> > +{
>> > +       ast_moutdwm(ast, 0x1E6E0074, 0x0000FFFF);
>> > +       ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
>> > +       if (mmc_test_burst(ast, 0) < 0)
>> > +               return false;
>> > +       if (mmc_test_burst(ast, 1) < 0)
>> > +               return false;
>> > +       if (mmc_test_burst(ast, 2) < 0)
>> > +               return false;
>> > +       if (mmc_test_burst(ast, 3) < 0)
>> > +               return false;
>> > +       if (mmc_test_single_2500(ast, 0) < 0)
>> > +               return false;
>> > +       return false;
>>
>> Final return should be true... either things got funny with v2 or the
>> this patch was never tested ?
>>
This here.

Regards,
Emil