[RFC 0/5] OMAP groundwork for IOMMU-based DMA API

[RFC 0/5] OMAP groundwork for IOMMU-based DMA API

[Ohad Ben-Cohen]

Another step towards generic IOMMU support on OMAP: eliminating
the public omap-specific find_iommu_device API.

Instead, we now bind iommu clients with their respective iommu device
using a private iommu member which is added to ARM's dev_archdata.

With this in hand, generic IOMMU API users will now work on OMAP without
utilizing any omap-specific API.

The changes are tested on OMAP3 (with omap3isp) and OMAP4 (with
remoteproc/rpmsg).

This is still RFC (2nd patch is probably the least elegant).

Ohad Ben-Cohen (5):
  ARM: dev_archdata: add private iommu extension
  ARM: OMAP: omap_device: add a method to set iommu private archdata
  ARM: OMAP: iommu: declare a private iommu binding struct
  ARM: OMAP3: bind omap3isp_device to its iommu device
  iommu/omap: eliminate the public omap_find_iommu_device() method

 arch/arm/include/asm/device.h                 |    3 +
 arch/arm/mach-omap2/devices.c                 |    7 +++
 arch/arm/plat-omap/include/plat/iommu.h       |   31 ++++++++++++-
 arch/arm/plat-omap/include/plat/iovmm.h       |   12 +++---
 arch/arm/plat-omap/include/plat/omap_device.h |    6 +++
 arch/arm/plat-omap/omap_device.c              |   39 +++++++++++++++-
 drivers/iommu/omap-iommu.c                    |   58 +++++++++++--------------
 drivers/iommu/omap-iovmm.c                    |   31 +++++++++-----
 drivers/media/video/omap3isp/isp.c            |   30 ++-----------
 drivers/media/video/omap3isp/isp.h            |    2 -
 drivers/media/video/omap3isp/ispccdc.c        |   18 ++++----
 drivers/media/video/omap3isp/ispstat.c        |    8 ++--
 drivers/media/video/omap3isp/ispvideo.c       |    4 +-
 13 files changed, 152 insertions(+), 97 deletions(-)

-- 
1.7.4.1

[RFC 2/5] ARM: OMAP: omap_device: add a method to set iommu private archdata

[Kevin Hilman]

Hi Ohad,

Ohad Ben-Cohen <ohad@wizery.com> writes:

> Make it possible to set an iommu private archdata before a newly-created
> omap device is registered.
>
> Binding iommu client devices with their respective iommu data this way
> is needed so the generic IOMMU API can later be used without employing
> any omap-specific IOMMU plumbing.
>
> This patch just crudely adds an omap_device_build_ss_ext() method which
> accepts an iommu private data, but we may actually want to do something
> more generic here: e.g., split the omap_device_build API to alloc+add
> methods, so users can just manipulate the device as needed before it is
> registered (very much like we can do with plain devices).

Yes indeed.  

Benoit did just this in preparation for DT. 

       http://marc.info/?l=linux-omap&m=131672480111927&w=2

Will that meet your needs?

Kevin

[RFC 2/5] ARM: OMAP: omap_device: add a method to set iommu private archdata

[Ohad Ben-Cohen]

Hi Kevin,

On Tue, Sep 27, 2011 at 1:53 AM, Kevin Hilman <khilman@ti.com> wrote:
> Benoit did just this in preparation for DT.
>
> ? ? ? http://marc.info/?l=linux-omap&m=131672480111927&w=2
>
> Will that meet your needs?

It's almost there, but not entirely.

Benoit's alloc/delete functions focus on the omap_device part, leaving
the handling of the platform device (allocation and pdata setting) to
omap_device_build_ss(), which at the same time registers the pdev.

I'd need to split omap_device_build_ss() into two: an alloc() part
which does everything but registering the pdev, and a register() part.
Users will first call alloc(), manually set archdata members, and then
call the register() part.

Something like this (compile-tested only, based on Benoit's
for_3.2/4_omap4_dt_early_devices branch):

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 0ae9e7f..9b8008c 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -671,7 +671,7 @@ struct platform_device *omap_device_build(const
char *pdev_name, int pdev_id,
 }

 /**
- * omap_device_build_ss - build and register an omap_device with
multiple hwmods
+ * omap_device_alloc_ss - build an omap_device with multiple hwmods
  * @pdev_name: name of the platform_device driver to use
  * @pdev_id: this platform_device's connection ID
  * @oh: ptr to the single omap_hwmod that backs this omap_device
@@ -679,19 +679,19 @@ struct platform_device *omap_device_build(const
char *pdev_name, int pdev_id,
  * @pdata_len: amount of memory pointed to by @pdata
  * @pm_lats: pointer to a omap_device_pm_latency array for this device
  * @pm_lats_cnt: ARRAY_SIZE() of @pm_lats
- * @is_early_device: should the device be registered as an early device or not
  *
- * Convenience function for building and registering an omap_device
+ * Convenience function for building (only) an omap_device
  * subsystem record.  Subsystem records consist of multiple
- * omap_hwmods.  This function in turn builds and registers a
- * platform_device record.  Returns an ERR_PTR() on error, or passes
- * along the return value of omap_device_register().
+ * omap_hwmods.  This function in turn builds (but doesn't register) a
+ * platform_device record, so callers can manipulate it (typically by
+ * setting one or more archdata members) before it's registered.
+ * Returns an ERR_PTR() on error, or the allocated pdev on success.
  */
