2020-10-05 06:01:48

by Maulik Shah

[permalink] [raw]
Subject: [RESEND v2 0/2] Modularize RPMH driver

Resending the patches rebasing on top of latest linux-next (next-20201002)

Changes in v2:
- Update commit message in patch 1
- send [4] again instead of revert's revert in patch 2.

This series is to modularize RPMH driver

The tracepoint in RPMH driver was changed to _rcuidle variant based on the
test results of unmerged series [1] where .power_off callback from genpd
reported RCU warnings.

The series which finally got merged [2] uses CPU PM notifications
and genpd .power_off callback is not implemented in RPMH driver to invoke
rpmh_flush(). The CPU PM notifications are done with RCU non idle in kernel
(see cpu_pm_notify() uses rcu_irq_enter_irqson() before notifications)

However using _rcuidle variant prevented RPMH driver to compile as module
since these _rcuidle are not exported symbols for tracepoints.

This seris reverts the change [3] to remove _rcuidle variant for tracepoint
as its no more valid test case (genpd .power_off is not implemented)
and bring backs the change [4] that was reverted due to _rcuidle preventing
to become modular.

[1] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=243931
[2] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=269733
[3] https://lore.kernel.org/r/[email protected]
[4] https://lore.kernel.org/r/[email protected]

John Stultz (1):
soc: qcom: rpmh: Allow RPMH driver to be loaded as a module

Maulik Shah (1):
Revert "drivers: qcom: rpmh-rsc: Use rcuidle tracepoints for rpmh"

drivers/soc/qcom/Kconfig | 2 +-
drivers/soc/qcom/rpmh-rsc.c | 7 ++++++-
2 files changed, 7 insertions(+), 2 deletions(-)

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


2020-10-05 06:02:41

by Maulik Shah

[permalink] [raw]
Subject: [RESEND v2 2/2] soc: qcom: rpmh: Allow RPMH driver to be loaded as a module

From: John Stultz <[email protected]>

This patch allow the rpmh driver to be loaded as a permenent
module. Meaning it can be loaded from a module, but then cannot
be unloaded.

Ideally, it would include a remove hook and related logic, but
the rpmh driver is fairly core to the system, so once its loaded
with almost anything else to get the system to go, the dependencies
are not likely to ever also be removed.

So making it a permanent module at least improves things slightly
over requiring it to be a built in driver.

Cc: Todd Kjos <[email protected]>
Cc: Saravana Kannan <[email protected]>
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Rajendra Nayak <[email protected]>
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
[mkshah: Fix typos in commit message, send after removing _rcuidle trace]
Signed-off-by: Maulik Shah <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
---
drivers/soc/qcom/Kconfig | 2 +-
drivers/soc/qcom/rpmh-rsc.c | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 3dc3e3d..892bdc7 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -92,7 +92,7 @@ config QCOM_RMTFS_MEM
Say y here if you intend to boot the modem remoteproc.

config QCOM_RPMH
- bool "Qualcomm RPM-Hardened (RPMH) Communication"
+ tristate "Qualcomm RPM-Hardened (RPMH) Communication"
depends on ARCH_QCOM || COMPILE_TEST
help
Support for communication with the hardened-RPM blocks in
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 35a7a50..37969dc 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -13,6 +13,7 @@
#include <linux/iopoll.h>
#include <linux/kernel.h>
#include <linux/list.h>
+#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
@@ -1018,6 +1019,7 @@ static const struct of_device_id rpmh_drv_match[] = {
{ .compatible = "qcom,rpmh-rsc", },
{ }
};
+MODULE_DEVICE_TABLE(of, rpmh_drv_match);

static struct platform_driver rpmh_driver = {
.probe = rpmh_rsc_probe,
@@ -1033,3 +1035,6 @@ static int __init rpmh_driver_init(void)
return platform_driver_register(&rpmh_driver);
}
arch_initcall(rpmh_driver_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. RPMh Driver");
+MODULE_LICENSE("GPL v2");
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2020-10-05 06:04:53

by Maulik Shah

[permalink] [raw]
Subject: [RESEND v2 1/2] Revert "drivers: qcom: rpmh-rsc: Use rcuidle tracepoints for rpmh"

Commit efde2659b0fe ("drivers: qcom: rpmh-rsc: Use rcuidle tracepoints
for rpmh") was written to fix a bug seen in an unmerged series that
implemented a struct generic_pm_domain::power_off() callback calling
rpmh_flush(). See stack trace below.

Call trace:
dump_backtrace+0x0/0x174
show_stack+0x20/0x2c
dump_stack+0xc8/0x124
lockdep_rcu_suspicious+0xe4/0x104
__tcs_buffer_write+0x230/0x2d0
rpmh_rsc_write_ctrl_data+0x210/0x270
rpmh_flush+0x84/0x24c
rpmh_domain_power_off+0x78/0x98
_genpd_power_off+0x40/0xc0
genpd_power_off+0x168/0x208

Later the final merged solution is to use CPU PM notification to invoke
rpmh_flush() and power_off() callback of genpd is not implemented in the
driver.

CPU PM notifiers are run with RCU enabled/watching (see cpu_pm_notify()
and how it calls rcu_irq_enter_irqson() before calling the notifiers).

Remove this change since RCU will not be idle during CPU PM notifications
hence not required to use _rcuidle tracepoint. Using _rcuidle tracepoint
prevented rpmh driver to be loadable module as these are not exported
symbols.

This reverts commit efde2659b0fe835732047357b2902cca14f054d9.

Cc: Sai Prakash Ranjan <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
Reviewed-by: Sai Prakash Ranjan <[email protected]>
Reviewed-by: Ulf Hansson <[email protected]>
Signed-off-by: Maulik Shah <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
---
drivers/soc/qcom/rpmh-rsc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index a297911..35a7a50 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -497,7 +497,7 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
write_tcs_cmd(drv, RSC_DRV_CMD_MSGID, tcs_id, j, msgid);
write_tcs_cmd(drv, RSC_DRV_CMD_ADDR, tcs_id, j, cmd->addr);
write_tcs_cmd(drv, RSC_DRV_CMD_DATA, tcs_id, j, cmd->data);
- trace_rpmh_send_msg_rcuidle(drv, tcs_id, j, msgid, cmd);
+ trace_rpmh_send_msg(drv, tcs_id, j, msgid, cmd);
}

write_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, tcs_id, cmd_complete);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2020-10-19 10:25:49

by Maulik Shah

[permalink] [raw]
Subject: Re: [RESEND v2 0/2] Modularize RPMH driver

Hi Bjorn,

Can you please pick these changes.

Thanks,
Maulik

On 10/5/2020 11:29 AM, Maulik Shah wrote:
> Resending the patches rebasing on top of latest linux-next (next-20201002)
>
> Changes in v2:
> - Update commit message in patch 1
> - send [4] again instead of revert's revert in patch 2.
>
> This series is to modularize RPMH driver
>
> The tracepoint in RPMH driver was changed to _rcuidle variant based on the
> test results of unmerged series [1] where .power_off callback from genpd
> reported RCU warnings.
>
> The series which finally got merged [2] uses CPU PM notifications
> and genpd .power_off callback is not implemented in RPMH driver to invoke
> rpmh_flush(). The CPU PM notifications are done with RCU non idle in kernel
> (see cpu_pm_notify() uses rcu_irq_enter_irqson() before notifications)
>
> However using _rcuidle variant prevented RPMH driver to compile as module
> since these _rcuidle are not exported symbols for tracepoints.
>
> This seris reverts the change [3] to remove _rcuidle variant for tracepoint
> as its no more valid test case (genpd .power_off is not implemented)
> and bring backs the change [4] that was reverted due to _rcuidle preventing
> to become modular.
>
> [1] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=243931
> [2] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=269733
> [3] https://lore.kernel.org/r/[email protected]
> [4] https://lore.kernel.org/r/[email protected]
>
> John Stultz (1):
> soc: qcom: rpmh: Allow RPMH driver to be loaded as a module
>
> Maulik Shah (1):
> Revert "drivers: qcom: rpmh-rsc: Use rcuidle tracepoints for rpmh"
>
> drivers/soc/qcom/Kconfig | 2 +-
> drivers/soc/qcom/rpmh-rsc.c | 7 ++++++-
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation