Re: [PATCH 01/32] arm/omap: use system_wq in mailbox

2011-01-03 Thread Kanigeri, Hari
Tejun,

On Mon, Jan 3, 2011 at 7:49 AM, Tejun Heo  wrote:
> With cmwq, there's no reason to use a separate workqueue for mailbox.
> Use the system_wq instead.  mbox->rxq->work is sync flushed in
> omap_mbox_fini() to make sure it's not running on any cpu, which makes
> sure that no mbox work is running when omap_mbox_exit() is entered.
>
> Signed-off-by: Tejun Heo 
> Cc: Tony Lindgren 
> Cc: linux-omap@vger.kernel.org
> ---
> Only compile tested.  Please feel free to take it into the subsystem
> tree or simply ack - I'll route it through the wq tree.

This was changed to dedicated work queue because of performance issues
when there is heavy mailbox traffic between the cores.

Reference:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg24240.html


>
> Thanks.
>
>  arch/arm/plat-omap/mailbox.c |   10 ++
>  1 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index d2fafb8..5bc4d7b 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -31,7 +31,6 @@
>
>  #include 
>
> -static struct workqueue_struct *mboxd;
>  static struct omap_mbox **mboxes;
>  static bool rq_full;
>
> @@ -186,7 +185,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
>        /* no more messages in the fifo. clear IRQ source. */
>        ack_mbox_irq(mbox, IRQ_RX);
>  nomem:
> -       queue_work(mboxd, &mbox->rxq->work);
> +       schedule_work(&mbox->rxq->work);
>  }
>
>  static irqreturn_t mbox_interrupt(int irq, void *p)
> @@ -291,7 +290,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
>  {
>        free_irq(mbox->irq, mbox);
>        tasklet_kill(&mbox->txq->tasklet);
> -       flush_work(&mbox->rxq->work);
> +       flush_work_sync(&mbox->rxq->work);
>        mbox_queue_free(mbox->txq);
>        mbox_queue_free(mbox->rxq);
>
> @@ -385,10 +384,6 @@ static int __init omap_mbox_init(void)
>        if (err)
>                return err;
>
> -       mboxd = create_workqueue("mboxd");
> -       if (!mboxd)
> -               return -ENOMEM;
> -
>        /* kfifo size sanity check: alignment and minimal size */
>        mbox_kfifo_size = ALIGN(mbox_kfifo_size, sizeof(mbox_msg_t));
>        mbox_kfifo_size = max_t(unsigned int, mbox_kfifo_size, 
> sizeof(mbox_msg_t));
> @@ -399,7 +394,6 @@ subsys_initcall(omap_mbox_init);
>
>  static void __exit omap_mbox_exit(void)
>  {
> -       destroy_workqueue(mboxd);
>        class_unregister(&omap_mbox_class);
>  }
>  module_exit(omap_mbox_exit);
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] staging: tidspbridge: protect dmm_map properly

2010-12-20 Thread Kanigeri, Hari
Felipe,

On Mon, Dec 20, 2010 at 11:12 AM, Felipe Contreras
 wrote:
> We need to protect not only the dmm_map list, but the individual
> map_obj's, otherwise, we might be building the scatter-gather list with
> garbage. So, use the existing proc_lock for that.
>
> I observed race conditions which caused kernel panics while running
> stress tests. This patch fixes those.
>
> Signed-off-by: Felipe Contreras 
> ---
>  drivers/staging/tidspbridge/rmgr/proc.c |   18 ++
>  1 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/tidspbridge/rmgr/proc.c 
> b/drivers/staging/tidspbridge/rmgr/proc.c
> index b47d7aa..21052e3 100644
> --- a/drivers/staging/tidspbridge/rmgr/proc.c
> +++ b/drivers/staging/tidspbridge/rmgr/proc.c
> @@ -781,12 +781,14 @@ int proc_begin_dma(void *hprocessor, void *pmpu_addr, 
> u32 ul_size,
>                                                        (u32)pmpu_addr,
>                                                        ul_size, dir);
>
> +       mutex_lock(&proc_lock);

May be you should use mutex_lock_interruptable instead of  mutex_lock.

> +
>        /* find requested memory are in cached mapping information */
>        map_obj = find_containing_mapping(pr_ctxt, (u32) pmpu_addr, ul_size);
>        if (!map_obj) {
>                pr_err("%s: find_containing_mapping failed\n", __func__);
>                status = -EFAULT;
> -               goto err_out;
> +               goto no_map;
>        }
>
>        if (memory_give_ownership(map_obj, (u32) pmpu_addr, ul_size, dir)) {
> @@ -795,6 +797,8 @@ int proc_begin_dma(void *hprocessor, void *pmpu_addr, u32 
> ul_size,
>                status = -EFAULT;
>        }
>
> +no_map:
> +       mutex_unlock(&proc_lock);
>  err_out:
>
>        return status;
> @@ -819,12 +823,14 @@ int proc_end_dma(void *hprocessor, void *pmpu_addr, u32 
> ul_size,
>                                                        (u32)pmpu_addr,
>                                                        ul_size, dir);
>
> +       mutex_lock(&proc_lock);
> +
>        /* find requested memory are in cached mapping information */
>        map_obj = find_containing_mapping(pr_ctxt, (u32) pmpu_addr, ul_size);
>        if (!map_obj) {
>                pr_err("%s: find_containing_mapping failed\n", __func__);
>                status = -EFAULT;
> -               goto err_out;
> +               goto no_map;
>        }
>
>        if (memory_regain_ownership(map_obj, (u32) pmpu_addr, ul_size, dir)) {
> @@ -834,6 +840,8 @@ int proc_end_dma(void *hprocessor, void *pmpu_addr, u32 
> ul_size,
>                goto err_out;

Mutex is not released in this case as it is released only at no_map.


>        }
>
> +no_map:
> +       mutex_unlock(&proc_lock);
>  err_out:
>        return status;
>  }
> @@ -1726,9 +1734,8 @@ int proc_un_map(void *hprocessor, void *map_addr,
>                    (p_proc_object->hbridge_context, va_align, size_align);
>        }
>
> -       mutex_unlock(&proc_lock);
>        if (status)
> -               goto func_end;
> +               goto unmap_failed;
>
>        /*
>         * A successful unmap should be followed by removal of map_obj
> @@ -1737,6 +1744,9 @@ int proc_un_map(void *hprocessor, void *map_addr,
>         */
>        remove_mapping_information(pr_ctxt, (u32) map_addr, size_align);
>
> +unmap_failed:
> +       mutex_unlock(&proc_lock);
> +
>  func_end:
>        dev_dbg(bridge, "%s: hprocessor: 0x%p map_addr: 0x%p status: 0x%x\n",
>                __func__, hprocessor, map_addr, status);
> --
> 1.7.3.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] OMAP: mailbox: use runtime pm for clk and sysc handling

2010-12-16 Thread Kanigeri, Hari
On Thu, Dec 16, 2010 at 11:01 AM, Ramirez Luna, Omar
 wrote:
> Hi,
>
> On Thu, Dec 16, 2010 at 10:32 AM, Kanigeri, Hari  wrote:
>>> @@ -130,12 +120,6 @@ static int omap2_mbox_startup(struct omap_mbox *mbox)
>>>        l = mbox_read_reg(MAILBOX_REVISION);
>>>        pr_debug("omap mailbox rev %d.%d\n", (l & 0xf0) >> 4, (l & 0x0f));
>>>
>>> -       if (cpu_is_omap44xx())
>>> -               l = OMAP4_SMARTIDLE;
>>> -       else
>>> -               l = SMARTIDLE | AUTOIDLE;
>>> -       mbox_write_reg(l, MAILBOX_SYSCONFIG);
>>> -
>>
>> The OMAP4 mailbox sysconfig register bits are laid out differently
>> from previous OMAP mailbox's. Example is smart idle bit location is
>> different from previous OMAPs. Can I know as how are you handling this
>> aspect in hwmod code ?
>
> hwmod framework provides definition for both IP models
> (omap_hwmod_sysc_type1for omap2/3 or omap_hwmod_sysc_type2 for omap4),
> these are located at arch/arm/mach-omap2/omap_hwmod_common_data.c
>

Thanks, Omar.

Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] OMAP: mailbox: use runtime pm for clk and sysc handling

2010-12-16 Thread Kanigeri, Hari
Omar,

On Thu, Dec 16, 2010 at 12:47 AM, Omar Ramirez Luna  wrote:
> Use runtime pm APIs to enable/disable mailbox clocks and
> to configure SYSC register.
>
> Based on the patch sent by Felipe Contreras:
> https://patchwork.kernel.org/patch/101662/
>
> Signed-off-by: Omar Ramirez Luna 
> ---
>  arch/arm/mach-omap2/mailbox.c |   27 +--
>  1 files changed, 5 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
> index 40ddeca..f5f72ba 100644
> --- a/arch/arm/mach-omap2/mailbox.c
> +++ b/arch/arm/mach-omap2/mailbox.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> @@ -34,12 +35,8 @@
>  #define MAILBOX_IRQ_NOTFULL(m)         (1 << (2 * (m) + 1))
>
>  /* SYSCONFIG: register bit definition */
> -#define AUTOIDLE       (1 << 0)
>  #define SOFTRESET      (1 << 1)
> -#define SMARTIDLE      (2 << 3)
>  #define OMAP4_SOFTRESET        (1 << 0)
> -#define OMAP4_NOIDLE   (1 << 2)
> -#define OMAP4_SMARTIDLE        (2 << 2)
>
>  /* SYSSTATUS: register bit definition */
>  #define RESETDONE      (1 << 0)
> @@ -70,8 +67,6 @@ struct omap_mbox2_priv {
>        unsigned long irqdisable;
>  };
>
> -static struct clk *mbox_ick_handle;
> -
>  static void omap2_mbox_enable_irq(struct omap_mbox *mbox,
>                                  omap_mbox_type_t irq);
>
> @@ -91,13 +86,8 @@ static int omap2_mbox_startup(struct omap_mbox *mbox)
>        u32 l;
>        unsigned long timeout;
>
> -       mbox_ick_handle = clk_get(NULL, "mailboxes_ick");
> -       if (IS_ERR(mbox_ick_handle)) {
> -               printk(KERN_ERR "Could not get mailboxes_ick: %ld\n",
> -                       PTR_ERR(mbox_ick_handle));
> -               return PTR_ERR(mbox_ick_handle);
> -       }
> -       clk_enable(mbox_ick_handle);
> +       pm_runtime_enable(mbox->dev->parent);
> +       pm_runtime_get_sync(mbox->dev->parent);
>
>        if (cpu_is_omap44xx()) {
>                mbox_write_reg(OMAP4_SOFTRESET, MAILBOX_SYSCONFIG);
> @@ -130,12 +120,6 @@ static int omap2_mbox_startup(struct omap_mbox *mbox)
>        l = mbox_read_reg(MAILBOX_REVISION);
>        pr_debug("omap mailbox rev %d.%d\n", (l & 0xf0) >> 4, (l & 0x0f));
>
> -       if (cpu_is_omap44xx())
> -               l = OMAP4_SMARTIDLE;
> -       else
> -               l = SMARTIDLE | AUTOIDLE;
> -       mbox_write_reg(l, MAILBOX_SYSCONFIG);
> -

The OMAP4 mailbox sysconfig register bits are laid out differently
from previous OMAP mailbox's. Example is smart idle bit location is
different from previous OMAPs. Can I know as how are you handling this
aspect in hwmod code ?


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL] OMAP: mailbox and iommu changes: for-next for v2.6.38

2010-12-15 Thread Kanigeri, Hari
Hi Tony,

The following changes since commit e8a7e48bb248a1196484d3f8afa53bded2b24e71:
 Linus Torvalds (1):
   Linux 2.6.37-rc4

are available in the git repository at:

 git://gitorious.org/iommu_mailbox/iommu_mailbox.git for_2.6.38

Felipe Contreras (1):
 OMAP: iommu: make iva2 iommu selectable

Fernando Guzman Lugo (1):
 OMAP: mailbox: change full flag per mailbox queue instead of global

Guzman Lugo, Fernando (4):
 OMAP: iovmm: no gap checking for fixed address
 OMAP: iovmm: add superpages support to fixed da address
 OMAP: iovmm: replace __iounmap with iounmap
 OMAP: iommu: create new api to set valid da range

Kanigeri, Hari (3):
 OMAP: mailbox: fix checkpatch warnings
 OMAP: mailbox: send message in process context
 OMAP: mailbox: add notification support for multiple readers

Omar Ramirez Luna (2):
 OMAP: mailbox: remove unreachable return
 OMAP: mailbox: fix detection for previously supported chips

 arch/arm/mach-omap2/mailbox.c |   19 +++--
 arch/arm/mach-omap2/omap-iommu.c  |   10 ++-
 arch/arm/plat-omap/Kconfig|3 +
 arch/arm/plat-omap/include/plat/iommu.h   |5 +
 arch/arm/plat-omap/include/plat/mailbox.h |8 +-
 arch/arm/plat-omap/iommu.c|   24 +
 arch/arm/plat-omap/iovmm.c|   81 ++
 arch/arm/plat-omap/mailbox.c  |  130 +
 8 files changed, 179 insertions(+), 101 deletions(-)

The above patch set is dependent on the following 2 Russell's patches.

ARM: io: make iounmap() a simple macro
ARM: io: simplify ioremap* and iounmap definitions
Ref: http://www.spinics.net/lists/linux-omap/msg42023.html

Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv7 0/4] omap: iovmm - fixes for iovmm module

2010-12-14 Thread Kanigeri, Hari
Fernando,

For omap patches, follow the convention

OMAP: iommu: 
OMAP: iovmm: 

Ref: http://www.spinics.net/lists/linux-omap/msg39956.html


On Tue, Dec 14, 2010 at 6:53 PM, Fernando Guzman Lugo
 wrote:
> Misc fixes found while working with iovmm module. They are
> needed in order to tidspbridge can work properly along with
> iovmm module.
>
> Version 7:
> * Change 4/4 patch base on Felipe Contreras comments about
>  having start/end in platform data and struct iommu.
>
> Version 6:
> * Rebase on Russell King branch.
>  - for details see:
>  http://marc.info/?l=linux-omap&m=129228495723001&w=2
>
> Version 5:
> * Changes in "iommu: create new api to set valid da range"
>  - Change range variables to platform data structure.
>
> Version 4:
> * Changes in "iommu: create new api to set valid da range"
>  - Validate range for fixed address.
>  - Change way of change boundaries to avoid possible overflow
>  instead of style :
>     start + bytes >= end     which start + end can overflow
>  use style:
>     end - start < bytes
>
> Version 3:
> * change patch 2 base on Felipe Contreras' comments,
>  now it uses min_t and I deleted some blank lines.
> * patch "create new api to set valid da range" is base on
>  "iovmm: IVA2 MMU range is from 0x1100 to 0x"
>  patch and on Hiroshi's comments and now it is added to
>  this set.
>
> Version 2:
> * Removed "iovmm: fixes for iovmm module" that patch was already
>  sent.
> * Modified "iovmm: fix roundup for next area and end check for the
>  last area" patch, base on Davin Cohen's comments and rename it
>  to a proper name that describes what it is doing now.
>
>
> Guzman Lugo, Fernando (4):
>  omap: iovmm - no gap checking for fixed address
>  omap: iovmm - add superpages support to fixed da address
>  omap: iovmm - replace __iounmap with iounmap
>  omap: iommu - create new api to set valid da range
>
>  arch/arm/mach-omap2/omap-iommu.c        |    8 +++
>  arch/arm/plat-omap/include/plat/iommu.h |    5 ++
>  arch/arm/plat-omap/iommu.c              |   24 +
>  arch/arm/plat-omap/iovmm.c              |   81 +-
>  4 files changed, 83 insertions(+), 35 deletions(-)
>
>



-- 
Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Inconsistent lock state caused by omap_mbox_msg_send() called from tidspbridge

2010-12-13 Thread Kanigeri, Hari
On Mon, Dec 13, 2010 at 2:49 PM, Ohad Ben-Cohen  wrote:
> On Mon, Dec 13, 2010 at 6:45 PM, Laurent Pinchart
>  >> On Sun, Dec 12, 2010 at 4:15
> PM, Ionut Nicu  wrote:
>>> > I noticed this too, but this patch should fix it:
>>> >
>>> > https://patchwork.kernel.org/patch/365292/
>>>
>>> True. And in this patch the move to spin_lock_bh() is justifiable,
>>> too, since it adds a sending path which is parallel to the mailbox
>>> tasklet.
>>
>> Is this patch set ready for inclusion in the mainline kernel, or does it need
>> to be reworked ?
>
> Better let Hari answer this one.
>

Yes, it is set to go into mainline kernel.


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/5] OMAP: mailbox: add omap_device latency information

2010-12-13 Thread Kanigeri, Hari
> -       mbox_ick_handle = clk_get(NULL, "mailboxes_ick");
> -       if (IS_ERR(mbox_ick_handle)) {
> -               printk(KERN_ERR "Could not get mailboxes_ick: %ld\n",
> -                       PTR_ERR(mbox_ick_handle));
> -               return PTR_ERR(mbox_ick_handle);
> -       }
> -       clk_enable(mbox_ick_handle);
> +       pm_runtime_get_sync(&mbox_pdev->dev);

Might be good to check the return value.


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] OMAP: mailbox and iommu changes: for-next for v2.6.38

2010-12-13 Thread Kanigeri, Hari
On Sun, Dec 12, 2010 at 5:20 AM, Russell King - ARM Linux
 wrote:
> On Mon, Dec 06, 2010 at 09:16:23AM -0600, Kanigeri, Hari wrote:
>> Ruseell,
>>
>> On Thu, Dec 2, 2010 at 9:43 AM, Guzman Lugo, Fernando
>>  wrote:
>> > On Thu, Dec 2, 2010 at 9:20 AM, Russell King - ARM Linux
>> >  wrote:
>> >> On Thu, Dec 02, 2010 at 08:50:07AM -0600, Guzman Lugo, Fernando wrote:
>> >>> On Thu, Dec 2, 2010 at 6:33 AM, Russell King - ARM Linux
>> >>>  wrote:
>> >>> > On Thu, Dec 02, 2010 at 06:07:23AM -0600, Kanigeri, Hari wrote:
>> >>> >> Hi Tony,
>> >>> >>
>> >>> >> The following changes since commit 
>> >>> >> e8a7e48bb248a1196484d3f8afa53bded2b24e71:
>> >>> >>   Linus Torvalds (1):
>> >>> >>         Linux 2.6.37-rc4
>> >>> >>
>> >>> >> are available in the git repository at:
>> >>> >>
>> >>> >>   git://gitorious.org/iommu_mailbox/iommu_mailbox.git for_2.6.38
>> >>> >>
>> >>> >> Fernando Guzman Lugo (5):
>> >>> >>       OMAP: mailbox: change full flag per mailbox queue instead of 
>> >>> >> global
>> >>> >>       omap: iovmm - no gap checking for fixed address
>> >>> >>       omap: iovmm - add superpages support to fixed da address
>> >>> >>       omap: iovmm - replace __iounmap with omap_iounmap
>> >>> >
>> >>> > This change is wrong.  Nothing should be directly referencing 
>> >>> > omap_iounmap
>> >>> > nor for that matter omap_ioremap.  Both are implementation details of 
>> >>> > the
>> >>> > standard ioremap/iounmap APIs.
>> >>> >
>> >>> > Use the official APIs rather than the implementation details behind 
>> >>> > them.
>> >>>
>> >>> if you see where the function is used, you will see that it is not
>> >>> calling the function, it is use as a parameter in unmap_vm_area(), if
>> >>> I used iounmap which is a macro there I will get a compilation error.
>> >>
>> >> Hmm, yes, because iounmap() is defined as a macro rather than iounmap.
>> >>
>> >> The solution to this is to fix iounmap and __arch_iounmap macros so
>> >> they aren't macros which take arguments.  That will then allow them
>> >> to be used in the way you desire.
>> >
>> > yes, that way it can be used in the function parameter. what is the
>> > right thing to do?
>> > 1) You send your patch and then I send the new version of the patches.
>> > 2) I make a new series of the patches with the change to iounmap and I
>> > include your patch in the series.
>> >
>>
>> Can you please suggest the approach we take here ? So, either you send
>> your suggested change as a patch and Fernando's patch will be based on
>> it, or he can take a TODO action item to patch again if you plan to
>> send this change later.
>
> Right, the patches are in my git tree, under the 'io' branch:
>
>  http://ftp.arm.linux.org.uk/pub/linux/arm/kernel/git-cur/linux-2.6-arm.git io
>
> which you can use to base stuff off of.  Make sure your tree has commits
> up to 2.6.37-rc5 before pulling to avoid grabbing MB's of pack files.
>


Thanks, Russell.

Fernando, can you please rebase your patches based on Russell's patch.
I will take care of queuing up the patches for git pull request.


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] OMAP: mailbox and iommu changes: for-next for v2.6.38

2010-12-06 Thread Kanigeri, Hari
Ruseell,

On Thu, Dec 2, 2010 at 9:43 AM, Guzman Lugo, Fernando
 wrote:
> On Thu, Dec 2, 2010 at 9:20 AM, Russell King - ARM Linux
>  wrote:
>> On Thu, Dec 02, 2010 at 08:50:07AM -0600, Guzman Lugo, Fernando wrote:
>>> On Thu, Dec 2, 2010 at 6:33 AM, Russell King - ARM Linux
>>>  wrote:
>>> > On Thu, Dec 02, 2010 at 06:07:23AM -0600, Kanigeri, Hari wrote:
>>> >> Hi Tony,
>>> >>
>>> >> The following changes since commit 
>>> >> e8a7e48bb248a1196484d3f8afa53bded2b24e71:
>>> >>   Linus Torvalds (1):
>>> >>         Linux 2.6.37-rc4
>>> >>
>>> >> are available in the git repository at:
>>> >>
>>> >>   git://gitorious.org/iommu_mailbox/iommu_mailbox.git for_2.6.38
>>> >>
>>> >> Fernando Guzman Lugo (5):
>>> >>       OMAP: mailbox: change full flag per mailbox queue instead of global
>>> >>       omap: iovmm - no gap checking for fixed address
>>> >>       omap: iovmm - add superpages support to fixed da address
>>> >>       omap: iovmm - replace __iounmap with omap_iounmap
>>> >
>>> > This change is wrong.  Nothing should be directly referencing omap_iounmap
>>> > nor for that matter omap_ioremap.  Both are implementation details of the
>>> > standard ioremap/iounmap APIs.
>>> >
>>> > Use the official APIs rather than the implementation details behind them.
>>>
>>> if you see where the function is used, you will see that it is not
>>> calling the function, it is use as a parameter in unmap_vm_area(), if
>>> I used iounmap which is a macro there I will get a compilation error.
>>
>> Hmm, yes, because iounmap() is defined as a macro rather than iounmap.
>>
>> The solution to this is to fix iounmap and __arch_iounmap macros so
>> they aren't macros which take arguments.  That will then allow them
>> to be used in the way you desire.
>
> yes, that way it can be used in the function parameter. what is the
> right thing to do?
> 1) You send your patch and then I send the new version of the patches.
> 2) I make a new series of the patches with the change to iounmap and I
> include your patch in the series.
>

Can you please suggest the approach we take here ? So, either you send
your suggested change as a patch and Fernando's patch will be based on
it, or he can take a TODO action item to patch again if you plan to
send this change later.


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL] OMAP: mailbox and iommu changes: for-next for v2.6.38

2010-12-02 Thread Kanigeri, Hari
Hi Tony,

The following changes since commit e8a7e48bb248a1196484d3f8afa53bded2b24e71:
  Linus Torvalds (1):
Linux 2.6.37-rc4

are available in the git repository at:

  git://gitorious.org/iommu_mailbox/iommu_mailbox.git for_2.6.38

Fernando Guzman Lugo (5):
  OMAP: mailbox: change full flag per mailbox queue instead of global
  omap: iovmm - no gap checking for fixed address
  omap: iovmm - add superpages support to fixed da address
  omap: iovmm - replace __iounmap with omap_iounmap
  omap: iommu - create new api to set valid da range

Kanigeri, Hari (3):
  OMAP: mailbox: fix checkpatch warnings
  OMAP: mailbox: send message in process context
  OMAP: mailbox: add notification support for multiple readers

Omar Ramirez Luna (2):
  OMAP: mailbox: remove unreachable return
  OMAP: mailbox: fix detection for previously supported chips

 arch/arm/mach-omap2/mailbox.c |   19 +++--
 arch/arm/plat-omap/include/plat/iommu.h   |3 +
 arch/arm/plat-omap/include/plat/mailbox.h |8 +-
 arch/arm/plat-omap/iommu.c|   33 +++
 arch/arm/plat-omap/iovmm.c|   84 +++
 arch/arm/plat-omap/mailbox.c  |  130 +
 6 files changed, 177 insertions(+), 100 deletions(-)

Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 1/8] TILER-DMM: DMM-PAT driver for TI TILER

2010-12-01 Thread Kanigeri, Hari
On Wed, Dec 1, 2010 at 8:10 PM, Kanigeri, Hari  wrote:
> On Wed, Dec 1, 2010 at 12:04 AM, Varadarajan, Charulatha  wrote:
>> David,
>
>>
>>> +               if (!oh)
>>> +                       goto error;
>>> +
>>> +               data->base = oh->_mpu_rt_va;
>>
>> not required. Make use of platform_get APIs in probe to extract the
>> base, dma and irq info using pdev.
>
> Not sure about using platform_get APIs. I think one has to use
> omap_hwmod_get_mpu_rt_va to get the address, which internally returns
> oh-<_mpu_rt_va.
small correction... omap_device_get_rt_va and not omap_hwmod_get_mpu_rt_va.

Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2 1/8] TILER-DMM: DMM-PAT driver for TI TILER

