From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Jon Hunter <jon-hunter@ti.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>,
linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
Tero Kristo <t-kristo@ti.com>
Subject: Re: [PATCH v2 09/11] mfd: twl-core: Collect global variables behind one private structure (global)
Date: Wed, 13 Feb 2013 14:59:17 +0100 [thread overview]
Message-ID: <511B9C35.907@ti.com> (raw)
In-Reply-To: <511A6A8C.2020000@ti.com>
Hi Jon,
On 02/12/2013 05:15 PM, Jon Hunter wrote:
>
> On 02/12/2013 01:26 AM, Peter Ujfalusi wrote:
>> On 02/11/2013 09:22 PM, Jon Hunter wrote:
>>> Good point. I just noticed that none of my omap2+ board were booting and
>>> on omap3/4 I was the panic in the twl code. I can't say that I checked
>>> the panic on omap2, so may be that was another problem?
>>
>> Do you have insights on the code path leading to a crash on OMAP3/4? I have
>> been running this code on several boards (BeagleBoard, Zoom2, PandaBoard,
>> Blaze) and have not seen a crash.
>
> Here is the panic log ...
Thanks.
>
> [ 2.286132] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [ 2.294738] pgd = c0004000
> [ 2.297576] [00000000] *pgd=00000000
> [ 2.301361] Internal error: Oops: 5 [#1] SMP ARM
> [ 2.306243] Modules linked in:
> [ 2.309448] CPU: 0 Not tainted (3.8.0-rc6-next-20130207-00016-g735c237 #35)
> [ 2.317169] PC is at twl_i2c_read+0x3c/0xec
> [ 2.321563] LR is at twl_i2c_read+0x1c/0xec
> [ 2.325988] pc : [<c0333950>] lr : [<c0333930>] psr: 80000153
> [ 2.325988] sp : c702fed0 ip : 00000000 fp : 00000000
> [ 2.338043] r10: c702e000 r9 : c06e84e8 r8 : c06e51c8
> [ 2.343536] r7 : 00000001 r6 : 00000006 r5 : c702fef6 r4 : 00000004
> [ 2.350402] r3 : c129508c r2 : 00000006 r1 : c702fef6 r0 : 0000000e
> [ 2.357269] Flags: Nzcv IRQs on FIQs off Mode SVC_32 ISA ARM Segment kernel
> [ 2.365051] Control: 10c5387d Table: 80004019 DAC: 00000017
> [ 2.371093] Process swapper/0 (pid: 1, stack limit = 0xc702e240)
> [ 2.377410] Stack: (0xc702fed0 to 0xc7030000)
> [ 2.382019] fec0: c0d42180 c0d429d0 c70a7640 c07354c4
> [ 2.390624] fee0: 00000001 c0719798 c0d42180 c06f2cc0 c04e76cc c06ee1ac 00000000 c07354c4
> [ 2.399230] ff00: 00000007 c06f2d64 00000017 c06fb308 00000000 c06f07a4 c0cb8580 c06edee0
> [ 2.407836] ff20: c06eded4 c06e8504 c0d42180 c0008768 0000009e c00611b4 00000001 00000000
> [ 2.416442] ff40: c061994c c06b7548 00000007 00000007 00000000 c07354c4 00000007 c0719798
> [ 2.425048] ff60: c0d42180 c06e51c8 c07197a0 0000009e 00000000 c06e590c 00000007 00000007
> [ 2.433654] ff80: c06e51c8 00000000 00000000 c04d26ec 00000000 00000000 00000000 00000000
> [ 2.442260] ffa0: 00000000 c04d26f4 00000000 c00137b0 00000000 00000000 00000000 00000000
> [ 2.450866] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 2.459472] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 80fb6c10 71bbcd20
> [ 2.468078] [<c0333950>] (twl_i2c_read+0x3c/0xec) from [<c06f2cc0>] (omap3_twl_set_sr_bit+0x3c/0xb4)
> [ 2.477722] [<c06f2cc0>] (omap3_twl_set_sr_bit+0x3c/0xb4) from [<c06f2d64>] (omap3_twl_init+0x2c/0x70)
> [ 2.487518] [<c06f2d64>] (omap3_twl_init+0x2c/0x70) from [<c06fb308>] (omap_pmic_late_init+0x18/0x24)
> [ 2.497222] [<c06fb308>] (omap_pmic_late_init+0x18/0x24) from [<c06f07a4>] (omap2_common_pm_late_init+0x18/0xd0)
> [ 2.507934] [<c06f07a4>] (omap2_common_pm_late_init+0x18/0xd0) from [<c06edee0>] (omap3_init_late+0xc/0x18)
> [ 2.518188] [<c06edee0>] (omap3_init_late+0xc/0x18) from [<c06e8504>] (init_machine_late+0x1c/0x28)
> [ 2.527740] [<c06e8504>] (init_machine_late+0x1c/0x28) from [<c0008768>] (do_one_initcall+0x100/0x16c)
> [ 2.537536] [<c0008768>] (do_one_initcall+0x100/0x16c) from [<c06e590c>] (kernel_init_freeable+0xfc/0x1c8)
> [ 2.547698] [<c06e590c>] (kernel_init_freeable+0xfc/0x1c8) from [<c04d26f4>] (kernel_init+0x8/0xe4)
> [ 2.557250] [<c04d26f4>] (kernel_init+0x8/0xe4) from [<c00137b0>] (ret_from_fork+0x14/0x24)
This is exactly the sort of thing which should not ever existed in the first
place...
The code in omap_twl.c, especially the omap3_twl_set_sr_bit() function is
executed in .init_late of the board. If the twl stack is not up by then we are
going to have issues.
Right now I don't see how to solve this (the call chain is related to
smartreflex) but IMHO we should not have these 'random' twl_write calls spread
across the kernel without integrating them to the twl stack in a proper
manner. What I mean is that we should only have twl_* calls from drivers,
which has been created by the twl-core MFD core.
--
Péter
WARNING: multiple messages have this Message-ID (diff)
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Jon Hunter <jon-hunter@ti.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>,
<linux-kernel@vger.kernel.org>, <linux-omap@vger.kernel.org>,
Tero Kristo <t-kristo@ti.com>
Subject: Re: [PATCH v2 09/11] mfd: twl-core: Collect global variables behind one private structure (global)
Date: Wed, 13 Feb 2013 14:59:17 +0100 [thread overview]
Message-ID: <511B9C35.907@ti.com> (raw)
In-Reply-To: <511A6A8C.2020000@ti.com>
Hi Jon,
On 02/12/2013 05:15 PM, Jon Hunter wrote:
>
> On 02/12/2013 01:26 AM, Peter Ujfalusi wrote:
>> On 02/11/2013 09:22 PM, Jon Hunter wrote:
>>> Good point. I just noticed that none of my omap2+ board were booting and
>>> on omap3/4 I was the panic in the twl code. I can't say that I checked
>>> the panic on omap2, so may be that was another problem?
>>
>> Do you have insights on the code path leading to a crash on OMAP3/4? I have
>> been running this code on several boards (BeagleBoard, Zoom2, PandaBoard,
>> Blaze) and have not seen a crash.
>
> Here is the panic log ...
Thanks.
>
> [ 2.286132] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [ 2.294738] pgd = c0004000
> [ 2.297576] [00000000] *pgd=00000000
> [ 2.301361] Internal error: Oops: 5 [#1] SMP ARM
> [ 2.306243] Modules linked in:
> [ 2.309448] CPU: 0 Not tainted (3.8.0-rc6-next-20130207-00016-g735c237 #35)
> [ 2.317169] PC is at twl_i2c_read+0x3c/0xec
> [ 2.321563] LR is at twl_i2c_read+0x1c/0xec
> [ 2.325988] pc : [<c0333950>] lr : [<c0333930>] psr: 80000153
> [ 2.325988] sp : c702fed0 ip : 00000000 fp : 00000000
> [ 2.338043] r10: c702e000 r9 : c06e84e8 r8 : c06e51c8
> [ 2.343536] r7 : 00000001 r6 : 00000006 r5 : c702fef6 r4 : 00000004
> [ 2.350402] r3 : c129508c r2 : 00000006 r1 : c702fef6 r0 : 0000000e
> [ 2.357269] Flags: Nzcv IRQs on FIQs off Mode SVC_32 ISA ARM Segment kernel
> [ 2.365051] Control: 10c5387d Table: 80004019 DAC: 00000017
> [ 2.371093] Process swapper/0 (pid: 1, stack limit = 0xc702e240)
> [ 2.377410] Stack: (0xc702fed0 to 0xc7030000)
> [ 2.382019] fec0: c0d42180 c0d429d0 c70a7640 c07354c4
> [ 2.390624] fee0: 00000001 c0719798 c0d42180 c06f2cc0 c04e76cc c06ee1ac 00000000 c07354c4
> [ 2.399230] ff00: 00000007 c06f2d64 00000017 c06fb308 00000000 c06f07a4 c0cb8580 c06edee0
> [ 2.407836] ff20: c06eded4 c06e8504 c0d42180 c0008768 0000009e c00611b4 00000001 00000000
> [ 2.416442] ff40: c061994c c06b7548 00000007 00000007 00000000 c07354c4 00000007 c0719798
> [ 2.425048] ff60: c0d42180 c06e51c8 c07197a0 0000009e 00000000 c06e590c 00000007 00000007
> [ 2.433654] ff80: c06e51c8 00000000 00000000 c04d26ec 00000000 00000000 00000000 00000000
> [ 2.442260] ffa0: 00000000 c04d26f4 00000000 c00137b0 00000000 00000000 00000000 00000000
> [ 2.450866] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 2.459472] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 80fb6c10 71bbcd20
> [ 2.468078] [<c0333950>] (twl_i2c_read+0x3c/0xec) from [<c06f2cc0>] (omap3_twl_set_sr_bit+0x3c/0xb4)
> [ 2.477722] [<c06f2cc0>] (omap3_twl_set_sr_bit+0x3c/0xb4) from [<c06f2d64>] (omap3_twl_init+0x2c/0x70)
> [ 2.487518] [<c06f2d64>] (omap3_twl_init+0x2c/0x70) from [<c06fb308>] (omap_pmic_late_init+0x18/0x24)
> [ 2.497222] [<c06fb308>] (omap_pmic_late_init+0x18/0x24) from [<c06f07a4>] (omap2_common_pm_late_init+0x18/0xd0)
> [ 2.507934] [<c06f07a4>] (omap2_common_pm_late_init+0x18/0xd0) from [<c06edee0>] (omap3_init_late+0xc/0x18)
> [ 2.518188] [<c06edee0>] (omap3_init_late+0xc/0x18) from [<c06e8504>] (init_machine_late+0x1c/0x28)
> [ 2.527740] [<c06e8504>] (init_machine_late+0x1c/0x28) from [<c0008768>] (do_one_initcall+0x100/0x16c)
> [ 2.537536] [<c0008768>] (do_one_initcall+0x100/0x16c) from [<c06e590c>] (kernel_init_freeable+0xfc/0x1c8)
> [ 2.547698] [<c06e590c>] (kernel_init_freeable+0xfc/0x1c8) from [<c04d26f4>] (kernel_init+0x8/0xe4)
> [ 2.557250] [<c04d26f4>] (kernel_init+0x8/0xe4) from [<c00137b0>] (ret_from_fork+0x14/0x24)
This is exactly the sort of thing which should not ever existed in the first
place...
The code in omap_twl.c, especially the omap3_twl_set_sr_bit() function is
executed in .init_late of the board. If the twl stack is not up by then we are
going to have issues.
Right now I don't see how to solve this (the call chain is related to
smartreflex) but IMHO we should not have these 'random' twl_write calls spread
across the kernel without integrating them to the twl stack in a proper
manner. What I mean is that we should only have twl_* calls from drivers,
which has been created by the twl-core MFD core.
--
Péter
next prev parent reply other threads:[~2013-02-13 13:59 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-16 13:53 [PATCH v2 00/11] MFD: twl-core: Cleanup part 2 for 3.9 Peter Ujfalusi
2013-01-16 13:53 ` Peter Ujfalusi
2013-01-16 13:53 ` [PATCH v2 01/11] ARM: OMAP: zoom-display: Remove the use of TWL4030_MODULE_PWM1 Peter Ujfalusi
2013-01-16 13:53 ` Peter Ujfalusi
2013-01-16 13:53 ` [PATCH v2 02/11] mfd: twl-core: Clean up module id lookup and definitions Peter Ujfalusi
2013-01-16 13:53 ` Peter Ujfalusi
2013-01-16 13:53 ` [PATCH v2 03/11] mfd: twl-core: No need to check for invalid subchip ID Peter Ujfalusi
2013-01-16 13:53 ` Peter Ujfalusi
2013-01-16 13:53 ` [PATCH v2 04/11] mfd: twl-core: Use the lookup table to find the correct subchip for the modules Peter Ujfalusi
2013-01-16 13:53 ` Peter Ujfalusi
2013-01-16 13:53 ` [PATCH v2 05/11] mfd: twl-core: Allocate twl_modules dynamically Peter Ujfalusi
2013-01-16 13:53 ` Peter Ujfalusi
2013-01-16 13:53 ` [PATCH v2 06/11] mfd: twl-core: Do not try to call legacy mfd add_children() when booted with DT Peter Ujfalusi
2013-01-16 13:53 ` Peter Ujfalusi
2013-01-16 13:53 ` [PATCH v2 07/11] mfd: twl-core: Do not create dummy pdata " Peter Ujfalusi
2013-01-16 13:53 ` Peter Ujfalusi
2013-01-16 13:53 ` [PATCH v2 08/11] mfd: twl-core: Move 'inuse' check early at probe time Peter Ujfalusi
2013-01-16 13:53 ` Peter Ujfalusi
2013-01-16 13:53 ` [PATCH v2 09/11] mfd: twl-core: Collect global variables behind one private structure (global) Peter Ujfalusi
2013-01-16 13:53 ` Peter Ujfalusi
2013-02-08 18:56 ` Jon Hunter
2013-02-08 18:56 ` Jon Hunter
2013-02-09 5:50 ` Peter Ujfalusi
2013-02-09 5:50 ` Peter Ujfalusi
2013-02-11 20:22 ` Jon Hunter
2013-02-11 20:22 ` Jon Hunter
2013-02-12 7:26 ` Peter Ujfalusi
2013-02-12 7:26 ` Peter Ujfalusi
2013-02-12 16:15 ` Jon Hunter
2013-02-12 16:15 ` Jon Hunter
2013-02-13 13:59 ` Peter Ujfalusi [this message]
2013-02-13 13:59 ` Peter Ujfalusi
2013-01-16 13:53 ` [PATCH v2 10/11] mfd: twl-core: Remove no longer valid comment regarding to write buffer size Peter Ujfalusi
2013-01-16 13:53 ` Peter Ujfalusi
2013-01-16 13:53 ` [PATCH v2 11/11] mfd: twl-core: Move twl_i2c_read/write_u8 to header as inline function Peter Ujfalusi
2013-01-16 13:53 ` Peter Ujfalusi
2013-02-03 17:01 ` [PATCH v2 00/11] MFD: twl-core: Cleanup part 2 for 3.9 Samuel Ortiz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=511B9C35.907@ti.com \
--to=peter.ujfalusi@ti.com \
--cc=jon-hunter@ti.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=sameo@linux.intel.com \
--cc=t-kristo@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.