[Skiboot] [PATCH 3/7] hdat: handle ISDIMM SPD blobs

[Vasant Hegde]

On 01/13/2017 12:26 PM, Oliver O'Halloran wrote:
> Previously the HDAT format was only ever used with IBM hardware so it
> would store vital product data (VPD) blobs in the IBM ASCII Keyword VPD
> format. With P9 HDAT is used on OpenPower machines which use Industry
> Standard DIMMs that provide their product data through a "Serial Present
> Detect" EEPROM mounted on the DIMM.
>
> The SPD blob has a different format and is exported in the device-tree
> under the "spd" property rather than the "ibm,vpd" property. This patch
> adds support for recognising these blobs and placing them in the
> appropriate DT property.
>
> Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
> ---
>   hdata/memory.c | 37 ++++++++++++++++++++++++++++---------
>   hdata/vpd.c    |  3 ++-
>   2 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/hdata/memory.c b/hdata/memory.c
> index 2b23aeaff03d..f95e67e2f2b8 100644
> --- a/hdata/memory.c
> +++ b/hdata/memory.c
> @@ -269,6 +269,7 @@ static void vpd_add_ram_area(const struct HDIF_common_hdr *msarea)
>   	const struct HDIF_ram_area_id *ram_id;
>   	struct dt_node *ram_node;
>   	u32 chip_id;
> +	const void *vpd_blob;
>
>   	ramptr = HDIF_child_arr(msarea, 0);
>   	if (!CHECK_SPPTR(ramptr)) {
> @@ -285,15 +286,33 @@ static void vpd_add_ram_area(const struct HDIF_common_hdr *msarea)
>   		if (!CHECK_SPPTR(ram_id))
>   			continue;
>
> -		if ((be16_to_cpu(ram_id->flags) & RAM_AREA_INSTALLED) &&
> -		    (be16_to_cpu(ram_id->flags) & RAM_AREA_FUNCTIONAL)) {
> -			ram_node = dt_add_vpd_node(ramarea, 0, 1);
> -			if (ram_node) {
> -				chip_id = add_chip_id_to_ram_area(msarea,
> -								  ram_node);
> -				add_bus_freq_to_ram_area(ram_node, chip_id);
> -				add_size_to_ram_area(ram_node, ramarea, 1);
> -			}
> +		/* Don't add VPD for non-existent RAM */
> +		if (!(be16_to_cpu(ram_id->flags) & RAM_AREA_INSTALLED))

Not sure why you removed RAM_AREA_FUNCTIONAL checking. I think we should retain 
that check.


> +			continue;
> +
> +		ram_node = dt_add_vpd_node(ramarea, 0, 1);
> +		if (!ram_node)
> +			continue;
> +
> +		chip_id = add_chip_id_to_ram_area(msarea, ram_node);
> +		add_bus_freq_to_ram_area(ram_node, chip_id);
> +
> +		vpd_blob = HDIF_get_idata(ramarea, 1, &ram_sz);

This works fine on FSP based system. I haven't got chance to play with  hostboot 
generated HDAT. Hopefully above logic works on Open Power system.

> +
> +		/*
> +		 * For direct-attached memory we have a DDR "Serial
> +		 * Presence Detection" blob rather than an IBM keyword
> +		 * blob.
> +		 */
> +		if (vpd_valid(vpd_blob, ram_sz)) {
> +			/* the ibm,vpd blob was added in dt_add_vpd_node() */
> +			add_size_to_ram_area(ram_node, ramarea, 1);
> +		} else {
> +			/*
> +			 * FIXME: There's probably a way to calculate the
> +			 * size of the DIMM from the SPD info.


Yes. SPD spec has that information. We can add it later.

-Vasant

> +			 */
> +			dt_add_property(ram_node, "spd", vpd_blob, ram_sz);
>   		}
>   	}
>   }
> diff --git a/hdata/vpd.c b/hdata/vpd.c
> index d9ee77c6335b..cb7a8daa398f 100644
> --- a/hdata/vpd.c
> +++ b/hdata/vpd.c
> @@ -590,7 +590,8 @@ struct dt_node *dt_add_vpd_node(const struct HDIF_common_hdr *hdr,
>   	}
>
>   	/* Parse VPD fields, ensure that it has not been added already */
> -	if (!dt_find_property(node, "ibm,vpd")) {
> +	if (vpd_valid(fruvpd, fruvpd_sz)
> +	    && !dt_find_property(node, "ibm,vpd")) {
>   		dt_add_property(node, "ibm,vpd", fruvpd, fruvpd_sz);
>   		vpd_vini_parse(node, fruvpd, fruvpd_sz);
>   	}
>