Re: [PATCH 1/2] Driver for Freescale 8610 and 5121 DIU

From: Jiri Slaby
Date: Wed Mar 12 2008 - 18:20:42 EST


On 03/12/2008 10:43 PM, York Sun wrote:
+static int fsl_diu_open(struct fb_info *info, int user)
+{
+ struct mfb_info *mfbi = info->par;
+ int res = 0;
+
+ spin_lock(&diu_lock);
+ mfbi->count++;
+ if (mfbi->count == 1) {
+ DPRINTK("open plane index %d\n", mfbi->index);
+ fsl_diu_check_var(&info->var, info);
+ fsl_diu_set_par(info);

Please retest your code (at least) with sleep-inside spinlock debug option. If I see correctly you call GFP_KERNEL allocation somewhere deeper in this function, which might sleep.

+ res = fsl_diu_enable_panel(info);
+ if (res < 0)
+ mfbi->count--;
+ }
+
+ spin_unlock(&diu_lock);
+ return res;
+}

+static void __exit uninstall_fb(struct fb_info *info)
+{
+ struct mfb_info *mfbi = info->par;
+
+ DPRINTK("Entered: uninstall_fb\n");

You don't need this stuff everywhere (kprobes).

+ if (!mfbi->registered)
+ return;
+
+ unregister_framebuffer(info);
+ unmap_video_memory(info);
+ if (&info->cmap)
+ fb_dealloc_cmap(&info->cmap);
+
+ mfbi->registered = 0;
+}
[...]
+static int fsl_diu_probe(struct platform_device *pdev)
+{
+ struct mfb_info *mfbi;
+ unsigned long dummy_ad_addr;
+ int ret, i, error = 0;
+
+ DPRINTK("Entered: fsl_diu_probe\n");
+
+ /* Area descriptor memory pool aligns to 64-bit boundary */
+ allocate_buf(&pool.ad, sizeof(struct diu_ad) * FSL_AOI_NUM, 8);
+
+ /* Get memory for Gamma Table - 32-byte aligned memory */
+ allocate_buf(&pool.gamma, 768, 32);
+
+ /* For performance, cursor bitmap buffer aligns to 32-byte boundary */
+ allocate_buf(&pool.cursor, MAX_CURS * MAX_CURS * 2, 32);
+
+ i = sizeof(fsl_diu_info) / sizeof(struct fb_info);
+ dummy_ad = (struct diu_ad *)((u32)pool.ad.vaddr + pool.ad.offset) + i;
+ dummy_ad->paddr = pool.ad.paddr + i * sizeof(struct diu_ad);
+ dummy_aoi_virt = fsl_diu_alloc(64, &dummy_ad_addr);
+ dummy_ad->addr = cpu_to_le32(dummy_ad_addr);
+ dummy_ad->pix_fmt = 0x88882317;
+ dummy_ad->src_size_g_alpha = cpu_to_le32((4 << 12) | 4);
+ dummy_ad->aoi_size = cpu_to_le32((4 << 16) | 2);
+ dummy_ad->offset_xyi = 0;
+ dummy_ad->offset_xyd = 0;
+ dummy_ad->next_ad = 0;
+ memset(dummy_aoi_virt, 0x00, 64);
+ write_reg(&(dr.diu_reg->desc[0]), dummy_ad->paddr);
+ write_reg(&(dr.diu_reg->desc[1]), dummy_ad->paddr);
+ write_reg(&(dr.diu_reg->desc[2]), dummy_ad->paddr);
+
+ for (i = 0; i < sizeof(fsl_diu_info) / sizeof(struct fb_info); i++) {
+ fsl_diu_info[i].fix.smem_start = 0;
+ mfbi = fsl_diu_info[i].par;
+ mfbi->ad = (struct diu_ad *)((u32)pool.ad.vaddr
+ + pool.ad.offset) + i;
+ mfbi->ad->paddr = pool.ad.paddr + i * sizeof(struct diu_ad);
+ ret = install_fb(&fsl_diu_info[i], pdev);
+ if (ret) {
+ printk(KERN_ERR "Failed to register framebuffer %d\n",
+ i);
+ return ret;

some kind of free here

+ }
+ }
[...]
+int __init fsl_diu_init(void)
+{
+ struct device_node *np;
+ struct resource r;
+ int error;
+ DPRINTK("Entered: fsl_diu_init\n");
+ np = of_find_compatible_node(NULL, NULL, "fsl-diu");
+ if (!np) {
+ printk(KERN_ERR "Err: can't find device node 'fsl-diu'\n");
+ return -ENODEV;
+ }
+
+ of_address_to_resource(np, 0, &r);
+ of_node_put(np);
+
+ DPRINTK("%s, r.start: 0x%08x\n", __func__, r.start);
+
+ dr.diu_reg = ioremap(r.start, sizeof(struct diu));

The arch never fails with remapping?

+
+ write_reg(&(dr.diu_reg->diu_mode), 0); /* disable DIU anyway*/
+
+ spin_lock_init(&dr.reg_lock);
+
+
+ /*
+ * For kernel boot options (in 'video=xxxfb:<options>' format)
+ */
+#ifndef MODULE
+ {
+ char *option;
+
+ if (fb_get_options("fslfb", &option))
+ return -ENODEV;
+ fsl_diu_setup(option);
+ }
+#endif
+ error = platform_driver_register(&fsl_diu_driver);
+
+ if (!error) {
+ error = platform_device_register(&fsl_diu_device);
+ if (error) {
+ printk(KERN_ERR "Err: "
+ "can't register FB device driver!\n");
+ platform_driver_unregister(&fsl_diu_driver);

iounmap

+ }
+ printk(KERN_INFO "FSL_DIU_FB: registed FB device driver!\n");
+ }

else iounmap

+
+ return error;
+}
[...]
diff --git a/drivers/video/fsl-diu-fb.h b/drivers/video/fsl-diu-fb.h
new file mode 100644
index 0000000..294d0bf
--- /dev/null
+++ b/drivers/video/fsl-diu-fb.h
@@ -0,0 +1,388 @@
[...]
+#ifndef __FSL_DIU_FB_H__
+#define __FSL_DIU_FB_H__
+
+/* FIXME: This should be changed to dev_dbg this will be done as soon as
+ * we can obtain dev through the dts setup

then use at least pr_debug() here:

+ */
+#ifdef DEBUG
+#define DPRINTK(fmt, args...) printk(KERN_DEBUG "%s: " fmt, __func__, ## args)
+#else
+#define DPRINTK(fmt, args...)
+#endif

regards,
--
Jiri Slaby
Faculty of Informatics, Masaryk University
Suse Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/