From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marek.vasut@gmail.com>
Date: Fri, 15 Jun 2018 13:43:11 +0200
Subject: [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795
 3.0 and newer
In-Reply-To: <CAO3366yB85UTK+T1Wq8_tV_J4AS0zwAWkKSDAfqUJWcLsDj=7g@mail.gmail.com>
References: <1529055605-29942-1-git-send-email-ulrich.hecht+renesas@gmail.com>
 <fc5a0c4f-8e2a-19bf-2d46-6ade92920b5c@gmail.com>
 <CAO3366yB85UTK+T1Wq8_tV_J4AS0zwAWkKSDAfqUJWcLsDj=7g@mail.gmail.com>
Message-ID: <3a3b322d-c04d-063e-392b-7967a9b8959c@gmail.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

On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>>
>> Will this call work on platforms without patched ATF ?
>> (I think not, don't you need to handle return value?)
> 
> I have not actually tested that, but if I understand the ATF code
> correctly, unimplemented calls return
> SMC_UNK (0xffffffff), which should be handled by the default case (NOP) below.

Which means the board has a memory size of 0 and fails to boot ?

>>> +             switch (res.a0) {
>>> +             case 1:
>>> +                     base[0] = 0x048000000ULL;
>>> +                     size[0] = 0x038000000ULL;
>>> +                     base[1] = 0x500000000ULL;
>>> +                     size[1] = 0x040000000ULL;
>>> +                     base[2] = 0x600000000ULL;
>>> +                     size[2] = 0x040000000ULL;
>>> +                     base[3] = 0x700000000ULL;
>>> +                     size[3] = 0x040000000ULL;
>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>> +                     break;
>>> +             case 2:
>>> +                     base[0] = 0x048000000ULL;
>>> +                     size[0] = 0x078000000ULL;
>>> +                     base[1] = 0x500000000ULL;
>>> +                     size[1] = 0x080000000ULL;
>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>>> +                     break;
>>> +             case 3:
>>> +                     base[0] = 0x048000000ULL;
>>> +                     size[0] = 0x078000000ULL;
>>> +                     base[1] = 0x500000000ULL;
>>> +                     size[1] = 0x080000000ULL;
>>> +                     base[2] = 0x600000000ULL;
>>> +                     size[2] = 0x080000000ULL;
>>> +                     base[3] = 0x700000000ULL;
>>> +                     size[3] = 0x080000000ULL;
>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>> +                     break;
>>
>> Obvious design question is -- since you're adding new SMC call anyway,
>> can't the call just return the memory layout table itself, so that it
>> won't be duplicated both in U-Boot and ATF ?
> 
> My gut feeling was to go with the smallest interface possible.

But this doesn't scale. The API here uses some ad-hoc constants to
identify memory layout tables which have to be encoded both in ATF and
U-Boot, both of which must be kept in sync.

The ATF already has those memory layout tables, it's only a matter of
passing them to U-Boot. If you do just that, the ad-hoc constants and
encoding of tables into U-Boot goes away and in fact simplifies the design.

Yet, I have to wonder if ATF doesn't already contain some sort of
standard SMC call to get memory topology. It surprises me that it wouldn't.

-- 
Best regards,
Marek Vasut