[S] Change in coreboot[main]: lib: Use platform-independent types in bmp_load_logo()

[Julius Werner (Code Review)]

Attention is currently required from: Andrey Petrov, Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, Ronak Kanabar, Sean Rhodes, Subrata Banik.

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81618?usp=email )

Change subject: lib: Use platform-independent types in bmp_load_logo()
......................................................................

Patch Set 1:

(2 comments)

File src/lib/bmp_logo.c:

https://review.coreboot.org/c/coreboot/+/81618/comment/f173b881_bf26f6e4 :
PS1, Line 19: void bmp_load_logo(uintptr_t *logo_ptr, size_t *logo_size)
Why does this function take two out-pointers and have no return value, anyway? I think that's a very weird design, and it's actually problematic because the function has no way to return errors. Functions like `fsp_convert_bmp_to_gop_blt()` check if `logo_ptr` is `NULL` afterwards to check for error, but that's not actually correct because this function doesn't set it to `NULL` before returning (and `fsp_convert_bmp_to_gop_blt()` didn't initialize the pointer it passes in).

I think it would make much more sense if this function returns a void pointer and only takes the `size_t *logo_size` argument, and if the returned pointer is `NULL` that signals error. If the caller needs a `uintptr_t` (some of them actually don't, e.g. `fsp_convert_bmp_to_gop_blt()` would be better off with a `void *` anyway), it can do the cast itself.

File src/soc/amd/mendocino/fsp_s_params.c:

https://review.coreboot.org/c/coreboot/+/81618/comment/d08b89c0_c9a3d1e2 :
PS1, Line 59: (uintptr_t *)
You can't do these kinds of things. You can't take a pointer to `uint32_t`, cast it to a pointer to `uintptr_t`, and then have another function write into that. If this code is running in 64-bit mode, then `uintptr_t` will be 8 bytes long and writing into the `uint32_t` location you're passing in here will overwrite the next 4 bytes after it.

If you want to do something like this, you'll need to put an intermediate variable of the right size on the stack to pass in, and do a narrowing copy later (same applies to `size_t` vs. `uint32_t`, of course):
```
uintptr_t logo_buffer;
bmp_load_logo(&logo_buffer, ...);
supd->FspsConfig.logo_bmp_buffer = logo_buffer;
```