linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* soc: imx: gpcv2: removing and probing fails
@ 2018-01-07 10:48 Stefan Agner
  2018-01-08  0:22 ` Andrey Smirnov
  2018-01-09 14:24 ` Lucas Stach
  0 siblings, 2 replies; 9+ messages in thread
From: Stefan Agner @ 2018-01-07 10:48 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Andrew,

I noticed that the driver fails when removing and probing again. As far
as I can see due to duplicate add of the platform devices.

As far as I can tell the driver should register the remove callback and
do a platform_device_unregister on the newly created platform devices.
However, as far as I can tell we don't hold on to a reference to them...
I guess we could keep references in imx_gpcv2_probe, but maybe there is
an easier way?

This can be reproduced by using:
CONFIG_DEBUG_TEST_DRIVER_REMOVE=y

The full stack trace:

[    0.673113] ------------[ cut here ]------------
[    0.676786] WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31
sysfs_warn_dup+0x64/0x74
[    0.686186] sysfs: cannot create duplicate filename
'/devices/platform/soc/30000000.aips-bus/303a0000.gpc/imx7-pgc-domain.1'
[    0.698594] Modules linked in:
[    0.700783] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.15.0-rc3-00027-g3b196a7dd3bd-dirty #220
[    0.711659] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.716746] [<8010f55c>] (unwind_backtrace) from [<8010b8e4>]
(show_stack+0x10/0x14)
[    0.726785] [<8010b8e4>] (show_stack) from [<80830124>]
(dump_stack+0x88/0x9c)
[    0.735813] [<80830124>] (dump_stack) from [<8011e58c>]
(__warn+0xdc/0xf4)
[    0.742232] [<8011e58c>] (__warn) from [<8011e5dc>]
(warn_slowpath_fmt+0x38/0x48)
[    0.752034] [<8011e5dc>] (warn_slowpath_fmt) from [<80276600>]
(sysfs_warn_dup+0x64/0x74)
[    0.762167] [<80276600>] (sysfs_warn_dup) from [<802766d8>]
(sysfs_create_dir_ns+0x84/0x90)
[    0.772513] [<802766d8>] (sysfs_create_dir_ns) from [<80834660>]
(kobject_add_internal+0xb4/0x30c)
[    0.783562] [<80834660>] (kobject_add_internal) from [<80834904>]
(kobject_add+0x4c/0x9c)
[    0.793846] [<80834904>] (kobject_add) from [<8050bdd0>]
(device_add+0xe0/0x594)
[    0.803418] [<8050bdd0>] (device_add) from [<80510178>]
(platform_device_add+0x110/0x224)
[    0.813805] [<80510178>] (platform_device_add) from [<8049bfa0>]
(imx_gpcv2_probe+0xdc/0x1f8)
[    0.824592] [<8049bfa0>] (imx_gpcv2_probe) from [<80510364>]
(platform_drv_probe+0x50/0xac)
[    0.835264] [<80510364>] (platform_drv_probe) from [<8050e9ac>]
(driver_probe_device+0x1b4/0x3c8)
[    0.846525] [<8050e9ac>] (driver_probe_device) from [<8050ec64>]
(__driver_attach+0xa4/0xa8)
[    0.857356] [<8050ec64>] (__driver_attach) from [<8050cd88>]
(bus_for_each_dev+0x4c/0x9c)
[    0.867935] [<8050cd88>] (bus_for_each_dev) from [<8050df40>]
(bus_add_driver+0x188/0x20c)
[    0.878650] [<8050df40>] (bus_add_driver) from [<8050f554>]
(driver_register+0x78/0xf4)
[    0.889101] [<8050f554>] (driver_register) from [<80101a5c>]
(do_one_initcall+0x44/0x168)
[    0.899733] [<80101a5c>] (do_one_initcall) from [<80c00db8>]
(kernel_init_freeable+0x14c/0x1d8)
[    0.910899] [<80c00db8>] (kernel_init_freeable) from [<80842638>]
(kernel_init+0x8/0x10c)
[    0.921529] [<80842638>] (kernel_init) from [<80107988>]
(ret_from_fork+0x14/0x2c)
[    0.931569] ---[ end trace 27014f64d1c1710e ]---
[    0.935967] ------------[ cut here ]------------
[    0.940537] WARNING: CPU: 0 PID: 1 at lib/kobject.c:240
kobject_add_internal+0x278/0x30c
[    0.951181] kobject_add_internal failed for imx7-pgc-domain.1 with
-EEXIST, don't try to register things with the same name in the same
directory.
[    0.966666] Modules linked in:
[    0.969363] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W       
4.15.0-rc3-00027-g3b196a7dd3bd-dirty #220
[    0.981886] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.987266] [<8010f55c>] (unwind_backtrace) from [<8010b8e4>]
(show_stack+0x10/0x14)
[    0.997423] [<8010b8e4>] (show_stack) from [<80830124>]
(dump_stack+0x88/0x9c)
[    1.006649] [<80830124>] (dump_stack) from [<8011e58c>]
(__warn+0xdc/0xf4)
[    1.013159] [<8011e58c>] (__warn) from [<8011e5dc>]
(warn_slowpath_fmt+0x38/0x48)
[    1.022994] [<8011e5dc>] (warn_slowpath_fmt) from [<80834824>]
(kobject_add_internal+0x278/0x30c)
[    1.033847] [<80834824>] (kobject_add_internal) from [<80834904>]
(kobject_add+0x4c/0x9c)
[    1.043990] [<80834904>] (kobject_add) from [<8050bdd0>]
(device_add+0xe0/0x594)
[    1.053333] [<8050bdd0>] (device_add) from [<80510178>]
(platform_device_add+0x110/0x224)
[    1.063508] [<80510178>] (platform_device_add) from [<8049bfa0>]
(imx_gpcv2_probe+0xdc/0x1f8)
[    1.074202] [<8049bfa0>] (imx_gpcv2_probe) from [<80510364>]
(platform_drv_probe+0x50/0xac)
[    1.084735] [<80510364>] (platform_drv_probe) from [<8050e9ac>]
(driver_probe_device+0x1b4/0x3c8)
[    1.095818] [<8050e9ac>] (driver_probe_device) from [<8050ec64>]
(__driver_attach+0xa4/0xa8)
[    1.106556] [<8050ec64>] (__driver_attach) from [<8050cd88>]
(bus_for_each_dev+0x4c/0x9c)
[    1.117104] [<8050cd88>] (bus_for_each_dev) from [<8050df40>]
(bus_add_driver+0x188/0x20c)
[    1.127752] [<8050df40>] (bus_add_driver) from [<8050f554>]
(driver_register+0x78/0xf4)
[    1.138150] [<8050f554>] (driver_register) from [<80101a5c>]
(do_one_initcall+0x44/0x168)
[    1.148780] [<80101a5c>] (do_one_initcall) from [<80c00db8>]
(kernel_init_freeable+0x14c/0x1d8)
[    1.159946] [<80c00db8>] (kernel_init_freeable) from [<80842638>]
(kernel_init+0x8/0x10c)
[    1.170577] [<80842638>] (kernel_init) from [<80107988>]
(ret_from_fork+0x14/0x2c)
[    1.180607] ---[ end trace 27014f64d1c1710f ]---
[    1.185014] ------------[ cut here ]------------
[    1.189571] Kernel BUG at 5168736d [verbose debug info unavailable]
[    1.195868] Internal error: Oops - BUG: 0 [#1] SMP ARM
[    1.200985] Modules linked in:
[    1.203978] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W       
4.15.0-rc3-00027-g3b196a7dd3bd-dirty #220
[    1.216449] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    1.221768] PC is at kfree+0xfc/0x140
[    1.225376] LR is at platform_device_release+0x10/0x34
[    1.230510] pc : [<801fd494>]    lr : [<8050ff50>]    psr: 40000013
[    1.236804] sp : ac079df0  ip : 00000000  fp : ffffffef
[    1.242008] r10: ac225400  r9 : ac225000  r8 : 80d23fa0
[    1.247201] r7 : af770d04  r6 : 00000000  r5 : ac225410  r4 :
ac225410
[    1.253741] r3 : af794494  r2 : af794480  r1 : a0000013  r0 :
80d241a0
[    1.260252] Flags: nZcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM 
Segment none
[    1.267425] Control: 10c5387d  Table: 8000406a  DAC: 00000051
[    1.273163] Process swapper/0 (pid: 1, stack limit = 0x7ec51c00)
[    1.279174] Stack: (0xac079df0 to 0xac07a000)
[    1.283497] 9de0:                                     ac225410
ac225418 ac225410 ac225410
[    1.293629] 9e00: ac225410 00000000 af770d04 8050ff50 ac225418
80509c4c ac225418 80d2a744
[    1.302856] 9e20: ac2a68c0 80834038 80aa8500 af770d04 af770de8
ac11fa10 80aa8500 8049c078
[    1.312114] 9e40: 00000000 00000000 ac32d180 00000001 80b40a88
ac11fa10 fffffffe 80d24010
[    1.321543] 9e60: fffffdfb 80d24010 80daef28 00000000 00000000
80510364 ac11fa10 00000000
[    1.331081] 9e80: 80daef24 00000000 80d24010 8050e9ac af770b0c
00000000 000000d7 ac11fa10
[    1.340735] 9ea0: 80d24010 ac11fa44 00000000 000000d7 80c5b83c
80c6c578 00000000 8050ec64
[    1.350475] 9ec0: 00000000 80d24010 8050ebc0 8050cd88 ac041758
ac114ab4 80d24010 ac2a3400
[    1.360365] 9ee0: 80d2ab20 8050df40 80afeb04 80c35e1c 80d24010
80d24010 00000000 80c35e4c
[    1.370388] 9f00: 80d58600 8050f554 ffffe000 00000000 80c35e4c
80101a5c 80b949dc 000000d7
[    1.380600] 9f20: 00000000 8013a238 00000000 80b1d70c 00000006
00000006 80aada0c 00000000
[    1.390896] 9f40: 80ab6b90 80aada80 affffc8a affffc92 00000000
00000007 80d58600 80c5b830
[    1.401286] 9f60: 00000007 80d58600 80c5b834 80d58600 000000d7
80c00db8 00000006 00000006
[    1.411792] 9f80: 00000000 80c005b0 00000000 80842630 00000000
00000000 00000000 00000000
[    1.422343] 9fa0: 00000000 80842638 00000000 80107988 00000000
00000000 00000000 00000000
[    1.432895] 9fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[    1.443458] 9fe0: 00000000 00000000 00000000 00000000 00000013
00000000 bf73ffff 4c5eafe0
[    1.454043] [<801fd494>] (kfree) from [<8050ff50>]
(platform_device_release+0x10/0x34)
[    1.464336] [<8050ff50>] (platform_device_release) from [<80509c4c>]
(device_release+0x2c/0x90)
[    1.475435] [<80509c4c>] (device_release) from [<80834038>]
(kobject_put+0x94/0xe4)
[    1.485474] [<80834038>] (kobject_put) from [<8049c078>]
(imx_gpcv2_probe+0x1b4/0x1f8)
[    1.495784] [<8049c078>] (imx_gpcv2_probe) from [<80510364>]
(platform_drv_probe+0x50/0xac)
[    1.506544] [<80510364>] (platform_drv_probe) from [<8050e9ac>]
(driver_probe_device+0x1b4/0x3c8)
[    1.517844] [<8050e9ac>] (driver_probe_device) from [<8050ec64>]
(__driver_attach+0xa4/0xa8)
[    1.528708] [<8050ec64>] (__driver_attach) from [<8050cd88>]
(bus_for_each_dev+0x4c/0x9c)
[    1.539313] [<8050cd88>] (bus_for_each_dev) from [<8050df40>]
(bus_add_driver+0x188/0x20c)
[    1.550011] [<8050df40>] (bus_add_driver) from [<8050f554>]
(driver_register+0x78/0xf4)
[    1.560449] [<8050f554>] (driver_register) from [<80101a5c>]
(do_one_initcall+0x44/0x168)
[    1.571073] [<80101a5c>] (do_one_initcall) from [<80c00db8>]
(kernel_init_freeable+0x14c/0x1d8)
[    1.582239] [<80c00db8>] (kernel_init_freeable) from [<80842638>]
(kernel_init+0x8/0x10c)
[    1.592874] [<80842638>] (kernel_init) from [<80107988>]
(ret_from_fork+0x14/0x2c)
[    1.602885] Code: 1a000003 e5923014 e3130001 1a000000 (e7f001f2)
[    1.608796] ---[ end trace 27014f64d1c17110 ]---
[    1.613635] Kernel panic - not syncing: Attempted to kill init!
exitcode=0x0000000b
[    1.613635]
[    1.627569] ---[ end Kernel panic - not syncing: Attempted to kill
init! exitcode=0x0000000b
[    1.627569]

--
Stefan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* soc: imx: gpcv2: removing and probing fails
  2018-01-07 10:48 soc: imx: gpcv2: removing and probing fails Stefan Agner
@ 2018-01-08  0:22 ` Andrey Smirnov
  2018-01-08  6:24   ` Andrey Smirnov
  2018-01-09 14:24 ` Lucas Stach
  1 sibling, 1 reply; 9+ messages in thread
From: Andrey Smirnov @ 2018-01-08  0:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Jan 7, 2018 at 2:48 AM, Stefan Agner <stefan@agner.ch> wrote:
> Hi Andrew,
>
> I noticed that the driver fails when removing and probing again. As far
> as I can see due to duplicate add of the platform devices.
>
> As far as I can tell the driver should register the remove callback and
> do a platform_device_unregister on the newly created platform devices.
> However, as far as I can tell we don't hold on to a reference to them...
> I guess we could keep references in imx_gpcv2_probe, but maybe there is
> an easier way?

Stefan:

Good catch and sorry for the inconvenience. I just spent a little bit
of time repro-ing this and it looks like there are two separate bugs,
actually. First one, as you correctly pointed out, is due to
re-registration of pm-domain platform drivers. That, however, should
only result in a WARNING and a failed driver probing, not in a killed
init due to BUG. So the second one, that BUG message in the stack
trace, is due to the fact that I incorrectly provide statically
allocated data via dev.platform_data and it ends up being kfree'd in
platform_device_release().

IMHO, this driver isn't really meant to be removed, so the simplest
solution to the first problem would be to specify
"imx_gpc_driver.driver.suppress_bind_attrs = true" and remove any
option to remove the driver, but I don't know if that's acceptable or
not.

Shawn, would the above be acceptable upstream?

Solution for bug #2 is trivial and I'll send patches for both once we
agree how to fix #1.

Thanks,
Andrey Smirnov

P.S: Also, since I based my code on gpc.c, I suspect that driver will
have exactly the same problem (I'll do some experiments to confirm)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* soc: imx: gpcv2: removing and probing fails
  2018-01-08  0:22 ` Andrey Smirnov
@ 2018-01-08  6:24   ` Andrey Smirnov
  2018-01-08 21:12     ` Stefan Agner
  0 siblings, 1 reply; 9+ messages in thread
From: Andrey Smirnov @ 2018-01-08  6:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Jan 7, 2018 at 4:22 PM, Andrey Smirnov <andrew.smirnov@gmail.com> wrote:
> On Sun, Jan 7, 2018 at 2:48 AM, Stefan Agner <stefan@agner.ch> wrote:
>> Hi Andrew,
>>
>> I noticed that the driver fails when removing and probing again. As far
>> as I can see due to duplicate add of the platform devices.
>>
>> As far as I can tell the driver should register the remove callback and
>> do a platform_device_unregister on the newly created platform devices.
>> However, as far as I can tell we don't hold on to a reference to them...
>> I guess we could keep references in imx_gpcv2_probe, but maybe there is
>> an easier way?
>
> Stefan:
>
> Good catch and sorry for the inconvenience. I just spent a little bit
> of time repro-ing this and it looks like there are two separate bugs,
> actually. First one, as you correctly pointed out, is due to
> re-registration of pm-domain platform drivers. That, however, should
> only result in a WARNING and a failed driver probing, not in a killed
> init due to BUG. So the second one, that BUG message in the stack
> trace, is due to the fact that I incorrectly provide statically
> allocated data via dev.platform_data and it ends up being kfree'd in
> platform_device_release().
>
> IMHO, this driver isn't really meant to be removed, so the simplest
> solution to the first problem would be to specify
> "imx_gpc_driver.driver.suppress_bind_attrs = true" and remove any
> option to remove the driver, but I don't know if that's acceptable or
> not.
>
> Shawn, would the above be acceptable upstream?
>
> Solution for bug #2 is trivial and I'll send patches for both once we
> agree how to fix #1.
>
> Thanks,
> Andrey Smirnov
>
> P.S: Also, since I based my code on gpc.c, I suspect that driver will
> have exactly the same problem (I'll do some experiments to confirm)

Done with experiments. Same problem happens with gpc.c as well.

Andrey Smirnov

^ permalink raw reply	[flat|nested] 9+ messages in thread

* soc: imx: gpcv2: removing and probing fails
  2018-01-08  6:24   ` Andrey Smirnov
@ 2018-01-08 21:12     ` Stefan Agner
  0 siblings, 0 replies; 9+ messages in thread
From: Stefan Agner @ 2018-01-08 21:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 2018-01-08 07:24, Andrey Smirnov wrote:
> On Sun, Jan 7, 2018 at 4:22 PM, Andrey Smirnov <andrew.smirnov@gmail.com> wrote:
>> On Sun, Jan 7, 2018 at 2:48 AM, Stefan Agner <stefan@agner.ch> wrote:
>>> Hi Andrew,
>>>
>>> I noticed that the driver fails when removing and probing again. As far
>>> as I can see due to duplicate add of the platform devices.
>>>
>>> As far as I can tell the driver should register the remove callback and
>>> do a platform_device_unregister on the newly created platform devices.
>>> However, as far as I can tell we don't hold on to a reference to them...
>>> I guess we could keep references in imx_gpcv2_probe, but maybe there is
>>> an easier way?
>>
>> Stefan:
>>
>> Good catch and sorry for the inconvenience. I just spent a little bit
>> of time repro-ing this and it looks like there are two separate bugs,
>> actually. First one, as you correctly pointed out, is due to
>> re-registration of pm-domain platform drivers. That, however, should
>> only result in a WARNING and a failed driver probing, not in a killed
>> init due to BUG. So the second one, that BUG message in the stack
>> trace, is due to the fact that I incorrectly provide statically
>> allocated data via dev.platform_data and it ends up being kfree'd in
>> platform_device_release().
>>
>> IMHO, this driver isn't really meant to be removed, so the simplest
>> solution to the first problem would be to specify
>> "imx_gpc_driver.driver.suppress_bind_attrs = true" and remove any
>> option to remove the driver, but I don't know if that's acceptable or
>> not.
>>
>> Shawn, would the above be acceptable upstream?
>>
>> Solution for bug #2 is trivial and I'll send patches for both once we
>> agree how to fix #1.
>>
>> Thanks,
>> Andrey Smirnov
>>
>> P.S: Also, since I based my code on gpc.c, I suspect that driver will
>> have exactly the same problem (I'll do some experiments to confirm)
> 
> Done with experiments. Same problem happens with gpc.c as well.

Yeah gpc.c also has another problem:
https://patchwork.kernel.org/patch/10148315/

But, yeah, you are right, even with that patch applied and when using
CONFIG_DEBUG_TEST_DRIVER_REMOVE=y it seems to show the problem as well.

I like the config symbol to test my own drivers, its just unfortunate
when mainline blows by default... But then, maybe it is the reason why
that config symbol got a UNSTABLE flag.

The full splash:
[    0.697548] ------------[ cut here ]------------
[    0.702212] WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31
sysfs_warn_dup+0x64/0x74
[    0.709695] sysfs: cannot create duplicate filename
'/devices/soc0/soc/2000000.aips-bus/20dc000.gpc/imx-pgc-power-domain.0'
[    0.720887] Modules linked in:
[    0.723965] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.15.0-rc3-00061-g38f99f72e8f3-dirty #246
[    0.732694] Hardware name: Freescale i.MX6 Quad/DualLite (Device
Tree)
[    0.739263] [<8010f61c>] (unwind_backtrace) from [<8010b984>]
(show_stack+0x10/0x14)
[    0.747045] [<8010b984>] (show_stack) from [<80832024>]
(dump_stack+0x88/0x9c)
[    0.754308] [<80832024>] (dump_stack) from [<8011e7ac>]
(__warn+0xdc/0xf4)
[    0.761214] [<8011e7ac>] (__warn) from [<8011e7fc>]
(warn_slowpath_fmt+0x38/0x48)
[    0.768732] [<8011e7fc>] (warn_slowpath_fmt) from [<802768b0>]
(sysfs_warn_dup+0x64/0x74)
[    0.776950] [<802768b0>] (sysfs_warn_dup) from [<80276988>]
(sysfs_create_dir_ns+0x84/0x90)
[    0.785339] [<80276988>] (sysfs_create_dir_ns) from [<80836560>]
(kobject_add_internal+0xb4/0x30c)
[    0.794337] [<80836560>] (kobject_add_internal) from [<80836804>]
(kobject_add+0x4c/0x9c)
[    0.802554] [<80836804>] (kobject_add) from [<8050c030>]
(device_add+0xe0/0x594)
[    0.809991] [<8050c030>] (device_add) from [<805103d8>]
(platform_device_add+0x110/0x224)
[    0.818208] [<805103d8>] (platform_device_add) from [<8049bb94>]
(imx_gpc_probe+0x184/0x380)
[    0.826685] [<8049bb94>] (imx_gpc_probe) from [<805105c4>]
(platform_drv_probe+0x50/0xac)
[    0.834901] [<805105c4>] (platform_drv_probe) from [<8050ec0c>]
(driver_probe_device+0x1b4/0x3c8)
[    0.843810] [<8050ec0c>] (driver_probe_device) from [<8050eec4>]
(__driver_attach+0xa4/0xa8)
[    0.852284] [<8050eec4>] (__driver_attach) from [<8050cfe8>]
(bus_for_each_dev+0x4c/0x9c)
[    0.860495] [<8050cfe8>] (bus_for_each_dev) from [<8050e1a0>]
(bus_add_driver+0x188/0x20c)
[    0.868795] [<8050e1a0>] (bus_add_driver) from [<8050f7b4>]
(driver_register+0x78/0xf4)
[    0.876835] [<8050f7b4>] (driver_register) from [<80101b00>]
(do_one_initcall+0x44/0x168)
[    0.885049] [<80101b00>] (do_one_initcall) from [<80c00db8>]
(kernel_init_freeable+0x14c/0x1d8)
[    0.893789] [<80c00db8>] (kernel_init_freeable) from [<80844538>]
(kernel_init+0x8/0x10c)
[    0.902006] [<80844538>] (kernel_init) from [<80107a28>]
(ret_from_fork+0x14/0x2c)
[    0.909639] ---[ end trace ade27083f156a989 ]---
[    0.914277] ------------[ cut here ]------------
[    0.918934] WARNING: CPU: 0 PID: 1 at lib/kobject.c:240
kobject_add_internal+0x278/0x30c
[    0.927073] kobject_add_internal failed for imx-pgc-power-domain.0
with -EEXIST, don't try to register things with the same name in the
same directory.
[    0.940690] Modules linked in:
[    0.943763] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W       
4.15.0-rc3-00061-g38f99f72e8f3-dirty #246
[    0.953796] Hardware name: Freescale i.MX6 Quad/DualLite (Device
Tree)
[    0.960357] [<8010f61c>] (unwind_backtrace) from [<8010b984>]
(show_stack+0x10/0x14)
[    0.968136] [<8010b984>] (show_stack) from [<80832024>]
(dump_stack+0x88/0x9c)
[    0.975394] [<80832024>] (dump_stack) from [<8011e7ac>]
(__warn+0xdc/0xf4)
[    0.982298] [<8011e7ac>] (__warn) from [<8011e7fc>]
(warn_slowpath_fmt+0x38/0x48)
[    0.989815] [<8011e7fc>] (warn_slowpath_fmt) from [<80836724>]
(kobject_add_internal+0x278/0x30c)
[    0.998725] [<80836724>] (kobject_add_internal) from [<80836804>]
(kobject_add+0x4c/0x9c)
[    1.006938] [<80836804>] (kobject_add) from [<8050c030>]
(device_add+0xe0/0x594)
[    1.014370] [<8050c030>] (device_add) from [<805103d8>]
(platform_device_add+0x110/0x224)
[    1.022583] [<805103d8>] (platform_device_add) from [<8049bb94>]
(imx_gpc_probe+0x184/0x380)
[    1.031057] [<8049bb94>] (imx_gpc_probe) from [<805105c4>]
(platform_drv_probe+0x50/0xac)
[    1.039269] [<805105c4>] (platform_drv_probe) from [<8050ec0c>]
(driver_probe_device+0x1b4/0x3c8)
[    1.048176] [<8050ec0c>] (driver_probe_device) from [<8050eec4>]
(__driver_attach+0xa4/0xa8)
[    1.056647] [<8050eec4>] (__driver_attach) from [<8050cfe8>]
(bus_for_each_dev+0x4c/0x9c)
[    1.064857] [<8050cfe8>] (bus_for_each_dev) from [<8050e1a0>]
(bus_add_driver+0x188/0x20c)
[    1.073153] [<8050e1a0>] (bus_add_driver) from [<8050f7b4>]
(driver_register+0x78/0xf4)
[    1.081191] [<8050f7b4>] (driver_register) from [<80101b00>]
(do_one_initcall+0x44/0x168)
[    1.089404] [<80101b00>] (do_one_initcall) from [<80c00db8>]
(kernel_init_freeable+0x14c/0x1d8)
[    1.098141] [<80c00db8>] (kernel_init_freeable) from [<80844538>]
(kernel_init+0x8/0x10c)
[    1.106355] [<80844538>] (kernel_init) from [<80107a28>]
(ret_from_fork+0x14/0x2c)
[    1.113979] ---[ end trace ade27083f156a98a ]---
[    1.118637] ------------[ cut here ]------------
[    1.123275] Kernel BUG at c46b9f56 [verbose debug info unavailable]
[    1.129564] Internal error: Oops - BUG: 0 [#1] SMP ARM
[    1.134719] Modules linked in:
[    1.137791] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W       
4.15.0-rc3-00061-g38f99f72e8f3-dirty #246
[    1.147823] Hardware name: Freescale i.MX6 Quad/DualLite (Device
Tree)
[    1.154375] PC is at kfree+0xfc/0x140
[    1.158054] LR is at platform_device_release+0x10/0x34
[    1.163209] pc : [<801fd744>]    lr : [<805101b0>]    psr: 40000053
[    1.169495] sp : 84057de8  ip : 00000000  fp : 842e9e00
[    1.174736] r10: 842e9800  r9 : 80d23968  r8 : 80aa8a40
[    1.179980] r7 : 80952bac  r6 : 00000000  r5 : 842e9810  r4 :
842e9810
[    1.186529] r3 : 87dd2474  r2 : 87dd2460  r1 : a0000053  r0 :
80d23a20
[    1.193080] Flags: nZcv  IRQs on  FIQs off  Mode SVC_32  ISA ARM 
Segment none
[    1.200328] Control: 10c5387d  Table: 10004059  DAC: 00000051
[    1.206093] Process swapper/0 (pid: 1, stack limit = 0x86b82e40)
[    1.212122] Stack: (0x84057de8 to 0x84058000)
[    1.216499] 7de0:                   842e9810 842e9818 842e9810
842e9810 842e9810 00000000
[    1.224710] 7e00: 80952bac 805101b0 842e9818 80509eac 842e9818
80d2a784 84251480 80835f38
[    1.232919] 7e20: 87da98f4 80952bac 87da9810 ffffffef 87da98f4
8049bd2c 00000000 00000000
[    1.241128] 7e40: 84155a10 00000042 842500c0 00000000 80b410d8
84155a10 fffffffe 80d239d8
[    1.249337] 7e60: fffffdfb 80d239d8 80daefe8 00000000 00000000
805105c4 84155a10 00000000
[    1.257548] 7e80: 80daefe4 00000000 80d239d8 8050ec0c 87da95f0
00000000 000000d7 84155a10
[    1.265756] 7ea0: 80d239d8 84155a44 00000000 000000d7 80c5b83c
80c6c57c 00000000 8050eec4
[    1.273965] 7ec0: 00000000 80d239d8 8050ee20 8050cfe8 84050358
8414f6b4 80d239d8 842b4f00
[    1.282174] 7ee0: 80d2ab60 8050e1a0 80afef10 80c35700 80d239d8
80d239d8 00000000 80c35ecc
[    1.290384] 7f00: 80d58680 8050f7b4 ffffe000 00000000 80c35ecc
80101b00 80b94c44 000000d7
[    1.298595] 7f20: 00000000 8013a458 00000000 80b1dd5c 00000006
00000006 80aadf4c 00000000
[    1.306803] 7f40: 80ab7130 80aadfc0 87fffaee 87fffaf4 00000000
00000007 80d58680 80c5b830
[    1.315012] 7f60: 00000007 80d58680 80c5b834 80d58680 000000d7
80c00db8 00000006 00000006
[    1.323221] 7f80: 00000000 80c005b0 00000000 80844530 00000000
00000000 00000000 00000000
[    1.331430] 7fa0: 00000000 80844538 00000000 80107a28 00000000
00000000 00000000 00000000
[    1.339639] 7fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[    1.347848] 7fe0: 00000000 00000000 00000000 00000000 00000013
00000000 3fbfd7ff fed779df
[    1.356064] [<801fd744>] (kfree) from [<805101b0>]
(platform_device_release+0x10/0x34)
[    1.364019] [<805101b0>] (platform_device_release) from [<80509eac>]
(device_release+0x2c/0x90)
[    1.372759] [<80509eac>] (device_release) from [<80835f38>]
(kobject_put+0x94/0xe4)
[    1.380447] [<80835f38>] (kobject_put) from [<8049bd2c>]
(imx_gpc_probe+0x31c/0x380)
[    1.388224] [<8049bd2c>] (imx_gpc_probe) from [<805105c4>]
(platform_drv_probe+0x50/0xac)
[    1.396435] [<805105c4>] (platform_drv_probe) from [<8050ec0c>]
(driver_probe_device+0x1b4/0x3c8)
[    1.405345] [<8050ec0c>] (driver_probe_device) from [<8050eec4>]
(__driver_attach+0xa4/0xa8)
[    1.413815] [<8050eec4>] (__driver_attach) from [<8050cfe8>]
(bus_for_each_dev+0x4c/0x9c)
[    1.422024] [<8050cfe8>] (bus_for_each_dev) from [<8050e1a0>]
(bus_add_driver+0x188/0x20c)
[    1.430320] [<8050e1a0>] (bus_add_driver) from [<8050f7b4>]
(driver_register+0x78/0xf4)
[    1.438357] [<8050f7b4>] (driver_register) from [<80101b00>]
(do_one_initcall+0x44/0x168)
[    1.446568] [<80101b00>] (do_one_initcall) from [<80c00db8>]
(kernel_init_freeable+0x14c/0x1d8)
[    1.455305] [<80c00db8>] (kernel_init_freeable) from [<80844538>]
(kernel_init+0x8/0x10c)
[    1.463518] [<80844538>] (kernel_init) from [<80107a28>]
(ret_from_fork+0x14/0x2c)
[    1.471118] Code: 1a000003 e5923014 e3130001 1a000000 (e7f001f2)
[    1.477236] ---[ end trace ade27083f156a98b ]---
[    1.482097] Kernel panic - not syncing: Attempted to kill init!
exitcode=0x0000000b
[    1.482097]
[    1.491281] ---[ end Kernel panic - not syncing: Attempted to kill
init! exitcode=0x0000000b
[    1.491281]

--
Stefan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* soc: imx: gpcv2: removing and probing fails
  2018-01-07 10:48 soc: imx: gpcv2: removing and probing fails Stefan Agner
  2018-01-08  0:22 ` Andrey Smirnov
@ 2018-01-09 14:24 ` Lucas Stach
  2018-01-09 14:44   ` Stefan Agner
  1 sibling, 1 reply; 9+ messages in thread
From: Lucas Stach @ 2018-01-09 14:24 UTC (permalink / raw)
  To: linux-arm-kernel

Am Sonntag, den 07.01.2018, 11:48 +0100 schrieb Stefan Agner:
> Hi Andrew,
> 
> I noticed that the driver fails when removing and probing again. As far
> as I can see due to duplicate add of the platform devices.
> 
> As far as I can tell the driver should register the remove callback and
> do a platform_device_unregister on the newly created platform devices.
> However, as far as I can tell we don't hold on to a reference to them...
> I guess we could keep references in imx_gpcv2_probe, but maybe there is
> an easier way?

The GPC v1 driver adds the necessary device dependency between the
power domain devices and the GPC parent device. See the
device_link_add() in imx_pgc_power_domain_probe().

Probably something similar can be done to the GPC v2 driver.

Regards,
Lucas

^ permalink raw reply	[flat|nested] 9+ messages in thread

* soc: imx: gpcv2: removing and probing fails
  2018-01-09 14:24 ` Lucas Stach
@ 2018-01-09 14:44   ` Stefan Agner
  2018-01-09 14:55     ` Lucas Stach
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Agner @ 2018-01-09 14:44 UTC (permalink / raw)
  To: linux-arm-kernel

On 2018-01-09 15:24, Lucas Stach wrote:
> Am Sonntag, den 07.01.2018, 11:48 +0100 schrieb Stefan Agner:
>> Hi Andrew,
>>
>> I noticed that the driver fails when removing and probing again. As far
>> as I can see due to duplicate add of the platform devices.
>>
>> As far as I can tell the driver should register the remove callback and
>> do a platform_device_unregister on the newly created platform devices.
>> However, as far as I can tell we don't hold on to a reference to them...
>> I guess we could keep references in imx_gpcv2_probe, but maybe there is
>> an easier way?
> 
> The GPC v1 driver adds the necessary device dependency between the
> power domain devices and the GPC parent device. See the
> device_link_add() in imx_pgc_power_domain_probe().

Note that despite device_link_add, GPC v1 seems to cause issue with
CONFIG_DEBUG_TEST_DRIVER_REMOVE=y:
https://marc.info/?l=linux-arm-kernel&m=151544599904423&w=4

(sorry, I made it confusing, by adding a stack trace when using GPC v1
in the gpcv2 thread...)

--
Stefan


> 
> Probably something similar can be done to the GPC v2 driver.
> 
> Regards,
> Lucas

^ permalink raw reply	[flat|nested] 9+ messages in thread

* soc: imx: gpcv2: removing and probing fails
  2018-01-09 14:44   ` Stefan Agner
@ 2018-01-09 14:55     ` Lucas Stach
  2018-01-09 19:26       ` Stefan Agner
  2018-01-09 21:08       ` Stefan Agner
  0 siblings, 2 replies; 9+ messages in thread
From: Lucas Stach @ 2018-01-09 14:55 UTC (permalink / raw)
  To: linux-arm-kernel

Am Dienstag, den 09.01.2018, 15:44 +0100 schrieb Stefan Agner:
> On 2018-01-09 15:24, Lucas Stach wrote:
> > Am Sonntag, den 07.01.2018, 11:48 +0100 schrieb Stefan Agner:
> > > Hi Andrew,
> > > 
> > > I noticed that the driver fails when removing and probing again.
> > > As far
> > > as I can see due to duplicate add of the platform devices.
> > > 
> > > As far as I can tell the driver should register the remove
> > > callback and
> > > do a platform_device_unregister on the newly created platform
> > > devices.
> > > However, as far as I can tell we don't hold on to a reference to
> > > them...
> > > I guess we could keep references in imx_gpcv2_probe, but maybe
> > > there is
> > > an easier way?
> > 
> > The GPC v1 driver adds the necessary device dependency between the
> > power domain devices and the GPC parent device. See the
> > device_link_add() in imx_pgc_power_domain_probe().
> 
> Note that despite device_link_add, GPC v1 seems to cause issue with
> CONFIG_DEBUG_TEST_DRIVER_REMOVE=y:
> https://marc.info/?l=linux-arm-kernel&m=151544599904423&w=4
> 
> (sorry, I made it confusing, by adding a stack trace when using GPC
> v1
> in the gpcv2 thread...)

IMHO this is an issue with the?CONFIG_DEBUG_TEST_DRIVER_REMOVE option,
as it just blindly calls the remove callback instead of doing a proper
__device_release_driver(). All the regular driver/device unbind paths
will properly unbind the consumer devices before removing the driver.

I think this should be fixed in the device driver core instead of
individual drivers.

Regards,
Lucas

^ permalink raw reply	[flat|nested] 9+ messages in thread

* soc: imx: gpcv2: removing and probing fails
  2018-01-09 14:55     ` Lucas Stach
@ 2018-01-09 19:26       ` Stefan Agner
  2018-01-09 21:08       ` Stefan Agner
  1 sibling, 0 replies; 9+ messages in thread
From: Stefan Agner @ 2018-01-09 19:26 UTC (permalink / raw)
  To: linux-arm-kernel

On 2018-01-09 15:55, Lucas Stach wrote:
> Am Dienstag, den 09.01.2018, 15:44 +0100 schrieb Stefan Agner:
>> On 2018-01-09 15:24, Lucas Stach wrote:
>> > Am Sonntag, den 07.01.2018, 11:48 +0100 schrieb Stefan Agner:
>> > > Hi Andrew,
>> > >
>> > > I noticed that the driver fails when removing and probing again.
>> > > As far
>> > > as I can see due to duplicate add of the platform devices.
>> > >
>> > > As far as I can tell the driver should register the remove
>> > > callback and
>> > > do a platform_device_unregister on the newly created platform
>> > > devices.
>> > > However, as far as I can tell we don't hold on to a reference to
>> > > them...
>> > > I guess we could keep references in imx_gpcv2_probe, but maybe
>> > > there is
>> > > an easier way?
>> >
>> > The GPC v1 driver adds the necessary device dependency between the
>> > power domain devices and the GPC parent device. See the
>> > device_link_add() in imx_pgc_power_domain_probe().
>>
>> Note that despite device_link_add, GPC v1 seems to cause issue with
>> CONFIG_DEBUG_TEST_DRIVER_REMOVE=y:
>> https://marc.info/?l=linux-arm-kernel&m=151544599904423&w=4
>>
>> (sorry, I made it confusing, by adding a stack trace when using GPC
>> v1
>> in the gpcv2 thread...)
> 
> IMHO this is an issue with the?CONFIG_DEBUG_TEST_DRIVER_REMOVE option,
> as it just blindly calls the remove callback instead of doing a proper
> __device_release_driver(). All the regular driver/device unbind paths
> will properly unbind the consumer devices before removing the driver.
> 

There still seem to be an isse (gpc v1). I even removed PD consumers in
the device tree....

root at colibri-imx6:~# echo 20dc000.gpc >
/sys/bus/platform/drivers/imx-gpc/unbind
[   18.821940] imx-pgc-pd imx-pgc-power-domain.0: Dropping the link to
20dc000.gpc
[   18.833249] imx-pgc-pd imx-pgc-power-domain.1: Dropping the link to
20dc000.gpc
root at colibri-imx6:~# echo 20dc000.gpc >
/sys/bus/platform/drivers/imx-gpc/bind
[   23.031945] ------------[ cut here ]------------
[   23.036600] WARNING: CPU: 0 PID: 432 at fs/sysfs/dir.c:31
sysfs_warn_dup+0x64/0x74
[   23.044251] sysfs: cannot create duplicate filename
'/devices/soc0/soc/2000000.aips-bus/20dc000.gpc/imx-pgc-power-domain.0'
[   23.055434] Modules linked in:
[   23.058505] CPU: 0 PID: 432 Comm: sh Not tainted
4.15.0-rc3-00061-g38f99f72e8f3-dirty #247
[   23.066778] Hardware name: Freescale i.MX6 Quad/DualLite (Device
Tree)
[   23.073332] [<8010f61c>] (unwind_backtrace) from [<8010b984>]
(show_stack+0x10/0x14)
[   23.081094] [<8010b984>] (show_stack) from [<80831f84>]
(dump_stack+0x88/0x9c)
[   23.088336] [<80831f84>] (dump_stack) from [<8011e7ac>]
(__warn+0xdc/0xf4)
[   23.095226] [<8011e7ac>] (__warn) from [<8011e7fc>]
(warn_slowpath_fmt+0x38/0x48)
[   23.102726] [<8011e7fc>] (warn_slowpath_fmt) from [<802768b0>]
(sysfs_warn_dup+0x64/0x74)
[   23.110923] [<802768b0>] (sysfs_warn_dup) from [<80276988>]
(sysfs_create_dir_ns+0x84/0x90)
[   23.119291] [<80276988>] (sysfs_create_dir_ns) from [<808364c0>]
(kobject_add_internal+0xb4/0x30c)
[   23.128264] [<808364c0>] (kobject_add_internal) from [<80836764>]
(kobject_add+0x4c/0x9c)
[   23.136469] [<80836764>] (kobject_add) from [<8050c030>]
(device_add+0xe0/0x594)
[   23.143892] [<8050c030>] (device_add) from [<8051033c>]
(platform_device_add+0x110/0x224)
[   23.152096] [<8051033c>] (platform_device_add) from [<8049bb94>]
(imx_gpc_probe+0x184/0x380)
[   23.160556] [<8049bb94>] (imx_gpc_probe) from [<80510528>]
(platform_drv_probe+0x50/0xac)
[   23.168758] [<80510528>] (platform_drv_probe) from [<8050ecac>]
(driver_probe_device+0x254/0x32c)
[   23.177652] [<8050ecac>] (driver_probe_device) from [<8050d3f4>]
(bind_store+0xac/0x140)
[   23.185765] [<8050d3f4>] (bind_store) from [<802757a4>]
(kernfs_fop_write+0xec/0x1f0)
[   23.193623] [<802757a4>] (kernfs_fop_write) from [<80206064>]
(__vfs_write+0x1c/0x120)
[   23.201562] [<80206064>] (__vfs_write) from [<80206310>]
(vfs_write+0xa4/0x1b4)
[   23.208890] [<80206310>] (vfs_write) from [<80206520>]
(SyS_write+0x3c/0x90)
[   23.215963] [<80206520>] (SyS_write) from [<80107940>]
(ret_fast_syscall+0x0/0x54)
[   23.223619] ---[ end trace a5f9524661a4dba7 ]---
[   23.228254] ------------[ cut here ]------------
[   23.232920] WARNING: CPU: 0 PID: 432 at lib/kobject.c:240
kobject_add_internal+0x278/0x30c
[   23.241205] kobject_add_internal failed for imx-pgc-power-domain.0
with -EEXIST, don't try to register things with the same name in the
same directory.
[   23.254808] Modules linked in:
[   23.257880] CPU: 0 PID: 432 Comm: sh Tainted: G        W       
4.15.0-rc3-00061-g38f99f72e8f3-dirty #247
[   23.267459] Hardware name: Freescale i.MX6 Quad/DualLite (Device
Tree)
[   23.274009] [<8010f61c>] (unwind_backtrace) from [<8010b984>]
(show_stack+0x10/0x14)
[   23.281776] [<8010b984>] (show_stack) from [<80831f84>]
(dump_stack+0x88/0x9c)
[   23.289022] [<80831f84>] (dump_stack) from [<8011e7ac>]
(__warn+0xdc/0xf4)
[   23.295914] [<8011e7ac>] (__warn) from [<8011e7fc>]
(warn_slowpath_fmt+0x38/0x48)
[   23.303417] [<8011e7fc>] (warn_slowpath_fmt) from [<80836684>]
(kobject_add_internal+0x278/0x30c)
[   23.312308] [<80836684>] (kobject_add_internal) from [<80836764>]
(kobject_add+0x4c/0x9c)
[   23.320506] [<80836764>] (kobject_add) from [<8050c030>]
(device_add+0xe0/0x594)
[   23.327922] [<8050c030>] (device_add) from [<8051033c>]
(platform_device_add+0x110/0x224)
[   23.336125] [<8051033c>] (platform_device_add) from [<8049bb94>]
(imx_gpc_probe+0x184/0x380)
[   23.344586] [<8049bb94>] (imx_gpc_probe) from [<80510528>]
(platform_drv_probe+0x50/0xac)
[   23.352783] [<80510528>] (platform_drv_probe) from [<8050ecac>]
(driver_probe_device+0x254/0x32c)
[   23.361677] [<8050ecac>] (driver_probe_device) from [<8050d3f4>]
(bind_store+0xac/0x140)
[   23.369788] [<8050d3f4>] (bind_store) from [<802757a4>]
(kernfs_fop_write+0xec/0x1f0)
[   23.377644] [<802757a4>] (kernfs_fop_write) from [<80206064>]
(__vfs_write+0x1c/0x120)
[   23.385583] [<80206064>] (__vfs_write) from [<80206310>]
(vfs_write+0xa4/0x1b4)
[   23.392910] [<80206310>] (vfs_write) from [<80206520>]
(SyS_write+0x3c/0x90)
[   23.399979] [<80206520>] (SyS_write) from [<80107940>]
(ret_fast_syscall+0x0/0x54)
[   23.407603] ---[ end trace a5f9524661a4dba8 ]---
[   23.412263] ------------[ cut here ]------------
[   23.416893] Kernel BUG at 9e1dfcde [verbose debug info unavailable]
[   23.423173] Internal error: Oops - BUG: 0 [#1] SMP ARM
[   23.428322] Modules linked in:
[   23.431391] CPU: 0 PID: 432 Comm: sh Tainted: G        W       
4.15.0-rc3-00061-g38f99f72e8f3-dirty #247
[   23.440972] Hardware name: Freescale i.MX6 Quad/DualLite (Device
Tree)
[   23.447514] PC is at kfree+0xfc/0x140
[   23.451189] LR is at platform_device_release+0x10/0x34
[   23.456339] pc : [<801fd744>]    lr : [<80510114>]    psr: 40010013
[   23.462615] sp : 84cfdde8  ip : 00000000  fp : 84b34e00
[   23.467849] r10: 84b35a00  r9 : 80d23968  r8 : 80aa8a40
[   23.473084] r7 : 80952bac  r6 : 00000000  r5 : 84b35a10  r4 :
84b35a10
[   23.479622] r3 : 87dd2474  r2 : 87dd2460  r1 : a0010013  r0 :
80d23a20
[   23.486162] Flags: nZcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM 
Segment none
[   23.493308] Control: 10c5387d  Table: 14bd0059  DAC: 00000051
[   23.499065] Process sh (pid: 432, stack limit = 0xd85916d3)
[   23.504647] Stack: (0x84cfdde8 to 0x84cfe000)
[   23.509017] dde0:                   84b35a10 84b35a18 84b35a10
84b35a10 84b35a10 00000000
[   23.517212] de00: 80952bac 80510114 84b35a18 80509eac 84b35a18
80d2a784 84355780 80835e98
[   23.525408] de20: 87da9920 80952bac 87da983c ffffffef 87da9920
8049bd2c 00000000 00000000
[   23.533602] de40: 84151a10 00000042 84c28840 00000000 80b410d8
84151a10 fffffffe 80d239d8
[   23.541797] de60: fffffdfb 80d239d8 00000000 00000003 84c2b610
80510528 84151a10 80daefe4
[   23.549991] de80: 80daefe8 00000000 80d239d8 8050ecac 84151a10
80d2ab60 80d239d8 84151a44
[   23.558185] dea0: 0000000c 84cfdf80 00000000 8050d3f4 0000000c
84c2b600 00000000 00000000
[   23.566379] dec0: 84355a40 802757a4 00000000 00000000 84563910
802756b8 84efbcc0 01a661a8
[   23.574574] dee0: 84cfdf80 00000000 0000000c 00000000 000a5608
80206064 0000000a 00000100
[   23.582767] df00: 84dfd100 84e797c0 0000000a 8022401c 84cfb600
ffffe000 0000000a 00000001
[   23.590961] df20: 84cfb600 84cfc000 0000000a 80224800 00000000
80845644 00000000 8457bd80
[   23.599157] df40: 0000000c 84efbcc0 01a661a8 84cfdf80 00000000
80206310 84cfb600 00000002
[   23.607352] df60: 0000000a 84efbcc0 84efbcc0 00000000 00000000
01a661a8 0000000c 80206520
[   23.615548] df80: 00000000 00000000 00000000 0000000c 01a661a8
76f6ed60 00000004 80107b24
[   23.623743] dfa0: 84cfc000 80107940 0000000c 01a661a8 00000001
01a661a8 0000000c 00000000
[   23.631938] dfc0: 0000000c 01a661a8 76f6ed60 00000004 0000000c
0000000c 00086920 000a5608
[   23.640133] dfe0: 00000000 7ed2395c 76e9cffc 76ef57e0 60010010
00000001 00000000 00000000
[   23.648334] [<801fd744>] (kfree) from [<80510114>]
(platform_device_release+0x10/0x34)
[   23.656276] [<80510114>] (platform_device_release) from [<80509eac>]
(device_release+0x2c/0x90)
[   23.664999] [<80509eac>] (device_release) from [<80835e98>]
(kobject_put+0x94/0xe4)
[   23.672679] [<80835e98>] (kobject_put) from [<8049bd2c>]
(imx_gpc_probe+0x31c/0x380)
[   23.680443] [<8049bd2c>] (imx_gpc_probe) from [<80510528>]
(platform_drv_probe+0x50/0xac)
[   23.688640] [<80510528>] (platform_drv_probe) from [<8050ecac>]
(driver_probe_device+0x254/0x32c)
[   23.697531] [<8050ecac>] (driver_probe_device) from [<8050d3f4>]
(bind_store+0xac/0x140)
[   23.705644] [<8050d3f4>] (bind_store) from [<802757a4>]
(kernfs_fop_write+0xec/0x1f0)
[   23.713499] [<802757a4>] (kernfs_fop_write) from [<80206064>]
(__vfs_write+0x1c/0x120)
[   23.721436] [<80206064>] (__vfs_write) from [<80206310>]
(vfs_write+0xa4/0x1b4)
[   23.728764] [<80206310>] (vfs_write) from [<80206520>]
(SyS_write+0x3c/0x90)
[   23.735832] [<80206520>] (SyS_write) from [<80107940>]
(ret_fast_syscall+0x0/0x54)
[   23.743420] Code: 1a000003 e5923014 e3130001 1a000000 (e7f001f2)
[   23.749526] ---[ end trace a5f9524661a4dba9 ]---

--
Stefan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* soc: imx: gpcv2: removing and probing fails
  2018-01-09 14:55     ` Lucas Stach
  2018-01-09 19:26       ` Stefan Agner
@ 2018-01-09 21:08       ` Stefan Agner
  1 sibling, 0 replies; 9+ messages in thread
From: Stefan Agner @ 2018-01-09 21:08 UTC (permalink / raw)
  To: linux-arm-kernel

On 2018-01-09 15:55, Lucas Stach wrote:
> Am Dienstag, den 09.01.2018, 15:44 +0100 schrieb Stefan Agner:
>> On 2018-01-09 15:24, Lucas Stach wrote:
>> > Am Sonntag, den 07.01.2018, 11:48 +0100 schrieb Stefan Agner:
>> > > Hi Andrew,
>> > >
>> > > I noticed that the driver fails when removing and probing again.
>> > > As far
>> > > as I can see due to duplicate add of the platform devices.
>> > >
>> > > As far as I can tell the driver should register the remove
>> > > callback and
>> > > do a platform_device_unregister on the newly created platform
>> > > devices.
>> > > However, as far as I can tell we don't hold on to a reference to
>> > > them...
>> > > I guess we could keep references in imx_gpcv2_probe, but maybe
>> > > there is
>> > > an easier way?
>> >
>> > The GPC v1 driver adds the necessary device dependency between the
>> > power domain devices and the GPC parent device. See the
>> > device_link_add() in imx_pgc_power_domain_probe().
>>
>> Note that despite device_link_add, GPC v1 seems to cause issue with
>> CONFIG_DEBUG_TEST_DRIVER_REMOVE=y:
>> https://marc.info/?l=linux-arm-kernel&m=151544599904423&w=4
>>
>> (sorry, I made it confusing, by adding a stack trace when using GPC
>> v1
>> in the gpcv2 thread...)
> 
> IMHO this is an issue with the?CONFIG_DEBUG_TEST_DRIVER_REMOVE option,
> as it just blindly calls the remove callback instead of doing a proper
> __device_release_driver(). All the regular driver/device unbind paths
> will properly unbind the consumer devices before removing the driver.

They do unbind the consumer device, but do not unregister the platform
device.

I tried to fix it by calling platform_device_unregister, e.g. with this
changes unbind seems to work:

--- a/drivers/soc/imx/gpc.c
+++ b/drivers/soc/imx/gpc.c
@@ -40,6 +40,7 @@
 
 struct imx_pm_domain {
        struct generic_pm_domain base;
+       struct platform_device *pdev;
        struct regmap *regmap;
        struct regulator *supply;
        struct clk *clk[GPC_CLK_MAX];
@@ -462,6 +465,8 @@ static int imx_gpc_probe(struct platform_device
*pdev)
                                of_node_put(np);
                                return ret;
                        }
+
+                       domain->pdev = pd_pdev;
                }
        }
 
@@ -470,7 +475,7 @@ static int imx_gpc_probe(struct platform_device
*pdev)
 
 static int imx_gpc_remove(struct platform_device *pdev)
 {
-       int ret;
+       int i, ret;
 
        /*
         * If the old DT binding is used the toplevel driver needs to
@@ -489,6 +494,21 @@ static int imx_gpc_remove(struct platform_device
*pdev)
                        return ret;
        }
 
+
+       for (i = 0; i < ARRAY_SIZE(imx_gpc_domains); i++) {
+               struct imx_pm_domain *domain = &imx_gpc_domains[i];
+
+               if (domain->pdev) {
+                       /*
+                        * Unlink platform_data to prevent
+                        * platform_device_unregister to kfree it.
+                        */
+                       domain->pdev->dev.platform_data = NULL;
+                       platform_device_unregister(domain->pdev);
+                       domain->pdev = NULL;
+               }
+       }
+
        return 0;
 }


However, it still leads to a stacktrace on next bind:

root at colibri-imx6:~# echo 20dc000.gpc >
/sys/bus/platform/drivers/imx-gpc/bind
[   24.741624] imx-pgc-pd imx-pgc-power-domain.0: Linked as a consumer
to 20dc000.gpc
[   24.749219] ------------[ cut here ]------------
[   24.753934] WARNING: CPU: 0 PID: 430 at drivers/base/core.c:417
device_links_driver_bound+0xd8/0xe0
...

--
Stefan

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-01-09 21:08 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-07 10:48 soc: imx: gpcv2: removing and probing fails Stefan Agner
2018-01-08  0:22 ` Andrey Smirnov
2018-01-08  6:24   ` Andrey Smirnov
2018-01-08 21:12     ` Stefan Agner
2018-01-09 14:24 ` Lucas Stach
2018-01-09 14:44   ` Stefan Agner
2018-01-09 14:55     ` Lucas Stach
2018-01-09 19:26       ` Stefan Agner
2018-01-09 21:08       ` Stefan Agner

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).