drm: omapdrm: dss: Support passing private data to debugfs show handlers
authorLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Tue, 13 Feb 2018 12:00:29 +0000 (14:00 +0200)
committerTomi Valkeinen <tomi.valkeinen@ti.com>
Thu, 1 Mar 2018 07:18:18 +0000 (09:18 +0200)
To simplify implementation of debugfs seq_file show handlers, the driver
passes the pointer to the show function through the debugfs_create_file
data pointer. This prevents using the pointer to pass driver private
data to the show handler, and requires all handlers to use global
variables to access private data.

To prepare for the removal of global private data in the driver, rework
the debugfs infrastructure to allow passing a private data pointer to
show handlers.

The price to pay is explicit removal of debugfs files to free the
internally allocated memory. This is desirable anyway as debugfs entries
should be removed when a component driver is unbound, otherwise crashes
will occur due to access to freed memory when the components will be
dynamically allocated instead of stored in global variables.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
drivers/gpu/drm/omapdrm/dss/dispc.c
drivers/gpu/drm/omapdrm/dss/dsi.c
drivers/gpu/drm/omapdrm/dss/dss.c
drivers/gpu/drm/omapdrm/dss/dss.h
drivers/gpu/drm/omapdrm/dss/hdmi.h
drivers/gpu/drm/omapdrm/dss/hdmi4.c
drivers/gpu/drm/omapdrm/dss/hdmi5.c
drivers/gpu/drm/omapdrm/dss/venc.c

index 867887151565d75a72e04332a6a5afb21c564198..3428ffea70ee06cf20a94813ee9a8bd308708834 100644 (file)
@@ -167,6 +167,8 @@ static struct {
        void __iomem    *base;
        struct dss_device *dss;
 
+       struct dss_debugfs_entry *debugfs;
+
        int irq;
        irq_handler_t user_handler;
        void *user_data;
@@ -3268,7 +3270,7 @@ void dispc_dump_clocks(struct seq_file *s)
        dispc_runtime_put();
 }
 
