Hi,
On 2024/2/5 16:17, Thomas Zimmermann wrote:
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.
return NULL is more easier and clear, it stand for "None" or "don't exist".
There is another reason that I want to tell you:
Some systems which don't have a good UEFI firmware support for uncommon GPUs.
the word "uncommon" means "not very popular GPU" or "extremely new GPU" or
"just refer to the GPUs that UEFI firmware don't know(recognize) about"
On such cases, there is no firmware framebuffer support. I means it is possible
that screen_info_resources() return -EINVAL because of not support yet. Then,
the screen_info_pci_dev(si) returns ERR_PTR(-EINVAL) and sysfb_pci_dev_is_enabled()
will take this error code as a pointer and de-reference it, cause the following
problem:
And even the x86-64 motherboard will not likely support all GPU(for example the one
with a old UEFI BIOS). And for an example, The intel Xe is the "extremely new GPU".
[ 5.031966] CPU 4 Unable to handle kernel paging request at virtual
address 000000000000081a, era == 900000000329b448, ra == 900000000329b440
[ 5.044587] Oops[#1]:
[ 5.046837] CPU: 4 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc1+ #7
[ 5.053062] Hardware name: Loongson
Loongson-3A6000-HV-7A2000-XA61200/Loongson-3A6000-HV-7A2000-XA61200,
BIOS Loongson-UDK2018-V4.0.05636-stable202311 12/
[ 5.066803] pc 900000000329b448 ra 900000000329b440 tp
90000001000d0000 sp 90000001000d3d40
[ 5.075100] a0 ffffffffffffffea a1 90000001000d3c38 a2
0000000000000003 a3 9000000003867ce8
[ 5.083398] a4 9000000003867ce0 a5 90000001000d3a80 a6
0000000000000001 a7 0000000000000001
[ 5.091695] t0 ac81f55e34713962 t1 ac81f55e34713962 t2
0000000000000000 t3 0000000000000001
[ 5.099992] t4 0000000000000004 t5 0000000000000000 t6
0000000000000030 t7 0000000000000000
[ 5.108290] t8 00000000000070b1 u0 0000000000000000 s9
0000000000000008 s0 9000000003d58b48
[ 5.116587] s1 9000000003c0b4a8 s2 9000000003787000 s3
9000000003778000 s4 90000000032c0578
[ 5.124884] s5 ffffffffffffffea s6 90000000032c0560 s7
90000000032df900 s8 ffffffffccccc000
[ 5.133182] ra: 900000000329b440 sysfb_init+0x80/0x1f0
[ 5.138545] ERA: 900000000329b448 sysfb_init+0x88/0x1f0
[ 5.143905] CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
[ 5.150048] PRMD: 00000004 (PPLV0 +PIE -PWE)
[ 5.154373] EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
[ 5.159131] ECFG: 00071c1c (LIE=2-4,10-12 VS=7)
[ 5.163717] ESTAT: 00010000 [PIL] (IS= ECode=1 EsubCode=0)
[ 5.169164] BADV: 000000000000081a
[ 5.172623] PRID: 0014d000 (Loongson-64bit, Loongson-3A6000-HV)
[ 5.178587] Modules linked in:
[ 5.181614] Process swapper/0 (pid: 1, threadinfo=(____ptrval____),
task=(____ptrval____))
[ 5.189827] Stack : 0000000000000000 0000000000000000
0000000000000000 0000000000000000
[ 5.197782] 0000000000000000 ac81f55e34713962
90000000032c0000 9000000003778000
[ 5.205736] 90000000032c0578 0000000000000000
900000000329b3c0 9000000003c54000
[ 5.213691] 90000001000d3db8 9000000002260154
0000000000000006 0000000000000000
[ 5.221645] 0000000000000000 0000000000000000
0000000000000000 0000000000000000
[ 5.229599] 0000000000000000 0000000000000000
0000000000000000 ac81f55e34713962
[ 5.237553] 90000000037468f8 90000000037468f8
90000000032c0578 90000000036a7658
[ 5.245508] 0000000000000006 9000000100041e00
0000000000000a55 9000000003260ff4
[ 5.251549] ata3: SATA link down (SStatus 0 SControl 300)
[ 5.253463] 0000000000000000 90000000032600b0
0000000000000000 0000000000000000
[ 5.266777] 0000000000000000 0000000000000000
0000000000000000 0000000000000000
[ 5.274731] ...
[ 5.277154] Call Trace:
[ 5.277155] [<900000000329b448>] sysfb_init+0x88/0x1f0
[ 5.284678] [<9000000002260154>] do_one_initcall+0x78/0x1cc
[ 5.290213] [<9000000003260ff4>] kernel_init_freeable+0x228/0x298
[ 5.296267] [<900000000324d104>] kernel_init+0x20/0x110
[ 5.301455] [<90000000022611e8>] ret_from_kernel_thread+0xc/0xa4
[ 5.307421]
[ 5.308892] Code: 561667fe 0015009c 40007080 <2408308c> 29403860
02c2e09b 0040818c 6400180c 1a007d45
[ 5.318579]
[ 5.320053] ---[ end trace 0000000000000000 ]---
[ 5.324640] Kernel panic - not syncing: Attempted to kill init!
exitcode=0x0000000b
[ 5.332247] Kernel relocated by 0x2040000
[ 5.336226] .text @ 0x9000000002240000
[ 5.340031] .data @ 0x90000000032f0000
[ 5.343835] .bss @ 0x9000000003c3f200
[ 5.347640] ---[ end Kernel panic - not syncing: Attempted to kill
init! exitcode=0x0000000b ]---
Best regards
Thomas
