diff mbox

[6/7] omap: iommu: remove unused exported API

Message ID 1313622608-30397-7-git-send-email-ohad@wizery.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ohad Ben Cohen Aug. 17, 2011, 11:10 p.m. UTC
Remove unused public APIs from OMAP's iommu driver.

IOMMU functionality should be exposed only via the generic IOMMU API;
this way drivers stay generic, and different IOMMU drivers
don't need to duplicate similar functionalities.

The rest of the API still exposed by OMAP's iommu will be evaluated
and eventually either added to the generic IOMMU API (if relevant),
or completely removed.

The intention is that OMAP's iommu driver will eventually not expose
any public API.

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 arch/arm/plat-omap/include/plat/iommu.h |    3 --
 drivers/iommu/omap-iommu.c              |   59 -------------------------------
 2 files changed, 0 insertions(+), 62 deletions(-)

Comments

Hiroshi DOYU Aug. 18, 2011, 10:49 a.m. UTC | #1
Hi Ohad,

Just small question inlined....

From: ext Ohad Ben-Cohen <ohad@wizery.com>
Subject: [PATCH 6/7] omap: iommu: remove unused exported API
Date: Thu, 18 Aug 2011 02:10:07 +0300

> Remove unused public APIs from OMAP's iommu driver.
> 
> IOMMU functionality should be exposed only via the generic IOMMU API;
> this way drivers stay generic, and different IOMMU drivers
> don't need to duplicate similar functionalities.
> 
> The rest of the API still exposed by OMAP's iommu will be evaluated
> and eventually either added to the generic IOMMU API (if relevant),
> or completely removed.
> 
> The intention is that OMAP's iommu driver will eventually not expose
> any public API.
> 
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> ---
>  arch/arm/plat-omap/include/plat/iommu.h |    3 --
>  drivers/iommu/omap-iommu.c              |   59 -------------------------------
>  2 files changed, 0 insertions(+), 62 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h
> index 961b64f..eed5bdc 100644
> --- a/arch/arm/plat-omap/include/plat/iommu.h
> +++ b/arch/arm/plat-omap/include/plat/iommu.h
> @@ -153,12 +153,9 @@ struct iommu_platform_data {
>  extern u32 iommu_arch_version(void);
>  
>  extern void iotlb_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e);
> -extern void iommu_set_twl(struct iommu *obj, bool on);

This function was introduced by "Hari Kanigeri" for some OMAP4 case,
which is only using TLB, not H/W table walk.

> -extern void flush_iotlb_range(struct iommu *obj, u32 start, u32 end);
> -extern int iommu_set_da_range(struct iommu *obj, u32 start, u32 end);

This function was introduced by "David Cohen" to specify the available
ISP virtual address range.

Both requirements were quite reasonable at that time. Not necessary anymore?

