Re: Using OMAP3 ISP live display and snapshot sample applications

Re: Using OMAP3 ISP live display and snapshot sample applications

[Laurent Pinchart]

Hi James,

(CC'ing Ohad, Joerg and the IOMMU mailing list)

On Wednesday 08 February 2012 16:29:14 James wrote:
>
> I baked a new kernel from your branch using OE & your defconfig but I got a
> "Kernel Panic" when booting it.
> 
> Attached is the console log from the Overo+Tobi board+Caspa setup.
> 
> Can you advise my next course of resolution?

Does the following patch fix the problem ?

diff --git a/drivers/Makefile b/drivers/Makefile
index 1b31421..2ec48f2 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_XEN)		+= xen/
 
 # regulators early, since some subsystems rely on them to initialize
 obj-$(CONFIG_REGULATOR)		+= regulator/
+obj-$(CONFIG_IOMMU_SUPPORT)	+= iommu/
 
 # tty/ comes before char/ so that the VT console is the boot-time
 # default.
@@ -125,7 +126,6 @@ obj-y				+= clk/
 
 obj-$(CONFIG_HWSPINLOCK)	+= hwspinlock/
 obj-$(CONFIG_NFC)		+= nfc/
-obj-$(CONFIG_IOMMU_SUPPORT)	+= iommu/
 
 # Virtualization drivers
 obj-$(CONFIG_VIRT_DRIVERS)	+= virt/

Ohad, Joerg, would this be an acceptable fix ? If so I'll submit a patch.

-- 
Regards,

Laurent Pinchart

Re: Using OMAP3 ISP live display and snapshot sample applications

[Ohad Ben-Cohen]

On Thu, Feb 9, 2012 at 9:25 AM, Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> Ohad, Joerg, would this be an acceptable fix ?

What does this fix ? care to attach the oops ?

Thanks,
Ohad.

Re: Using OMAP3 ISP live display and snapshot sample applications

[Joerg Roedel]

On Thu, Feb 09, 2012 at 08:25:27AM +0100, Laurent Pinchart wrote:
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 1b31421..2ec48f2 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_XEN)		+= xen/
>  
>  # regulators early, since some subsystems rely on them to initialize
>  obj-$(CONFIG_REGULATOR)		+= regulator/
> +obj-$(CONFIG_IOMMU_SUPPORT)	+= iommu/
>  
>  # tty/ comes before char/ so that the VT console is the boot-time
>  # default.
> @@ -125,7 +126,6 @@ obj-y				+= clk/
>  
>  obj-$(CONFIG_HWSPINLOCK)	+= hwspinlock/
>  obj-$(CONFIG_NFC)		+= nfc/
> -obj-$(CONFIG_IOMMU_SUPPORT)	+= iommu/
>  
>  # Virtualization drivers
>  obj-$(CONFIG_VIRT_DRIVERS)	+= virt/
> 
> Ohad, Joerg, would this be an acceptable fix ? If so I'll submit a patch.

I also want to understand the problem first. Can you provide some
context?


	Joerg


-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: Using OMAP3 ISP live display and snapshot sample applications

[Joerg Roedel]

On Mon, Feb 13, 2012 at 09:28:30AM +0800, James wrote:

> >> @@ -28,6 +28,7 @@ obj-$(CONFIG_XEN)           += xen/
> >>
> >>  # regulators early, since some subsystems rely on them to initialize
> >>  obj-$(CONFIG_REGULATOR)              += regulator/
> >> +obj-$(CONFIG_IOMMU_SUPPORT)  += iommu/
> >>
> >>  # tty/ comes before char/ so that the VT console is the boot-time
> >>  # default.
> >> @@ -125,7 +126,6 @@ obj-y                             += clk/
> >>
> >>  obj-$(CONFIG_HWSPINLOCK)     += hwspinlock/
> >>  obj-$(CONFIG_NFC)            += nfc/
> >> -obj-$(CONFIG_IOMMU_SUPPORT)  += iommu/
> >>
> >>  # Virtualization drivers
> >>  obj-$(CONFIG_VIRT_DRIVERS)   += virt/

I don't see how this patch solves a problem like below.