-struct platform_device *omap_device_build_ss(const char *pdev_name,
int pdev_id,
+struct platform_device *omap_device_alloc_ss(const char *pdev_name,
int pdev_id,
 					 struct omap_hwmod **ohs, int oh_cnt,
 					 void *pdata, int pdata_len,
 					 struct omap_device_pm_latency *pm_lats,
-					 int pm_lats_cnt, int is_early_device)
+					 int pm_lats_cnt)
 {
 	int ret = -ENOMEM;
 	struct platform_device *pdev;
@@ -723,13 +723,6 @@ struct platform_device
*omap_device_build_ss(const char *pdev_name, int pdev_id,
 	if (ret)
 		goto odbs_exit2;

-	if (is_early_device)
-		ret = omap_early_device_register(pdev);
-	else
-		ret = omap_device_register(pdev);
-	if (ret)
-		goto odbs_exit2;
-
 	return pdev;

 odbs_exit2:
@@ -744,6 +737,93 @@ odbs_exit:
 }

 /**
+ * omap_device_delete_ss - free an omap_device-based platform device
+ * @pdev: platform_device that represents this omap_device
+ *
+ * Convenience function for freeing a platform_device record, which
+ * is based on an omap_device subsystem record.
+ *
+ * Use this function only if @pdev was created using omap_device_alloc_ss(),
+ * most commonly because a subsequent call to omap_device_register_ss() failed.
+ */
+void omap_device_delete_ss(struct platform_device *pdev)
+{
+	struct omap_device *od = to_omap_device(pdev);
+
+	omap_device_delete(od);
+	platform_device_put(pdev);
+}
+
+/**
+ * omap_device_register_ss - register an omap_device-based platform device
+ * @pdev: platform_device that represents this omap_device
+ * @is_early_device: should the device be registered as an early device or not
+ *
+ * Convenience function for registering a platform_device record, which
+ * is based on an omap_device subsystem record, which was created using
+ * omap_device_alloc_ss().
+ *
+ * Returns 0 on success, or an appropriate error value otherwise (essentially
+ * by returning the value of platform_device_register())
+ */
+int omap_device_register_ss(struct platform_device *pdev, int is_early_device)
+{
+	int ret;
+
+	if (is_early_device)
+		ret = omap_early_device_register(pdev);
+	else
+		ret = omap_device_register(pdev);
+
+	return ret;
+}
+
+/**
+ * omap_device_build_ss - build and register an omap_device with
multiple hwmods
+ * @pdev_name: name of the platform_device driver to use
+ * @pdev_id: this platform_device's connection ID
+ * @oh: ptr to the single omap_hwmod that backs this omap_device
+ * @pdata: platform_data ptr to associate with the platform_device
+ * @pdata_len: amount of memory pointed to by @pdata
+ * @pm_lats: pointer to a omap_device_pm_latency array for this device
+ * @pm_lats_cnt: ARRAY_SIZE() of @pm_lats
+ * @is_early_device: should the device be registered as an early device or not
+ *
+ * Convenience function for building and registering an omap_device
+ * subsystem record.  Subsystem records consist of multiple
+ * omap_hwmods.  This function in turn builds and registers a
+ * platform_device record.  Returns an ERR_PTR() on error, or passes
+ * along the built pdev on success.
+ */
+struct platform_device *omap_device_build_ss(const char *pdev_name,
int pdev_id,
+					 struct omap_hwmod **ohs, int oh_cnt,
+					 void *pdata, int pdata_len,
+					 struct omap_device_pm_latency *pm_lats,
+					 int pm_lats_cnt, int is_early_device)
+{
+	struct platform_device *pdev;
+	int ret;
+
+	pdev = omap_device_alloc_ss(pdev_name, pdev_id, ohs, oh_cnt, pdata,
+			pdata_len, pm_lats, pm_lats_cnt);
+
+	if (IS_ERR(pdev))
+		goto out;
+
+	ret = omap_device_register_ss(pdev, is_early_device);
+	if (ret) {
+		pdev = ERR_PTR(ret);
+		goto delete_od;
+	}
+
+	return pdev;
+delete_od:
+	omap_device_delete_ss(pdev);
+out:
+	return pdev;
+}
+
+/**
  * omap_early_device_register - register an omap_device as an early platform
  * device.
  * @od: struct omap_device * to register


That's the idea; please tell me how you'd like to see this go forward
(there are at least several personal-taste issues here, e.g., naming:
now we have two sets of alloc/delete functions which have different
semantics) and which branch would you like me to base this work off of
(not sure if Benoit's patches already went into your
for_3.2/omap_device branch) and I'll respin this patch properly.

Thanks a lot!
Ohad.

[RFC 2/5] ARM: OMAP: omap_device: add a method to set iommu private archdata

[Kevin Hilman]

Ohad Ben-Cohen <ohad@wizery.com> writes:

> Hi Kevin,
>
> On Tue, Sep 27, 2011 at 1:53 AM, Kevin Hilman <khilman@ti.com> wrote:
>> Benoit did just this in preparation for DT.
>>
>> ? ? ? http://marc.info/?l=linux-omap&m=131672480111927&w=2
>>
>> Will that meet your needs?
>
> It's almost there, but not entirely.
>
> Benoit's alloc/delete functions focus on the omap_device part, leaving
> the handling of the platform device (allocation and pdata setting) to
> omap_device_build_ss(), which at the same time registers the pdev.
>
> I'd need to split omap_device_build_ss() into two: an alloc() part
> which does everything but registering the pdev, and a register() part.
> Users will first call alloc(), manually set archdata members, and then
> call the register() part.
>
> Something like this (compile-tested only, based on Benoit's
> for_3.2/4_omap4_dt_early_devices branch):

[...]

> That's the idea; please tell me how you'd like to see this go forward
> (there are at least several personal-taste issues here, e.g., naming:
> now we have two sets of alloc/delete functions which have different
> semantics) 

The approach is OK with me, but I'm a bit torn about whether or not to
merge this since the need for this should go away when converting to DT.
Is anyone working on IOMMU DT description?

I guess if I do merge this, we can also clean this up significantly
after all devices are converted to DT.

> and which branch would you like me to base this work off of
> (not sure if Benoit's patches already went into your
> for_3.2/omap_device branch) and I'll respin this patch properly.

Unless it has other dependencies on Benoit's further DT patches, you can
base this on my for_3.2/omap_device-2 branch which includes Benoit's
alloc changes.

Kevin

[RFC 2/5] ARM: OMAP: omap_device: add a method to set iommu private archdata

[Ohad Ben-Cohen]

On Tue, Oct 4, 2011 at 8:15 PM, Kevin Hilman <khilman@ti.com> wrote:
> The approach is OK with me, but I'm a bit torn about whether or not to
> merge this since the need for this should go away when converting to DT.

I guess it will still take some time until our boards are fully
functional with DT, so I think we should regard this as an interim
solution. Without this patch, a huge deal of code consolidation and
generalization is gated (namely - generic iommu and dma api, and
everything that depends on it, e.g. rpmsg/remoteproc).

> Is anyone working on IOMMU DT description?

Unless Benoit is, I'll happily pick this up.

> I guess if I do merge this, we can also clean this up significantly
> after all devices are converted to DT.

Sure.

> Unless it has other dependencies on Benoit's further DT patches, you can
> base this on my for_3.2/omap_device-2 branch which includes Benoit's
> alloc changes.

Ok, thanks a lot !
Ohad.

[RFC 2/5] ARM: OMAP: omap_device: add a method to set iommu private archdata

[Kevin Hilman]

Ohad Ben-Cohen <ohad@wizery.com> writes:

> On Tue, Oct 4, 2011 at 8:15 PM, Kevin Hilman <khilman@ti.com> wrote:
>> The approach is OK with me, but I'm a bit torn about whether or not to
>> merge this since the need for this should go away when converting to DT.
>
> I guess it will still take some time until our boards are fully
> functional with DT, so I think we should regard this as an interim
> solution. Without this patch, a huge deal of code consolidation and
> generalization is gated (namely - generic iommu and dma api, and
> everything that depends on it, e.g. rpmsg/remoteproc).
>
>> Is anyone working on IOMMU DT description?
>
> Unless Benoit is, I'll happily pick this up.

tag.  you're it.

>> I guess if I do merge this, we can also clean this up significantly
>> after all devices are converted to DT.
>
> Sure.

OK, as long as I have some confidence that this is going in the right
direction (and having you on the job is a good sign!)  I'm willing to
merge something like this as an interim solution.

Kevin

>> Unless it has other dependencies on Benoit's further DT patches, you can
>> base this on my for_3.2/omap_device-2 branch which includes Benoit's
>> alloc changes.
>
> Ok, thanks a lot !
> Ohad.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[RFC 2/5] ARM: OMAP: omap_device: add a method to set iommu private archdata

[Ohad Ben-Cohen]

On Tue, Oct 4, 2011 at 10:29 PM, Kevin Hilman <khilman@ti.com> wrote:
> tag. ?you're it.

Consider it done :)

> OK, as long as I have some confidence that this is going in the right
> direction (and having you on the job is a good sign!) ?I'm willing to
> merge something like this as an interim solution.

Thanks a lot!
Ohad.

[RFC 2/5] ARM: OMAP: omap_device: add a method to set iommu private archdata

[Cousson, Benoit]

On 10/4/2011 9:54 PM, Ohad Ben-Cohen wrote:
> On Tue, Oct 4, 2011 at 8:15 PM, Kevin Hilman<khilman@ti.com>  wrote:
>> The approach is OK with me, but I'm a bit torn about whether or not to
>> merge this since the need for this should go away when converting to DT.
>
> I guess it will still take some time until our boards are fully
> functional with DT, so I think we should regard this as an interim
> solution. Without this patch, a huge deal of code consolidation and
> generalization is gated (namely - generic iommu and dma api, and
> everything that depends on it, e.g. rpmsg/remoteproc).
>
>> Is anyone working on IOMMU DT description?
>
> Unless Benoit is, I'll happily pick this up.

Cool, a volunteer... You're in.

Thanks,
Benoit

[RFC 1/5] ARM: dev_archdata: add private iommu extension

[Grant Likely]

On Sun, Sep 25, 2011 at 01:58:53PM +0300, Ohad Ben-Cohen wrote:
> Add a private iommu pointer to the ARM-specific arch data in the
> device struct, which will be used to attach iommu-specific data
> to devices which require iommu support.
> 
> Different iommu implementations (on different platforms) will attach
> different types of data to this pointer, so 'void *' is currently used
> (the downside is reduced typesafety).
> 
> Note: ia64, x86 and sparc have this exact iommu extension as well, and
> if others are likely to adopt it too, we might want to consider
> adding this to the device struct itself directly.
> 
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> ---
>  arch/arm/include/asm/device.h |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
> index 9f390ce..6615f03 100644
> --- a/arch/arm/include/asm/device.h
> +++ b/arch/arm/include/asm/device.h
> @@ -10,6 +10,9 @@ struct dev_archdata {
>  #ifdef CONFIG_DMABOUNCE
>  	struct dmabounce_device_info *dmabounce;
>  #endif
> +#ifdef CONFIG_IOMMU_API
> +	void *iommu; /* private IOMMU data */
> +#endif

Blech.  Oh well.  Not much point in doing something different if x86
uses a void*.

g.

[RFC 1/5] ARM: dev_archdata: add private iommu extension

[Ohad Ben-Cohen]

On Tue, Sep 27, 2011 at 4:30 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> Blech. ?Oh well. ?Not much point in doing something different if x86
> uses a void*.

x86 probably does this because two different implementations needs to
plug their private data there: intel-iommu plugs there a 'struct
device_domain_info *' and amd_iommu uses it with a 'struct
iommu_dev_data *'.

On ARM we'd eventually end up with even a bigger variety, and I guess
that even if we'd use a type-safe member here, it would itself end up
having 'void *'.

If it looks reasonable to you, can I please have your Ack ?

Russell, can you please take a look too and ack/nack ?

Thanks,
Ohad.

[RFC 1/5] ARM: dev_archdata: add private iommu extension

[Ohad Ben-Cohen]

Hi Russell,

On Sun, Sep 25, 2011 at 1:58 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> Add a private iommu pointer to the ARM-specific arch data in the
> device struct, which will be used to attach iommu-specific data
> to devices which require iommu support.

Can you please take a look and ack/nack ?

With this change in hand, generic code will now be able to control
OMAP's iommu devices too, without having to use any OMAP-specific API
(this is demonstrated in this patch set).

This will allow us to further clean the existing mainline OMAP iommu
users, and focus on generic IOMMU code for future users
(rpmsg/remoteproc and Marek's upcoming generic DMA API).

The change is only effective when an iommu driver, which supports the
generic IOMMU API, is built (all of them must select
CONFIG_IOMMU_API), and otherwise is a nop.

Obviously the change itself is not OMAP-specific, and it will be
useful for other platforms too.

Thanks!
Ohad.

> Different iommu implementations (on different platforms) will attach
> different types of data to this pointer, so 'void *' is currently used
> (the downside is reduced typesafety).
>
> Note: ia64, x86 and sparc have this exact iommu extension as well, and
> if others are likely to adopt it too, we might want to consider
> adding this to the device struct itself directly.
>
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> ---
> ?arch/arm/include/asm/device.h | ? ?3 +++
> ?1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
> index 9f390ce..6615f03 100644
> --- a/arch/arm/include/asm/device.h
> +++ b/arch/arm/include/asm/device.h
> @@ -10,6 +10,9 @@ struct dev_archdata {
> ?#ifdef CONFIG_DMABOUNCE
> ? ? ? ?struct dmabounce_device_info *dmabounce;
> ?#endif
> +#ifdef CONFIG_IOMMU_API
> + ? ? ? void *iommu; /* private IOMMU data */
> +#endif
> ?};
>
> ?struct pdev_archdata {
> --
> 1.7.4.1
>
>

[RFC 1/5] ARM: dev_archdata: add private iommu extension

[Ohad Ben-Cohen]

On Wed, Oct 5, 2011 at 11:26 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> Hi Russell,
>
> On Sun, Sep 25, 2011 at 1:58 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>> Add a private iommu pointer to the ARM-specific arch data in the
>> device struct, which will be used to attach iommu-specific data
>> to devices which require iommu support.
>
> Can you please take a look and ack/nack ?

gentle reminder

[RFC 1/5] ARM: dev_archdata: add private iommu extension

[Russell King - ARM Linux]

On Thu, Oct 13, 2011 at 11:15:39AM +0200, Ohad Ben-Cohen wrote:
> On Wed, Oct 5, 2011 at 11:26 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> > Hi Russell,
> >
> > On Sun, Sep 25, 2011 at 1:58 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> >> Add a private iommu pointer to the ARM-specific arch data in the
> >> device struct, which will be used to attach iommu-specific data
> >> to devices which require iommu support.
> >
> > Can you please take a look and ack/nack ?
> 
> gentle reminder

Ok, I'll take it if you put it in the patch system.

Thanks.

[RFC 1/5] ARM: dev_archdata: add private iommu extension

[Ohad Ben-Cohen]

On Thu, Oct 13, 2011 at 12:42 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> Ok, I'll take it if you put it in the patch system.

Done, thanks.

[RFC 5/5] iommu/omap: eliminate the public omap_find_iommu_device() method

[Laurent Pinchart]

Hi Ohad,

Thanks for the patch.

On Sunday 25 September 2011 12:58:57 Ohad Ben-Cohen wrote:
> Eliminate the public omap_find_iommu_device() method, and don't
> expect clients to provide the omap_iommu handle anymore.
> 
> Instead, OMAP's iommu driver should now utilize dev_archdata's private
> iommu extension to be able to access the required iommu information.
> 
> Update omap3isp appropriately.
> 
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> Cc: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Tony Lindgren <tony@atomide.com>

For the OMAP3 ISP driver,

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

-- 
Regards,

Laurent Pinchart

[RFC 5/5] iommu/omap: eliminate the public omap_find_iommu_device() method

[Ohad Ben-Cohen]

Hi Laurent,

On Tue, Sep 27, 2011 at 2:46 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Sunday 25 September 2011 12:58:57 Ohad Ben-Cohen wrote:
>> Eliminate the public omap_find_iommu_device() method, and don't
>> expect clients to provide the omap_iommu handle anymore.
>>
>> Instead, OMAP's iommu driver should now utilize dev_archdata's private
>> iommu extension to be able to access the required iommu information.
>>
>> Update omap3isp appropriately.
>>
>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>> Cc: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>
> For the OMAP3 ISP driver,
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks !

Can you please also ack the 4th patch in this series (that's the part
the makes this fifth patch possible):

https://lkml.org/lkml/2011/9/25/46

I could then ask Tony to Ack it so we could merge this via the iommu tree.

Thanks,
Ohad.

[RFC 5/5] iommu/omap: eliminate the public omap_find_iommu_device() method

[Laurent Pinchart]

Hi Ohad,

On Wednesday 09 November 2011 09:50:30 Ohad Ben-Cohen wrote:
> On Tue, Sep 27, 2011 at 2:46 PM, Laurent Pinchart wrote:
> > On Sunday 25 September 2011 12:58:57 Ohad Ben-Cohen wrote:
> >> Eliminate the public omap_find_iommu_device() method, and don't
> >> expect clients to provide the omap_iommu handle anymore.
> >> 
> >> Instead, OMAP's iommu driver should now utilize dev_archdata's private
> >> iommu extension to be able to access the required iommu information.
> >> 
> >> Update omap3isp appropriately.
> >> 
> >> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> >> Cc: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> Cc: Tony Lindgren <tony@atomide.com>
> > 
> > For the OMAP3 ISP driver,
> > 
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thanks !
> 
> Can you please also ack the 4th patch in this series (that's the part
> the makes this fifth patch possible):
> 
> https://lkml.org/lkml/2011/9/25/46

Sure. Done.

> I could then ask Tony to Ack it so we could merge this via the iommu tree.

-- 
Regards,

Laurent Pinchart

[RFC 5/5] iommu/omap: eliminate the public omap_find_iommu_device() method

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [111109 00:42]:
> Hi Ohad,
> 
> On Wednesday 09 November 2011 09:50:30 Ohad Ben-Cohen wrote:
> > On Tue, Sep 27, 2011 at 2:46 PM, Laurent Pinchart wrote:
> > > On Sunday 25 September 2011 12:58:57 Ohad Ben-Cohen wrote:
> > >> Eliminate the public omap_find_iommu_device() method, and don't
> > >> expect clients to provide the omap_iommu handle anymore.
> > >> 
> > >> Instead, OMAP's iommu driver should now utilize dev_archdata's private
> > >> iommu extension to be able to access the required iommu information.
> > >> 
> > >> Update omap3isp appropriately.
> > >> 
> > >> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> > >> Cc: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >> Cc: Tony Lindgren <tony@atomide.com>
> > > 
> > > For the OMAP3 ISP driver,
> > > 
> > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Thanks !
> > 
> > Can you please also ack the 4th patch in this series (that's the part
> > the makes this fifth patch possible):
> > 
> > https://lkml.org/lkml/2011/9/25/46
> 
> Sure. Done.
> 
> > I could then ask Tony to Ack it so we could merge this via the iommu tree.

This too:

Acked-by: Tony Lindgren <tony@atomide.com>

[RFC 4/5] ARM: OMAP3: bind omap3isp_device to its iommu device

[Laurent Pinchart]

Hi Ohad,

On Sunday 25 September 2011 12:58:56 Ohad Ben-Cohen wrote:
> Bind OMAP3's isp device to the isp's dedicated iommu, by setting
> the device's archdata iommu member.
> 
> This way omap3isp will be able to use the generic IOMMU API without
> having to call any omap-specific binding method.
> 
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>  arch/arm/mach-omap2/devices.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index 1077ad6..44122f7 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -28,6 +28,7 @@
>  #include <plat/mcbsp.h>
>  #include <mach/gpio.h>
>  #include <plat/mmc.h>
> +#include <plat/iommu.h>
>  #include <plat/dma.h>
>  #include <plat/omap_hwmod.h>
>  #include <plat/omap_device.h>
> @@ -207,9 +208,15 @@ static struct platform_device omap3isp_device = {
>  	.resource	= omap3isp_resources,
>  };
> 
> +static struct omap_iommu_arch_data omap3_isp_iommu = {
> +	.name = "isp",
> +};
> +
>  int omap3_init_camera(struct isp_platform_data *pdata)
>  {
>  	omap3isp_device.dev.platform_data = pdata;
> +	omap3isp_device.dev.archdata.iommu = &omap3_isp_iommu;
> +
>  	return platform_device_register(&omap3isp_device);
>  }

-- 
Regards,

Laurent Pinchart

[RFC 4/5] ARM: OMAP3: bind omap3isp_device to its iommu device

[Ohad Ben-Cohen]

On Wed, Nov 9, 2011 at 11:17 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Ohad,
>
> On Sunday 25 September 2011 12:58:56 Ohad Ben-Cohen wrote:
>> Bind OMAP3's isp device to the isp's dedicated iommu, by setting
>> the device's archdata iommu member.
>>
>> This way omap3isp will be able to use the generic IOMMU API without
>> having to call any omap-specific binding method.
>>
>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, Laurent!

Tony, can you please Ack the OMAP parts of this patch set:

https://lkml.org/lkml/2011/9/25/44 (patch 3/5)
https://lkml.org/lkml/2011/9/25/46 (patch 4/5)
https://lkml.org/lkml/2011/9/25/45 (patch 5/5)

With this in hand, generic iommu users will now work on OMAP platforms
too (without calling any omap-specific API).

Thanks,
Ohad.

[RFC 4/5] ARM: OMAP3: bind omap3isp_device to its iommu device

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [111109 00:42]:
> Hi Ohad,
> 
> On Sunday 25 September 2011 12:58:56 Ohad Ben-Cohen wrote:
> > Bind OMAP3's isp device to the isp's dedicated iommu, by setting
> > the device's archdata iommu member.
> > 
> > This way omap3isp will be able to use the generic IOMMU API without
> > having to call any omap-specific binding method.
> > 
> > Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Tony Lindgren <tony@atomide.com>
 
> > ---
> >  arch/arm/mach-omap2/devices.c |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> > index 1077ad6..44122f7 100644
> > --- a/arch/arm/mach-omap2/devices.c
> > +++ b/arch/arm/mach-omap2/devices.c
> > @@ -28,6 +28,7 @@
> >  #include <plat/mcbsp.h>
> >  #include <mach/gpio.h>
> >  #include <plat/mmc.h>
> > +#include <plat/iommu.h>
> >  #include <plat/dma.h>
> >  #include <plat/omap_hwmod.h>
> >  #include <plat/omap_device.h>
> > @@ -207,9 +208,15 @@ static struct platform_device omap3isp_device = {
> >  	.resource	= omap3isp_resources,
> >  };
> > 
> > +static struct omap_iommu_arch_data omap3_isp_iommu = {
> > +	.name = "isp",
> > +};
> > +
> >  int omap3_init_camera(struct isp_platform_data *pdata)
> >  {
> >  	omap3isp_device.dev.platform_data = pdata;
> > +	omap3isp_device.dev.archdata.iommu = &omap3_isp_iommu;
> > +
> >  	return platform_device_register(&omap3isp_device);
> >  }
> 
> -- 
> Regards,
> 
> Laurent Pinchart

[RFC 3/5] ARM: OMAP: iommu: declare a private iommu binding struct

[Tony Lindgren]

* Ohad Ben-Cohen <ohad@wizery.com> [110925 03:27]:
> Declare an omap iommu private struct, which binds an iommu user
> to its iommu device. This struct should be placed at the iommu user's
> dev_archdata so generic IOMMU API can be used without having to
> utilize omap-specific plumbing anymore.
> 
> While at it, provide an accessor method to ease the retrieval of the
> omap_iommu handle from a user device.
> 
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>

Acked-by: Tony Lindgren <tony@atomide.com>


> ---
>  arch/arm/plat-omap/include/plat/iommu.h |   26 ++++++++++++++++++++++++++
>  1 files changed, 26 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h
> index a1d79ee..fa11ee2 100644
> --- a/arch/arm/plat-omap/include/plat/iommu.h
> +++ b/arch/arm/plat-omap/include/plat/iommu.h
> @@ -111,6 +111,32 @@ struct iommu_platform_data {
>  	u32 da_end;
>  };
>  
> +/**
> + * struct iommu_arch_data - omap iommu private data
> + * @name: name of the iommu device
> + * @iommu_dev: handle of the iommu device
> + *
> + * This is an omap iommu private data object, which binds an iommu user
> + * to its iommu device. This object should be placed at the iommu user's
> + * dev_archdata so generic IOMMU API can be used without having to
> + * utilize omap-specific plumbing anymore.
> + */
> +struct omap_iommu_arch_data {
> +	const char *name;
> +	struct omap_iommu *iommu_dev;
> +};
> +
> +/**
> + * dev_to_omap_iommu() - retrieves an omap iommu object from a user device
> + * @dev: iommu client device
> + */
> +static inline struct omap_iommu *dev_to_omap_iommu(struct device *dev)
> +{
> +	struct omap_iommu_arch_data *arch_data = dev->archdata.iommu;
> +
> +	return arch_data->iommu_dev;
> +}
> +
>  /* IOMMU errors */
>  #define OMAP_IOMMU_ERR_TLB_MISS		(1 << 0)
>  #define OMAP_IOMMU_ERR_TRANS_FAULT	(1 << 1)
> -- 
> 1.7.4.1
> 

[RFC 0/5] OMAP groundwork for IOMMU-based DMA API

[Roedel, Joerg]

Hi Ohad,

On Sun, Sep 25, 2011 at 06:58:52AM -0400, Ohad Ben-Cohen wrote:
> Ohad Ben-Cohen (5):
>   ARM: dev_archdata: add private iommu extension
>   ARM: OMAP: omap_device: add a method to set iommu private archdata
>   ARM: OMAP: iommu: declare a private iommu binding struct
>   ARM: OMAP3: bind omap3isp_device to its iommu device
>   iommu/omap: eliminate the public omap_find_iommu_device() method

It doesn't apply cleanly, can you please rebase the code, collect the
Acked-by's and resend?

Thanks,

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

[RFC 0/5] OMAP groundwork for IOMMU-based DMA API

[Ohad Ben-Cohen]

Hi Joerg,

On Tue, Nov 15, 2011 at 12:39 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> It doesn't apply cleanly, can you please rebase the code, collect the
> Acked-by's and resend?

Sure. I planned on resending after the "hardware pgsize" patches are
merged, but please tell me if you prefer it the other way around.

Thanks,
Ohad.

[RFC 0/5] OMAP groundwork for IOMMU-based DMA API

[Roedel, Joerg]

On Tue, Nov 15, 2011 at 12:45:33PM +0200, Ohad Ben-Cohen wrote:
> Hi Joerg,
> 
> On Tue, Nov 15, 2011 at 12:39 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> > It doesn't apply cleanly, can you please rebase the code, collect the
> > Acked-by's and resend?
> 
> Sure. I planned on resending after the "hardware pgsize" patches are
> merged, but please tell me if you prefer it the other way around.

It is almost merged :) I just had to struggle with some compile errors
from upstream in my tests. But I think I will finish testing today and
push everything out.

Thanks,

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

[RFC 0/5] OMAP groundwork for IOMMU-based DMA API

[Ohad Ben-Cohen]

On Tue, Nov 15, 2011 at 1:10 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> It is almost merged :) I just had to struggle with some compile errors
> from upstream in my tests. But I think I will finish testing today and
> push everything out.
>

ok great thanks :)

