diff mbox series

[3/3] device-tree vpd/xscom@: add more VPD to xscom@ nodes

Message ID 20180406051646.2938-4-stewart@linux.ibm.com
State Rejected
Headers show
Series Fix various FWTS device tree warnings | expand

Commit Message

Stewart Smith April 6, 2018, 5:16 a.m. UTC 
From: Stewart Smith <stewart@linux.vnet.ibm.com>

Notably, add board-info and vendor (if in VPD), which is
expected by FWTS and was present on POWER8.

We get properties like:
serial-number    "YA1934092546"
board-info       "PROCESSOR MODULE"
part-number      "02AA863"
vendor           "IBM             "

This change will also affect what we put in the /vpd hierarchy,
but by making it more complete.

On POWER8 OpenPOWER systems, these were a mix of read from the VPD
by hostboot and added to the device tree and hard coded.

Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
---
 hdata/test/p8-840-spira.dts  |  2 ++
 hdata/test/p81-811.spira.dts |  4 ++++
 hdata/vpd.c                  | 41 ++++++++++++++++++++++-------------------
 3 files changed, 28 insertions(+), 19 deletions(-)

Comments

Vasant Hegde April 7, 2018, 11:58 a.m. UTC | #1 
On 04/06/2018 10:46 AM, Stewart Smith wrote:
> From: Stewart Smith <stewart@linux.vnet.ibm.com>
> 
> Notably, add board-info and vendor (if in VPD), which is
> expected by FWTS and was present on POWER8.
> 
> We get properties like:
> serial-number    "YA1934092546"
> board-info       "PROCESSOR MODULE"

How did you get this property? I don't  think we are adding this property.

Also this patch broke `make check` for me.

> part-number      "02AA863"
> vendor           "IBM             "
> 
> This change will also affect what we put in the /vpd hierarchy,
> but by making it more complete.

AFAIK VINI, VRML and OPFR are mutually exclusive. We should expect either one of 
these type VPD not mixed one (Of course in some system we are getting mixed 
one). IMO that's bug in hostboot side of fix.

-Vasant
Stewart Smith April 10, 2018, 4:25 a.m. UTC | #2 
Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes:
> On 04/06/2018 10:46 AM, Stewart Smith wrote:
>> From: Stewart Smith <stewart@linux.vnet.ibm.com>
>> 
>> Notably, add board-info and vendor (if in VPD), which is
>> expected by FWTS and was present on POWER8.
>> 
>> We get properties like:
>> serial-number    "YA1934092546"
>> board-info       "PROCESSOR MODULE"
>
> How did you get this property? I don't  think we are adding this
> property.

It's coming from the VPD, at least on my witherspoon it is.

> Also this patch broke `make check` for me.

Doh, it shouldn't have... I may have to check that then :)

>> part-number      "02AA863"
>> vendor           "IBM             "
>> 
>> This change will also affect what we put in the /vpd hierarchy,
>> but by making it more complete.
>
> AFAIK VINI, VRML and OPFR are mutually exclusive. We should expect either one of 
> these type VPD not mixed one (Of course in some system we are getting mixed 
> one). IMO that's bug in hostboot side of fix.

yeah, we're certainly getting things mixed together in the VPD blobs
that we export in the DT at least. I'm not sure where Hostboot picks
these up from though and if we're really just getting a few things
mashed together.

Oliver/Ananth, by any chance do you know?
Ananth N Mavinakayanahalli April 10, 2018, 5:56 a.m. UTC | #3 
On Tue, Apr 10, 2018 at 02:25:24PM +1000, Stewart Smith wrote:
> Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes:
> > On 04/06/2018 10:46 AM, Stewart Smith wrote:
> >> From: Stewart Smith <stewart@linux.vnet.ibm.com>
> 
> >> part-number      "02AA863"
> >> vendor           "IBM             "
> >> 
> >> This change will also affect what we put in the /vpd hierarchy,
> >> but by making it more complete.
> >
> > AFAIK VINI, VRML and OPFR are mutually exclusive. We should expect either one of 
> > these type VPD not mixed one (Of course in some system we are getting mixed 
> > one). IMO that's bug in hostboot side of fix.
> 
> yeah, we're certainly getting things mixed together in the VPD blobs
> that we export in the DT at least. I'm not sure where Hostboot picks
> these up from though and if we're really just getting a few things
> mashed together.
> 
> Oliver/Ananth, by any chance do you know?