-static void dispc_dump_regs(struct seq_file *s)
+static int dispc_dump_regs(struct seq_file *s, void *p)
 {
        int i, j;
        const char *mgr_names[] = {
@@ -3289,7 +3291,7 @@ static void dispc_dump_regs(struct seq_file *s)
 #define DUMPREG(r) seq_printf(s, "%-50s %08x\n", #r, dispc_read_reg(r))
 
        if (dispc_runtime_get())
-               return;
+               return 0;
 
        /* DISPC common registers */
        DUMPREG(DISPC_REVISION);
@@ -3461,6 +3463,8 @@ static void dispc_dump_regs(struct seq_file *s)
 
 #undef DISPC_REG
 #undef DUMPREG
+
+       return 0;
 }
 
 /* calculate clock rates using dividers in cinfo */
@@ -4620,7 +4624,8 @@ static int dispc_bind(struct device *dev, struct device *master, void *data)
 
        dispc_set_ops(&dispc_ops);
 
-       dss_debugfs_create_file("dispc", dispc_dump_regs);
+       dispc.debugfs = dss_debugfs_create_file("dispc", dispc_dump_regs,
+                                               &dispc);
 
        return 0;
 
@@ -4632,6 +4637,8 @@ err_runtime_get:
 static void dispc_unbind(struct device *dev, struct device *master,
                               void *data)
 {
+       dss_debugfs_remove_file(dispc.debugfs);
+
        dispc_set_ops(NULL);
 
        pm_runtime_disable(dev);
index 26f4122f6784713d034f35e42ee36ba4c42dd1e9..a676d27dd47930669e21c85e81fc2820ad0730ad 100644 (file)
@@ -401,6 +401,10 @@ struct dsi_data {
 #endif
        int debug_read;
        int debug_write;
+       struct {
+               struct dss_debugfs_entry *irqs;
+               struct dss_debugfs_entry *regs;
+       } debugfs;
 
 #ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS
        spinlock_t irq_stats_lock;
@@ -1660,18 +1664,20 @@ static void dsi_dump_dsidev_irqs(struct platform_device *dsidev,
 #undef PIS
 }
 
-static void dsi1_dump_irqs(struct seq_file *s)
+static int dsi1_dump_irqs(struct seq_file *s, void *p)
 {
        struct platform_device *dsidev = dsi_get_dsidev_from_id(0);
 
        dsi_dump_dsidev_irqs(dsidev, s);
+       return 0;
 }
 
-static void dsi2_dump_irqs(struct seq_file *s)
+static int dsi2_dump_irqs(struct seq_file *s, void *p)
 {
        struct platform_device *dsidev = dsi_get_dsidev_from_id(1);
 
        dsi_dump_dsidev_irqs(dsidev, s);
+       return 0;
 }
 #endif
 
@@ -1759,18 +1765,20 @@ static void dsi_dump_dsidev_regs(struct platform_device *dsidev,
 #undef DUMPREG
 }
 
-static void dsi1_dump_regs(struct seq_file *s)
+static int dsi1_dump_regs(struct seq_file *s, void *p)
 {
        struct platform_device *dsidev = dsi_get_dsidev_from_id(0);
 
        dsi_dump_dsidev_regs(dsidev, s);
+       return 0;
 }
 
-static void dsi2_dump_regs(struct seq_file *s)
+static int dsi2_dump_regs(struct seq_file *s, void *p)
 {
        struct platform_device *dsidev = dsi_get_dsidev_from_id(1);
 
        dsi_dump_dsidev_regs(dsidev, s);
+       return 0;
 }
 
 enum dsi_cio_power_state {
@@ -5569,15 +5577,22 @@ static int dsi_bind(struct device *dev, struct device *master, void *data)
        dsi_runtime_put(dsidev);
 
        if (dsi->module_id == 0)
-               dss_debugfs_create_file("dsi1_regs", dsi1_dump_regs);
-       else if (dsi->module_id == 1)
-               dss_debugfs_create_file("dsi2_regs", dsi2_dump_regs);
-
+               dsi->debugfs.regs = dss_debugfs_create_file("dsi1_regs",
+                                                           dsi1_dump_regs,
+                                                           &dsi);
+       else
+               dsi->debugfs.regs = dss_debugfs_create_file("dsi2_regs",
+                                                           dsi2_dump_regs,
+                                                           &dsi);
 #ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS
        if (dsi->module_id == 0)
-               dss_debugfs_create_file("dsi1_irqs", dsi1_dump_irqs);
-       else if (dsi->module_id == 1)
-               dss_debugfs_create_file("dsi2_irqs", dsi2_dump_irqs);
+               dsi->debugfs.irqs = dss_debugfs_create_file("dsi1_irqs",
+                                                           dsi1_dump_irqs,
+                                                           &dsi);
+       else
+               dsi->debugfs.irqs = dss_debugfs_create_file("dsi2_irqs",
+                                                           dsi2_dump_irqs,
+                                                           &dsi);
 #endif
 
        return 0;
@@ -5596,6 +5611,9 @@ static void dsi_unbind(struct device *dev, struct device *master, void *data)
        struct platform_device *dsidev = to_platform_device(dev);
        struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 
+       dss_debugfs_remove_file(dsi->debugfs.irqs);
+       dss_debugfs_remove_file(dsi->debugfs.regs);
+
        of_platform_depopulate(&dsidev->dev);
 
        WARN_ON(dsi->scp_clk_refcount > 0);
index 4b00faa1a8cce700c6248e8b23e7c9cd010cb850..14a86e2c6d83e2d7cf6cbb2136d4a5347f9461b8 100644 (file)
@@ -365,14 +365,14 @@ static void dss_dump_clocks(struct dss_device *dss, struct seq_file *s)
 }
 #endif
 
-static void dss_dump_regs(struct seq_file *s)
+static int dss_dump_regs(struct seq_file *s, void *p)
 {
        struct dss_device *dss = s->private;
 
 #define DUMPREG(dss, r) seq_printf(s, "%-35s %08x\n", #r, dss_read_reg(dss, r))
 
        if (dss_runtime_get(dss))
-               return;
+               return 0;
 
        DUMPREG(dss, DSS_REVISION);
        DUMPREG(dss, DSS_SYSCONFIG);
@@ -387,6 +387,7 @@ static void dss_dump_regs(struct seq_file *s)
 
        dss_runtime_put(dss);
 #undef DUMPREG
+       return 0;
 }
 
 static int dss_get_channel_index(enum omap_channel channel)
@@ -888,7 +889,7 @@ struct dss_device *dss_get_device(struct device *dev)
 
 /* DEBUGFS */
 #if defined(CONFIG_OMAP2_DSS_DEBUGFS)
-static void dss_debug_dump_clocks(struct seq_file *s)
+static int dss_debug_dump_clocks(struct seq_file *s, void *p)
 {
        struct dss_device *dss = s->private;
 
@@ -897,28 +898,9 @@ static void dss_debug_dump_clocks(struct seq_file *s)
 #ifdef CONFIG_OMAP2_DSS_DSI
        dsi_dump_clocks(s);
 #endif
-}
-
-static int dss_debug_show(struct seq_file *s, void *unused)
-{
-       void (*func)(struct seq_file *) = s->private;
-
-       func(s);
        return 0;
 }
 
-static int dss_debug_open(struct inode *inode, struct file *file)
-{
-       return single_open(file, dss_debug_show, inode->i_private);
-}
-
-static const struct file_operations dss_debug_fops = {
-       .open           = dss_debug_open,
-       .read           = seq_read,
-       .llseek         = seq_lseek,
-       .release        = single_release,
-};
-
 static struct dentry *dss_debugfs_dir;
 
 static int dss_initialize_debugfs(struct dss_device *dss)
@@ -931,9 +913,6 @@ static int dss_initialize_debugfs(struct dss_device *dss)
                return err;
        }
 
-       debugfs_create_file("clk", S_IRUGO, dss_debugfs_dir,
-                       &dss_debug_dump_clocks, &dss_debug_fops);
-
        return 0;
 }
 
@@ -943,15 +922,59 @@ static void dss_uninitialize_debugfs(void)
                debugfs_remove_recursive(dss_debugfs_dir);
 }
 
-int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
+struct dss_debugfs_entry {
+       struct dentry *dentry;
+       int (*show_fn)(struct seq_file *s, void *data);
+       void *data;
+};
+
+static int dss_debug_open(struct inode *inode, struct file *file)
+{
+       struct dss_debugfs_entry *entry = inode->i_private;
+
+       return single_open(file, entry->show_fn, entry->data);
+}
+
+static const struct file_operations dss_debug_fops = {
+       .open           = dss_debug_open,
+       .read           = seq_read,
+       .llseek         = seq_lseek,
+       .release        = single_release,
+};
+
+struct dss_debugfs_entry *dss_debugfs_create_file(const char *name,
+               int (*show_fn)(struct seq_file *s, void *data), void *data)
 {
+       struct dss_debugfs_entry *entry;
        struct dentry *d;
 
-       d = debugfs_create_file(name, S_IRUGO, dss_debugfs_dir,
-                       write, &dss_debug_fops);
+       entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+       if (!entry)
+               return ERR_PTR(-ENOMEM);
+
+       entry->show_fn = show_fn;
+       entry->data = data;
 
-       return PTR_ERR_OR_ZERO(d);
+       d = debugfs_create_file(name, 0444, dss_debugfs_dir, entry,
+                               &dss_debug_fops);
+       if (IS_ERR(d)) {
+               kfree(entry);
+               return ERR_PTR(PTR_ERR(d));
+       }
+
+       entry->dentry = d;
+       return entry;
+}
+
+void dss_debugfs_remove_file(struct dss_debugfs_entry *entry)
+{
+       if (IS_ERR_OR_NULL(entry))
+               return;
+
+       debugfs_remove(entry->dentry);
+       kfree(entry);
 }
+
 #else /* CONFIG_OMAP2_DSS_DEBUGFS */
 static inline int dss_initialize_debugfs(struct dss_device *dss)
 {
@@ -1449,7 +1472,9 @@ static int dss_probe(struct platform_device *pdev)
        if (r)
                goto err_pm_runtime_disable;
 
-       dss_debugfs_create_file("dss", dss_dump_regs);
+       dss->debugfs.clk = dss_debugfs_create_file("clk", dss_debug_dump_clocks,
+                                                  dss);
+       dss->debugfs.dss = dss_debugfs_create_file("dss", dss_dump_regs, dss);
 
        /* Add all the child devices as components. */
        device_for_each_child(&pdev->dev, &match, dss_add_child_component);
@@ -1461,6 +1486,8 @@ static int dss_probe(struct platform_device *pdev)
        return 0;
 
 err_uninit_debugfs:
+       dss_debugfs_remove_file(dss->debugfs.clk);
+       dss_debugfs_remove_file(dss->debugfs.dss);
        dss_uninitialize_debugfs();
 
 err_pm_runtime_disable:
@@ -1488,6 +1515,8 @@ static int dss_remove(struct platform_device *pdev)
 
        component_master_del(&pdev->dev, &dss_component_ops);
 
+       dss_debugfs_remove_file(dss->debugfs.clk);
+       dss_debugfs_remove_file(dss->debugfs.dss);
        dss_uninitialize_debugfs();
 
        pm_runtime_disable(&pdev->dev);
index 89d708e8e970e9011026121a4dcc6032972f80f6..ec8e40e091418110799a4fe5740a2630bfb2a079 100644 (file)
 
 #include "omapdss.h"
 
+struct dss_debugfs_entry;
+struct platform_device;
+struct seq_file;
+
 #define MAX_DSS_LCD_MANAGERS   3
 #define MAX_NUM_DSI            2
 
@@ -233,9 +237,6 @@ struct dss_lcd_mgr_config {
        int lcden_sig_polarity;
 };
 
-struct seq_file;
-struct platform_device;
-
 #define DSS_SZ_REGS                    SZ_512
 
 struct dss_device {
@@ -261,6 +262,11 @@ struct dss_device {
 
        const struct dss_features *feat;
 
+       struct {
+               struct dss_debugfs_entry *clk;
+               struct dss_debugfs_entry *dss;
+       } debugfs;
+
        struct dss_pll  *video1_pll;
        struct dss_pll  *video2_pll;
 };
@@ -283,12 +289,20 @@ static inline bool dss_mgr_is_lcd(enum omap_channel id)
 
 /* DSS */
 #if defined(CONFIG_OMAP2_DSS_DEBUGFS)
-int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *));
+struct dss_debugfs_entry *dss_debugfs_create_file(const char *name,
+               int (*show_fn)(struct seq_file *s, void *data), void *data);
+void dss_debugfs_remove_file(struct dss_debugfs_entry *entry);
 #else
-static inline int dss_debugfs_create_file(const char *name,
-                                         void (*write)(struct seq_file *))
+static inline struct dss_debugfs_entry *
+dss_debugfs_create_file(const char *name,
+                       int (*show_fn)(struct seq_file *s, void *data),
+                       void *data)
+{
+       return NULL;
+}
+
+static inline void dss_debugfs_remove_file(struct dss_debugfs_entry *entry)
 {
-       return 0;
 }
 #endif /* CONFIG_OMAP2_DSS_DEBUGFS */
 
@@ -360,9 +374,6 @@ static inline void sdi_uninit_port(struct device_node *port)
 
 #ifdef CONFIG_OMAP2_DSS_DSI
 
-struct dentry;
-struct file_operations;
-
 void dsi_dump_clocks(struct seq_file *s);
 
 void dsi_irq_handler(void);
index 0563e1955048ac1ef026ad70046cb3a79f5d7236..fa2fbdaa427c09d29892c08ba0216fd473098b8a 100644 (file)
@@ -361,6 +361,8 @@ struct omap_hdmi {
        struct platform_device *pdev;
        struct dss_device *dss;
 
+       struct dss_debugfs_entry *debugfs;
+
        struct hdmi_wp_data     wp;
        struct hdmi_pll_data    pll;
        struct hdmi_phy_data    phy;
index 3e4a5cf2d06f64a7d379c77f54fc2be837fdc8c1..e528b7a223e19d7ac72bb56fe2b04ab4968efdfc 100644 (file)
@@ -303,13 +303,13 @@ static void hdmi_display_get_timings(struct omap_dss_device *dssdev,
        *vm = hdmi.cfg.vm;
 }
 
-static void hdmi_dump_regs(struct seq_file *s)
+static int hdmi_dump_regs(struct seq_file *s, void *p)
 {
        mutex_lock(&hdmi.lock);
 
        if (hdmi_runtime_get()) {
                mutex_unlock(&hdmi.lock);
-               return;
+               return 0;
        }
 
        hdmi_wp_dump(&hdmi.wp, s);
@@ -319,6 +319,7 @@ static void hdmi_dump_regs(struct seq_file *s)
 
        hdmi_runtime_put();
        mutex_unlock(&hdmi.lock);
+       return 0;
 }
 
 static int read_edid(u8 *buf, int len)
@@ -779,7 +780,7 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
                return r;
        }
 
