[PATCH] OMAP: omap_device: use pdev as parameter to get rt_va

[PATCH] OMAP: omap_device: use pdev as parameter to get rt_va

[Omar Ramirez Luna]

Make the parameter received by omap_device_get_mpu_rt_va
consistent with the functions meant to be called by drivers.

Also move its header declaration to appear in the set of
functions to be used by drivers, as per the comment there.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
Acked-by: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/plat-omap/include/plat/omap_device.h |    3 +--
 arch/arm/plat-omap/omap_device.c              |    8 ++++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index 28e2d1a..1877c1a 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -80,6 +80,7 @@ struct omap_device {
 int omap_device_enable(struct platform_device *pdev);
 int omap_device_idle(struct platform_device *pdev);
 int omap_device_shutdown(struct platform_device *pdev);
+void __iomem *omap_device_get_rt_va(struct platform_device *pdev);
 
 /* Core code interface */
 
@@ -101,8 +102,6 @@ struct omap_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 int omap_device_register(struct omap_device *od);
 int omap_early_device_register(struct omap_device *od);
 
-void __iomem *omap_device_get_rt_va(struct omap_device *od);
-
 /* OMAP PM interface */
 int omap_device_align_pm_lat(struct platform_device *pdev,
 			     u32 new_wakeup_lat_limit);
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index abe933c..9d11195 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -681,7 +681,7 @@ struct powerdomain *omap_device_get_pwrdm(struct omap_device *od)
 
 /**
  * omap_device_get_mpu_rt_va - return the MPU's virtual addr for the hwmod base
- * @od: struct omap_device *
+ * @pdev: struct platform_device *
  *
  * Return the MPU's virtual address for the base of the hwmod, from
  * the ioremap() that the hwmod code does.  Only valid if there is one
@@ -690,8 +690,12 @@ struct powerdomain *omap_device_get_pwrdm(struct omap_device *od)
  * otherwise, passes along the return value from
  * omap_hwmod_get_mpu_rt_va().
  */
-void __iomem *omap_device_get_rt_va(struct omap_device *od)
+void __iomem *omap_device_get_rt_va(struct platform_device *pdev)
 {
+	struct omap_device *od;
+
+	od = _find_by_pdev(pdev);
+
 	if (od->hwmods_cnt != 1)
 		return NULL;
 
-- 
1.7.1

[PATCH] OMAP: omap_device: use pdev as parameter to get rt_va

[Cousson, Benoit]

Hi Omar,

On 12/11/2010 12:45 AM, Ramirez Luna, Omar wrote:
> Make the parameter received by omap_device_get_mpu_rt_va
> consistent with the functions meant to be called by drivers.
>
> Also move its header declaration to appear in the set of
> functions to be used by drivers, as per the comment there.

Please note that even if Paul submitted this API upon request from 
Santosh, we do not want driver to us it most of the time.

All drivers should us the generic Linux way to get physical mem resource 
and then ioremap it.

I assume that if you want to update this API, you are probably already 
using it.
Why cannot you use the generic way?

Regards,
Benoit

>
> Signed-off-by: Omar Ramirez Luna<omar.ramirez@ti.com>
> Acked-by: Kevin Hilman<khilman@deeprootsystems.com>
> ---
>   arch/arm/plat-omap/include/plat/omap_device.h |    3 +--
>   arch/arm/plat-omap/omap_device.c              |    8 ++++++--
>   2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
> index 28e2d1a..1877c1a 100644
> --- a/arch/arm/plat-omap/include/plat/omap_device.h
> +++ b/arch/arm/plat-omap/include/plat/omap_device.h
> @@ -80,6 +80,7 @@ struct omap_device {
>   int omap_device_enable(struct platform_device *pdev);
>   int omap_device_idle(struct platform_device *pdev);
>   int omap_device_shutdown(struct platform_device *pdev);
> +void __iomem *omap_device_get_rt_va(struct platform_device *pdev);
>
>   /* Core code interface */
>
> @@ -101,8 +102,6 @@ struct omap_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
>   int omap_device_register(struct omap_device *od);
>   int omap_early_device_register(struct omap_device *od);
>
> -void __iomem *omap_device_get_rt_va(struct omap_device *od);
> -
>   /* OMAP PM interface */
>   int omap_device_align_pm_lat(struct platform_device *pdev,
>   			     u32 new_wakeup_lat_limit);
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index abe933c..9d11195 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -681,7 +681,7 @@ struct powerdomain *omap_device_get_pwrdm(struct omap_device *od)
>
>   /**
>    * omap_device_get_mpu_rt_va - return the MPU's virtual addr for the hwmod base
> - * @od: struct omap_device *
> + * @pdev: struct platform_device *
>    *
>    * Return the MPU's virtual address for the base of the hwmod, from
>    * the ioremap() that the hwmod code does.  Only valid if there is one
> @@ -690,8 +690,12 @@ struct powerdomain *omap_device_get_pwrdm(struct omap_device *od)
>    * otherwise, passes along the return value from
>    * omap_hwmod_get_mpu_rt_va().
>    */
> -void __iomem *omap_device_get_rt_va(struct omap_device *od)
> +void __iomem *omap_device_get_rt_va(struct platform_device *pdev)
>   {
> +	struct omap_device *od;
> +
> +	od = _find_by_pdev(pdev);
> +
>   	if (od->hwmods_cnt != 1)
>   		return NULL;
>

[PATCH] OMAP: omap_device: use pdev as parameter to get rt_va

[Ramirez Luna, Omar]

Hi Benoit,

On Mon, Dec 13, 2010 at 2:12 AM, Cousson, Benoit <b-cousson@ti.com> wrote:
> Hi Omar,
>
> On 12/11/2010 12:45 AM, Ramirez Luna, Omar wrote:
>>
>> Make the parameter received by omap_device_get_mpu_rt_va
>> consistent with the functions meant to be called by drivers.
>>
>> Also move its header declaration to appear in the set of
>> functions to be used by drivers, as per the comment there.
>
> Please note that even if Paul submitted this API upon request from Santosh,
> we do not want driver to us it most of the time.

Oh, ok. Yes, I was under the impression that this ioremap was internal
to hwmod, and drivers should do their own one; but then I noticed that
API and since it was under the "public functions through struct
platform data", I thought it was easier to pass pdev than od.

> All drivers should us the generic Linux way to get physical mem resource and
> then ioremap it.

Then I guess this function belongs to the "public for core code" and
not for drivers along with the omap_device_get_pwrdm.

> I assume that if you want to update this API, you are probably already using
> it.
> Why cannot you use the generic way?

I can leave the generic way along with ioremap, the purpose was to use
omap_device APIs as much as possible.

Thanks,

Omar

[PATCH] OMAP: omap_device: use pdev as parameter to get rt_va

[Paul Walmsley]

Hello Omar

On Mon, 13 Dec 2010, Ramirez Luna, Omar wrote:

> On Mon, Dec 13, 2010 at 2:12 AM, Cousson, Benoit <b-cousson@ti.com> wrote:
> > On 12/11/2010 12:45 AM, Ramirez Luna, Omar wrote:
> >>
> >> Make the parameter received by omap_device_get_mpu_rt_va
> >> consistent with the functions meant to be called by drivers.
> >>
> >> Also move its header declaration to appear in the set of
> >> functions to be used by drivers, as per the comment there.
> >
> > Please note that even if Paul submitted this API upon request from Santosh,
> > we do not want driver to us it most of the time.
> 
> Oh, ok. Yes, I was under the impression that this ioremap was internal
> to hwmod, and drivers should do their own one but then I noticed that
> API and since it was under the "public functions through struct
> platform data", I thought it was easier to pass pdev than od.
> 
> > All drivers should us the generic Linux way to get physical mem resource and
> > then ioremap it.
> 
> Then I guess this function belongs to the "public for core code" and
> not for drivers along with the omap_device_get_pwrdm.
> 
> > I assume that if you want to update this API, you are probably already using
> > it.
> > Why cannot you use the generic way?
> 
> I can leave the generic way along with ioremap, the purpose was to use
> omap_device APIs as much as possible.

The above isn't quite it.  What I'd suggest you do in your code is this:

- In your driver-to-core-OMAP integration code in arch/arm/*omap*, call 
omap_hwmod_get_mpu_rt_va(), and pass that to the driver via an entry in 
the struct platform_data.

- In your driver code in drivers/*, test to see if that struct 
platform_data entry is NULL.  If it is, only then should your driver 
ioremap().  Otherwise, it should use the address from the struct 
platform_data.

This is because it's not guaranteed that ioremap() on OMAP will continue 
to reuse the current static mapping in the future.  We've heard from some 
mobile OS vendors that they are under significant address space 
constraints, so on those distributions it might make sense to only map 
devices that are actually in use, and take the additional TLB entry cost.

Does this make sense?


- Paul

[PATCH] OMAP: omap_device: use pdev as parameter to get rt_va

[Paul Walmsley]

Hello Omar

On Tue, 14 Dec 2010, Paul Walmsley wrote:

> - In your driver-to-core-OMAP integration code in arch/arm/*omap*, call 
> omap_hwmod_get_mpu_rt_va(), and pass that to the driver via an entry in 
> the struct platform_data.

One brief note here - this should have been more clear:

- In your driver-to-core-OMAP integration code in arch/arm/*omap*, call 
  omap_hwmod_get_mpu_rt_va(), and pass the virtual address that it returns 
  to the driver via an entry in the struct platform_data.


- Paul

[PATCH] OMAP: omap_device: use pdev as parameter to get rt_va

[Russell King - ARM Linux]

On Tue, Dec 14, 2010 at 05:17:28PM -0700, Paul Walmsley wrote:
> Hello Omar
> 
> On Mon, 13 Dec 2010, Ramirez Luna, Omar wrote:
> 
> > On Mon, Dec 13, 2010 at 2:12 AM, Cousson, Benoit <b-cousson@ti.com> wrote:
> > > On 12/11/2010 12:45 AM, Ramirez Luna, Omar wrote:
> > >>
> > >> Make the parameter received by omap_device_get_mpu_rt_va
> > >> consistent with the functions meant to be called by drivers.
> > >>
> > >> Also move its header declaration to appear in the set of
> > >> functions to be used by drivers, as per the comment there.
> > >
> > > Please note that even if Paul submitted this API upon request from Santosh,
> > > we do not want driver to us it most of the time.
> > 
> > Oh, ok. Yes, I was under the impression that this ioremap was internal
> > to hwmod, and drivers should do their own one but then I noticed that
> > API and since it was under the "public functions through struct
> > platform data", I thought it was easier to pass pdev than od.
> > 
> > > All drivers should us the generic Linux way to get physical mem resource and
> > > then ioremap it.
> > 
> > Then I guess this function belongs to the "public for core code" and
> > not for drivers along with the omap_device_get_pwrdm.
> > 
> > > I assume that if you want to update this API, you are probably already using
> > > it.
> > > Why cannot you use the generic way?
> > 
> > I can leave the generic way along with ioremap, the purpose was to use
> > omap_device APIs as much as possible.
> 
> The above isn't quite it.  What I'd suggest you do in your code is this:
> 
> - In your driver-to-core-OMAP integration code in arch/arm/*omap*, call 
> omap_hwmod_get_mpu_rt_va(), and pass that to the driver via an entry in 
> the struct platform_data.
> 
> - In your driver code in drivers/*, test to see if that struct 
> platform_data entry is NULL.  If it is, only then should your driver 
> ioremap().  Otherwise, it should use the address from the struct 
> platform_data.
> 
> This is because it's not guaranteed that ioremap() on OMAP will continue 
> to reuse the current static mapping in the future.  We've heard from some 
> mobile OS vendors that they are under significant address space 
> constraints, so on those distributions it might make sense to only map 
> devices that are actually in use, and take the additional TLB entry cost.
> 
> Does this make sense?

No it doesn't - this is not a valid reason to bugger about with drivers
to make them non-standard.

omap_ioremap() can be improved to retain its re-use of static mappings
by having some kind of tree structure or something like that - ioremap
is after all not a performance critical path.  There's no need to start
passing virtual addresses to drivers.

[PATCH] OMAP: omap_device: use pdev as parameter to get rt_va

[Paul Walmsley]

On Wed, 15 Dec 2010, Russell King - ARM Linux wrote:

> No it doesn't - this is not a valid reason to bugger about with drivers
> to make them non-standard.
> 
> omap_ioremap() can be improved to retain its re-use of static mappings
> by having some kind of tree structure or something like that - ioremap
> is after all not a performance critical path.  There's no need to start
> passing virtual addresses to drivers.

OK, that's the approach we'll take then, when or if that needs to happen.

Omar, please ignore my earlier post and just call ioremap() directly from 
your driver code.


- Paul