2010-12-01 Thread Kanigeri, Hari
On Wed, Dec 1, 2010 at 12:04 AM, Varadarajan, Charulatha  wrote:
> David,

>
>> +               if (!oh)
>> +                       goto error;
>> +
>> +               data->base = oh->_mpu_rt_va;
>
> not required. Make use of platform_get APIs in probe to extract the
> base, dma and irq info using pdev.

Not sure about using platform_get APIs. I think one has to use
omap_hwmod_get_mpu_rt_va to get the address, which internally returns
oh-<_mpu_rt_va.
omap_hwmod_get_mpu_rt_va was added so that the individual drivers can
avoid doing the ioremap.
check the discussion regarding this.
http://www.spinics.net/lists/linux-omap/msg33048.html


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] OMAP: mailbox: add notification support for multiple readers

2010-11-24 Thread Kanigeri, Hari
On Wed, Nov 24, 2010 at 7:12 AM, Varadarajan, Charulatha  wrote:
> On Wed, Nov 24, 2010 at 18:39, Kanigeri, Hari  wrote:
>> On Tue, Nov 23, 2010 at 11:29 PM, Varadarajan, Charulatha  
>> wrote:
>>> On Wed, Nov 24, 2010 at 02:56, Hari Kanigeri  wrote:
>>>> In the current mailbox driver, the mailbox internal pointer for
>>>> callback can be directly manipulated by the Users, so a second
>>>> User can easily corrupt the first user's callback pointer.
>>>> The initial effort to correct this issue can be referred here:
>>>> https://patchwork.kernel.org/patch/107520/
>>>>
>>>> Along with fixing the above stated issue, this patch  adds the
>>>> flexibility option to register notifications from
>>>> multiple readers to the events received on a mailbox instance.
>>>> The discussion regarding this can be referred here.
>>>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg30671.html
>>>>
>>>> Signed-off-by: Hari Kanigeri 
>>>> Signed-off-by: Fernando Guzman Lugo 
>>>> ---
>>>>  arch/arm/plat-omap/include/plat/mailbox.h |    7 +-
>>>>  arch/arm/plat-omap/mailbox.c              |  104 
>>>> -
>>>>  2 files changed, 62 insertions(+), 49 deletions(-)
>>>>
>>>
>>> <>
>>>
>>>> @@ -363,10 +372,13 @@ int omap_mbox_register(struct device *parent, struct 
>>>> omap_mbox **list)
>>>>                        ret = PTR_ERR(mbox->dev);
>>>>                        goto err_out;
>>>>                }
>>>> +
>>>
>>> Do this change in the patch where the below code was added.
>>
>> I am sorry I didn't get you. you mean BLOCKING_INIT_NOTIFIER_HEAD line
>> ? yes, it is needed for this patch.
>
> No. I was saying that adding a new line could be done in the patch where this
> piece of code (below 4 lines) was introduced. Sorry if that was not clear :-(

Got it. extra line can be removed.

Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 4/5] OMAP: mailbox: send message in process context

2010-11-24 Thread Kanigeri, Hari
On Tue, Nov 23, 2010 at 11:26 PM, Varadarajan, Charulatha  wrote:
> On Wed, Nov 24, 2010 at 02:56, Hari Kanigeri  wrote:
>> Schedule the Tasklet to send only when mailbox fifo is full and there are
>> pending messages in kifo, else send the message directly in the Process
>
> Typo -> kifo

Thanks :). My bad I I think overlooked Felipe's comment on my previous patch.

Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] OMAP: mailbox: add notification support for multiple readers

2010-11-24 Thread Kanigeri, Hari
On Tue, Nov 23, 2010 at 11:29 PM, Varadarajan, Charulatha  wrote:
> On Wed, Nov 24, 2010 at 02:56, Hari Kanigeri  wrote:
>> In the current mailbox driver, the mailbox internal pointer for
>> callback can be directly manipulated by the Users, so a second
>> User can easily corrupt the first user's callback pointer.
>> The initial effort to correct this issue can be referred here:
>> https://patchwork.kernel.org/patch/107520/
>>
>> Along with fixing the above stated issue, this patch  adds the
>> flexibility option to register notifications from
>> multiple readers to the events received on a mailbox instance.
>> The discussion regarding this can be referred here.
>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg30671.html
>>
>> Signed-off-by: Hari Kanigeri 
>> Signed-off-by: Fernando Guzman Lugo 
>> ---
>>  arch/arm/plat-omap/include/plat/mailbox.h |    7 +-
>>  arch/arm/plat-omap/mailbox.c              |  104 
>> -
>>  2 files changed, 62 insertions(+), 49 deletions(-)
>>
>
> <>
>
>> @@ -363,10 +372,13 @@ int omap_mbox_register(struct device *parent, struct 
>> omap_mbox **list)
>>                        ret = PTR_ERR(mbox->dev);
>>                        goto err_out;
>>                }
>> +
>
> Do this change in the patch where the below code was added.

I am sorry I didn't get you. you mean BLOCKING_INIT_NOTIFIER_HEAD line
? yes, it is needed for this patch.
>
>>                if (cpu_is_omap44xx())
>>                        mbox->rev = OMAP_MBOX_IP_VERSION_2;
>>                else
>>                        mbox->rev = OMAP_MBOX_IP_LEGACY;
>> +
>> +               BLOCKING_INIT_NOTIFIER_HEAD(&mbox->notifier);
>>        }
>>        return 0;
>>
>> --
>> 1.7.0
>>
>



-- 
Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 2/5] OMAP: mailbox: fix rx interrupt disable in omap4

2010-11-24 Thread Kanigeri, Hari
On Wed, Nov 24, 2010 at 2:50 AM, Varadarajan, Charulatha  wrote:
> On Wed, Nov 24, 2010 at 13:52, Felipe Balbi  wrote:
>> On Wed, Nov 24, 2010 at 10:46:04AM +0530, Varadarajan, Charulatha wrote:

 diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
 index 48e161c..a1c6bd9 100644
 --- a/arch/arm/plat-omap/mailbox.c
 +++ b/arch/arm/plat-omap/mailbox.c
 @@ -358,6 +358,10 @@ int omap_mbox_register(struct device *parent, struct
 omap_mbox **list)
                        ret = PTR_ERR(mbox->dev);
                        goto err_out;
                }
 +               if (cpu_is_omap44xx())
>>>
>>> Do not use cpu_is* checks in plat-omap/*
>>
>> see the previous thread.
>
> Referring to [1], I do not find why cpu_is* checks is used in plat-omap and
> why it can't be avoided.
>
> In [1], it was suggested to create the pdata field that will be
> populated at init
> time using the cpu_is* check. But in this version, I am finding that
> this is done
> in plat-omap. This can be handled in mach-omap layer itself and can be passed
> as a pdata field which can be extracted during probe.
>
> [1] https://patchwork.kernel.org/patch/337131/
>

Here are these  reasons why I did this way.

- The function omap_mbox_register is called only once during probe
time, and I thought it was ok to call cpu check once during probe
time. Since each mbox instance needs to be aware of the rev field this
was the right place to add since this function iterates through the
list during probe time. There are already calls to cpu_is_omap44xx in
mailbox probe function.

- platform data is not present for mailbox module. I could add it for
revision sake but I would prefer not to do that since this will be a
throw away code once the hwmod infrastructure is ready (note: mailbox
hwmod patches are under review), and amount of mailbox driver rework
might be considerable.

- I could wait till the hwmod patches are ready to include this
change, but don't want to put the dependency with hwmod since this is
a critical fix and want to make it available in the mainline kernel.

Please let me know what you suggest.

Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 1/5] OMAP: mailbox: change full flag per mailbox queue instead of global

2010-11-24 Thread Kanigeri, Hari
On Tue, Nov 23, 2010 at 11:21 PM, Varadarajan, Charulatha  wrote:
> On Wed, Nov 24, 2010 at 02:56, Hari Kanigeri  wrote:
>> From: Fernando Guzman Lugo 
>>
>> The variable rq_full flag is a global variable, so if there are multiple
>> mailbox users there will be conflicts. Now there is a full flag per
>> mailbox queue.
>>
>> Reported-by: Ohad Ben-Cohen 
>> Signed-off-by: Fernando Guzman Lugo 
>> Signed-off-by: Hari Kanigeri 
>> ---
>>  arch/arm/plat-omap/include/plat/mailbox.h |    1 +
>>  arch/arm/plat-omap/mailbox.c              |    9 +++--
>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/include/plat/mailbox.h 
>> b/arch/arm/plat-omap/include/plat/mailbox.h
>> index 9976565..13f2ef3 100644
>> --- a/arch/arm/plat-omap/include/plat/mailbox.h
>> +++ b/arch/arm/plat-omap/include/plat/mailbox.h
>> @@ -48,6 +48,7 @@ struct omap_mbox_queue {
>>        struct tasklet_struct   tasklet;
>>        int     (*callback)(void *);
>>        struct omap_mbox        *mbox;
>> +       bool full;
>>  };
>>
>>  struct omap_mbox {
>> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>> index d2fafb8..48e161c 100644
>> --- a/arch/arm/plat-omap/mailbox.c
>> +++ b/arch/arm/plat-omap/mailbox.c
>> @@ -33,7 +33,6 @@
>>
>>  static struct workqueue_struct *mboxd;
>>  static struct omap_mbox **mboxes;
>> -static bool rq_full;
>>
>>  static int mbox_configured;
>>  static DEFINE_MUTEX(mbox_configured_lock);
>> @@ -148,6 +147,12 @@ static void mbox_rx_work(struct work_struct *work)
>>
>>                if (mq->callback)
>>                        mq->callback((void *)msg);
>> +               spin_lock_irq(&mq->lock);
>> +               if (mq->full) {
>> +                       mq->full = false;
>> +                       omap_mbox_enable_irq(mq->mbox, IRQ_RX);
>> +               }
>> +               spin_unlock_irq(&mq->lock);
>>        }
>>  }
>>
>> @@ -170,7 +175,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
>>        while (!mbox_fifo_empty(mbox)) {
>>                if (unlikely(kfifo_avail(&mq->fifo) < sizeof(msg))) {
>>                        omap_mbox_disable_irq(mbox, IRQ_RX);
>> -                       rq_full = true;
>> +                       mq->full = true;
>
> Should this also be inside spin_lock_irq?
>

Not needed. this is already run from interrupt context.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: DSP Bridge video decode of above VGA videos

2010-11-22 Thread Kanigeri, Hari
James,

On Mon, Nov 22, 2010 at 7:14 AM, James Adams  wrote:
> Hi All,
>
> We are running Android on a custom Gumstix Overo Water platform. We
> have ported the 25.12 Zoom2 release to do this.
>
> It is almost working except that when trying to decode H264 videos
> with resolution above 640x480 (either horizontally or vertically, e.g.
> 640*544) the DSP module appears to crash. Decoding anything at VGA or
> below is working fine.
>
> DSP bridge is configured with 0x60 memory (but larger doesn't seem
> to help).  Is anyone aware of other configuration options that we
> should be checking (e.g. in DSP bridge, OMAP video drivers, or
> android?) that might cause large video decodes to fail?

Can you please post any error/failure logs ?


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4

2010-11-22 Thread Kanigeri, Hari
Felipe,

On Mon, Nov 22, 2010 at 5:51 AM, Felipe Balbi  wrote:
> Hi,
>
> On Mon, Nov 22, 2010 at 05:46:53AM -0600, Kanigeri, Hari wrote:
>>>
>>> From the dmtimer stuff, it looks like the driver defines the version
>>
>> types, which hwmod uses to define the rev field.
>>
>> /* timer ip constants */
>> #define OMAP_TIMER_IP_LEGACY                    0x1
>> #define OMAP_TIMER_IP_VERSION_2                 0x2
>
> in that case, it's the same as if you pass in a flag via platform_data.
> You might as well call it rev, then the diff when converting to hwmod
> will be smaller maybe :-p

I agree with you, the changes should be minimal converting to hwmod.


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4

2010-11-22 Thread Kanigeri, Hari
On Mon, Nov 22, 2010 at 4:08 AM, Felipe Balbi  wrote:
> On Fri, Nov 19, 2010 at 03:50:02PM +0100, Cousson, Benoit wrote:
>>
>> Most of the time, we do not want to use the IP revision because it is
>> un-accurate and does not reflect the change we'd like to track.
>> For example some time a minor change in the RTL that will not impact the
>> SW at all might trigger a change in the IP revision, whereas on the other
>> hand a major bug fix that will impact the SW is not capture in the IP
>> revision... yeah, that's bad, but this can happen.
>>
>> That's why we are relying on a rev field in the hwmod.
>
> But then, what's inside this rev field ? Is it some internal revision of
> hwmod or do you read from the hw ?
>

>From the dmtimer stuff, it looks like the driver defines the version
types, which hwmod uses to define the rev field.

/* timer ip constants */
#define OMAP_TIMER_IP_LEGACY0x1
#define OMAP_TIMER_IP_VERSION_2 0x2


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/5] OMAP: mailbox: add notification support for multiple readers

2010-11-20 Thread Kanigeri, Hari
Felipe,

On Sat, Nov 20, 2010 at 5:31 AM, Felipe Balbi  wrote:
> Hi Hari,
>
> On Fri, 2010-11-19 at 22:01 -0600, Kanigeri, Hari wrote:
>> Of course :), profiling was done before releasing this code and no
>> difference observed with or without blocking notifier. All the OMAP4
>
> would you share some numbers ?

Sure. On an average, 135 us for round trip message from A9 userspace
Process to a thread running on M3 that handles this message.
The test is with the entire IPC stack on both ends.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/5] OMAP: mailbox: add notification support for multiple readers

2010-11-19 Thread Kanigeri, Hari
Felipe,

On Fri, Nov 19, 2010 at 5:07 PM, Felipe Balbi  wrote:
> Hi Hari,
>
> On Fri, 2010-11-19 at 08:44 -0600, Kanigeri, Hari wrote:
>> Not really :). Please let me know if if I am wrong, what you
>> addressing is getting the confirmation that a message is sent and what
>> I am addressing with the patch is that a response is received from
>> M3/DSP.
>
> You got it wrong. My proposal addresses the same what you say, but in a
> different and, IMO, better fashion. There's no need to add a blocking
> notifier which you can't be sure when that'll be scheduled.

hmm, it is called in the work queue context after a mailbox interrupt
is received. Blocking notifier just calls all the registered
callbacks, and in this case only one callback is registered so it is
same as today where a function pointer in mailbox is set to callback
function.

> Have you
> measured possible worst case scenario of this patch ? What's the latency
> added by the blocking notifier ?

Of course :), profiling was done before releasing this code and no
difference observed with or without blocking notifier. All the OMAP4
use cases are exercising this code. Just curious , are you doubting
the blocking notifier mechanism ?

> Imagine user cpu is highly busy, and it
> takes a long time to call the blocking notifier, is that acceptable ?

The mailbox rx interrupt schedules the RX work queue, from where the
blocking Notifier is called to notify the interested clients of the
response.
In previous implementation, the work queue was calling the callback
that was registered by setting the mailbox's internal function
pointer, and the only change I did is to change the function pointer
to Notifier call so that the users don't muck with the driver's
internal pointer. Latency wise there is no difference between calling
the function directly or through blocking Notifier, since blocking
notifier mechanism just calls the registered callbacks.

>
>> > Then you kick the transfers which will:
>> >
>> > request = list_first_entry(mbox->req_list);
>> > setup_correct_registers();
>> > enable_irq();
>> > kick_transfer();
>>
>> Writing to the mailbox fifo delivers the message to other side, and if
>> the fifo is full the messages are queued up in mbox kfifo, which are
>> then deliverd in the order they are received.
>
> isn't that the same as what I suggested ? Messages are queued in the
> ordered they are received and sent to BIOS at the same order.
>
This is already handled with the Kfifo implementation in the mailbox driver.

>> I don't see a use case where the senders need to know that their
>> message is actually written to mbox fifo, if there is one we can look
>> into it.
>
> I never said there's such usecase :-)

I agree :), was just asking to get the confirmation since we are miles apart ;)

>
>> That's not true. Even if BIOS doesn't respond to a request, you could
>> still keep sending the messages.
>
> but then when do you consider a message "completed" ? When it gets sent
> or when you receive a response from BIOS ?

The application after exercising the mailbox put,  for him the message
is sent out, and will/will not wait on the asynchronous response from
M3/DSP, which could happen at any later time. And in some cases, he
might not even expect a response. But as I mentioned earlier this kind
of intelligence whether to wait/not wait will be in the IPC driver and
not in the low level driver such as mailbox.

Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/5] OMAP: mailbox: add notification support for multiple readers

2010-11-19 Thread Kanigeri, Hari
Felipe,


On Fri, Nov 19, 2010 at 8:25 AM, Felipe Balbi  wrote:
> Hi, (at home already),
>
> On Fri, 2010-11-19 at 07:57 -0600, Kanigeri, Hari wrote:
>> > think of it as FIFO. So the first completed message is the first in your
>> > list of requests. Ain't that easy ? :-)
>>
>> Actually it isn't that easy ;) because there is no guarantee that the
>> completed message is the first one in the list of requests.
>> Responses could be asynchronous and we cannot depend on serialized
>> handling of requests on BIOS...even in some cases you might not even
>> get a response for particular request in which case you have to handle
>> that in IPC module.
>
> you did not get what I said then.

Not really :). Please let me know if if I am wrong, what you
addressing is getting the confirmation that a message is sent and what
I am addressing with the patch is that a response is received from
M3/DSP.

>
> Then you kick the transfers which will:
>
> request = list_first_entry(mbox->req_list);
> setup_correct_registers();
> enable_irq();
> kick_transfer();

Writing to the mailbox fifo delivers the message to other side, and if
the fifo is full the messages are queued up in mbox kfifo, which are
then deliverd in the order they are received.
I don't see a use case where the senders need to know that their
message is actually written to mbox fifo, if there is one we can look
into it.
You enable TX irq only when the mbox fifo is full.

>
> return;
>
> then on IRQ handler, when the message is completed you do:
>
> request = list_first_entry(mbox->req_list);
> list_del(request->list);
>
> /* unlock */
> request->complete();
> /* lock again */
>
> you will always one message at a time, so you know that when you get the
> IRQ, the first message on your list is what was just completed.
>
>> eg: Process A sends a message to M3 core to kick off some translation
>> in some codec, the thread handling this request running on BIOS would
>> kick off the translation and wait for the completion before sending
>> the response. In the mean time BIOS will be handling other requests
>> coming to it, and shouldn't be blocked until the completion of first
>> request.
>
> why you're involving BIOS here ? mailbox should not have to know what's
> on the other side. All it needs to know is how to get the message to the
> other side, no matter _what_ it is.
>
> So until BIOS sends the response, you would not start another transfer
> on mailbox neither you would any IRQ, right ?

That's not true. Even if BIOS doesn't respond to a request, you could
still keep sending the messages.


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4

2010-11-19 Thread Kanigeri, Hari
Felipe,

On Fri, Nov 19, 2010 at 2:32 AM, Felipe Balbi  wrote:
> On Thu, Nov 18, 2010 at 06:07:40PM -0600, Kanigeri, Hari wrote:
>>
>> Benoit,
>>
>> On Thu, Nov 18, 2010 at 5:28 PM, Cousson, Benoit  wrote:
>>>
>>> On 11/18/2010 8:15 PM, Hari Kanigeri wrote:
>>>>
>>>> disabling rx interrupt on omap4 is different than its pre-decessors.
>>>> The bit in OMAP4_MAILBOX_IRQENABLE_CLR should be set to disable the
>>>> interrupts instead of clearing the bit.
>>>>
>>>> Signed-off-by: Hari Kanigeri
>>>> ---
>>>>  arch/arm/mach-omap2/mailbox.c |    5 -
>>>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/mailbox.c
>>>> b/arch/arm/mach-omap2/mailbox.c
>>>> index 42dbfa4..82b5ced 100644
>>>> --- a/arch/arm/mach-omap2/mailbox.c
>>>> +++ b/arch/arm/mach-omap2/mailbox.c
>>>> @@ -195,7 +195,10 @@ static void omap2_mbox_disable_irq(struct omap_mbox
>>>> *mbox,
>>>>        struct omap_mbox2_priv *p = (struct omap_mbox2_priv *)mbox->priv;
>>>>        u32 l, bit = (irq == IRQ_TX) ? p->notfull_bit : p->newmsg_bit;
>>>>        l = mbox_read_reg(p->irqdisable);
>>>> -       l&= ~bit;
>>>> +       if (cpu_is_omap44xx())
>>>
>>> Since it is not omap version specific but IP version specific, you should
>>> not use cpu_is_ to do that. Moreover cpu_is calls should be used during
>>> init
>>> only.
>>> You can use the rev field in hwmod_class in order to detect the IP
>>> version.
>>> Smartreflex series for 3630 is already using that kind of mechanism.
>>> You will have to copy that revision information into pdata struct and
>>> then
>>> use that here.
>>
>> I see your point, but since mailbox hwmod patches from Omar are still
>> under review I didn't find any other option than to enable this
>> This is critical functionality that I want to include in and not wait
>> till the hwmod patches are accepted.
>> Please let me know if there is any other way of approaching this problem ?
>
> how about you read the IP revision yourself during probe ? Or pass in a
> flag like I said on the other email ?
>

I like your proposal of reading the IP revision in probe. I will send
a revised patch for this.

Benoit, is this ok until we move to hwmod implemenation ?


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/5] OMAP: mailbox: add notification support for multiple readers

2010-11-19 Thread Kanigeri, Hari
Felipe,

On Fri, Nov 19, 2010 at 6:53 AM, Felipe Balbi  wrote:
> Hi Hari,
>
> On Fri, Nov 19, 2010 at 06:29:39AM -0600, Kanigeri, Hari wrote:
>>
>> How do you know that a response is received for a particular sender ?
>
> think of it as FIFO. So the first completed message is the first in your
> list of requests. Ain't that easy ? :-)

Actually it isn't that easy ;) because there is no guarantee that the
completed message is the first one in the list of requests.
Responses could be asynchronous and we cannot depend on serialized
handling of requests on BIOS...even in some cases you might not even
get a response for particular request in which case you have to handle
that in IPC module.

eg: Process A sends a message to M3 core to kick off some translation
in some codec, the thread handling this request running on BIOS would
kick off the translation and wait for the completion before sending
the response. In the mean time BIOS will be handling other requests
coming to it, and shouldn't be blocked until the completion of first
request.


>
> Still it doesn't the fact that currently you allow for that. There are
> malware writers everywhere. But hey, it would be cool to have mailbox
> appear on securityfocus.com :-p
>

Agree about malware writers. Add PM interfaces as well to the list as
one can misuse them as well ;)


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/5] OMAP: mailbox: add notification support for multiple readers

2010-11-19 Thread Kanigeri, Hari
Felipe,

>
>> I think handling of per-message callback should be handled at one
>> level above mailbox, like in IPC modules such as dspbridge,
>> syslink..etc.
>
> then you'll have duplication of functionality :-)
>
yes in mailbox :). One solution doesn't fit all and this should be
handled at IPC driver.

>> Mailbox module should be free of any protocols and should take care of
>
> it's not a protocol that I'm proposing. It's just one way to give
> mailbox users a more async API.
>
>> just writing to the mailbox fifo and delivering the message to the
>> interested listener(s).
>
> exactly. Only to the interested listener. With your patch, you'll be
> delivering the message to all listeners and listeners have to check if
> that particular message belongs to them.
>
>>> What will happen is that every user will have to check if every message
>>> belongs to them based on the notifications.
>>>
>>> Imagine a hypothetical situation where you have 100 users each with 100
>>> messages. You will have 100 * 100 (10.000)
>>> "does-this-message-belongs-to-me" checks.
>>>
>>
>> Ideally one shouldn't design their IPC this way. They should have a
>> IPC driver and the knowledge of notification to multiple users should
>> be routed from there.
>
> only if the message is interesting for N listeners. But if each
> listeners will have its own message towards the other side of the link,
> then mailbox should only give the result to one listener.
>
>> What my patch is addressing is ensure that the users of mailbox don't
>> mess with the mailbox's internal pointer, and also provide the
>> flexibility of multiple listeners. Designers of IPC should make their
>> own judgment whether to use the flexibility option based on their use
>> cases.
> but with your patch, you are delivering the same message to all
> listeners. You give the oportunity even for some attacks. I could fiddle
> with other listeners' message just by attaching a notifier_block and
> checking what's the content of every message.

IMHO, an kernel API have a scope to be misused if the Users misuse it,
and I think the users of the Kernel APIs should know what they are
doing before using an API.

