On Wed, Sep 1, 2021 at 2:33 PM Linus Torvalds torvalds@linux-foundation.org wrote:
On Wed, Sep 1, 2021 at 10:57 AM Linus Torvalds torvalds@linux-foundation.org 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.
Fix looks good. Thanks,
Alex