Am 11.05.22 um 13:24 schrieb Javier Martinez Canillas:
These can be used by subsystems to unregister a platform device registered by sysfb and also to disable future platform device registration in sysfb. Suggested-by: Daniel Vetter <daniel.vetter@xxxxxxxx> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> --- (no changes since v4) Changes in v4: - Make sysfb_disable() to also attempt to unregister a device. Changes in v2: - Add kernel-doc comments and include in other_interfaces.rst (Daniel Vetter). .../driver-api/firmware/other_interfaces.rst | 6 ++ drivers/firmware/sysfb.c | 87 +++++++++++++++++-- include/linux/sysfb.h | 19 ++++ 3 files changed, 106 insertions(+), 6 deletions(-) diff --git a/Documentation/driver-api/firmware/other_interfaces.rst b/Documentation/driver-api/firmware/other_interfaces.rst index b81794e0cfbb..06ac89adaafb 100644 --- a/Documentation/driver-api/firmware/other_interfaces.rst +++ b/Documentation/driver-api/firmware/other_interfaces.rst @@ -13,6 +13,12 @@ EDD Interfaces .. kernel-doc:: drivers/firmware/edd.c :internal:+Generic System Framebuffers Interface+------------------------------------- + +.. kernel-doc:: drivers/firmware/sysfb.c + :export: + Intel Stratix10 SoC Service Layer --------------------------------- Some features of the Intel Stratix10 SoC require a level of privilege diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c index b032f40a92de..6768968949e6 100644 --- a/drivers/firmware/sysfb.c +++ b/drivers/firmware/sysfb.c @@ -34,21 +34,92 @@ #include <linux/screen_info.h> #include <linux/sysfb.h>+static struct platform_device *pd;+static DEFINE_MUTEX(disable_lock); +static bool disabled; + +static bool sysfb_unregister(void) +{ + if (IS_ERR_OR_NULL(pd)) + return false; + + platform_device_unregister(pd); + pd = NULL; + + return true; +} + +/** + * sysfb_disable() - disable the Generic System Framebuffers support + * + * This disables the registration of system framebuffer devices that match the + * generic drivers that make use of the system framebuffer set up by firmware. + * + * It also unregisters a device if this was already registered by sysfb_init().
Why? I still cannot wrap my mind around, why we need to store *pd at all and use sysfb_try_unregister() for unregistering.
+ * + * Context: The function can sleep. A @disable_lock mutex is acquired to serialize + * against sysfb_init(), that registers a system framebuffer device and + * sysfb_try_unregister(), that tries to unregister a framebuffer device. + */ +void sysfb_disable(void) +{ + mutex_lock(&disable_lock); + sysfb_unregister(); + disabled = true; + mutex_unlock(&disable_lock); +} +EXPORT_SYMBOL_GPL(sysfb_disable); + +/** + * sysfb_try_unregister() - attempt to unregister a system framebuffer device + * @dev: device to unregister + * + * This tries to unregister a system framebuffer device if this was registered + * by the Generic System Framebuffers. The device will only be unregistered if + * it was registered by sysfb_init(), otherwise it will not be unregistered. + * + * Context: The function can sleep. a @load_lock mutex is acquired to serialize + * against sysfb_init(), that registers a simple framebuffer device and + * sysfb_disable(), that disables the Generic System Framebuffers support. + * + * Return: + * * true - the device was unregistered successfully + * * false - the device was not unregistered + */ +bool sysfb_try_unregister(struct device *dev)
As it stands, I strongly object the use of this function as still don't really get the purpose. It looks like a glorified wrapper around platform_device_unregister(). Do we need disable_lock to serialize with something else?
Best regards Thomas
+{ + bool ret = false; + + mutex_lock(&disable_lock); + if (IS_ERR_OR_NULL(pd) || pd != to_platform_device(dev)) + goto unlock_mutex; + + ret = sysfb_unregister(); + +unlock_mutex: + mutex_unlock(&disable_lock); + return ret; +} +EXPORT_SYMBOL_GPL(sysfb_try_unregister); + static __init int sysfb_init(void) { struct screen_info *si = &screen_info; struct simplefb_platform_data mode; - struct platform_device *pd; const char *name; bool compatible; - int ret; + int ret = 0; + + mutex_lock(&disable_lock); + if (disabled) + goto unlock_mutex;/* try to create a simple-framebuffer device */compatible = sysfb_parse_mode(si, &mode); if (compatible) { pd = sysfb_create_simplefb(si, &mode); if (!IS_ERR(pd)) - return 0; + goto unlock_mutex; }/* if the FB is incompatible, create a legacy framebuffer device */@@ -60,8 +131,10 @@ static __init int sysfb_init(void) name = "platform-framebuffer";pd = platform_device_alloc(name, 0);- if (!pd) - return -ENOMEM; + if (!pd) { + ret = -ENOMEM; + goto unlock_mutex; + }sysfb_apply_efi_quirks(pd); @@ -73,9 +146,11 @@ static __init int sysfb_init(void)if (ret) goto err;- return 0;+ goto unlock_mutex; err: platform_device_put(pd); +unlock_mutex: + mutex_unlock(&disable_lock); return ret; }diff --git a/include/linux/sysfb.h b/include/linux/sysfb.hindex 708152e9037b..e8c0313fac8f 100644 --- a/include/linux/sysfb.h +++ b/include/linux/sysfb.h @@ -55,6 +55,25 @@ struct efifb_dmi_info { int flags; };+#ifdef CONFIG_SYSFB+ +void sysfb_disable(void); +bool sysfb_try_unregister(struct device *dev); + +#else /* CONFIG_SYSFB */ + +static inline void sysfb_disable(void) +{ + +} + +static inline bool sysfb_try_unregister(struct device *dev) +{ + return false; +} + +#endif /* CONFIG_SYSFB */ + #ifdef CONFIG_EFIextern struct efifb_dmi_info efifb_dmi_list[];
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature