Hi Laurent, > On Tuesday 30 September 2014 16:16:07 Suman Anna wrote: >> The debugfs support for OMAP IOMMU is currently implemented >> as a module, warranting certain OMAP-specific IOMMU API to >> be exported. The OMAP IOMMU, when enabled, can only be built-in >> into the kernel, so integrate the OMAP IOMMU debug module >> into the OMAP IOMMU driver. This helps in eliminating the >> need to export most of the current OMAP IOMMU API. >> >> The following are the main changes: >> - The OMAP_IOMMU_DEBUG Kconfig option is eliminated, and >> the OMAP IOMMU debugfs support is built alongside the OMAP >> IOMMU driver, and enabled automatically only when debugfs >> is enabled. > > That's the part I'm unsure about. We're loosing the ability to save space by > not building the omap-iommu debugfs support when debugfs is enabled. Yeah, I thought about it a bit, and went in favor of eliminating the Kconfig. I don't have a strong opinion on this, but if saving space is what is preferred, I can easily rework this patch to add it back. Joerg, any preference which way we should go here? > > For the rest of the series, > > Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> Thank you for reviewing the patches. regards Suman > >> - The debugfs directory and entry creation logic is reversed, >> the calls are invoked by the OMAP IOMMU driver now. >> - The current iffy circular logic of adding IOMMU archdata >> to the IOMMU devices itself to get a pointer to the omap_iommu >> object in the debugfs support code is replaced by directly >> using the omap_iommu structure while creating the debugfs >> entries. >> - The debugfs root directory is renamed from the generic name >> "iommu" to a specific name "omap_iommu". >> - Unneeded headers have also been cleaned up while at this. >> - There will no longer be a omap-iommu-debug.ko module after >> this patch. >> >> Signed-off-by: Suman Anna <s-anna@xxxxxx> >> --- >> drivers/iommu/Kconfig | 9 ---- >> drivers/iommu/Makefile | 3 +- >> drivers/iommu/omap-iommu-debug.c | 102 >> ++++++++++++--------------------------- drivers/iommu/omap-iommu.c | >> 11 +++-- >> drivers/iommu/omap-iommu.h | 7 +++ >> 5 files changed, 45 insertions(+), 87 deletions(-) >> >> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig >> index dd51122..9724d4a 100644 >> --- a/drivers/iommu/Kconfig >> +++ b/drivers/iommu/Kconfig >> @@ -143,15 +143,6 @@ config OMAP_IOMMU >> depends on ARCH_OMAP2PLUS >> select IOMMU_API >> >> -config OMAP_IOMMU_DEBUG >> - tristate "Export OMAP IOMMU internals in DebugFS" >> - depends on OMAP_IOMMU && DEBUG_FS >> - help >> - Select this to see extensive information about >> - the internal state of OMAP IOMMU in debugfs. >> - >> - Say N unless you know you need this. >> - >> config TEGRA_IOMMU_GART >> bool "Tegra GART IOMMU Support" >> depends on ARCH_TEGRA_2x_SOC >> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile >> index 18fa446..e0a4fed 100644 >> --- a/drivers/iommu/Makefile >> +++ b/drivers/iommu/Makefile >> @@ -10,8 +10,7 @@ obj-$(CONFIG_DMAR_TABLE) += dmar.o >> obj-$(CONFIG_INTEL_IOMMU) += iova.o intel-iommu.o >> obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o >> obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o >> -obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o >> -obj-$(CONFIG_OMAP_IOMMU_DEBUG) += omap-iommu-debug.o >> +obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o omap-iommu-debug.o >> obj-$(CONFIG_TEGRA_IOMMU_GART) += tegra-gart.o >> obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-smmu.o >> obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o >> diff --git a/drivers/iommu/omap-iommu-debug.c >> b/drivers/iommu/omap-iommu-debug.c index 28de657..54a0dc6 100644 >> --- a/drivers/iommu/omap-iommu-debug.c >> +++ b/drivers/iommu/omap-iommu-debug.c >> @@ -10,15 +10,11 @@ >> * published by the Free Software Foundation. >> */ >> >> -#include <linux/module.h> >> #include <linux/err.h> >> -#include <linux/clk.h> >> #include <linux/io.h> >> #include <linux/slab.h> >> #include <linux/uaccess.h> >> -#include <linux/platform_device.h> >> #include <linux/debugfs.h> >> -#include <linux/omap-iommu.h> >> #include <linux/platform_data/iommu-omap.h> >> >> #include "omap-iopgtable.h" >> @@ -31,8 +27,7 @@ static struct dentry *iommu_debug_root; >> static ssize_t debug_read_regs(struct file *file, char __user *userbuf, >> size_t count, loff_t *ppos) >> { >> - struct device *dev = file->private_data; >> - struct omap_iommu *obj = dev_to_omap_iommu(dev); >> + struct omap_iommu *obj = file->private_data; >> char *p, *buf; >> ssize_t bytes; >> >> @@ -55,8 +50,7 @@ static ssize_t debug_read_regs(struct file *file, char >> __user *userbuf, static ssize_t debug_read_tlb(struct file *file, char >> __user *userbuf, size_t count, loff_t *ppos) >> { >> - struct device *dev = file->private_data; >> - struct omap_iommu *obj = dev_to_omap_iommu(dev); >> + struct omap_iommu *obj = file->private_data; >> char *p, *buf; >> ssize_t bytes, rest; >> >> @@ -141,8 +135,7 @@ out: >> static ssize_t debug_read_pagetable(struct file *file, char __user >> *userbuf, size_t count, loff_t *ppos) >> { >> - struct device *dev = file->private_data; >> - struct omap_iommu *obj = dev_to_omap_iommu(dev); >> + struct omap_iommu *obj = file->private_data; >> char *p, *buf; >> size_t bytes; >> >> @@ -181,93 +174,58 @@ DEBUG_FOPS_RO(pagetable); >> #define __DEBUG_ADD_FILE(attr, mode) \ >> { \ >> struct dentry *dent; \ >> - dent = debugfs_create_file(#attr, mode, parent, \ >> - dev, &debug_##attr##_fops); \ >> + dent = debugfs_create_file(#attr, mode, obj->debug_dir, \ >> + obj, &debug_##attr##_fops); \ >> if (!dent) \ >> - return -ENOMEM; \ >> + goto err; \ >> } >> >> #define DEBUG_ADD_FILE_RO(name) __DEBUG_ADD_FILE(name, 0400) >> >> -static int iommu_debug_register(struct device *dev, void *data) >> +void omap_iommu_debugfs_add(struct omap_iommu *obj) >> { >> - struct platform_device *pdev = to_platform_device(dev); >> - struct omap_iommu *obj = platform_get_drvdata(pdev); >> - struct omap_iommu_arch_data *arch_data; >> - struct dentry *d, *parent; >> - >> - if (!obj || !obj->dev) >> - return -EINVAL; >> - >> - arch_data = kzalloc(sizeof(*arch_data), GFP_KERNEL); >> - if (!arch_data) >> - return -ENOMEM; >> - >> - arch_data->iommu_dev = obj; >> + struct dentry *d; >> >> - dev->archdata.iommu = arch_data; >> + if (!iommu_debug_root) >> + return; >> >> - d = debugfs_create_dir(obj->name, iommu_debug_root); >> - if (!d) >> - goto nomem; >> - parent = d; >> + obj->debug_dir = debugfs_create_dir(obj->name, iommu_debug_root); >> + if (!obj->debug_dir) >> + return; >> >> - d = debugfs_create_u8("nr_tlb_entries", 0400, parent, >> + d = debugfs_create_u8("nr_tlb_entries", 0400, obj->debug_dir, >> (u8 *)&obj->nr_tlb_entries); >> if (!d) >> - goto nomem; >> + return; >> >> DEBUG_ADD_FILE_RO(regs); >> DEBUG_ADD_FILE_RO(tlb); >> DEBUG_ADD_FILE_RO(pagetable); >> >> - return 0; >> + return; >> >> -nomem: >> - kfree(arch_data); >> - return -ENOMEM; >> +err: >> + debugfs_remove_recursive(obj->debug_dir); >> } >> >> -static int iommu_debug_unregister(struct device *dev, void *data) >> +void omap_iommu_debugfs_remove(struct omap_iommu *obj) >> { >> - if (!dev->archdata.iommu) >> - return 0; >> - >> - kfree(dev->archdata.iommu); >> - >> - dev->archdata.iommu = NULL; >> + if (!obj->debug_dir) >> + return; >> >> - return 0; >> + debugfs_remove_recursive(obj->debug_dir); >> } >> >> -static int __init iommu_debug_init(void) >> +void __init omap_iommu_debugfs_init(void) >> { >> - struct dentry *d; >> - int err; >> - >> - d = debugfs_create_dir("iommu", NULL); >> - if (!d) >> - return -ENOMEM; >> - iommu_debug_root = d; >> - >> - err = omap_foreach_iommu_device(d, iommu_debug_register); >> - if (err) >> - goto err_out; >> - return 0; >> - >> -err_out: >> - debugfs_remove_recursive(iommu_debug_root); >> - return err; >> + if (debugfs_initialized()) { >> + iommu_debug_root = debugfs_create_dir("omap_iommu", NULL); >> + if (!iommu_debug_root) >> + pr_err("can't create debugfs dir\n"); >> + } >> } >> -module_init(iommu_debug_init) >> >> -static void __exit iommu_debugfs_exit(void) >> +void __exit omap_iommu_debugfs_exit(void) >> { >> - debugfs_remove_recursive(iommu_debug_root); >> - omap_foreach_iommu_device(NULL, iommu_debug_unregister); >> + debugfs_remove(iommu_debug_root); >> } >> -module_exit(iommu_debugfs_exit) >> - >> -MODULE_DESCRIPTION("omap iommu: debugfs interface"); >> -MODULE_AUTHOR("Hiroshi DOYU <Hiroshi.DOYU@xxxxxxxxx>"); >> -MODULE_LICENSE("GPL v2"); >> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c >> index 9ace5557..717d654 100644 >> --- a/drivers/iommu/omap-iommu.c >> +++ b/drivers/iommu/omap-iommu.c >> @@ -472,8 +472,6 @@ static void flush_iotlb_all(struct omap_iommu *obj) >> pm_runtime_put_sync(obj->dev); >> } >> >> -#if defined(CONFIG_OMAP_IOMMU_DEBUG) || >> defined(CONFIG_OMAP_IOMMU_DEBUG_MODULE) - >> #define pr_reg(name) \ >> do { \ >> ssize_t bytes; \ >> @@ -602,8 +600,6 @@ int omap_foreach_iommu_device(void *data, int >> (*fn)(struct device *, void *)) } >> EXPORT_SYMBOL_GPL(omap_foreach_iommu_device); >> >> -#endif /* CONFIG_OMAP_IOMMU_DEBUG_MODULE */ >> - >> /* >> * H/W pagetable operations >> */ >> @@ -1077,6 +1073,8 @@ static int omap_iommu_probe(struct platform_device >> *pdev) pm_runtime_irq_safe(obj->dev); >> pm_runtime_enable(obj->dev); >> >> + omap_iommu_debugfs_add(obj); >> + >> dev_info(&pdev->dev, "%s registered\n", obj->name); >> return 0; >> } >> @@ -1086,6 +1084,7 @@ static int omap_iommu_remove(struct platform_device >> *pdev) struct omap_iommu *obj = platform_get_drvdata(pdev); >> >> iopgtable_clear_entry_all(obj); >> + omap_iommu_debugfs_remove(obj); >> >> pm_runtime_disable(obj->dev); >> >> @@ -1403,6 +1402,8 @@ static int __init omap_iommu_init(void) >> >> bus_set_iommu(&platform_bus_type, &omap_iommu_ops); >> >> + omap_iommu_debugfs_init(); >> + >> return platform_driver_register(&omap_iommu_driver); >> } >> /* must be ready before omap3isp is probed */ >> @@ -1413,6 +1414,8 @@ static void __exit omap_iommu_exit(void) >> kmem_cache_destroy(iopte_cachep); >> >> platform_driver_unregister(&omap_iommu_driver); >> + >> + omap_iommu_debugfs_exit(); >> } >> module_exit(omap_iommu_exit); >> >> diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h >> index 0516e0e..6e9a8d2 100644 >> --- a/drivers/iommu/omap-iommu.h >> +++ b/drivers/iommu/omap-iommu.h >> @@ -30,6 +30,7 @@ struct omap_iommu { >> void __iomem *regbase; >> struct device *dev; >> struct iommu_domain *domain; >> + struct dentry *debug_dir; >> >> spinlock_t iommu_lock; /* global for this whole object */ >> >> @@ -202,6 +203,12 @@ omap_iommu_dump_ctx(struct omap_iommu *obj, char *buf, >> ssize_t len); extern size_t >> omap_dump_tlb_entries(struct omap_iommu *obj, char *buf, ssize_t len); >> >> +void omap_iommu_debugfs_init(void); >> +void omap_iommu_debugfs_exit(void); >> + >> +void omap_iommu_debugfs_add(struct omap_iommu *obj); >> +void omap_iommu_debugfs_remove(struct omap_iommu *obj); >> + >> /* >> * register accessors >> */ > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html