[Skiboot] [PATCH v2 5/5] hdata: Parse SPD data

[Vasant Hegde]

On 09/01/2017 12:39 PM, Stewart Smith wrote:
> Vasant Hegde <hegdevasant at linux.vnet.ibm.com> writes:
>> Parse SPD data and populate device tree.
>>
>> list of properites parsing from SPD:
>> -----------------------------------
>> [root at ltc-wspoon dimm at 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 at 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