Hi

Am 02.02.24 um 18:03 schrieb Sui Jingfeng:

Hi,


On 2024/2/2 19:58, Thomas Zimmermann wrote:

quoted

+
+/**
+ * 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

quoted

+    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)