Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] The driver unconditionally rejects perfectly working cards if the 
video BIOS ROM is inaccessible or in an unsupported format.
--

--- Patch [2]: [PATCH 2/5] fbdev: tdfxfb: Attempt to detect if the card wasn't 
booted ---
commit e450df3d917986ae9c59e1033b55fdddafa9cf19
Author: Daniel Palmer <[email protected]>
Subject: fbdev: tdfxfb: Attempt to detect if the card wasn't booted

This commit adds hardware initialization checks to tdfxfb_probe() by
attempting to read the configuration from the video BIOS. If the register
values do not match the BIOS config, it rejects the card to avoid providing
a blank display.

> diff --git a/drivers/video/fbdev/tdfxfb.c b/drivers/video/fbdev/tdfxfb.c
> --- a/drivers/video/fbdev/tdfxfb.c
> +++ b/drivers/video/fbdev/tdfxfb.c
[ ... ]
> +/*
> + * Try to work out if the card was booted or not, just checks
> + * if the register reported memory amount matches what the BIOS
> + * reports for now.
> + */
> +static int tdfxfb_hw_init(struct fb_info *info, struct pci_dev *pdev)
> +{
> +     struct tdfx_par *par = info->par;
> +     struct tdfx_bios_cfg cfg;
> +
> +     if (tdfxfb_get_bios_cfg(pdev, &cfg) &&
> +         tdfx_inl(par, DRAMINIT0) == le32_to_cpu(cfg.draminit0))
> +             return 0;
> +

[Severity: High]
Will this unconditionally reject working cards if the video BIOS ROM is
inaccessible or in an unsupported format?

Looking at tdfxfb_get_bios_cfg(), it can naturally return false if
pci_map_rom() fails. This is common on architectures without PCI ROM
support, or if the ROM BAR is disabled. It will also return false if the
BIOS ROM does not contain the specific 3dfx OEM configuration table.

Because of the logical && short-circuit, if tdfxfb_get_bios_cfg() fails to
read the config, the tdfx_inl() check is skipped and the function falls
through to return -ENODEV.

Could this cause a regression for hardware that was previously supported but
lacks a readable BIOS ROM?

> +     dev_err(&pdev->dev,
> +             "Card hasn't booted and is unusable\n");
> +     return -ENODEV;
> +}

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to