| Message ID | 20240202120140.3517-6-tzimmermann@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | firmware/sysfb: Track parent device for screen_info | expand |
Am 02.02.24 um 18:50 schrieb Sui Jingfeng: > HI, > > > On 2024/2/2 19:58, Thomas Zimmermann wrote: >> 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. > > > For *all* ? > > I think the functionality this patch implemented is only target for the > PCIe device firmware framebuffers, the framebuffer consumed by the simplefb > driver (fbdev/simplefb.c) is also a kind of firmware framebuffer, but it is > target for the platform device only. > > So, the correct description would be: "this patch implements the > functionality > for the PCIe firmware framebuffers". Fair enough. > >> v2: >> * rework sysfb_pci_dev_is_enabled() (Javier) >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> >> --- >> drivers/firmware/sysfb.c | 30 +++++++++++++++++++++++++++++- >> 1 file changed, 29 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c >> index d02945b0d8ea1..ab5cbc0326f6d 100644 >> --- a/drivers/firmware/sysfb.c >> +++ b/drivers/firmware/sysfb.c >> @@ -70,13 +70,39 @@ void sysfb_disable(void) >> } >> EXPORT_SYMBOL_GPL(sysfb_disable); >> +#if defined(CONFIG_PCI) >> +static __init bool sysfb_pci_dev_is_enabled(struct pci_dev *pdev) >> +{ >> + /* >> + * 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 >> +static __init bool sysfb_pci_dev_is_enabled(struct pci_dev *pdev) >> +{ >> + return false; >> +} >> +#endif >> + >> static __init struct device *sysfb_parent_dev(const struct >> screen_info *si) >> { >> struct pci_dev *pdev; >> pdev = screen_info_pci_dev(si); >> - if (pdev) >> + if (pdev) { >> + if (!sysfb_pci_dev_is_enabled(pdev)) >> + return ERR_PTR(-ENODEV); > > > Is it better to move the call of sysfb_pci_dev_is_enabled() out of > sysfb_parent_dev() ? > Because then we don't need check the returned value by calling the > IS_ERR() inthe sysfb_init() function. > > >> return &pdev->dev; >> + } >> return NULL; >> } >> @@ -97,6 +123,8 @@ static __init int sysfb_init(void) >> sysfb_apply_efi_quirks(); >> parent = sysfb_parent_dev(si); >> + if (IS_ERR(parent)) >> + goto unlock_mutex; > > if (!sysfb_pci_dev_is_enabled(parent)) > goto unlock_mutex; > >> /* try to create a simple-framebuffer device */ >> compatible = sysfb_parse_mode(si, &mode);
diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c index d02945b0d8ea1..ab5cbc0326f6d 100644 --- a/drivers/firmware/sysfb.c +++ b/drivers/firmware/sysfb.c @@ -70,13 +70,39 @@ void sysfb_disable(void) } EXPORT_SYMBOL_GPL(sysfb_disable); +#if defined(CONFIG_PCI) +static __init bool sysfb_pci_dev_is_enabled(struct pci_dev *pdev) +{ + /* + * 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 +static __init bool sysfb_pci_dev_is_enabled(struct pci_dev *pdev) +{ + return false; +} +#endif + static __init struct device *sysfb_parent_dev(const struct screen_info *si) { struct pci_dev *pdev; pdev = screen_info_pci_dev(si); - if (pdev) + if (pdev) { + if (!sysfb_pci_dev_is_enabled(pdev)) + return ERR_PTR(-ENODEV); return &pdev->dev; + } return NULL; } @@ -97,6 +123,8 @@ static __init int sysfb_init(void) sysfb_apply_efi_quirks(); parent = sysfb_parent_dev(si); + if (IS_ERR(parent)) + goto unlock_mutex; /* try to create a simple-framebuffer device */ compatible = sysfb_parse_mode(si, &mode);