AFAIR, all the OP* keyword VPD was added exclusively for OP boxes, but
whether or not the older keywords continue to exist with them was
unclear at the time. I am checking with the FW folks... will update
with findings...

Ananth
Stewart Smith July 26, 2018, 7:58 a.m. UTC | #4 
Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> writes:
> On Tue, Apr 10, 2018 at 02:25:24PM +1000, Stewart Smith wrote:
>> Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes:
>> > On 04/06/2018 10:46 AM, Stewart Smith wrote:
>> >> From: Stewart Smith <stewart@linux.vnet.ibm.com>
>> 
>> >> part-number      "02AA863"
>> >> vendor           "IBM             "
>> >> 
>> >> This change will also affect what we put in the /vpd hierarchy,
>> >> but by making it more complete.
>> >
>> > AFAIK VINI, VRML and OPFR are mutually exclusive. We should expect either one of 
>> > these type VPD not mixed one (Of course in some system we are getting mixed 
>> > one). IMO that's bug in hostboot side of fix.
>> 
>> yeah, we're certainly getting things mixed together in the VPD blobs
>> that we export in the DT at least. I'm not sure where Hostboot picks
>> these up from though and if we're really just getting a few things
>> mashed together.
>> 
>> Oliver/Ananth, by any chance do you know?
>
> AFAIR, all the OP* keyword VPD was added exclusively for OP boxes, but
> whether or not the older keywords continue to exist with them was
> unclear at the time. I am checking with the FW folks... will update
> with findings...

Any findings?

I have to admit I haven't gone back and looked at this closely either,
but maybe I should :)
Ananth N Mavinakayanahalli July 26, 2018, 9:11 a.m. UTC | #5 
On Thu, Jul 26, 2018 at 05:58:30PM +1000, Stewart Smith wrote:
> Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> writes:
> > On Tue, Apr 10, 2018 at 02:25:24PM +1000, Stewart Smith wrote:
> >> Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes:
> >> > On 04/06/2018 10:46 AM, Stewart Smith wrote:
> >> >> From: Stewart Smith <stewart@linux.vnet.ibm.com>
> >> 
> >> >> part-number      "02AA863"
> >> >> vendor           "IBM             "
> >> >> 
> >> >> This change will also affect what we put in the /vpd hierarchy,
> >> >> but by making it more complete.
> >> >
> >> > AFAIK VINI, VRML and OPFR are mutually exclusive. We should expect either one of 
> >> > these type VPD not mixed one (Of course in some system we are getting mixed 
> >> > one). IMO that's bug in hostboot side of fix.
> >> 
> >> yeah, we're certainly getting things mixed together in the VPD blobs
> >> that we export in the DT at least. I'm not sure where Hostboot picks
> >> these up from though and if we're really just getting a few things
> >> mashed together.
> >> 
> >> Oliver/Ananth, by any chance do you know?
> >
> > AFAIR, all the OP* keyword VPD was added exclusively for OP boxes, but
> > whether or not the older keywords continue to exist with them was
> > unclear at the time. I am checking with the FW folks... will update
> > with findings...
> 
> Any findings?
> 
> I have to admit I haven't gone back and looked at this closely either,
> but maybe I should :)

As of now, older keywords will continue to exist. In the future, VINI, VSRC,
VRTN and VMSC records will be deprecated.

Ananth
Vasant Hegde July 28, 2018, 2:06 p.m. UTC | #6 
On 07/26/2018 02:41 PM, Ananth N Mavinakayanahalli wrote:
> On Thu, Jul 26, 2018 at 05:58:30PM +1000, Stewart Smith wrote:
>> Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> writes:
>>> On Tue, Apr 10, 2018 at 02:25:24PM +1000, Stewart Smith wrote:
>>>> Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes:
>>>>> On 04/06/2018 10:46 AM, Stewart Smith wrote:
>>>>>> From: Stewart Smith <stewart@linux.vnet.ibm.com>
>>>>
>>>>>> part-number      "02AA863"
>>>>>> vendor           "IBM             "
>>>>>>
>>>>>> This change will also affect what we put in the /vpd hierarchy,
>>>>>> but by making it more complete.
>>>>>
>>>>> AFAIK VINI, VRML and OPFR are mutually exclusive. We should expect either one of
>>>>> these type VPD not mixed one (Of course in some system we are getting mixed
>>>>> one). IMO that's bug in hostboot side of fix.
>>>>
>>>> yeah, we're certainly getting things mixed together in the VPD blobs
>>>> that we export in the DT at least. I'm not sure where Hostboot picks
>>>> these up from though and if we're really just getting a few things
>>>> mashed together.
>>>>
>>>> Oliver/Ananth, by any chance do you know?
>>>
>>> AFAIR, all the OP* keyword VPD was added exclusively for OP boxes, but
>>> whether or not the older keywords continue to exist with them was
>>> unclear at the time. I am checking with the FW folks... will update
>>> with findings...
>>
>> Any findings?
>>
>> I have to admit I haven't gone back and looked at this closely either,
>> but maybe I should :)
> 
> As of now, older keywords will continue to exist. In the future, VINI, VSRC,
> VRTN and VMSC records will be deprecated.