>  extern int iommu_set_isr(const char *name,
>  			 int (*isr)(struct iommu *obj, u32 da, u32 iommu_errs,
>  				    void *priv),
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index 7280e5b..d6b77c4 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -357,26 +357,6 @@ static void flush_iotlb_page(struct iommu *obj, u32 da)
>  }
>  
>  /**
> - * flush_iotlb_range - Clear an iommu tlb entries
> - * @obj:	target iommu
> - * @start:	iommu device virtual address(start)
> - * @end:	iommu device virtual address(end)
> - *
> - * Clear an iommu tlb entry which includes 'da' address.
> - **/
> -void flush_iotlb_range(struct iommu *obj, u32 start, u32 end)
> -{
> -	u32 da = start;
> -
> -	while (da < end) {
> -		flush_iotlb_page(obj, da);
> -		/* FIXME: Optimize for multiple page size */
> -		da += IOPTE_SIZE;
> -	}
> -}
> -EXPORT_SYMBOL_GPL(flush_iotlb_range);
> -
> -/**
>   * flush_iotlb_all - Clear all iommu tlb entries
>   * @obj:	target iommu
>   **/
> @@ -395,23 +375,6 @@ static void flush_iotlb_all(struct iommu *obj)
>  	clk_disable(obj->clk);
>  }
>  
> -/**
> - * iommu_set_twl - enable/disable table walking logic
> - * @obj:	target iommu
> - * @on:		enable/disable
> - *
> - * Function used to enable/disable TWL. If one wants to work
> - * exclusively with locked TLB entries and receive notifications
> - * for TLB miss then call this function to disable TWL.
> - */
> -void iommu_set_twl(struct iommu *obj, bool on)
> -{
> -	clk_enable(obj->clk);
> -	arch_iommu->set_twl(obj, on);
> -	clk_disable(obj->clk);
> -}
> -EXPORT_SYMBOL_GPL(iommu_set_twl);
> -
>  #if defined(CONFIG_OMAP_IOMMU_DEBUG_MODULE)
>  
>  ssize_t iommu_dump_ctx(struct iommu *obj, char *buf, ssize_t bytes)
> @@ -848,28 +811,6 @@ static int device_match_by_alias(struct device *dev, void *data)
>  }
>  
>  /**
> - * iommu_set_da_range - Set a valid device address range
> - * @obj:		target iommu
> - * @start		Start of valid range
> - * @end			End of valid range
> - **/
> -int iommu_set_da_range(struct iommu *obj, u32 start, u32 end)
> -{
> -
> -	if (!obj)
> -		return -EFAULT;
> -
> -	if (end < start || !PAGE_ALIGN(start | end))
> -		return -EINVAL;
> -
> -	obj->da_start = start;
> -	obj->da_end = end;
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL_GPL(iommu_set_da_range);
> -
> -/**
>   * omap_find_iommu_device() - find an omap iommu device by name
>   * @name:	name of the iommu device
>   *
> -- 
> 1.7.4.1
> 
--
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
Ohad Ben Cohen Aug. 18, 2011, 11:01 a.m. UTC | #2
Hi Hiroshi,

On Thu, Aug 18, 2011 at 1:49 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
>> -extern void iommu_set_twl(struct iommu *obj, bool on);
>
> This function was introduced by "Hari Kanigeri" for some OMAP4 case,
> which is only using TLB, not H/W table walk.

We discussed that internally, and concluded it's not needed anymore. I
still kept the underlying plumbing mechanism, just in case it'd still
be needed for some hw quirks in the future, but removed the interface
(in case a future hw revision will seem to need it, it would probably
make more sense to invoke the relevant mechanism from the underlying
layers rather than from a higher layer).

>> -extern int iommu_set_da_range(struct iommu *obj, u32 start, u32 end);
>
> This function was introduced by "David Cohen" to specify the available
> ISP virtual address range.

Not sure if David or anyone is using this, but if someone is, it must
be out-of-tree.

In that case, even if it is needed, adding a mainline user for this
function at this point will most likely not be accepted anymore, and
instead, one will be suggested to use (and extend as necessary) the
DMA-API.

Thanks,
Ohad.
--
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
David Cohen Aug. 18, 2011, 1:40 p.m. UTC | #3
Hi Ohad,

On Thu, Aug 18, 2011 at 2:01 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> Hi Hiroshi,
>
> On Thu, Aug 18, 2011 at 1:49 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
>>> -extern void iommu_set_twl(struct iommu *obj, bool on);
>>
>> This function was introduced by "Hari Kanigeri" for some OMAP4 case,
>> which is only using TLB, not H/W table walk.
>
> We discussed that internally, and concluded it's not needed anymore. I
> still kept the underlying plumbing mechanism, just in case it'd still
> be needed for some hw quirks in the future, but removed the interface
> (in case a future hw revision will seem to need it, it would probably
> make more sense to invoke the relevant mechanism from the underlying
> layers rather than from a higher layer).
>
>>> -extern int iommu_set_da_range(struct iommu *obj, u32 start, u32 end);
>>
>> This function was introduced by "David Cohen" to specify the available
>> ISP virtual address range.

This function wasn't actually introduced by me, but I remember to refer it
when I was discussing to not define address 0x0 as valid in da range by
default (unless intentionally specified by client).

>
> Not sure if David or anyone is using this, but if someone is, it must
> be out-of-tree.

I am fine to remove it.

Br,

David

>
> In that case, even if it is needed, adding a mainline user for this
> function at this point will most likely not be accepted anymore, and
> instead, one will be suggested to use (and extend as necessary) the
> DMA-API.
>
> Thanks,
> Ohad.
> --
> 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
>
--
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
Ohad Ben Cohen Aug. 18, 2011, 1:45 p.m. UTC | #4
Hi David,

On Thu, Aug 18, 2011 at 4:40 PM, David Cohen <dacohen@gmail.com> wrote:
>> Not sure if David or anyone is using this, but if someone is, it must
>> be out-of-tree.
>
> I am fine to remove it.

OK, thanks for confirming!

Ohad.
--
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/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h
index 961b64f..eed5bdc 100644
--- a/arch/arm/plat-omap/include/plat/iommu.h
+++ b/arch/arm/plat-omap/include/plat/iommu.h
@@ -153,12 +153,9 @@  struct iommu_platform_data {
 extern u32 iommu_arch_version(void);
 
 extern void iotlb_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e);
-extern void iommu_set_twl(struct iommu *obj, bool on);
-extern void flush_iotlb_range(struct iommu *obj, u32 start, u32 end);
 
 extern int iopgtable_store_entry(struct iommu *obj, struct iotlb_entry *e);
 
-extern int iommu_set_da_range(struct iommu *obj, u32 start, u32 end);
 extern int iommu_set_isr(const char *name,
 			 int (*isr)(struct iommu *obj, u32 da, u32 iommu_errs,
 				    void *priv),
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 7280e5b..d6b77c4 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -357,26 +357,6 @@  static void flush_iotlb_page(struct iommu *obj, u32 da)
 }
 
 /**
- * flush_iotlb_range - Clear an iommu tlb entries
- * @obj:	target iommu
- * @start:	iommu device virtual address(start)
- * @end:	iommu device virtual address(end)
- *
- * Clear an iommu tlb entry which includes 'da' address.
- **/
-void flush_iotlb_range(struct iommu *obj, u32 start, u32 end)
-{
-	u32 da = start;
-
-	while (da < end) {
-		flush_iotlb_page(obj, da);
-		/* FIXME: Optimize for multiple page size */
-		da += IOPTE_SIZE;
-	}
-}
-EXPORT_SYMBOL_GPL(flush_iotlb_range);
-
-/**
  * flush_iotlb_all - Clear all iommu tlb entries
  * @obj:	target iommu
  **/
@@ -395,23 +375,6 @@  static void flush_iotlb_all(struct iommu *obj)
 	clk_disable(obj->clk);
 }
 
