On Wed, Sep 1, 2021 at 10:57 AM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> No worries. I enjoyed seeing the AMD code-names in the conflicts, they
> are using positively kernel-level naming.
Oh, I spoke too soon.
The conflict in amdgpu_ras_eeprom.c is trivial to fix up, but the
*code* is garbage.
It does this (from commit 14fb496a84f1: "drm/amdgpu: set RAS EEPROM
address from VBIOS"):
...
control->i2c_address = 0;
if (amdgpu_atomfirmware_ras_rom_addr(adev,
(uint8_t*)&control->i2c_address))
{
if (control->i2c_address == 0xA0)
control->i2c_address = 0;
...
and honestly, that just hurts to look at. It's completely wrong, even
if it happens to work on a little-endian machine.
Yes, yes, BE is irrelevant, and doubly so for an AMD GPU driver, but still.
It's assigning a 8-bit value to a 32-bit entity by doing a pointer
cast on the address, and then mixing things up by using/assigning to
that same field.
That's just *wrong* and nasty.
Oh, the resolution would be easy - just take that broken code as-is -
but I can't actually make myself do that.
So I fixed it up to not be that incredibly ugly garbage.
Please holler if I did something wrong.
Linus