[RFC 0/5] OMAP groundwork for IOMMU-based DMA API

[Ohad Ben-Cohen]

Hi Joerg,

On Tue, Nov 15, 2011 at 12:39 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> On Sun, Sep 25, 2011 at 06:58:52AM -0400, Ohad Ben-Cohen wrote:
>> Ohad Ben-Cohen (5):
>> ? ARM: dev_archdata: add private iommu extension
>> ? ARM: OMAP: omap_device: add a method to set iommu private archdata
>> ? ARM: OMAP: iommu: declare a private iommu binding struct
>> ? ARM: OMAP3: bind omap3isp_device to its iommu device
>> ? iommu/omap: eliminate the public omap_find_iommu_device() method
>
> It doesn't apply cleanly, can you please rebase the code, collect the
> Acked-by's and resend?

I have collected Tony's and Laurent's Acked-by's and amended them to
the commit logs.

I have also made sure it applies cleanly to a recent 3.2, and tested
it with OMAP4/remoteproc. I couldn't test it with omap3isp though,
because I'm gated by (what looks like a) 3.2 omap3isp regression (see
http://www.spinics.net/lists/linux-omap/msg60510.html). I did test it
of course with omap3isp and 3.1, so I don't expect any omap3isp/iommu
issues with these patches. Anyway, when the omap3isp issue will be
resolved I'll make sure there isn't any iommu regression.

