----- Original Message -----
> From: "Martyn Welch" <martyn.we...@ge.com>
> Sent: Thursday, November 7, 2013 7:00:35 AM
>
> On 05/11/13 20:58, Aaron Sierra wrote:
> > ----- Original Message -----
> >> From: "Martyn Welch" <martyn.we...@ge.com>
> >> Subject: Re: [PATCH 1/3] vme_user: Ensure driver compiles after VME
> >> bridges
> >>
> >> On 05/11/13 17:53, Aaron Sierra wrote:
> >>> Martyn,
> >>> Can you please elaborate on why you feel it is not a fix? For instance,
> >>> will this type of solution not be accepted upstream? Is this solution not
> >>> complete enough? Do you feel that it doesn't resolve any issue?
> >>>
> >>> -Aaron
> >>
> >> The vme_user driver is in the staging tree as it is not in a fit state to
> >> incorporate into the main kernel tree. As far as I can see, the below
> >> patch
> >> just causes the driver to be built at a subtly different time, from a
> >> subtly
> >> different location (outside of the staging tree). To be honest, I'm not
> >> sure
> >> how this is fixing anything.
> >>
> >> Martyn
> >
> > Martyn,
> > You are correct about the function of this patch, however, I know that
> > it resolves a real problem despite its inconsequential appearance.
> >
> > Using a 3.10.6 kernel, if I compile vme, vme_tsi148, and vme_user as
> > modules, then attempting to insert vme_user into the kernel before the
> > vme subsystem driver has been loaded results in the module failing to
> > load with the following (this is good):
> >
> > vme_user: Unknown symbol vme_master_read (err 0)
> > vme_user: Unknown symbol vme_master_write (err 0)
> > vme_user: Unknown symbol vme_free_consistent (err 0)
> > vme_user: Unknown symbol vme_register_driver (err 0)
> > vme_user: Unknown symbol vme_irq_generate (err 0)
> > vme_user: Unknown symbol vme_alloc_consistent (err 0)
> > vme_user: Unknown symbol vme_slave_request (err 0)
> > vme_user: Unknown symbol vme_master_free (err 0)
> > vme_user: Unknown symbol vme_master_request (err 0)
> > vme_user: Unknown symbol vme_slave_free (err 0)
> > vme_user: Unknown symbol vme_slave_set (err 0)
> > vme_user: Unknown symbol vme_master_get (err 0)
> > vme_user: Unknown symbol vme_slave_get (err 0)
> > vme_user: Unknown symbol vme_master_set (err 0)
> > vme_user: Unknown symbol vme_unregister_driver (err 0)
> > vme_user: Unknown symbol vme_get_size (err 0)
> >
> > This "Unknown symbol" error forces modules to be loaded in this order
> > at the very least; vme -> vme_user.
> >
> > However, when these modules are compiled into the kernel, unfortunate
> > things start to take place. This is the order that the three VME
> > drivers are compiled when using an unpatched kernel:
> >
> > CC drivers/staging/vme/devices/vme_user.o
> > LD drivers/staging/vme/devices/built-in.o
> > LD drivers/staging/vme/built-in.o
> > LD drivers/staging/built-in.o
> > CC drivers/vme/vme.o
> > CC drivers/vme/bridges/vme_tsi148.o
> > LD drivers/vme/bridges/built-in.o
> > LD drivers/vme/built-in.o
> > LD drivers/built-in.o
> >
> > Note that this is not only the order that the drivers are compiled, but
> > also the order that they are initialized. Also notice that since all
> > three drivers are compiled into the kernel, there are no "Unknown
> > symbols" to prevent the vme_user driver from initializing as if the VME
> > bus subsystem has already been initialized. Therefore the following load
> > order occurs; vme_user -> vme -> vme_tsi148.
> >
> > This is the result of the unpatched initialization order:
> >
> > [snip]
> > hidraw: raw HID events driver (C) Jiri Kosina
> > usbcore: registered new interface driver usbhid
> > usbhid: USB HID core driver
> > vme_user: VME User Space Access Driver
> > ------------[ cut here ]------------
> > kernel BUG at drivers/base/driver.c:169!
> > invalid opcode: 0000 [#1] SMP
> > Modules linked in:
> > CPU: 1 PID: 1 Comm: swapper/0 Not tainted
> > 3.10.6-xes_r3-00018-g300fcda-dirty #112
> > Hardware name: Extreme Engineering Solutions, Inc. (X-ES)
> > XCalibur4331/XCalibur4331, BIOS 1-2.21.1 12/10/2012
> > task: ffff880232888000 ti: ffff880232884000 task.ti: ffff880232884000
> > RIP: 0010:[<ffffffff812ba23a>] [<ffffffff812ba23a>]
> > driver_register+0x1d/0x106
> > RSP: 0000:ffff880232885e18 EFLAGS: 00010246
> > RAX: ffffffff8184ef20 RBX: ffffffff8184ee68 RCX: 0000000000000026
> > RDX: 0000000000000023 RSI: 0000000000000020 RDI: ffffffff8184ee68
> > RBP: ffff880232885e48 R08: 0000000000000001 R09: 0000000000000000
> > R10: 0000000000000000 R11: ffffffff81d5f420 R12: ffffffff8184ee30
> > R13: 0000000000000000 R14: 0000000000000000 R15: ffffffff8184eee0
> > FS: 0000000000000000(0000) GS:ffff88023bc80000(0000)
> > knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> > CR2: 0000000000000000 CR3: 000000000180c000 CR4: 00000000000007e0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> > Stack:
> > 0000000000000001 ffffffff818a576d ffffffff8184ee30 0000000000000000
> > 0000000000000000 ffffffff8184eee0 ffff880232885eb8 ffffffff813a63aa
> > ffff880232885eb8 ffffffff814bc12b ffff880232885eb8 0000002000000008
> > Call Trace:
> > [<ffffffff818a576d>] ? staging_init+0x8/0x8
> > [<ffffffff813a63aa>] vme_register_driver+0x4c/0x23b
> > [<ffffffff814bc12b>] ? printk+0x48/0x4a
> > [<ffffffff818a576d>] ? staging_init+0x8/0x8
> > [<ffffffff818a5790>] vme_user_init+0x23/0x25
> > [<ffffffff81000243>] do_one_initcall+0x7b/0x10c
> > [<ffffffff81880d5e>] kernel_init_freeable+0x101/0x190
> > [<ffffffff818806b0>] ? do_early_param+0x86/0x86
> > [<ffffffff814b7e4b>] ? rest_init+0x6f/0x6f
> > usb 1-1: new high-speed USB device number 2 using ehci-pci
> > [<ffffffff814b7e54>] kernel_init+0x9/0xd1
> > [<ffffffff814c276c>] ret_from_fork+0x7c/0xb0
> > [<ffffffff814b7e4b>] ? rest_init+0x6f/0x6f
> > Code: 9e f0 ff 48 8b 83 90 00 00 00 5a 5b 5d c3 55 48 89 e5 41 57 41 56 41
> > 55 41 54 53 48 89 fb 41 50 48 8b 47 08 48 83 78 78 00 75 02 <0f> 0b 48 83
> > 78 40 00 74 07 48 83 7f 38 00 75 1c 48 83 78 48 00
> > RIP [<ffffffff812ba23a>] driver_register+0x1d/0x106
> > RSP <ffff880232885e18>
> > ---[ end trace 0ccf2d5a87725d2a ]---
> > Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> > [snip]
> >
>
> Ouch - yes I see this as well.
>
> > After applying the patch that I submitted, the three VME drivers are
> > compiled as such:
> >
> > CC drivers/vme/vme.o
> > CC drivers/vme/../staging/vme/devices/vme_user.o
> > LD drivers/vme/../staging/vme/devices/built-in.o
> > LD drivers/vme/../staging/vme/built-in.o
> > CC drivers/vme/bridges/vme_tsi148.o
> > LD drivers/vme/bridges/built-in.o
> > LD drivers/vme/built-in.o
> >
> > You should see that this forces the vme_user driver to compile after the
> > VME subsystem driver and therefore initialize in the necessary order
> > (and without the kernel panic):
> >
>
> The problem with this approach is that it only solves it for the narrow case
> of the contents of drivers/staging/vme/. If we have a GPIO driver, which
> resides in drivers/gpio/, that happens to be a VME device and is compiled
> into
> the kernel, this will also happen. The answer seems to be that we shouldn't
> be
> treating the core subsystem code as a module - it needs to be treated like
> other subsystems. By doing something like:
>
> diff --git a/drivers/vme/Kconfig b/drivers/vme/Kconfig
> index c5c2246..a6a6f95 100644
> --- a/drivers/vme/Kconfig
> +++ b/drivers/vme/Kconfig
> @@ -3,7 +3,7 @@
> #
>
> menuconfig VME_BUS
> - tristate "VME bridge support"
> + bool "VME bridge support"
> depends on PCI
> ---help---
> If you say Y here you get support for the VME bridge Framework.
> diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
> index f6856b4..c4ad939 100644
> --- a/drivers/vme/vme.c
> +++ b/drivers/vme/vme.c
> @@ -1512,9 +1512,5 @@ static void __exit vme_exit(void)
> bus_unregister(&vme_bus_type);
> }
>
> -MODULE_DESCRIPTION("VME bridge driver framework");
> -MODULE_AUTHOR("Martyn Welch <martyn.we...@ge.com");
> -MODULE_LICENSE("GPL");
> -
> -module_init(vme_init);
> +subsys_initcall(vme_init);
> module_exit(vme_exit);
>
> Martyn
Martyn,
Your suggestion resolves the kernel panic associated with the vme_user
driver being initialized before the VME bus core driver, but it doesn't
address the issue of the vme_user driver being registered before any
buses have been probed:
[snip]
usbhid: USB HID core driver
vme_user: VME User Space Access Driver
vme_user: No cards, skipping registration
vme_tsi148 0000:04:04.0: Board is the VME system controller
vme_tsi148 0000:04:04.0: VME geographical address is 1
vme_tsi148 0000:04:04.0: VME Write and flush and error check is disabled
vme_tsi148 0000:04:04.0: CR/CSR Offset: 1
vme_tsi148 0000:04:04.0: Enabling CR/CSR space
[snip]
Resolving the kernel panic seems like a more immediate priority, though, so
I will submit my version of this patch. I plan to include your Signed-off-by
since you suggested the code change, unless you have an argument against
me doing so.
-Aaron
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
