| Message ID | 1477751795-20128-5-git-send-email-hegdevasant@linux.vnet.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: > + /* > + * XXX: Spec says vendor information is availale from hdat > + * version >= 0x5f. But we are getting '00' on version > + * 0x5f. Hence check for processor generation. > + */ Can we get clarity on what actually happens on 0x5f and above and clarity in the spec? Venkatesh? > + if (proc_gen >= proc_gen_p9) > + dt_add_property_string(dt_root, "vendor", p->sys_vendor); > + else > + dt_add_property_string(dt_root, "vendor", "IBM"); I'd prefer clarity in the spec of when we can expect it to be correct and do things based on it... otherwise, could we go something like: const char* vendor = NULL; if (version >= 0x5f) vendor = p->sys_vendor; if (!vendor || vendor[0]=='\0') // Workaround a bug where we have null vendor vendor = "IBM"; dt_add_property_string(dt_root, "vendor", vendor); ??
On 12/20/2016 10:01 AM, Stewart Smith wrote: > Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: >> + /* >> + * XXX: Spec says vendor information is availale from hdat >> + * version >= 0x5f. But we are getting '00' on version >> + * 0x5f. Hence check for processor generation. >> + */ > > Can we get clarity on what actually happens on 0x5f and above and > clarity in the spec? Venkatesh? > >> + if (proc_gen >= proc_gen_p9) >> + dt_add_property_string(dt_root, "vendor", p->sys_vendor); >> + else >> + dt_add_property_string(dt_root, "vendor", "IBM"); > > I'd prefer clarity in the spec of when we can expect it to be correct > and do things based on it... otherwise, could we go something like: I added processor generation check so that we won't hurt P8 based system. My understanding is starting from P9 we will have this field. Not sure about latest P8 hdat. > > const char* vendor = NULL; > > if (version >= 0x5f) > vendor = p->sys_vendor; > > if (!vendor || vendor[0]=='\0') // Workaround a bug where we have null vendor > vendor = "IBM"; > > dt_add_property_string(dt_root, "vendor", vendor); > Yep. it works. will fix in next version. -Vasant
diff --git a/hdata/spira.c b/hdata/spira.c index fa08824..9481bb6 100644 --- a/hdata/spira.c +++ b/hdata/spira.c @@ -821,6 +821,16 @@ static void add_iplparams_sys_params(const void *iplp, struct dt_node *node) dt_add_property_cells(dt_root, "nest-frequency", hi32(freq), lo32(freq)); } + + /* + * XXX: Spec says vendor information is availale from hdat + * version >= 0x5f. But we are getting '00' on version + * 0x5f. Hence check for processor generation. + */ + if (proc_gen >= proc_gen_p9) + dt_add_property_string(dt_root, "vendor", p->sys_vendor); + else + dt_add_property_string(dt_root, "vendor", "IBM"); } static void add_iplparams_ipl_params(const void *iplp, struct dt_node *node) diff --git a/hdata/test/p8-840-spira.dt b/hdata/test/p8-840-spira.dt index f538d22..ad257af 100644 --- a/hdata/test/p8-840-spira.dt +++ b/hdata/test/p8-840-spira.dt @@ -40,10 +40,10 @@ prop: #size-cells size: 4 val: 00000002 prop: lid-type size: 5 val: 7068797000 prop: compatible size: 24 val: 69626d2c706f7765726e760069626d2c666972656e7a6500 prop: nest-frequency size: 8 val: 0000000077359400 +prop: vendor size: 4 val: 49424d00 prop: skiboot,maxmem size: 8 val: 80000007ffffffff prop: model size: 9 val: 383238362d34314100 prop: model-name size: 22 val: 49424d20506f7765722053797374656d205338313400 -prop: vendor size: 4 val: 49424d00 prop: system-id size: 8 val: 5455303031363300 prop: system-brand size: 3 val: 533000 prop: ibm,hbrt-mini-fdt size: 4096 val: d00dfeed000005c20000012800000528000000280000001100000010000000000000009a000004 diff --git a/hdata/test/p81-811.spira.dt b/hdata/test/p81-811.spira.dt index 2286915..146ec7f 100644 --- a/hdata/test/p81-811.spira.dt +++ b/hdata/test/p81-811.spira.dt @@ -91,10 +91,10 @@ prop: #size-cells size: 4 val: 00000002 prop: lid-type size: 5 val: 7068797000 prop: compatible size: 24 val: 69626d2c706f7765726e760069626d2c666972656e7a6500 prop: nest-frequency size: 8 val: 0000000077359400 +prop: vendor size: 4 val: 49424d00 prop: skiboot,maxmem size: 8 val: 8000001fffffffff prop: model size: 9 val: 383234372d32324c00 prop: model-name size: 23 val: 49424d20506f7765722053797374656d20533832324c00 -prop: vendor size: 4 val: 49424d00 prop: system-id size: 8 val: 3130313043384100 prop: system-brand size: 3 val: 533000 prop: ibm,hbrt-mini-fdt size: 4096 val: d00dfeed0000026e0000012800000214000000280000001100000010000000000000005a000000 diff --git a/hdata/vpd.c b/hdata/vpd.c index d9ee77c..a463170 100644 --- a/hdata/vpd.c +++ b/hdata/vpd.c @@ -649,7 +649,6 @@ static void sysvpd_parse(void) } free(str); - dt_add_property_string(dt_root, "vendor", "IBM"); system_id = vpd_find(sysvpd, sysvpd_sz, "VSYS", "SE", &sz); if (!system_id)
Latest spec added vendor information to IPL PARAMS ntuple. Grab this information when available instead of hardcoding vendor name. Also move vendor property code from vpd.c to iplparms parsing function in spira.c. Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> --- hdata/spira.c | 10 ++++++++++ hdata/test/p8-840-spira.dt | 2 +- hdata/test/p81-811.spira.dt | 2 +- hdata/vpd.c | 1 - 4 files changed, 12 insertions(+), 3 deletions(-)