Re: [PATCH 2/4] fbdev/efifb: Use screen_info pointer from device

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

 



Hi Javier

Am 01.12.23 um 09:54 schrieb Javier Martinez Canillas:
Thomas Zimmermann <tzimmermann@xxxxxxx> writes:

Use the screen_info instance from the device instead of dereferencing
the global screen_info state. Decouples the driver from per-architecture
code. Duplicated the screen_info data, so that efifb can modify it at
will.

Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
---

[...]

+	si = dev_get_platdata(&dev->dev);
+	if (!si)

I would add a comment that this platform data is set when the device is
registered by sysfb.

+		return -ENODEV;
+	si = devm_kmemdup(&dev->dev, si, sizeof(*si), GFP_KERNEL);
+	if (!si)
+		return -ENOMEM;
+

Why a copy? In any case maybe the global screen_info should be duplicated
when is set as the device platform data in sysfb_init() ?

We get our own copy of the global screen_info as platform-device data. Efifb modifies some of the values in our copy in efifb_setup(). If probing afterwards fails, the kernel might try a different driver, which would then operate on the values modified by efifb. Hence, there's this internal copy. The situation with vesafb is similar.

Best regards
Thomas


I agree with the direction of the patch though, so whatever you decide:

Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>


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