-/**
- * iommu_set_twl - enable/disable table walking logic
- * @obj:	target iommu
- * @on:		enable/disable
- *
- * Function used to enable/disable TWL. If one wants to work
- * exclusively with locked TLB entries and receive notifications
- * for TLB miss then call this function to disable TWL.
- */
-void iommu_set_twl(struct iommu *obj, bool on)
-{
-	clk_enable(obj->clk);
-	arch_iommu->set_twl(obj, on);
-	clk_disable(obj->clk);
-}
-EXPORT_SYMBOL_GPL(iommu_set_twl);
-
 #if defined(CONFIG_OMAP_IOMMU_DEBUG_MODULE)
 
 ssize_t iommu_dump_ctx(struct iommu *obj, char *buf, ssize_t bytes)
@@ -848,28 +811,6 @@  static int device_match_by_alias(struct device *dev, void *data)
 }
 
 /**
- * iommu_set_da_range - Set a valid device address range
- * @obj:		target iommu
- * @start		Start of valid range
- * @end			End of valid range
- **/
-int iommu_set_da_range(struct iommu *obj, u32 start, u32 end)
-{
-
-	if (!obj)
-		return -EFAULT;
-
-	if (end < start || !PAGE_ALIGN(start | end))
-		return -EINVAL;
-
-	obj->da_start = start;
-	obj->da_end = end;
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(iommu_set_da_range);
-
-/**
  * omap_find_iommu_device() - find an omap iommu device by name
  * @name:	name of the iommu device
  *