Please pull, thanks a lot!

The following changes since commit cfcfc9eca2bcbd26a8e206baeb005b055dbf8e37:

  Linux 3.2-rc2 (2011-11-15 15:02:59 -0200)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/ohad/linux.git for-joerg

Ohad Ben-Cohen (3):
      ARM: OMAP: iommu: declare a private iommu binding struct
      ARM: OMAP3: bind omap3isp_device to its iommu device
      iommu/omap: eliminate the public omap_find_iommu_device() method

 arch/arm/mach-omap2/devices.c           |    7 ++++
 arch/arm/plat-omap/include/plat/iommu.h |   31 +++++++++++++++--
 arch/arm/plat-omap/include/plat/iovmm.h |   12 +++---
 drivers/iommu/omap-iommu.c              |   58 ++++++++++++++-----------------
 drivers/iommu/omap-iovmm.c              |   31 +++++++++++------
 drivers/media/video/omap3isp/isp.c      |   30 +++-------------
 drivers/media/video/omap3isp/isp.h      |    2 -
 drivers/media/video/omap3isp/ispccdc.c  |   18 +++++-----
 drivers/media/video/omap3isp/ispstat.c  |    8 ++--
 drivers/media/video/omap3isp/ispvideo.c |    4 +-
 10 files changed, 107 insertions(+), 94 deletions(-)

