[Skiboot] [PATCH v2 03/12] hdat: Add new fields to IPL params structure

[Vasant Hegde]

On 11/02/2016 09:18 AM, Oliver O'Halloran wrote:
> On Sun, Oct 30, 2016 at 1:36 AM, Vasant Hegde
> <hegdevasant at linux.vnet.ibm.com> wrote:
>> Add new fields to sys params structure and update sys family for p9.
>>
>> In P9 the compatible string is supplied by hostboot through the HDAT.
>> This patch add support for using these strings to set the compatible
>> property in the device tree rather than using the machine ID number
>> scheme traditionally used in the HDAT.
>>
>> Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
>> [Folded Oliver's changes to original patch - Vasant]
>> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
>> ---
>>   hdata/spira.c | 63 +++++++++++++++++++++++++++++++++++++----------------------
>>   hdata/spira.h | 17 +++++++++++++++-
>>   2 files changed, 56 insertions(+), 24 deletions(-)
>>
>> diff --git a/hdata/spira.c b/hdata/spira.c
>> index 59ffc1d..fa08824 100644
>> --- a/hdata/spira.c
>> +++ b/hdata/spira.c
>> @@ -752,8 +752,6 @@ static void add_nx(void)
>>   static void add_iplparams_sys_params(const void *iplp, struct dt_node *node)
>>   {
>>          const struct iplparams_sysparams *p;
>> -       u32 sys_type;
>> -       const char *sys_family;
>>          const struct HDIF_common_hdr *hdif = iplp;
>>          u16 version = be16_to_cpu(hdif->version);
>>
>> @@ -772,29 +770,48 @@ static void add_iplparams_sys_params(const void *iplp, struct dt_node *node)
>>
>>          dt_add_property_nstr(node, "ibm,sys-model", p->sys_model, 4);
>>
>> -       /* Compatible is 2 entries: ibm,powernv and ibm,<platform>
>> +       /*
>> +        * Compatible has up to three entries:
>> +        *      "ibm,powernv", the system family and system type.
>> +        *
>> +        * On P9 and above the family and type strings come from the HDAT
>> +        * directly. On P8 we find it from the system ID numbers.
>>           */
>> -       sys_type = be32_to_cpu(p->system_type);
>> -       switch(sys_type >> 28) {
>> -       case 0:
>> -               sys_family = "ibm,squadrons";
>> -               break;
>> -       case 1:
>> -               sys_family = "ibm,eclipz";
>> -               break;
>> -       case 2:
>> -               sys_family = "ibm,apollo";
>> -               break;
>> -       case 3:
>> -               sys_family = "ibm,firenze";
>> -               break;
>> -       default:
>> -               sys_family = NULL;
>> -               prerror("IPLPARAMS: Unknown system family\n");
>> -               break;
>> +       if (proc_gen >= proc_gen_p9) {
>> +               /*
>> +                * FIXME: We haven't defined P9 system model. Hence append
>> +                *        "ibm,firenze" so that we can boot the system. Remove
>> +                *        this once we define P9 system model.
>> +                */
>> +               dt_add_property_strings(dt_root, "compatible",
>> +                                       "ibm,powernv", "ibm,firenze",
>> +                                       p->sys_family_str, p->sys_type_str);
>
> Is there any reason you're keying this off the processor generation
> rather than the HDIF structure version?

Spec says these fields are implemented from HDAT v0x60.. But many cases its not 
implemented on that version.. My understanding in all HDAT on P9 onwards will 
have this field enabled. Anyway we don't care this field on P8.

Hence instead of relying on HDAT version I used processor generation.


 > Currently hostboot doesn't
> populate the type strings (and leaves the struct version on 0x51 as a
> result), but it will set the sys_type field correctly. We should
> always use the sys_type test in the else case if the strings aren't

But they are suppose to implement this field at least on P9 !

> populated. Hardcoding "ibm,firenze" in the else case will cause us
> less issues once the strings are populated too.

We don't enter else part in newer hdat anyway.

-Vasant