From f1013d8405af0f4fe1812b5901cbac9ce4d1fb6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Mon, 25 Sep 2023 11:55:31 +0200 Subject: [PATCH 1/5] soc/xilinx: zynqmp_power: Convert to platform remove callback returning void MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new() which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20230925095532.1984344-41-u.kleine-koenig@pengutronix.de Signed-off-by: Michal Simek --- drivers/soc/xilinx/zynqmp_power.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/soc/xilinx/zynqmp_power.c b/drivers/soc/xilinx/zynqmp_power.c index c2c819701eec..9d27f850f491 100644 --- a/drivers/soc/xilinx/zynqmp_power.c +++ b/drivers/soc/xilinx/zynqmp_power.c @@ -275,7 +275,7 @@ static int zynqmp_pm_probe(struct platform_device *pdev) return 0; } -static int zynqmp_pm_remove(struct platform_device *pdev) +static void zynqmp_pm_remove(struct platform_device *pdev) { sysfs_remove_file(&pdev->dev.kobj, &dev_attr_suspend_mode.attr); if (event_registered) @@ -283,8 +283,6 @@ static int zynqmp_pm_remove(struct platform_device *pdev) if (!rx_chan) mbox_free_channel(rx_chan); - - return 0; } static const struct of_device_id pm_of_match[] = { @@ -295,7 +293,7 @@ MODULE_DEVICE_TABLE(of, pm_of_match); static struct platform_driver zynqmp_pm_platform_driver = { .probe = zynqmp_pm_probe, - .remove = zynqmp_pm_remove, + .remove_new = zynqmp_pm_remove, .driver = { .name = "zynqmp_power", .of_match_table = pm_of_match, From daed80ed07580e5adc0e6d8bc79933a35154135a Mon Sep 17 00:00:00 2001 From: HariBabu Gattem Date: Thu, 26 Oct 2023 22:56:22 -0700 Subject: [PATCH 2/5] soc: xilinx: Fix for call trace due to the usage of smp_processor_id() When preemption is enabled in kernel and if any task which can be preempted should not use smp_processor_id() directly, since CPU switch can happen at any time, the previous value of cpu_id differs with current cpu_id. As a result we see the below call trace during xlnx_event_manager_probe. [ 6.140197] dump_backtrace+0x0/0x190 [ 6.143884] show_stack+0x18/0x40 [ 6.147220] dump_stack_lvl+0x7c/0xa0 [ 6.150907] dump_stack+0x18/0x34 [ 6.154241] check_preemption_disabled+0x124/0x134 [ 6.159068] debug_smp_processor_id+0x20/0x2c [ 6.163453] xlnx_event_manager_probe+0x48/0x250 To protect cpu_id, It is recommended to use get_cpu()/put_cpu() to disable preemption, get the cpu_id and enable preemption respectively. (For Reference, Documentation/locking/preempt-locking.rst and Documentation/kernel-hacking/hacking.rst) Use preempt_disable()/smp_processor_id()/preempt_enable() API's to achieve the same. Signed-off-by: HariBabu Gattem Signed-off-by: Jay Buddhabhatti Link: https://lore.kernel.org/r/20231027055622.21544-1-jay.buddhabhatti@amd.com Signed-off-by: Michal Simek --- drivers/soc/xilinx/xlnx_event_manager.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/soc/xilinx/xlnx_event_manager.c b/drivers/soc/xilinx/xlnx_event_manager.c index 86a048a10a13..edfb1d5c10c6 100644 --- a/drivers/soc/xilinx/xlnx_event_manager.c +++ b/drivers/soc/xilinx/xlnx_event_manager.c @@ -555,7 +555,7 @@ static void xlnx_disable_percpu_irq(void *data) static int xlnx_event_init_sgi(struct platform_device *pdev) { int ret = 0; - int cpu = smp_processor_id(); + int cpu; /* * IRQ related structures are used for the following: * for each SGI interrupt ensure its mapped by GIC IRQ domain @@ -592,9 +592,12 @@ static int xlnx_event_init_sgi(struct platform_device *pdev) sgi_fwspec.param[0] = sgi_num; virq_sgi = irq_create_fwspec_mapping(&sgi_fwspec); + cpu = get_cpu(); per_cpu(cpu_number1, cpu) = cpu; ret = request_percpu_irq(virq_sgi, xlnx_event_handler, "xlnx_event_mgmt", &cpu_number1); + put_cpu(); + WARN_ON(ret); if (ret) { irq_dispose_mapping(virq_sgi); From 4c0afac2dfa1e11bbea9ef69da5c3c2bfdfe4c43 Mon Sep 17 00:00:00 2001 From: Michal Simek Date: Fri, 27 Oct 2023 23:41:35 +0530 Subject: [PATCH 3/5] soc: xilinx: fix quoted string split across lines Fix checkpatch warning "quoted string split across lines". No functional change. Signed-off-by: Michal Simek Signed-off-by: Radhey Shyam Pandey Link: https://lore.kernel.org/r/1698430295-2731040-1-git-send-email-radhey.shyam.pandey@amd.com --- drivers/soc/xilinx/zynqmp_power.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/soc/xilinx/zynqmp_power.c b/drivers/soc/xilinx/zynqmp_power.c index 9d27f850f491..abcf7e883279 100644 --- a/drivers/soc/xilinx/zynqmp_power.c +++ b/drivers/soc/xilinx/zynqmp_power.c @@ -83,8 +83,8 @@ static irqreturn_t zynqmp_pm_isr(int irq, void *data) pm_suspend(PM_SUSPEND_MEM); break; default: - pr_err("%s Unsupported InitSuspendCb reason " - "code %d\n", __func__, payload[1]); + pr_err("%s Unsupported InitSuspendCb reason code %d\n", + __func__, payload[1]); } } @@ -252,8 +252,8 @@ static int zynqmp_pm_probe(struct platform_device *pdev) dev_name(&pdev->dev), &pdev->dev); if (ret) { - dev_err(&pdev->dev, "devm_request_threaded_irq '%d' " - "failed with %d\n", irq, ret); + dev_err(&pdev->dev, "devm_request_threaded_irq '%d' failed with %d\n", + irq, ret); return ret; } } else { From 9c6724abf969251af53cdae525ad8100ec78d3c2 Mon Sep 17 00:00:00 2001 From: Tanmay Shah Date: Fri, 27 Oct 2023 23:53:59 +0530 Subject: [PATCH 4/5] soc: xilinx: fix unhandled SGI warning message Xen broadcasts SGI to each VM when multiple VMs run on Xen hypervisor. In such case spurious SGI is expected if one event is registered by one VM and not registered by another VM. We let users know that Unhandled SGI is not error and expected if kernel is running on Xen hypervisor. Signed-off-by: Tanmay Shah Signed-off-by: Radhey Shyam Pandey Link: https://lore.kernel.org/r/1698431039-2734260-1-git-send-email-radhey.shyam.pandey@amd.com Signed-off-by: Michal Simek --- drivers/soc/xilinx/xlnx_event_manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soc/xilinx/xlnx_event_manager.c b/drivers/soc/xilinx/xlnx_event_manager.c index edfb1d5c10c6..042553abe1bf 100644 --- a/drivers/soc/xilinx/xlnx_event_manager.c +++ b/drivers/soc/xilinx/xlnx_event_manager.c @@ -477,7 +477,7 @@ static void xlnx_call_notify_cb_handler(const u32 *payload) } } if (!is_callback_found) - pr_warn("Didn't find any registered callback for 0x%x 0x%x\n", + pr_warn("Unhandled SGI node 0x%x event 0x%x. Expected with Xen hypervisor\n", payload[1], payload[2]); } From 87fda1acfc3b9048261799817ef749e6deb95724 Mon Sep 17 00:00:00 2001 From: Naman Trivedi Manojbhai Date: Thu, 7 Dec 2023 16:24:08 +0100 Subject: [PATCH 5/5] soc: xilinx: Add error message for invalid payload received from IPI callback. payload[0] of response buffer of zynqmp_pm_get_callback_data() contains valid payload or error code in case of error. Added error message to inform user about the error code received in payload[0]. Signed-off-by: Naman Trivedi Manojbhai Signed-off-by: Michal Simek Link: https://lore.kernel.org/r/85749bde3e71148533d31ea2092f4514ec347768.1701962639.git.michal.simek@amd.com --- drivers/soc/xilinx/zynqmp_power.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/soc/xilinx/zynqmp_power.c b/drivers/soc/xilinx/zynqmp_power.c index abcf7e883279..07d735b38b49 100644 --- a/drivers/soc/xilinx/zynqmp_power.c +++ b/drivers/soc/xilinx/zynqmp_power.c @@ -86,6 +86,8 @@ static irqreturn_t zynqmp_pm_isr(int irq, void *data) pr_err("%s Unsupported InitSuspendCb reason code %d\n", __func__, payload[1]); } + } else { + pr_err("%s() Unsupported Callback %d\n", __func__, payload[0]); } return IRQ_HANDLED;