| Message ID | 20130716090529.5541.54331.stgit@localhost (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tony, This patch causes boot failure when I've applied my patch "[RFC] ARM: OMAP2+: omap_device: add pinctrl handling" https://lkml.org/lkml/2013/6/21/309 on top of it: [ 0.264007] L310 cache controller enabled [ 0.268310] l2x0: 16 ways, CACHE_ID 0x410000c4, AUX_CTRL 0x7e470000, Cache size: 1048576 B [ 0.282440] CPU1: Booted secondary processor [ 0.366760] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001 [ 0.367401] Brought up 2 CPUs [ 0.380920] SMP: Total of 2 processors activated (2387.96 BogoMIPS). [ 0.387573] CPU: All CPU(s) started in SVC mode. [ 0.395324] devtmpfs: initialized [ 0.468658] pinctrl core: initialized pinctrl subsystem [ 0.477996] regulator-dummy: no parameters [ 0.485412] NET: Registered protocol family 16 [ 0.499145] DMA: preallocated 256 KiB pool for atomic coherent allocations [ 0.573181] Unable to handle kernel NULL pointer dereference at virtual address 00000008 [ 0.581573] pgd = c0004000 [ 0.584472] [00000008] *pgd=00000000 [ 0.588226] Internal error: Oops: 5 [#1] SMP ARM [ 0.593078] Modules linked in: [ 0.596313] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.11.0-rc1-00005-g37e15e6-dirty #100 [ 0.604888] task: de07f3c0 ti: de080000 task.ti: de080000 [ 0.610565] PC is at pinctrl_pm_select_active_state+0x4/0xc [ 0.616394] LR is at _od_runtime_resume+0xc/0x20 [ 0.621215] pc : [<c02d4e2c>] lr : [<c002e930>] psr: 60000193 [ 0.621215] sp : de081cc0 ip : 60000193 fp : 00000000 [ 0.633209] r10: de080000 r9 : c0067e90 r8 : 00000004 [ 0.638671] r7 : c07800c0 r6 : c002e924 r5 : de0cf4a0 r4 : de0cf410 [ 0.645477] r3 : 00000000 r2 : 00000004 r1 : 00000470 r0 : de0cf410 [ 0.652282] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel [ 0.660003] Control: 10c53c7d Table: 8000404a DAC: 00000017 [ 0.665985] Process swapper/0 (pid: 1, stack limit = 0xde080240) [ 0.672241] Stack: (0xde081cc0 to 0xde082000) [ 0.676818] 1cc0: de0cf410 c0335310 de0cf410 de0cb410 00000001 c0335374 de0cf410 de0cb410 [ 0.685333] 1ce0: 00000001 c033664c c0336b14 00000004 c1bbdedc 00000000 00000000 c0507c5c [ 0.693847] 1d00: de0cf4a0 de0cf410 60000113 00000004 c1bbdedc 00000000 00000000 c0336b24 [ 0.702362] 1d20: de07f3c0 de11ea10 de0cf410 de0cf400 de0cd780 c02df144 de0cd740 00000000 [ 0.710845] 1d40: de0cf410 c0d6a634 de0cf410 00000000 00000000 c07efe7c 00000000 00000000 [ 0.719360] 1d60: 00000000 c032d794 c032d77c c032c554 de0cf410 c032c784 00000000 00000000 [ 0.727874] 1d80: c0d6a5f0 c032ac98 de07bed8 de0fd714 de0cf410 de0cf444 c07f65e8 c032c48c [ 0.736389] 1da0: de0cf410 de0cf410 c07f65e8 c032b[RFC] ARM: OMAP2+: omap_device: add pinctrl handlinga94 de0cf410 de0cf418 de0cb410 c0329ef8 [ 0.744873] 1dc0: 4a3101ff 00000000 00000200 00000000 00000000 00000000 c076f630 00000000 [ 0.753387] 1de0: de0cf400 00000000 de0cb410 c076f630 de0cb410 00000000 00000000 c042c2dc [ 0.761901] 1e00: de0cb410 c1bbdedc 00000000 00000000 00000001 c042c3dc 00000001 c076f630 [ 0.770385] 1e20: 00000000 de0cb410 00000000 c0099068 60000113 c080b22c c1bbdd98 00000001 [ 0.778900] 1e40: c076f630 c1bbcaec 00000000 c1bbdedc 00000001 c076f630 00000000 de0cb410 [ 0.787414] 1e60: 00000000 c042c440 00000001 c076f630 00000000 00000001 00000000 c0099068 [ 0.795928] 1e80: 60000113 c080b22c 00000000 00000000 c076f630 c1bbcaec c1bbc09c 00000000 [ 0.804412] 1ea0: 00000000 c076f630 00000000 00000001 00000000 c042c4e4 00000001 c072c37c [ 0.812927] 1ec0: c07221e0 de080000 c0762648 00000000 000000a4 c077979c c071f1c8 c0730a6c [ 0.821441] 1ee0: c0760aec c07221fc 00000000 c0008978 00000083 de07f3c0 60000193 00000000 [ 0.829925] 1f00: 00000006 c07dbcd4 c07dbcd4 60000113 c02bf100 00000000 c07dbcd0 c07dbcd0 [ 0.838439] 1f20: 00000000 c0507c5c 00000002 de080000 c06f1920 c1bc531d 000000a4 c00655ec [ 0.846954] 1f40: c06b2bd8 c06f0ee0 00000003 00000003 60000113 c0762668 00000003 c0762648 [ 0.855468] 1f60: c0817500 000000a4 c077979c c071f1c8 00000000 c071f91c 00000003 00000003 [ 0.863952] 1f80: c071f1c8 00000000 00000000 c04fd9ac 00000000 00000000 00000000 00000000 [ 0.872467] 1fa0: 00000000 c04fd9b4 00000000 c0013ac8 00000000 00000000 00000000 00000000 [ 0.880981] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 0.889465] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ce5331ac 3153ceac [ 0.898010] [<c02d4e2c>] (pinctrl_pm_select_active_state+0x4/0xc) from [<c002e930>] (_od_runtime_resume+0xc/0x20) [ 0.908660] [<c002e930>] (_od_runtime_resume+0xc/0x20) from [<c0335310>] (__rpm_callback+0x34/0x70) [ 0.918060] [<c0335310>] (__rpm_callback+0x34/0x70) from [<c0335374>] (rpm_callback+0x28/0x88) [ 0.927032] [<c0335374>] (rpm_callback+0x28/0x88) from [<c033664c>] (rpm_resume+0x3c8/0x624) [ 0.935821] [<c033664c>] (rpm_resume+0x3c8/0x624) from [<c0336b24>] (__pm_runtime_resume+0x48/0x60) [ 0.945220] [<c0336b24>] (__pm_runtime_resume+0x48/0x60) from [<c02df144>] (omap_gpio_probe+0x254/0x6c0) [ 0.955078] [<c02df144>] (omap_gpio_probe+0x254/0x6c0) from [<c032d794>] (platform_drv_probe+0x18/0x1c) [ 0.964843] [<c032d794>] (platform_drv_probe+0x18/0x1c) from [<c032c554>] (driver_probe_device+0x88/0x220) [ 0.974884] [<c032c554>] (driver_probe_device+0x88/0x220) from [<c032ac98>] (bus_for_each_drv+0x5c/0x88) [ 0.984710] [<c032ac98>] (bus_for_each_drv+0x5c/0x88) from [<c032c48c>] (device_attach+0x80/0xa4) [ 0.993957] [<c032c48c>] (device_attach+0x80/0xa4) from [<c032ba94>] (bus_probe_device+0x88/0xac) [ 1.003173] [<c032ba94>] (bus_probe_device+0x88/0xac) from [<c0329ef8>] (device_add+0x418/0x61c) [ 1.012329] [<c0329ef8>] (device_add+0x418/0x61c) from [<c042c2dc>] (of_platform_device_create_pdata+0x5c/0x80) [ 1.022796] [<c042c2dc>] (of_platform_device_create_pdata+0x5c/0x80) from [<c042c3dc>] (of_platform_bus_create+0xdc/0x184) [ 1.034271] [<c042c3dc>] (of_platform_bus_create+0xdc/0x184) from [<c042c440>] (of_platform_bus_create+0x140/0x184) [ 1.045104] [<c042c440>] (of_platform_bus_create+0x140/0x184) from [<c042c4e4>] (of_platform_populate+0x60/0x98) [ 1.055664] [<c042c4e4>] (of_platform_populate+0x60/0x98) from [<c0730a6c>] (omap_generic_init+0x24/0x60) [ 1.065612] [<c0730a6c>] (omap_generic_init+0x24/0x60) from [<c07221fc>] (customize_machine+0x1c/0x40) [ 1.075286] [<c07221fc>] (customize_machine+0x1c/0x40) from [<c0008978>] (do_one_initcall+0xe4/0x148) [ 1.084869] [<c0008978>] (do_one_initcall+0xe4/0x148) from [<c071f91c>] (kernel_init_freeable+0xfc/0x1c8) [ 1.094818] [<c071f91c>] (kernel_init_freeable+0xfc/0x1c8) from [<c04fd9b4>] (kernel_init+0x8/0xe4) [ 1.104248] [<c04fd9b4>] (kernel_init+0x8/0xe4) from [<c0013ac8>] (ret_from_fork+0x14/0x2c) [ 1.112915] Code: e59031b4 e593100c eaffffde e59031b4 (e5931008) [ 1.119323] ---[ end trace fd7907bbe82cc699 ]--- [ 1.124206] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b [ 1.124206] [ 1.133789] CPU1: stopping [ 1.136657] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G D 3.11.0-rc1-00005-g37e15e6-dirty #100 [ 1.146270] [<c001b62c>] (unwind_backtrace+0x0/0xf0) from [<c00177fc>] (show_stack+0x10/0x14) [ 1.155151] [<c00177fc>] (show_stack+0x10/0x14) from [<c05027cc>] (dump_stack+0x70/0x8c) [ 1.163574] [<c05027cc>] (dump_stack+0x70/0x8c) from [<c0019384>] (handle_IPI+0x130/0x158) [ 1.172180] [<c0019384>] (handle_IPI+0x130/0x158) from [<c0008760>] (gic_handle_irq+0x54/0x5c) [ 1.181121] [<c0008760>] (gic_handle_irq+0x54/0x5c) from [<c0508624>] (__irq_svc+0x44/0x5c) [ 1.189819] Exception stack(0xde0a7f90 to 0xde0a7fd8) [ 1.195098] 7f80: c0014ca8 000002da 00000000 00000000 [ 1.203613] 7fa0: de0a6000 00000000 10c03c7d c0817774 c0514820 c0815e40 c07869a8 00000000 [ 1.212097] 7fc0: 01000000 de0a7fd8 c0014ca8 c0014cac 60000113 ffffffff [ 1.219024] [<c0508624>] (__irq_svc+0x44/0x5c) from [<c0014cac>] (arch_cpu_idle+0x38/0x44) [ 1.227630] [<c0014cac>] (arch_cpu_idle+0x38/0x44) from [<c00875d4>] (cpu_startup_entry+0xa8/0x228) [ 1.237030] [<c00875d4>] (cpu_startup_entry+0xa8/0x228) from [<80008264>] (0x80008264) On 07/16/2013 12:05 PM, Tony Lindgren wrote: > There's no need to duplicate essentially the same functions. Let's > introduce static int pinctrl_pm_select_state() and make the other > related functions call that. > > This allows us to add support later on for multiple active states, > and more optimized dynamic remuxing. > > Note that we still need to export the various pinctrl_pm_select > functions as we want to keep struct pinctrl_state private to the > pinctrl code, and cannot replace those with inline functions. > > Cc: Stephen Warren <swarren@wwwdotorg.org> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > drivers/pinctrl/core.c | 47 +++++++++++++++++++---------------------------- > 1 file changed, 19 insertions(+), 28 deletions(-) > > diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c > index 5b272bf..bda2c61 100644 > --- a/drivers/pinctrl/core.c > +++ b/drivers/pinctrl/core.c > @@ -1227,23 +1227,34 @@ EXPORT_SYMBOL_GPL(pinctrl_force_default); > #ifdef CONFIG_PM > > /** > - * pinctrl_pm_select_default_state() - select default pinctrl state for PM > + * pinctrl_pm_select_state() - select pinctrl state for PM > * @dev: device to select default state for > + * @state: state to set > */ > -int pinctrl_pm_select_default_state(struct device *dev) > +static int pinctrl_pm_select_state(struct device *dev, struct pinctrl_state *state) > { > struct dev_pin_info *pins = dev->pins; > int ret; > > if (!pins) > return 0; > - if (IS_ERR(pins->default_state)) > - return 0; /* No default state */ > - ret = pinctrl_select_state(pins->p, pins->default_state); > + if (IS_ERR(state)) > + return 0; /* No such state */ > + ret = pinctrl_select_state(pins->p, state); > if (ret) > - dev_err(dev, "failed to activate default pinctrl state\n"); > + dev_err(dev, "failed to activate pinctrl state %s\n", > + state->name); > return ret; > } > + > +/** > + * pinctrl_pm_select_default_state() - select default pinctrl state for PM > + * @dev: device to select default state for > + */ > +int pinctrl_pm_select_default_state(struct device *dev) > +{ Seems, need to keep check for !dev->pins here if (!dev->pins) return 0; > + return pinctrl_pm_select_state(dev, dev->pins->default_state); > +} > EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state); > > /** > @@ -1252,17 +1263,7 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state); > */ > int pinctrl_pm_select_sleep_state(struct device *dev) > { > - struct dev_pin_info *pins = dev->pins; > - int ret; > - > - if (!pins) > - return 0; > - if (IS_ERR(pins->sleep_state)) > - return 0; /* No sleep state */ > - ret = pinctrl_select_state(pins->p, pins->sleep_state); > - if (ret) > - dev_err(dev, "failed to activate pinctrl sleep state\n"); > - return ret; here if (!dev->pins) return 0; > + return pinctrl_pm_select_state(dev, dev->pins->sleep_state); > } > EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state); > > @@ -1272,17 +1273,7 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state); > */ > int pinctrl_pm_select_idle_state(struct device *dev) > { > - struct dev_pin_info *pins = dev->pins; > - int ret; > - > - if (!pins) > - return 0; > - if (IS_ERR(pins->idle_state)) > - return 0; /* No idle state */ > - ret = pinctrl_select_state(pins->p, pins->idle_state); > - if (ret) > - dev_err(dev, "failed to activate pinctrl idle state\n"); > - return ret; here if (!dev->pins) return 0; > + return pinctrl_pm_select_state(dev, dev->pins->idle_state); > } > EXPORT_SYMBOL_GPL(pinctrl_pm_select_idle_state); > #endif > > -- > 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 >
* Grygorii Strashko <grygorii.strashko@ti.com> [130716 06:22]: > Hi Tony, > > This patch causes boot failure when I've applied my patch > "[RFC] ARM: OMAP2+: omap_device: add pinctrl handling" > https://lkml.org/lkml/2013/6/21/309 > > on top of it: Hmm this patch alone removes duplicate code and if it causes issues I must have made a typo somewhere. I cannot see a typo though, but in your dmesg I see something.. > [ 0.610565] PC is at pinctrl_pm_select_active_state+0x4/0xc ..looks like you have something applied for the active_state that only gets introduced later on in this series. Maybe you have the earlier version of drivers/base/pinctrl.c active_state patch that was in linux next for a while but got reverted as we noticed we had to rework some things? So maybe try with v3.11-rc1 + these four patches + your omap_device patch? Regards, Tony
Hi Tony, On 07/16/2013 04:41 PM, Tony Lindgren wrote: > * Grygorii Strashko <grygorii.strashko@ti.com> [130716 06:22]: >> Hi Tony, >> >> This patch causes boot failure when I've applied my patch >> "[RFC] ARM: OMAP2+: omap_device: add pinctrl handling" >> https://lkml.org/lkml/2013/6/21/309 >> >> on top of it: > > Hmm this patch alone removes duplicate code and if it causes > issues I must have made a typo somewhere. No typo :) You've removed the check for !dev-pins. And the failure place is: int pinctrl_pm_select_active_state(struct device *dev) { return pinctrl_pm_select_state(dev, dev->pins->active_state); ^^^^ here } If I understand everything right, the pinctrl support in Device core assumed to be optional - so, It's valid case, when there are no definition for device's pinctrl in DT at all. And in this case dev->pins == NULL and pinctrl_pm_select_*() API should return 0 always. > > I cannot see a typo though, but in your dmesg I see something.. > >> [ 0.610565] PC is at pinctrl_pm_select_active_state+0x4/0xc Yep. This will happen if there are no pinctrl states defined in DT - currently it's crashed when GPIO driver probed. > > ..looks like you have something applied for the active_state > that only gets introduced later on in this series. > > Maybe you have the earlier version of drivers/base/pinctrl.c > active_state patch that was in linux next for a while but > got reverted as we noticed we had to rework some things? > > So maybe try with v3.11-rc1 + these four patches + your > omap_device patch? I'm on v3.11-rc1 > > Regards, > > Tony >
* Grygorii Strashko <grygorii.strashko@ti.com> [130716 07:32]: > Hi Tony, > > On 07/16/2013 04:41 PM, Tony Lindgren wrote: > >* Grygorii Strashko <grygorii.strashko@ti.com> [130716 06:22]: > >>Hi Tony, > >> > >>This patch causes boot failure when I've applied my patch > >>"[RFC] ARM: OMAP2+: omap_device: add pinctrl handling" > >>https://lkml.org/lkml/2013/6/21/309 > >> > >>on top of it: > > > >Hmm this patch alone removes duplicate code and if it causes > >issues I must have made a typo somewhere. > > No typo :) You've removed the check for !dev-pins. Oh OK, sorry that was not intentional. > And the failure place is: > int pinctrl_pm_select_active_state(struct device *dev) > { > return pinctrl_pm_select_state(dev, dev->pins->active_state); > ^^^^ here > } > > If I understand everything right, the pinctrl support in Device core > assumed to be optional - so, It's valid case, when there are no > definition for device's pinctrl in DT at all. > > And in this case dev->pins == NULL and pinctrl_pm_select_*() API > should return 0 always. Care to post your patch as it sounds like you have it fixed and tested? Regards, Tony
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c index 5b272bf..bda2c61 100644 --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c @@ -1227,23 +1227,34 @@ EXPORT_SYMBOL_GPL(pinctrl_force_default); #ifdef CONFIG_PM /** - * pinctrl_pm_select_default_state() - select default pinctrl state for PM + * pinctrl_pm_select_state() - select pinctrl state for PM * @dev: device to select default state for + * @state: state to set */ -int pinctrl_pm_select_default_state(struct device *dev) +static int pinctrl_pm_select_state(struct device *dev, struct pinctrl_state *state) { struct dev_pin_info *pins = dev->pins; int ret; if (!pins) return 0; - if (IS_ERR(pins->default_state)) - return 0; /* No default state */ - ret = pinctrl_select_state(pins->p, pins->default_state); + if (IS_ERR(state)) + return 0; /* No such state */ + ret = pinctrl_select_state(pins->p, state); if (ret) - dev_err(dev, "failed to activate default pinctrl state\n"); + dev_err(dev, "failed to activate pinctrl state %s\n", + state->name); return ret; } + +/** + * pinctrl_pm_select_default_state() - select default pinctrl state for PM + * @dev: device to select default state for + */ +int pinctrl_pm_select_default_state(struct device *dev) +{ + return pinctrl_pm_select_state(dev, dev->pins->default_state); +} EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state); /** @@ -1252,17 +1263,7 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state); */ int pinctrl_pm_select_sleep_state(struct device *dev) { - struct dev_pin_info *pins = dev->pins; - int ret; - - if (!pins) - return 0; - if (IS_ERR(pins->sleep_state)) - return 0; /* No sleep state */ - ret = pinctrl_select_state(pins->p, pins->sleep_state); - if (ret) - dev_err(dev, "failed to activate pinctrl sleep state\n"); - return ret; + return pinctrl_pm_select_state(dev, dev->pins->sleep_state); } EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state); @@ -1272,17 +1273,7 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state); */ int pinctrl_pm_select_idle_state(struct device *dev) { - struct dev_pin_info *pins = dev->pins; - int ret; - - if (!pins) - return 0; - if (IS_ERR(pins->idle_state)) - return 0; /* No idle state */ - ret = pinctrl_select_state(pins->p, pins->idle_state); - if (ret) - dev_err(dev, "failed to activate pinctrl idle state\n"); - return ret; + return pinctrl_pm_select_state(dev, dev->pins->idle_state); } EXPORT_SYMBOL_GPL(pinctrl_pm_select_idle_state); #endif
There's no need to duplicate essentially the same functions. Let's introduce static int pinctrl_pm_select_state() and make the other related functions call that. This allows us to add support later on for multiple active states, and more optimized dynamic remuxing. Note that we still need to export the various pinctrl_pm_select functions as we want to keep struct pinctrl_state private to the pinctrl code, and cannot replace those with inline functions. Cc: Stephen Warren <swarren@wwwdotorg.org> Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/pinctrl/core.c | 47 +++++++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 28 deletions(-)