On 11/02/2021 16:07, Segher Boessenkool wrote:
But that loses information, there is no hint *why* the device is ignored anymore. Maybe you can do a better name (one that actually means the same as blacklisted), or add a comment somewhere?
Any suggestion for better names?
ignore_broken_device perhaps... Something like that. Someone who knows exactly why the device is blacklisted can tell you.
FWIW the comments in the pci_xbox_blacklisted() already give the reasoning:
/* * The Xbox MCPX chipset is a derivative of the nForce 1 * chipset. It almost has the same bus layout; some devices * cannot be used, because they have been removed. */
...
/* * Devices 00:00.1 and 00:00.2 used to be memory controllers on * the nForce chipset, but on the Xbox, using them will lockup * the chipset. */
...
/* * Bus 1 only contains a VGA controller at 01:00.0. When you try * to probe beyond that device, you only get garbage, which * could cause lockups. */
...
/* * Bus 2 used to contain the AGP controller, but the Xbox MCPX * doesn't have one. Probing it can cause lockups. */
Just "ignore_device" would fit the code perfectly well here, but then you need to add a comment (where it is used) saying why that is done. The existing name suggests it is because the device is broken or similar; that it cannot be handled like other devices anyway.
I prefer to keep the pci_xbox prefix as per Philippe's v3 patch since it makes it clear that this only relevant to XBox (and indeed, the #ifdef enforces it).
How about a single line comment above the call to pci_xbox_ignore_device() that says something like this?
/* Some in-built XBox PCI devices are unsupported/unavailable */
You cannot just replace terminology with other words that do not have the same connotations without losing important information, so any such replacement should be done by people who understand the code well :-(
Indeed, it is important to understand the context of the original code when coming up with a replacement.
ATB,
Mark.