> [    3.663269] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> [    3.671844] pgd = c0004000
> [    3.674682] [00000058] *pgd=00000000
> [    3.678527] Internal error: Oops: 5 [#1]
> [    3.682647] Modules linked in:
> [    3.685852] CPU: 0    Tainted: G        W     (3.2.0-rc2 #1)
> [    3.691802] PC is at klist_next+0x10/0xb4
> [    3.696014] LR is at next_device+0x8/0x14
> [    3.700225] pc : [<c03eeac4>]    lr : [<c024deb8>]    psr: 60000013
> [    3.700225] sp : de82de98  ip : 00000000  fp : c04300c0
> [    3.712249] r10: 000003ff  r9 : c05e2c48  r8 : 00000001
> [    3.717742] r7 : c078ed68  r6 : 00000000  r5 : de82deb8  r4 : de82deb8
> [    3.724578] r3 : c031ff60  r2 : 00000000  r1 : de82deb8  r0 : 00000024
> [    3.731414] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> [    3.739074] Control: 10c5387d  Table: 80004019  DAC: 00000015
> [    3.745117] Process swapper (pid: 1, stack limit = 0xde82c2f0)
> [    3.751220] Stack: (0xde82de98 to 0xde82e000)
> [    3.755798] de80:                                                       c031ff60 c04f8ff4
> [    3.764373] dea0: de82deb8 c031ff60 c078ed68 c024deb8 c031ff60 c024df28 00000024 00000000
> [    3.772949] dec0: c04f8ff4 debe8000 00007990 c05e2c40 0000000e c02e9578 22222222 debd1660
> [    3.781524] dee0: 00000000 c011801c c05e2c48 c0056368 debe8580 00000003 de893e80 00000000
> [    3.790100] df00: c05e2c7c c05e2c48 c05e2c7c c078c144 c078c144 00000000 00000000 00000000
> [    3.798675] df20: 00000000 c024e800 c024e7ec c024d720 00000000 c05e2c48 c05e2c7c c078c144
> [    3.807281] df40: 00000000 c024d844 c078c144 de82df58 c024d7dc c024c9a0 de825060 de888898
> [    3.815856] df60: c078c144 c078c144 debd3ec0 c0781a10 00000000 c024d02c c04f8fef c05fc430
> [    3.824432] df80: de81b040 c078c144 c05d9168 c000dc9c c05c4068 00000000 00000000 c024dd88
> [    3.833007] dfa0: 00000000 c05d904c c05d9168 c000dc9c c05c4068 00000000 00000000 c00087cc
> [    3.841583] dfc0: c05c4068 de81b040 c0797fc0 00000000 c05d904c c05d9168 c000dc9c 00000013
> [    3.850158] dfe0: 00000000 c05a77c8 de81b040 00000000 c05a7750 c000dc9c 00000000 00002000
> [    3.858764] [<c03eeac4>] (klist_next+0x10/0xb4) from [<c024deb8>] (next_device+0x8/0x14)
> [    3.867248] [<c024deb8>] (next_device+0x8/0x14) from [<c024df28>] (driver_find_device+0x64/0x7c)
> [    3.876464] [<c024df28>] (driver_find_device+0x64/0x7c) from [<c02e9578>] (isp_probe+0x228/0xa34)
> [    3.885772] [<c02e9578>] (isp_probe+0x228/0xa34) from [<c024e800>] (platform_drv_probe+0x14/0x18)
> [    3.895111] [<c024e800>] (platform_drv_probe+0x14/0x18) from [<c024d720>] (driver_probe_device+0xc8/0x184)
> [    3.905212] [<c024d720>] (driver_probe_device+0xc8/0x184) from [<c024d844>] (__driver_attach+0x68/0x8c)
> [    3.915100] [<c024d844>] (__driver_attach+0x68/0x8c) from [<c024c9a0>] (bus_for_each_dev+0x4c/0x78)
> [    3.924591] [<c024c9a0>] (bus_for_each_dev+0x4c/0x78) from [<c024d02c>] (bus_add_driver+0x9c/0x220)
> [    3.934082] [<c024d02c>] (bus_add_driver+0x9c/0x220) from [<c024dd88>] (driver_register+0xa0/0x120)
> [    3.943572] [<c024dd88>] (driver_register+0xa0/0x120) from [<c00087cc>] (do_one_initcall+0x90/0x164)
> [    3.953155] [<c00087cc>] (do_one_initcall+0x90/0x164) from [<c05a77c8>] (kernel_init+0x78/0x11c)
> [    3.962402] [<c05a77c8>] (kernel_init+0x78/0x11c) from [<c000dc9c>] (kernel_thread_exit+0x0/0x8)
> [    3.971618] Code: e92d40f8 e1a04000 e5900000 e5946004 (e5907034) 
> [    3.978057] ---[ end trace 1b75b31a2719ed1e ]---
> [    3.983001] Kernel panic - not syncing: Attempted to kill init!

This looks more like data corruption or something. But this is definitly
not caused by some random order in the Makefile.
Please debug this issue down to the real cause. Bisecting may help here.


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: Using OMAP3 ISP live display and snapshot sample applications

[Laurent Pinchart]

Hi Joerg,

On Tuesday 14 February 2012 11:44:37 Joerg Roedel wrote:
> On Mon, Feb 13, 2012 at 09:28:30AM +0800, James wrote:
> > >> @@ -28,6 +28,7 @@ obj-$(CONFIG_XEN)           += xen/
> > >> 
> > >>  # regulators early, since some subsystems rely on them to initialize
> > >>  obj-$(CONFIG_REGULATOR)              += regulator/
> > >> +obj-$(CONFIG_IOMMU_SUPPORT)  += iommu/
> > >> 
> > >>  # tty/ comes before char/ so that the VT console is the boot-time
> > >>  # default.
> > >> @@ -125,7 +126,6 @@ obj-y                             += clk/
> > >> 
> > >>  obj-$(CONFIG_HWSPINLOCK)     += hwspinlock/
> > >>  obj-$(CONFIG_NFC)            += nfc/
> > >> -obj-$(CONFIG_IOMMU_SUPPORT)  += iommu/
> > >> 
> > >>  # Virtualization drivers
> > >>  obj-$(CONFIG_VIRT_DRIVERS)   += virt/
> 
> I don't see how this patch solves a problem like below.
> 
> > [    3.663269] Unable to handle kernel NULL pointer dereference at virtual
> > address 00000058

[snip]

> > [<c03eeac4>] (klist_next+0x10/0xb4) from [<c024deb8>]
> > (next_device+0x8/0x14)
> > [<c024deb8>] (next_device+0x8/0x14) from [<c024df28>]
> > (driver_find_device+0x64/0x7c)
> > [<c024df28>] (driver_find_device+0x64/0x7c) from [<c02e9578>]
> > (isp_probe+0x228/0xa34)

[snip]

> This looks more like data corruption or something. But this is definitly
> not caused by some random order in the Makefile.
> Please debug this issue down to the real cause. Bisecting may help here.

There's actually a line missing from the backtrace for some reason.

isp_probe() calls iommu_attach_device(), which ends up calling 
omap_iommu_attach(). That function then calls driver_find_device(), passing it 
omap_iommu_driver.driver as its first argument. However, as the OMAP3 ISP 
driver is linked in the kernel before the OMAP3 IOMMU driver, 
omap_iommu_init() has not be called yet by this time. omap_iommu_driver.driver 
is thus uninitialized, and klist iteration oopses as the linked list head is 
NULL.

-- 
Regards,

Laurent Pinchart

Re: Using OMAP3 ISP live display and snapshot sample applications

[Laurent Pinchart]

Hi Joerg and Ohad,

Ping ? I've discussed this with Ohad last week, and we agreed that there was 
indeed an issue with the IOMMU subsystem and/or the OMAP3 IOMMU driver. Could 
you please have a look at my explanation below ?

On Thursday 16 February 2012 07:46:58 Laurent Pinchart wrote:
> On Tuesday 14 February 2012 11:44:37 Joerg Roedel wrote:
> > On Mon, Feb 13, 2012 at 09:28:30AM +0800, James wrote:
> > > >> @@ -28,6 +28,7 @@ obj-$(CONFIG_XEN)           += xen/
> > > >> 
> > > >>  # regulators early, since some subsystems rely on them to initialize
> > > >>  obj-$(CONFIG_REGULATOR)              += regulator/
> > > >> 
> > > >> +obj-$(CONFIG_IOMMU_SUPPORT)  += iommu/
> > > >> 
> > > >>  # tty/ comes before char/ so that the VT console is the boot-time
> > > >>  # default.
> > > >> 
> > > >> @@ -125,7 +126,6 @@ obj-y                             += clk/
> > > >> 
> > > >>  obj-$(CONFIG_HWSPINLOCK)     += hwspinlock/
> > > >>  obj-$(CONFIG_NFC)            += nfc/
> > > >> 
> > > >> -obj-$(CONFIG_IOMMU_SUPPORT)  += iommu/
> > > >> 
> > > >>  # Virtualization drivers
> > > >>  obj-$(CONFIG_VIRT_DRIVERS)   += virt/
> > 
> > I don't see how this patch solves a problem like below.
> > 
> > > [    3.663269] Unable to handle kernel NULL pointer dereference at
> > > virtual
> > > address 00000058
> 
> [snip]
> 
> > > [<c03eeac4>] (klist_next+0x10/0xb4) from [<c024deb8>]
> > > (next_device+0x8/0x14)
> > > [<c024deb8>] (next_device+0x8/0x14) from [<c024df28>]
> > > (driver_find_device+0x64/0x7c)
> > > [<c024df28>] (driver_find_device+0x64/0x7c) from [<c02e9578>]
> > > (isp_probe+0x228/0xa34)
> 
> [snip]
> 
> > This looks more like data corruption or something. But this is definitly
> > not caused by some random order in the Makefile.
> > Please debug this issue down to the real cause. Bisecting may help here.
> 
> There's actually a line missing from the backtrace for some reason.
> 
> isp_probe() calls iommu_attach_device(), which ends up calling
> omap_iommu_attach(). That function then calls driver_find_device(), passing
> it omap_iommu_driver.driver as its first argument. However, as the OMAP3
> ISP driver is linked in the kernel before the OMAP3 IOMMU driver,
> omap_iommu_init() has not be called yet by this time.
> omap_iommu_driver.driver is thus uninitialized, and klist iteration oopses
> as the linked list head is NULL.

-- 
Regards,

Laurent Pinchart

Re: Using OMAP3 ISP live display and snapshot sample applications

[Ohad Ben-Cohen]

Hi Laurent,

On Thu, Feb 23, 2012 at 6:38 PM, Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> Ping ? I've discussed this with Ohad last week, and we agreed that there was
> indeed an issue with the IOMMU subsystem and/or the OMAP3 IOMMU driver. Could
> you please have a look at my explanation below ?

Oops - for some reason I didn't realize I had an action item, my bad :)

Can you try the below and tell me if it helps ?

diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 609ea2d..f992395 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -412,7 +412,7 @@ static void __exit omap2_mbox_exit(void)
        platform_driver_unregister(&omap2_mbox_driver);
 }

-module_init(omap2_mbox_init);
+subsys_initcall(omap2_mbox_init);
 module_exit(omap2_mbox_exit);

 MODULE_LICENSE("GPL v2");


If it works for you I'd prefer something in those lines, because it
makes the problem (and its solution) more visible/explicit.

Anyway I'd only consider this as an interim solution until the
deferred probing mechanism (by Grant) is merged.

Thanks!
Ohad.

Re: Using OMAP3 ISP live display and snapshot sample applications

[Ohad Ben-Cohen]

On Thu, Feb 23, 2012 at 6:59 PM, Ohad Ben-Cohen <ohad-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org> wrote:
> diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
> index 609ea2d..f992395 100644
> --- a/arch/arm/mach-omap2/mailbox.c
> +++ b/arch/arm/mach-omap2/mailbox.c
> @@ -412,7 +412,7 @@ static void __exit omap2_mbox_exit(void)
>        platform_driver_unregister(&omap2_mbox_driver);
>  }
>
> -module_init(omap2_mbox_init);
> +subsys_initcall(omap2_mbox_init);
>  module_exit(omap2_mbox_exit);
>
>  MODULE_LICENSE("GPL v2");