>
>>> Rather than doing it this way, I would, as the easiest path, add a
>>> "callback" parameter to omap_mbox_request_send() and then, internally,
>>> allocate a e.g. struct omap_mbox_request and add that to a list of
>>> pending
>>> messages. Something like:
>>>
>>> struct omap_mbox_request {
>>>        struct omap_mbox        *mbox;
>>>        int                     (*complete)(void *context);
>>>        void                    *context;
>>>        void                    *buf;
>>>        unsigned                len;
>>>        struct list_head        list;
>>> };
>>>
>>> [...]
>>>
>>> int omap_mbox_request_send(struct omap_mbox *mbox, mbox_msg_t msg, int
>>>                (*complete)(void *context), void *context, gfp_t flags)
>>> {
>>>        struct omap_mbox_queue  *mq = mbox->txq;
>>>        struct omap_mbox_request        *req;
>>>        int                     ret = 0;
>>>        int                     len;
>>>
>>>        req = kzalloc(sizeof(*req), flags);
>>>        if (!req) {
>>>                [...]
>>>        }
>>>
>>>        memcpy(req->buf, &msg, sizeof(msg));
>>>        req->len = sizeof(msg));
>>>        req->mbox = msg;
>>>        req->complete = complete;
>>>        req->context = context;
>>>        INIT_LIST_HEAD(&req->list);
>>>
>>>        list_add_tail(&req->list, &mq->req_list);
>>>
>>>        /* kick the tasklet here */
>>>
>>>        return 0;
>>> }
>>>
>>> then on your tasklet, you simply iterate over the list and start sending
>>> one by one and calling callback when it completes. You would be giving

How do you know that a response is received for a particular sender ?
By reading mailbox payload or by reading some shared memory ? I think
this itself would constitute building up protocol in mailbox driver.

>>> your users a more asynchronous API and you wouldn't need this notifier
>>> which, IMO, isn't a good solution at all.
>>>
>>
>> Your ideas are good. But this type of intelligence is already inbuilt
>> in IPC drivers such as dspbridge and syslink. And I think mailbox
>> driver should be free of protocols.
>
> Like I said before, this is not a protocol. And if both dspbridge and
> syslink have the same kind of thing, don't you think it's a duplication?
> Don't you, also, think common features should be done at framework
> levels ? Since we don't have an IPC framework, we consider the mailbox
> driver a framework for using the OMAP mailbox, then the API I proposed
> is just one way to address the problem you described. There's nothing
> like a protocol here.

I fully agree with you about common framework, and the proposed
solution for the common IPC framework is Syslink. This was discussed
during the recent LPC meeting to come up with a common IPC framework
that is currently missing in Kernel.
I can share with you the details if you are i

Re: [PATCH v3 3/5] OMAP: mailbox: fix checkpatch warnings

2010-11-19 Thread Kanigeri, Hari
Felipe,

On Fri, Nov 19, 2010 at 2:33 AM, Felipe Balbi  wrote:
> Hi,
>
> On Thu, Nov 18, 2010 at 01:15:40PM -0600, Hari Kanigeri wrote:
>>
>> Fix the checkpatch warnings observed in mailbox module
>
> you should put which warnings are those here :-)
>

Sure, will add some descriptions in next patch revision.


Thank you,
Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/5] OMAP: mailbox: add notification support for multiple readers

2010-11-19 Thread Kanigeri, Hari
Felipe,

On Fri, Nov 19, 2010 at 2:50 AM, Felipe Balbi  wrote:
> Hi,
>
> On Thu, Nov 18, 2010 at 01:15:42PM -0600, Hari Kanigeri wrote:
>>
>> In the current mailbox driver, the mailbox internal pointer for
>> callback can be directly manipulated by the Users, so a second
>> User can easily corrupt the first user's callback pointer.
>> The initial effort to correct this issue can be referred here:
>> https://patchwork.kernel.org/patch/107520/
>>
>> Along with fixing the above stated issue, this patch  adds the
>> flexibility option to register notifications from
>> multiple readers to the events received on a mailbox instance.
>> The discussion regarding this can be referred here.
>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg30671.html
>>
>> Signed-off-by: Hari Kanigeri 
>> Signed-off-by: Fernando Guzman Lugo 
>
> Personally, I don't like this patch. I think it's much better to have a
> per-message callback then a "global" notification which all users will
> listen to.
>

Thanks for your comments :).
I think handling of per-message callback should be handled at one
level above mailbox, like in IPC modules such as dspbridge,
syslink..etc.
Mailbox module should be free of any protocols and should take care of
just writing to the mailbox fifo and delivering the message to the
interested listener(s).


> What will happen is that every user will have to check if every message
> belongs to them based on the notifications.
>
> Imagine a hypothetical situation where you have 100 users each with 100
> messages. You will have 100 * 100 (10.000)
> "does-this-message-belongs-to-me" checks.
>

Ideally one shouldn't design their IPC this way. They should have a
IPC driver and the knowledge of notification to multiple users should
be routed from there.

What my patch is addressing is ensure that the users of mailbox don't
mess with the mailbox's internal pointer, and also provide the
flexibility of multiple listeners. Designers of IPC should make their
own judgment whether to use the flexibility option based on their use
cases.

> Rather than doing it this way, I would, as the easiest path, add a
> "callback" parameter to omap_mbox_request_send() and then, internally,
> allocate a e.g. struct omap_mbox_request and add that to a list of pending
> messages. Something like:
>
> struct omap_mbox_request {
>        struct omap_mbox        *mbox;
>        int                     (*complete)(void *context);
>        void                    *context;
>        void                    *buf;
>        unsigned                len;
>        struct list_head        list;
> };
>
> [...]
>
> int omap_mbox_request_send(struct omap_mbox *mbox, mbox_msg_t msg, int
>                (*complete)(void *context), void *context, gfp_t flags)
> {
>        struct omap_mbox_queue  *mq = mbox->txq;
>        struct omap_mbox_request        *req;
>        int                     ret = 0;
>        int                     len;
>
>        req = kzalloc(sizeof(*req), flags);
>        if (!req) {
>                [...]
>        }
>
>        memcpy(req->buf, &msg, sizeof(msg));
>        req->len = sizeof(msg));
>        req->mbox = msg;
>        req->complete = complete;
>        req->context = context;
>        INIT_LIST_HEAD(&req->list);
>
>        list_add_tail(&req->list, &mq->req_list);
>
>        /* kick the tasklet here */
>
>        return 0;
> }
>
> then on your tasklet, you simply iterate over the list and start sending
> one by one and calling callback when it completes. You would be giving
> your users a more asynchronous API and you wouldn't need this notifier
> which, IMO, isn't a good solution at all.
>

Your ideas are good. But this type of intelligence is already inbuilt
in IPC drivers such as dspbridge and syslink. And I think mailbox
driver should be free of protocols.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/5] OMAP: mailbox: fix rx interrupt disable in omap4

2010-11-18 Thread Kanigeri, Hari
Benoit,

On Thu, Nov 18, 2010 at 5:28 PM, Cousson, Benoit  wrote:
> On 11/18/2010 8:15 PM, Hari Kanigeri wrote:
>>
>> disabling rx interrupt on omap4 is different than its pre-decessors.
>> The bit in OMAP4_MAILBOX_IRQENABLE_CLR should be set to disable the
>> interrupts instead of clearing the bit.
>>
>> Signed-off-by: Hari Kanigeri
>> ---
>>  arch/arm/mach-omap2/mailbox.c |    5 -
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
>> index 42dbfa4..82b5ced 100644
>> --- a/arch/arm/mach-omap2/mailbox.c
>> +++ b/arch/arm/mach-omap2/mailbox.c
>> @@ -195,7 +195,10 @@ static void omap2_mbox_disable_irq(struct omap_mbox
>> *mbox,
>>        struct omap_mbox2_priv *p = (struct omap_mbox2_priv *)mbox->priv;
>>        u32 l, bit = (irq == IRQ_TX) ? p->notfull_bit : p->newmsg_bit;
>>        l = mbox_read_reg(p->irqdisable);
>> -       l&= ~bit;
>> +       if (cpu_is_omap44xx())
>
> Since it is not omap version specific but IP version specific, you should
> not use cpu_is_ to do that. Moreover cpu_is calls should be used during init
> only.
> You can use the rev field in hwmod_class in order to detect the IP version.
> Smartreflex series for 3630 is already using that kind of mechanism.
> You will have to copy that revision information into pdata struct and then
> use that here.

I see your point, but since mailbox hwmod patches from Omar are still
under review I didn't find any other option than to enable this
This is critical functionality that I want to include in and not wait
till the hwmod patches are accepted.
Please let me know if there is any other way of approaching this problem ?

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] OMAP4: clocks: add dummy clock for mailbox

2010-11-18 Thread Kanigeri, Hari
Benoit,

>
> OK, that one is easy but... I will still have to update the script to
> generate it, so:

Sure, thank you.

Best regards,
Hari Kanigeri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/3] omap: add hwspinlock device

2010-10-21 Thread Kanigeri, Hari
Mugdha,

> > >>
> > >> This does not require any smart IPC and it will allow us to get
> > rid of
> > >> the omap_hwspinlock_request_specific() API and its early-callers
> > >> requirement.
> > >
> > > Yes, that would indeed simplify things.
> >
> > Balaji, Nishant, are you OK with this ?
> >
> The problem with this approach is that the i2c driver would have to
> sync up on the shared memory location that it uses to share the
> information of the spinlock ID. What location would this be? How would
> the i2c driver on the m3 know about this location? Does this mean
> hard-coding of the shared memory address? If yes, then there would be
> an impact to users if they wanted to change their memory map. Any kind
> of hard-coding of this sort can be painful in such cases, especially
> if it happens in multiple drivers. On the other hand, hard-coding
> (reserving) spinlock IDs is a much safer option and does not impact
> anything else.
> 

We can avoid the hard-coding of shared memory location if I2C Driver maps using 
iommu some dynamic memory for shared memory location to M3 virtual address and 
shares this information to I2c driver counter-part on M3 using the IPC call. 
But this might not be trivial and this would be against what Ohad mentioned 
about not requiring any smart IPC :).
I prefer hard-coding of spinlock id to keep things simple.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 05/11] omap3: Remove non-existent config option

2010-09-27 Thread Kanigeri, Hari
Felipe,

> -Original Message-
> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
> ow...@vger.kernel.org] On Behalf Of Felipe Contreras
> Sent: Monday, September 27, 2010 10:56 AM
> To: Marathe, Yogesh
> Cc: Premi, Sanjeev; Tony Lindgren; linux-arm-ker...@lists.infradead.org;
> linux-omap@vger.kernel.org
> Subject: Re: [PATCH 05/11] omap3: Remove non-existent config option
> 
> On Mon, Sep 27, 2010 at 2:02 PM, Marathe, Yogesh 
> wrote:
> >> When you merge iommu support, then either you enable
> >> CONFIG_MPU_BRIDGE_IOMMU unconditionally, or you apply this patch, but
> >> this patch alone will only break things.
> >
> > Any other driver which does not depend on bridge and interested in using
> iommu should get the handle when iommu_get("iva2") is called.
> 
> That's a hypothetical driver, right? The only driver that would ever
> be interested in the "iva2" iommu is tidspbridge, and this patch would
> brake it.
> 
> > It is not happening in original case. I think there should not be
> restrictions on other drivers to define un-related compile time define if
> they just want to use iommu driver. I feel the implementation that is
> breaking due to removal of this define should be fixed.
> 
> I couldn't parse that correctly. However, what's wrong with the
> proposal? Let's think about CONFIG_MPU_BRIDGE_IOMMU when the iommu
> patches come.
> 

Yogesh is coming from dsplink requirement to use iommu. I see his comment as 
why Bridge requirement should be imposed on other IPCs that need iommu.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: Query on clock naming conventions in clockxxxx_data.c

2010-09-24 Thread Kanigeri, Hari
Archit,

> -Original Message-
> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
> ow...@vger.kernel.org] On Behalf Of Taneja, Archit
> Sent: Friday, September 24, 2010 7:41 AM
> To: Nayak, Rajendra
> Cc: linux-omap@vger.kernel.org; Guruswamy, Senthilvadivu
> Subject: Query on clock naming conventions in clock_data.c
> 
> Hi,
> 
> I had a couple of queries regarding the clock structures in the
> clock_data.c files:
> 
> -I have seen that the name of the structure itself explains the name of
> the clock as
> given in the TRM.
> -The member "name" also tries to mimic the clock name.
> 
> The drivers get the clock struct using the "name" member in the clk_get()
> api. Is it
> okay if we can change the "name" member to a more generic string. So that
> the driver
> code stays more generic?
> 
> For example, the SYS_CLK which comes into DSS is called "dss2_alwon_fck"
> on omap3 and
> "dss_sys_clk" on omap4. This will make our driver need to have cpu_is_omap
> checks while
> calling clk_get().
> 

I guess migrating to hwmod should resolve the issue you mentioned.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v3 1/4] omap4 hsmmc: Adding card detect support for MMC1

2010-09-22 Thread Kanigeri, Hari
Kishore,

> +int twl6030_mmc_card_detect(struct device *dev, int slot)
> +{
> + int ret = -EIO;
> + u8 read_reg = 0;
> + struct platform_device *pdev = to_platform_device(dev);
> +
> + switch (pdev->id) {
> + case 0:
> + /*
> +  * BIT0 of REG_MMC_CTRL
> +  * 0 - Card not present ,1 - Card present
> +  */
> + ret = twl_i2c_read_u8(TWL6030_MODULE_ID0, &read_reg,
> + TWL6030_MMCCTRL);
> + if (ret >= 0)
> + ret = read_reg & STS_MMC;
> + break;

nitpick: may be you don't need a switch statement for only one case.

> + default:
> + pr_err("Unkown MMC controller %d in %s\n", pdev->id,
> __func__);


Thank you,
Best regards,
Hari


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/4 v2] omap: hwspinlock: Created driver for OMAP hardware spinlock.

2010-09-22 Thread Kanigeri, Hari
Tony,

Thanks for your comments.

> * Que, Simon  [100811 17:22]:
> > Created driver for OMAP hardware spinlock.
> >
> >  - Reserved spinlocks for internal use
> >  - Dynamic allocation of unreserved locks
> >  - Lock, unlock, and trylock functions, with or without disabling
> irqs/preempt
> >  - Registered as a platform device driver
> 
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/hwspinlocks.c
> > @@ -0,0 +1,65 @@
> > +int __init hwspinlocks_init(void)
> > +{
> > +   int retval = 0;
> > +
> > +   struct omap_hwmod *oh;
> > +   const char *oh_name, *pdev_name;
> > +
> > +   oh_name = "spinlock";
> > +   oh = omap_hwmod_lookup(oh_name);
> > +   if (WARN_ON(oh == NULL))
> > +   return -EINVAL;
> > +
> > +   pdev_name = "hwspinlock";
> > +
> > +   /* Pass data to device initialization */
> > +   omap_device_build(pdev_name, 0, oh, NULL, 0,
> omap_spinlock_latency,
> > +   ARRAY_SIZE(omap_spinlock_latency),
> false);
> > +
> > +   return retval;
> > +}
> > +postcore_initcall(hwspinlocks_init);
> 
> Is this initcall safe to run on all omaps or do you need some
> extra checks for it?

It is not since hwspinlock is not even present in pre-omap4. Do you suggest 
adding a check for " if (cpu_is_omap44xx())" in the init function ?


> 
> > --- /dev/null
> > +++ b/arch/arm/plat-omap/hwspinlock.c
> > @@ -0,0 +1,353 @@
> > +EXPORT_SYMBOL(hwspinlock_lock);
> > +EXPORT_SYMBOL(hwspinlock_trylock);
> > +EXPORT_SYMBOL(hwspinlock_unlock);
> > +EXPORT_SYMBOL(hwspinlock_request);
> > +EXPORT_SYMBOL(hwspinlock_request_specific);
> > +EXPORT_SYMBOL(hwspinlock_free);
> 
> Do we really want to export these functions? I think we're better
> off implementing low-level functions in the platform init code that
> are passed to the drivers in the platform_data.
> 
> This way the drivers stay generic, and we don't add yet another
> non-standard layer that will get misused all over the drivers.
> 
> If you really want to export these functions to the drivers,
> then all this code should live somewherew under drivers as well.

I agree. Will make this change in next revision.

> 
Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [RFC v.4] omap: hwspinlock: Added hwspinlock driver

2010-09-22 Thread Kanigeri, Hari
Ohad,

Sorry for the late response, was away from linux-omap mailing list last few 
days.
Please see my response.

> 
> It would probably make more sense to call pm_runtime_get_sync during
> hwspinlock_request{_specific}, and then call pm_runtime_put during
> hwspinlock_free.

I agree, this looks like a good approach.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] omap:iommu-load cam register before flushing the entry

2010-08-20 Thread Kanigeri, Hari
Sergio

> -Original Message-
> From: Aguirre, Sergio
> Sent: Friday, August 20, 2010 5:09 PM
> To: Kanigeri, Hari; Hiroshi Doyu; Linux omap
> Subject: RE: [PATCH] omap:iommu-load cam register before flushing the
> entry
> 
> Hi Hari,
> 
> > -Original Message-
> > From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
> > ow...@vger.kernel.org] On Behalf Of Kanigeri, Hari
> > Sent: Friday, August 20, 2010 8:50 AM
> > To: Hiroshi Doyu; Linux omap
> > Cc: Kanigeri, Hari
> > Subject: [PATCH] omap:iommu-load cam register before flushing the entry
> >
> > The flush_iotlb_page is not loading the cam register before flushing
> > the cam entry. This causes wrong entry to be flushed out from the TLB,
> and
> > if the entry happens to be a locked TLB entry it would lead to MMU
> faults.
> >
> > The fix is to load the cam register with the address to be flushed
> before
> > flushing the TLB entry.
> 
> I'm curious... does this impact OMAP3 ISP aswell? Or is it for OMAP4 only?
> 

This is valid for OMAP3 as well.

> Regards,
> Sergio
> 
> >
> > Signed-off-by: Hari Kanigeri 
> > ---
> >  arch/arm/plat-omap/iommu.c |1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
> > index 2e603fe..c534280 100644
> > --- a/arch/arm/plat-omap/iommu.c
> > +++ b/arch/arm/plat-omap/iommu.c
> > @@ -315,6 +315,7 @@ void flush_iotlb_page(struct iommu *obj, u32 da)
> > if ((start <= da) && (da < start + bytes)) {
> > dev_dbg(obj->dev, "%s: %08x<=%08x(%x)\n",
> > __func__, start, da, bytes);
> > +   iotlb_load_cr(obj, &cr);
> > iommu_write_reg(obj, 1, MMU_FLUSH_ENTRY);
> > }
> > }
> > --
> > 1.7.0
> >
> > 

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [RFC v.4] omap: hwspinlock: Added hwspinlock driver

2010-08-11 Thread Kanigeri, Hari
Partha and Benoit,

> > +/* Attempt to acquire a spinlock once */
> > +int hwspinlock_trylock(struct hwspinlock *handle)
> > +{
> > +   int retval = 0;
> > +
> > +   if (WARN_ON(handle == NULL))
> > +   return -EINVAL;
> > +
> > +   if (WARN_ON(in_irq()))
> > +   return -EPERM;
> > +
> > +   if (pm_runtime_get(&handle->pdev->dev) < 0)
> > +   return -ENODEV;
> > +
> > +   /* Attempt to acquire the lock by reading from it */
> > +   retval = readl(handle->lock_reg);
> > +
> > +   if (retval == HWSPINLOCK_BUSY)
> > +   pm_runtime_put(&handle->pdev->dev);
> Any reason for using pm_runtime_put instead of pm_runtime_put_sync?
> 
> Using pm_runtime_gett_sync & pm_runtime_put_sync have been recommended by
> Kevin Hilman.

Actually is there a need to call pm_runtime_put_sync for hwspinlock ? Spinlocks 
are used by the co-processors and we have to ensure that the device doesn't 
enter some low power mode without the knowledge of Co-processor. I don't think 
run time PM is needed for hwspinlock.

Just doing pm_runtime_get_sync at probe time for all the spinlock instances 
should be good.

> 

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] omap3: Remove non-existent config option

2010-08-05 Thread Kanigeri, Hari
Hiroshi,
 

