Hi Christoph,
Thanks for your comments. I'm also replying here to your comments on the
previous series.
On Tuesday, March 17, 2020 10:28:35 AM EDT Christoph Hellwig wrote:
> Any reason drivers can't just use pci_map_rom insteadἅ which already
> does the right thing?
Some machines don't expose the video BIOS in the PCI BAR and instead only make
it available via EFI boot services calls. So drivers need to be able to use
the ROM provided by EFI calls, but only if they can't find a valid one anywhere
else.
At one point, the EFI provided ROM in pdev->rom *was* exposed via
pci_map_rom(). However it had to be split out into a separate function so that
drivers could have more control over which sources were preferred.
On Tuesday, March 17, 2020 10:29:13 AM EDT Christoph Hellwig wrote:
> This and the next patch really need to be folded into the previous
> one to avoid regressions (assuming my other suggestion doesn't work
> for some reason).
Addressed in v2
On Tuesday, March 17, 2020 10:47:31 AM EDT Christoph Hellwig wrote:
> What is the value of this helper over just open coding an ioremap
> of pdev->rom in the callers?
I think the direct access to pdev->rom you suggest makes sense, especially
because users of the pci_platform_rom() are exposed to the fact that it just
calls ioremap().
I decided against wrapping the iounmap() with a pci_unmap_platform_rom(), but
I didn't apply the same consideration to the existing function.
How about something like this (with pci_platform_rom() removed)?
static bool radeon_read_platform_bios(struct radeon_device *rdev)
{
phys_addr_t rom = rdev->pdev->rom;
size_t romlen = rdev->pdev->romlen;
void __iomem *bios;
rdev->bios = NULL;
if (!rom || romlen == 0)
return false;
rdev->bios = kzalloc(romlen, GFP_KERNEL);
if (!rdev->bios)
return false;
bios = ioremap(rom, romlen);
if (!bios)
goto free_bios;
memcpy_fromio(rdev->bios, bios, romlen);
iounmap(bios);
if (rdev->bios[0] != 0x55 || rdev->bios[1] != 0xaa)
goto free_bios;
return true;
free_bios:
kfree(rdev->bios);
return false;
}
If this is preferred, I'll send an updated series. I'm assuming that the
removal of pci_platform_rom() and updating of all the callers should be
combined into this patch.
Thanks,
Mikel
