drivers/firmware/arm_scmi/notify.c | 39 +++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 9 deletions(-)
Some platforms could be configured not to support notification events from
specific sources and such a case is already handled properly by avoiding
even to attempt to send a notification enable request since it would be
doomed to fail anyway.
In an extreme scenario, though, a platform could support not even one
single source on a specific event: in such a case would be meaningless to
even allow to register a notifier and we can bail-out immediately, saving
a lot of needless computation.
Flag such condition, when detected at protocol initialization time, and
reject upfront any attempt to register a notifier for such completely
unsupported events with -ENOTSUPP.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
NOT FOR MERGE until tested properly even with late loaded protocols.
DOES NOT address the issues with verobosity of messages and lack of
details about failures (which protos ? which resources ?)
---
drivers/firmware/arm_scmi/notify.c | 39 +++++++++++++++++++++++-------
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
index e160ecb22948..dee9f238f6fd 100644
--- a/drivers/firmware/arm_scmi/notify.c
+++ b/drivers/firmware/arm_scmi/notify.c
@@ -318,6 +318,9 @@ struct scmi_registered_events_desc {
* customized event report
* @num_sources: The number of possible sources for this event as stated at
* events' registration time
+ * @not_supported_by_platform: A flag to indicate that not even one source was
+ * found to be supported by the platform for this
+ * event
* @sources: A reference to a dynamically allocated array used to refcount the
* events' enable requests for all the existing sources
* @sources_mtx: A mutex to serialize the access to @sources
@@ -334,6 +337,7 @@ struct scmi_registered_event {
const struct scmi_event *evt;
void *report;
u32 num_sources;
+ bool not_supported_by_platform;
refcount_t *sources;
/* locking to serialize the access to sources */
struct mutex sources_mtx;
@@ -811,10 +815,19 @@ int scmi_register_protocol_events(const struct scmi_handle *handle, u8 proto_id,
if (!r_evt->report)
return -ENOMEM;
- for (id = 0; id < r_evt->num_sources; id++)
- if (ee->ops->is_notify_supported &&
- !ee->ops->is_notify_supported(ph, r_evt->evt->id, id))
- refcount_set(&r_evt->sources[id], NOTIF_UNSUPP);
+ if (ee->ops->is_notify_supported) {
+ int supported = 0;
+
+ for (id = 0; id < r_evt->num_sources; id++) {
+ if (!ee->ops->is_notify_supported(ph, r_evt->evt->id, id))
+ refcount_set(&r_evt->sources[id], NOTIF_UNSUPP);
+ else
+ supported++;
+ }
+
+ /* Not even one source has been found to be supported */
+ r_evt->not_supported_by_platform = !supported;
+ }
pd->registered_events[i] = r_evt;
/* Ensure events are updated */
@@ -936,6 +949,11 @@ static inline int scmi_bind_event_handler(struct scmi_notify_instance *ni,
* of protocol instance.
*/
hash_del(&hndl->hash);
+
+ /* Bailout if event is not supported at all */
+ if (r_evt->not_supported_by_platform)
+ return -EOPNOTSUPP;
+
/*
* Acquire protocols only for NON pending handlers, so as NOT to trigger
* protocol initialization when a notifier is registered against a still
@@ -1060,6 +1078,9 @@ __scmi_event_handler_get_ops(struct scmi_notify_instance *ni,
r_evt = SCMI_GET_REVT(ni, KEY_XTRACT_PROTO_ID(evt_key),
KEY_XTRACT_EVT_ID(evt_key));
+ if (r_evt && r_evt->not_supported_by_platform)
+ return ERR_PTR(-EOPNOTSUPP);
+
mutex_lock(&ni->pending_mtx);
/* Search registered events at first ... if possible at all */
if (r_evt) {
@@ -1087,7 +1108,7 @@ __scmi_event_handler_get_ops(struct scmi_notify_instance *ni,
hndl->key);
/* this hndl can be only a pending one */
scmi_put_handler_unlocked(ni, hndl);
- hndl = NULL;
+ hndl = ERR_PTR(-EINVAL);
}
}
mutex_unlock(&ni->pending_mtx);
@@ -1370,8 +1391,8 @@ static int scmi_notifier_register(const struct scmi_handle *handle,
evt_key = MAKE_HASH_KEY(proto_id, evt_id,
src_id ? *src_id : SRC_ID_MASK);
hndl = scmi_get_or_create_handler(ni, evt_key);
- if (!hndl)
- return -EINVAL;
+ if (IS_ERR(hndl))
+ return PTR_ERR(hndl);
blocking_notifier_chain_register(&hndl->chain, nb);
@@ -1416,8 +1437,8 @@ static int scmi_notifier_unregister(const struct scmi_handle *handle,
evt_key = MAKE_HASH_KEY(proto_id, evt_id,
src_id ? *src_id : SRC_ID_MASK);
hndl = scmi_get_handler(ni, evt_key);
- if (!hndl)
- return -EINVAL;
+ if (IS_ERR(hndl))
+ return PTR_ERR(hndl);
/*
* Note that this chain unregistration call is safe on its own
--
2.47.0Hi Cristian,
On Tue, Jun 17, 2025 at 02:50:38PM +0100, Cristian Marussi wrote:
>Some platforms could be configured not to support notification events from
>specific sources and such a case is already handled properly by avoiding
>even to attempt to send a notification enable request since it would be
>doomed to fail anyway.
>
>In an extreme scenario, though, a platform could support not even one
>single source on a specific event: in such a case would be meaningless to
>even allow to register a notifier and we can bail-out immediately, saving
>a lot of needless computation.
>
>Flag such condition, when detected at protocol initialization time, and
>reject upfront any attempt to register a notifier for such completely
>unsupported events with -ENOTSUPP.
>
>Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
>---
>NOT FOR MERGE until tested properly even with late loaded protocols.
>DOES NOT address the issues with verobosity of messages and lack of
>details about failures (which protos ? which resources ?)
I tested this patch on i.MX95, no error log anymore. Except
the one in cpufreq:
scmi-cpufreq scmi_dev.5: failed to register for limits change notifier for domain 8
The ret is -EOPNOTSUPP.
Thanks,
Peng
>---
> drivers/firmware/arm_scmi/notify.c | 39 +++++++++++++++++++++++-------
> 1 file changed, 30 insertions(+), 9 deletions(-)
>
>diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
>index e160ecb22948..dee9f238f6fd 100644
>--- a/drivers/firmware/arm_scmi/notify.c
>+++ b/drivers/firmware/arm_scmi/notify.c
>@@ -318,6 +318,9 @@ struct scmi_registered_events_desc {
> * customized event report
> * @num_sources: The number of possible sources for this event as stated at
> * events' registration time
>+ * @not_supported_by_platform: A flag to indicate that not even one source was
>+ * found to be supported by the platform for this
>+ * event
> * @sources: A reference to a dynamically allocated array used to refcount the
> * events' enable requests for all the existing sources
> * @sources_mtx: A mutex to serialize the access to @sources
>@@ -334,6 +337,7 @@ struct scmi_registered_event {
> const struct scmi_event *evt;
> void *report;
> u32 num_sources;
>+ bool not_supported_by_platform;
> refcount_t *sources;
> /* locking to serialize the access to sources */
> struct mutex sources_mtx;
>@@ -811,10 +815,19 @@ int scmi_register_protocol_events(const struct scmi_handle *handle, u8 proto_id,
> if (!r_evt->report)
> return -ENOMEM;
>
>- for (id = 0; id < r_evt->num_sources; id++)
>- if (ee->ops->is_notify_supported &&
>- !ee->ops->is_notify_supported(ph, r_evt->evt->id, id))
>- refcount_set(&r_evt->sources[id], NOTIF_UNSUPP);
>+ if (ee->ops->is_notify_supported) {
>+ int supported = 0;
>+
>+ for (id = 0; id < r_evt->num_sources; id++) {
>+ if (!ee->ops->is_notify_supported(ph, r_evt->evt->id, id))
>+ refcount_set(&r_evt->sources[id], NOTIF_UNSUPP);
>+ else
>+ supported++;
>+ }
>+
>+ /* Not even one source has been found to be supported */
>+ r_evt->not_supported_by_platform = !supported;
>+ }
>
> pd->registered_events[i] = r_evt;
> /* Ensure events are updated */
>@@ -936,6 +949,11 @@ static inline int scmi_bind_event_handler(struct scmi_notify_instance *ni,
> * of protocol instance.
> */
> hash_del(&hndl->hash);
>+
>+ /* Bailout if event is not supported at all */
>+ if (r_evt->not_supported_by_platform)
>+ return -EOPNOTSUPP;
>+
> /*
> * Acquire protocols only for NON pending handlers, so as NOT to trigger
> * protocol initialization when a notifier is registered against a still
>@@ -1060,6 +1078,9 @@ __scmi_event_handler_get_ops(struct scmi_notify_instance *ni,
> r_evt = SCMI_GET_REVT(ni, KEY_XTRACT_PROTO_ID(evt_key),
> KEY_XTRACT_EVT_ID(evt_key));
>
>+ if (r_evt && r_evt->not_supported_by_platform)
>+ return ERR_PTR(-EOPNOTSUPP);
>+
> mutex_lock(&ni->pending_mtx);
> /* Search registered events at first ... if possible at all */
> if (r_evt) {
>@@ -1087,7 +1108,7 @@ __scmi_event_handler_get_ops(struct scmi_notify_instance *ni,
> hndl->key);
> /* this hndl can be only a pending one */
> scmi_put_handler_unlocked(ni, hndl);
>- hndl = NULL;
>+ hndl = ERR_PTR(-EINVAL);
> }
> }
> mutex_unlock(&ni->pending_mtx);
>@@ -1370,8 +1391,8 @@ static int scmi_notifier_register(const struct scmi_handle *handle,
> evt_key = MAKE_HASH_KEY(proto_id, evt_id,
> src_id ? *src_id : SRC_ID_MASK);
> hndl = scmi_get_or_create_handler(ni, evt_key);
>- if (!hndl)
>- return -EINVAL;
>+ if (IS_ERR(hndl))
>+ return PTR_ERR(hndl);
>
> blocking_notifier_chain_register(&hndl->chain, nb);
>
>@@ -1416,8 +1437,8 @@ static int scmi_notifier_unregister(const struct scmi_handle *handle,
> evt_key = MAKE_HASH_KEY(proto_id, evt_id,
> src_id ? *src_id : SRC_ID_MASK);
> hndl = scmi_get_handler(ni, evt_key);
>- if (!hndl)
>- return -EINVAL;
>+ if (IS_ERR(hndl))
>+ return PTR_ERR(hndl);
>
> /*
> * Note that this chain unregistration call is safe on its own
>--
>2.47.0
>
On Tue, Jun 17, 2025 at 02:50:38PM +0100, Cristian Marussi wrote: > Some platforms could be configured not to support notification events from > specific sources and such a case is already handled properly by avoiding > even to attempt to send a notification enable request since it would be > doomed to fail anyway. > ... btw....not sure even if all of this is worth just to cut down on needless computation...maybe just reviewing the noise level of the emitted error message could be enough. Thanks, Cristian
© 2016 - 2025 Red Hat, Inc.