Re: [PATCH 1/6] ARM: OMAP2+: PM: protect the power domain state change by a mutex

2012-05-06 Thread Paul Walmsley
Hi

On Wed, 18 Apr 2012, jean.pi...@newoldbits.com wrote:

> From: Jean Pihet 
> 
> Signed-off-by: Jean Pihet 

This patch is missing a description, which would describe why this lock is 
needed and what it protects against.  Please add this.

> ---
>  arch/arm/mach-omap2/pm.c  |8 ++--
>  arch/arm/mach-omap2/powerdomain.c |1 +
>  arch/arm/mach-omap2/powerdomain.h |3 ++-
>  3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index d0c1c96..6918a13 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -100,15 +100,17 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 
> pwrst)
>   if (!pwrdm || IS_ERR(pwrdm))
>   return -EINVAL;
>  
> + mutex_lock(&pwrdm->lock);
> +
>   while (!(pwrdm->pwrsts & (1 << pwrst))) {
>   if (pwrst == PWRDM_POWER_OFF)
> - return ret;
> + goto out;
>   pwrst--;
>   }
>  
>   next_pwrst = pwrdm_read_next_pwrst(pwrdm);
>   if (next_pwrst == pwrst)
> - return ret;
> + goto out;
>  
>   curr_pwrst = pwrdm_read_pwrst(pwrdm);
>   if (curr_pwrst < PWRDM_POWER_ON) {
> @@ -141,6 +143,8 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 
> pwrst)
>   break;
>   }
>  
> +out:
> + mutex_unlock(&pwrdm->lock);
>   return ret;
>  }
>  

So this mutex would protect against simultaneous calls to 
omap_set_pwrdm_state(), but shouldn't this protection be extended to 
anything that would change the powerdomain's state?  For example, couldn't 
other calls to pwrdm_set_next_pwrst() race against this function?

> diff --git a/arch/arm/mach-omap2/powerdomain.c 
> b/arch/arm/mach-omap2/powerdomain.c
> index 96ad3dbe..319b277 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -102,6 +102,7 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
>   INIT_LIST_HEAD(&pwrdm->voltdm_node);
>   voltdm_add_pwrdm(voltdm, pwrdm);
>  
> + mutex_init(&pwrdm->lock);
>   list_add(&pwrdm->node, &pwrdm_list);
>  
>   /* Initialize the powerdomain's state counter */
> diff --git a/arch/arm/mach-omap2/powerdomain.h 
> b/arch/arm/mach-omap2/powerdomain.h
> index 0d72a8a..6c6567d 100644
> --- a/arch/arm/mach-omap2/powerdomain.h
> +++ b/arch/arm/mach-omap2/powerdomain.h
> @@ -19,7 +19,7 @@
>  
>  #include 
>  #include 
> -
> +#include 
>  #include 
>  
>  #include 
> @@ -116,6 +116,7 @@ struct powerdomain {
>   struct clockdomain *pwrdm_clkdms[PWRDM_MAX_CLKDMS];
>   struct list_head node;
>   struct list_head voltdm_node;
> + struct mutex lock;
>   int state;
>   unsigned state_counter[PWRDM_MAX_PWRSTS];
>   unsigned ret_logic_off_counter;

Please add a kerneldoc entry in the struct powerdomain documentation for 
this field.


- Paul
--
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 05/10] ARM: OMAP2+: SmartReflex: introduce a busy loop condition test macro

