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
> [snip]
> hidraw: raw HID events driver (C) Jiri Kosina
> usbcore: registered new interface driver usbhid
> usbhid: USB HID core driver
> 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
> vme_user: VME User Space Access Driver
> [snip]
>
> Thanks for reconsidering.
>
> -Aaron
> _______________________________________________
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
>
--
Martyn Welch (Lead Software Engineer) | Registered in England and Wales
GE Intelligent Platforms | (3828642) at 100 Barbirolli Square
T +44(0)1327322748 | Manchester, M2 3AB
E martyn.we...@ge.com | VAT:GB 927559189
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
