Thread (29 messages) 29 messages, 4 authors, 2014-03-07

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help