2012-05-06 Thread J, KEERTHY
On Fri, May 4, 2012 at 2:42 PM, AnilKumar, Chimata  wrote:
> On Thu, Apr 26, 2012 at 23:10:36, J, KEERTHY wrote:
>> From: Jean Pihet 
>>
>> Now that omap_test_timeout is only accessible from mach-omap2/,
>> introduce a similar function for SR.
>>
>> This change makes the SmartReflex implementation ready for the move
>> to drivers/.
>>
>> Signed-off-by: Jean Pihet 
>> Signed-off-by: J Keerthy 
>> ---
>>  arch/arm/mach-omap2/smartreflex.c |   12 ++--
>>  include/linux/power/smartreflex.h |   23 ++-
>>  2 files changed, 28 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/smartreflex.c 
>> b/arch/arm/mach-omap2/smartreflex.c
>> index d859277..acef08d 100644
>> --- a/arch/arm/mach-omap2/smartreflex.c
>> +++ b/arch/arm/mach-omap2/smartreflex.c
>> @@ -289,9 +289,9 @@ static void sr_v1_disable(struct omap_sr *sr)
>>        * Wait for SR to be disabled.
>>        * wait until ERRCONFIG.MCUDISACKINTST = 1. Typical latency is 1us.
>>        */
>> -     omap_test_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
>> -                     ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
>> -                     timeout);
>> +     sr_test_cond_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
>> +                          ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
>> +                          timeout);
>>
>>       if (timeout >= SR_DISABLE_TIMEOUT)
>>               dev_warn(&sr->pdev->dev, "%s: Smartreflex disable timedout\n",
>> @@ -334,9 +334,9 @@ static void sr_v2_disable(struct omap_sr *sr)
>>        * Wait for SR to be disabled.
>>        * wait until IRQSTATUS.MCUDISACKINTST = 1. Typical latency is 1us.
>>        */
>> -     omap_test_timeout((sr_read_reg(sr, IRQSTATUS) &
>> -                     IRQSTATUS_MCUDISABLEACKINT), SR_DISABLE_TIMEOUT,
>> -                     timeout);
>> +     sr_test_cond_timeout((sr_read_reg(sr, IRQSTATUS) &
>> +                          IRQSTATUS_MCUDISABLEACKINT), SR_DISABLE_TIMEOUT,
>> +                          timeout);
>>
>>       if (timeout >= SR_DISABLE_TIMEOUT)
>>               dev_warn(&sr->pdev->dev, "%s: Smartreflex disable timedout\n",
>> diff --git a/include/linux/power/smartreflex.h 
>> b/include/linux/power/smartreflex.h
>> index 884eaee..78b795e 100644
>> --- a/include/linux/power/smartreflex.h
>> +++ b/include/linux/power/smartreflex.h
>> @@ -22,7 +22,7 @@
>>
>>  #include 
>>  #include 
>> -
>> +#include 
>>  #include 
>>
>>  /*
>> @@ -168,6 +168,27 @@ struct omap_sr {
>>  };
>>
>>  /**
>> + * test_cond_timeout - busy-loop, testing a condition
>> + * @cond: condition to test until it evaluates to true
>> + * @timeout: maximum number of microseconds in the timeout
>> + * @index: loop index (integer)
>> + *
>> + * Loop waiting for @cond to become true or until at least @timeout
>> + * microseconds have passed.  To use, define some integer @index in the
>> + * calling code.  After running, if @index == @timeout, then the loop has
>> + * timed out.
>> + *
>> + * Copied from omap_test_timeout */
>> +#define sr_test_cond_timeout(cond, timeout, index)           \
>> +({                                                           \
>> +     for (index = 0; index < timeout; index++) {             \
>> +             if (cond)                                       \
>> +                     break;                                  \
>> +             udelay(1);                                      \
>> +     }                                                       \
>> +})
>
> I think we can use time_after()/time_before() APIs for timeout and 
> cpu_relax() for
> tight loops instead of udelay().

cpu_relax() changes the priority everytime to low and will yield to
another thread.
Considering that we are checking the condition every microsecond does it make
some saving using cpu_relax().

>
> Regards
> AnilKumar



-- 
Regards and Thanks,
Keerthy
--
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] gpio/omap: fix incorrect initialization of omap_gpio_mod_init

2012-05-06 Thread DebBarma, Tarun Kanti
Hi,

On Sun, May 6, 2012 at 3:25 AM, Grazvydas Ignotas  wrote:
> I've noticed that current mainline enables _all_ possible GPIO
> interrupts, this patch fixes that problem.
OK, thanks.

> Also Grant doesn't seem to be on CC so might not be able to pick this up.
I have added Cc: in the patch and was expecting that would take care.
Looks like that has not happened. Anyways, I have explicitly added in the email.
--
Tarun
>
> On Mon, Apr 30, 2012 at 10:20 AM, Tarun Kanti DebBarma
>  wrote:
>> Initialization of irqenable, irqstatus registers is the common
>> operation done in this function for all OMAP platforms, viz. OMAP1,
>> OMAP2+. The latter _gpio_rmw()'s which supposedly got introduced
>> wrongly to take care of OMAP2+ platforms were overwriting initially
>> programmed OMAP1 value breaking functionality on OMAP1.
>> Somehow incorrect assumption was made that each _gpio_rmw()'s were
>> mutually exclusive. On close observation it is found that the first
>> _gpio_rmw() which is supposedly done to take care of OMAP1 platform
>> is generic enough and takes care of OMAP2+ platform as well.
>> Therefore remove the latter _gpio_rmw() to irqenable as they are
>> redundant now.
>>
>> Writing to ctrl and debounce_en registers for OMAP2+ platforms are
>> modified to match the original(pre-cleanup) code where the registers
>> are initialized with 0. In the cleanup series since we are using
>> _gpio_rmw(reg, 0, 1), instead of __raw_writel(), we are just reading
>> and writing the same values to ctrl and debounce_en. This is not an
>> issue for debounce_en register because it has 0x0 as the default value.
>> But in the case of ctrl register the default value is 0x2 (GATINGRATIO
>>  = 0x1) so that we end up writing 0x2 instead of intended 0 value.
>> Therefore changing back to __raw_writel() as this is sufficient for
>> this case besides simpler to understand.
>>
>> Also, change irqstatus initalization logic that avoids comparison
>> with bool, besides making it fit in a single line.
>>
>> Cc: sta...@vger.kernel.org
>> Cc: Tony Lindgren 
>> Cc: Kevin Hilman 
>> Cc: Santosh Shilimkar 
>> Cc: Grant Likely 
>> Reported-by: Janusz Krzysztofik 
>> Signed-off-by: Tarun Kanti DebBarma 
>> ---
>> v2:
>> Avoid irqstatus initialization sequence change.
>> Use __raw_writel() to update debounce_en and ctrl registers.
>> Update changelog to elaborate how redundant _gpio_rmw() got added.
>>
>>  drivers/gpio/gpio-omap.c |    9 +++--
>>  1 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index 59a4af1..9b71f04 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -957,18 +957,15 @@ static void omap_gpio_mod_init(struct gpio_bank *bank)
>>        }
>>
>>        _gpio_rmw(base, bank->regs->irqenable, l, bank->regs->irqenable_inv);
>> -       _gpio_rmw(base, bank->regs->irqstatus, l,
>> -                                       bank->regs->irqenable_inv == false);
>> -       _gpio_rmw(base, bank->regs->irqenable, l, bank->regs->debounce_en != 
>> 0);
>> -       _gpio_rmw(base, bank->regs->irqenable, l, bank->regs->ctrl != 0);
>> +       _gpio_rmw(base, bank->regs->irqstatus, l, 
>> !bank->regs->irqenable_inv);
>>        if (bank->regs->debounce_en)
>> -               _gpio_rmw(base, bank->regs->debounce_en, 0, 1);
>> +               __raw_writel(0, base + bank->regs->debounce_en);
>>
>>        /* Save OE default value (0x) in the context */
>>        bank->context.oe = __raw_readl(bank->base + bank->regs->direction);
>>         /* Initialize interface clk ungated, module enabled */
>>        if (bank->regs->ctrl)
>> -               _gpio_rmw(base, bank->regs->ctrl, 0, 1);
>> +               __raw_writel(0, base + bank->regs->ctrl);
>>  }
>>
>>  static __devinit void
>> --
>> 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
>
>
>
> --
> Gražvydas
--
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 12/13] ARM: OMAP5: Add the build support