Unfortunately we have all sorts of mix and match (like : VINI and OPFR in same 
vpd blob for
module vpd).  And we will continue to have it for P9 generation. Hopefully it 
will be cleaned
up properly in future.

So for now I think what we have in Device tree is sufficient. Lets fix FWTS itself.


-Vasant
diff mbox series

Patch

diff --git a/hdata/test/p8-840-spira.dts b/hdata/test/p8-840-spira.dts
index 62a2f52bc327..1971e0044727 100644
--- a/hdata/test/p8-840-spira.dts
+++ b/hdata/test/p8-840-spira.dts
@@ -968,6 +968,7 @@ 
 		ibm,module-vpd = <0xcafebeef 0x10000 0xa502aa2f>;
 		part-number = "00KV631";
 		serial-number = "YA1932063562";
+		board-info = "06-WAY PROC CUOD";
 		ibm,ccm-node-id = <0x0>;
 		ibm,hw-card-id = <0x0>;
 		ibm,hw-module-id = <0x0>;
@@ -1033,6 +1034,7 @@ 
 		ibm,module-vpd = <0xcafebeef 0x10000 0x9f87fa41>;
 		part-number = "00KV631";
 		serial-number = "YA1932063562";
+		board-info = "06-WAY PROC CUOD";
 		ibm,ccm-node-id = <0x0>;
 		ibm,hw-card-id = <0x0>;
 		ibm,hw-module-id = <0x0>;
diff --git a/hdata/test/p81-811.spira.dts b/hdata/test/p81-811.spira.dts
index ba9b3551f630..eb9972fc7d0d 100644
--- a/hdata/test/p81-811.spira.dts
+++ b/hdata/test/p81-811.spira.dts
@@ -2187,6 +2187,7 @@ 
 		ibm,module-vpd = <0xcafebeef 0x10000 0x3e067c18>;
 		part-number = "00KV627";
 		serial-number = "YA1932096951";
+		board-info = "10-WAY PROC CUOD";
 		ibm,ccm-node-id = <0x0>;
 		ibm,hw-card-id = <0x0>;
 		ibm,hw-module-id = <0x0>;
@@ -2252,6 +2253,7 @@ 
 		ibm,module-vpd = <0xcafebeef 0x10000 0x1b85218f>;
 		part-number = "00KV627";
 		serial-number = "YA1932096951";
+		board-info = "10-WAY PROC CUOD";
 		ibm,ccm-node-id = <0x0>;
 		ibm,hw-card-id = <0x0>;
 		ibm,hw-module-id = <0x0>;
@@ -2314,6 +2316,7 @@ 
 		ibm,module-vpd = <0xcafebeef 0x10000 0x36f99ead>;
 		part-number = "00KV627";
 		serial-number = "YA1932096950";
+		board-info = "10-WAY PROC CUOD";
 		ibm,ccm-node-id = <0x0>;
 		ibm,hw-card-id = <0x0>;
 		ibm,hw-module-id = <0x1>;
@@ -2367,6 +2370,7 @@ 
 		ibm,module-vpd = <0xcafebeef 0x10000 0x65fe8f66>;
 		part-number = "00KV627";
 		serial-number = "YA1932096950";
+		board-info = "10-WAY PROC CUOD";
 		ibm,ccm-node-id = <0x0>;
 		ibm,hw-card-id = <0x0>;
 		ibm,hw-module-id = <0x1>;
