Change in coreboot[master]: soc/qualcomm/common: Add dram information to CBMEM table

[Julius Werner (Code Review)]

Attention is currently required from: Ravi kumar.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59195 )

Change subject: soc/qualcomm/common: Add dram information to CBMEM table
......................................................................

Patch Set 42:

(5 comments)

Patchset:

PS42: 
Argh... this is what I get for typing too slowly. This CL is broken in several ways, please submit a follow-up patch shortly.

File src/soc/qualcomm/common/qclib.c:

https://review.coreboot.org/c/coreboot/+/59195/comment/ba81bfde_6b1d4bf3 
PS41, Line 37: 	if (sizeof(struct mem_chip_info) != 0)
What is this supposed to test, this can never be false? The thing you were supposed to check is `(te->size == sizeof(struct mem_chip_info))` up there in write_mem_chip_information(), and if not you don't set the mem_chip_addr pointer. Except that now since we changed the channels array in struct mem_chip_info to [0] (instead of [2]), that test needs to be a little more complicated: `(te->size > sizeof(struct mem_chip_info) && te->size == mem_chip_info_size((void *)te->blob_address)` (see below for mem_chip_info_size()).

And then here in this function you should have

if (!mem_chip_addr) {
   printk(BIOS_ERR, "Did not receive valid mem_chip_info from QcLib!");
   return;
 }

https://review.coreboot.org/c/coreboot/+/59195/comment/e14f4ed9_3031295f 
PS41, Line 39: sizeof(struct mem_chip_info)
This is no longer correct now because we changed the declaration of the channels array in struct mem_chip_info from [2] to [0]. We need a way to determine the total size of a mem_chip_info in multiple places... it's probably worth adding a helper function to mem_chip_info.h for that:

size_t mem_chip_info_size(struct mem_chip_info *info) {
   return sizeof(*info) + sizeof(info->channels[0]) * info->num_channels;
 };

https://review.coreboot.org/c/coreboot/+/59195/comment/26e5d093_4aba453a 
PS41, Line 43: sizeof(struct mem_chip_info)
Same here.

https://review.coreboot.org/c/coreboot/+/59195/comment/c224acae_f124ba0b 
PS41, Line 174: mem_chip_addr, sizeof(mem_chip_addr)
This was supposed to be NULL and 0, and the comment above should clarify that the blob_address for this entry will be overwritten by QcLib to point to an info struct that it allocates itself.