2012-05-06 Thread R, Sricharan
Hi Tony,

>> -     select NEON if ARCH_OMAP3 || ARCH_OMAP4
>> +     select NEON if ARCH_OMAP3 || ARCH_OMAP4 || ARCH_OMAP5
>>       select SERIAL_OMAP
>>       select SERIAL_OMAP_CONSOLE
>>       select I2C
>
> If we add CONFIG_SOC_OMAP3PLUS and CONFIG_SOC_OMAP4PLUS, then
> this becomes more future proof with select NEON if SOC_OMAP3PLUS.
>
 ok. will change this.

>> +config ARCH_OMAP5
>> +     bool "TI OMAP5"
>> +     depends on ARCH_OMAP2PLUS
>> +     select CPU_V7
>> +     select ARM_GIC
>> +     select HAVE_SMP
>
> No need to have depends on ARCH_OMAP2PLUS, it's all inside
> if ARCH_OMAP2PLUS anyways. I removed those already once, but that
> had to be reverted because the patch was doing other things too
> that did not work out too well..
>ll
 ok, will remove this then.

>> +config MACH_OMAP5_SEVM
>> +     bool "OMAP5 sevm Board"
>> +     depends on ARCH_OMAP5
>> +
>>  config OMAP3_EMU
>>       bool "OMAP3 debugging peripherals"
>>       depends on ARCH_OMAP3
>
> No need for it here either. Actually, I think this whole chunk
> can be now left out since it's DT based?
>he
 ok, but the concern here was that without this macro
 the print from compress and subsequently early