And this, too:

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 4e661f0..95adb7e 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1222,7 +1222,7 @@ static int __init omap_iommu_init(void)

        return platform_driver_register(&omap_iommu_driver);
 }
-module_init(omap_iommu_init);
+subsys_initcall(omap_iommu_init);

 static void __exit omap_iommu_exit(void)
 {

Re: Using OMAP3 ISP live display and snapshot sample applications

[Joerg Roedel]

On Thu, Feb 23, 2012 at 07:01:05PM +0200, Ohad Ben-Cohen wrote:
> On Thu, Feb 23, 2012 at 6:59 PM, Ohad Ben-Cohen <ohad-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org> wrote:
> > diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
> > index 609ea2d..f992395 100644
> > --- a/arch/arm/mach-omap2/mailbox.c
> > +++ b/arch/arm/mach-omap2/mailbox.c
> > @@ -412,7 +412,7 @@ static void __exit omap2_mbox_exit(void)
> >        platform_driver_unregister(&omap2_mbox_driver);
> >  }
> >
> > -module_init(omap2_mbox_init);
> > +subsys_initcall(omap2_mbox_init);
> >  module_exit(omap2_mbox_exit);
> >
> >  MODULE_LICENSE("GPL v2");
> 
> And this, too:
> 
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index 4e661f0..95adb7e 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -1222,7 +1222,7 @@ static int __init omap_iommu_init(void)
> 
>         return platform_driver_register(&omap_iommu_driver);
>  }
> -module_init(omap_iommu_init);
> +subsys_initcall(omap_iommu_init);
> 
>  static void __exit omap_iommu_exit(void)
>  {

These patches make a lot more sense to me than fixing this by relying on
some random linker behavior. Can you please send a fix like this with a
proper commit-msg?

Thanks,

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: Using OMAP3 ISP live display and snapshot sample applications

[Ohad Ben-Cohen]

On Fri, Feb 24, 2012 at 1:01 PM, Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org> wrote:
> These patches make a lot more sense to me than fixing this by relying on
> some random linker behavior. Can you please send a fix like this with a
> proper commit-msg?

Done, thanks.