> -Original Message-
> From: linux-omap-ow...@vger.kernel.org 
> [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Hiroshi DOYU
> Sent: Wednesday, August 04, 2010 6:20 AM
> To: t...@atomide.com
> Cc: Marathe, Yogesh; linux-omap@vger.kernel.org; Premi, Sanjeev
> Subject: Re: [PATCH] omap3: Remove non-existent config option
> 
> From: ext Tony Lindgren 
> Subject: Re: [PATCH] omap3: Remove non-existent config option
> Date: Wed, 4 Aug 2010 13:11:47 +0200
> 
> > * Marathe, Yogesh  [100803 11:03]:
> >> ping..
> > 
> > Hiroshi ack/nak?
> 
> Nak.
> 
> http://www.spinics.net/lists/linux-omap/msg32869.html
> 
> "tidspbridge" is in staging now.

Can you please elaborate what this means ? 
Yogesh patch enables the IOMMU for BRIDGE by default and we need this as IOMMU 
is going to get use in 3430.

> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-omap" in the body of a message to 
> majord...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver

2010-08-01 Thread Kanigeri, Hari


Thank you,
Best regards,
Hari

> -Original Message-
> From: Marathe, Yogesh
> Sent: Friday, July 30, 2010 1:53 AM
> To: Kanigeri, Hari; Premi, Sanjeev; Linux Omap; Tony Lindgren
> Cc: Shilimkar, Santosh; Cousson, Benoit; Que, Simon
> Subject: RE: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver
> 
> Hari,
> 
> > -Original Message-
> > From: Kanigeri, Hari
> > Sent: Thursday, July 29, 2010 6:44 PM
> > To: Marathe, Yogesh; Premi, Sanjeev; Linux Omap; Tony Lindgren
> > Cc: Shilimkar, Santosh; Cousson, Benoit; Que, Simon
> > Subject: RE: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver
> >
> > Yogesh,
> >
> > Nice to see your email.
> >
> > > > > +/* Release a spinlock */
> > > > > +int hwspinlock_unlock(struct hwspinlock *handle)
> > > > > +{
> > > > > + if (WARN_ON(handle == NULL))
> > > > > + return -EINVAL;
> > > > > +
> > > > > + /* Release it by writing 0 to it */
> > > > > + writel(0, handle->lock_reg);
> > >
> > > [[Yogesh Marathe]] Releasing the spinlock without knowing who owns it
> is
> > > risky. There should be a check for ownership and if authenticated user
> has
> > > called this api  only then it should be released otherwise permission
> > > denied error should be returned.
> >
> > -- I think if there is another Kernel client that is trying to release
> that is not owned by
> > it then that Kernel client itself is buggy and needs to be fixed. Please
> share your
> > thoughts on how we can ensure that we can add some protection.
> 
> [Yogesh Marathe] I agree the client has to be fixed but at least the
> client should get some error message instead of crashing someone else. I
> think if hwspinlock_request() and hwspinlock_request_specific() can keep
> the owner info (say thread id) in some module state structure it is
> possible to verify against it before acquiring and releasing the lock

-- I agree with Benoit's point. We don't have to complicate this basic IP block 
by adding additional checks. 
I2C Kernel module is one of the client and I am not sure if you can rely on 
thread id for ownership. 

> 
> >
> >
> > >
> > > > > +
> > > > > + pm_runtime_put(&handle->pdev->dev);
> > > > > +
> > > > > + return 0;
> >
> > Thank you,
> > Best regards,
> > Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver

2010-07-29 Thread Kanigeri, Hari
Sanjeev,

Thanks for your comments.

> -Original Message-
> From: Premi, Sanjeev
> Sent: Wednesday, July 28, 2010 11:58 AM
> To: Kanigeri, Hari; Linux Omap; Tony Lindgren
> Cc: Shilimkar, Santosh; Cousson, Benoit; Que, Simon
> Subject: RE: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver
>
> > -Original Message-
> > From: linux-omap-ow...@vger.kernel.org
> > [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Kanigeri, Hari
> > Sent: Monday, July 19, 2010 10:20 PM
> > To: Linux Omap; Tony Lindgren
> > Cc: Shilimkar, Santosh; Cousson, Benoit; Que, Simon; Kanigeri, Hari
> > Subject: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver
> >
> > From: Simon Que 
> >
> > Created driver for OMAP hardware spinlock.  This driver supports:
> > - Reserved spinlocks for internal use
> [sp] How do we reserver the spinlocks? I didn't see any API or parameter
>  used to reserve them.
>  If this refers to hwspinlock_request_specific(), then it isn't
>  really reservation. Reservation is usually is blocks and I would
>  expect range input.

hwspinlock_request_specific() is the one. If you want range you can either call 
hwspinlock_request_specific() in a loop or we can create a new function to 
handle it. It shouldn't be difficult to add a new function to reserve range of 
spinlocks but I would rather wait till we hit that use-case.

>
>  How is this reservation made known to other processors who would
>  be attempting to use these spinlocks - in a multi processor scenario
>  e.g. OMAP4. Both processors need to be aware of this "reservation".
>

This should be handled either:
- Based on pre-agreement on what spinlock should be used by its 
counter-part on Co-Processor.
- Communicate this information using IPC call.

> > - Dynamic allocation of unreserved locks
> > - Lock, unlock, and trylock functions, with or without
> > disabling irqs/preempt
> > - Registered as a platform device driver
> >
> > The device initialization uses hwmod to configure the devices.
> > One device will be created for each IP.  It will pass
> > spinlock register offset
> > info to the driver.  The device initialization file is:
> > arch/arm/mach-omap2/hwspinlocks.c
> >
> > The driver takes in register offset info passed in device
> > initialization.
> > It uses hwmod to obtain the base address of the hardware
> > spinlock module.
> > Then it reads info from the registers.  The function
> > hwspinlock_probe()
> > initializes the array of spinlock structures, each containing
> > a spinlock
> > register address calculated from the base address and lock offsets.
> > The device driver file is:
> > arch/arm/plat-omap/hwspinlock.c
> >
> > Here's an API summary:
> > int hwspinlock_lock(struct hwspinlock *);
> > Attempt to lock a hardware spinlock.  If it is busy,
> > the function will
> > keep trying until it succeeds.  This is a blocking function.
> > int hwspinlock_trylock(struct hwspinlock *);
> > Attempt to lock a hardware spinlock.  If it is busy,
> > the function will
> > return BUSY.  If it succeeds in locking, the function
> > will return
> > ACQUIRED.  This is a non-blocking function.
> > int hwspinlock_unlock(struct hwspinlock *);
> > Unlock a hardware spinlock.
> >
> > struct hwspinlock *hwspinlock_request(void);
> > Provides for "dynamic allocation" of a hardware
> > spinlock.  It returns
> > the handle to the next available (unallocated)
> > spinlock.  If no more
> > locks are available, it returns NULL.
> > struct hwspinlock *hwspinlock_request_specific(unsigned int);
> > Provides for "static allocation" of a specific hardware
> > spinlock. This
> > allows the system to use a specific spinlock,
> > identified by an ID. If
> > the ID is invalid or if the desired lock is already
> > allocated, this
> > will return NULL.  Otherwise it returns a spinlock handle.
> > int hwspinlock_free(struct hwspinlock *);
> > Frees an allocated hardware spinlock (either reserved
> > or unreserved).
> >
> > Signed-off-by: Simon Que 
> > Signed-off-by: Hari Kanigeri 
> > ---
> >  arch/arm/mach-omap2/hwspinlocks.c|   70 ++
> >  arch/arm/plat-omap/hwspinlock.c  |  335
> > ++
> >  arch/arm/plat-omap/include/plat/hwspinlock.h |   29 +++
> >  3 files changed, 434 insertions(+), 0 deletions(-)
> >  create mode 100644 ar

RE: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver

2010-07-29 Thread Kanigeri, Hari
Yogesh,

Nice to see your email.

> > > +/* Release a spinlock */
> > > +int hwspinlock_unlock(struct hwspinlock *handle)
> > > +{
> > > + if (WARN_ON(handle == NULL))
> > > + return -EINVAL;
> > > +
> > > + /* Release it by writing 0 to it */
> > > + writel(0, handle->lock_reg);
> 
> [[Yogesh Marathe]] Releasing the spinlock without knowing who owns it is
> risky. There should be a check for ownership and if authenticated user has
> called this api  only then it should be released otherwise permission
> denied error should be returned.

-- I think if there is another Kernel client that is trying to release that is 
not owned by it then that Kernel client itself is buggy and needs to be fixed. 
Please share your thoughts on how we can ensure that we can add some protection.


> 
> > > +
> > > + pm_runtime_put(&handle->pdev->dev);
> > > +
> > > + return 0;

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 5/5] omap:hwspinlocks-ensure the order of registration

2010-07-28 Thread Kanigeri, Hari
Sanjeev,

Thanks for your comments.

> 
> > -Original Message-
> > From: linux-omap-ow...@vger.kernel.org
> > [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Kanigeri, Hari
> > Sent: Monday, July 19, 2010 10:20 PM
> > To: Linux Omap; Tony Lindgren
> > Cc: Shilimkar, Santosh; Cousson, Benoit; Que, Simon; Kanigeri, Hari
> > Subject: [PATCH 5/5] omap:hwspinlocks-ensure the order of registration
> >
> > Ensure that the hwspinlock driver is registered prior to
> > I2C driver registration since I2C is dependent on hwspinlock.
> 
> [sp] Is there another patch where this dependency is introduced?

Basically, hwspinlock driver should be registered prior to any other modules 
that uses it. I2C driver is currently using hwspinlock and we noticed an issue 
if hwspinlock wasn't registered early on.

I don't know if the I2C patch was communicated on mailing list yet. Balaji, can 
you please share your patch.

> 
> >

Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2/5] omap:hwspinlock-define HWSPINLOCK base address

2010-07-28 Thread Kanigeri, Hari
> 
> > > > Sanjeev,
> > > >
> > > > > > @@ -49,5 +49,10 @@
> > > > > >  #define OMAP44XX_MAILBOX_BASE
> > (L4_44XX_BASE + 0xF4000)
> > > > > >  #define OMAP44XX_HSUSB_OTG_BASE
> > (L4_44XX_BASE + 0xAB000)
> > > > > >
> > > > > > +#define OMAP4_MMU1_BASE0x55082000
> > > > > > +#define OMAP4_MMU2_BASE0x4A066000
> > > > >
> > > > > [sp] Are these 2 base addresses related to this patch?
> > > >
> > > > Nope. Thanks for finding this. I will update the patch.
> > >
> > > [sp] Then additional patch only to add the base address
> > won't make much
> > > sense.
> > >  You may want to combine it with appropriately with
> > another one in
> > > this
> > >  series.
> >
> > I think the define to add SPINLOCK base address is
> > independent of adding spinlock driver functionality, and I
> > don't see a reason why it shouldn't be a separate patch.
> > Example: The driver patches might take time to get
> > upstreamed, but that shouldn't stop this patch that adds the
> > missing Base address.
> 
> [sp] Without driver would this base address be of any use?
I could find you examples where there are definitions without being used.
But any case, it is a small change I will follow your suggestion.

> 
> >
> > >
> > > >
> > > > >
> > > > > ~sanjeev
> > > > >
> > > > > > +
> > > > > > +#define OMAP44XX_SPINLOCK_BASE
> > (L4_44XX_BASE + 0xF6000)
> > > > > > +
> > > > > >  #endif /* __ASM_ARCH_OMAP44XX_H */
> > > > > >
> > > > > > --
> > > > > > 1.7.0
> > > > > >
> > > > > >
> > > >
> > > > Best regards,
> > > > Hari
> > > >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2/5] omap:hwspinlock-define HWSPINLOCK base address

2010-07-28 Thread Kanigeri, Hari
Sanjeev,


> -Original Message-
> From: Premi, Sanjeev
> Sent: Wednesday, July 28, 2010 9:12 AM
> To: Kanigeri, Hari; Linux Omap; Tony Lindgren
> Cc: Shilimkar, Santosh; Cousson, Benoit; Que, Simon
> Subject: RE: [PATCH 2/5] omap:hwspinlock-define HWSPINLOCK base address
> 
> 
> 
> > -Original Message-
> > From: Kanigeri, Hari
> > Sent: Tuesday, July 27, 2010 10:20 PM
> > To: Premi, Sanjeev; Linux Omap; Tony Lindgren
> > Cc: Shilimkar, Santosh; Cousson, Benoit; Que, Simon
> > Subject: RE: [PATCH 2/5] omap:hwspinlock-define HWSPINLOCK
> > base address
> >
> > Sanjeev,
> >
> > > > @@ -49,5 +49,10 @@
> > > >  #define OMAP44XX_MAILBOX_BASE  (L4_44XX_BASE + 0xF4000)
> > > >  #define OMAP44XX_HSUSB_OTG_BASE(L4_44XX_BASE + 0xAB000)
> > > >
> > > > +#define OMAP4_MMU1_BASE0x55082000
> > > > +#define OMAP4_MMU2_BASE0x4A066000
> > >
> > > [sp] Are these 2 base addresses related to this patch?
> >
> > Nope. Thanks for finding this. I will update the patch.
> 
> [sp] Then additional patch only to add the base address won't make much
> sense.
>  You may want to combine it with appropriately with another one in
> this
>  series.

I think the define to add SPINLOCK base address is independent of adding 
spinlock driver functionality, and I don't see a reason why it shouldn't be a 
separate patch.
Example: The driver patches might take time to get upstreamed, but that 
shouldn't stop this patch that adds the missing Base address.

> 
> >
> > >
> > > ~sanjeev
> > >
> > > > +
> > > > +#define OMAP44XX_SPINLOCK_BASE (L4_44XX_BASE + 0xF6000)
> > > > +
> > > >  #endif /* __ASM_ARCH_OMAP44XX_H */
> > > >
> > > > --
> > > > 1.7.0
> > > >
> > > >
> >
> > Best regards,
> > Hari
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2/5] omap:hwspinlock-define HWSPINLOCK base address

2010-07-27 Thread Kanigeri, Hari
Sanjeev,

> > @@ -49,5 +49,10 @@
> >  #define OMAP44XX_MAILBOX_BASE  (L4_44XX_BASE + 0xF4000)
> >  #define OMAP44XX_HSUSB_OTG_BASE(L4_44XX_BASE + 0xAB000)
> >
> > +#define OMAP4_MMU1_BASE0x55082000
> > +#define OMAP4_MMU2_BASE0x4A066000
> 
> [sp] Are these 2 base addresses related to this patch?

Nope. Thanks for finding this. I will update the patch.

> 
> ~sanjeev
> 
> > +
> > +#define OMAP44XX_SPINLOCK_BASE (L4_44XX_BASE + 0xF6000)
> > +
> >  #endif /* __ASM_ARCH_OMAP44XX_H */
> >
> > --
> > 1.7.0
> >
> >

Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver

2010-07-26 Thread Kanigeri, Hari
Benoit and Santosh,

Thanks for your comments. I will get back after reviewing your comments.

Thank you,
Best regards,
Hari

> -Original Message-
> From: Cousson, Benoit
> Sent: Saturday, July 24, 2010 11:44 AM
> To: Kanigeri, Hari
> Cc: Linux Omap; Tony Lindgren; Shilimkar, Santosh; Que, Simon
> Subject: Re: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver
>
> Hi Simon and Hari,
>
> On 7/19/2010 6:50 PM, Kanigeri, Hari wrote:
> > From: Simon Que
> >
> > Created driver for OMAP hardware spinlock.  This driver supports:
> > - Reserved spinlocks for internal use
> > - Dynamic allocation of unreserved locks
> > - Lock, unlock, and trylock functions, with or without disabling
> irqs/preempt
> > - Registered as a platform device driver
> >
> > The device initialization uses hwmod to configure the devices.
> > One device will be created for each IP.  It will pass spinlock register
> offset
> > info to the driver.  The device initialization file is:
> >  arch/arm/mach-omap2/hwspinlocks.c
> >
> > The driver takes in register offset info passed in device
> initialization.
> > It uses hwmod to obtain the base address of the hardware spinlock
> module.
> > Then it reads info from the registers.  The function hwspinlock_probe()
> > initializes the array of spinlock structures, each containing a spinlock
> > register address calculated from the base address and lock offsets.
> > The device driver file is:
> >  arch/arm/plat-omap/hwspinlock.c
> >
> > Here's an API summary:
> > int hwspinlock_lock(struct hwspinlock *);
> >  Attempt to lock a hardware spinlock.  If it is busy, the
> function will
> >  keep trying until it succeeds.  This is a blocking function.
> > int hwspinlock_trylock(struct hwspinlock *);
> >  Attempt to lock a hardware spinlock.  If it is busy, the
> function will
> >  return BUSY.  If it succeeds in locking, the function will
> return
> >  ACQUIRED.  This is a non-blocking function.
> > int hwspinlock_unlock(struct hwspinlock *);
> >  Unlock a hardware spinlock.
> >
> > struct hwspinlock *hwspinlock_request(void);
> >  Provides for "dynamic allocation" of a hardware spinlock.  It
> returns
> >  the handle to the next available (unallocated) spinlock.  If no
> more
> >  locks are available, it returns NULL.
> > struct hwspinlock *hwspinlock_request_specific(unsigned int);
> >  Provides for "static allocation" of a specific hardware
> spinlock. This
> >  allows the system to use a specific spinlock, identified by an
> ID. If
> >  the ID is invalid or if the desired lock is already allocated,
> this
> >  will return NULL.  Otherwise it returns a spinlock handle.
> > int hwspinlock_free(struct hwspinlock *);
> >  Frees an allocated hardware spinlock (either reserved or
> unreserved).
> >
> > Signed-off-by: Simon Que
> > Signed-off-by: Hari Kanigeri
> > ---
> >   arch/arm/mach-omap2/hwspinlocks.c|   70 ++
> >   arch/arm/plat-omap/hwspinlock.c  |  335
> ++
> >   arch/arm/plat-omap/include/plat/hwspinlock.h |   29 +++
> >   3 files changed, 434 insertions(+), 0 deletions(-)
> >   create mode 100644 arch/arm/mach-omap2/hwspinlocks.c
> >   create mode 100644 arch/arm/plat-omap/hwspinlock.c
> >   create mode 100644 arch/arm/plat-omap/include/plat/hwspinlock.h
> >
> > diff --git a/arch/arm/mach-omap2/hwspinlocks.c b/arch/arm/mach-
> omap2/hwspinlocks.c
> > new file mode 100644
> > index 000..f0f05e1
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/hwspinlocks.c
> > @@ -0,0 +1,70 @@
> > +/*
> > + * OMAP hardware spinlock device initialization
> > + *
> > + * Copyright (C) 2010 Texas Instruments. All rights reserved.
> > + *
> > + * Contact: Simon Que
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with 

RE: [PATCH 0/5] omap:hwspinlock support-omap4

2010-07-20 Thread Kanigeri, Hari
Santosh,


> >
> > Hari Kanigeri (1):
> >   omap:hwspinlocks-ensure the order of registration
> >
> > Simon Que (4):
> >   omap:hwmod-hwspinlock-enable
> >   omap:hwspinlock-define HWSPINLOCK base address
> I think you should fold patch 1/5 , 2/5 into patch 3/5.
> At least patch 2/5 o.w git-bisect will break

Can you please explain why this would break ? 
> 

Thank you,
Best regards,
Hari

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 8/9] dspbridge: add map support for big buffers

2010-07-02 Thread Kanigeri, Hari
Fernando,

> -Original Message-
> From: Guzman Lugo, Fernando
> Sent: Friday, July 02, 2010 11:27 AM
> To: Kanigeri, Hari; linux-omap@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Cc: o...@wizery.com; hiroshi.d...@nokia.com; ameya.pala...@nokia.com;
> felipe.contre...@nokia.com
> Subject: RE: [PATCH 8/9] dspbridge: add map support for big buffers
> 
> 
> 
> Hi Hari,
> 
> > -----Original Message-
> > From: Kanigeri, Hari
> > Sent: Thursday, July 01, 2010 6:36 PM
> > To: Guzman Lugo, Fernando; linux-omap@vger.kernel.org; linux-
> > ker...@vger.kernel.org
> > Cc: o...@wizery.com; hiroshi.d...@nokia.com; ameya.pala...@nokia.com;
> > felipe.contre...@nokia.com; Guzman Lugo, Fernando
> > Subject: RE: [PATCH 8/9] dspbridge: add map support for big buffers
> >
> > Fernando,
> >
> > > - for_each_sg(sgt->sgl, sg, sgt->nents, i)
> > > - sg_set_page(sg, usr_pgs[i], PAGE_SIZE, 0);
> > > + da = iommu_vmap(mmu, da, sgt, IOVMF_ENDIAN_LITTLE |
> > > + IOVMF_ELSZ_32);
> >
> > -- iommu_vmap does the Kernel mapping to the buffers you are mapping to
> > DSP MMU. Why do you need Kernel mappings ?
> >
> > If there is no benefit in maintaining Kernel mapping I would rather call
> > iopgtable_store_entry directly to map the entries.
> 
> Where inside iommu_vmap is the mapping done?

-- The mapping is done to track down the Device mappings. But since you already 
have it in dmm.c this is kind of redundant right now, and we might see 
performance impact due to this. 

I think it might be good to transition to iovmm when we phase out dmm.c. 
Few things to take into account transitioning to iovmm approach:
- DSPBridge used to have linked list approach to track down the mapped 
entries and profiling showed it took considerable amount of traversing through 
the list. Jeff Taylor's algorithm in dmm.c helped to reduce this impact. 
- How would you manage the Device virtual pool moving to iovmm ? And 
how about the reservation ?

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 8/9] dspbridge: add map support for big buffers

2010-07-01 Thread Kanigeri, Hari
Fernando,

> - for_each_sg(sgt->sgl, sg, sgt->nents, i)
> - sg_set_page(sg, usr_pgs[i], PAGE_SIZE, 0);
> + da = iommu_vmap(mmu, da, sgt, IOVMF_ENDIAN_LITTLE |
> + IOVMF_ELSZ_32);

-- iommu_vmap does the Kernel mapping to the buffers you are mapping to DSP 
MMU. Why do you need Kernel mappings ? 

If there is no benefit in maintaining Kernel mapping I would rather call 
iopgtable_store_entry directly to map the entries. 

Thank you,
Best regards, Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCHv3 5/9] dspbridge: add mmufault support

2010-07-01 Thread Kanigeri, Hari
> > > + da = iommu_read_reg(mmu, MMU_FAULT_AD);
> > > + iommu_write_reg(mmu, 0, MMU_IRQENABLE);
> >
> > -- Isn't the MMU already enabled at this point when the function
> callback
> > is called by iommu ?
> 
> This line is actually disabling the interrupts. I am writing "0x0" in the
> MMU_IRQENABLE.

-- oops ! sorry about that. Didn't pay attention to 0x0. Yes, this should work.

Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCHv3 5/9] dspbridge: add mmufault support

2010-07-01 Thread Kanigeri, Hari
Hi Fernando,

> +int mmu_fault_isr(struct iommu *mmu)
> 
> -/*
> - *   mmu_check_if_fault ===
> - *  Check to see if MMU Fault is valid TLB miss from DSP
> - *  Note: This function is called from an ISR
> - */
> -static bool mmu_check_if_fault(struct bridge_dev_context *dev_context)
>  {
> + struct deh_mgr *dm;
> + u32 da;
> +
> + dev_get_deh_mgr(dev_get_first(), &dm);
> +
> + if (!dm)
> + return -EPERM;
> +
> + da = iommu_read_reg(mmu, MMU_FAULT_AD);
> + iommu_write_reg(mmu, 0, MMU_IRQENABLE);

-- Isn't the MMU already enabled at this point when the function callback is 
called by iommu ?

> + dm->err_info.dw_val1 = da;
> + tasklet_schedule(&dm->dpc_tasklet);

-- The iommu fault isr disables the IOMMU at the end of the fault handler, so 
by the time your tasklet is scheduled you might have the MMU in a disabled 
state. Looks to me either this requires change in iommu to remove the disable 
part or enable the MMU in the tasklet instead of doing it early in 
mmu_fault_isr.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 22/33] Removing dead MPU_{BRIDGE,TESLA}_IOMMU

2010-06-30 Thread Kanigeri, Hari

> -Original Message-
> From: Hiroshi DOYU [mailto:hiroshi.d...@nokia.com]
> Sent: Wednesday, June 30, 2010 1:24 PM
> To: sicce...@cs.fau.de; Kanigeri, Hari
> Cc: t...@atomide.com; li...@arm.linux.org.uk; linux-omap@vger.kernel.org;
> linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org;
> va...@i4.informatik.uni-erlangen.de
> Subject: Re: [PATCH 22/33] Removing dead MPU_{BRIDGE,TESLA}_IOMMU
> 
> From: ext Christoph Egger 
> Subject: [PATCH 22/33] Removing dead MPU_{BRIDGE,TESLA}_IOMMU
> Date: Wed, 30 Jun 2010 18:01:01 +0200
> 
> > MPU_{BRIDGE,TESLA}_IOMMU doesn't exist in Kconfig, therefore removing
> > all references for it from the source code.
> 
> Coming soon, I guess. Hari?

Coming soon for both Bridge and Tesla. As I am writing this email, Fernando is 
preparing the Bridge patches to migrate to iommu.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/4] omap mailbox: extend API to take a callback param

2010-06-24 Thread Kanigeri, Hari
Ohad,

> 
> On Thu, Jun 24, 2010 at 6:02 PM, Kanigeri, Hari 
> wrote:
> > Looks good.
> 
> PCMIIW: from your other mail, I now understand that on OMAP4 you call
> omap_mbox_get several times to allow several concurrent mbox senders.

I don't know what PCMIIW stands for :). omap_mbox_get can be called by several 
concurrent mbox senders (1 sender per co-processor).

It looks like the variable mbox_configured that was added previously doesn't 
cover all the issues. It covers the shutdown issue with reference counting but 
as you mentioned that it poses an issue at startup as the RX interrupt for the 
second mbox instance doesn't get enabled.


> 
> If that's the case, this new API concept is a bit broken: it should
> allow several senders to call it, but will only support a single rx
> callback (probably by ignoring NULL callbacks, but that's not really
> safe).
> 
> We have two options here:
> 
> 1. do not extend omap_mbox_get to get a callback. instead, add a new
> API that explicitely sets the callback pointer.
> 
> 2. do extend omap_mbox_get to get a callback, but then employ
> something like a notifier chain of callbacks, essentially supporting
> multiple callbacks. this will allow a more flexible API, something
> that Hiroshi also wanted.

-- Even though this might not be relevant to mbox_configured issue, this looks 
like a nice feature to add. So this means we are going to support multiple 
writers to mailbox instance?

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/4] omap mailbox: remove mbox_configured scheme

2010-06-24 Thread Kanigeri, Hari
Ohad,

> mbox_configured is global and therefore does not support
> concurrent usage of multiple mailbox instances.

-- Why do you say that it doesn't support concurrent usage of multiple mailbox 
instances ? If you take example of OMAP4, we have 2 mailbox instances, one 
talking to DSP and other to Ducati and they should be supported concurrently.

If you remove the mbox_configured variable, then the mailbox module would shut 
down once the first instance calls the omap_mbox_put function.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/4] omap mailbox: extend API to take a callback param

2010-06-24 Thread Kanigeri, Hari
Looks good.

Iommu get has similar signature, probably we could change that too to take the 
mmu fault notification function as a function argument.

Thank you,
Best regards,
Hari

> -Original Message-
> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
> ow...@vger.kernel.org] On Behalf Of Ohad Ben-Cohen
> Sent: Tuesday, June 22, 2010 7:12 PM
> To: linux-omap@vger.kernel.org
> Cc: Hiroshi Doyu; Ramirez Luna, Omar; Ohad Ben-Cohen
> Subject: [PATCH 1/4] omap mailbox: extend API to take a callback param
> 
> Let mailbox users set the callback pointer via the
> omap_mbox_get API, instead of having users directly
> manipulating the mailbox structures.
> 
> Signed-off-by: Ohad Ben-Cohen 
> ---
> I can also be contacted at < ohadb at ti dot com >
> 
>  arch/arm/plat-omap/include/plat/mailbox.h |2 +-
>  arch/arm/plat-omap/mailbox.c  |4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-
> omap/include/plat/mailbox.h
> index 9976565..0b45664 100644
> --- a/arch/arm/plat-omap/include/plat/mailbox.h
> +++ b/arch/arm/plat-omap/include/plat/mailbox.h
> @@ -62,7 +62,7 @@ struct omap_mbox {
>  int omap_mbox_msg_send(struct omap_mbox *, mbox_msg_t msg);
>  void omap_mbox_init_seq(struct omap_mbox *);
> 
> -struct omap_mbox *omap_mbox_get(const char *);
> +struct omap_mbox *omap_mbox_get(const char *name, int (*callback)(void
> *));
>  void omap_mbox_put(struct omap_mbox *);
> 
>  int omap_mbox_register(struct device *parent, struct omap_mbox **);
> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index d2fafb8..14b716d 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -305,7 +305,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
>   }
>  }
> 
> -struct omap_mbox *omap_mbox_get(const char *name)
> +struct omap_mbox *omap_mbox_get(const char *name, int (*callback)(void
> *))
>  {
>   struct omap_mbox *mbox;
>   int ret;
> @@ -324,6 +324,8 @@ struct omap_mbox *omap_mbox_get(const char *name)
>   if (ret)
>   return ERR_PTR(-ENODEV);
> 
> + mbox->rxq->callback = callback;
> +
>   return mbox;
>  }
>  EXPORT_SYMBOL(omap_mbox_get);
> --
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/5] DSPBRIDGE: Remove unused typedefs.

2010-06-21 Thread Kanigeri, Hari
> 
>  /* Macro's */
> -#define REG16(A)(*(reg_uword16 *)(A))
> -
>  #define CLEAR_BIT(reg, mask) (reg &= ~mask)
>  #define SET_BIT(reg, mask)   (reg |= mask)
> 

-- Another candidate for cleanup :)

> --
> 1.5.4.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [RFC/PATCH 1/2] omap: mailbox: initial hwmod support

2010-05-23 Thread Kanigeri, Hari
Felipe,


> -Original Message-
> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
> ow...@vger.kernel.org] On Behalf Of Felipe Contreras
> Sent: Saturday, May 22, 2010 12:25 PM
> To: linux-omap
> Cc: Hiroshi Doyu; Kevin Hilman; Paul Walmsley; Ohad Ben-Cohen; Felipe
> Contreras
> Subject: [RFC/PATCH 1/2] omap: mailbox: initial hwmod support
> 
> Only OMAP3 would work.

-- This is good start. 
I will look into adding/verifying OMAP4 support. If any one already started 
this effort please inform.
 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-
> omap2/omap_hwmod_3xxx_data.c
> index ed60840..62b8fa8 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> @@ -164,12 +164,57 @@ static struct omap_hwmod omap3xxx_mpu_hwmod = {
>   .omap_chip  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>  };
> 

-- Probably this should form a new patch.


Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] omap:hwmod-remove prm header from prm-regbits-xxxx headers

2010-05-20 Thread Kanigeri, Hari
Hi Kevin