prints appear broken.

machine_is_omap5_sevm becomes zero without this config and
machine_is_ is used by the macro _DEBUG_LL_ENTRY
uncompress.h.

Thanks,
 Sricharan
--
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 01/16] FS: Added demand paging markers to filesystem

2012-05-06 Thread Dave Chinner
On Thu, May 03, 2012 at 07:53:00PM +0530, Venkatraman S wrote:
> From: Ilan Smith 
> 
> Add attribute to identify demand paging requests.
> Mark readpages with demand paging attribute.
> 
> Signed-off-by: Ilan Smith 
> Signed-off-by: Alex Lemberg 
> Signed-off-by: Venkatraman S 
> ---
>  fs/mpage.c|2 ++
>  include/linux/bio.h   |7 +++
>  include/linux/blk_types.h |2 ++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/fs/mpage.c b/fs/mpage.c
> index 0face1c..8b144f5 100644
> --- a/fs/mpage.c
> +++ b/fs/mpage.c
> @@ -386,6 +386,8 @@ mpage_readpages(struct address_space *mapping, struct 
> list_head *pages,
>   &last_block_in_bio, &map_bh,
>   &first_logical_block,
>   get_block);
> + if (bio)
> + bio->bi_rw |= REQ_RW_DMPG;

Have you thought about the potential for DOSing a machine
with this? That is, user data reads can now preempt writes of any
kind, effectively stalling writeback and memory reclaim which will
lead to OOM situations. Or, alternatively, journal flushing will get
stalled and no new modifications can take place until the read
stream stops.

This really seems like functionality that belongs in an IO
scheduler so that write starvation can be avoided, not in high-level
data read paths where we have no clue about anything else going on
in the IO subsystem

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com
--
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 1/3] omap: mailbox: enable mailbox irq per instance

2012-05-06 Thread Ohad Ben-Cohen
Hi Juan,

On Sat, Apr 14, 2012 at 2:39 AM, Juan Gutierrez  wrote:
> The machine-specific omap2_mbox_startup is called only once
> to initialize the whole mbox module. Enabling mbox irq at
> startup is only enabling the irq of the very first mailbox
> instance created.
>
> The enable_irq function should be called every time a
> new mbox instance is created,

s/created/opened/

> in the generic platform mailbox layer.

This patch removes an omap2-specific code and then adds it to the
"generic" layer, which also deals with omap1.

Are we sure it's ok ?

OMAP1 doesn't seem to enable its irq at this point: it doesn't even
have a ->startup() handler (which actually somewhat implies it's been
anyway broken for a long time now: omap_mbox_startup() seem to
unhappily bail out if it doesn't find a machine-specific ->startup()
handler).

> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index ad32621..ebc1ba5 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -282,6 +282,8 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>                }
>                mbox->rxq = mq;
>                mq->mbox = mbox;
> +
> +               mbox->ops->enable_irq(mbox, IRQ_RX);

Might be nicer to use omap_mbox_enable_irq() here instead of reaching
out for the internal ops.

