Re: [PATCH 5/8] firmware/sysfb: Create firmware device only for enabled PCI devices

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 1683 bytes --]

Hi

Am 29.01.24 um 12:36 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
> 
>> Test if the firmware framebuffer's parent PCI device, if any, has
>> been enabled. If not, the firmware framebuffer is most likely not
>> working. Hence, do not create a device for the firmware framebuffer
>> on disabled PCI devices.
>>
>> So far, efifb tracked the status of the PCI parent device internally
>> and did not bind if it was disabled. This patch implements the
>> functionality for all firmware framebuffers.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
> 
> [...]
> 
>>   
>> +static __init bool sysfb_pci_dev_is_enabled(struct pci_dev *pdev)
>> +{
>> +#if defined(CONFIG_PCI)
>> +	/*
>> +	 * TODO: Try to integrate this code into the PCI subsystem
>> +	 */
>> +	int ret;
>> +	u16 command;
>> +
>> +	ret = pci_read_config_word(pdev, PCI_COMMAND, &command);
>> +	if (ret != PCIBIOS_SUCCESSFUL)
>> +		return false;
>> +	if (!(command & PCI_COMMAND_MEMORY))
>> +		return false;
>> +	return true;
>> +#else
>> +	// Getting here without PCI support is probably a bug.
>> +	return false;
> 
> Should we warn before return in this case ?

I would not do so as the bug is not here, but in screen_info_pci_dev(). 
I'm going to update this chunk such that the non-PCI case is a separate 
function.

> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]