All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman@deeprootsystems.com>
To: Omar Ramirez Luna <omar.ramirez@ti.com>
Cc: Tony Lindgren <tony@atomide.com>, Paul Walmsley <paul@pwsan.com>,
	Russell King <linux@arm.linux.org.uk>,
	Thara Gopinath <thara@ti.com>, Hari Kanigeri <h-kanigeri2@ti.com>,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] OMAP: device: make rt_va easily avaialable to drivers
Date: Thu, 09 Dec 2010 09:16:59 -0800	[thread overview]
Message-ID: <87oc8uc1dg.fsf@deeprootsystems.com> (raw)
In-Reply-To: <1291852792-3584-1-git-send-email-omar.ramirez@ti.com> (Omar Ramirez Luna's message of "Wed, 8 Dec 2010 17:59:52 -0600")

Omar Ramirez Luna <omar.ramirez@ti.com> writes:

> Patch "OMAP: hwmod/device: add omap_{device, hwmod}_get_mpu_rt_va"[1],
> introduces omap_device_get_rt_va which is meant to be called
> by drivers to retrieve the _mpu_rt_va, however this function
> receives a pointer to an omap_device; since there is no
> practical way for a driver to get this parameter without
> fiddling with pdev and container_of macro, and omap_device code
> already does this, it is better for it to handle this case.
>
> Also moved header declaration to appear in the set of
> functions to be used by drivers, as per the comment there.
>
> [1] http://marc.info/?l=linux-omap&m=127808467703366&w=2
>
> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>

Looks right, since all the other driver-accessible functions take a
platform_device pointer, not an omap_device pointer.

Acked-by: Kevin Hilman <khilman@deeprootsystems.com>

However, I thing the subject/shortlog should be clearer.  IMO, this
patch makes the API more consistent rather than making it easier.  
Also, subject prefix should be preferrably 'OMAP: omap_device:'.

Kevin

> ---
>  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;

WARNING: multiple messages have this Message-ID (diff)
From: khilman@deeprootsystems.com (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] OMAP: device: make rt_va easily avaialable to drivers
Date: Thu, 09 Dec 2010 09:16:59 -0800	[thread overview]
Message-ID: <87oc8uc1dg.fsf@deeprootsystems.com> (raw)
In-Reply-To: <1291852792-3584-1-git-send-email-omar.ramirez@ti.com> (Omar Ramirez Luna's message of "Wed, 8 Dec 2010 17:59:52 -0600")

Omar Ramirez Luna <omar.ramirez@ti.com> writes:

> Patch "OMAP: hwmod/device: add omap_{device, hwmod}_get_mpu_rt_va"[1],
> introduces omap_device_get_rt_va which is meant to be called
> by drivers to retrieve the _mpu_rt_va, however this function
> receives a pointer to an omap_device; since there is no
> practical way for a driver to get this parameter without
> fiddling with pdev and container_of macro, and omap_device code
> already does this, it is better for it to handle this case.
>
> Also moved header declaration to appear in the set of
> functions to be used by drivers, as per the comment there.
>
> [1] http://marc.info/?l=linux-omap&m=127808467703366&w=2
>
> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>

Looks right, since all the other driver-accessible functions take a
platform_device pointer, not an omap_device pointer.

Acked-by: Kevin Hilman <khilman@deeprootsystems.com>

However, I thing the subject/shortlog should be clearer.  IMO, this
patch makes the API more consistent rather than making it easier.  
Also, subject prefix should be preferrably 'OMAP: omap_device:'.

Kevin

> ---
>  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;

  reply	other threads:[~2010-12-09 17:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-08 23:59 [PATCH] OMAP: device: make rt_va easily avaialable to drivers Omar Ramirez Luna
2010-12-08 23:59 ` Omar Ramirez Luna
2010-12-09 17:16 ` Kevin Hilman [this message]
2010-12-09 17:16   ` Kevin Hilman
2010-12-10 18:47   ` Ramirez Luna, Omar
2010-12-10 18:47     ` Ramirez Luna, Omar
2010-12-09 22:15 ` Hari Kanigeri
2010-12-09 22:15   ` Hari Kanigeri

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87oc8uc1dg.fsf@deeprootsystems.com \
    --to=khilman@deeprootsystems.com \
    --cc=h-kanigeri2@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=omar.ramirez@ti.com \
    --cc=paul@pwsan.com \
    --cc=thara@ti.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.