Hi,

On Wednesday 10 November 2010, Jimmy Rubin wrote:

quoted

+
+static struct platform_device mcde_fb_device = {
+	.name = "mcde_fb",
+	.id = -1,
+};

Do not introduce new static devices. We are trying to remove them and
they will stop working. Why do you even need a device here if there is
only one of them?

quoted

+struct fb_info *mcde_fb_create(struct mcde_display_device *ddev,
+	u16 w, u16 h, u16 vw, u16 vh, enum mcde_ovly_pix_fmt

pix_fmt,

quoted

+	u32 rotate)
+{

Here you have another device, which you could just use!

I will do that.

quoted

+/* Overlay fbs' platform device */
+static int mcde_fb_probe(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static int mcde_fb_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct platform_driver mcde_fb_driver = {
+	.probe  = mcde_fb_probe,
+	.remove = mcde_fb_remove,
+	.driver = {
+		.name  = "mcde_fb",
+		.owner = THIS_MODULE,
+	},
+};
+
+/* MCDE fb init */
+
+int __init mcde_fb_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&mcde_fb_driver);
+	if (ret)
+		goto fb_driver_failed;
+	ret = platform_device_register(&mcde_fb_device);
+	if (ret)
+		goto fb_device_failed;
+
+	goto out;
+fb_device_failed:
+	platform_driver_unregister(&mcde_fb_driver);
+fb_driver_failed:
+out:
+	return ret;
+}
+
+void mcde_fb_exit(void)
+{
+	platform_device_unregister(&mcde_fb_device);
+	platform_driver_unregister(&mcde_fb_driver);
+}

This appears to be an entirely useless registration for something that
does not exist and that you are not using anywhere ...

Will look into this and remove it.

quoted

+
+#include <linux/fb.h>
+#include <linux/ioctl.h>
+#if !defined(__KERNEL__) && !defined(_KERNEL)
+#include <stdint.h>
+#else
+#include <linux/types.h>
+#endif
+
+#ifdef __KERNEL__
+#include "mcde_dss.h"
+#endif
+
+#ifdef __KERNEL__
+#define to_mcde_fb(x) ((struct mcde_fb *)(x)->par)

Everything in this file is enclosed in #ifdef __KERNEL__, and the file
is not even exported. You can remove the #ifdef and the #else path
everywhere AFAICT.

Agree, no IOCTLs there for the moment so only kernel include. 

/Jimmy