Re: [PATCH 02/11] x86: sysfb: remove sysfb when probing real hw
From: Ingo Molnar <mingo@kernel.org>
Date: 2014-01-23 16:51:24
Also in:
dri-devel, lkml
Just a couple of small nits: * David Herrmann [off-list ref] wrote:
quoted hunk ↗ jump to hunk
--- a/arch/x86/kernel/sysfb.c +++ b/arch/x86/kernel/sysfb.c@@ -33,11 +33,76 @@ #include <linux/init.h> #include <linux/kernel.h> #include <linux/mm.h> +#include <linux/mutex.h> #include <linux/platform_data/simplefb.h> #include <linux/platform_device.h> #include <linux/screen_info.h> #include <asm/sysfb.h> +static DEFINE_MUTEX(sysfb_lock); +static struct platform_device *sysfb_dev; + +int __init sysfb_register(const char *name, int id, + const struct resource *res, unsigned int res_num, + const void *data, size_t data_size) +{ + struct platform_device *pd; + int ret = 0; + + mutex_lock(&sysfb_lock); + if (!sysfb_dev) { + pd = platform_device_register_resndata(NULL, name, id, + res, res_num, + data, data_size); + if (IS_ERR(pd)) + ret = PTR_ERR(pd); + else + sysfb_dev = pd; + } + mutex_unlock(&sysfb_lock); + + return ret; +} + +static bool sysfb_match(const struct apertures_struct *apert) +{ + struct screen_info *si = &screen_info; + unsigned int i; + const struct aperture *a; + + for (i = 0; i < apert->count; ++i) { + a = &apert->ranges[i]; + if (a->base >= si->lfb_base && + a->base < si->lfb_base + ((u64)si->lfb_size << 16)) + return true; + if (si->lfb_base >= a->base && + si->lfb_base < a->base + a->size) + return true; + } + + return false; +} + +/* Remove sysfb and disallow new sysfbs from now on. Can be called from any + * context except recursively (see also remove_conflicting_framebuffers()). */ +void sysfb_unregister(const struct apertures_struct *apert, bool primary)
Please use the customary (multi-line) comment style: /* * Comment ..... * ...... goes here. */ specified in Documentation/CodingStyle.
+#ifdef CONFIG_X86_SYSFB +# include <asm/sysfb.h> +#endif
I guess a single space is sufficient? Better yet, I'd include sysfb.h unconditionally:
quoted hunk ↗ jump to hunk
@@ -1773,6 +1780,10 @@ register_framebuffer(struct fb_info *fb_info) { int ret; +#ifdef CONFIG_X86_SYSFB + sysfb_unregister(fb_info->apertures, fb_is_primary_device(fb_info)); +#endif
So, if a dummy sysfb_unregister() inline was defined in the !CONFIG_X86_SYSFB case then this ugly #ifdef could possibly be removed? Especially as it's used twice. Thanks, Ingo