From 45b9e04d5ba0b043783dfe2b19bb728e712cb32e Mon Sep 17 00:00:00 2001 From: Etienne Carriere Date: Thu, 8 Oct 2020 16:37:22 +0200 Subject: firmware: arm_scmi: Fix ARCH_COLD_RESET The defination for ARCH_COLD_RESET is wrong. Let us fix it according to the SCMI specification. Link: https://lore.kernel.org/r/20201008143722.21888-5-etienne.carriere@linaro.org Fixes: 95a15d80aa0d ("firmware: arm_scmi: Add RESET protocol in SCMI v2.0") Signed-off-by: Etienne Carriere Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/reset.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/reset.c b/drivers/firmware/arm_scmi/reset.c index f063cfe17e02..a981a22cfe89 100644 --- a/drivers/firmware/arm_scmi/reset.c +++ b/drivers/firmware/arm_scmi/reset.c @@ -36,9 +36,7 @@ struct scmi_msg_reset_domain_reset { #define EXPLICIT_RESET_ASSERT BIT(1) #define ASYNCHRONOUS_RESET BIT(2) __le32 reset_state; -#define ARCH_RESET_TYPE BIT(31) -#define COLD_RESET_STATE BIT(0) -#define ARCH_COLD_RESET (ARCH_RESET_TYPE | COLD_RESET_STATE) +#define ARCH_COLD_RESET 0 }; struct scmi_msg_reset_notify { -- cgit v1.2.3 From 7adb2c8aaaa6a387af7140e57004beba2c04a4c6 Mon Sep 17 00:00:00 2001 From: Etienne Carriere Date: Thu, 8 Oct 2020 16:37:21 +0200 Subject: firmware: arm_scmi: Expand SMC/HVC message pool to more than one SMC/HVC can transmit only one message at the time as the shared memory needs to be protected and the calls are synchronous. However, in order to allow multiple threads to send SCMI messages simultaneously, we need a larger poll of memory. Let us just use value of 20 to keep it in sync mailbox transport implementation. Any other value must work perfectly. Link: https://lore.kernel.org/r/20201008143722.21888-4-etienne.carriere@linaro.org Fixes: 1dc6558062da ("firmware: arm_scmi: Add smc/hvc transport") Cc: Peng Fan Signed-off-by: Etienne Carriere [sudeep.holla: reworded the commit message to indicate the practicality] Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/smc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c index 1a03c3ec0230..82a82a5dc86a 100644 --- a/drivers/firmware/arm_scmi/smc.c +++ b/drivers/firmware/arm_scmi/smc.c @@ -149,6 +149,6 @@ static const struct scmi_transport_ops scmi_smc_ops = { const struct scmi_desc scmi_smc_desc = { .ops = &scmi_smc_ops, .max_rx_timeout_ms = 30, - .max_msg = 1, + .max_msg = 20, .max_msg_size = 128, }; -- cgit v1.2.3 From 9724722fde8f9bbd2b87340f00b9300c9284001e Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Mon, 12 Oct 2020 14:26:24 +0100 Subject: firmware: arm_scmi: Add missing Rx size re-initialisation Few commands provide the list of description partially and require to be called consecutively until all the descriptors are fetched completely. In such cases, we don't release the buffers and reuse them for consecutive transmits. However, currently we don't reset the Rx size which will be set as per the response for the last transmit. This may result in incorrect response size being interpretted as the firmware may repond with size greater than the one set but we read only upto the size set by previous response. Let us reset the receive buffer size to max possible in such cases as we don't know the exact size of the response. Link: https://lore.kernel.org/r/20201012141746.32575-1-sudeep.holla@arm.com Fixes: b6f20ff8bd94 ("firmware: arm_scmi: add common infrastructure and support for base protocol") Reported-by: Etienne Carriere Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/base.c | 2 ++ drivers/firmware/arm_scmi/clock.c | 2 ++ drivers/firmware/arm_scmi/common.h | 2 ++ drivers/firmware/arm_scmi/driver.c | 8 ++++++++ drivers/firmware/arm_scmi/perf.c | 2 ++ drivers/firmware/arm_scmi/sensors.c | 2 ++ 6 files changed, 18 insertions(+) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c index 9853bd3c4d45..017e5d8bd869 100644 --- a/drivers/firmware/arm_scmi/base.c +++ b/drivers/firmware/arm_scmi/base.c @@ -197,6 +197,8 @@ static int scmi_base_implementation_list_get(const struct scmi_handle *handle, protocols_imp[tot_num_ret + loop] = *(list + loop); tot_num_ret += loop_num_ret; + + scmi_reset_rx_to_maxsz(handle, t); } while (loop_num_ret); scmi_xfer_put(handle, t); diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index c1cfe3ee3d55..4645677d86f1 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -192,6 +192,8 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, } tot_rate_cnt += num_returned; + + scmi_reset_rx_to_maxsz(handle, t); /* * check for both returned and remaining to avoid infinite * loop due to buggy firmware diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 37fb583f1bf5..65063fa948d4 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -147,6 +147,8 @@ int scmi_do_xfer_with_response(const struct scmi_handle *h, struct scmi_xfer *xfer); int scmi_xfer_get_init(const struct scmi_handle *h, u8 msg_id, u8 prot_id, size_t tx_size, size_t rx_size, struct scmi_xfer **p); +void scmi_reset_rx_to_maxsz(const struct scmi_handle *handle, + struct scmi_xfer *xfer); int scmi_handle_put(const struct scmi_handle *handle); struct scmi_handle *scmi_handle_get(struct device *dev); void scmi_set_handle(struct scmi_device *scmi_dev); diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index c5dea87edf8f..3dfd8b6a0ebf 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -402,6 +402,14 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) return ret; } +void scmi_reset_rx_to_maxsz(const struct scmi_handle *handle, + struct scmi_xfer *xfer) +{ + struct scmi_info *info = handle_to_scmi_info(handle); + + xfer->rx.len = info->desc->max_msg_size; +} + #define SCMI_MAX_RESPONSE_TIMEOUT (2 * MSEC_PER_SEC) /** diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index ed475b40bd08..82fb3babff72 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -304,6 +304,8 @@ scmi_perf_describe_levels_get(const struct scmi_handle *handle, u32 domain, } tot_opp_cnt += num_returned; + + scmi_reset_rx_to_maxsz(handle, t); /* * check for both returned and remaining to avoid infinite * loop due to buggy firmware diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c index 9703cf6356a0..b4232d611033 100644 --- a/drivers/firmware/arm_scmi/sensors.c +++ b/drivers/firmware/arm_scmi/sensors.c @@ -166,6 +166,8 @@ static int scmi_sensor_description_get(const struct scmi_handle *handle, } desc_index += num_returned; + + scmi_reset_rx_to_maxsz(handle, t); /* * check for both returned and remaining to avoid infinite * loop due to buggy firmware -- cgit v1.2.3 From c7821c2d9c0dda0adf2bcf88e79b02a19a430be4 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Tue, 13 Oct 2020 14:31:09 +0100 Subject: firmware: arm_scmi: Fix locking in notifications When a protocol registers its events, the notification core takes care to rescan the hashtable of pending event handlers and activate all the possibly existent handlers referring to any of the events that are just registered by the new protocol. When a pending handler becomes active the core requests and enables the corresponding events in the SCMI firmware. If, for whatever reason, the enable fails, such invalid event handler must be finally removed and freed. Let us ensure to use the scmi_put_active_handler() helper which handles properly the needed additional locking. Failing to properly acquire all the needed mutexes exposes a race that leads to the following splat being observed: WARNING: CPU: 0 PID: 388 at lib/refcount.c:28 refcount_warn_saturate+0xf8/0x148 Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Platform, BIOS EDK II Jun 30 2020 pstate: 40000005 (nZcv daif -PAN -UAO BTYPE=--) pc : refcount_warn_saturate+0xf8/0x148 lr : refcount_warn_saturate+0xf8/0x148 Call trace: refcount_warn_saturate+0xf8/0x148 scmi_put_handler_unlocked.isra.10+0x204/0x208 scmi_put_handler+0x50/0xa0 scmi_unregister_notifier+0x1bc/0x240 scmi_notify_tester_remove+0x4c/0x68 [dummy_scmi_consumer] scmi_dev_remove+0x54/0x68 device_release_driver_internal+0x114/0x1e8 driver_detach+0x58/0xe8 bus_remove_driver+0x88/0xe0 driver_unregister+0x38/0x68 scmi_driver_unregister+0x1c/0x28 scmi_drv_exit+0x1c/0xae0 [dummy_scmi_consumer] __arm64_sys_delete_module+0x1a4/0x268 el0_svc_common.constprop.3+0x94/0x178 do_el0_svc+0x2c/0x98 el0_sync_handler+0x148/0x1a8 el0_sync+0x158/0x180 Link: https://lore.kernel.org/r/20201013133109.49821-1-cristian.marussi@arm.com Fixes: e7c215f358a35 ("firmware: arm_scmi: Add notification callbacks-registration") Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/notify.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c index 2754f9d01636..c24e427dce0d 100644 --- a/drivers/firmware/arm_scmi/notify.c +++ b/drivers/firmware/arm_scmi/notify.c @@ -1403,15 +1403,21 @@ static void scmi_protocols_late_init(struct work_struct *work) "finalized PENDING handler - key:%X\n", hndl->key); ret = scmi_event_handler_enable_events(hndl); + if (ret) { + dev_dbg(ni->handle->dev, + "purging INVALID handler - key:%X\n", + hndl->key); + scmi_put_active_handler(ni, hndl); + } } else { ret = scmi_valid_pending_handler(ni, hndl); - } - if (ret) { - dev_dbg(ni->handle->dev, - "purging PENDING handler - key:%X\n", - hndl->key); - /* this hndl can be only a pending one */ - scmi_put_handler_unlocked(ni, hndl); + if (ret) { + dev_dbg(ni->handle->dev, + "purging PENDING handler - key:%X\n", + hndl->key); + /* this hndl can be only a pending one */ + scmi_put_handler_unlocked(ni, hndl); + } } } mutex_unlock(&ni->pending_mtx); -- cgit v1.2.3 From b9ceca6be43233845be70792be9b5ab315d2e010 Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Tue, 13 Oct 2020 19:17:37 -0700 Subject: firmware: arm_scmi: Fix duplicate workqueue name When more than a single SCMI device are present in the system, the creation of the notification workqueue with the WQ_SYSFS flag will lead to the following sysfs duplicate node warning: sysfs: cannot create duplicate filename '/devices/virtual/workqueue/scmi_notify' CPU: 0 PID: 20 Comm: kworker/0:1 Not tainted 5.9.0-gdf4dd84a3f7d #29 Hardware name: Broadcom STB (Flattened Device Tree) Workqueue: events deferred_probe_work_func Backtrace: show_stack + 0x20/0x24 dump_stack + 0xbc/0xe0 sysfs_warn_dup + 0x70/0x80 sysfs_create_dir_ns + 0x15c/0x1a4 kobject_add_internal + 0x140/0x4d0 kobject_add + 0xc8/0x138 device_add + 0x1dc/0xc20 device_register + 0x24/0x28 workqueue_sysfs_register + 0xe4/0x1f0 alloc_workqueue + 0x448/0x6ac scmi_notification_init + 0x78/0x1dc scmi_probe + 0x268/0x4fc platform_drv_probe + 0x70/0xc8 really_probe + 0x184/0x728 driver_probe_device + 0xa4/0x278 __device_attach_driver + 0xe8/0x148 bus_for_each_drv + 0x108/0x158 __device_attach + 0x190/0x234 device_initial_probe + 0x1c/0x20 bus_probe_device + 0xdc/0xec deferred_probe_work_func + 0xd4/0x11c process_one_work + 0x420/0x8f0 worker_thread + 0x4fc/0x91c kthread + 0x21c/0x22c ret_from_fork + 0x14/0x20 kobject_add_internal failed for scmi_notify with -EEXIST, don't try to register things with the same name in the same directory. arm-scmi brcm_scmi@1: SCMI Notifications - Initialization Failed. arm-scmi brcm_scmi@1: SCMI Notifications NOT available. arm-scmi brcm_scmi@1: SCMI Protocol v1.0 'brcm-scmi:' Firmware version 0x1 Fix this by using dev_name(handle->dev) which guarantees that the name is unique and this also helps correlate which notification workqueue corresponds to which SCMI device instance. Link: https://lore.kernel.org/r/20201014021737.287340-1-f.fainelli@gmail.com Fixes: bd31b249692e ("firmware: arm_scmi: Add notification dispatch and delivery") Signed-off-by: Florian Fainelli [sudeep.holla: trimmed backtrace to remove all unwanted hexcodes and timestamps] Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/notify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c index c24e427dce0d..ce336899d636 100644 --- a/drivers/firmware/arm_scmi/notify.c +++ b/drivers/firmware/arm_scmi/notify.c @@ -1474,7 +1474,7 @@ int scmi_notification_init(struct scmi_handle *handle) ni->gid = gid; ni->handle = handle; - ni->notify_wq = alloc_workqueue("scmi_notify", + ni->notify_wq = alloc_workqueue(dev_name(handle->dev), WQ_UNBOUND | WQ_FREEZABLE | WQ_SYSFS, 0); if (!ni->notify_wq) -- cgit v1.2.3