diff --git a/hdata/vpd.c b/hdata/vpd.c
index 038569af072a..76bc15bc184c 100644
--- a/hdata/vpd.c
+++ b/hdata/vpd.c
@@ -234,27 +234,27 @@  static void vpd_opfr_parse(struct dt_node *node,
 
 	/* Vendor Name */
 	kw = vpd_find(fruvpd, fruvpd_sz, "OPFR", "VN", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "vendor"))
 		dt_add_property_nstr(node, "vendor", kw, sz);
 
 	/* FRU Description */
 	kw = vpd_find(fruvpd, fruvpd_sz, "OPFR", "DR", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "description"))
 		dt_add_property_nstr(node, "description", kw, sz);
 
 	/* Part number */
 	kw = vpd_find(fruvpd, fruvpd_sz, "OPFR", "VP", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "part-number"))
 		dt_add_property_nstr(node, "part-number", kw, sz);
 
 	/* Serial number */
 	kw = vpd_find(fruvpd, fruvpd_sz, "OPFR", "VS", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "serial-number"))
 		dt_add_property_nstr(node, "serial-number", kw, sz);
 
 	/* Build date in BCD */
 	kw = vpd_find(fruvpd, fruvpd_sz, "OPFR", "MB", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "build-date"))
 		dt_add_property_nstr(node, "build-date", kw, sz);
 
 	return;
@@ -271,12 +271,12 @@  static void vpd_vrml_parse(struct dt_node *node,
 
 	/* Part number */
 	kw = vpd_find(fruvpd, fruvpd_sz, "VRML", "PN", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "part-number"))
 		dt_add_property_nstr(node, "part-number", kw, sz);
 
 	/* Serial number */
 	kw = vpd_find(fruvpd, fruvpd_sz, "VRML", "SN", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "serial-number"))
 		dt_add_property_nstr(node, "serial-number", kw, sz);
 
 	return;
@@ -291,47 +291,47 @@  static void vpd_vini_parse(struct dt_node *node,
 
 	/* FRU Stocking Part Number */
 	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "FN", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "fru-number"))
 		dt_add_property_nstr(node, "fru-number", kw, sz);
 
 	/* Serial Number */
 	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "SN", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "serial-number"))
 		dt_add_property_nstr(node, "serial-number", kw, sz);
 
 	/* Part Number */
 	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "PN", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "part-number"))
 		dt_add_property_nstr(node, "part-number", kw, sz);
 
 	/* Vendor Name */
 	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "VN", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "vendor"))
 		dt_add_property_nstr(node, "vendor", kw, sz);
 
 	/* CCIN Extension */
 	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CE", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "ccin-extension"))
 		dt_add_property_nstr(node, "ccin-extension", kw, sz);
 
 	/* HW Version info */
 	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "HW", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "hw-version"))
 		dt_add_property_nstr(node, "hw-version", kw, sz);
 
 	/* Card type info */
 	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CT", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "card-type"))
 		dt_add_property_nstr(node, "card-type", kw, sz);
 
 	/* HW characteristics info */
 	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "B3", &sz);
-	if (kw)
+	if (kw && !dt_find_property(node, "hw-characteristics"))
 		dt_add_property_nstr(node, "hw-characteristics", kw, sz);
 
 	/* Customer Card Identification Number (CCIN) */
 	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CC", &sz);
-	if (kw) {
+	if (kw && !dt_find_property(node, "ccin")) {
 		dt_add_property_nstr(node, "ccin", kw, sz);
 
 		cinfo = card_info_lookup((char *)kw);
@@ -366,12 +366,15 @@  static bool valid_child_entry(const struct slca_entry *entry)
 
 void vpd_data_parse(struct dt_node *node, const void *fruvpd, u32 fruvpd_sz)
 {
+	/*
+	 * Whatever gets found first stays, if there's duplicates,
+	 * first in wins.
+	 */
 	if (vpd_find_record(fruvpd, fruvpd_sz, "OPFR", NULL))
 		vpd_opfr_parse(node, fruvpd, fruvpd_sz);
-	else if (vpd_find_record(fruvpd, fruvpd_sz, "VRML", NULL))
+	if (vpd_find_record(fruvpd, fruvpd_sz, "VRML", NULL))
 		vpd_vrml_parse(node, fruvpd, fruvpd_sz);
-	else
-		vpd_vini_parse(node, fruvpd, fruvpd_sz);
+	vpd_vini_parse(node, fruvpd, fruvpd_sz);
 }
 
 /* Create the /vpd node and add its children */