Hi Marek,
On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> > On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> >> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> >>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> >>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut 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.
> >>
> >> In fact, Laurent (CCed) was solving some similar issue with lossy decomp
> >> and I think this involved some passing of memory layout information from
> >> ATF to U-Boot too, or am I mistaken ?
> >
> > That's correct, ATF stores information about the memory layout at a fixed
> > address in system memory, and U-Boot can read it.
>
> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
> then? :)
I'd prefer that, yes.
Let's not duplicate the mechanism used to pass FCNL information from ATF to U-
Boot, but instead create a data table format that can store all the
information we need, in an easily extensible way.
To see how the mechanism is implemented for FCNL, search for 47FD7000 in the
Renesas ATF sources (git://github.com/renesas-rcar/arm-trusted-firmware.git).
--
Regards,
Laurent Pinchart