> -Original Message-
> From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
> Sent: Thursday, May 20, 2010 12:13 PM
> To: Kanigeri, Hari
> Cc: Paul Walmsley; Linux Omap
> Subject: Re: [PATCH] omap:hwmod-remove prm header from prm-regbits-
> headers
> 
> Hari Kanigeri  writes:
> 
> > The prm-regbits-.h header files are not dependent on prm.h
> > header file.
> >
> > Signed-off-by: Hari Kanigeri 
> > ---
> >  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |1 +
> 
> This file is not in mailine, or linux-omap master.  Please
> state any dependencies in this part of the changelog.
> 

This is based on the latest omap4_next branch on dev.omapzoom.org, so most 
probably didn't make it to linux omap yet.

I came to know through Benoit that this file is auto-generated using Perl 
script, so I guess this change can be handled in the generator script.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


omap: hwmod questions

2010-05-19 Thread Kanigeri, Hari
Hi,

I have few questions regarding hwmod usage. Can any one please respond to this.

1. I see omap_device_build function doing the platform_device_register, but I 
didn't find the equivalent function that does platform_device_unregister. Is 
this expected?

2. What is the purpose of omap_device_pm_latency structure that is provided to 
omap_device_build function?

Thank you,
Best regards,
Hari

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2/2] omap: iommu-add functionality to get TLB miss interrupt

2010-05-19 Thread Kanigeri, Hari
Hi Hiroshi,


> -Original Message-
> From: Hiroshi DOYU [mailto:hiroshi.d...@nokia.com]
> Sent: Wednesday, May 19, 2010 4:28 AM
> To: Kanigeri, Hari
> Cc: linux-omap@vger.kernel.org; t...@atomide.com; Gupta, Ramesh
> Subject: Re: [PATCH 2/2] omap: iommu-add functionality to get TLB miss
> interrupt
> 
> From: "ext Kanigeri, Hari" 
> Subject: RE: [PATCH 2/2] omap: iommu-add functionality to get TLB miss
> interrupt
> Date: Tue, 18 May 2010 19:03:55 +0200
> 
> >> > diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-
> >> omap/include/plat/iommu.h
> >> > index 0752af9..52a3852 100644
> >> > --- a/arch/arm/plat-omap/include/plat/iommu.h
> >> > +++ b/arch/arm/plat-omap/include/plat/iommu.h
> >> > @@ -80,6 +80,7 @@ struct iommu_functions {
> >> >
> >> >  int (*enable)(struct iommu *obj);
> >> >  void (*disable)(struct iommu *obj);
> >> > +void (*disable_twl)(struct iommu *obj);
> >> >  u32 (*fault_isr)(struct iommu *obj, u32 *ra);
> >> >
> >> >  void (*tlb_read_cr)(struct iommu *obj, struct cr_regs *cr);
> >> > @@ -143,6 +144,7 @@ extern void iotlb_cr_to_e(struct cr_regs *cr,
> struct
> 
> Just one thought, isn't the following a little bit flexible?
> 
> - void (*disable_twl)(struct iommu *obj);
> + void (*set_twl)(struct iommu *obj, int on);

-- Probabaly if one want to toggle between enabling and disabling twl during 
run time. I agree with you about the change so that this can be flexible in 
future. I will make this change.
Thank you for your inputs.

Thank you,
Best regards,
Hari

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2/2] omap: iommu-add functionality to get TLB miss interrupt

2010-05-18 Thread Kanigeri, Hari
Hi Hiroshi,


> -Original Message-
> From: Hiroshi DOYU [mailto:hiroshi.d...@nokia.com]
> Sent: Tuesday, May 18, 2010 1:12 AM
> To: Kanigeri, Hari
> Cc: linux-omap@vger.kernel.org; t...@atomide.com; Gupta, Ramesh
> Subject: Re: [PATCH 2/2] omap: iommu-add functionality to get TLB miss
> interrupt
> 
> Hi Hari,
> 
> From: ext Hari Kanigeri 
> Subject: [PATCH 2/2] omap: iommu-add functionality to get TLB miss
> interrupt
> Date: Tue, 18 May 2010 01:12:30 +0200
> 
> > In order to enable TLB miss interrupt, the TWL should be
> > disabled. This patch provides the functionality to get the
> > MMU fault interrupt for a TLB miss in the cases where the
> > users are working with the locked TLB entries and with TWL
> > disabled.
> 
> I want to keep leave the functionality to allow the locked TLB and
> TWL enabled at the same time too. Is it still feasible with this?
> 

-- Absolutely. You can still work with locked TLB entries with TWL enabled.
My patch just provides the functionality where the users want to work only with 
locked TLB entries and with TWL disabled, and this is a valid use case. 

So basically, the user has to call this new function after calling iommu_get 
function if they want to have TWL disabled.

Reference: Check the MMU_IRQENABLE register definition in TRM for enabling TLB 
miss interrupt.

Please let me know if you need additional clarification.


> > New interface is added to disable twl and enable TLB miss
> > interrupt.
> >
> > Signed-off-by: Hari Kanigeri 
> > Signed-off-by: Ramesh Gupta 
> > ---
> >  arch/arm/mach-omap2/iommu2.c|   13 +
> >  arch/arm/plat-omap/include/plat/iommu.h |2 ++
> >  arch/arm/plat-omap/iommu.c  |   12 
> >  3 files changed, 27 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
> > index fcf4f4a..2e78cea 100644
> > --- a/arch/arm/mach-omap2/iommu2.c
> > +++ b/arch/arm/mach-omap2/iommu2.c
> > @@ -124,6 +124,18 @@ static void omap2_iommu_disable(struct iommu *obj)
> > dev_dbg(obj->dev, "%s is shutting down\n", obj->name);
> >  }
> >
> > +static void omap2_iommu_disable_twl(struct iommu *obj)
> > +{
> > +   u32 l = iommu_read_reg(obj, MMU_CNTL);
> > +
> > +   l &= ~MMU_CNTL_MASK;
> > +   l |= (MMU_CNTL_MMU_EN);
> > +   iommu_write_reg(obj, l, MMU_CNTL);
> > +
> > +   /* Enable TLB miss interrupt */
> > +   iommu_write_reg(obj, MMU_IRQ_TLB_MISS_MASK, MMU_IRQENABLE);
> > +}
> > +
> >  static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra)
> >  {
> > int i;
> > @@ -306,6 +318,7 @@ static const struct iommu_functions omap2_iommu_ops
> = {
> >
> > .enable = omap2_iommu_enable,
> > .disable= omap2_iommu_disable,
> > +   .disable_twl= omap2_iommu_disable_twl,
> > .fault_isr  = omap2_iommu_fault_isr,
> >
> > .tlb_read_cr= omap2_tlb_read_cr,
> > diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-
> omap/include/plat/iommu.h
> > index 0752af9..52a3852 100644
> > --- a/arch/arm/plat-omap/include/plat/iommu.h
> > +++ b/arch/arm/plat-omap/include/plat/iommu.h
> > @@ -80,6 +80,7 @@ struct iommu_functions {
> >
> > int (*enable)(struct iommu *obj);
> > void (*disable)(struct iommu *obj);
> > +   void (*disable_twl)(struct iommu *obj);
> > u32 (*fault_isr)(struct iommu *obj, u32 *ra);
> >
> > void (*tlb_read_cr)(struct iommu *obj, struct cr_regs *cr);
> > @@ -143,6 +144,7 @@ extern void iotlb_cr_to_e(struct cr_regs *cr, struct
> iotlb_entry *e);
> >  extern u32 iotlb_cr_to_virt(struct cr_regs *cr);
> >
> >  extern int load_iotlb_entry(struct iommu *obj, struct iotlb_entry *e);
> > +extern void iommu_disable_twl(struct iommu *obj);
> >  extern void flush_iotlb_page(struct iommu *obj, u32 da);
> >  extern void flush_iotlb_range(struct iommu *obj, u32 start, u32 end);
> >  extern void flush_iotlb_all(struct iommu *obj);
> > diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
> > index b2b3937..d64a4d8 100644
> > --- a/arch/arm/plat-omap/iommu.c
> > +++ b/arch/arm/plat-omap/iommu.c
> > @@ -370,6 +370,18 @@ void flush_iotlb_all(struct iommu *obj)
> >  }
> >  EXPORT_SYMBOL_GPL(flush_iotlb_all);
> >
> > +/**
> > + * Call this function if working with locked TLB entries and
> > + * TWL disabled
> > + */
> 
> nitpick: It may be better to follow this for global func.
> 
> Fro

RE: [PATCH 1/2] omap: iommu-update irq mask to be specific about twl

2010-05-18 Thread Kanigeri, Hari
Hi Hiroshi,

> -Original Message-
> From: Hiroshi DOYU [mailto:hiroshi.d...@nokia.com]
> Sent: Tuesday, May 18, 2010 1:12 AM
> To: Kanigeri, Hari
> Cc: linux-omap@vger.kernel.org; t...@atomide.com
> Subject: Re: [PATCH 1/2] omap: iommu-update irq mask to be specific about
> twl
> 
> > Update the irq mask so that is is clear that the MMU
>   ~~typo?
> 

-- thanks for spotting this.

> > interrupt is related to TWL fault.
> >
> > Signed-off-by: Hari Kanigeri 
> > ---
> >  arch/arm/mach-omap2/iommu2.c |   10 --
> >  1 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
> > index e82da68..fcf4f4a 100644
> > --- a/arch/arm/mach-omap2/iommu2.c
> > +++ b/arch/arm/mach-omap2/iommu2.c
> > @@ -46,7 +46,13 @@
> >  #define MMU_IRQ_TLBMISS(1 << 0)
> >  #define MMU_IRQ_MASK   \
> > (MMU_IRQ_MULTIHITFAULT | MMU_IRQ_TABLEWALKFAULT | MMU_IRQ_EMUMISS |
> \
> > -MMU_IRQ_TRANSLATIONFAULT)
> > + MMU_IRQ_TRANSLATIONFAULT | MMU_IRQ_TLBMISS)
> > +#define MMU_IRQ_TWL_MASK   \
> > +   (MMU_IRQ_MULTIHITFAULT | MMU_IRQ_TABLEWALKFAULT | MMU_IRQ_EMUMISS |
> \
> > + MMU_IRQ_TRANSLATIONFAULT)
> > +#define MMU_IRQ_TLB_MISS_MASK  \
> > +   (MMU_IRQ_MULTIHITFAULT | MMU_IRQ_TLBMISS | MMU_IRQ_EMUMISS | \
> > + MMU_IRQ_TRANSLATIONFAULT)
> 
> nitpick: The above could look better as blew?

-- I agree. I will update this in my next patch revision.

> 
> #define __MMU_IRQ_FAULT   (MMU_IRQ_MULTIHITFAULT | 
> MMU_IRQ_EMUMISS |
> MMU_IRQ_TRANSLATIONFAULT)
> #define MMU_IRQ_MASK  (__MMU_IRQ_FAULT | MMU_IRQ_TABLEWALKFAULT |
> MMU_IRQ_TLBMISS)
> #define MMU_IRQ_TWL_MASK  (__MMU_IRQ_FAULT | MMU_IRQ_TABLEWALKFAULT)
> #define MMU_IRQ_TLB_MISS_MASK (__MMU_IRQ_FAULT | MMU_IRQ_TLBMISS)
> 
> >
> >  /* MMU_CNTL */
> >  #define MMU_CNTL_SHIFT 1
> > @@ -96,7 +102,7 @@ static int omap2_iommu_enable(struct iommu *obj)
> > l |= (MMU_SYS_IDLE_SMART | MMU_SYS_AUTOIDLE);
> > iommu_write_reg(obj, l, MMU_SYSCONFIG);
> >
> > -   iommu_write_reg(obj, MMU_IRQ_MASK, MMU_IRQENABLE);
> > +   iommu_write_reg(obj, MMU_IRQ_TWL_MASK, MMU_IRQENABLE);
> > iommu_write_reg(obj, pa, MMU_TTB);
> >
> > l = iommu_read_reg(obj, MMU_CNTL);
> > --
> > 1.7.0
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v2 08/17] omap: mailbox: only compile for configured archs

2010-05-14 Thread Kanigeri, Hari
> 
> - if (cpu_is_omap3430()) {
> + if (false);

-- Is this statement needed ?

> +#if defined(CONFIG_ARCH_OMAP3430)
> + else if (cpu_is_omap3430()) {
>   list = omap3_mboxes;
> 
>   list[0]->irq = res[1].start;

Thank you,
Best regards,
Hari

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCHv2] DSPBRIDGE: Include missing info in MMU Fault debugging trace

2010-05-13 Thread Kanigeri, Hari
Ernesto

> @@ -2136,6 +2138,7 @@ dsp_status dump_dsp_stack(struct bridge_dev_context
> *bridge_context)
>   total_size = MAX_MMU_DBGBUFF;
> 
>   buffer = kzalloc(total_size, GFP_ATOMIC);
> + buffer_beg = buffer;
>   buffer_end =  buffer + total_size / 4;
> 
>   if (!buffer) {

-- Shouldn't this check be present before the above 2 statements ?

>   for (i = 0; buffer < buffer_end; i++, buffer++) {
>   if ((*buffer > 0x0100) && (node_find_addr(node_mgr,

-- Is 0x0100 MAX_MMU_DBGBUFF ? 

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/5] omap iommu: Reject unaligned physical address at setting page table entry

2010-05-07 Thread Kanigeri, Hari
Hiroshi,
 


> From: Hiroshi DOYU 
> 
> This rejects unaligned physical address at setting page table 
> entry and informs error to caller. Otherwise, a wrong address 
> may be used by IO device.
> 
> Signed-off-by: Hiroshi DOYU 
> ---
>  arch/arm/plat-omap/iommu.c |   18 ++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/iommu.c 
> b/arch/arm/plat-omap/iommu.c index 0e13766..b3591ee 100644
> --- a/arch/arm/plat-omap/iommu.c
> +++ b/arch/arm/plat-omap/iommu.c
> @@ -503,6 +503,12 @@ static int iopgd_alloc_section(struct 
> iommu *obj, u32 da, u32 pa, u32 prot)  {
>   u32 *iopgd = iopgd_offset(obj, da);
>  
> + if (pa & ~IOSECTION_MASK) {
> + dev_err(obj->dev, "%s: pa:%08x should aligned 
> on %08lx\n",
> + __func__, pa, IOSECTION_SIZE);
> + return -EINVAL;
> + }
> +

-- I think the alignment check should be present even for da address. Same 
comment for below checks.

>   *iopgd = (pa & IOSECTION_MASK) | prot | IOPGD_SECTION;
>   flush_iopgd_range(iopgd, iopgd);
>   return 0;
> @@ -513,6 +519,12 @@ static int iopgd_alloc_super(struct 
> iommu *obj, u32 da, u32 pa, u32 prot)
>   u32 *iopgd = iopgd_offset(obj, da);
>   int i;
>  
> + if (pa & ~IOSUPER_MASK) {
> + dev_err(obj->dev, "%s: pa:%08x should aligned 
> on %08lx\n",
> + __func__, pa, IOSUPER_SIZE);
> + return -EINVAL;
> + }
> +
>   for (i = 0; i < 16; i++)
>   *(iopgd + i) = (pa & IOSUPER_MASK) | prot | IOPGD_SUPER;
>   flush_iopgd_range(iopgd, iopgd + 15);
> @@ -542,6 +554,12 @@ static int iopte_alloc_large(struct 
> iommu *obj, u32 da, u32 pa, u32 prot)
>   u32 *iopte = iopte_alloc(obj, iopgd, da);
>   int i;
>  
> + if (pa & ~IOLARGE_MASK) {
> + dev_err(obj->dev, "%s: pa:%08x should aligned 
> on %08lx\n",
> + __func__, pa, IOLARGE_SIZE);
> + return -EINVAL;
> + }
> +
>   if (IS_ERR(iopte))
>   return PTR_ERR(iopte);
>  
> --
> 1.7.1.rc1
> 
> --
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [Query][omap iommu] Consulting iommu if a physical region is "mappable" before actually mapping it

2010-05-03 Thread Kanigeri, Hari
Sergio,

> 
> Can the iommu driver be "consulted" if a certain area (contiguous or not)
> can be mapped or not, before even trying to do it?
>

-- As long as there are physical pages backing the area it should be mappable 
right ? 

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [RFC/PATCH 6/8] omap: mailbox: more more stuff to omap2_mbox_init

2010-05-03 Thread Kanigeri, Hari
Felipe,

> > Small suggestion...if we are re-organizing can we make it look similar
> to how iommu is structured? This way we can maintain consistency.
> 
> I thought I did. What exactly do you have in mind?

1. What Tony mentioned in another email about using #ifdefs for the 
platforms. That will be bring close to what is in omap-iommu.c

2. Compare iommu_get with mailbox_get. iommu_get uses 
driver_find_device to get the iommu structure. I guess we can apply the same 
logic to get mailbox structure. This way we can get rid of omap_mbox_register, 
omap_mbox_unregister, and find_mboxes functions.

> Also, I noticed that since you made omap3-iommu not omap3-specific,
> perhaps it makes sense to remove the omap prefix and just name it
> 'iommu'.
>

-- You are right. It doesn't make sense to add omap prefix any more.

Thank you,
Best regards,
Hari

> -Original Message-
> From: Felipe Contreras [mailto:felipe.contre...@gmail.com]
> Sent: Monday, May 03, 2010 10:09 AM
> To: Kanigeri, Hari
> Cc: linux-omap; Tony Lindgren; Hiroshi Doyu; Ohad Ben-Cohen
> Subject: Re: [RFC/PATCH 6/8] omap: mailbox: more more stuff to
> omap2_mbox_init
> 
> On Mon, May 3, 2010 at 4:42 PM, Kanigeri, Hari  wrote:
> > Small suggestion...if we are re-organizing can we make it look similar
> to how iommu is structured? This way we can maintain consistency.
> 
> I thought I did. What exactly do you have in mind?
> 
> Also, I noticed that since you made omap3-iommu not omap3-specific,
> perhaps it makes sense to remove the omap prefix and just name it
> 'iommu'.
> 
> Cheers.
> 
> --
> Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [RFC/PATCH 6/8] omap: mailbox: more more stuff to omap2_mbox_init

2010-05-03 Thread Kanigeri, Hari
Felipe,

Small suggestion...if we are re-organizing can we make it look similar to how 
iommu is structured? This way we can maintain consistency. 


Thank you,
Best regards,
Hari

> -Original Message-
> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
> ow...@vger.kernel.org] On Behalf Of Felipe Contreras
> Sent: Sunday, May 02, 2010 7:03 PM
> To: linux-omap
> Cc: Tony Lindgren; Hiroshi Doyu; Ohad Ben-Cohen; Felipe Contreras
> Subject: [RFC/PATCH 6/8] omap: mailbox: more more stuff to omap2_mbox_init
> 
> Will be needed to split the platform_driver.
> 
> Signed-off-by: Felipe Contreras 
> ---
>  arch/arm/mach-omap2/mailbox.c |   49 ++--
> 
>  1 files changed, 22 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
> index e9a803c..474c1e7 100644
> --- a/arch/arm/mach-omap2/mailbox.c
> +++ b/arch/arm/mach-omap2/mailbox.c
> @@ -418,31 +418,6 @@ static int __devinit omap2_mbox_probe(struct
> platform_device *pdev)
> 
>   res = pdev->resource;
> 
> - mbox_base = ioremap(res->start, resource_size(res));
> - if (!mbox_base)
> - return -ENOMEM;
> -
> - if (cpu_is_omap3430()) {
> - list.num = ARRAY_SIZE(omap3_mboxes);
> - list.mbox = omap3_mboxes;
> -
> - list.mbox[0]->irq = res[1].start;
> - }
> - else if (cpu_is_omap2420()) {
> - list.num = ARRAY_SIZE(omap2_mboxes);
> - list.mbox = omap2_mboxes;
> -
> - list.mbox[0]->irq = res[1].start;
> - list.mbox[1]->irq = res[2].start;
> - }
> - else if (cpu_is_omap44xx()) {
> - list.num = ARRAY_SIZE(omap4_mboxes);
> - list.mbox = omap4_mboxes;
> -
> - list.mbox[0]->irq = res[1].start;
> - list.mbox[1]->irq = res[1].start;
> - }
> -
>   for (i = 0; i < list.num; i++) {
>   struct omap_mbox *mbox = list.mbox[i];
>   ret = omap_mbox_register(&pdev->dev, mbox);
> @@ -454,7 +429,6 @@ static int __devinit omap2_mbox_probe(struct
> platform_device *pdev)
>  err_out:
>   while (i--)
>   omap_mbox_unregister(list.mbox[i]);
> - iounmap(mbox_base);
>   return ret;
>  }
> 
> @@ -465,7 +439,6 @@ static int __devexit omap2_mbox_remove(struct
> platform_device *pdev)
>   for (i = 0; i < list.num; i++)
>   omap_mbox_unregister(list.mbox[i]);
> 
> - iounmap(mbox_base);
>   return 0;
>  }
> 
> @@ -483,18 +456,33 @@ static int __init omap2_mbox_init(void)
>   struct platform_device *pdev;
>   struct resource *res;
>   unsigned num;
> + struct omap_mbox_list list;
> 
>   if (cpu_is_omap3430()) {
>   res = omap3_mbox_resources;
>   num = ARRAY_SIZE(omap3_mbox_resources);
> + list.num = ARRAY_SIZE(omap3_mboxes);
> + list.mbox = omap3_mboxes;
> +
> + list.mbox[0]->irq = res[1].start;
>   }
>   else if (cpu_is_omap2420()) {
>   res = omap2_mbox_resources;
>   num = ARRAY_SIZE(omap2_mbox_resources);
> + list.num = ARRAY_SIZE(omap2_mboxes);
> + list.mbox = omap2_mboxes;
> +
> + list.mbox[0]->irq = res[1].start;
> + list.mbox[1]->irq = res[2].start;
>   }
>   else if (cpu_is_omap44xx()) {
>   res = omap4_mbox_resources;
>   num = ARRAY_SIZE(omap4_mbox_resources);
> + list.num = ARRAY_SIZE(omap4_mboxes);
> + list.mbox = omap4_mboxes;
> +
> + list.mbox[0]->irq = res[1].start;
> + list.mbox[1]->irq = res[1].start;
>   }
>   else {
>   pr_err("%s: platform not supported\n", __func__);
> @@ -515,6 +503,12 @@ static int __init omap2_mbox_init(void)
>   if (err)
>   goto err_out;
> 
> + mbox_base = ioremap(res[0].start, resource_size(&res[0]));
> + if (!mbox_base) {
> + platform_device_put(pdev);
> + return -ENOMEM;
> + }
> +
>   return platform_driver_register(&omap2_mbox_driver);
> 
>  err_out:
> @@ -524,6 +518,7 @@ err_out:
>  static void __exit omap2_mbox_exit(void)
>  {
>   platform_driver_unregister(&omap2_mbox_driver);
> + iounmap(mbox_base);
>  }
> 
>  module_init(omap2_mbox_init);
> --
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock

2010-04-30 Thread Kanigeri, Hari
Ohad, 

Looks like the conversion was missed in few places resulting in compile 
warnings.

Please see the below fix. Let me know if you agree with the change.

#
[PATCH] omap: mailbox: rwlocks to spinlock: compilation fix

fixed the missed  rwlocks in few places resultion in following
compiler warning.

arch/arm/plat-omap/mailbox.c: In function 'omap_mbox_startup':
arch/arm/plat-omap/mailbox.c:246: warning: passing argument 1 of 
'_raw_write_lock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c:251: warning: passing argument 1 of 
'_raw_write_unlock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c:255: warning: passing argument 1 of 
'_raw_write_unlock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c: In function 'omap_mbox_fini':
arch/arm/plat-omap/mailbox.c:301: warning: passing argument 1 of 
'_raw_write_lock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c:306: warning: passing argument 1 of 
'_raw_write_unlock' from incompatible pointer type

Signed-off-by: Hari Kanigeri 
---
 arch/arm/plat-omap/mailbox.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 27a8d98..d6a700d 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -243,16 +243,16 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
struct omap_mbox_queue *mq;

if (likely(mbox->ops->startup)) {
-   write_lock(&mboxes_lock);
+   spin_lock(&mboxes_lock);
if (!mbox_configured)
ret = mbox->ops->startup(mbox);

if (unlikely(ret)) {
-   write_unlock(&mboxes_lock);
+   spin_unlock(&mboxes_lock);
return ret;
}
mbox_configured++;
-   write_unlock(&mboxes_lock);
+   spin_unlock(&mboxes_lock);
}

ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED,
@@ -298,12 +298,12 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox->irq, mbox);

if (likely(mbox->ops->shutdown)) {
-   write_lock(&mboxes_lock);
+   spin_lock(&mboxes_lock);
if (mbox_configured > 0)
mbox_configured--;
if (!mbox_configured)
mbox->ops->shutdown(mbox);
-   write_unlock(&mboxes_lock);
+   spin_unlock(&mboxes_lock);
}
 }

--



> -Original Message-----
> From: Ohad Ben-Cohen [mailto:o...@wizery.com] 
> Sent: Tuesday, April 27, 2010 12:56 PM
> To: linux-omap@vger.kernel.org
> Cc: Kanigeri, Hari; Hiroshi Doyu; Ohad Ben-Cohen
> Subject: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks 
> to spinlock
> 
> rwlocks are slower and have potential starvation issues so 
> spinlocks are generally preferred
> 
> Signed-off-by: Ohad Ben-Cohen 
> ---
>  arch/arm/plat-omap/mailbox.c |   20 ++--
>  1 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/mailbox.c 
> b/arch/arm/plat-omap/mailbox.c index 08a2df7..d73d51a 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -31,7 +31,7 @@
>  
>  static struct workqueue_struct *mboxd;
>  static struct omap_mbox *mboxes;
> -static DEFINE_RWLOCK(mboxes_lock);
> +static DEFINE_SPINLOCK(mboxes_lock);
>  
>  static int mbox_configured;
>  
> @@ -330,14 +330,14 @@ struct omap_mbox *omap_mbox_get(const 
> char *name)
>   struct omap_mbox *mbox;
>   int ret;
>  
> - read_lock(&mboxes_lock);
> + spin_lock(&mboxes_lock);
>   mbox = *(find_mboxes(name));
>   if (mbox == NULL) {
> - read_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
>   return ERR_PTR(-ENOENT);
>   }
>  
> - read_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
>  
>   ret = omap_mbox_startup(mbox);
>   if (ret)
> @@ -363,15 +363,15 @@ int omap_mbox_register(struct device 
> *parent, struct omap_mbox *mbox)
>   if (mbox->next)
>   return -EBUSY;
>  
> - write_lock(&mboxes_lock);
> + spin_lock(&mboxes_lock);
>   tmp = find_mboxes(mbox->name);
>   if (*tmp) {
>   ret = -EBUSY;
> - write_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
>   goto err_find;
>   }
>   *tmp = mbox;
> - write_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
>  
>   re

RE: [PATCH] ARM:iommu support for OMAP4

2010-04-27 Thread Kanigeri, Hari

Hi,

> -Original Message-
> From: Hiroshi DOYU [mailto:hiroshi.d...@nokia.com]
> Sent: Monday, April 19, 2010 1:50 AM
> To: Kanigeri, Hari; p...@pwsan.com; khil...@deeprootsystems.com
> Cc: linux-omap@vger.kernel.org; Shilimkar, Santosh; t...@atomide.com
> Subject: Re: [PATCH] ARM:iommu support for OMAP4
> 
> Hi Hari,
> 
> From: "ext Kanigeri, Hari" 
> Subject: [PATCH] ARM:iommu support for OMAP4
> Date: Fri, 16 Apr 2010 18:17:09 +0200
> 
> > From 708914e1a82a608d423b050cb31b4deb46eb8411 Mon Sep 17 00:00:00 2001
> > From: Hari Kanigeri 
> > Date: Mon, 8 Mar 2010 17:55:21 -0600
> > Subject: [PATCH] ARM:iommu support for OMAP4
> >
> > This patch provides the iommu support for OMAP4 co-processors.
> 
> This looks ok for now mostly. some minor fix are pointed out by
> Santosh, though.
> 
> In the long run, I guess that, this should be converted to "hwmod"?
> 
> ref: http://marc.info/?l=linux-omap&m=127012520411256&w=2 

Looks like for OMAP4 we currently don't have the mechanism to manage the 3 
resets to Ducati (MMU, M3_0, M3_1) independently and they are all tied to one 
hwmod (ducati). I didn't check the OMAP3 implementation but hopefully the reset 
lines for MMU and IVA are not tied to one hwmod.

We would need independent control over these reset pins as RST3 (MMU) will be 
needed for programming MMU prior to bringing cores out of reset, RST_1 will be 
needed to control just M3_0 core, and RST_2 will be needed for M3_1.

I think until this change is done it will be difficult to migrate to hwmod for 
iommu.

Thank you,
Best regards,
Hari

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 4/4][v4] OMAP:iommu- add TLB preservation support

2010-04-26 Thread Kanigeri, Hari
Hi Hiroshi,


> -Original Message-
> From: Hiroshi DOYU [mailto:hiroshi.d...@nokia.com]
> Sent: Saturday, April 24, 2010 1:45 AM
> To: Kanigeri, Hari
> Cc: Gupta, Ramesh; linux-omap@vger.kernel.org
> Subject: Re: [PATCH 4/4][v4] OMAP:iommu- add TLB preservation support
> 
> From: "ext Kanigeri, Hari" 
> Subject: RE: [PATCH 4/4][v4] OMAP:iommu- add TLB preservation support
> Date: Sat, 24 Apr 2010 01:43:19 +0200
> 
> > Hi Hiroshi,
> >
> >>
> >> diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
> >> index 1e83fac..d09a0a1 100644
> >> --- a/arch/arm/plat-omap/iommu.c
> >> +++ b/arch/arm/plat-omap/iommu.c
> >> @@ -25,6 +25,9 @@
> >>
> >>  #include "iopgtable.h"
> >>
> >> +#define for_each_iotlb_cr(obj, n, i, cr)  \
> >> +  for (i = 0; (i < n) && (cr = get_iotlb_cr(obj, i)); i++)
> >> +
> > -- This code is giving compilation error.
> 
> Should be like:
> 
> +#define for_each_iotlb_cr(obj, n, __i, cr)   \
> + for (__i = 0;   \
> +  (__i < (n)) && (cr = __iotlb_read_cr((obj), __i), true);   \
> +  __i++)
> +

-- The change looks good to me.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 4/4][v4] OMAP:iommu- add TLB preservation support

2010-04-23 Thread Kanigeri, Hari
Hi Hiroshi,

> 
> diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
> index 1e83fac..d09a0a1 100644
> --- a/arch/arm/plat-omap/iommu.c
> +++ b/arch/arm/plat-omap/iommu.c
> @@ -25,6 +25,9 @@
> 
>  #include "iopgtable.h"
> 
> +#define for_each_iotlb_cr(obj, n, i, cr) \
> + for (i = 0; (i < n) && (cr = get_iotlb_cr(obj, i)); i++)
> +
-- This code is giving compilation error. 
Can you please send a new patch after fixing this on top of my patches.

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 4/4][v4] OMAP:iommu- add TLB preservation support

