* RE: [PATCH 1/1] DSPBRIDGE Fix for image autoload
From: Menon, Nishanth @ 2009-03-12 7:52 UTC (permalink / raw)
To: Gupta, Ramesh, linux-omap@vger.kernel.org
Cc: Kanigeri, Hari, Guzman Lugo, Fernando
In-Reply-To: <EAF47CD23C76F840A9E7FCE10091EFAB02A8F2BF1D@dbde02.ent.ti.com>
> -----Original Message-----
> From: Menon, Nishanth
> Sent: Thursday, March 12, 2009 9:39 AM
> To: Gupta, Ramesh; linux-omap@vger.kernel.org
> Cc: Kanigeri, Hari; Guzman Lugo, Fernando
> Subject: RE: [PATCH 1/1] DSPBRIDGE Fix for image autoload
>
> > -----Original Message-----
> > From: Gupta, Ramesh
> > Sent: Thursday, March 12, 2009 8:01 AM
> > To: Menon, Nishanth; linux-omap@vger.kernel.org
> > Cc: Kanigeri, Hari; Guzman Lugo, Fernando
> > Subject: RE: [PATCH 1/1] DSPBRIDGE Fix for image autoload
> >
> > I have tried this on linux-omap pm branch . This can be applied on top
> of
> > my DVFS
> > Patches earlier.
> >
> Your patch does not apply clean :(
> $ wget -O ramesh.auto.patch "http://marc.info/?l=linux-
> omap&m=123683598308417&q=raw"
> $ vim ramesh.auto.patch (edited to remove the linux-omap trailer)
> $ patch -p1<./ramesh.auto.patch --dry-run
> patching file drivers/dsp/bridge/rmgr/drv_interface.c
> Hunk #1 succeeded at 400 (offset -5 lines).
> Hunk #2 succeeded at 408 (offset -5 lines).
> Hunk #3 succeeded at 450 with fuzz 1 (offset -5 lines).
> Hunk #4 succeeded at 470 (offset -5 lines).
> Hunk #5 succeeded at 655 (offset -5 lines).
>
> Could you rebase your patch against the following:
> git clone git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap-
> 2.6.git
> cd linux-omap-2.6.git
> git checkout -b pm --track origin/pm
> git fetch git://gitorious.org/lk/mainline.git tidspbridge-pm:tidspbridge-
> pm
> git checkout tidspbridge-pm
> git merge pm
And the patch does not work :(
# insmod ./bridgedriver.ko phys_mempool_base=0x87000000 phys_mempool_size=0x600000 base_img=dsp/baseimage.dof
<4>------------[ cut here ]------------
------------[ cut here ]------------
<4>WARNING: at arch/arm/plat-omap/omap-pm-srf.c:241 omap_pm_cpu_set_freq+0x20/0x44()
WARNING: at arch/arm/plat-omap/omap-pm-srf.c:241 omap_pm_cpu_set_freq+0x20/0x44()
Modules linked in:Modules linked in: bridgedriver(+) bridgedriver(+) [last unloaded: bridgedriver] [last unloaded: bridgedriver]
[<c0031e20>] [<c0031e20>] (dump_stack+0x0/0x14) (dump_stack+0x0/0x14) from [<c00562c8>] from [<c00562c8>] (warn_on_slowpath+0x4c/0x68)
(warn_on_slowpath+0x4c/0x68)
[<c005627c>] [<c005627c>] (warn_on_slowpath+0x0/0x68) (warn_on_slowpath+0x0/0x68) from [<c0043c44>] from [<c0043c44>] (omap_pm_cpu_set_freq+0x20/0x44)
(omap_pm_cpu_set_freq+0x20/0x44)
r6:00008000 r6:00008000 r5:80008010 r5:80008010 r4:c3dc5b08 r4:c3dc5b08
[<c0043c24>] [<c0043c24>] (omap_pm_cpu_set_freq+0x0/0x44) (omap_pm_cpu_set_freq+0x0/0x44) from [<bf046544>] from [<bf046544>] (PROC_Load+0x2c8/0x40c [bridgedriver])
(PROC_Load+0x2c8/0x40c [bridgedriver])
[<bf04627c>] [<bf04627c>] (PROC_Load+0x0/0x40c [bridgedriver]) (PROC_Load+0x0/0x40c [bridgedriver]) from [<bf046ac0>] from [<bf046ac0>] (PROC_AutoStart+0x174/0x1c4 [bridgedriver)
(PROC_AutoStart+0x174/0x1c4 [bridgedriver])
[<bf04694c>] [<bf04694c>] (PROC_AutoStart+0x0/0x1c4 [bridgedriver]) (PROC_AutoStart+0x0/0x1c4 [bridgedriver]) from [<bf03add0>] from [<bf03add0>] (WCD_InitComplete2+0x58/0x8c [b)
(WCD_InitComplete2+0x58/0x8c [bridgedriver])
r7:00000000 r7:00000000 r6:c3dc5dd4 r6:c3dc5dd4 r5:00008000 r5:00008000 r4:c3981600 r4:c3981600
[<bf03ad78>] [<bf03ad78>] (WCD_InitComplete2+0x0/0x8c [bridgedriver]) (WCD_InitComplete2+0x0/0x8c [bridgedriver]) from [<bf0487e0>] from [<bf0487e0>] (DSP_Init+0xd8/0xf0 [bridge)
(DSP_Init+0xd8/0xf0 [bridgedriver])
r5:c3dc5c99 r5:c3dc5c99 r4:00008000 r4:00008000
[<bf048708>] [<bf048708>] (DSP_Init+0x0/0xf0 [bridgedriver]) (DSP_Init+0x0/0xf0 [bridgedriver]) from [<bf05d33c>] from [<bf05d33c>] (bridge_init+0x33c/0x3ec [bridgedriver])
(bridge_init+0x33c/0x3ec [bridgedriver])
r6:bf05a288 r6:bf05a288 r5:bf05ab88 r5:bf05ab88 r4:bf05ab88 r4:bf05ab88
[<bf05d000>] [<bf05d000>] (bridge_init+0x0/0x3ec [bridgedriver]) (bridge_init+0x0/0x3ec [bridgedriver]) from [<c002d2d4>] from [<c002d2d4>] (do_one_initcall+0x64/0x198)
(do_one_initcall+0x64/0x198)
r8:c002df28 r8:c002df28 r7:00000000 r7:00000000 r6:4023a000 r6:4023a000 r5:bf05a520 r5:bf05a520 r4:c03aa340 r4:c03aa340
[<c002d270>] [<c002d270>] (do_one_initcall+0x0/0x198) (do_one_initcall+0x0/0x198) from [<c0078cd4>] from [<c0078cd4>] (sys_init_module+0x98/0x188)
(sys_init_module+0x98/0x188)
[<c0078c3c>] [<c0078c3c>] (sys_init_module+0x0/0x188) (sys_init_module+0x0/0x188) from [<c002dd80>] from [<c002dd80>] (ret_fast_syscall+0x0/0x2c)
(ret_fast_syscall+0x0/0x2c)
r7:00000080 r7:00000080 r6:00000000 r6:00000000 r5:0000000b r5:0000000b r4:00000000 r4:00000000
<4>---[ end trace 7a555f9f02586741 ]---
---[ end trace 7a555f9f02586741 ]---
<4>------------[ cut here ]------------
------------[ cut here ]------------
<4>WARNING: at arch/arm/plat-omap/omap-pm-srf.c:241 omap_pm_cpu_set_freq+0x20/0x44()
WARNING: at arch/arm/plat-omap/omap-pm-srf.c:241 omap_pm_cpu_set_freq+0x20/0x44()
Modules linked in:Modules linked in: bridgedriver(+) bridgedriver(+) [last unloaded: bridgedriver] [last unloaded: bridgedriver]
[<c0031e20>] [<c0031e20>] (dump_stack+0x0/0x14) (dump_stack+0x0/0x14) from [<c00562c8>] from [<c00562c8>] (warn_on_slowpath+0x4c/0x68)
(warn_on_slowpath+0x4c/0x68)
[<c005627c>] [<c005627c>] (warn_on_slowpath+0x0/0x68) (warn_on_slowpath+0x0/0x68) from [<c0043c44>] from [<c0043c44>] (omap_pm_cpu_set_freq+0x20/0x44)
(omap_pm_cpu_set_freq+0x20/0x44)
r6:00008000 r6:00008000 r5:80008010 r5:80008010 r4:00008000 r4:00008000
[<c0043c24>] [<c0043c24>] (omap_pm_cpu_set_freq+0x0/0x44) (omap_pm_cpu_set_freq+0x0/0x44) from [<bf046580>] from [<bf046580>] (PROC_Load+0x304/0x40c [bridgedriver])
(PROC_Load+0x304/0x40c [bridgedriver])
[<bf04627c>] [<bf04627c>] (PROC_Load+0x0/0x40c [bridgedriver]) (PROC_Load+0x0/0x40c [bridgedriver]) from [<bf046ac0>] from [<bf046ac0>] (PROC_AutoStart+0x174/0x1c4 [bridgedriver)
(PROC_AutoStart+0x174/0x1c4 [bridgedriver])
[<bf04694c>] [<bf04694c>] (PROC_AutoStart+0x0/0x1c4 [bridgedriver]) (PROC_AutoStart+0x0/0x1c4 [bridgedriver]) from [<bf03add0>] from [<bf03add0>] (WCD_InitComplete2+0x58/0x8c [b)
(WCD_InitComplete2+0x58/0x8c [bridgedriver])
r7:00000000 r7:00000000 r6:c3dc5dd4 r6:c3dc5dd4 r5:00008000 r5:00008000 r4:c3981600 r4:c3981600
[<bf03ad78>] [<bf03ad78>] (WCD_InitComplete2+0x0/0x8c [bridgedriver]) (WCD_InitComplete2+0x0/0x8c [bridgedriver]) from [<bf0487e0>] from [<bf0487e0>] (DSP_Init+0xd8/0xf0 [bridge)
(DSP_Init+0xd8/0xf0 [bridgedriver])
r5:c3dc5c99 r5:c3dc5c99 r4:00008000 r4:00008000
[<bf048708>] [<bf048708>] (DSP_Init+0x0/0xf0 [bridgedriver]) (DSP_Init+0x0/0xf0 [bridgedriver]) from [<bf05d33c>] from [<bf05d33c>] (bridge_init+0x33c/0x3ec [bridgedriver])
(bridge_init+0x33c/0x3ec [bridgedriver])
r6:bf05a288 r6:bf05a288 r5:bf05ab88 r5:bf05ab88 r4:bf05ab88 r4:bf05ab88
[<bf05d000>] [<bf05d000>] (bridge_init+0x0/0x3ec [bridgedriver]) (bridge_init+0x0/0x3ec [bridgedriver]) from [<c002d2d4>] from [<c002d2d4>] (do_one_initcall+0x64/0x198)
(do_one_initcall+0x64/0x198)
r8:c002df28 r8:c002df28 r7:00000000 r7:00000000 r6:4023a000 r6:4023a000 r5:bf05a520 r5:bf05a520 r4:c03aa340 r4:c03aa340
[<c002d270>] [<c002d270>] (do_one_initcall+0x0/0x198) (do_one_initcall+0x0/0x198) from [<c0078cd4>] from [<c0078cd4>] (sys_init_module+0x98/0x188)
(sys_init_module+0x98/0x188)
[<c0078c3c>] [<c0078c3c>] (sys_init_module+0x0/0x188) (sys_init_module+0x0/0x188) from [<c002dd80>] from [<c002dd80>] (ret_fast_syscall+0x0/0x2c)
(ret_fast_syscall+0x0/0x2c)
r7:00000080 r7:00000080 r6:00000000 r6:00000000 r5:0000000b r5:0000000b r4:00000000 r4:00000000
<4>---[ end trace 7a555f9f02586741 ]---
---[ end trace 7a555f9f02586741 ]---
# <4>SR1: VDD autocomp is not active
SR1: VDD autocomp is not active
<4>SR1: VDD autocomp is not active
SR1: VDD autocomp is not active
Regards,
Nishanth Menon
^ permalink raw reply
* RE: [PATCH 1/1] DSPBRIDGE Fix for image autoload
From: Menon, Nishanth @ 2009-03-12 7:39 UTC (permalink / raw)
To: Gupta, Ramesh, linux-omap@vger.kernel.org
Cc: Kanigeri, Hari, Guzman Lugo, Fernando
In-Reply-To: <EAF47CD23C76F840A9E7FCE10091EFAB02A8F2BF1D@dbde02.ent.ti.com>
> -----Original Message-----
> From: Gupta, Ramesh
> Sent: Thursday, March 12, 2009 8:01 AM
> To: Menon, Nishanth; linux-omap@vger.kernel.org
> Cc: Kanigeri, Hari; Guzman Lugo, Fernando
> Subject: RE: [PATCH 1/1] DSPBRIDGE Fix for image autoload
>
> I have tried this on linux-omap pm branch . This can be applied on top of
> my DVFS
> Patches earlier.
>
Your patch does not apply clean :(
$ wget -O ramesh.auto.patch "http://marc.info/?l=linux-omap&m=123683598308417&q=raw"
$ vim ramesh.auto.patch (edited to remove the linux-omap trailer)
$ patch -p1<./ramesh.auto.patch --dry-run
patching file drivers/dsp/bridge/rmgr/drv_interface.c
Hunk #1 succeeded at 400 (offset -5 lines).
Hunk #2 succeeded at 408 (offset -5 lines).
Hunk #3 succeeded at 450 with fuzz 1 (offset -5 lines).
Hunk #4 succeeded at 470 (offset -5 lines).
Hunk #5 succeeded at 655 (offset -5 lines).
Could you rebase your patch against the following:
git clone git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap-2.6.git
cd linux-omap-2.6.git
git checkout -b pm --track origin/pm
git fetch git://gitorious.org/lk/mainline.git tidspbridge-pm:tidspbridge-pm
git checkout tidspbridge-pm
git merge pm
Regards,
Nishanth Menon
^ permalink raw reply
* RE: [PATCH] OMAP:clock: missing list_del for clk_notifier_unregister
From: Menon, Nishanth @ 2009-03-12 7:33 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap@vger.kernel.org, Woodruff, Richard, Nayak, Rajendra,
Gupta, Ramesh, Palande Ameya
In-Reply-To: <alpine.DEB.2.00.0903120128570.26959@utopia.booyaka.com>
> -----Original Message-----
> From: Paul Walmsley [mailto:paul@pwsan.com]
> Sent: Thursday, March 12, 2009 9:31 AM
> To: Menon, Nishanth
> Cc: linux-omap@vger.kernel.org; Woodruff, Richard; Nayak, Rajendra; Gupta,
> Ramesh; Palande Ameya
> Subject: Re: [PATCH] OMAP:clock: missing list_del for
> clk_notifier_unregister
> Just to clarify, this is currently against the PM branch. There's a new
Yes. This is against the PM branch
> version of the clock notifier patch coming out soon against l-o; will roll
> this fix in.
>
Thanks.
Regards,
Nishanth Menon
^ permalink raw reply
* Re: [PATCH] OMAP:clock: missing list_del for clk_notifier_unregister
From: Paul Walmsley @ 2009-03-12 7:30 UTC (permalink / raw)
To: Nishanth Menon
Cc: linux-omap, Richard Woodruff, Rajendra Nayak, Ramesh Gupta,
Palande Ameya
In-Reply-To: <1236790153-9324-1-git-send-email-nm@ti.com>
Hi,
On Wed, 11 Mar 2009, Nishanth Menon wrote:
> clk_notifier_unregister should clean the list before
> freeing clock notifier, else clk_notifier_list is
> filled with dangling pointers
>
> Issue seen while repetative loading/unloading of bridgedriver
>
> Ref: http://marc.info/?t=123678326300002&r=1&w=2
>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> arch/arm/plat-omap/clock.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
> index c8d9e96..523d1b0 100644
> --- a/arch/arm/plat-omap/clock.c
> +++ b/arch/arm/plat-omap/clock.c
> @@ -725,8 +725,11 @@ int clk_notifier_unregister(struct clk *clk, struct notifier_block *nb)
> * XXX ugh, layering violation. there should be some
> * support in the notifier code for this.
> */
> - if (!cn->notifier_head.head)
> + if (!cn->notifier_head.head) {
> + /* Free up my clock node too */
> + list_del(&cn->node);
> kfree(cn);
> + }
>
> } else {
> r = -ENOENT;
> --
> 1.5.4.3
>
Thanks, this looks good.
Just to clarify, this is currently against the PM branch. There's a new
version of the clock notifier patch coming out soon against l-o; will roll
this fix in.
- Paul
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-omap: Call request_irq with IRQF_DISABLED
From: Paul Walmsley @ 2009-03-12 6:46 UTC (permalink / raw)
To: Kauppi Ari (EXT-Ixonos/Oulu)
Cc: Ben Dooks, ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org,
linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1236839178.6478.40.camel@kauppi-desktop>
Hello Ari,
On Thu, 12 Mar 2009, Kauppi Ari (EXT-Ixonos/Oulu) wrote:
> I was seeing some Spurious irq's for the I2C1 (IRQ 56).
>
> I'm aware of flushing posted write and the very first thing I tried was
> to use read-after-write for OMAP_I2C_STAT_REG (in all applicable
> places). However, it didn't make any difference.
Strange.
> Applying Richard Woodruff's patch (mentioned earlier in thread) that
> disables dev->b_hw hack for 3430 (STT/STP bits written together) and
> double clears ARDY fixed the spurious IRQ issues for I2C1.
>
> However, with the STT/STP+ARDY patch I was seeing Spurious interrupts
> all over the place and the IRQF_DISABLED in i2c-omap seemed to tame them
> quite well. I do agree that my approach might not be the proper one in
> long term.
Could you clarify this? Do Richard's patches fix the spurious IRQs, or
cause spurious interrupts to appear?
regards,
- Paul
^ permalink raw reply
* RE: [PATCHv2] PM : cpuidle - Update statistics for correct state
From: Premi, Sanjeev @ 2009-03-12 6:43 UTC (permalink / raw)
To: Kevin Hilman; +Cc: Högander Jouni, linux-omap@vger.kernel.org
In-Reply-To: <87r613k3tt.fsf@deeprootsystems.com>
> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> Sent: Thursday, March 12, 2009 5:31 AM
> To: Premi, Sanjeev
> Cc: Högander Jouni; linux-omap@vger.kernel.org
> Subject: Re: [PATCHv2] PM : cpuidle - Update statistics for
> correct state
>
> "Premi, Sanjeev" <premi@ti.com> writes:
>
> >>
> >> I would gladly take a patch which uses the 'valid' field
> for this and
> >> then the enter hook whould drop to the next lower valid
> state if the
> >> state requested is not valid.
> >>
> >> Kevin
> >
> > [sp] I have not yet tested it (working offline for sometime).
> > But, wanted to share the changes for an early review.
>
> Thanks, some comments below.
>
> > Initially, I was trying see if the CPUIDLE framework could
> > use ".valid" as an additional argument in cpuidle_state.
> > But, may be that's for later...
>
> Yeah, I looked at that too, but it currently doesn't have a
> concept of valid states, so for now I recommend we implement
> that in the OMAP-specific code as you have done.
>
> > diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
> > b/arch/arm/mach-omap2/cpuidle34xx index c25158c..9493553 100644
> > --- a/arch/arm/mach-omap2/cpuidle34xx.c
> > +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> > @@ -88,16 +88,19 @@ static int omap3_enter_idle(struct
> cpuidle_device *dev,
> > goto return_sleep_time;
> >
> > /*
> > + * Check if the chosen idle state is valid.
> > + * If no, drop down to a lower valid state. Expects
> the lowest
> > + * state will always be active.
> > */
> > + if (!cx->valid) {
> > + for (idx = (cx->type - 1); idx == 1; idx--) {
> ^^^^^^^^
>
> I think you mean idx >= 1 here.
[sp] Yes. A typo. Still haven't tried this patch myself.
>
> Also, while you're working on this, could you fix this up so
> the omap3_power_states[] array is zero based insted of
> 1-based, it would make this code and the other code walking
> this array easier to follow.
>
> That means defining OMAP_STATE_C1 = 0 and so on.
[sp] Definitely.
>
> > + if (omap3_power_states[idx].valid)
> > + break;
> > }
> > + state = &(dev->states[idx]);
> > + dev->last_state = state ;
> > +
> > + cx = cpuidle_get_statedata(state);
> > }
> >
> > current_cx_state = *cx;
> > @@ -150,6 +153,26 @@ static int omap3_enter_idle_bm(struct
> cpuidle_device *dev,
> > return omap3_enter_idle(dev, new_state); }
> >
> > +/**
> > + * omap3_toggle_off_states - Enable / Disable validity of
> idle states
> > + * @flag: Enable/ Disable support for OFF mode
> > + *
> > + * Called as result of change to "enable_off_mode".
> > + */
> > +void omap3_toggle_off_states(unsigned short flag) {
>
> How about calling this omap3_cpuidle_update_states() and
> taking no arguments. Rather than the 'flag' argument,
> internally it just checks the global 'enable_off_mode.'
>
> This allows for potential further expansion down the road of
> other reasons to update CPUidle valid states. For example,
> we've talked about updating the CPUidle state latencies on
> the fly depending on various other chip settings.
>
> > + if (flag) {
> > + omap3_power_states[OMAP3_STATE_C3].valid = 1;
> > + omap3_power_states[OMAP3_STATE_C5].valid = 1;
> > + omap3_power_states[OMAP3_STATE_C6].valid = 1;
> > + }
> > + else {
> > + omap3_power_states[OMAP3_STATE_C3].valid = 0;
> > + omap3_power_states[OMAP3_STATE_C5].valid = 0;
> > + omap3_power_states[OMAP3_STATE_C6].valid = 0;
> > + }
> > +}
> > +
>
> Rather than set set specific OMAP3_STATE_Cx values, it would
> be better to just walk the array, and check for
> [mpu|core]_state = PWRDM_POWER_OFF.
> If the state has either set, then update the valid flag.
>
> > DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
> >
> > /* omap3_init_power_states - Initialises the OMAP3
> specific C states.
> > diff --git a/arch/arm/mach-omap2/pm.c
> b/arch/arm/mach-omap2/pm.c index
> > 61c6dfb..6785850 100644
> > --- a/arch/arm/mach-omap2/pm.c
> > +++ b/arch/arm/mach-omap2/pm.c
> > @@ -47,6 +47,8 @@ atomic_t sleep_block = ATOMIC_INIT(0);
> static int
> > vdd1_locked; static int vdd2_locked;
> >
> > +extern void omap3_toggle_off_states(unsigned short);
> > +
>
> Move the definition into pm.h as
> omap3_cpuidle_update_states(void) as described above.
>
> > static ssize_t idle_show(struct kobject *, struct
> kobj_attribute *,
> > char *); static ssize_t idle_store(struct kobject *k,
> struct kobj_attribute *,
> > const char *buf, size_t n); @@
> -112,6 +114,8
> > @@ static ssize_t idle_store(struct kobject *kobj, struct kobj_
> > } else if (attr == &enable_off_mode_attr) {
> > enable_off_mode = value;
> > omap3_pm_off_mode_enable(enable_off_mode);
> > + if (cpu_is_omap34xx())
> > + omap3_toggle_off_states(value);
>
> Then, don't modify pm.c, rather just call
> omap3_cpuidle_update_states() from omap3_pm_off_mode_enable().
>
> Kevin
>
> --
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-omap: Call request_irq with IRQF_DISABLED
From: Kauppi Ari (EXT-Ixonos/Oulu) @ 2009-03-12 6:26 UTC (permalink / raw)
To: ext Paul Walmsley
Cc: Ben Dooks, ben-linux@fluff.org, linux-omap@vger.kernel.org,
linux-i2c@vger.kernel.org
In-Reply-To: <alpine.DEB.2.00.0903111803170.26959@utopia.booyaka.com>
On Thu, 2009-03-12 at 01:04 +0100, ext Paul Walmsley wrote:
Hi Paul,
> On Wed, 11 Mar 2009, Paul Walmsley wrote:
>
> > > On Fri, Mar 06, 2009 at 03:34:54PM +0200, Ari Kauppi wrote:
> > > > I have observed some Spurious IRQ's for I2C1 when all kernel hacking options
> > > > (and thus LOCKDEP) are disabled.
> >
> > Ari, are you seeing "Spurious irq XX: XXXXXXXX, please flush posted write
> > for irq" messages? If so, the correct fix for this is to read from the
> > device interrupt status register immediately after writing to it. This
> > forces the ARM to wait until the write to the device is complete. Ari,
> > could you make this change to i2c-omap.c:omap_i2c_isr() instead, and test
> > whether this fixes the problem?
> >
> > + u32 tmp;
> >
> > ...
> >
> > omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, stat);
> > + tmp = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); /* OCP barrier */
>
> You'll also want to make a similar change in omap_i2c_ack_stat(), to add a
> read immediately after that write.
I was seeing some Spurious irq's for the I2C1 (IRQ 56).
I'm aware of flushing posted write and the very first thing I tried was
to use read-after-write for OMAP_I2C_STAT_REG (in all applicable
places). However, it didn't make any difference.
Applying Richard Woodruff's patch (mentioned earlier in thread) that
disables dev->b_hw hack for 3430 (STT/STP bits written together) and
double clears ARDY fixed the spurious IRQ issues for I2C1.
However, with the STT/STP+ARDY patch I was seeing Spurious interrupts
all over the place and the IRQF_DISABLED in i2c-omap seemed to tame them
quite well. I do agree that my approach might not be the proper one in
long term.
Best regards,
--
Ari
^ permalink raw reply
* RE: [PATCH 1/1] DSPBRIDGE Fix for image autoload
From: Gupta, Ramesh @ 2009-03-12 6:00 UTC (permalink / raw)
To: Menon, Nishanth, linux-omap@vger.kernel.org
Cc: Kanigeri, Hari, Guzman Lugo, Fernando
In-Reply-To: <7A436F7769CA33409C6B44B358BFFF0CFF51DBBC@dlee02.ent.ti.com>
Nishant,
> -----Original Message-----
> From: Menon, Nishanth
> Sent: Thursday, March 12, 2009 11:19 AM
> To: Gupta, Ramesh; linux-omap@vger.kernel.org
> Cc: Kanigeri, Hari; Guzman Lugo, Fernando
> Subject: RE: [PATCH 1/1] DSPBRIDGE Fix for image autoload
>
> Ramesh,
> > -----Original Message-----
> > From: Gupta, Ramesh
> > Sent: Thursday, March 12, 2009 7:32 AM
> > To: linux-omap@vger.kernel.org
> > Cc: Menon, Nishanth; Kanigeri, Hari; Guzman Lugo, Fernando
> > Subject: [PATCH 1/1] DSPBRIDGE Fix for image autoload
> >
> > From fbbf5c9c308c2e1e90e70c57a48798c5d05a6b1d Mon Sep 17
> 00:00:00 2001
> > From: Ramesh Gupta <x0023949@ti.com>
> > Date: Thu, 12 Mar 2009 10:48:08 +0530
> > Subject: [PATCH 1/1] DSPBRIDGE Fix for image autoload.
> >
> > This fixes the issue with the image autoloading while
> installing the
> > bridgedriver with install param base_img
> What is your code base? Omapzoom or gitorious? Could you send
> us the link to the git log of the branch you are sending this for?
I have tried this on linux-omap pm branch . This can be applied on top of my DVFS
Patches earlier.
My git log shows
commit fbbf5c9c308c2e1e90e70c57a48798c5d05a6b1d
Author: Ramesh Gupta <x0023949@linuxomap.(none)>
Date: Thu Mar 12 10:48:08 2009 +0530
DSPBRIDGE Fix for image autoload.
This fixes the issue with the image autoloading while
installing the bridgedriver with install param base_img
Signed-off-by: Ramesh Gupta G <grgupta@ti.com>
commit a384d11047130358223a24080d58e57dcef92550
Author: Ramesh Gupta <x0023949@linuxomap.(none)>
Date: Wed Mar 11 19:34:43 2009 +0530
DVFS OFF MODE Support.
commit 4f422d583e2e233c19eb20754b8cfad6ed9e460a
Author: Nayak, Rajendra <rnayak@ti.com>
Date: Fri Feb 13 11:30:57 2009 +0530
OMAP3: PM: Update voltage levels for OPP1/2 on VDD1/2
This patch updates the voltage levels for VDD1 OPP1/2 and
VDD2 OPP1/2 according to the latest operating condition
addendum for 3430.
The new voltage levels at various OPP's for VDD1/2 are as below
VDD1 OPP1 0.975v
VDD1 OPP2 1.050v
VDD1 OPP3 1.200v
VDD1 OPP4 1.270v
VDD1 OPP5 1.350v
VDD2 OPP1 0.975v
VDD2 OPP2 1.050v
VDD2 OPP3 1.150v
The patch applies on the latest pm head and is validated on 3430SDP.
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
commit 65d02c2c7fe676369b7162459feec60268c7f4ba
Author: Kevin Hilman <khilman@deeprootsystems.com>
Date: Fri Feb 13 11:07:18 2009 -0800
OMAP3: PM: fix compile warning when !CONFIG_SUSPEND
Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
Please let me know if you see any issues in applying this.
Regards
Ramesh Gupta G
^ permalink raw reply
* RE: [PATCH 1/1] DSPBRIDGE Fix for image autoload
From: Menon, Nishanth @ 2009-03-12 5:48 UTC (permalink / raw)
To: Gupta, Ramesh, linux-omap@vger.kernel.org
Cc: Kanigeri, Hari, Guzman Lugo, Fernando
In-Reply-To: <EAF47CD23C76F840A9E7FCE10091EFAB02A8F2BED3@dbde02.ent.ti.com>
Ramesh,
> -----Original Message-----
> From: Gupta, Ramesh
> Sent: Thursday, March 12, 2009 7:32 AM
> To: linux-omap@vger.kernel.org
> Cc: Menon, Nishanth; Kanigeri, Hari; Guzman Lugo, Fernando
> Subject: [PATCH 1/1] DSPBRIDGE Fix for image autoload
>
> From fbbf5c9c308c2e1e90e70c57a48798c5d05a6b1d Mon Sep 17 00:00:00 2001
> From: Ramesh Gupta <x0023949@ti.com>
> Date: Thu, 12 Mar 2009 10:48:08 +0530
> Subject: [PATCH 1/1] DSPBRIDGE Fix for image autoload.
>
> This fixes the issue with the image autoloading while
> installing the bridgedriver with install param base_img
What is your code base? Omapzoom or gitorious? Could you send us the link to the git log of the branch you are sending this for?
Regards,
Nishanth Menon
^ permalink raw reply
* [PATCH 1/1] DSPBRIDGE Fix for image autoload
From: Gupta, Ramesh @ 2009-03-12 5:32 UTC (permalink / raw)
To: linux-omap@vger.kernel.org
Cc: Menon, Nishanth, Kanigeri, Hari, Guzman Lugo, Fernando
>From fbbf5c9c308c2e1e90e70c57a48798c5d05a6b1d Mon Sep 17 00:00:00 2001
From: Ramesh Gupta <x0023949@ti.com>
Date: Thu, 12 Mar 2009 10:48:08 +0530
Subject: [PATCH 1/1] DSPBRIDGE Fix for image autoload.
This fixes the issue with the image autoloading while
installing the bridgedriver with install param base_img
Signed-off-by: Ramesh Gupta G <grgupta@ti.com>
---
drivers/dsp/bridge/rmgr/drv_interface.c | 28 ++++++++++++++--------------
1 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c b/drivers/dsp/bridge/rmgr/drv_interface.c
index 9c82ef4..c5c9ee5 100755
--- a/drivers/dsp/bridge/rmgr/drv_interface.c
+++ b/drivers/dsp/bridge/rmgr/drv_interface.c
@@ -405,7 +405,7 @@ static int __init bridge_init(void)
REG_SetValue(NULL, NULL, AUTOSTART, REG_DWORD, (u8 *)&temp,
sizeof(temp));
REG_SetValue(NULL, NULL, DEFEXEC, REG_SZ, (u8 *)base_img,
- strlen(base_img) + 1);
+ strlen(base_img) + 1);
} else {
temp = false;
REG_SetValue(NULL, NULL, AUTOSTART, REG_DWORD, (u8 *)&temp,
@@ -413,7 +413,7 @@ static int __init bridge_init(void)
REG_SetValue(NULL, NULL, DEFEXEC, REG_SZ, (u8 *) "\0", (u32)2);
}
REG_SetValue(NULL, NULL, NUMPROCS, REG_SZ, (u8 *) num_procs,
- strlen(num_procs) + 1);
+ strlen(num_procs) + 1);
if (shm_size >= 0x10000) { /* 64 KB */
initStatus = REG_SetValue(NULL, NULL, SHMSIZE, REG_DWORD,
@@ -455,15 +455,6 @@ static int __init bridge_init(void)
sizeof(tc_wordswapon));
}
if (DSP_SUCCEEDED(initStatus)) {
- driverContext = DSP_Init(&initStatus);
- if (DSP_FAILED(initStatus)) {
- status = -1;
- GT_0trace(driverTrace, GT_7CLASS,
- "DSP/BIOS Bridge initialization Failed\n");
- } else {
- GT_0trace(driverTrace, GT_5CLASS,
- "DSP/BIOS Bridge driver loaded\n");
- }
#ifdef CONFIG_BRIDGE_DVFS
for (i = 0; i < 5; i++)
pdata->mpu_speed[i] = vdd1_rate_table_bridge[i].rate;
@@ -484,6 +475,15 @@ static int __init bridge_init(void)
"clk_notifier_register FAIL for iva2_ck \n");
}
#endif
+ driverContext = DSP_Init(&initStatus);
+ if (DSP_FAILED(initStatus)) {
+ status = -1;
+ GT_0trace(driverTrace, GT_7CLASS,
+ "DSP/BIOS Bridge initialization Failed\n");
+ } else {
+ GT_0trace(driverTrace, GT_5CLASS,
+ "DSP/BIOS Bridge driver loaded\n");
+ }
}
DBC_Assert(status == 0);
@@ -660,11 +660,11 @@ func_cont:
(struct DRV_OBJECT *)hDrvObject, &pPctxt);
if (pPctxt != NULL) {
- /* Return PID instead of process handle */
- hProcess = current->pid;
+ /* Return PID instead of process handle */
+ hProcess = current->pid;
DRV_ProcUpdatestate(pPctxt, PROC_RES_ALLOCATED);
- DRV_ProcSetPID(pPctxt, hProcess);
+ DRV_ProcSetPID(pPctxt, hProcess);
}
#endif
--
1.5.3.2
^ permalink raw reply related
* [PATCH 0/1] DSPBRIDGE Fix for image autoload
From: Gupta, Ramesh @ 2009-03-12 5:31 UTC (permalink / raw)
To: linux-omap@vger.kernel.org
Cc: Menon, Nishanth, Kanigeri, Hari, Guzman Lugo, Fernando
All,
This patch set fixes an issue with dsp image autoloading while bridgedriver module
intialization.
This can be applied along with the other DSPBRIDGE patches, validated on OMAP3430 SDP.
drivers/dsp/bridge/rmgr/drv_interface.c | 28 ++++++++++++++--------------
1 files changed, 14 insertions(+), 14 deletions(-)
Regards
Ramesh Gupta G
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-omap: Call request_irq with IRQF_DISABLED
From: Paul Walmsley @ 2009-03-12 3:30 UTC (permalink / raw)
To: Felipe Balbi
Cc: Ari Kauppi, Ben Dooks, ben-linux-elnMNo+KYs3YtjvyW6yDsg,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20090312002353.GD19038@gandalf>
Hi Felipe,
On Thu, 12 Mar 2009, Felipe Balbi wrote:
> This is the link where Ingo discusses why !IRQF_DISABLED could cause
> stack overflow:
>
> http://lkml.org/lkml/2009/3/3/71
>
> You probably wanna take a look at the whole thread to figure out the
> discussion, but basically Ingo and Peter (Zijlstra) believe
> !IRQF_DISABLED is a bug and drivers needing that probably should be
> using threaded irqs (which is not yet merged) or the hw is broken, and
> for those an IRQF_ENABLED flag will be created and a TAINT will also be
> placed.
>
> Once that gets merged, all drivers will be forced (at some point) to
> IRQF_DISABLED and those which don't want that will be moved gradually to
> threaded irq.
>
> This patch is also interesting:
>
> http://lkml.org/lkml/2009/3/2/33
Thanks for the links. Those issues are indeed distinct from the spurious
IRQ problem that Ari is reporting.
- Paul
^ permalink raw reply
* [patch 2.6.29-rc7 regulator-next] regulator: init fixes
From: David Brownell @ 2009-03-12 2:32 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown; +Cc: lkml, OMAP
In-Reply-To: <200903111743.34708.david-b@pacbell.net>
From: David Brownell <dbrownell@users.sourceforge.net>
Make the regulator setup code cope more consistently with
regulators undesirably left enabled by the bootloader.
Building on the previous refcount patch:
* Unless the "boot_on" or "always_on" machine constraints
were set, disable() the regulator. This gives drivers
a clean start state: enable state matches usecount,
regardless of boot_on/always_on flag state.
* To help make some integration stages easier, add a new
"devmode" machine constraint where state the bootloader
left isn't touched, but enable state and usecount may
not match. (System boots but some drivers act odd ...
debuggable. System dies part way through booting ...
often painful.)
Consider a bootloader that leaves an MMC/SD regulator active
when it loads Linux from an SD card. It may take some time
before the MMC/SD driver gets loaded, if ever ... to save
power, that (LDO) regulator should be disabled ASAP. Then
later when the MMC driver starts up, the Linux MMC stack will
need to start from a "power off" state. It can't just
if (regulator_is_enabled(r))
regulator_disable(r);
unless enable state and usecount are matched ... but without
this patch, they *will* be mismatched whenever the bootloader
happens to have left that regulator active! Similar issues
can crop up with almost any regulator.
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
drivers/regulator/core.c | 47 +++++++++++++++++++++++++++++-------
include/linux/regulator/machine.h | 3 +-
2 files changed, 40 insertions(+), 10 deletions(-)
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -799,18 +799,47 @@ static int set_machine_constraints(struc
}
}
- /* If the constraints say the regulator should be on at this point
- * and we have control then make sure it is enabled.
+ /* During integration, developers may need time to sort out what
+ * to do with this regulator; leave the bootloader's setting alone.
+ * Regulator consumers won't get consistent behavior.
+ *
+ * Else the constraints say whether it should be on or off; we
+ * don't leave it in an unknown state.
*/
- if ((constraints->always_on || constraints->boot_on) && ops->enable) {
- ret = ops->enable(rdev);
- if (ret < 0) {
- printk(KERN_ERR "%s: failed to enable %s\n",
- __func__, name);
- rdev->constraints = NULL;
- goto out;
+ if (constraints->devmode) {
+ char *label = "unknown";
+
+ if (ops->is_enabled) {
+ ret = ops->is_enabled(rdev);
+ if (ret == 0)
+ label = "disabled";
+ else if (ret > 0)
+ label = "enabled";
+ ret = 0;
+ }
+ pr_warning("%s: devmode regulator '%s' state is '%s'\n",
+ __func__, name, label);
+ } else if (constraints->always_on || constraints->boot_on) {
+ if (ops->enable) {
+ ret = ops->enable(rdev);
+ if (ret < 0) {
+ pr_err("%s: failed enabling %s\n",
+ __func__, name);
+ rdev->constraints = NULL;
+ goto out;
+ }
}
rdev->use_count = 1;
+ } else {
+ if (ops->disable) {
+ ret = ops->disable(rdev);
+ if (ret < 0) {
+ pr_err("%s: failed disabling %s\n",
+ __func__, name);
+ rdev->constraints = NULL;
+ goto out;
+ }
+ }
}
print_constraints(rdev);
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -117,7 +117,8 @@ struct regulation_constraints {
/* mode to set on startup */
unsigned int initial_mode;
- /* constriant flags */
+ /* constraint flags */
+ unsigned devmode:1; /* state after setup is indeterminate */
unsigned always_on:1; /* regulator never off when system is on */
unsigned boot_on:1; /* bootloader/firmware enabled regulator */
unsigned apply_uV:1; /* apply uV constraint iff min == max */
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-omap: Call request_irq with IRQF_DISABLED
From: David Brownell @ 2009-03-12 1:28 UTC (permalink / raw)
To: me; +Cc: Paul Walmsley, Ari Kauppi, Ben Dooks, ben-linux, linux-omap,
linux-i2c
In-Reply-To: <20090311235908.GC19038@gandalf>
On Wednesday 11 March 2009, Felipe Balbi wrote:
> According to Thomas (and Ingo, I'd say) all drivers should call
> request_irq() with IRQF_DISABLED and that's gonna be true as soon as the
> threaded irq handler support gets merged, if I'm not wrong.
Well, they're wrong.
Folk like Dave Miller, Andrew Morton, Benjamin Herrenschmdit,
and Alan Cox have come out on the saner side of that.
^ permalink raw reply
* [patch 2.6.29-rc7 regulator-next] regulator: refcount fixes
From: David Brownell @ 2009-03-12 0:43 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown; +Cc: lkml, OMAP
From: David Brownell <dbrownell@users.sourceforge.net>
Fix some refcounting issues in the regulator framework, supporting
regulator_disable() for regulators that were enabled at boot time
via machine constraints:
- Update those regulators' usecounts after enabling, so they
can cleanly be disabled at that level.
- Remove the problematic per-consumer usecount, so there's
only one level of enable/disable.
Buggy consumers could notice different bug symptoms. The main
example would be refcounting bugs; also, any (out-of-tree) users
of the experimental regulator_set_optimum_mode() stuff which
don't call it when they're done using a regulator.
This is a net minor codeshrink.
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
Against the regulator-next tree; mainline has similar issues.
drivers/regulator/core.c | 30 ++++++++----------------------
1 file changed, 8 insertions(+), 22 deletions(-)
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -52,7 +52,6 @@ struct regulator {
int uA_load;
int min_uV;
int max_uV;
- int enabled; /* count of client enables */
char *supply_name;
struct device_attribute dev_attr;
struct regulator_dev *rdev;
@@ -811,6 +810,7 @@ static int set_machine_constraints(struc
rdev->constraints = NULL;
goto out;
}
+ rdev->use_count = 1;
}
print_constraints(rdev);
@@ -1066,10 +1066,6 @@ void regulator_put(struct regulator *reg
mutex_lock(®ulator_list_mutex);
rdev = regulator->rdev;
- if (WARN(regulator->enabled, "Releasing supply %s while enabled\n",
- regulator->supply_name))
- _regulator_disable(rdev);
-
/* remove any sysfs entries */
if (regulator->dev) {
sysfs_remove_link(&rdev->dev.kobj, regulator->supply_name);
@@ -1144,12 +1140,7 @@ int regulator_enable(struct regulator *r
int ret = 0;
mutex_lock(&rdev->mutex);
- if (regulator->enabled == 0)
- ret = _regulator_enable(rdev);
- else if (regulator->enabled < 0)
- ret = -EIO;
- if (ret == 0)
- regulator->enabled++;
+ ret = _regulator_enable(rdev);
mutex_unlock(&rdev->mutex);
return ret;
}
@@ -1160,6 +1151,11 @@ static int _regulator_disable(struct reg
{
int ret = 0;
+ if (WARN(rdev->use_count <= 0,
+ "unbalanced disables for %s\n",
+ rdev->desc->name))
+ return -EIO;
+
/* are we the last user and permitted to disable ? */
if (rdev->use_count == 1 && !rdev->constraints->always_on) {
@@ -1208,16 +1204,7 @@ int regulator_disable(struct regulator *
int ret = 0;
mutex_lock(&rdev->mutex);
- if (regulator->enabled == 1) {
- ret = _regulator_disable(rdev);
- if (ret == 0)
- regulator->uA_load = 0;
- } else if (WARN(regulator->enabled <= 0,
- "unbalanced disables for supply %s\n",
- regulator->supply_name))
- ret = -EIO;
- if (ret == 0)
- regulator->enabled--;
+ ret = _regulator_disable(rdev);
mutex_unlock(&rdev->mutex);
return ret;
}
@@ -1264,7 +1251,6 @@ int regulator_force_disable(struct regul
int ret;
mutex_lock(®ulator->rdev->mutex);
- regulator->enabled = 0;
regulator->uA_load = 0;
ret = _regulator_force_disable(regulator->rdev);
mutex_unlock(®ulator->rdev->mutex);
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-omap: Call request_irq with IRQF_DISABLED
From: Felipe Balbi @ 2009-03-12 0:25 UTC (permalink / raw)
To: Felipe Contreras
Cc: me, Paul Walmsley, Ari Kauppi, Ben Dooks, ben-linux, linux-omap,
linux-i2c
In-Reply-To: <94a0d4530903111711r7728aeabo8aad5ccd51a33d38@mail.gmail.com>
On Thu, Mar 12, 2009 at 02:11:17AM +0200, Felipe Contreras wrote:
> That's my understanding too, but I think it has always been true:
> http://marc.info/?l=linux-kernel&m=123607685804562&w=2
sure, not merged yet though. And still the threaded irq support can't
handle twl4030 where the irq demuxing logic has to run in a thread due
to the need of communicating via i2c.
--
balbi
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-omap: Call request_irq with IRQF_DISABLED
From: Felipe Balbi @ 2009-03-12 0:23 UTC (permalink / raw)
To: Paul Walmsley
Cc: Felipe Balbi, Ari Kauppi, Ben Dooks, ben-linux, linux-omap,
linux-i2c
In-Reply-To: <alpine.DEB.2.00.0903111804510.26959@utopia.booyaka.com>
On Wed, Mar 11, 2009 at 06:07:00PM -0600, Paul Walmsley wrote:
> Hi Felipe,
>
> On Thu, 12 Mar 2009, Felipe Balbi wrote:
>
> > On Wed, Mar 11, 2009 at 05:55:50PM -0600, Paul Walmsley wrote:
> > > Ben's right, there shouldn't be any need for this. This patch could cause
> > > some unnecessary interrupt service latency.
> >
> > That's not what Thomas Gleixner thinks. How about the possibility of
> > stack overflow ?
>
> That sounds like a separate issue from the spurious IRQ problem that the
> patch was intended to fix.
>
> I'm not familiar with the discussion on the stack overflow issue. Could
> you send a link?
This is the link where Ingo discusses why !IRQF_DISABLED could cause
stack overflow:
http://lkml.org/lkml/2009/3/3/71
You probably wanna take a look at the whole thread to figure out the
discussion, but basically Ingo and Peter (Zijlstra) believe
!IRQF_DISABLED is a bug and drivers needing that probably should be
using threaded irqs (which is not yet merged) or the hw is broken, and
for those an IRQF_ENABLED flag will be created and a TAINT will also be
placed.
Once that gets merged, all drivers will be forced (at some point) to
IRQF_DISABLED and those which don't want that will be moved gradually to
threaded irq.
This patch is also interesting:
http://lkml.org/lkml/2009/3/2/33
--
balbi
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-omap: Call request_irq with IRQF_DISABLED
From: Kevin Hilman @ 2009-03-12 0:20 UTC (permalink / raw)
To: Paul Walmsley
Cc: Felipe Balbi, Ari Kauppi, Ben Dooks,
ben-linux-elnMNo+KYs3YtjvyW6yDsg,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.DEB.2.00.0903111804510.26959-rwI8Ez+7Ko+d5PgPZx9QOdBPR1lH4CV8@public.gmane.org>
Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org> writes:
> Hi Felipe,
>
> On Thu, 12 Mar 2009, Felipe Balbi wrote:
>
>> On Wed, Mar 11, 2009 at 05:55:50PM -0600, Paul Walmsley wrote:
>> > Ben's right, there shouldn't be any need for this. This patch could cause
>> > some unnecessary interrupt service latency.
>>
>> That's not what Thomas Gleixner thinks. How about the possibility of
>> stack overflow ?
>
> That sounds like a separate issue from the spurious IRQ problem that the
> patch was intended to fix.
I agree. The IRQF_DISABLED happens to fix this issue, but it may be
masking the real spurious issue as Paul suggested.
> I'm not familiar with the discussion on the stack overflow issue. Could
> you send a link?
>
Here's one:
http://marc.info/?l=linux-kernel&m=123607359500596&w=2
Kevin
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-omap: Call request_irq with IRQF_DISABLED
From: Felipe Contreras @ 2009-03-12 0:11 UTC (permalink / raw)
To: me-uiRdBs8odbtmTBlB0Cgj/Q
Cc: Paul Walmsley, Ari Kauppi, Ben Dooks,
ben-linux-elnMNo+KYs3YtjvyW6yDsg,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20090311235908.GC19038@gandalf>
On Thu, Mar 12, 2009 at 1:59 AM, Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org> wrote:
> On Wed, Mar 11, 2009 at 05:55:50PM -0600, Paul Walmsley wrote:
>> Ben's right, there shouldn't be any need for this. This patch could cause
>> some unnecessary interrupt service latency.
>
> That's not what Thomas Gleixner thinks. How about the possibility of
> stack overflow ?
>
> According to Thomas (and Ingo, I'd say) all drivers should call
> request_irq() with IRQF_DISABLED and that's gonna be true as soon as the
> threaded irq handler support gets merged, if I'm not wrong.
That's my understanding too, but I think it has always been true:
http://marc.info/?l=linux-kernel&m=123607685804562&w=2
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-omap: Call request_irq with IRQF_DISABLED
From: Paul Walmsley @ 2009-03-12 0:07 UTC (permalink / raw)
To: Felipe Balbi
Cc: Ari Kauppi, Ben Dooks, ben-linux-elnMNo+KYs3YtjvyW6yDsg,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20090311235908.GC19038@gandalf>
Hi Felipe,
On Thu, 12 Mar 2009, Felipe Balbi wrote:
> On Wed, Mar 11, 2009 at 05:55:50PM -0600, Paul Walmsley wrote:
> > Ben's right, there shouldn't be any need for this. This patch could cause
> > some unnecessary interrupt service latency.
>
> That's not what Thomas Gleixner thinks. How about the possibility of
> stack overflow ?
That sounds like a separate issue from the spurious IRQ problem that the
patch was intended to fix.
I'm not familiar with the discussion on the stack overflow issue. Could
you send a link?
> According to Thomas (and Ingo, I'd say) all drivers should call
> request_irq() with IRQF_DISABLED and that's gonna be true as soon as the
> threaded irq handler support gets merged, if I'm not wrong.
- Paul
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-omap: Call request_irq with IRQF_DISABLED
From: Paul Walmsley @ 2009-03-12 0:04 UTC (permalink / raw)
To: Ari Kauppi
Cc: Ben Dooks, ben-linux-elnMNo+KYs3YtjvyW6yDsg,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.DEB.2.00.0903111741270.26959-rwI8Ez+7Ko+d5PgPZx9QOdBPR1lH4CV8@public.gmane.org>
Hi Ari,
One thing that I missed -
On Wed, 11 Mar 2009, Paul Walmsley wrote:
> > On Fri, Mar 06, 2009 at 03:34:54PM +0200, Ari Kauppi wrote:
> > > I have observed some Spurious IRQ's for I2C1 when all kernel hacking options
> > > (and thus LOCKDEP) are disabled.
>
> Ari, are you seeing "Spurious irq XX: XXXXXXXX, please flush posted write
> for irq" messages? If so, the correct fix for this is to read from the
> device interrupt status register immediately after writing to it. This
> forces the ARM to wait until the write to the device is complete. Ari,
> could you make this change to i2c-omap.c:omap_i2c_isr() instead, and test
> whether this fixes the problem?
>
> + u32 tmp;
>
> ...
>
> omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, stat);
> + tmp = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); /* OCP barrier */
You'll also want to make a similar change in omap_i2c_ack_stat(), to add a
read immediately after that write.
- Paul
^ permalink raw reply
* DSPBRIDGE: segmentation fault after reloading dspbridge several times due to a memory leak.
From: Guzman Lugo, Fernando @ 2009-03-12 0:03 UTC (permalink / raw)
To: linux-omap@vger.kernel.org
Hi All,
Reloading the dspbridge several times I am getting a Segmentation fault. Seeing the log it seems that the memory was exhausted
The error happens when ioremap is called
void MEM_ExtPhysPoolInit(u32 poolPhysBase, u32 poolSize)
{
u32 poolVirtBase;
/* get the virtual address for the physical memory pool passed */
poolVirtBase = (u32)ioremap(poolPhysBase, poolSize);
.
Putting some printk's and printing the address returned by ioremap, I realized that address returned by ioremap each time I reload the dspbridge is different, in fact the address is increasing. I also put a printk where iounmap is called to make sure it is called and yes it is actually called. However testing with a kernel + bridge for linux 23x I always get the same address for pool memory. Any idea what the problem is? I have included the console output where you can see the address increasing.
Regards,
Fernando.
The first 15 loads and unloads
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = c8000000
Releasing External memory pool, address = c8000000
interation 2
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = c9000000
Releasing External memory pool, address = c9000000
interation 3
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = c9800000
Releasing External memory pool, address = c9800000
interation 4
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = ca800000
Releasing External memory pool, address = ca800000
interation 5
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = cb000000
Releasing External memory pool, address = cb000000
interation 6
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = cc000000
Releasing External memory pool, address = cc000000
interation 7
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = cc800000
Releasing External memory pool, address = cc800000
interation 8
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = c8800000
Releasing External memory pool, address = c8800000
interation 9
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = ca000000
Releasing External memory pool, address = ca000000
interation 10
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = cd000000
Releasing External memory pool, address = cd000000
interation 11
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = cd800000
Releasing External memory pool, address = cd800000
interation 12
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = ce800000
Releasing External memory pool, address = ce800000
interation 13
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = cf000000
Releasing External memory pool, address = cf000000
interation 14
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = d0000000
Releasing External memory pool, address = d0000000
interation 15
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = d0800000
Releasing External memory pool, address = d0800000
...
and It crashes in the 33rd reload.
interation 32
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
[PHYS_POOL]Mapping External, address = d7800000
Releasing External memory pool, address = d7800000
interation 33
INFO: For help run script as ./install_bridge help
INFO:Installed Bridgedriver from current directory:/dspbridge_os
<4>vmap allocation failed: use vmalloc=<size> to increase size.
vmap allocation failed: use vmalloc=<size> to increase size.
[PHYS_POOL]Mapping External, address = 0
<4>coherent allocation too big (requested 0x400000 mask 0xffffffff)
coherent allocation too big (requested 0x400000 mask 0xffffffff)
drivers/dsp/bridge/rmgr/drv.c, line 1546: Assertion ((DSP_SUCCEEDED(status) && p
DevNodeString != NULL && !LST_IsEmpty(pDRVObject->devNodeString)) || (DSP_FAILED
(status) && *pDevNodeString == 0)) failed.
drivers/dsp/bridge/rmgr/drv.c, line 1546: Assertion ((DSP_SUCCEEDED(status) && p
DevNodeString != NULL && !LST_IsEmpty(pDRVObject->devNodeString)) || (DSP_FAILED
(status) && *pDevNodeString == 0)) failed.
drivers/dsp/bridge/rmgr/drv_interface.c, line 484: Assertion (status == 0) faile
d.
drivers/dsp/bridge/rmgr/drv_interface.c, line 484: Assertion (status == 0) faile
d.
drivers/dsp/bridge/rmgr/drv_interface.c, line 485: Assertion (DSP_SUCCEEDED(init
Status)) failed.
drivers/dsp/bridge/rmgr/drv_interface.c, line 485: Assertion (DSP_SUCCEEDED(init
Status)) failed.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCHv2] PM : cpuidle - Update statistics for correct state
From: Kevin Hilman @ 2009-03-12 0:00 UTC (permalink / raw)
To: Premi, Sanjeev; +Cc: Högander Jouni, linux-omap@vger.kernel.org
In-Reply-To: <B85A65D85D7EB246BE421B3FB0FBB59301CC1F6C1F@dbde02.ent.ti.com>
"Premi, Sanjeev" <premi@ti.com> writes:
>>
>> I would gladly take a patch which uses the 'valid' field for
>> this and then the enter hook whould drop to the next lower
>> valid state if the state requested is not valid.
>>
>> Kevin
>
> [sp] I have not yet tested it (working offline for sometime).
> But, wanted to share the changes for an early review.
Thanks, some comments below.
> Initially, I was trying see if the CPUIDLE framework could
> use ".valid" as an additional argument in cpuidle_state.
> But, may be that's for later...
Yeah, I looked at that too, but it currently doesn't have a concept
of valid states, so for now I recommend we implement that in the
OMAP-specific code as you have done.
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx
> index c25158c..9493553 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -88,16 +88,19 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
> goto return_sleep_time;
>
> /*
> + * Check if the chosen idle state is valid.
> + * If no, drop down to a lower valid state. Expects the lowest
> + * state will always be active.
> */
> + if (!cx->valid) {
> + for (idx = (cx->type - 1); idx == 1; idx--) {
^^^^^^^^
I think you mean idx >= 1 here.
Also, while you're working on this, could you fix this up so the
omap3_power_states[] array is zero based insted of 1-based, it would
make this code and the other code walking this array easier to follow.
That means defining OMAP_STATE_C1 = 0 and so on.
> + if (omap3_power_states[idx].valid)
> + break;
> }
> + state = &(dev->states[idx]);
> + dev->last_state = state ;
> +
> + cx = cpuidle_get_statedata(state);
> }
>
> current_cx_state = *cx;
> @@ -150,6 +153,26 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
> return omap3_enter_idle(dev, new_state);
> }
>
> +/**
> + * omap3_toggle_off_states - Enable / Disable validity of idle states
> + * @flag: Enable/ Disable support for OFF mode
> + *
> + * Called as result of change to "enable_off_mode".
> + */
> +void omap3_toggle_off_states(unsigned short flag)
> +{
How about calling this omap3_cpuidle_update_states() and taking
no arguments. Rather than the 'flag' argument, internally it
just checks the global 'enable_off_mode.'
This allows for potential further expansion down the road of other
reasons to update CPUidle valid states. For example, we've talked
about updating the CPUidle state latencies on the fly depending on
various other chip settings.
> + if (flag) {
> + omap3_power_states[OMAP3_STATE_C3].valid = 1;
> + omap3_power_states[OMAP3_STATE_C5].valid = 1;
> + omap3_power_states[OMAP3_STATE_C6].valid = 1;
> + }
> + else {
> + omap3_power_states[OMAP3_STATE_C3].valid = 0;
> + omap3_power_states[OMAP3_STATE_C5].valid = 0;
> + omap3_power_states[OMAP3_STATE_C6].valid = 0;
> + }
> +}
> +
Rather than set set specific OMAP3_STATE_Cx values, it would be better
to just walk the array, and check for [mpu|core]_state = PWRDM_POWER_OFF.
If the state has either set, then update the valid flag.
> DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
>
> /* omap3_init_power_states - Initialises the OMAP3 specific C states.
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index 61c6dfb..6785850 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -47,6 +47,8 @@ atomic_t sleep_block = ATOMIC_INIT(0);
> static int vdd1_locked;
> static int vdd2_locked;
>
> +extern void omap3_toggle_off_states(unsigned short);
> +
Move the definition into pm.h as omap3_cpuidle_update_states(void) as
described above.
> static ssize_t idle_show(struct kobject *, struct kobj_attribute *, char *);
> static ssize_t idle_store(struct kobject *k, struct kobj_attribute *,
> const char *buf, size_t n);
> @@ -112,6 +114,8 @@ static ssize_t idle_store(struct kobject *kobj, struct kobj_
> } else if (attr == &enable_off_mode_attr) {
> enable_off_mode = value;
> omap3_pm_off_mode_enable(enable_off_mode);
> + if (cpu_is_omap34xx())
> + omap3_toggle_off_states(value);
Then, don't modify pm.c, rather just call omap3_cpuidle_update_states() from
omap3_pm_off_mode_enable().
Kevin
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-omap: Call request_irq with IRQF_DISABLED
From: Felipe Balbi @ 2009-03-11 23:59 UTC (permalink / raw)
To: Paul Walmsley
Cc: Ari Kauppi, Ben Dooks, ben-linux-elnMNo+KYs3YtjvyW6yDsg,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.DEB.2.00.0903111741270.26959-rwI8Ez+7Ko+d5PgPZx9QOdBPR1lH4CV8@public.gmane.org>
On Wed, Mar 11, 2009 at 05:55:50PM -0600, Paul Walmsley wrote:
> Ben's right, there shouldn't be any need for this. This patch could cause
> some unnecessary interrupt service latency.
That's not what Thomas Gleixner thinks. How about the possibility of
stack overflow ?
According to Thomas (and Ingo, I'd say) all drivers should call
request_irq() with IRQF_DISABLED and that's gonna be true as soon as the
threaded irq handler support gets merged, if I'm not wrong.
--
balbi
^ permalink raw reply
* Re: [PATCH 2/2] i2c: i2c-omap: Call request_irq with IRQF_DISABLED
From: Paul Walmsley @ 2009-03-11 23:55 UTC (permalink / raw)
To: Ari Kauppi; +Cc: Ben Dooks, ben-linux, linux-omap, linux-i2c
In-Reply-To: <20090310005222.GE19758@fluff.org.uk>
Hello Ari et al.,
On Tue, 10 Mar 2009, Ben Dooks wrote:
> On Fri, Mar 06, 2009 at 03:34:54PM +0200, Ari Kauppi wrote:
> > I have observed some Spurious IRQ's for I2C1 when all kernel hacking options
> > (and thus LOCKDEP) are disabled.
> >
> > Applying Richard Woodruff's 'I2C bug fixes for L-O and L-Z' seems to help
> > but IRQF_DISABLED is needed for proper behaviour.
> >
> > Signed-off-by: Ari Kauppi <Ext-Ari.Kauppi@nokia.com>
> > Acked-by: Felipe Balbi <felipe.balbi@nokia.com>
> > ---
> > drivers/i2c/busses/i2c-omap.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> > index 0c3ed41..18af43f 100644
> > --- a/drivers/i2c/busses/i2c-omap.c
> > +++ b/drivers/i2c/busses/i2c-omap.c
> > @@ -847,7 +847,7 @@ omap_i2c_probe(struct platform_device *pdev)
> > omap_i2c_init(dev);
> >
> > isr = (dev->rev < OMAP_I2C_REV_2) ? omap_i2c_rev1_isr : omap_i2c_isr;
> > - r = request_irq(dev->irq, isr, 0, pdev->name, dev);
> > + r = request_irq(dev->irq, isr, IRQF_DISABLED, pdev->name, dev);
>
> Is disabling all IRQs on the system the right thing to do?
Ben's right, there shouldn't be any need for this. This patch could cause
some unnecessary interrupt service latency.
Ari, are you seeing "Spurious irq XX: XXXXXXXX, please flush posted write
for irq" messages? If so, the correct fix for this is to read from the
device interrupt status register immediately after writing to it. This
forces the ARM to wait until the write to the device is complete. Ari,
could you make this change to i2c-omap.c:omap_i2c_isr() instead, and test
whether this fixes the problem?
+ u32 tmp;
...
omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, stat);
+ tmp = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); /* OCP barrier */
- Paul
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox