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: 19+ 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 ` [PATCH v2 01/11] ARM: OMAP: zoom-display: Remove the use of TWL4030_MODULE_PWM1 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 ` [PATCH v2 03/11] mfd: twl-core: No need to check for invalid subchip ID 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 ` [PATCH v2 05/11] mfd: twl-core: Allocate twl_modules dynamically 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 ` [PATCH v2 07/11] mfd: twl-core: Do not create dummy pdata " 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 ` [PATCH v2 09/11] mfd: twl-core: Collect global variables behind one private structure (global) Peter Ujfalusi
2013-02-08 18:56 ` Jon Hunter
2013-02-09 5:50 ` Peter Ujfalusi
2013-02-11 20:22 ` Jon Hunter
2013-02-12 7:26 ` Peter Ujfalusi
2013-02-12 16:15 ` Jon Hunter
2013-02-13 13:59 ` Peter Ujfalusi [this message]
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 ` [PATCH v2 11/11] mfd: twl-core: Move twl_i2c_read/write_u8 to header as inline function 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).