| Message ID | 20170828071803.9406-5-hegdevasant@linux.vnet.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/5] hdata: Fix vpd parse | expand |
Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: > Parse SPD data and populate device tree. > > list of properites parsing from SPD: > ----------------------------------- > [root@ltc-wspoon dimm@d00f]# lsprop . > memory-id 0000000c (12) <-- DIMM type > device_type "memory-dimm-ddr4" > serial 15d9ad22 (366587170) > part-number "36ASF2G72PZ-2G6B2 " > manufacturer-id 0000802c (32812) <-- Vendor ID, we can get vendor name from this ID > > Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > --- > hdata/memory.c | 43 ++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 42 insertions(+), 1 deletion(-) Does this match well enough whats in doc/device-tree/vpd.rst ? There should probably be a couple of updates there, as it'll mean it's no longer FSP machine specific :) > > diff --git a/hdata/memory.c b/hdata/memory.c > index d1570eb..956e0d7 100644 > --- a/hdata/memory.c > +++ b/hdata/memory.c > @@ -319,16 +319,49 @@ static void vpd_add_ram_area(const struct HDIF_common_hdr *msarea) > } > } > > +static void vpd_parse_spd(struct dt_node *dimm, const char *spd, u32 size) > +{ > + u16 *vendor; > + u32 *sn; > + > + /* SPD is too small */ > + if (size < 512) { > + prlog(PR_WARNING, "MSVPD: Invalid SPD size. " > + "Expected 512 byts, got %d\n", size); > + return; > + } > + > + /* Supports DDR4 format pasing only */ > + if (spd[0x2] < 0xc) { > + prlog(PR_WARNING, > + "MSVPD: SPD format (%x) not supported\n", spd[0x2]); > + return; > + } For both of these we should probably have FWTS annotations explaining what's wrong. > + > + dt_add_property_string(dimm, "device_type", "memory-dimm-ddr4"); > + > + dt_add_property_cells(dimm, "memory-id", be16_to_cpu(spd[0x2])); > + > + sn = (u32 *)&spd[0x145]; > + dt_add_property_cells(dimm, "serial", be32_to_cpu(*sn)); > + > + dt_add_property_nstr(dimm, "part-number", &spd[0x149], 20); > + > + vendor = (u16 *)&spd[0x140]; > + dt_add_property_cells(dimm, "manufacturer-id", be16_to_cpu(*vendor)); > +} Do we have a good way to link back DIMM to memory address?
On 09/01/2017 12:39 PM, Stewart Smith wrote: > Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: >> Parse SPD data and populate device tree. >> >> list of properites parsing from SPD: >> ----------------------------------- >> [root@ltc-wspoon dimm@d00f]# lsprop . >> memory-id 0000000c (12) <-- DIMM type >> device_type "memory-dimm-ddr4" >> serial 15d9ad22 (366587170) >> part-number "36ASF2G72PZ-2G6B2 " >> manufacturer-id 0000802c (32812) <-- Vendor ID, we can get vendor name from this ID >> >> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> >> --- >> hdata/memory.c | 43 ++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 42 insertions(+), 1 deletion(-) > > Does this match well enough whats in doc/device-tree/vpd.rst ? There > should probably be a couple of updates there, as it'll mean it's no > longer FSP machine specific :) Sure will update documentation. > >> >> diff --git a/hdata/memory.c b/hdata/memory.c >> index d1570eb..956e0d7 100644 >> --- a/hdata/memory.c >> +++ b/hdata/memory.c >> @@ -319,16 +319,49 @@ static void vpd_add_ram_area(const struct HDIF_common_hdr *msarea) >> } >> } >> >> +static void vpd_parse_spd(struct dt_node *dimm, const char *spd, u32 size) >> +{ >> + u16 *vendor; >> + u32 *sn; >> + >> + /* SPD is too small */ >> + if (size < 512) { >> + prlog(PR_WARNING, "MSVPD: Invalid SPD size. " >> + "Expected 512 byts, got %d\n", size); >> + return; >> + } >> + >> + /* Supports DDR4 format pasing only */ >> + if (spd[0x2] < 0xc) { >> + prlog(PR_WARNING, >> + "MSVPD: SPD format (%x) not supported\n", spd[0x2]); >> + return; >> + } > > For both of these we should probably have FWTS annotations explaining > what's wrong. Actually we are parsing DDR4 dimms only. If we endup having different dimms then we will throw warning unnecessarily. > >> + >> + dt_add_property_string(dimm, "device_type", "memory-dimm-ddr4"); >> + >> + dt_add_property_cells(dimm, "memory-id", be16_to_cpu(spd[0x2])); >> + >> + sn = (u32 *)&spd[0x145]; >> + dt_add_property_cells(dimm, "serial", be32_to_cpu(*sn)); >> + >> + dt_add_property_nstr(dimm, "part-number", &spd[0x149], 20); >> + >> + vendor = (u16 *)&spd[0x140]; >> + dt_add_property_cells(dimm, "manufacturer-id", be16_to_cpu(*vendor)); >> +} > > Do we have a good way to link back DIMM to memory address? We can map this to address range. But haven't found concrete way to map dimm to actual address. Let me see if I can get this info. -Vasant
diff --git a/hdata/memory.c b/hdata/memory.c index d1570eb..956e0d7 100644 --- a/hdata/memory.c +++ b/hdata/memory.c @@ -319,16 +319,49 @@ static void vpd_add_ram_area(const struct HDIF_common_hdr *msarea) } } +static void vpd_parse_spd(struct dt_node *dimm, const char *spd, u32 size) +{ + u16 *vendor; + u32 *sn; + + /* SPD is too small */ + if (size < 512) { + prlog(PR_WARNING, "MSVPD: Invalid SPD size. " + "Expected 512 byts, got %d\n", size); + return; + } + + /* Supports DDR4 format pasing only */ + if (spd[0x2] < 0xc) { + prlog(PR_WARNING, + "MSVPD: SPD format (%x) not supported\n", spd[0x2]); + return; + } + + dt_add_property_string(dimm, "device_type", "memory-dimm-ddr4"); + + dt_add_property_cells(dimm, "memory-id", be16_to_cpu(spd[0x2])); + + sn = (u32 *)&spd[0x145]; + dt_add_property_cells(dimm, "serial", be32_to_cpu(*sn)); + + dt_add_property_nstr(dimm, "part-number", &spd[0x149], 20); + + vendor = (u16 *)&spd[0x140]; + dt_add_property_cells(dimm, "manufacturer-id", be16_to_cpu(*vendor)); +} + static void add_mca_dimm_info(struct dt_node *mca, const struct HDIF_common_hdr *msarea) { - unsigned int i; + unsigned int i, size; const struct HDIF_child_ptr *ramptr; const struct HDIF_common_hdr *ramarea; const struct spira_fru_id *fru_id; const struct HDIF_ram_area_id *ram_id; const struct HDIF_ram_area_size *ram_area_sz; struct dt_node *dimm; + const void *vpd_blob; ramptr = HDIF_child_arr(msarea, 0); if (!CHECK_SPPTR(ramptr)) { @@ -371,6 +404,14 @@ static void add_mca_dimm_info(struct dt_node *mca, dt_add_property_string(dimm, "status", "okay"); else dt_add_property_string(dimm, "status", "disabled"); + + vpd_blob = HDIF_get_idata(ramarea, 1, &size); + if (!CHECK_SPPTR(vpd_blob)) + continue; + if (vpd_valid(vpd_blob, size)) + vpd_data_parse(dimm, vpd_blob, size); + else + vpd_parse_spd(dimm, vpd_blob, size); } }
Parse SPD data and populate device tree. list of properites parsing from SPD: ----------------------------------- [root@ltc-wspoon dimm@d00f]# lsprop . memory-id 0000000c (12) <-- DIMM type device_type "memory-dimm-ddr4" serial 15d9ad22 (366587170) part-number "36ASF2G72PZ-2G6B2 " manufacturer-id 0000802c (32812) <-- Vendor ID, we can get vendor name from this ID Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> --- hdata/memory.c | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-)