Hi Huang,
On Mon, Nov 11, 2013 at 06:40:19PM +0800, Huang Shijie wrote:
> The imx23 board will check the fingerprint, so it will call the
> mx23_check_transcription_stamp. This function will use @chip->buffers->databuf
> as its buffer which is allocated in the nand_scan_tail().
>
> Unfortunately, the mx23_check_transcription_stamp is called before the
> nand_scan_tail(). So we will meet a NULL pointer bug:
>
> --------------------------------------------------------------------
> [ 1.150000] NAND device: Manufacturer ID: 0xec, Chip ID: 0xd7 (Samsung NAND 4GiB 3,3V 8-bit), 4096MiB, page size: 4096, OOB size: 8
> [ 1.160000] Unable to handle kernel NULL pointer dereference at virtual address 000005d0
> [ 1.170000] pgd = c0004000
> [ 1.170000] [000005d0] *pgd=00000000
> [ 1.180000] Internal error: Oops: 5 [#1] ARM
> [ 1.180000] Modules linked in:
> [ 1.180000] CPU: 0 PID: 1 Comm: swapper Not tainted 3.12.0 #89
> [ 1.180000] task: c7440000 ti: c743a000 task.ti: c743a000
> [ 1.180000] PC is at memcmp+0x10/0x54
> [ 1.180000] LR is at gpmi_nand_probe+0x42c/0x894
> [ 1.180000] pc : [<c025fcb0>] lr : [<c02f6a68>] psr: 20000053
> [ 1.180000] sp : c743be2c ip : 600000d3 fp : ffffffff
> [ 1.180000] r10: 000005d0 r9 : c02f5f08 r8 : 00000000
> [ 1.180000] r7 : c75858a8 r6 : c75858a8 r5 : c7585b18 r4 : c7585800
> [ 1.180000] r3 : 000005d0 r2 : 00000004 r1 : c05c33e4 r0 : 000005d0
> [ 1.180000] Flags: nzCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment kernel
> [ 1.180000] Control: 0005317f Table: 40004000 DAC: 00000017
> [ 1.180000] Process swapper (pid: 1, stack limit = 0xc743a1c0)
> --------------------------------------------------------------------
>
> This patch does two things:
> 1.) Allocates the @chip->buffers itself, and set the NAND_OWN_BUFFERS
> for chip->options.
> 2.) Also initialize the @chip->oob_poi which is used by the
> mx23_write_transcription_stamp(). We replace the @chip->ecc.write_page_raw
> with chip->write_buf, since the the @chip->ecc.write_page_raw is
> initialize in the nand_scan_tail() too. If we not do so, we will meet
> a NULL pointer too.
It seems like your GPMI init structure is yet again trying to circumvent
nand_base. I think there is a better solution that can help you avoid
working around things that *should* be initialized in nand_scan_tail.
I'll explain.
Your current init sequence consists of the following:
|_ nand_scan_ident
|_ nand_init_last
|_ gpmi_init_last
|_ gpmi_pre_bbt_scan
|_ gpmi_set_geometry
|_ nand_boot_init
|_ mx23_boot_init
|_ mx23_check_transcription_stamp (*)
|_ mx23_write_transcription_stamp (*)
|_ nand_scan_tail
The (*) portions are incorrect right now. But they also seem to be
related to bad block scanning, not the initial configuration of the
device. Furthermore, they depend on some functionality of
nand_scan_tail.
So I would recommend the following: set the NAND_SKIP_BBTSCAN, then
rearrange to the following call structure:
|_ nand_scan_ident
|_ chip->options |= NAND_SKIP_BBTSCAN;
|_ nand_init_last
|_ gpmi_init_last [1]
|_ gpmi_set_geometry
|_ nand_scan_tail
|_ gpmi_pre_bbt_scan [2]
|_ nand_boot_init
|_ mx23_boot_init
|_ mx23_check_transcription_stamp
|_ mx23_write_transcription_stamp
|_ chip->scan_bbt() [3]
Regarding [1] and [2]: we can split apart the code from gpmi_init_last()
so that have the stuff that is *only* necessary to do before [3] placed
under [2]. That way, nand_scan_tail() will initialize remaining pieces,
and all you have to do is configure and run your BBT scan. No more
dancing around uninitialized callback functions or buffers.
What do you think?
Brian
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html