2010-04-23 Thread Kanigeri, Hari
Hi Hiroshi,

> -Original Message-
> From: Hiroshi DOYU [mailto:hiroshi.d...@nokia.com]
> Sent: Friday, April 23, 2010 7:34 AM
> To: Kanigeri, Hari
> Cc: Gupta, Ramesh; linux-omap@vger.kernel.org
> Subject: Re: [PATCH 4/4][v4] OMAP:iommu- add TLB preservation support
> 
> Hi Hari,
> 
> From: ext Hari Kanigeri 
> Subject: [PATCH 4/4][v4] OMAP:iommu- add TLB preservation support
> Date: Fri, 23 Apr 2010 01:16:36 +0200
> 
> > This patch adds TLB preservation support to IOMMU module
> >
> > Signed-off-by: Hari Kanigeri 
> > Signed-off-by: Hiroshi Doyu 
> 
> The above should be "Acked-by:"?

-- By you :)

> 
> BTW:
> Now the code gets a bit complicated for tlb iteration. So what about
> introducing the following macro?
> 
> Not yet tested, though

-- I like it. Let me test your changes, and will send a revised patch with 
these changes.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 4/4][v3] OMAP:iommu- add TLB preservation support

2010-04-21 Thread Kanigeri, Hari
Hi Hiroshi,

> > @@ -241,6 +238,11 @@ int load_iotlb_entry(struct iommu *obj, struct
> iotlb_entry *e)
> > break;
> > }
> >
> > +   if (l.base == obj->nr_tlb_entries) {
> > +   dev_dbg(obj->dev, "%s: preserve entries full\n", __func__);
> > +   err = -EBUSY;
> 
> I am afraid that this doesn't work. There are two cases where TLB
> entries are set/updated:
> 
> 1) H/W TWL always updates TLB entries automatically, IOW, increments
>"vict" implicitly.
> 2) A client module adds preservation TLB entry with explictly calling
>"load_iotlb_entry()". IOW, increments both  "vict" and "base"
>intentionally.
> 
> Because of 1), "vict" gets full soon and this is normal, but even if
> this "vict" is full, a preservation entry "base" should be able to be
> set by a client module unless "base" itself rearches full. IOW, Even
> if all TLB entries are valid, a new preservation TLB entry should be
> set by replacing one of the existing normal entry.
> 
> In the case of "e" with perservation bit , "load_iotlb_entry()"
> doesn't usually fail because it replaces the exisiting normal entry,
> but it would fail only if all TLB entries are preserved.
> 

-- I agree with your comments. My implementation was under the assumption that 
the User locks the TLB entries only at startup time, but as you mentioned this 
would not work during run time as the entries would already be having valid 
entries. DSPBridge and the OMAP4 IPCs both locks the entries at init time.

The following changes in load_tlb_entry function should handle the run time 
case too. This change would lock an entry irrespective of existing entry is 
valid or not.
Please let me know if you agree.

@@ -230,22 +227,32 @@ int load_iotlb_entry(struct iommu *obj, struct 
iotlb_entry *e)

clk_enable(obj->clk);

-   for (i = 0; i < obj->nr_tlb_entries; i++) {
-   struct cr_regs tmp;
-
-   iotlb_lock_get(obj, &l);
-   l.vict = i;
-   iotlb_lock_set(obj, &l);
-   iotlb_read_cr(obj, &tmp);
-   if (!iotlb_cr_valid(&tmp))
-   break;
-   }
-
-   if (i == obj->nr_tlb_entries) {
-   dev_dbg(obj->dev, "%s: full: no entry\n", __func__);
+   iotlb_lock_get(obj, &l);
+   if (l.base == obj->nr_tlb_entries) {
+   dev_dbg(obj->dev, "%s: preserve entries full\n", __func__);
err = -EBUSY;
goto out;
}
+   if (!e->prsvd) {
+   for (i = l.base; i < obj->nr_tlb_entries; i++) {
+   struct cr_regs tmp;
+
+   iotlb_lock_get(obj, &l);
+   l.vict = i;
+   iotlb_lock_set(obj, &l);
+   iotlb_read_cr(obj, &tmp);
+   if (!iotlb_cr_valid(&tmp))
+   break;
+   }
+   if (i == obj->nr_tlb_entries) {
+   dev_dbg(obj->dev, "%s: full: no entry\n", __func__);
+   err = -EBUSY;
+   goto out;
+   }
+   } else {
+   l.vict = l.base;
+   iotlb_lock_set(obj, &l);
+   }

cr = iotlb_alloc_cr(obj, e);
if (IS_ERR(cr)) {
@@ -256,9 +263,11 @@ int load_iotlb_entry(struct iommu *obj, struct iotlb_entry 
*e)
iotlb_load_cr(obj, cr);
kfree(cr);

+   if (e->prsvd)
+   l.base++;
/* increment victim for next tlb load */
if (++l.vict == obj->nr_tlb_entries)
-   l.vict = 0;
+   l.vict = l.base;
iotlb_lock_set(obj, &l);
 out:
clk_disable(obj->clk);
--

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2/4][v3] OMAP:iommu support for OMAP4

2010-04-21 Thread Kanigeri, Hari
Tony,

> -Original Message-
> From: Tony Lindgren [mailto:t...@atomide.com]
> Sent: Wednesday, April 21, 2010 5:05 PM
> To: Kanigeri, Hari
> Cc: linux-omap@vger.kernel.org; Shilimkar, Santosh; Felipe Contreras;
> Hiroshi DOYU
> Subject: Re: [PATCH 2/4][v3] OMAP:iommu support for OMAP4
> 
> * Kanigeri, Hari  [100420 15:52]:
> > From 9a2bcae7a2de6890884c23c45563eece1e6838de Mon Sep 17 00:00:00 2001
> > From: Hari Kanigeri 
> > Date: Tue, 20 Apr 2010 17:39:18 -0500
> > Subject: [PATCH 2/4] OMAP:iommu support for OMAP4
> >
> > This patch provides the iommu support for OMAP4 co-processors.
> >
> > Signed-off-by: Hari Kanigeri 
> > ---
> >  arch/arm/mach-omap2/omap-iommu.c   |   26
> ++
> >  arch/arm/plat-omap/include/plat/omap44xx.h |3 +++
> >  2 files changed, 29 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-
> omap2/omap-iommu.c
> > index 416a65d..f569371 100644
> > --- a/arch/arm/mach-omap2/omap-iommu.c
> > +++ b/arch/arm/mach-omap2/omap-iommu.c
> > @@ -13,6 +13,7 @@
> >  #include 
> >
> >  #include 
> > +#include 
> >
> >  struct iommu_device {
> > resource_size_t base;
> > @@ -46,6 +47,31 @@ static struct iommu_device devices[] = {
> >  };
> >  #endif
> >
> > +#ifdef CONFIG_ARCH_OMAP4
> > +static struct iommu_device devices[] = {
> > +   {
> > +   .base = OMAP4_MMU1_BASE,
> > +   .irq = INT_44XX_DUCATI_MMU_IRQ,
> > +   .pdata = {
> > +   .name = "ducati",
> > +   .nr_tlb_entries = 32,
> > +   .clk_name = "ducati_ick",
> > +   },
> > +   },
> > +#if defined(CONFIG_MPU_TESLA_IOMMU)
> > +   {
> > +   .base = OMAP4_MMU2_BASE,
> > +   .irq = INT_44XX_DSP_MMU,
> > +   .pdata = {
> > +   .name = "tesla",
> > +   .nr_tlb_entries = 32,
> > +   .clk_name = "tesla_ick",
> > +   },
> > +   },
> > +#endif
> > +};
> > +#endif
> 
> This should use:
> 
> static struct iommu_device *devices;
> 
> #ifdef CONFIG_ARCH_OMAP3
> static struct iommu_device omap3_devices[] = {
> ...
> #else
> #define omap3_devices NULL
> #endif
> 
> #ifdef CONFIG_ARCH_OMAP4
> static struct iommu_device omap4_devices[] = {
> ...
> #else
> #define omap4_devices NULL
> #endif
> 
> Then in init, just set devices based on the omap type.
> 

Appreciate your input. I will send revised patches.

Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/4][v3] OMAP:iommu- add TLB preservation support

2010-04-20 Thread Kanigeri, Hari
>From b76cbee7bb442f61dd2f29ed64ccf98efd871791 Mon Sep 17 00:00:00 2001
From: Hari Kanigeri 
Date: Tue, 20 Apr 2010 12:11:16 -0500
Subject: [PATCH 4/4] OMAP:iommu- add TLB preservation support

This patch adds TLB preservation support to IOMMU module

Signed-off-by: Hari Kanigeri 
---
 arch/arm/mach-omap2/iommu2.c |4 +++-
 arch/arm/plat-omap/iommu.c   |   12 
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index f01f985..5ce76e4 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -146,6 +146,7 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra)
printk("\n");
 
iommu_write_reg(obj, stat, MMU_IRQSTATUS);
+   omap2_iommu_disable(obj);
return stat;
 }
 
@@ -211,7 +212,8 @@ static ssize_t omap2_dump_cr(struct iommu *obj, struct 
cr_regs *cr, char *buf)
char *p = buf;
 
/* FIXME: Need more detail analysis of cam/ram */
-   p += sprintf(p, "%08x %08x\n", cr->cam, cr->ram);
+   p += sprintf(p, "%08x %08x %01x\n", cr->cam, cr->ram,
+   (cr->cam & MMU_CAM_P) ? 1 : 0);
 
return p - buf;
 }
diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 463d638..a1d4ccf 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -171,15 +171,12 @@ static void iotlb_lock_get(struct iommu *obj, struct 
iotlb_lock *l)
l->base = MMU_LOCK_BASE(val);
l->vict = MMU_LOCK_VICT(val);
 
-   BUG_ON(l->base != 0); /* Currently no preservation is used */
 }
 
 static void iotlb_lock_set(struct iommu *obj, struct iotlb_lock *l)
 {
u32 val;
 
-   BUG_ON(l->base != 0); /* Currently no preservation is used */
-
val = (l->base << MMU_LOCK_BASE_SHIFT);
val |= (l->vict << MMU_LOCK_VICT_SHIFT);
 
@@ -241,6 +238,11 @@ int load_iotlb_entry(struct iommu *obj, struct iotlb_entry 
*e)
break;
}
 
+   if (l.base == obj->nr_tlb_entries) {
+   dev_dbg(obj->dev, "%s: preserve entries full\n", __func__);
+   err = -EBUSY;
+   goto out;
+   }
if (i == obj->nr_tlb_entries) {
dev_dbg(obj->dev, "%s: full: no entry\n", __func__);
err = -EBUSY;
@@ -256,9 +258,11 @@ int load_iotlb_entry(struct iommu *obj, struct iotlb_entry 
*e)
iotlb_load_cr(obj, cr);
kfree(cr);
 
+   if (e->prsvd)
+   l.base++;
/* increment victim for next tlb load */
if (++l.vict == obj->nr_tlb_entries)
-   l.vict = 0;
+   l.vict = l.base;
iotlb_lock_set(obj, &l);
 out:
clk_disable(obj->clk);
-- 
1.7.0


Thank you,
Best regards,
Hari



0004-OMAP-iommu-add-TLB-preservation-support.patch
Description: 0004-OMAP-iommu-add-TLB-preservation-support.patch


[PATCH 3/4][v3] OMAP:iommu - missing check for TLB valid entry

2010-04-20 Thread Kanigeri, Hari
>From 76471c57074ff5a816ad590aca64e3958ed487fa Mon Sep 17 00:00:00 2001
From: Hari Kanigeri 
Date: Tue, 20 Apr 2010 12:08:26 -0500
Subject: [PATCH 3/4] OMAP:iommu - missing check for TLB valid entry

Added the missing TLB valid entry setting for cam register

Signed-off-by: Hari Kanigeri 
---
 arch/arm/mach-omap2/iommu2.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 6f4b7cc..f01f985 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -183,7 +183,7 @@ static struct cr_regs *omap2_alloc_cr(struct iommu *obj, 
struct iotlb_entry *e)
if (!cr)
return ERR_PTR(-ENOMEM);
 
-   cr->cam = (e->da & MMU_CAM_VATAG_MASK) | e->prsvd | e->pgsz;
+   cr->cam = (e->da & MMU_CAM_VATAG_MASK) | e->prsvd | e->pgsz | e->valid;
cr->ram = e->pa | e->endian | e->elsz | e->mixed;
 
return cr;
-- 
1.7.0


Thank you,
Best regards,
Hari



0003-OMAP-iommu-missing-check-for-TLB-valid-entry.patch
Description: 0003-OMAP-iommu-missing-check-for-TLB-valid-entry.patch


[PATCH 2/4][v3] OMAP:iommu support for OMAP4

2010-04-20 Thread Kanigeri, Hari
>From 9a2bcae7a2de6890884c23c45563eece1e6838de Mon Sep 17 00:00:00 2001
From: Hari Kanigeri 
Date: Tue, 20 Apr 2010 17:39:18 -0500
Subject: [PATCH 2/4] OMAP:iommu support for OMAP4

This patch provides the iommu support for OMAP4 co-processors.

Signed-off-by: Hari Kanigeri 
---
 arch/arm/mach-omap2/omap-iommu.c   |   26 ++
 arch/arm/plat-omap/include/plat/omap44xx.h |3 +++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
index 416a65d..f569371 100644
--- a/arch/arm/mach-omap2/omap-iommu.c
+++ b/arch/arm/mach-omap2/omap-iommu.c
@@ -13,6 +13,7 @@
 #include 
 
 #include 
+#include 
 
 struct iommu_device {
resource_size_t base;
@@ -46,6 +47,31 @@ static struct iommu_device devices[] = {
 };
 #endif
 
+#ifdef CONFIG_ARCH_OMAP4
+static struct iommu_device devices[] = {
+   {
+   .base = OMAP4_MMU1_BASE,
+   .irq = INT_44XX_DUCATI_MMU_IRQ,
+   .pdata = {
+   .name = "ducati",
+   .nr_tlb_entries = 32,
+   .clk_name = "ducati_ick",
+   },
+   },
+#if defined(CONFIG_MPU_TESLA_IOMMU)
+   {
+   .base = OMAP4_MMU2_BASE,
+   .irq = INT_44XX_DSP_MMU,
+   .pdata = {
+   .name = "tesla",
+   .nr_tlb_entries = 32,
+   .clk_name = "tesla_ick",
+   },
+   },
+#endif
+};
+#endif
+
 #define NR_IOMMU_DEVICES ARRAY_SIZE(devices)
 
 static struct platform_device *omap_iommu_pdev[NR_IOMMU_DEVICES];
diff --git a/arch/arm/plat-omap/include/plat/omap44xx.h 
b/arch/arm/plat-omap/include/plat/omap44xx.h
index 9cb1e9d..1b5b786 100644
--- a/arch/arm/plat-omap/include/plat/omap44xx.h
+++ b/arch/arm/plat-omap/include/plat/omap44xx.h
@@ -47,5 +47,8 @@
 #define OMAP44XX_MAILBOX_BASE  (L4_44XX_BASE + 0xF4000)
 #define OMAP44XX_HSUSB_OTG_BASE(L4_44XX_BASE + 0xAB000)
 
+#define OMAP4_MMU1_BASE0x55082000
+#define OMAP4_MMU2_BASE0x4A066000
+
 #endif /* __ASM_ARCH_OMAP44XX_H */
 
-- 
1.7.0


Thank you,
Best regards,
Hari



0002-OMAP-iommu-support-for-OMAP4.patch
Description: 0002-OMAP-iommu-support-for-OMAP4.patch


[PATCH 1/4] [v3]OMAP:iommu renamed omap3-iommu to omap-iommu

2010-04-20 Thread Kanigeri, Hari
>From ea10f0d6550d521ff575e0ea03d5ca4fc258b80e Mon Sep 17 00:00:00 2001
From: Hari Kanigeri 
Date: Tue, 20 Apr 2010 17:30:09 -0500
Subject: [PATCH 1/4] OMAP:iommu renamed omap3-iommu to omap-iommu

This patch includes changes to omap3-iommu.c file to make it generic
for all OMAPs. Renamed omap3-iommu.c to omap-iommu.c

Signed-off-by: Hari Kanigeri 
---
 arch/arm/mach-omap2/Makefile  |4 +-
 arch/arm/mach-omap2/omap-iommu.c  |  108 +
 arch/arm/mach-omap2/omap3-iommu.c |  105 ---
 3 files changed, 109 insertions(+), 108 deletions(-)
 create mode 100644 arch/arm/mach-omap2/omap-iommu.c
 delete mode 100644 arch/arm/mach-omap2/omap3-iommu.c

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 32d8e7c..ed706fd 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -83,9 +83,7 @@ obj-$(CONFIG_OMAP3_EMU)   += emu.o
 obj-$(CONFIG_OMAP_MBOX_FWK)+= mailbox_mach.o
 mailbox_mach-objs  := mailbox.o
 
-iommu-y+= iommu2.o
-iommu-$(CONFIG_ARCH_OMAP3) += omap3-iommu.o
-
+iommu-y+= iommu2.o omap-iommu.o
 obj-$(CONFIG_OMAP_IOMMU)   += $(iommu-y)
 
 i2c-omap-$(CONFIG_I2C_OMAP):= i2c.o
diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
new file mode 100644
index 000..416a65d
--- /dev/null
+++ b/arch/arm/mach-omap2/omap-iommu.c
@@ -0,0 +1,108 @@
+/*
+ * omap iommu: omap device registration
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+
+#include 
+
+struct iommu_device {
+   resource_size_t base;
+   int irq;
+   struct iommu_platform_data pdata;
+   struct resource res[2];
+};
+
+#ifdef CONFIG_ARCH_OMAP3
+static struct iommu_device devices[] = {
+   {
+   .base = 0x480bd400,
+   .irq = 24,
+   .pdata = {
+   .name = "isp",
+   .nr_tlb_entries = 8,
+   .clk_name = "cam_ick",
+   },
+   },
+#if defined(CONFIG_MPU_BRIDGE_IOMMU)
+   {
+   .base = 0x5d00,
+   .irq = 28,
+   .pdata = {
+   .name = "iva2",
+   .nr_tlb_entries = 32,
+   .clk_name = "iva2_ck",
+   },
+   },
+#endif
+};
+#endif
+
+#define NR_IOMMU_DEVICES ARRAY_SIZE(devices)
+
+static struct platform_device *omap_iommu_pdev[NR_IOMMU_DEVICES];
+
+static int __init omap_iommu_init(void)
+{
+   int i, err;
+   struct resource res[] = {
+   { .flags = IORESOURCE_MEM },
+   { .flags = IORESOURCE_IRQ },
+   };
+
+   for (i = 0; i < NR_IOMMU_DEVICES; i++) {
+   struct platform_device *pdev;
+   const struct iommu_device *d = &devices[i];
+
+   pdev = platform_device_alloc("omap-iommu", i);
+   if (!pdev) {
+   err = -ENOMEM;
+   goto err_out;
+   }
+
+   res[0].start = d->base;
+   res[0].end = d->base + MMU_REG_SIZE - 1;
+   res[1].start = res[1].end = d->irq;
+
+   err = platform_device_add_resources(pdev, res,
+   ARRAY_SIZE(res));
+   if (err)
+   goto err_out;
+   err = platform_device_add_data(pdev, &d->pdata,
+  sizeof(d->pdata));
+   if (err)
+   goto err_out;
+   err = platform_device_add(pdev);
+   if (err)
+   goto err_out;
+   omap_iommu_pdev[i] = pdev;
+   }
+   return 0;
+
+err_out:
+   while (i--)
+   platform_device_put(omap_iommu_pdev[i]);
+   return err;
+}
+module_init(omap_iommu_init);
+
+static void __exit omap_iommu_exit(void)
+{
+   int i;
+
+   for (i = 0; i < NR_IOMMU_DEVICES; i++)
+   platform_device_unregister(omap_iommu_pdev[i]);
+}
+module_exit(omap_iommu_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU");
+MODULE_DESCRIPTION("omap iommu: omap device registration");
+MODULE_LICENSE("GPL v2");
diff --git a/arch/arm/mach-omap2/omap3-iommu.c 
b/arch/arm/mach-omap2/omap3-iommu.c
deleted file mode 100644
index fbbcb5c..000
--- a/arch/arm/mach-omap2/omap3-iommu.c
+++ /dev/null
@@ -1,105 +0,0 @@
-/*
- * omap iommu: omap3 device registration
- *
- * Copyright (C) 2008-2009 Nokia Corporation
- *
- * Written by Hiroshi DOYU 
- *
- * This program is free software; you can redistribute it and/or modify

[PATCH 0/4][V3] OMAP: IOMMU patches to support OMAP4 and TLB preservation support

2010-04-20 Thread Kanigeri, Hari
Following are the revised patches addressing the comments from Santosh, Felipe, 
and Hiroshi.

The detailed changed list is:

Hari Kanigeri (4)
OMAP:iommu renamed omap3-iommu to omap-iommu
OMAP:iommu support for OMAP4
OMAP:iommu - missing check for TLB valid entry
OMAP:iommu- add TLB preservation support


 arch/arm/mach-omap2/Makefile   |4 +-
 arch/arm/mach-omap2/iommu2.c   |6 +-
 arch/arm/mach-omap2/omap-iommu.c   |  134 
 arch/arm/mach-omap2/omap3-iommu.c  |  105 --
 arch/arm/plat-omap/include/plat/omap44xx.h |3 +
 arch/arm/plat-omap/iommu.c |   12 ++-
 6 files changed, 150 insertions(+), 114 deletions(-)

Thank you,
Best regards,
Hari

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/3][v2] OMAP:iommu support for OMAP4

2010-04-20 Thread Kanigeri, Hari
Felipe,

> -Original Message-
> From: Felipe Contreras [mailto:felipe.contre...@gmail.com]
> Sent: Tuesday, April 20, 2010 4:49 PM
> To: Kanigeri, Hari
> Cc: linux-omap@vger.kernel.org; Shilimkar, Santosh; Hiroshi DOYU;
> t...@atomide.com
> Subject: Re: [PATCH 1/3][v2] OMAP:iommu support for OMAP4
> 
> On Wed, Apr 21, 2010 at 12:31 AM, Kanigeri, Hari 
> wrote:
> > From 9a6f91b0c108fd010afdb6bddcae92579c87fcbd Mon Sep 17 00:00:00 2001
> > From: Hari Kanigeri 
> > Date: Tue, 20 Apr 2010 12:03:03 -0500
> > Subject: [PATCH 1/3] OMAP:iommu support for OMAP4
> >
> > This patch provides the iommu support for OMAP4 co-processors.
> > This includes changes to omap3-iommu.c file to make it generic
> > for all OMAPs. Renamed omap3-iommu.c to omap-iommu.c
> 
> This smells like two patches: one that moves omap3-iommu to
> omap-iommu, and another one that adds omap4 support.
> 

-- I agree. I will resend the patches.

> But unfortunately this would break multi-omap support. I think both
> omap3 and omap4 devices should be there, and selected at run-time.
> 
> Cheers.
> 
> --
> Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/3][v2] OMAP:iommu support for OMAP4

2010-04-20 Thread Kanigeri, Hari
Felipe,

> -Original Message-
> From: Felipe Contreras [mailto:felipe.contre...@gmail.com]
> Sent: Tuesday, April 20, 2010 4:49 PM
> To: Kanigeri, Hari
> Cc: linux-omap@vger.kernel.org; Shilimkar, Santosh; Hiroshi DOYU;
> t...@atomide.com
> Subject: Re: [PATCH 1/3][v2] OMAP:iommu support for OMAP4
> 
> On Wed, Apr 21, 2010 at 12:31 AM, Kanigeri, Hari 
> wrote:
> > From 9a6f91b0c108fd010afdb6bddcae92579c87fcbd Mon Sep 17 00:00:00 2001
> > From: Hari Kanigeri 
> > Date: Tue, 20 Apr 2010 12:03:03 -0500
> > Subject: [PATCH 1/3] OMAP:iommu support for OMAP4
> >
> > This patch provides the iommu support for OMAP4 co-processors.
> > This includes changes to omap3-iommu.c file to make it generic
> > for all OMAPs. Renamed omap3-iommu.c to omap-iommu.c
> 
> This smells like two patches: one that moves omap3-iommu to
> omap-iommu, and another one that adds omap4 support.
> 
> But unfortunately this would break multi-omap support. I think both
> omap3 and omap4 devices should be there, and selected at run-time.
> 

-- I am not sure as why this would break multi-omap support. We would select 
either OMAP3 or OMAP4 specific structures at compile time based on the platform 
selected, and rest of the code is same for both OMAP3/4.

> Cheers.
> 
> --
> Felipe Contreras

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3][v2] OMAP:iommu- add TLB preservation support

2010-04-20 Thread Kanigeri, Hari
>From 2cecc5abd54c7a139495d135260a6a66810318e8 Mon Sep 17 00:00:00 2001
From: Hari Kanigeri 
Date: Tue, 20 Apr 2010 12:11:16 -0500
Subject: [PATCH 3/3] OMAP:iommu- add TLB preservation support

This patch adds TLB preservation support to IOMMU module

Signed-off-by: Hari Kanigeri 
---
 arch/arm/mach-omap2/iommu2.c |4 +++-
 arch/arm/plat-omap/iommu.c   |   12 
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index f01f985..5ce76e4 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -146,6 +146,7 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra)
printk("\n");
 
iommu_write_reg(obj, stat, MMU_IRQSTATUS);
+   omap2_iommu_disable(obj);
return stat;
 }
 
@@ -211,7 +212,8 @@ static ssize_t omap2_dump_cr(struct iommu *obj, struct 
cr_regs *cr, char *buf)
char *p = buf;
 
/* FIXME: Need more detail analysis of cam/ram */
-   p += sprintf(p, "%08x %08x\n", cr->cam, cr->ram);
+   p += sprintf(p, "%08x %08x %01x\n", cr->cam, cr->ram,
+   (cr->cam & MMU_CAM_P) ? 1 : 0);
 
return p - buf;
 }
diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 463d638..a1d4ccf 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -171,15 +171,12 @@ static void iotlb_lock_get(struct iommu *obj, struct 
iotlb_lock *l)
l->base = MMU_LOCK_BASE(val);
l->vict = MMU_LOCK_VICT(val);
 
-   BUG_ON(l->base != 0); /* Currently no preservation is used */
 }
 
 static void iotlb_lock_set(struct iommu *obj, struct iotlb_lock *l)
 {
u32 val;
 
-   BUG_ON(l->base != 0); /* Currently no preservation is used */
-
val = (l->base << MMU_LOCK_BASE_SHIFT);
val |= (l->vict << MMU_LOCK_VICT_SHIFT);
 
@@ -241,6 +238,11 @@ int load_iotlb_entry(struct iommu *obj, struct iotlb_entry 
*e)
break;
}
 