[RFC 0/5] OMAP groundwork for IOMMU-based DMA API

[Roedel, Joerg]

On Mon, Dec 05, 2011 at 04:21:21PM +0200, Ohad Ben-Cohen wrote:
> Hi Joerg,
> 
> On Tue, Nov 15, 2011 at 12:39 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> > On Sun, Sep 25, 2011 at 06:58:52AM -0400, Ohad Ben-Cohen wrote:
> >> Ohad Ben-Cohen (5):
> >> ? ARM: dev_archdata: add private iommu extension
> >> ? ARM: OMAP: omap_device: add a method to set iommu private archdata
> >> ? ARM: OMAP: iommu: declare a private iommu binding struct
> >> ? ARM: OMAP3: bind omap3isp_device to its iommu device
> >> ? iommu/omap: eliminate the public omap_find_iommu_device() method
> >
> > It doesn't apply cleanly, can you please rebase the code, collect the
> > Acked-by's and resend?
> 
> I have collected Tony's and Laurent's Acked-by's and amended them to
> the commit logs.
> 
> I have also made sure it applies cleanly to a recent 3.2, and tested
> it with OMAP4/remoteproc. I couldn't test it with omap3isp though,
> because I'm gated by (what looks like a) 3.2 omap3isp regression (see
> http://www.spinics.net/lists/linux-omap/msg60510.html). I did test it
> of course with omap3isp and 3.1, so I don't expect any omap3isp/iommu
> issues with these patches. Anyway, when the omap3isp issue will be
> resolved I'll make sure there isn't any iommu regression.
> 
> Please pull, thanks a lot!
> 
> The following changes since commit cfcfc9eca2bcbd26a8e206baeb005b055dbf8e37:
> 
>   Linux 3.2-rc2 (2011-11-15 15:02:59 -0200)
> 
> are available in the git repository at:
>   git://git.kernel.org/pub/scm/linux/kernel/git/ohad/linux.git for-joerg
> 
> Ohad Ben-Cohen (3):
>       ARM: OMAP: iommu: declare a private iommu binding struct
>       ARM: OMAP3: bind omap3isp_device to its iommu device
>       iommu/omap: eliminate the public omap_find_iommu_device() method
> 
>  arch/arm/mach-omap2/devices.c           |    7 ++++
>  arch/arm/plat-omap/include/plat/iommu.h |   31 +++++++++++++++--
>  arch/arm/plat-omap/include/plat/iovmm.h |   12 +++---
>  drivers/iommu/omap-iommu.c              |   58 ++++++++++++++-----------------
>  drivers/iommu/omap-iovmm.c              |   31 +++++++++++------
>  drivers/media/video/omap3isp/isp.c      |   30 +++-------------
>  drivers/media/video/omap3isp/isp.h      |    2 -
>  drivers/media/video/omap3isp/ispccdc.c  |   18 +++++-----
>  drivers/media/video/omap3isp/ispstat.c  |    8 ++--
>  drivers/media/video/omap3isp/ispvideo.c |    4 +-
>  10 files changed, 107 insertions(+), 94 deletions(-)

Pulled into arm/omap, thanks Ohad.

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632