-       dss_debugfs_create_file("hdmi", hdmi_dump_regs);
+       hdmi.debugfs = dss_debugfs_create_file("hdmi", hdmi_dump_regs, &hdmi);
 
        return 0;
 err:
@@ -791,6 +792,8 @@ static void hdmi4_unbind(struct device *dev, struct device *master, void *data)
 {
        struct platform_device *pdev = to_platform_device(dev);
 
+       dss_debugfs_remove_file(hdmi.debugfs);
+
        if (hdmi.audio_pdev)
                platform_device_unregister(hdmi.audio_pdev);
 
index e7a71012b1cd83184f66619c746da63a2350b3be..b3b9d17eb0e21475d2c583ea181c8634d5aa11f6 100644 (file)
@@ -299,13 +299,13 @@ static void hdmi_display_get_timings(struct omap_dss_device *dssdev,
        *vm = hdmi.cfg.vm;
 }
 
-static void hdmi_dump_regs(struct seq_file *s)
+static int hdmi_dump_regs(struct seq_file *s, void *p)
 {
        mutex_lock(&hdmi.lock);
 
        if (hdmi_runtime_get()) {
                mutex_unlock(&hdmi.lock);
-               return;
+               return 0;
        }
 
        hdmi_wp_dump(&hdmi.wp, s);
@@ -315,6 +315,7 @@ static void hdmi_dump_regs(struct seq_file *s)
 
        hdmi_runtime_put();
        mutex_unlock(&hdmi.lock);
+       return 0;
 }
 
 static int read_edid(u8 *buf, int len)
@@ -776,7 +777,7 @@ static int hdmi5_bind(struct device *dev, struct device *master, void *data)
                return r;
        }
 
-       dss_debugfs_create_file("hdmi", hdmi_dump_regs);
+       hdmi.debugfs = dss_debugfs_create_file("hdmi", hdmi_dump_regs, &hdmi);
 
        return 0;
 err:
@@ -788,6 +789,8 @@ static void hdmi5_unbind(struct device *dev, struct device *master, void *data)
 {
        struct platform_device *pdev = to_platform_device(dev);
 
+       dss_debugfs_remove_file(hdmi.debugfs);
+
        if (hdmi.audio_pdev)
                platform_device_unregister(hdmi.audio_pdev);
 
index 08bae18be188d46093c2fd3fa6cabb7b3d78d9ad..967a192e178996417d48cb64f92bf28cd885dd20 100644 (file)
@@ -327,6 +327,8 @@ static struct {
        struct regulator *vdda_dac_reg;
        struct dss_device *dss;
 
+       struct dss_debugfs_entry *debugfs;
+
        struct clk      *tv_dac_clk;
 
        struct videomode vm;
@@ -670,12 +672,12 @@ static int venc_init_regulator(void)
        return 0;
 }
 
-static void venc_dump_regs(struct seq_file *s)
+static int venc_dump_regs(struct seq_file *s, void *p)
 {
 #define DUMPREG(r) seq_printf(s, "%-35s %08x\n", #r, venc_read_reg(r))
 
        if (venc_runtime_get())
-               return;
+               return 0;
 
        DUMPREG(VENC_F_CONTROL);
        DUMPREG(VENC_VIDOUT_CTRL);
@@ -722,6 +724,7 @@ static void venc_dump_regs(struct seq_file *s)
        venc_runtime_put();
 
 #undef DUMPREG
+       return 0;
 }
 
 static int venc_get_clocks(struct platform_device *pdev)
@@ -914,7 +917,7 @@ static int venc_bind(struct device *dev, struct device *master, void *data)
                goto err_probe_of;
        }
 
-       dss_debugfs_create_file("venc", venc_dump_regs);
+       venc.debugfs = dss_debugfs_create_file("venc", venc_dump_regs, &venc);
 
        venc_init_output(pdev);
 
@@ -930,6 +933,8 @@ static void venc_unbind(struct device *dev, struct device *master, void *data)
 {
        struct platform_device *pdev = to_platform_device(dev);
 
+       dss_debugfs_remove_file(venc.debugfs);
+
        venc_uninit_output(pdev);
 
        pm_runtime_disable(&pdev->dev);