Re: [v2,2/8] video: Provide screen_info_get_pci_dev() to find screen_info's PCI device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 02.02.24 um 18:03 schrieb Sui Jingfeng:
Hi,


On 2024/2/2 19:58, Thomas Zimmermann wrote:
+
+/**
+ * screen_info_pci_dev() - Return PCI parent device that contains screen_info's framebuffer
+ * @si: the screen_info
+ *
+ * Returns:
+ * The screen_info's parent device on success, or NULL otherwise.
+ */
+struct pci_dev *screen_info_pci_dev(const struct screen_info *si)
+{
+    struct resource res[SCREEN_INFO_MAX_RESOURCES];
+    ssize_t i, numres;
+
+    numres = screen_info_resources(si, res, ARRAY_SIZE(res));
+    if (numres < 0)
+        return ERR_PTR(numres);


Please return NULL at here, otherwise we have to use the IS_ERR or IS_ERR_OR_NULL() in the caller function to check the returned value. Meanwhile, I noticed that you didn't actually call IS_ERR() in the sysfb_parent_dev() function (introduced by the
3/8 patch), so I think we probably should return NULL at here.

Please also consider that the comments of this function says that it return NULL on
the otherwise cases.

Right. The idea is to return NULL is there is no parent device. The errno pointer is for actual runtime problems. The doc comment is still incorrect and I think I need to review the callers of this function. Thanks for pointing this out.

Best regards
Thomas



+    for (i = 0; i < numres; ++i) {
+        struct pci_dev *pdev = __screen_info_pci_dev(&res[i]);
+
+        if (pdev)
+            return pdev;
+    }
+
+    return NULL;
+}
+EXPORT_SYMBOL(screen_info_pci_dev);

--
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: OpenPGP_signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux