From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Omar Ramirez Luna <omar.ramirez@ti.com>,
Felipe Contreras <felipe.contreras@gmail.com>
Cc: linux-omap@vger.kernel.org
Subject: Re: [PATCH 1/2] tidspbridge: Refactor mapping find/remove operations
Date: Fri, 12 Oct 2012 23:32:02 +0200 [thread overview]
Message-ID: <32932849.mzoVLgpiLb@avalon> (raw)
In-Reply-To: <1348528386-4627-1-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Felipe, Omar,
Could I please get your feedback on this patch and 2/2 ? They try to split
08/15 in easier to review chunks.
On Tuesday 25 September 2012 01:13:05 Laurent Pinchart wrote:
> Split the remove_mapping_information() function into find_dsp_mapping()
> to locate the mapping and remove_mapping_information() to remove it.
> Rename find_containing_mapping() to find_mpu_mapping() and share the
> search code between find_dsp_mapping() and find_mpu_mapping().
>
> This prepares the driver for VM_PFNMAP support.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/staging/tidspbridge/rmgr/proc.c | 116 ++++++++++++++--------------
> 1 files changed, 59 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/staging/tidspbridge/rmgr/proc.c
> b/drivers/staging/tidspbridge/rmgr/proc.c index 7e4f12f..64b1bba 100644
> --- a/drivers/staging/tidspbridge/rmgr/proc.c
> +++ b/drivers/staging/tidspbridge/rmgr/proc.c
> @@ -145,47 +145,67 @@ static struct dmm_map_object *add_mapping_info(struct
> process_context *pr_ctxt, return map_obj;
> }
>
> -static int match_exact_map_obj(struct dmm_map_object *map_obj,
> - u32 dsp_addr, u32 size)
> +static void remove_mapping_information(struct process_context *pr_ctxt,
> + struct dmm_map_object *map_obj)
> {
> - if (map_obj->dsp_addr == dsp_addr && map_obj->size != size)
> - pr_err("%s: addr match (0x%x), size don't (0x%x != 0x%x)\n",
> - __func__, dsp_addr, map_obj->size, size);
> + if (map_obj == NULL)
> + return;
>
> - return map_obj->dsp_addr == dsp_addr &&
> - map_obj->size == size;
> + pr_debug("%s: match, deleting map info\n", __func__);
> +
> + spin_lock(&pr_ctxt->dmm_map_lock);
> + list_del(&map_obj->link);
> + spin_unlock(&pr_ctxt->dmm_map_lock);
> +
> + kfree(map_obj->dma_info.sg);
> + kfree(map_obj->pages);
> + kfree(map_obj);
> }
>
> -static void remove_mapping_information(struct process_context *pr_ctxt,
> - u32 dsp_addr, u32 size)
> +static struct dmm_map_object *
> +find_mapping(struct process_context *pr_ctxt, u32 addr, u32 size,
> + int (*match)(struct dmm_map_object *, u32, u32))
> {
> struct dmm_map_object *map_obj;
>
> - pr_debug("%s: looking for virt 0x%x size 0x%x\n", __func__,
> - dsp_addr, size);
> -
> spin_lock(&pr_ctxt->dmm_map_lock);
> list_for_each_entry(map_obj, &pr_ctxt->dmm_map_list, link) {
> - pr_debug("%s: candidate: mpu_addr 0x%x virt 0x%x size 0x%x\n",
> - __func__,
> - map_obj->mpu_addr,
> - map_obj->dsp_addr,
> - map_obj->size);
> -
> - if (match_exact_map_obj(map_obj, dsp_addr, size)) {
> - pr_debug("%s: match, deleting map info\n", __func__);
> - list_del(&map_obj->link);
> - kfree(map_obj->dma_info.sg);
> - kfree(map_obj->pages);
> - kfree(map_obj);
> + pr_debug("%s: candidate: mpu_addr 0x%x dsp_addr 0x%x size 0x%x\n",
> + __func__, map_obj->mpu_addr, map_obj->dsp_addr,
> + map_obj->size);
> +
> + if (match(map_obj, addr, size)) {
> + pr_debug("%s: match!\n", __func__);
> goto out;
> }
> - pr_debug("%s: candidate didn't match\n", __func__);
> +
> + pr_debug("%s: no match!\n", __func__);
> }
>
> - pr_err("%s: failed to find given map info\n", __func__);
> + map_obj = NULL;
> out:
> spin_unlock(&pr_ctxt->dmm_map_lock);
> + return map_obj;
> +}
> +
> +static int match_exact_map_obj(struct dmm_map_object *map_obj,
> + u32 dsp_addr, u32 size)
> +{
> + if (map_obj->dsp_addr == dsp_addr && map_obj->size != size)
> + pr_err("%s: addr match (0x%x), size don't (0x%x != 0x%x)\n",
> + __func__, dsp_addr, map_obj->size, size);
> +
> + return map_obj->dsp_addr == dsp_addr &&
> + map_obj->size == size;
> +}
> +
> +static struct dmm_map_object *
> +find_dsp_mapping(struct process_context *pr_ctxt, u32 dsp_addr, u32 size)
> +{
> + pr_debug("%s: looking for virt 0x%x size 0x%x\n", __func__,
> + dsp_addr, size);
> +
> + return find_mapping(pr_ctxt, dsp_addr, size, match_exact_map_obj);
> }
>
> static int match_containing_map_obj(struct dmm_map_object *map_obj,
> @@ -197,33 +217,13 @@ static int match_containing_map_obj(struct
> dmm_map_object *map_obj, mpu_addr + size <= map_obj_end;
> }
>
> -static struct dmm_map_object *find_containing_mapping(
> - struct process_context *pr_ctxt,
> - u32 mpu_addr, u32 size)
> +static struct dmm_map_object *
> +find_mpu_mapping(struct process_context *pr_ctxt, u32 mpu_addr, u32 size)
> {
> - struct dmm_map_object *map_obj;
> pr_debug("%s: looking for mpu_addr 0x%x size 0x%x\n", __func__,
> - mpu_addr, size);
> + mpu_addr, size);
>
> - spin_lock(&pr_ctxt->dmm_map_lock);
> - list_for_each_entry(map_obj, &pr_ctxt->dmm_map_list, link) {
> - pr_debug("%s: candidate: mpu_addr 0x%x virt 0x%x size 0x%x\n",
> - __func__,
> - map_obj->mpu_addr,
> - map_obj->dsp_addr,
> - map_obj->size);
> - if (match_containing_map_obj(map_obj, mpu_addr, size)) {
> - pr_debug("%s: match!\n", __func__);
> - goto out;
> - }
> -
> - pr_debug("%s: no match!\n", __func__);
> - }
> -
> - map_obj = NULL;
> -out:
> - spin_unlock(&pr_ctxt->dmm_map_lock);
> - return map_obj;
> + return find_mapping(pr_ctxt, mpu_addr, size, match_containing_map_obj);
> }
>
> static int find_first_page_in_cache(struct dmm_map_object *map_obj,
> @@ -755,9 +755,9 @@ int proc_begin_dma(void *hprocessor, void *pmpu_addr,
> u32 ul_size, mutex_lock(&proc_lock);
>
> /* find requested memory are in cached mapping information */
> - map_obj = find_containing_mapping(pr_ctxt, (u32) pmpu_addr, ul_size);
> + map_obj = find_mpu_mapping(pr_ctxt, (u32) pmpu_addr, ul_size);
> if (!map_obj) {
> - pr_err("%s: find_containing_mapping failed\n", __func__);
> + pr_err("%s: find_mpu_mapping failed\n", __func__);
> status = -EFAULT;
> goto no_map;
> }
> @@ -795,9 +795,9 @@ int proc_end_dma(void *hprocessor, void *pmpu_addr, u32
> ul_size, mutex_lock(&proc_lock);
>
> /* find requested memory are in cached mapping information */
> - map_obj = find_containing_mapping(pr_ctxt, (u32) pmpu_addr, ul_size);
> + map_obj = find_mpu_mapping(pr_ctxt, (u32) pmpu_addr, ul_size);
> if (!map_obj) {
> - pr_err("%s: find_containing_mapping failed\n", __func__);
> + pr_err("%s: find_mpu_mapping failed\n", __func__);
> status = -EFAULT;
> goto no_map;
> }
> @@ -1273,7 +1273,7 @@ int proc_map(void *hprocessor, void *pmpu_addr, u32
> ul_size, u32 size_align;
> int status = 0;
> struct proc_object *p_proc_object = (struct proc_object *)hprocessor;
> - struct dmm_map_object *map_obj;
> + struct dmm_map_object *map_obj = NULL;
> u32 tmp_addr = 0;
>
> #ifdef CONFIG_TIDSPBRIDGE_CACHE_LINE_CHECK
> @@ -1324,7 +1324,7 @@ int proc_map(void *hprocessor, void *pmpu_addr, u32
> ul_size, /* Mapped address = MSB of VA | LSB of PA */
> *pp_map_addr = (void *) tmp_addr;
> } else {
> - remove_mapping_information(pr_ctxt, tmp_addr, size_align);
> + remove_mapping_information(pr_ctxt, map_obj);
> dmm_un_map_memory(dmm_mgr, va_align, &size_align);
> }
> mutex_unlock(&proc_lock);
> @@ -1600,6 +1600,7 @@ int proc_un_map(void *hprocessor, void *map_addr,
> {
> int status = 0;
> struct proc_object *p_proc_object = (struct proc_object *)hprocessor;
> + struct dmm_map_object *map_obj;
> struct dmm_object *dmm_mgr;
> u32 va_align;
> u32 size_align;
> @@ -1637,7 +1638,8 @@ int proc_un_map(void *hprocessor, void *map_addr,
> * from dmm_map_list, so that mapped memory resource tracking
> * remains uptodate
> */
> - remove_mapping_information(pr_ctxt, (u32) map_addr, size_align);
> + map_obj = find_dsp_mapping(pr_ctxt, (u32) map_addr, size_align);
> + remove_mapping_information(pr_ctxt, map_obj);
>
> unmap_failed:
> mutex_unlock(&proc_lock);
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2012-10-12 21:31 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-19 12:06 [PATCH v2 00/15] tidspbridge driver MMU-related cleanups Laurent Pinchart
2012-09-19 12:06 ` [PATCH v2 01/15] tidspbridge: hw_mmu: Reorder functions to avoid forward declarations Laurent Pinchart
2012-09-19 12:06 ` [PATCH v2 02/15] tidspbridge: hw_mmu: Removed unused functions Laurent Pinchart
2012-09-19 12:06 ` [PATCH v2 03/15] tidspbridge: tiomap3430: Reorder functions to avoid forward declarations Laurent Pinchart
2012-09-19 12:06 ` [PATCH v2 04/15] tidspbridge: tiomap3430: Remove unneeded dev_context local variables Laurent Pinchart
2012-09-19 12:06 ` [PATCH v2 05/15] tidspbridge: tiomap3430: Factor out common page release code Laurent Pinchart
2012-09-19 12:06 ` [PATCH v2 06/15] tidspbridge: tiomap3430: Remove ul_ prefix Laurent Pinchart
2012-09-19 12:06 ` [PATCH v2 07/15] tidspbridge: tiomap3430: Remove unneeded local variables Laurent Pinchart
2012-09-19 12:06 ` [PATCH v2 08/15] tidspbridge: Fix VM_PFNMAP mapping Laurent Pinchart
2012-09-21 18:37 ` Felipe Contreras
2012-09-24 23:11 ` Laurent Pinchart
2012-09-24 23:13 ` [PATCH 1/2] tidspbridge: Refactor mapping find/remove operations Laurent Pinchart
2012-09-24 23:13 ` [PATCH 2/2] tidspbridge: Fix VM_PFNMAP mapping Laurent Pinchart
2012-10-12 21:32 ` Laurent Pinchart [this message]
2012-09-19 12:06 ` [PATCH v2 09/15] tidspbridge: Remove unused hw_mmu_map_attrs_t::donotlockmpupage field Laurent Pinchart
2012-09-19 12:06 ` [PATCH v2 10/15] ARM: OMAP: iommu: fix including iommu.h without IOMMU_API selected Laurent Pinchart
2012-09-19 12:06 ` [PATCH v2 11/15] arm: omap: iommu: Include required headers in iommu.h and iopgtable.h Laurent Pinchart
2012-09-19 12:07 ` [PATCH v2 12/15] tidspbridge: Use constants defined in IOMMU platform headers Laurent Pinchart
2012-09-19 12:07 ` [PATCH v2 13/15] tidspbridge: Simplify pte_update and mem_map_vmalloc functions Laurent Pinchart
2012-09-19 12:07 ` [PATCH v2 14/15] tidspbridge: Use correct types to describe physical, MPU, DSP addresses Laurent Pinchart
2012-09-19 12:07 ` [PATCH v2 15/15] tidspbridge: Replace hw_mmu_map_attrs_t structure with a prot bitfield Laurent Pinchart
2012-09-21 16:18 ` [PATCH v2 00/15] tidspbridge driver MMU-related cleanups Ramirez Luna, Omar
2012-09-24 23:15 ` Laurent Pinchart
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=32932849.mzoVLgpiLb@avalon \
--to=laurent.pinchart@ideasonboard.com \
--cc=felipe.contreras@gmail.com \
--cc=linux-omap@vger.kernel.org \
--cc=omar.ramirez@ti.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.