It also looks like there's a race here: omap_mbox_get() registers the
notifier_block only after omap_mbox_startup() returns, which probably
means there's a small window where an interrupt can be received
without us invoking the user's notifier callback.

This isn't related to your patch of course, but since you're touching
this area, it might be nice if you can fix it (i.e. simply by
registering the notifier_block before enabling the mbox's irq).

>        }
>        mutex_unlock(&mbox_configured_lock);
>        return 0;
> @@ -305,6 +307,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
>        mutex_lock(&mbox_configured_lock);
>
>        if (!--mbox->use_count) {
> +               mbox->ops->disable_irq(mbox, IRQ_RX);

omap_mbox_disable_irq() ?

Thanks,
Ohad.
--
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: dma: Clear status registers on enable/disable irq.

2012-05-06 Thread Jarkko Nikula
On 05/04/2012 10:39 PM, Janusz Krzysztofik wrote:
>> This seems like a nice fix to me. As it affects all omaps, I'd like to
>> see some tested-by from Janusz/Jarkko/Peter. Can you guys give it a 
> try
>> with some audio tests?
> 
> OK, I can do, but perhaps not before next Saturday, when I'm back home, 
> able to actually listen to the audio, not only watch the IRQ counters 
> rising up ;-).
> 
Ok from omap3

Tested-by: Jarkko Nikula 
--
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 2/3] OMAP4: iommu: fix irq and clock name for dsp-iommu

2012-05-06 Thread Ohad Ben-Cohen
Hi Juan,

On Sat, Apr 14, 2012 at 2:39 AM, Juan Gutierrez  wrote:
> Irq and clock names were wrong for dsp iommu configuration
> for omap4.

Looks good.

> Renamed tesla_ick to dsp_fck.

If you're resubmitting the series, please indicate here why this
change is needed (specifically let's mention commit 0e43327 "OMAP4:
clock: Fix clock names and align with hwmod names").

Thanks,
Ohad.
--
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 3/3] omap: remoteproc: add support for a boot register

2012-05-06 Thread Ohad Ben-Cohen
Hi Juan,

Thanks for the patches ! it's exciting to see that support for OMAP's
DSP is (relatively) easily achieved now. I hope your work can be a
good basis for an OMAP3 port, too.

Overall the patches look good, I have only some minor comments inline.

And - sorry for the late response. I've just left for a (somewhat
long) business trip when you posted this.

