diff mbox

[12/17] iommu/omap: Integrate omap-iommu-debug into omap-iommu

Message ID 1412111772-38006-13-git-send-email-s-anna@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Suman Anna Sept. 30, 2014, 9:16 p.m. UTC
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.
- 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@ti.com>
---
 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(-)

Comments

Laurent Pinchart Oct. 1, 2014, 10:52 a.m. UTC | #1
Hi Suman,

Thank you for the patch.

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.

For the rest of the series,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> - 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@ti.com>
> ---
>  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@nokia.com>");
> -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
>   */
Suman Anna Oct. 1, 2014, 4:07 p.m. UTC | #2
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@ideasonboard.com>

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@ti.com>
>> ---
>>  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@nokia.com>");
>> -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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suman Anna Oct. 21, 2014, 9:28 p.m. UTC | #3
Hi Joerg,

On 10/01/2014 11:07 AM, Anna, Suman wrote:
> 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?

I am looking to refresh this series for 3.19, and this is the only patch
that may need some changes. Please let me know what your preference is,
and I can rework this patch if needed. Either way,
the plan is to not have an OMAP IOMMU debugfs module, but only to
have an option not to build the debugfs portions.

regards
Suman

> 
>>
>> For the rest of the series,
>>
>> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thank you for reviewing the patches.
> 
> regards
> Suman
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joerg Roedel Oct. 22, 2014, 1:29 p.m. UTC | #4
On Tue, Oct 21, 2014 at 04:28:27PM -0500, Suman Anna wrote:
> I am looking to refresh this series for 3.19, and this is the only patch
> that may need some changes. Please let me know what your preference is,
> and I can rework this patch if needed. Either way,
> the plan is to not have an OMAP IOMMU debugfs module, but only to
> have an option not to build the debugfs portions.

Can I apply this series without this patch and you send me an updated
version of only this patch? If yes, please let me know, otherwise please
rebase the series on v3.18-rc1 and resend.

Thanks,

	Joerg

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suman Anna Oct. 22, 2014, 6:38 p.m. UTC | #5
Hi Joerg,

On 10/22/2014 08:29 AM, Joerg Roedel wrote:
> On Tue, Oct 21, 2014 at 04:28:27PM -0500, Suman Anna wrote:
>> I am looking to refresh this series for 3.19, and this is the only patch
>> that may need some changes. Please let me know what your preference is,
>> and I can rework this patch if needed. Either way,
>> the plan is to not have an OMAP IOMMU debugfs module, but only to
>> have an option not to build the debugfs portions.
> 
> Can I apply this series without this patch and you send me an updated
> version of only this patch? If yes, please let me know, otherwise please
> rebase the series on v3.18-rc1 and resend.

Some of the patches following this patch don't apply without this patch,
so this patch alone cannot be dropped. The preceding patches (Patches 1
through 11) are not affected though. I will rebase and resend the entire
series.

regards
Suman


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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@nokia.com>");
-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
  */