+   if (l.base == obj->nr_tlb_entries) {
+   dev_dbg(obj->dev, "%s: preserve entries full\n", __func__);
+   err = -EBUSY;
+   goto out;
+   }
if (i == obj->nr_tlb_entries) {
dev_dbg(obj->dev, "%s: full: no entry\n", __func__);
err = -EBUSY;
@@ -256,9 +258,11 @@ int load_iotlb_entry(struct iommu *obj, struct iotlb_entry 
*e)
iotlb_load_cr(obj, cr);
kfree(cr);
 
+   if (e->prsvd)
+   l.base++;
/* increment victim for next tlb load */
if (++l.vict == obj->nr_tlb_entries)
-   l.vict = 0;
+   l.vict = l.base;
iotlb_lock_set(obj, &l);
 out:
clk_disable(obj->clk);
-- 
1.7.0


Thank you,
Best regards,
Hari



0003-OMAP-iommu-add-TLB-preservation-support.patch
Description: 0003-OMAP-iommu-add-TLB-preservation-support.patch


[PATCH 2/3][v2] OMAP:iommu - missing check for TLB valid entry

2010-04-20 Thread Kanigeri, Hari
>From a93efa72db7e5bcf601c7c14866f081df176289b Mon Sep 17 00:00:00 2001
From: Hari Kanigeri 
Date: Tue, 20 Apr 2010 12:08:26 -0500
Subject: [PATCH 2/3] OMAP:iommu - missing check for TLB valid entry

Added the missing TLB valid entry setting for cam register

Signed-off-by: Hari Kanigeri 
---
 arch/arm/mach-omap2/iommu2.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 6f4b7cc..f01f985 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -183,7 +183,7 @@ static struct cr_regs *omap2_alloc_cr(struct iommu *obj, 
struct iotlb_entry *e)
if (!cr)
return ERR_PTR(-ENOMEM);
 
-   cr->cam = (e->da & MMU_CAM_VATAG_MASK) | e->prsvd | e->pgsz;
+   cr->cam = (e->da & MMU_CAM_VATAG_MASK) | e->prsvd | e->pgsz | e->valid;
cr->ram = e->pa | e->endian | e->elsz | e->mixed;
 
return cr;
-- 
1.7.0

Thank you,
Best regards,
Hari



0002-OMAP-iommu-missing-check-for-TLB-valid-entry.patch
Description: 0002-OMAP-iommu-missing-check-for-TLB-valid-entry.patch


[PATCH 1/3][v2] OMAP:iommu support for OMAP4

2010-04-20 Thread Kanigeri, Hari
>From 9a6f91b0c108fd010afdb6bddcae92579c87fcbd Mon Sep 17 00:00:00 2001
From: Hari Kanigeri 
Date: Tue, 20 Apr 2010 12:03:03 -0500
Subject: [PATCH 1/3] OMAP:iommu support for OMAP4

This patch provides the iommu support for OMAP4 co-processors.
This includes changes to omap3-iommu.c file to make it generic
for all OMAPs. Renamed omap3-iommu.c to omap-iommu.c

Signed-off-by: Hari Kanigeri 
---
 arch/arm/mach-omap2/Makefile   |4 +-
 arch/arm/mach-omap2/omap-iommu.c   |  134 
 arch/arm/mach-omap2/omap3-iommu.c  |  105 --
 arch/arm/plat-omap/include/plat/omap44xx.h |3 +
 4 files changed, 138 insertions(+), 108 deletions(-)
 create mode 100644 arch/arm/mach-omap2/omap-iommu.c
 delete mode 100644 arch/arm/mach-omap2/omap3-iommu.c

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 32d8e7c..ed706fd 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -83,9 +83,7 @@ obj-$(CONFIG_OMAP3_EMU)   += emu.o
 obj-$(CONFIG_OMAP_MBOX_FWK)+= mailbox_mach.o
 mailbox_mach-objs  := mailbox.o
 
-iommu-y+= iommu2.o
-iommu-$(CONFIG_ARCH_OMAP3) += omap3-iommu.o
-
+iommu-y+= iommu2.o omap-iommu.o
 obj-$(CONFIG_OMAP_IOMMU)   += $(iommu-y)
 
 i2c-omap-$(CONFIG_I2C_OMAP):= i2c.o
diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
new file mode 100644
index 000..f569371
--- /dev/null
+++ b/arch/arm/mach-omap2/omap-iommu.c
@@ -0,0 +1,134 @@
+/*
+ * omap iommu: omap device registration
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+
+#include 
+#include 
+
+struct iommu_device {
+   resource_size_t base;
+   int irq;
+   struct iommu_platform_data pdata;
+   struct resource res[2];
+};
+
+#ifdef CONFIG_ARCH_OMAP3
+static struct iommu_device devices[] = {
+   {
+   .base = 0x480bd400,
+   .irq = 24,
+   .pdata = {
+   .name = "isp",
+   .nr_tlb_entries = 8,
+   .clk_name = "cam_ick",
+   },
+   },
+#if defined(CONFIG_MPU_BRIDGE_IOMMU)
+   {
+   .base = 0x5d00,
+   .irq = 28,
+   .pdata = {
+   .name = "iva2",
+   .nr_tlb_entries = 32,
+   .clk_name = "iva2_ck",
+   },
+   },
+#endif
+};
+#endif
+
+#ifdef CONFIG_ARCH_OMAP4
+static struct iommu_device devices[] = {
+   {
+   .base = OMAP4_MMU1_BASE,
+   .irq = INT_44XX_DUCATI_MMU_IRQ,
+   .pdata = {
+   .name = "ducati",
+   .nr_tlb_entries = 32,
+   .clk_name = "ducati_ick",
+   },
+   },
+#if defined(CONFIG_MPU_TESLA_IOMMU)
+   {
+   .base = OMAP4_MMU2_BASE,
+   .irq = INT_44XX_DSP_MMU,
+   .pdata = {
+   .name = "tesla",
+   .nr_tlb_entries = 32,
+   .clk_name = "tesla_ick",
+   },
+   },
+#endif
+};
+#endif
+
+#define NR_IOMMU_DEVICES ARRAY_SIZE(devices)
+
+static struct platform_device *omap_iommu_pdev[NR_IOMMU_DEVICES];
+
+static int __init omap_iommu_init(void)
+{
+   int i, err;
+   struct resource res[] = {
+   { .flags = IORESOURCE_MEM },
+   { .flags = IORESOURCE_IRQ },
+   };
+
+   for (i = 0; i < NR_IOMMU_DEVICES; i++) {
+   struct platform_device *pdev;
+   const struct iommu_device *d = &devices[i];
+
+   pdev = platform_device_alloc("omap-iommu", i);
+   if (!pdev) {
+   err = -ENOMEM;
+   goto err_out;
+   }
+
+   res[0].start = d->base;
+   res[0].end = d->base + MMU_REG_SIZE - 1;
+   res[1].start = res[1].end = d->irq;
+
+   err = platform_device_add_resources(pdev, res,
+   ARRAY_SIZE(res));
+   if (err)
+   goto err_out;
+   err = platform_device_add_data(pdev, &d->pdata,
+  sizeof(d->pdata));
+   if (err)
+   goto err_out;
+   err = platform_device_add(pdev);
+   if (err)
+   goto err_out;
+   omap_iommu_pdev[i] = pdev;
+   }
+   return 0;
+
+err_out:
+   while (i--)
+   platform_device_put

[PATCH 0/3][V2] OMAP: IOMMU patches to support OMAP4 and TLB preservation support

2010-04-20 Thread Kanigeri, Hari
Following are the revised patches addressing the comments from Santosh, Felipe, 
and Hiroshi.

The detailed changed list is:

Hari Kanigeri(3):
OMAP:iommu support for OMAP4
OMAP:iommu - missing check for TLB valid entry
OMAP:iommu- add TLB preservation support


 arch/arm/mach-omap2/Makefile   |4 +-
 arch/arm/mach-omap2/iommu2.c   |6 +-
 arch/arm/mach-omap2/omap-iommu.c   |  134 
 arch/arm/mach-omap2/omap3-iommu.c  |  105 --
 arch/arm/plat-omap/include/plat/omap44xx.h |3 +
 arch/arm/plat-omap/iommu.c |   13 ++-
 6 files changed, 151 insertions(+), 114 deletions(-)

Thank you,
Best regards,
Hari

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] ARM: OMAP add TLB preservation support to IOMMU

2010-04-20 Thread Kanigeri, Hari
Hi Hiroshi,

> -Original Message-
> From: Hiroshi DOYU [mailto:hiroshi.d...@nokia.com]
> Sent: Monday, April 19, 2010 3:32 AM
> To: Kanigeri, Hari
> Cc: linux-omap@vger.kernel.org; t...@atomide.com; Shilimkar, Santosh
> Subject: Re: [PATCH] ARM: OMAP add TLB preservation support to IOMMU
> 
> Hi Hari,
> 
> From: "ext Kanigeri, Hari" 
> Subject: [PATCH] ARM: OMAP add TLB preservation support to IOMMU
> Date: Fri, 16 Apr 2010 18:18:59 +0200
> 
> > From bcdd232666a163d2661d704f9c21d055bacfd178 Mon Sep 17 00:00:00 2001
> > From: Hari Kanigeri 
> > Date: Mon, 8 Mar 2010 18:00:36 -0600
> > Subject: [PATCH] ARM: OMAP add TLB preservation support to IOMMU
> >
> > This patch adds TLB preservation support to IOMMU module
> >
> > Signed-off-by: Hari Kanigeri 
> > ---
> >  arch/arm/mach-omap2/iommu2.c |7 +--
> >  arch/arm/plat-omap/iommu.c   |   16 +---
> >  2 files changed, 14 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
> > index 6f4b7cc..2735bd7 100644
> > --- a/arch/arm/mach-omap2/iommu2.c
> > +++ b/arch/arm/mach-omap2/iommu2.c
> > @@ -146,6 +146,8 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj,
> u32 *ra)
> > printk("\n");
> >
> > iommu_write_reg(obj, stat, MMU_IRQSTATUS);
> > +   /* Disable MMU to stop continuous generation of MMU faults */
> > +   omap2_iommu_disable(obj);
> 
> The above comment may look a bit redundant.

-- I will fix this.

> 
> > return stat;
> >  }
> >
> > @@ -183,7 +185,7 @@ static struct cr_regs *omap2_alloc_cr(struct iommu
> *obj, struct iotlb_entry *e)
> > if (!cr)
> > return ERR_PTR(-ENOMEM);
> >
> > -   cr->cam = (e->da & MMU_CAM_VATAG_MASK) | e->prsvd | e->pgsz;
> > +   cr->cam = (e->da & MMU_CAM_VATAG_MASK) | e->prsvd | e->pgsz | e-
> >valid;
> > cr->ram = e->pa | e->endian | e->elsz | e->mixed;
> 
> Good finding. This can be a separate patch.

-- I will send this as a separate patch.
> 
> > return cr;
> > @@ -211,7 +213,8 @@ static ssize_t omap2_dump_cr(struct iommu *obj,
> struct cr_regs *cr, char *buf)
> > char *p = buf;
> >
> > /* FIXME: Need more detail analysis of cam/ram */
> > -   p += sprintf(p, "%08x %08x\n", cr->cam, cr->ram);
> > +   p += sprintf(p, "%08x %08x %01x\n", cr->cam, cr->ram,
> > +   (cr->cam & MMU_CAM_P) ? 1 : 0);
> 
> This is ok for now.
> 
> As described in FIXME comment, this cam/ram whole anaylysis can be
> improved/implemented in order to display all items/bits in the end.
> 
> > return p - buf;
> >  }
> > diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
> > index 5186728..64d676e 100644
> > --- a/arch/arm/plat-omap/iommu.c
> > +++ b/arch/arm/plat-omap/iommu.c
> > @@ -171,15 +171,12 @@ static void iotlb_lock_get(struct iommu *obj,
> struct iotlb_lock *l)
> > l->base = MMU_LOCK_BASE(val);
> > l->vict = MMU_LOCK_VICT(val);
> >
> > -   BUG_ON(l->base != 0); /* Currently no preservation is used */
> >  }
> >
> >  static void iotlb_lock_set(struct iommu *obj, struct iotlb_lock *l)
> >  {
> > u32 val;
> >
> > -   BUG_ON(l->base != 0); /* Currently no preservation is used */
> > -
> > val = (l->base << MMU_LOCK_BASE_SHIFT);
> > val |= (l->vict << MMU_LOCK_VICT_SHIFT);
> >
> > @@ -241,7 +238,7 @@ int load_iotlb_entry(struct iommu *obj, struct
> iotlb_entry *e)
> > break;
> > }
> >
> > -   if (i == obj->nr_tlb_entries) {
> > +   if (i == obj->nr_tlb_entries || (l.base == obj->nr_tlb_entries)) {
> 
> The above should be dealt separately, since no vacant entry is normal,
> but no room to be replaced because of preservation should be warned,
> at least.

-- I will fix this.

> 
> > dev_dbg(obj->dev, "%s: full: no entry\n", __func__);
> > err = -EBUSY;
> > goto out;
> > @@ -252,13 +249,18 @@ int load_iotlb_entry(struct iommu *obj, struct
> iotlb_entry *e)
> > clk_disable(obj->clk);
> > return PTR_ERR(cr);
> > }
> > -
> > iotlb_load_cr(obj, cr);
> > kfree(cr);
> >
> > +   /* Increment base number if preservation is set */
> > +   if (e->prsvd)
> > +   l.base++;
> 
> redundant c

RE: [PATCH] ARM:iommu support for OMAP4

2010-04-20 Thread Kanigeri, Hari
> This looks like based on an old version of omap3-iommu.c; maybe you
> should update it.
>

-- I thought about doing this, but then I felt it doesn't make sense putting 
OMAP4 specific definitions in a file that is named after omap3 and all the 
functions in it having omap3 pre-fix.

May be we can rename the omap3-iommu file to omap-iommu and change omap3 prefix 
in functions to omap, and just change the devices structure based on the 
platform. This way it would look more generic to all OMAPs.

Please let me know your thoughts.

Thank you,
Best regards,
Hari

> -Original Message-
> From: Felipe Contreras [mailto:felipe.contre...@gmail.com]
> Sent: Monday, April 19, 2010 2:12 AM
> To: Kanigeri, Hari
> Cc: linux-omap@vger.kernel.org; Doyu Hiroshi (Nokia-D/Helsinki);
> Shilimkar, Santosh; t...@atomide.com
> Subject: Re: [PATCH] ARM:iommu support for OMAP4
> 
> On Fri, Apr 16, 2010 at 7:17 PM, Kanigeri, Hari 
> wrote:
> > From 708914e1a82a608d423b050cb31b4deb46eb8411 Mon Sep 17 00:00:00 2001
> > From: Hari Kanigeri 
> > Date: Mon, 8 Mar 2010 17:55:21 -0600
> > Subject: [PATCH] ARM:iommu support for OMAP4
> >
> > This patch provides the iommu support for OMAP4 co-processors.
> >
> > Signed-off-by: Hari Kanigeri 
> 
> This looks like based on an old version of omap3-iommu.c; maybe you
> should update it.
> 
> --
> Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] ARM: OMAP add TLB preservation support to IOMMU

2010-04-16 Thread Kanigeri, Hari
>From bcdd232666a163d2661d704f9c21d055bacfd178 Mon Sep 17 00:00:00 2001
From: Hari Kanigeri 
Date: Mon, 8 Mar 2010 18:00:36 -0600
Subject: [PATCH] ARM: OMAP add TLB preservation support to IOMMU

This patch adds TLB preservation support to IOMMU module

Signed-off-by: Hari Kanigeri 
---
 arch/arm/mach-omap2/iommu2.c |7 +--
 arch/arm/plat-omap/iommu.c   |   16 +---
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 6f4b7cc..2735bd7 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -146,6 +146,8 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra)
printk("\n");
 
iommu_write_reg(obj, stat, MMU_IRQSTATUS);
+   /* Disable MMU to stop continuous generation of MMU faults */
+   omap2_iommu_disable(obj);
return stat;
 }
 
@@ -183,7 +185,7 @@ static struct cr_regs *omap2_alloc_cr(struct iommu *obj, 
struct iotlb_entry *e)
if (!cr)
return ERR_PTR(-ENOMEM);
 
-   cr->cam = (e->da & MMU_CAM_VATAG_MASK) | e->prsvd | e->pgsz;
+   cr->cam = (e->da & MMU_CAM_VATAG_MASK) | e->prsvd | e->pgsz | e->valid;
cr->ram = e->pa | e->endian | e->elsz | e->mixed;
 
return cr;
@@ -211,7 +213,8 @@ static ssize_t omap2_dump_cr(struct iommu *obj, struct 
cr_regs *cr, char *buf)
char *p = buf;
 
/* FIXME: Need more detail analysis of cam/ram */
-   p += sprintf(p, "%08x %08x\n", cr->cam, cr->ram);
+   p += sprintf(p, "%08x %08x %01x\n", cr->cam, cr->ram,
+   (cr->cam & MMU_CAM_P) ? 1 : 0);
 
return p - buf;
 }
diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 5186728..64d676e 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -171,15 +171,12 @@ static void iotlb_lock_get(struct iommu *obj, struct 
iotlb_lock *l)
l->base = MMU_LOCK_BASE(val);
l->vict = MMU_LOCK_VICT(val);
 
-   BUG_ON(l->base != 0); /* Currently no preservation is used */
 }
 
 static void iotlb_lock_set(struct iommu *obj, struct iotlb_lock *l)
 {
u32 val;
 
-   BUG_ON(l->base != 0); /* Currently no preservation is used */
-
val = (l->base << MMU_LOCK_BASE_SHIFT);
val |= (l->vict << MMU_LOCK_VICT_SHIFT);
 
@@ -241,7 +238,7 @@ int load_iotlb_entry(struct iommu *obj, struct iotlb_entry 
*e)
break;
}
 
