Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/amdgpu: Fixes to returning VBIOS RAS EEPROM address

1) Generalize the function--if the user didn't set
i2c_address, still return true/false to
indicate whether VBIOS contains the RAS EEPROM
address. This function shouldn't evaluate
whether the user set the i2c_address pointer or
not.

2) Don't touch the caller's i2c_address, unless
you have to--this function shouldn't have side
effects.

3) Correctly set the function comment as a
kernel-doc comment.

Cc: John Clements <john.clements@amd.com>
Cc: Hawking Zhang <Hawking.Zhang@amd.com>
Cc: Alex Deucher <Alexander.Deucher@amd.com>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
Reviewed-by: Alex Deucher <Alexander.Deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

authored by

Luben Tuikov and committed by
Alex Deucher
a6a355a2 fbd2a600

+33 -17
+33 -17
drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
··· 468 468 return (fw_cap & ATOM_FIRMWARE_CAP_DYNAMIC_BOOT_CFG_ENABLE) ? true : false; 469 469 } 470 470 471 - /* 472 - * Helper function to query RAS EEPROM address 471 + /** 472 + * amdgpu_atomfirmware_ras_rom_addr -- Get the RAS EEPROM addr from VBIOS 473 + * adev: amdgpu_device pointer 474 + * i2c_address: pointer to u8; if not NULL, will contain 475 + * the RAS EEPROM address if the function returns true 473 476 * 474 - * @adev: amdgpu_device pointer 475 - * 476 - * Return true if vbios supports ras rom address reporting 477 + * Return true if VBIOS supports RAS EEPROM address reporting, 478 + * else return false. If true and @i2c_address is not NULL, 479 + * will contain the RAS ROM address. 477 480 */ 478 - bool amdgpu_atomfirmware_ras_rom_addr(struct amdgpu_device *adev, uint8_t* i2c_address) 481 + bool amdgpu_atomfirmware_ras_rom_addr(struct amdgpu_device *adev, 482 + u8 *i2c_address) 479 483 { 480 484 struct amdgpu_mode_info *mode_info = &adev->mode_info; 481 485 int index; ··· 487 483 union firmware_info *firmware_info; 488 484 u8 frev, crev; 489 485 490 - if (i2c_address == NULL) 491 - return false; 492 - 493 - *i2c_address = 0; 494 - 495 486 index = get_index_into_master_table(atom_master_list_of_data_tables_v2_1, 496 - firmwareinfo); 487 + firmwareinfo); 497 488 498 489 if (amdgpu_atom_parse_data_header(adev->mode_info.atom_context, 499 - index, &size, &frev, &crev, &data_offset)) { 490 + index, &size, &frev, &crev, 491 + &data_offset)) { 500 492 /* support firmware_info 3.4 + */ 501 493 if ((frev == 3 && crev >=4) || (frev > 3)) { 502 494 firmware_info = (union firmware_info *) 503 495 (mode_info->atom_context->bios + data_offset); 504 - *i2c_address = firmware_info->v34.ras_rom_i2c_slave_addr; 496 + /* The ras_rom_i2c_slave_addr should ideally 497 + * be a 19-bit EEPROM address, which would be 498 + * used as is by the driver; see top of 499 + * amdgpu_eeprom.c. 500 + * 501 + * When this is the case, 0 is of course a 502 + * valid RAS EEPROM address, in which case, 503 + * we'll drop the first "if (firm...)" and only 504 + * leave the check for the pointer. 505 + * 506 + * The reason this works right now is because 507 + * ras_rom_i2c_slave_addr contains the EEPROM 508 + * device type qualifier 1010b in the top 4 509 + * bits. 510 + */ 511 + if (firmware_info->v34.ras_rom_i2c_slave_addr) { 512 + if (i2c_address) 513 + *i2c_address = firmware_info->v34.ras_rom_i2c_slave_addr; 514 + return true; 515 + } 505 516 } 506 517 } 507 - 508 - if (*i2c_address != 0) 509 - return true; 510 518 511 519 return false; 512 520 }