On Sat, Apr 14, 2012 at 2:39 AM, Juan Gutierrez  wrote:
> Some remote processors (like Omap4's DSP) need to explicitly
> configure a boot address in a special register or memory
> location

Let's just slightly rephrase this to prevent confusion: some remote
processors "need to have the boot address configured" in a special
register (as opposed to "need to configure").

> @@ -30,6 +30,7 @@ struct platform_device;
>  * @ops: start/stop rproc handlers
>  * @device_enable: omap-specific handler for enabling a device
>  * @device_shutdown: omap-specific handler for shutting down a device
> + * @boot_reg: physical address of the control register for storing boot 
> address
>  */
>  struct omap_rproc_pdata {
>        const char *name;
> @@ -40,6 +41,7 @@ struct omap_rproc_pdata {
>        const struct rproc_ops *ops;
>        int (*device_enable) (struct platform_device *pdev);
>        int (*device_shutdown) (struct platform_device *pdev);
> +       u32 boot_reg;

We might want to use an IORESOURCE_MEM resource instead, since we're
dealing with a platform device anyway.

The driver can then fetch the address using platform_get_resource.

> @@ -203,6 +215,9 @@ static int __devinit omap_rproc_probe(struct 
> platform_device *pdev)
>        return 0;
>
>  free_rproc:
> +       if (oproc->boot_reg)
> +               iounmap(oproc->boot_reg);
> +err_ioremap:
>        rproc_free(rproc);
>        return ret;
>  }

I tend to call those cleanup snippets with names that indicate what they do.

In this case I'd slightly prefer to keep calling the lower snippet
with "free_rproc" and just name the new snippet with something like
"unmap_bootreg". It's then a bit easier to read the code that calls
into those snippets (the reader easily know which cleanup snippet is
invoked by each 'goto').

Thanks,
Ohad.
--
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 07/13] ARM: OMAP5: l3: Add l3 error handler support for omap5.

2012-05-06 Thread R, Sricharan
Hi Tony,

>> -     if (!(cpu_is_omap44xx()))
>> +     if ((!(cpu_is_omap44xx())) && (!cpu_is_omap54xx()))
>>               return -ENODEV;
>>
>>       for (i = 0; i < L3_MODULES; i++) {
>
> Isn't there some unnecessary parens here?

 You mean in this above for loop?.
 There are multiple statements .

Thanks,
 Sricharan
--
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 02/13] ARM: OMAP5: Add minimal support for OMAP5430 SOC

2012-05-06 Thread R, Sricharan
Hi Tony,
[snip]
> OK this seems to be based on Santosh' Makefile cleanup.
>
 yes..

>>
>> -#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
>> +#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4) || \
>> +                             defined(CONFIG_ARCH_OMAP5)
>>
>
> How about we add CONFIG_SOC_OMAP3PLUS in the clean-up series?
> Then this becomes just:
>
> #ifdef CONFIG_SOC_OMAP3PLUS
>
 Ok, thanks for the example later. I will do a cleanup patch and
rebase this one.


>> -# ifdef CONFIG_ARCH_OMAP4
>> +#if defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_ARCH_OMAP5)
>>  extern int omap4_cminst_wait_module_idle(u8 part, u16 inst, s16 cdoffs,
>>                                        u16 clkctrl_offs);
>
> And this would be:
>
> #ifdef CONFIG_SOC_OMAP4PLUS
>
 ok..

>
> Also, please change the whole series to use CONFIG_SOC_OMAP5 instead
> of CONFIG_ARCH_OMAP5. CONFIG_ARCH_OMAP stuff will go away except
> for CONFIG_ARCH_OMAP2PLUS. Sorry forgot to mention that earlier.
>
 ok. sure. will change this.


>> -#if defined(CONFIG_ARCH_OMAP4) && !(defined(CONFIG_ARCH_OMAP2) ||    \
>> -                                     defined(CONFIG_ARCH_OMAP3))
>> +#if (defined(CONFIG_ARCH_OMAP5) || defined(CONFIG_ARCH_OMAP4)) && \
>> +             !(defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3))
>> +
>>  static inline u32 omap2_prm_read_mod_reg(s16 module, u16 idx)
>>  {
>>       WARN(1, "prm: omap2xxx/omap3xxx specific function and "
>
> Maybe these functions could be just set up as __weak to avoid the
> ifdeffery?
>
 sorry to understand,
 you mean make this weak and have a strong override for OMAP2 ?

>>  #define CK_443X              (1 << 11)
>>  #define CK_TI816X    (1 << 12)
>>  #define CK_446X              (1 << 13)
>> +#define CK_54XX              (1 << 14)
>>  #define CK_1710              (1 << 15)       /* 1710 extra for rate 
>> selection */
>>
>>
>
> Are we going to have to patch tons of existing clocks just to add
> CK_54XX? If so, we should init clocks using SoC specific lists instead.
>

>> diff --git a/arch/arm/plat-omap/include/plat/clock.h 
>> b/arch/arm/plat-omap/include/plat/clock.h
>> index d0ef57c..41858f4 100644
>> --- a/arch/arm/plat-omap/include/plat/clock.h
>> +++ b/arch/arm/plat-omap/include/plat/clock.h
>> @@ -61,6 +61,7 @@ struct clkops {
>>  #define RATE_IN_4460         (1 << 7)
>>  #define RATE_IN_AM33XX               (1 << 8)
>>  #define RATE_IN_TI814X               (1 << 9)
>> +#define RATE_IN_54XX         (1 << 10)
>
> This too may have similar issues, but I guess that's really a different
> patch series to sort out..
>
 ok. agree

>> -#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
>> +#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4) \
>> +             || defined(CONFIG_ARCH_OMAP5)
>>       void __iomem            *autoidle_reg;
>>       void __iomem            *idlest_reg;
>>       u32                     autoidle_mask;
>
>
> #ifdef CONFIG_SOC_OMAP3PLUS could be used here too
>
 ok..

Thanks,
 Sricharan
--
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