* [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD @ 2019-01-18 4:48 Rajendra Nayak 2019-01-18 17:39 ` Stephen Boyd 0 siblings, 1 reply; 12+ messages in thread From: Rajendra Nayak @ 2019-01-18 4:48 UTC (permalink / raw) To: andy.gross, david.brown Cc: linux-arm-msm, linux-kernel, linux, Rajendra Nayak Since QCOM_RPMPD is bool and it depends on QCOM_SMD_RPM which is tristate, configurations such as arm64:allmodconfig result in CONFIG_QCOM_RPMPD=y CONFIG_QCOM_SMD_RPM=m This in turn results in drivers/soc/qcom/rpmpd.o: In function `rpmpd_send_corner': rpmpd.c:(.text+0x10c): undefined reference to `qcom_rpm_smd_write' drivers/soc/qcom/rpmpd.o: In function `rpmpd_power_on': rpmpd.c:(.text+0x3b4): undefined reference to `qcom_rpm_smd_write' drivers/soc/qcom/rpmpd.o: In function `rpmpd_power_off': rpmpd.c:(.text+0x520): undefined reference to `qcom_rpm_smd_write' make: *** [vmlinux] Error 1 Fix it by making QCOM_RPMPD depend on QCOM_SMD_RPM=y Reported-by: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> --- Andy, this one should be applied on top of my v11 to add rpmpd/rpmhpd drivers [1] and the fix from Bjorn to drop A family RPM dependency [2] [1] https://lkml.org/lkml/2019/1/9/1257 [2] https://lkml.org/lkml/2019/1/17/5 drivers/soc/qcom/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index a5d5167c3f16..1ee298f6bf17 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -109,7 +109,7 @@ config QCOM_RPMHPD config QCOM_RPMPD bool "Qualcomm RPM Power domain driver" - depends on QCOM_SMD_RPM + depends on QCOM_SMD_RPM=y help QCOM RPM Power domain driver to support power-domains with performance states. The driver communicates a performance state -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD 2019-01-18 4:48 [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD Rajendra Nayak @ 2019-01-18 17:39 ` Stephen Boyd 0 siblings, 0 replies; 12+ messages in thread From: Stephen Boyd @ 2019-01-18 17:39 UTC (permalink / raw) To: andy.gross, david.brown Cc: linux-arm-msm, linux-kernel, linux, Rajendra Nayak Quoting Rajendra Nayak (2019-01-17 20:48:01) > drivers/soc/qcom/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig > index a5d5167c3f16..1ee298f6bf17 100644 > --- a/drivers/soc/qcom/Kconfig > +++ b/drivers/soc/qcom/Kconfig > @@ -109,7 +109,7 @@ config QCOM_RPMHPD > > config QCOM_RPMPD > bool "Qualcomm RPM Power domain driver" Just curious, does it need to be bool for some reason? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD @ 2019-01-18 17:39 ` Stephen Boyd 0 siblings, 0 replies; 12+ messages in thread From: Stephen Boyd @ 2019-01-18 17:39 UTC (permalink / raw) To: Rajendra Nayak, andy.gross, david.brown Cc: linux-arm-msm, linux-kernel, linux, Rajendra Nayak Quoting Rajendra Nayak (2019-01-17 20:48:01) > drivers/soc/qcom/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig > index a5d5167c3f16..1ee298f6bf17 100644 > --- a/drivers/soc/qcom/Kconfig > +++ b/drivers/soc/qcom/Kconfig > @@ -109,7 +109,7 @@ config QCOM_RPMHPD > > config QCOM_RPMPD > bool "Qualcomm RPM Power domain driver" Just curious, does it need to be bool for some reason? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD 2019-01-18 17:39 ` Stephen Boyd (?) @ 2019-01-22 2:30 ` Rajendra Nayak 2019-01-22 5:08 ` Guenter Roeck -1 siblings, 1 reply; 12+ messages in thread From: Rajendra Nayak @ 2019-01-22 2:30 UTC (permalink / raw) To: Stephen Boyd, andy.gross, david.brown; +Cc: linux-arm-msm, linux-kernel, linux On 1/18/2019 11:09 PM, Stephen Boyd wrote: > Quoting Rajendra Nayak (2019-01-17 20:48:01) >> drivers/soc/qcom/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig >> index a5d5167c3f16..1ee298f6bf17 100644 >> --- a/drivers/soc/qcom/Kconfig >> +++ b/drivers/soc/qcom/Kconfig >> @@ -109,7 +109,7 @@ config QCOM_RPMHPD >> >> config QCOM_RPMPD >> bool "Qualcomm RPM Power domain driver" > > Just curious, does it need to be bool for some reason? Here's the link to the discussion around it on the v3 patchset of this series https://lkml.org/lkml/2018/6/12/111 -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD 2019-01-22 2:30 ` Rajendra Nayak @ 2019-01-22 5:08 ` Guenter Roeck 2019-01-22 9:54 ` Rajendra Nayak 0 siblings, 1 reply; 12+ messages in thread From: Guenter Roeck @ 2019-01-22 5:08 UTC (permalink / raw) To: Rajendra Nayak, Stephen Boyd, andy.gross, david.brown Cc: linux-arm-msm, linux-kernel On 1/21/19 6:30 PM, Rajendra Nayak wrote: > > > On 1/18/2019 11:09 PM, Stephen Boyd wrote: >> Quoting Rajendra Nayak (2019-01-17 20:48:01) >>> drivers/soc/qcom/Kconfig | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig >>> index a5d5167c3f16..1ee298f6bf17 100644 >>> --- a/drivers/soc/qcom/Kconfig >>> +++ b/drivers/soc/qcom/Kconfig >>> @@ -109,7 +109,7 @@ config QCOM_RPMHPD >>> config QCOM_RPMPD >>> bool "Qualcomm RPM Power domain driver" >> >> Just curious, does it need to be bool for some reason? > > Here's the link to the discussion around it on the v3 patchset of this series > https://lkml.org/lkml/2018/6/12/111 > I think you were missing the implication of "if possible", the implication being that the driver can now not be used in a multi-platform image, and that having a bool driver depend on tristate drivers doesn't make much if any sense. The argument in the exchange is odd - one can not remove any driver as long there are client drivers / devices attached to it. This is true for all drivers, not just for this one, and handled quite nicely by the driver core. Just run "lsmod" and try to remove a driver with any attached users. Guenter ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD 2019-01-22 5:08 ` Guenter Roeck @ 2019-01-22 9:54 ` Rajendra Nayak 2019-01-22 10:08 ` Viresh Kumar 0 siblings, 1 reply; 12+ messages in thread From: Rajendra Nayak @ 2019-01-22 9:54 UTC (permalink / raw) To: Guenter Roeck, Stephen Boyd, andy.gross, david.brown Cc: linux-arm-msm, linux-kernel, Viresh Kumar, Ulf Hansson Adding Viresh and Ulf to this thread, On 1/22/2019 10:38 AM, Guenter Roeck wrote: > On 1/21/19 6:30 PM, Rajendra Nayak wrote: >> >> >> On 1/18/2019 11:09 PM, Stephen Boyd wrote: >>> Quoting Rajendra Nayak (2019-01-17 20:48:01) >>>> drivers/soc/qcom/Kconfig | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig >>>> index a5d5167c3f16..1ee298f6bf17 100644 >>>> --- a/drivers/soc/qcom/Kconfig >>>> +++ b/drivers/soc/qcom/Kconfig >>>> @@ -109,7 +109,7 @@ config QCOM_RPMHPD >>>> config QCOM_RPMPD >>>> bool "Qualcomm RPM Power domain driver" >>> >>> Just curious, does it need to be bool for some reason? >> >> Here's the link to the discussion around it on the v3 patchset of this series >> https://lkml.org/lkml/2018/6/12/111 >> > > I think you were missing the implication of "if possible", the implication > being that the driver can now not be used in a multi-platform image, and that > having a bool driver depend on tristate drivers doesn't make much if any sense. Fair enough, I hadn't thought about the implications with multi platform builds, I guess it makes sense to have these drivers as tristate then. I was doing some quick testing by adding the calls to of_genpd_remove_last() as suggested by Ulf for cleaning up the genpd registrations, and I run into this backtrace when the driver re-probes followed by a remove [ 59.204525] kobject ((____ptrval____)): tried to init an initialized object, something is seriously wr. [ 59.214262] CPU: 3 PID: 1600 Comm: sh Not tainted 5.0.0-rc1-00012-g0513915837c5-dirty #32 [ 59.222500] Hardware name: Qualcomm Technologies, Inc. SDM845 MTP (DT) [ 59.229081] Call trace: [ 59.231574] dump_backtrace+0x0/0x148 [ 59.235276] show_stack+0x14/0x20 [ 59.238631] dump_stack+0x8c/0xac [ 59.241980] kobject_init+0x8c/0xa0 [ 59.245506] device_initialize+0x34/0xc8 [ 59.249474] pm_genpd_init+0x170/0x260 [ 59.253261] rpmhpd_probe+0x194/0x2b0 [ 59.256966] platform_drv_probe+0x4c/0xa8 [ 59.261011] really_probe+0x1e4/0x2c8 [ 59.264711] driver_probe_device+0x58/0x10 [ 59.268927] bind_store+0xdc/0x178 [ 59.272356] drv_attr_store+0x20/0x30 [ 59.276061] sysfs_kf_write+0x48/0x58 [ 59.279760] kernfs_fop_write+0xcc/0x1c8 [ 59.283728] __vfs_write+0x34/0x170 [ 59.287245] vfs_write+0xa8/0x1b8 [ 59.290592] ksys_write+0x5c/0xc8 [ 59.293941] __arm64_sys_write+0x14/0x20 [ 59.297897] el0_svc_common+0xb4/0x118 [ 59.301676] el0_svc_handler+0x2c/0x80 [ 59.305455] el0_svc+0x8/0xc Viresh/Ulf, looks like we need some cleanup of whats done by device_initialize() in pm_genpd_init() to happen as part of the pm_genpd_remove()? > > The argument in the exchange is odd - one can not remove any driver as long > there are client drivers / devices attached to it. This is true for all > drivers, not just for this one, and handled quite nicely by the driver core. > Just run "lsmod" and try to remove a driver with any attached users. > > Guenter -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD 2019-01-22 9:54 ` Rajendra Nayak @ 2019-01-22 10:08 ` Viresh Kumar 2019-01-22 10:35 ` Rajendra Nayak 0 siblings, 1 reply; 12+ messages in thread From: Viresh Kumar @ 2019-01-22 10:08 UTC (permalink / raw) To: Rajendra Nayak Cc: Guenter Roeck, Stephen Boyd, andy.gross, david.brown, linux-arm-msm, linux-kernel, Ulf Hansson On 22-01-19, 15:24, Rajendra Nayak wrote: > I was doing some quick testing by adding the calls to of_genpd_remove_last() as > suggested by Ulf for cleaning up the genpd registrations, and I run into this > backtrace when the driver re-probes followed by a remove > > [ 59.204525] kobject ((____ptrval____)): tried to init an initialized object, something is seriously wr. > [ 59.214262] CPU: 3 PID: 1600 Comm: sh Not tainted 5.0.0-rc1-00012-g0513915837c5-dirty #32 > [ 59.222500] Hardware name: Qualcomm Technologies, Inc. SDM845 MTP (DT) > [ 59.229081] Call trace: > [ 59.231574] dump_backtrace+0x0/0x148 > [ 59.235276] show_stack+0x14/0x20 > [ 59.238631] dump_stack+0x8c/0xac > [ 59.241980] kobject_init+0x8c/0xa0 > [ 59.245506] device_initialize+0x34/0xc8 > [ 59.249474] pm_genpd_init+0x170/0x260 > [ 59.253261] rpmhpd_probe+0x194/0x2b0 > [ 59.256966] platform_drv_probe+0x4c/0xa8 > [ 59.261011] really_probe+0x1e4/0x2c8 > [ 59.264711] driver_probe_device+0x58/0x10 > [ 59.268927] bind_store+0xdc/0x178 > [ 59.272356] drv_attr_store+0x20/0x30 > [ 59.276061] sysfs_kf_write+0x48/0x58 > [ 59.279760] kernfs_fop_write+0xcc/0x1c8 > [ 59.283728] __vfs_write+0x34/0x170 > [ 59.287245] vfs_write+0xa8/0x1b8 > [ 59.290592] ksys_write+0x5c/0xc8 > [ 59.293941] __arm64_sys_write+0x14/0x20 > [ 59.297897] el0_svc_common+0xb4/0x118 > [ 59.301676] el0_svc_handler+0x2c/0x80 > [ 59.305455] el0_svc+0x8/0xc > > Viresh/Ulf, looks like we need some cleanup of whats done by device_initialize() > in pm_genpd_init() to happen as part of the pm_genpd_remove()? Yeah, we normally need to call put_device() for that. Maybe something like this: diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 500de1dee967..d5b984f042ec 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1812,6 +1812,7 @@ static int genpd_remove(struct generic_pm_domain *genpd) genpd_unlock(genpd); cancel_work_sync(&genpd->power_off_work); kfree(genpd->free); + put_device(&genpd->dev); pr_debug("%s: removed %s\n", __func__, genpd->name); return 0; -- viresh ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD 2019-01-22 10:08 ` Viresh Kumar @ 2019-01-22 10:35 ` Rajendra Nayak 2019-01-23 11:16 ` Viresh Kumar 0 siblings, 1 reply; 12+ messages in thread From: Rajendra Nayak @ 2019-01-22 10:35 UTC (permalink / raw) To: Viresh Kumar Cc: Guenter Roeck, Stephen Boyd, andy.gross, david.brown, linux-arm-msm, linux-kernel, Ulf Hansson On 1/22/2019 3:38 PM, Viresh Kumar wrote: > On 22-01-19, 15:24, Rajendra Nayak wrote: >> I was doing some quick testing by adding the calls to of_genpd_remove_last() as >> suggested by Ulf for cleaning up the genpd registrations, and I run into this >> backtrace when the driver re-probes followed by a remove >> >> [ 59.204525] kobject ((____ptrval____)): tried to init an initialized object, something is seriously wr. >> [ 59.214262] CPU: 3 PID: 1600 Comm: sh Not tainted 5.0.0-rc1-00012-g0513915837c5-dirty #32 >> [ 59.222500] Hardware name: Qualcomm Technologies, Inc. SDM845 MTP (DT) >> [ 59.229081] Call trace: >> [ 59.231574] dump_backtrace+0x0/0x148 >> [ 59.235276] show_stack+0x14/0x20 >> [ 59.238631] dump_stack+0x8c/0xac >> [ 59.241980] kobject_init+0x8c/0xa0 >> [ 59.245506] device_initialize+0x34/0xc8 >> [ 59.249474] pm_genpd_init+0x170/0x260 >> [ 59.253261] rpmhpd_probe+0x194/0x2b0 >> [ 59.256966] platform_drv_probe+0x4c/0xa8 >> [ 59.261011] really_probe+0x1e4/0x2c8 >> [ 59.264711] driver_probe_device+0x58/0x10 >> [ 59.268927] bind_store+0xdc/0x178 >> [ 59.272356] drv_attr_store+0x20/0x30 >> [ 59.276061] sysfs_kf_write+0x48/0x58 >> [ 59.279760] kernfs_fop_write+0xcc/0x1c8 >> [ 59.283728] __vfs_write+0x34/0x170 >> [ 59.287245] vfs_write+0xa8/0x1b8 >> [ 59.290592] ksys_write+0x5c/0xc8 >> [ 59.293941] __arm64_sys_write+0x14/0x20 >> [ 59.297897] el0_svc_common+0xb4/0x118 >> [ 59.301676] el0_svc_handler+0x2c/0x80 >> [ 59.305455] el0_svc+0x8/0xc >> >> Viresh/Ulf, looks like we need some cleanup of whats done by device_initialize() >> in pm_genpd_init() to happen as part of the pm_genpd_remove()? > > Yeah, we normally need to call put_device() for that. Maybe something like this: > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 500de1dee967..d5b984f042ec 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -1812,6 +1812,7 @@ static int genpd_remove(struct generic_pm_domain *genpd) > genpd_unlock(genpd); > cancel_work_sync(&genpd->power_off_work); > kfree(genpd->free); > + put_device(&genpd->dev); > pr_debug("%s: removed %s\n", __func__, genpd->name); > > return 0; Seeing this now during remove [ 34.008216] ------------[ cut here ]------------ [ 34.012894] Device 'mss' does not have a release() function, it is broken and must be fixed. See Docum. [ 34.024143] WARNING: CPU: 6 PID: 1600 at drivers/base/core.c:922 device_release+0x84/0x98 [ 34.032379] Modules linked in: [ 34.035477] CPU: 6 PID: 1600 Comm: sh Not tainted 5.0.0-rc1-00012-g0513915837c5-dirty #34 [ 34.043724] Hardware name: Qualcomm Technologies, Inc. SDM845 MTP (DT) [ 34.050305] pstate: 60400005 (nZCv daif +PAN -UAO) [ 34.055141] pc : device_release+0x84/0x98 [ 34.059186] lr : device_release+0x84/0x98 [ 34.063229] sp : ffff0000183c3b40 [ 34.066574] x29: ffff0000183c3b40 x28: ffff800174b55100 [ 34.071930] x27: 0000000000000000 x26: 0000000000000000 [ 34.077285] x25: 0000000056000000 x24: 0000000000000015 [ 34.082640] x23: dead000000000100 x22: dead000000000200 [ 34.087997] x21: 0000000000000000 x20: ffff0000112f9780 [ 34.093352] x19: ffff0000112f9770 x18: ffff0000111dd6c8 [ 34.098719] x17: 0000000000000000 x16: 0000000000000000 [ 34.104072] x15: ffff0000983c3867 x14: 6966206562207473 [ 34.109438] x13: 756d20646e61206e x12: 656b6f7262207369 [ 34.114791] x11: 207469202c6e6f69 x10: 74636e7566202928 [ 34.120147] x9 : 657361656c657220 x8 : 742e7463656a626f [ 34.125514] x7 : 6b2f6e6f69746174 x6 : 0000000000000149 [ 34.130867] x5 : 0000000000000020 x4 : 000000000001c200 [ 34.136223] x3 : 00000000ffffffff x2 : ffff0000111f5828 [ 34.141579] x1 : bc1d8ed5415ab200 x0 : 0000000000000000 [ 34.146936] Call trace: [ 34.149413] device_release+0x84/0x98 [ 34.153116] kobject_put+0x98/0xe8 [ 34.156551] put_device+0x14/0x28 [ 34.159899] genpd_remove+0x108/0x190 [ 34.163598] of_genpd_remove_last+0xbc/0xf0 [ 34.167828] rpmhpd_remove+0x2c/0x48 [ 34.171436] platform_drv_remove+0x24/0x68 [ 34.175574] device_release_driver_internal+0x184/0x218 [ 34.180845] device_release_driver+0x14/0x20 [ 34.185152] unbind_store+0xec/0x138 [ 34.188760] drv_attr_store+0x20/0x30 [ 34.192465] sysfs_kf_write+0x48/0x58 [ 34.196163] kernfs_fop_write+0xcc/0x1c8 [ 34.200129] __vfs_write+0x34/0x170 [ 34.203654] vfs_write+0xa8/0x1b8 [ 34.206998] ksys_write+0x5c/0xc8 [ 34.210345] __arm64_sys_write+0x14/0x20 [ 34.214318] el0_svc_common+0xb4/0x118 [ 34.218107] el0_svc_handler+0x2c/0x80 [ 34.221899] el0_svc+0x8/0xc [ 34.224804] ---[ end trace 9cee1458d0b6c175 ]--- [ 34.229490] ------------[ cut here ]------------ -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD 2019-01-22 10:35 ` Rajendra Nayak @ 2019-01-23 11:16 ` Viresh Kumar 2019-01-23 14:22 ` Guenter Roeck 0 siblings, 1 reply; 12+ messages in thread From: Viresh Kumar @ 2019-01-23 11:16 UTC (permalink / raw) To: Rajendra Nayak Cc: Guenter Roeck, Stephen Boyd, andy.gross, david.brown, linux-arm-msm, linux-kernel, Ulf Hansson On 22-01-19, 16:05, Rajendra Nayak wrote: > Seeing this now during remove > > [ 34.008216] ------------[ cut here ]------------ > [ 34.012894] Device 'mss' does not have a release() function, it is broken and must be fixed. See Docum. > [ 34.024143] WARNING: CPU: 6 PID: 1600 at drivers/base/core.c:922 device_release+0x84/0x98 > [ 34.032379] Modules linked in: > [ 34.035477] CPU: 6 PID: 1600 Comm: sh Not tainted 5.0.0-rc1-00012-g0513915837c5-dirty #34 > [ 34.043724] Hardware name: Qualcomm Technologies, Inc. SDM845 MTP (DT) > [ 34.050305] pstate: 60400005 (nZCv daif +PAN -UAO) > [ 34.055141] pc : device_release+0x84/0x98 > [ 34.059186] lr : device_release+0x84/0x98 > [ 34.063229] sp : ffff0000183c3b40 > [ 34.066574] x29: ffff0000183c3b40 x28: ffff800174b55100 > [ 34.071930] x27: 0000000000000000 x26: 0000000000000000 > [ 34.077285] x25: 0000000056000000 x24: 0000000000000015 > [ 34.082640] x23: dead000000000100 x22: dead000000000200 > [ 34.087997] x21: 0000000000000000 x20: ffff0000112f9780 > [ 34.093352] x19: ffff0000112f9770 x18: ffff0000111dd6c8 > [ 34.098719] x17: 0000000000000000 x16: 0000000000000000 > [ 34.104072] x15: ffff0000983c3867 x14: 6966206562207473 > [ 34.109438] x13: 756d20646e61206e x12: 656b6f7262207369 > [ 34.114791] x11: 207469202c6e6f69 x10: 74636e7566202928 > [ 34.120147] x9 : 657361656c657220 x8 : 742e7463656a626f > [ 34.125514] x7 : 6b2f6e6f69746174 x6 : 0000000000000149 > [ 34.130867] x5 : 0000000000000020 x4 : 000000000001c200 > [ 34.136223] x3 : 00000000ffffffff x2 : ffff0000111f5828 > [ 34.141579] x1 : bc1d8ed5415ab200 x0 : 0000000000000000 > [ 34.146936] Call trace: > [ 34.149413] device_release+0x84/0x98 > [ 34.153116] kobject_put+0x98/0xe8 > [ 34.156551] put_device+0x14/0x28 > [ 34.159899] genpd_remove+0x108/0x190 > [ 34.163598] of_genpd_remove_last+0xbc/0xf0 > [ 34.167828] rpmhpd_remove+0x2c/0x48 > [ 34.171436] platform_drv_remove+0x24/0x68 > [ 34.175574] device_release_driver_internal+0x184/0x218 > [ 34.180845] device_release_driver+0x14/0x20 > [ 34.185152] unbind_store+0xec/0x138 > [ 34.188760] drv_attr_store+0x20/0x30 > [ 34.192465] sysfs_kf_write+0x48/0x58 > [ 34.196163] kernfs_fop_write+0xcc/0x1c8 > [ 34.200129] __vfs_write+0x34/0x170 > [ 34.203654] vfs_write+0xa8/0x1b8 > [ 34.206998] ksys_write+0x5c/0xc8 > [ 34.210345] __arm64_sys_write+0x14/0x20 > [ 34.214318] el0_svc_common+0xb4/0x118 > [ 34.218107] el0_svc_handler+0x2c/0x80 > [ 34.221899] el0_svc+0x8/0xc > [ 34.224804] ---[ end trace 9cee1458d0b6c175 ]--- > [ 34.229490] ------------[ cut here ]------------ This shall fix it: diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 500de1dee967..8a44b1f356d0 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1711,6 +1711,15 @@ static void genpd_lock_init(struct generic_pm_domain *genpd) } } +static void genpd_device_release(struct device *dev) +{ +} + +struct device_type genpd_device_type = { + .name = "genpd_device", + .release = genpd_device_release, +}; + /** * pm_genpd_init - Initialize a generic I/O PM domain object. * @genpd: PM domain object to initialize. @@ -1770,6 +1779,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd, pr_warn("%s : no governor for states\n", genpd->name); } + genpd->dev.type = &genpd_device_type; device_initialize(&genpd->dev); dev_set_name(&genpd->dev, "%s", genpd->name); @Ulf: You fine with this ? -- viresh ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD 2019-01-23 11:16 ` Viresh Kumar @ 2019-01-23 14:22 ` Guenter Roeck 2019-01-24 6:45 ` Viresh Kumar 0 siblings, 1 reply; 12+ messages in thread From: Guenter Roeck @ 2019-01-23 14:22 UTC (permalink / raw) To: Viresh Kumar, Rajendra Nayak Cc: Stephen Boyd, andy.gross, david.brown, linux-arm-msm, linux-kernel, Ulf Hansson On 1/23/19 3:16 AM, Viresh Kumar wrote: > On 22-01-19, 16:05, Rajendra Nayak wrote: >> Seeing this now during remove >> >> [ 34.008216] ------------[ cut here ]------------ >> [ 34.012894] Device 'mss' does not have a release() function, it is broken and must be fixed. See Docum. >> [ 34.024143] WARNING: CPU: 6 PID: 1600 at drivers/base/core.c:922 device_release+0x84/0x98 >> [ 34.032379] Modules linked in: >> [ 34.035477] CPU: 6 PID: 1600 Comm: sh Not tainted 5.0.0-rc1-00012-g0513915837c5-dirty #34 >> [ 34.043724] Hardware name: Qualcomm Technologies, Inc. SDM845 MTP (DT) >> [ 34.050305] pstate: 60400005 (nZCv daif +PAN -UAO) >> [ 34.055141] pc : device_release+0x84/0x98 >> [ 34.059186] lr : device_release+0x84/0x98 >> [ 34.063229] sp : ffff0000183c3b40 >> [ 34.066574] x29: ffff0000183c3b40 x28: ffff800174b55100 >> [ 34.071930] x27: 0000000000000000 x26: 0000000000000000 >> [ 34.077285] x25: 0000000056000000 x24: 0000000000000015 >> [ 34.082640] x23: dead000000000100 x22: dead000000000200 >> [ 34.087997] x21: 0000000000000000 x20: ffff0000112f9780 >> [ 34.093352] x19: ffff0000112f9770 x18: ffff0000111dd6c8 >> [ 34.098719] x17: 0000000000000000 x16: 0000000000000000 >> [ 34.104072] x15: ffff0000983c3867 x14: 6966206562207473 >> [ 34.109438] x13: 756d20646e61206e x12: 656b6f7262207369 >> [ 34.114791] x11: 207469202c6e6f69 x10: 74636e7566202928 >> [ 34.120147] x9 : 657361656c657220 x8 : 742e7463656a626f >> [ 34.125514] x7 : 6b2f6e6f69746174 x6 : 0000000000000149 >> [ 34.130867] x5 : 0000000000000020 x4 : 000000000001c200 >> [ 34.136223] x3 : 00000000ffffffff x2 : ffff0000111f5828 >> [ 34.141579] x1 : bc1d8ed5415ab200 x0 : 0000000000000000 >> [ 34.146936] Call trace: >> [ 34.149413] device_release+0x84/0x98 >> [ 34.153116] kobject_put+0x98/0xe8 >> [ 34.156551] put_device+0x14/0x28 >> [ 34.159899] genpd_remove+0x108/0x190 >> [ 34.163598] of_genpd_remove_last+0xbc/0xf0 >> [ 34.167828] rpmhpd_remove+0x2c/0x48 >> [ 34.171436] platform_drv_remove+0x24/0x68 >> [ 34.175574] device_release_driver_internal+0x184/0x218 >> [ 34.180845] device_release_driver+0x14/0x20 >> [ 34.185152] unbind_store+0xec/0x138 >> [ 34.188760] drv_attr_store+0x20/0x30 >> [ 34.192465] sysfs_kf_write+0x48/0x58 >> [ 34.196163] kernfs_fop_write+0xcc/0x1c8 >> [ 34.200129] __vfs_write+0x34/0x170 >> [ 34.203654] vfs_write+0xa8/0x1b8 >> [ 34.206998] ksys_write+0x5c/0xc8 >> [ 34.210345] __arm64_sys_write+0x14/0x20 >> [ 34.214318] el0_svc_common+0xb4/0x118 >> [ 34.218107] el0_svc_handler+0x2c/0x80 >> [ 34.221899] el0_svc+0x8/0xc >> [ 34.224804] ---[ end trace 9cee1458d0b6c175 ]--- >> [ 34.229490] ------------[ cut here ]------------ > > This shall fix it: > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 500de1dee967..8a44b1f356d0 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -1711,6 +1711,15 @@ static void genpd_lock_init(struct generic_pm_domain *genpd) > } > } > > +static void genpd_device_release(struct device *dev) > +{ > +} > + See Documentation/kobject.txt. "One important point cannot be overstated: every kobject must have a release() method, and the kobject must persist (in a consistent state) until that method is called. If these constraints are not met, the code is flawed. Note that the kernel will warn you if you forget to provide a release() method. Do not try to get rid of this warning by providing an "empty" release function. If all your cleanup function needs to do is call kfree(), then you must create a wrapper function which uses container_of() to upcast to the correct type (as shown in the example above) and then calls kfree() on the overall structure." Guenter > +struct device_type genpd_device_type = { > + .name = "genpd_device", > + .release = genpd_device_release, > +}; > + > /** > * pm_genpd_init - Initialize a generic I/O PM domain object. > * @genpd: PM domain object to initialize. > @@ -1770,6 +1779,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd, > pr_warn("%s : no governor for states\n", genpd->name); > } > > + genpd->dev.type = &genpd_device_type; > device_initialize(&genpd->dev); > dev_set_name(&genpd->dev, "%s", genpd->name); > > > @Ulf: You fine with this ? > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD 2019-01-23 14:22 ` Guenter Roeck @ 2019-01-24 6:45 ` Viresh Kumar 0 siblings, 0 replies; 12+ messages in thread From: Viresh Kumar @ 2019-01-24 6:45 UTC (permalink / raw) To: Guenter Roeck Cc: Rajendra Nayak, Stephen Boyd, andy.gross, david.brown, linux-arm-msm, linux-kernel, Ulf Hansson On 23-01-19, 06:22, Guenter Roeck wrote: > See Documentation/kobject.txt. > > "One important point cannot be overstated: every kobject must have a > release() method, and the kobject must persist (in a consistent state) > until that method is called. If these constraints are not met, the code is > flawed. Note that the kernel will warn you if you forget to provide a > release() method. Do not try to get rid of this warning by providing an > "empty" release function. > > If all your cleanup function needs to do is call kfree(), then you must > create a wrapper function which uses container_of() to upcast to the correct > type (as shown in the example above) and then calls kfree() on the overall > structure." Guenter, you are of course correct but I am not sure what else we can do here. In the most common case device & kobject are allocated along with the real entity, like struct genpd here, and we free those from the release routine (i.e. kfree(genpd)). But in our case most of the times the genpd is created statically by platform drivers and not by the genpd core, and so we can't free it from release(). Over that the genpd->dev is only used by the OPP core currently and the device isn't even registered with the device core. -- viresh ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD 2019-01-18 17:39 ` Stephen Boyd (?) (?) @ 2019-01-31 1:32 ` Bjorn Andersson -1 siblings, 0 replies; 12+ messages in thread From: Bjorn Andersson @ 2019-01-31 1:32 UTC (permalink / raw) To: Stephen Boyd, Rob Herring, Alexander Graf Cc: Rajendra Nayak, andy.gross, david.brown, linux-arm-msm, linux-kernel, linux On Fri 18 Jan 09:39 PST 2019, Stephen Boyd wrote: > Quoting Rajendra Nayak (2019-01-17 20:48:01) > > drivers/soc/qcom/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig > > index a5d5167c3f16..1ee298f6bf17 100644 > > --- a/drivers/soc/qcom/Kconfig > > +++ b/drivers/soc/qcom/Kconfig > > @@ -109,7 +109,7 @@ config QCOM_RPMHPD > > > > config QCOM_RPMPD > > bool "Qualcomm RPM Power domain driver" > > Just curious, does it need to be bool for some reason? > It's unfortunately not possible to have any genpd, iommu or pinctrl drivers compiled as modules, because once you pass lateinit probe deferral is purposefully broken. See driver_deferred_probe_check_state(). This also means that if you're unlucky and your kernel reached lateinit before the SMD communication with RPM is established and has brought up the rpmpd, you are left with a completely broken system. Unfortunately this isn't that hard to reproduce with a minimal defconfig. Regards, Bjorn ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-01-31 1:32 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-18 4:48 [PATCH] soc: qcom: update config dependencies for QCOM_RPMPD Rajendra Nayak 2019-01-18 17:39 ` Stephen Boyd 2019-01-18 17:39 ` Stephen Boyd 2019-01-22 2:30 ` Rajendra Nayak 2019-01-22 5:08 ` Guenter Roeck 2019-01-22 9:54 ` Rajendra Nayak 2019-01-22 10:08 ` Viresh Kumar 2019-01-22 10:35 ` Rajendra Nayak 2019-01-23 11:16 ` Viresh Kumar 2019-01-23 14:22 ` Guenter Roeck 2019-01-24 6:45 ` Viresh Kumar 2019-01-31 1:32 ` Bjorn Andersson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.