Hi Javier

Am 01.12.23 um 09:54 schrieb Javier Martinez Canillas:
Thomas Zimmermann <tzimmerm...@suse.de> 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 <tzimmerm...@suse.de>
---

[...]

+       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 <javi...@redhat.com>


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

Reply via email to