-   if (i == obj->nr_tlb_entries) {
+   if (i == obj->nr_tlb_entries || (l.base == obj->nr_tlb_entries)) {
dev_dbg(obj->dev, "%s: full: no entry\n", __func__);
err = -EBUSY;
goto out;
@@ -252,13 +249,18 @@ int load_iotlb_entry(struct iommu *obj, struct 
iotlb_entry *e)
clk_disable(obj->clk);
return PTR_ERR(cr);
}
-
iotlb_load_cr(obj, cr);
kfree(cr);
 
+   /* Increment base number if preservation is set */
+   if (e->prsvd)
+   l.base++;
/* increment victim for next tlb load */
-   if (++l.vict == obj->nr_tlb_entries)
-   l.vict = 0;
+   if (++l.vict == obj->nr_tlb_entries) {
+   l.vict = l.base;
+   goto out;
+   }
+
iotlb_lock_set(obj, &l);
 out:
clk_disable(obj->clk);
-- 
1.7.0


Thank you,
Best regards,
Hari



0001-ARM-OMAP-add-TLB-preservation-support-to-IOMMU.patch
Description: 0001-ARM-OMAP-add-TLB-preservation-support-to-IOMMU.patch


[PATCH] ARM:iommu support for OMAP4

2010-04-16 Thread Kanigeri, Hari
>From 708914e1a82a608d423b050cb31b4deb46eb8411 Mon Sep 17 00:00:00 2001
From: Hari Kanigeri 
Date: Mon, 8 Mar 2010 17:55:21 -0600
Subject: [PATCH] ARM:iommu support for OMAP4

This patch provides the iommu support for OMAP4 co-processors.

Signed-off-by: Hari Kanigeri 
---
 arch/arm/mach-omap2/Makefile  |2 +-
 arch/arm/mach-omap2/omap4-iommu.c |  110 +
 arch/arm/plat-omap/iommu.c|   10 ++-
 3 files changed, 117 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/mach-omap2/omap4-iommu.c

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index d3e54da..1395125 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -77,7 +77,7 @@ mailbox_mach-objs := mailbox.o
 
 iommu-y+= iommu2.o
 iommu-$(CONFIG_ARCH_OMAP3) += omap3-iommu.o
-
+iommu-$(CONFIG_ARCH_OMAP4)  += omap4-iommu.o
 obj-$(CONFIG_OMAP_IOMMU)   += $(iommu-y)
 
 i2c-omap-$(CONFIG_I2C_OMAP):= i2c.o
diff --git a/arch/arm/mach-omap2/omap4-iommu.c 
b/arch/arm/mach-omap2/omap4-iommu.c
new file mode 100644
index 000..6225616
--- /dev/null
+++ b/arch/arm/mach-omap2/omap4-iommu.c
@@ -0,0 +1,110 @@
+/*
+ * omap iommu: omap4 device registration
+ *
+ * Copyright (C) 2009-2010 Nokia Corporation
+ *
+ * Written by Hari Kanigeri 
+ *
+ * Added support for OMAP4. This is based on original file
+ * omap3-iommu.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+
+#include 
+#include 
+
+#define OMAP4_MMU1_BASE0x55082000
+#define OMAP4_MMU2_BASE0x4A066000
+
+#define OMAP4_MMU1_IRQ INT_44XX_DUCATI_MMU_IRQ
+#define OMAP4_MMU2_IRQ INT_44XX_DSP_MMU
+
+
+
+static unsigned long iommu_base[] __initdata = {
+   OMAP4_MMU1_BASE,
+   OMAP4_MMU2_BASE,
+};
+
+static int iommu_irq[] __initdata = {
+   OMAP4_MMU1_IRQ,
+   OMAP4_MMU2_IRQ,
+};
+
+static const struct iommu_platform_data omap4_iommu_pdata[] __initconst = {
+   {
+   .name = "ducati",
+   .nr_tlb_entries = 32,
+   },
+#if defined(CONFIG_MPU_TESLA_IOMMU)
+   {
+   .name = "tesla",
+   .nr_tlb_entries = 32,
+   },
+#endif
+};
+#define NR_IOMMU_DEVICES ARRAY_SIZE(omap4_iommu_pdata)
+
+static struct platform_device *omap4_iommu_pdev[NR_IOMMU_DEVICES];
+
+static int __init omap4_iommu_init(void)
+{
+   int i, err;
+
+   for (i = 0; i < NR_IOMMU_DEVICES; i++) {
+   struct platform_device *pdev;
+   struct resource res[2];
+
+   pdev = platform_device_alloc("omap-iommu", i);
+   if (!pdev) {
+   err = -ENOMEM;
+   goto err_out;
+   }
+
+   memset(res, 0,  sizeof(res));
+   res[0].start = iommu_base[i];
+   res[0].end = iommu_base[i] + MMU_REG_SIZE - 1;
+   res[0].flags = IORESOURCE_MEM;
+   res[1].start = res[1].end = iommu_irq[i];
+   res[1].flags = IORESOURCE_IRQ;
+
+   err = platform_device_add_resources(pdev, res,
+   ARRAY_SIZE(res));
+   if (err)
+   goto err_out;
+   err = platform_device_add_data(pdev, &omap4_iommu_pdata[i],
+  sizeof(omap4_iommu_pdata[0]));
+   if (err)
+   goto err_out;
+   err = platform_device_add(pdev);
+   if (err)
+   goto err_out;
+   omap4_iommu_pdev[i] = pdev;
+   }
+   return 0;
+
+err_out:
+   while (i--)
+   platform_device_put(omap4_iommu_pdev[i]);
+   return err;
+}
+module_init(omap4_iommu_init);
+
+static void __exit omap4_iommu_exit(void)
+{
+   int i;
+
+   for (i = 0; i < NR_IOMMU_DEVICES; i++)
+   platform_device_unregister(omap4_iommu_pdev[i]);
+}
+module_exit(omap4_iommu_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU, Hari Kanigeri");
+MODULE_DESCRIPTION("omap iommu: omap4 device registration");
+MODULE_LICENSE("GPL v2");
+
diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 905ed83..5186728 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -862,10 +862,12 @@ static int __devinit omap_iommu_probe(struct 
platform_device *pdev)
if (!obj)
return -ENOMEM;
 
-   obj->clk = clk_get(&pdev->dev, pdata->clk_name);
-   if (IS_ERR(obj->clk))
-   goto err_clk;
-
+   /* FIX ME: OMAP4 PM framework not ready */
+   if (!cpu_is_omap44xx()) {
+   obj->clk = clk_get(&pdev->dev, pdata->clk_name);
+   if (IS_ERR(obj->clk))
+   goto err_clk;
+

RE: [PATCH 03/13] DSPBRIDGE: Moving functions from mem.c to drv.c

2010-04-16 Thread Kanigeri, Hari
Ivan,

> In the commit 702b94bff3c50542a6e4ab9a4f4cef093262fe65 (2.6.34) the
> functions
> dmac_inv_range and dmac_clean_range were removed.
> 
> I'm wondering how to fix this in order to rebase to 2.6.34.
> 
> Thanks

2 options:

1. Replace dmac_inv_range and dmac_clean_range functions with the new dma map 
and unmap functions. These are supposed to be the new functions to do the same 
functionality, but in our limited testing some time ago, the dma_unmap function 
wasn't invalidating the cache. You can check this at your end.

2. Revert the patch that deprecated these APIs to unblock you temporarily.

Thank you,
Best regards,
Hari

> -Original Message-
> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
> ow...@vger.kernel.org] On Behalf Of Víctor M. Jáquez L.
> Sent: Friday, April 16, 2010 5:37 AM
> To: Gomez Castellanos, Ivan
> Cc: linux-omap@vger.kernel.org; Hiroshi DOYU; Felipe Contreras; Ameya
> Palande; Menon, Nishanth
> Subject: Re: [PATCH 03/13] DSPBRIDGE: Moving functions from mem.c to drv.c
> 
> On Thu, Apr 08, 2010 at 06:47:13PM -0500, Gomez Castellanos, Ivan wrote:
> 
> > +/*
> > + *   mem_flush_cache ===
> > + *  Purpose:
> > + *  Flush cache
> > + */
> > +void mem_flush_cache(void *pMemBuf, u32 byte_size, s32 FlushType)
> > +{
> > +   if (!pMemBuf)
> > +   return;
> > +
> > +   switch (FlushType) {
> > +   /* invalidate only */
> > +   case PROC_INVALIDATE_MEM:
> > +   dmac_inv_range(pMemBuf, pMemBuf + byte_size);
> > +   outer_inv_range(__pa((u32) pMemBuf), __pa((u32) pMemBuf
> +
> > + byte_size));
> > +   break;
> > +   /* writeback only */
> > +   case PROC_WRITEBACK_MEM:
> > +   dmac_clean_range(pMemBuf, pMemBuf + byte_size);
> > +   outer_clean_range(__pa((u32) pMemBuf), __pa((u32)
> pMemBuf +
> > +   byte_size));
> > +   break;
> > +   /* writeback and invalidate */
> > +   case PROC_WRITEBACK_INVALIDATE_MEM:
> > +   dmac_flush_range(pMemBuf, pMemBuf + byte_size);
> > +   outer_flush_range(__pa((u32) pMemBuf), __pa((u32)
> pMemBuf +
> > +   byte_size));
> > +   break;
> > +   }
> 
> In the commit 702b94bff3c50542a6e4ab9a4f4cef093262fe65 (2.6.34) the
> functions
> dmac_inv_range and dmac_clean_range were removed.
> 
> I'm wondering how to fix this in order to rebase to 2.6.34.
> 
> Thanks
> 
> vmjl
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: PATCH] OMAP3: add mailbox initialization for 3630

2010-03-25 Thread Kanigeri, Hari
Fernando,

> + if (cpu_is_omap2420() || cpu_is_omap3430() ||
> + cpu_is_omap3630() || cpu_is_omap44xx()) 

looks like this check is applied to all OMAPS. Can we just remove this check ?

Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/8] DSPBRIDGE: Increased DMM size to 256MB

2010-01-08 Thread Kanigeri, Hari
Nishant,

> Would it be better that we make this as a board specific memory
> requirement? not all boards will have the same needs right?

For every 1MB of DSP Virtual address 1Kbytes of physical memory is required for 
house keeping. So, for 256MB we would be taking 256K as opposed to 64K when 
using 64MB DSP virtual memory.

The requirement comes more from Multimedia requirements, and with all the new 
phones having the requirement of running multiple multimedia applications in 
parallel 256K would be safe.

Thank you, 
Best regards,
Hari

> -Original Message-
> From: Menon, Nishanth
> Sent: Friday, January 08, 2010 8:28 AM
> To: Kanigeri, Hari
> Cc: Ramirez Luna, Omar; linux-omap; Hiroshi Doyu; Ameya Palande; Felipe
> Contreras; Guzman Lugo, Fernando; Ramos Falcon, Ernesto; Aguilar Pena,
> Leed
> Subject: Re: [PATCH 3/8] DSPBRIDGE: Increased DMM size to 256MB
> 
> Kanigeri, Hari had written, on 01/07/2010 11:16 PM, the following:
> > Nishant,
> >
> > With 64MB we were seeing cases of running out of DSP virtual memory when
> running multiple Multimedia use cases in parallel at a time.
> Would it be better that we make this as a board specific memory
> requirement? not all boards will have the same needs right?
> 
> >
> > Thank you,
> > Best regards,
> > Hari
> >
> > -Original Message-
> > From: Menon, Nishanth
> > Sent: Friday, January 08, 2010 7:48 AM
> > To: Ramirez Luna, Omar
> > Cc: linux-omap; Hiroshi Doyu; Ameya Palande; Felipe Contreras; Guzman
> Lugo, Fernando; Ramos Falcon, Ernesto; Kanigeri, Hari; Aguilar Pena, Leed
> > Subject: Re: [PATCH 3/8] DSPBRIDGE: Increased DMM size to 256MB
> >
> > Ramirez Luna, Omar had written, on 01/07/2010 07:00 PM, the following:
> >> From: Hari Kanigeri 
> >>
> >> This patch increases the DMM from 64MB to 256MB.
> >
> > begs the question: Why?
> >
> >> Signed-off-by: Hari Kanigeri 
> >> Signed-off-by: Omar Ramirez Luna 
> >> Signed-off-by: Leed Aguilar 
> >> ---
> >>  arch/arm/plat-omap/include/dspbridge/dmm.h |2 +-
> >>  drivers/dsp/bridge/pmgr/dmm.c  |8 
> >>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch/arm/plat-omap/include/dspbridge/dmm.h
> b/arch/arm/plat-omap/include/dspbridge/dmm.h
> >> index 335edf8..af0c35a 100644
> >> --- a/arch/arm/plat-omap/include/dspbridge/dmm.h
> >> +++ b/arch/arm/plat-omap/include/dspbridge/dmm.h
> >> @@ -41,7 +41,7 @@
> >>u32 reserved;
> >>} ;
> >>
> >> -#define DMMPOOLSIZE  0x400
> >> +#define DMMPOOLSIZE  0x1000
> >>
> >>  /*
> >>   *   DMM_GetHandle 
> >> diff --git a/drivers/dsp/bridge/pmgr/dmm.c
> b/drivers/dsp/bridge/pmgr/dmm.c
> >> index 46c05c6..f878855 100644
> >> --- a/drivers/dsp/bridge/pmgr/dmm.c
> >> +++ b/drivers/dsp/bridge/pmgr/dmm.c
> >> @@ -103,10 +103,10 @@ static struct GT_Mask DMM_debugMask = { NULL,
> NULL };   /* GT trace variable */
> >>
> >>  static u32 cRefs; /* module reference count */
> >>  struct MapPage {
> >> -  u32   RegionSize:15;
> >> -  u32   MappedSize:15;
> >> -  u32   bReserved:1;
> >> -  u32   bMapped:1;
> >> +  u64   RegionSize:31;
> >> +  u64   MappedSize:31;
> >> +  u64   bReserved:1;
> >> +  u64   bMapped:1;
> 
> this does not make much sense meanwhile.. what does this have to do with
> the dmmpool size increase and what are these unused fields being used for?
> 
> >>  };
> >>
> >>  /*  Create the free list */
> >
> >
> 
> 
> --
> Regards,
> Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/8] DSPBRIDGE: Increased DMM size to 256MB

2010-01-08 Thread Kanigeri, Hari
Felipe,

> Will this work when shm_size=0x400?

It will. Increasing DMM pool size doesn't have any dependency on shm size. The 
SHM is used for IPC mechanism to exchange data and loading base images. The DMM 
is just the DSP virtual address pool that is used for dynamic memory mapping to 
the buffers allocated on ARM (this buffer is not coming from SHM).

Thank you,
Best regards,
Hari

-Original Message-
From: Felipe Contreras [mailto:felipe.contre...@nokia.com] 
Sent: Friday, January 08, 2010 7:38 PM
To: Ramirez Luna, Omar
Cc: linux-omap; Hiroshi Doyu; Ameya Palande; Guzman Lugo, Fernando; Ramos 
Falcon, Ernesto; Kanigeri, Hari; Aguilar Pena, Leed
Subject: Re: [PATCH 3/8] DSPBRIDGE: Increased DMM size to 256MB

On Fri, Jan 08, 2010 at 02:00:35AM +0100, Omar Ramirez Luna wrote:
> From: Hari Kanigeri 
> 
> This patch increases the DMM from 64MB to 256MB.

Will this work when shm_size=0x400?

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/8] DSPBRIDGE: Increased DMM size to 256MB

2010-01-07 Thread Kanigeri, Hari
Nishant,

With 64MB we were seeing cases of running out of DSP virtual memory when 
running multiple Multimedia use cases in parallel at a time. 

Thank you,
Best regards,
Hari  

-Original Message-
From: Menon, Nishanth 
Sent: Friday, January 08, 2010 7:48 AM
To: Ramirez Luna, Omar
Cc: linux-omap; Hiroshi Doyu; Ameya Palande; Felipe Contreras; Guzman Lugo, 
Fernando; Ramos Falcon, Ernesto; Kanigeri, Hari; Aguilar Pena, Leed
Subject: Re: [PATCH 3/8] DSPBRIDGE: Increased DMM size to 256MB

Ramirez Luna, Omar had written, on 01/07/2010 07:00 PM, the following:
> From: Hari Kanigeri 
> 
> This patch increases the DMM from 64MB to 256MB.

begs the question: Why?

> 
> Signed-off-by: Hari Kanigeri 
> Signed-off-by: Omar Ramirez Luna 
> Signed-off-by: Leed Aguilar 
> ---
>  arch/arm/plat-omap/include/dspbridge/dmm.h |2 +-
>  drivers/dsp/bridge/pmgr/dmm.c  |8 
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/dspbridge/dmm.h 
> b/arch/arm/plat-omap/include/dspbridge/dmm.h
> index 335edf8..af0c35a 100644
> --- a/arch/arm/plat-omap/include/dspbridge/dmm.h
> +++ b/arch/arm/plat-omap/include/dspbridge/dmm.h
> @@ -41,7 +41,7 @@
>   u32 reserved;
>   } ;
>  
> -#define DMMPOOLSIZE  0x400
> +#define DMMPOOLSIZE  0x1000
>  
>  /*
>   *   DMM_GetHandle 
> diff --git a/drivers/dsp/bridge/pmgr/dmm.c b/drivers/dsp/bridge/pmgr/dmm.c
> index 46c05c6..f878855 100644
> --- a/drivers/dsp/bridge/pmgr/dmm.c
> +++ b/drivers/dsp/bridge/pmgr/dmm.c
> @@ -103,10 +103,10 @@ static struct GT_Mask DMM_debugMask = { NULL, NULL };   
> /* GT trace variable */
>  
>  static u32 cRefs;/* module reference count */
>  struct MapPage {
> - u32   RegionSize:15;
> - u32   MappedSize:15;
> - u32   bReserved:1;
> - u32   bMapped:1;
> + u64   RegionSize:31;
> + u64   MappedSize:31;
> + u64   bReserved:1;
> + u64   bMapped:1;
>  };
>  
>  /*  Create the free list */


-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [DSPBRIDGE RFC] Combining Reserve and Map into a new IOCTL

2009-09-02 Thread Kanigeri, Hari
The initial idea behind DSPProcessor_ReserveMemory call was to provide the 
User's the option of doing DSP buffer management for a pool of memory by 
themselves. So, call Reserve one time, and do map/unmap multiple times within 
that region, and call Unreserve only once you are done using the Buffer. But 
obviously that's not how this is being used.

Thank you,
Best regards,
Hari

> -Original Message-
> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
> ow...@vger.kernel.org] On Behalf Of Felipe Contreras
> Sent: Wednesday, September 02, 2009 10:38 AM
> To: Palande Ameya (Nokia-D/Helsinki)
> Cc: linux-omap@vger.kernel.org; Ramirez Luna, Omar; Guzman Lugo, Fernando;
> Doyu Hiroshi (Nokia-D/Helsinki); Tereshonkov Roman (Nokia-D/Helsinki)
> Subject: Re: [DSPBRIDGE RFC] Combining Reserve and Map into a new IOCTL
> 
> On Tue, Sep 01, 2009 at 03:35:15PM +0200, Ameya Palande wrote:
> > Hi,
> >
> > Current DSPBridge MPU side API provides following IOCTLs which are
> related to
> > reserving and mapping DSP address space:
> >
> > 1. DSPProcessor_ReserveMemory(): Reserve a virtually contiguous region
> of DSP
> >address space.
> > 2. DSPProcessor_Map(): Maps an MPU buffer to the DSP virtual address
> space.
> > 3. DSPProcessor_UnMap(): Remove an MPU buffer mapping from the DSP
> virtual
> >address space.
> > 4. DSPProcessor_UnReserveMemory(): Frees a previously reserved region of
> the
> >DSP virtual address space.
> >
> > Typical call sequence is:
> >
> > DSPProcessor_ReserveMemory()
> > DSPProcessor_Map()
> > DSPProcessor_UnMap()
> > DSPProcessor_UnReserveMemory()
> >
> > Current approach has following problems:
> > 1. Caller has to perform 4 system calls in order to map and unmap a
> buffer.
> > 2. Kernel has no idea about the type of buffer (input/output). So
> depending
> >on buffer type caller has to explicitly call
> DSPProcessor_FlushMemory() or
> >DSPProcessor_InvalidateMemory().
> >
> > Proposed approach:
> > Introduce 2 new IOCTLs which combine (reserve, map) and (unmap,
> unreserve).
> > Caller should also specify buffer type (input/output) attribute as a
> > parameter to new mapping IOCTL.
> >
> > Benefits of new approach:
> > 1. Saves 2 system calls per map and unmap pair.
> > 2. By implementing lazy unreserve we can introduce cache of reserved
> >mappings, which can skip reserve, unreserve operations.
> > 3. Kernel can take care of flushing/invalidating cache depending on
> buffer
> >type, which saves system call overhead and removed explicit cache
> control
> >from user space.
> >
> > These IOCTLs can be added to the current set of API which doesn't break
> > compatibility with old applications.
> >
> > Waiting for comments!
> >
> > Ideas proposed in this document are from:
> > 1. Hiroshi Doyu 
> > 2. Felipe Contreras 
> 
> The whole proposal looks good to me :)
> 
> However, sometimes you would still want to do some extra cache
> operations afer Map() there should be a way to do 'flush', 'clean' and
> 'inv'.
> 
> Cheers.
> 
> --
> Felipe Contreras
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v1 3/3] ARM:OMAP4 iommu:provide build support for omap4 iommu

2009-08-05 Thread Kanigeri, Hari
Russell,

Thanks for your comments.

Thank you,
Best regards,
Hari

> -Original Message-
> From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
> Sent: Wednesday, August 05, 2009 3:23 PM
> To: Kanigeri, Hari
> Cc: Hiroshi DOYU; linux-omap@vger.kernel.org; t...@atomide.com; Shilimkar,
> Santosh
> Subject: Re: [PATCH v1 3/3] ARM:OMAP4 iommu:provide build support for
> omap4 iommu
> 
> On Wed, Aug 05, 2009 at 11:44:53AM -0500, Kanigeri, Hari wrote:
> > -- I agree with your comments from OMAP3 point of view.
> > For OMAP4, I guess Camera is not going to use IOMMU module. IOMMU
> > modules will be used only by the IPC that are communicating with 2
> > remote Cores (Ducati and Tesla).
> 
> In which case what you do is:
> 
> config OMAP_CAMERA
>   ...
>   select OMAP_IOMMU if ARCH_OMAP3
> 
> which means the IOMMU support will only be built if the camera is enabled
> and we're building for OMAP3.
> 
> However, if the OMAP3 hardware uses the IOMMU but the OMAP4 hardware
> doesn't, I'd hazard a guess (and it's only a guess) that the drivers
> will be soo different that squeezing them into one file/config option
> would be silly.
> 
> > Having said this, there might not
> > be any clients of IOMMU in the Kernel space as we are looking at
> > the option of using the IOMMU as a character driver from User-space.
> > For this reason, I think we should have the configuration option to
> > build iommu. Please let me know your comments.
> 
> In which case, the IOMMU character driver can select the core IOMMU
> support.
> 
> Making symbols which are 'selected' visible causes problems - you
> can't disable them and you can't work out why they're being
> forcefully enabled, which just annoys people who are trying to
> configure the kernel.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v1 3/3] ARM:OMAP4 iommu:provide build support for omap4 iommu

2009-08-05 Thread Kanigeri, Hari
Hi Russell and Hiroshi,

Thanks for your comments. Please see my below comments.

> -Original Message-
> From: Hiroshi DOYU [mailto:hiroshi.d...@nokia.com]
> Sent: Tuesday, August 04, 2009 11:36 PM
> To: Kanigeri, Hari
> Cc: linux-omap@vger.kernel.org; t...@atomide.com; li...@arm.linux.org.uk;
> Shilimkar, Santosh
> Subject: Re: [PATCH v1 3/3] ARM:OMAP4 iommu:provide build support for
> omap4 iommu
> 
> From: ext Russell King - ARM Linux 
> Subject: Re: [PATCH v1 3/3] ARM:OMAP4 iommu:provide build support for
> omap4 iommu
> Date: Wed, 5 Aug 2009 00:42:41 +0200
> 
> > On Tue, Aug 04, 2009 at 05:32:12PM -0500, Kanigeri, Hari wrote:
> > > diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
> > > index efe85d0..50aaefb 100644
> > > --- a/arch/arm/plat-omap/Kconfig
> > > +++ b/arch/arm/plat-omap/Kconfig
> > > @@ -118,8 +118,11 @@ config OMAP_MBOX_FWK
> > > DSP, IVA1.0 and IVA2 in OMAP1/2/3.
> > >
> > >  config OMAP_IOMMU
> > > - tristate
> > > -
> > > + tristate "iommu"
> >
> > Insufficiently verbose description.  "OMAP IOMMU Support" would be
> > better.  I thought the idea here was to arrange for things to select
> > OMAP_IOMMU when they require it rather than offering it as a separate
> > configuration option.
> >
> > The former way has the advantage that you don't need to know that you
> > need IOMMU support to (eg) use the Camera - enabling the Camera
> > should automatically enable IOMMU support.
> 
> The following is the original discussion.
> 
> http://lists.arm.linux.org.uk/lurker/message/20090518.130233.3f238e72.en.h
> tml

-- I agree with your comments from OMAP3 point of view. 
For OMAP4, I guess Camera is not going to use IOMMU module. IOMMU modules will 
be used only by the IPC that are communicating with 2 remote Cores (Ducati and 
Tesla). Having said this, there might not be any clients of IOMMU in the Kernel 
space as we are looking at the option of using the IOMMU as a character driver 
from User-space. For this reason, I think we should have the configuration 
option to build iommu. Please let me know your comments.

Thank you,
Best regards,
Hari


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   3   >