mbox series

[v5,00/13] SCMI Notifications Core Support

Message ID 20200316150334.47463-1-cristian.marussi@arm.com (mailing list archive)
Headers show
Series SCMI Notifications Core Support | expand

Message

Cristian Marussi March 16, 2020, 3:03 p.m. UTC
Hi all,

this series wants to introduce SCMI Notification Support, built on top of
the standard Kernel notification chain subsystem.

At initialization time each SCMI Protocol takes care to register with the
new SCMI notification core the set of its own events which it intends to
support.

Using the API exposed via scmi_handle.notify_ops a Kernel user can register
its own notifier_t callback (via a notifier_block as usual) against any
registered event as identified by the tuple:

		(proto_id, event_id, src_id)

where src_id represents a generic source identifier which is protocol
dependent like domain_id, performance_id, sensor_id and so forth.
(users can anyway do NOT provide any src_id, and subscribe instead to ALL
 the existing (if any) src_id sources for that proto_id/evt_id combination)

Each of the above tuple-specified event will be served on its own dedicated
blocking notification chain, dynamically allocated on-demand when at least
one user has shown interest on that event.

Upon a notification delivery all the users' registered notifier_t callbacks
will be in turn invoked and fed with the event_id as @action param and a
generated custom per-event struct _report as @data param.
(as in include/linux/scmi_protocol.h)

The final step of notification delivery via users' callback invocation is
instead delegated to a pool of deferred workers (Kernel cmwq): each
SCMI protocol has its own dedicated worker and dedicated queue to push
events from the rx ISR to the worker.

Based on scmi-next 5.6 [1], on top of:

commit 5c8a47a5a91d ("firmware: arm_scmi: Make scmi core independent of
		      the transport type")

This series has been tested on JUNO with an experimental firmware only
supporting Perf Notifications.

Thanks

Cristian

----
v4 --> v5:
- fixed kernel-doc
- added proper barriers around registered protocols and events initialization
- reviewed queues allocation using devm_add_action_or_reset
- reviewed REVT_NOTIFY_ENABLE macro

v3 --> v4:
- dropped RFC tag
- avoid one unneeded evt payload memcpy on the ISR RC code path by
  redesigning dispatcher to handle partial queue-reads (in_flight events,
  only header)
- fixed the initialization issue exposed by late SCMI modules loading by
  reviewing the init process to support possible late events registrations
  by protocols and early callbacks registrations by users (pending)
- cleanup/simplification of exit path: SCMI protocols are generally never
  de-initialized after the initial device creation, so do not deinit
  notification core either (we do halt the delivery, stop the wq and empty
  the queues though)
- reduced contention on regustered_events_handler to the minimum during
  delivery by splitting the common registered_events_handlers hashtable
  into a number of per-protocol tables
- converted registered_protocols and registered_events hastable to
  fixed size arrays: simpler and lockless in our usage scenario

v2 --> v3:
- added platform instance awareness to the notification core: a
  notification instance is created for each known handle
- reviewed notification core initialization and shutdown process
- removed generic non-handle-rooted registration API
- added WQ_SYSFS flag to workqueue instance

v1 --> v2:
- dropped anti-tampering patch
- rebased on top of scmi-for-next-5.6, which includes Viresh series that
  make SCMI core independent of transport (5c8a47a5a91d)
- add a few new SCMI transport methods on top of Viresh patch to address
  needs of SCMI Notifications
- reviewed/renamed scmi_handle_xfer_delayed_resp()
- split main SCMI Notification core patch (~1k lines) into three chunks:
  protocol-registration / callbacks-registration / dispatch-and-delivery
- removed awkward usage of IDR maps in favour of pure hashtables
- added enable/disable refcounting in notification core (was broken in v1)
- removed per-protocol candidate API: a single generic API is now proposed
  instead of scmi_register_<proto>_event_notifier(evt_id, *src_id, *nb)
- added handle->notify_ops as an alternative notification API
  for scmi_driver
- moved ALL_SRCIDs enabled handling from protocol code to core code
- reviewed protocol registration/unregistration logic to use devres
- reviewed cleanup phase on shutdown
- fixed  ERROR: reference preceded by free as reported by kbuild test robot

[1] git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git


Cristian Marussi (10):
  firmware: arm_scmi: Add notifications support in transport layer
  firmware: arm_scmi: Add notification protocol-registration
  firmware: arm_scmi: Add notification callbacks-registration
  firmware: arm_scmi: Add notification dispatch and delivery
  firmware: arm_scmi: Enable notification core
  firmware: arm_scmi: Add Power notifications support
  firmware: arm_scmi: Add Perf notifications support
  firmware: arm_scmi: Add Sensor notifications support
  firmware: arm_scmi: Add Reset notifications support
  firmware: arm_scmi: Add Base notifications support

Sudeep Holla (3):
  firmware: arm_scmi: Add receive buffer support for notifications
  firmware: arm_scmi: Update protocol commands and notification list
  firmware: arm_scmi: Add support for notifications message processing

 drivers/firmware/arm_scmi/Makefile  |    2 +-
 drivers/firmware/arm_scmi/base.c    |  116 +++
 drivers/firmware/arm_scmi/common.h  |   12 +
 drivers/firmware/arm_scmi/driver.c  |  118 ++-
 drivers/firmware/arm_scmi/mailbox.c |   17 +
 drivers/firmware/arm_scmi/notify.c  | 1460 +++++++++++++++++++++++++++
 drivers/firmware/arm_scmi/notify.h  |   77 ++
 drivers/firmware/arm_scmi/perf.c    |  135 +++
 drivers/firmware/arm_scmi/power.c   |  129 +++
 drivers/firmware/arm_scmi/reset.c   |   96 ++
 drivers/firmware/arm_scmi/sensors.c |   73 ++
 drivers/firmware/arm_scmi/shmem.c   |   15 +
 include/linux/scmi_protocol.h       |  107 ++
 13 files changed, 2330 insertions(+), 27 deletions(-)
 create mode 100644 drivers/firmware/arm_scmi/notify.c
 create mode 100644 drivers/firmware/arm_scmi/notify.h

Comments

Lukasz Luba March 18, 2020, 9:01 a.m. UTC | #1
Hi Cristian,

On 3/16/20 3:03 PM, Cristian Marussi wrote:
> Hi all,
> 
> this series wants to introduce SCMI Notification Support, built on top of
> the standard Kernel notification chain subsystem.
> 
> At initialization time each SCMI Protocol takes care to register with the
> new SCMI notification core the set of its own events which it intends to
> support.
> 
> Using the API exposed via scmi_handle.notify_ops a Kernel user can register
> its own notifier_t callback (via a notifier_block as usual) against any
> registered event as identified by the tuple:
> 
> 		(proto_id, event_id, src_id)
> 
> where src_id represents a generic source identifier which is protocol
> dependent like domain_id, performance_id, sensor_id and so forth.
> (users can anyway do NOT provide any src_id, and subscribe instead to ALL
>   the existing (if any) src_id sources for that proto_id/evt_id combination)
> 
> Each of the above tuple-specified event will be served on its own dedicated
> blocking notification chain, dynamically allocated on-demand when at least
> one user has shown interest on that event.
> 
> Upon a notification delivery all the users' registered notifier_t callbacks
> will be in turn invoked and fed with the event_id as @action param and a
> generated custom per-event struct _report as @data param.
> (as in include/linux/scmi_protocol.h)
> 
> The final step of notification delivery via users' callback invocation is
> instead delegated to a pool of deferred workers (Kernel cmwq): each
> SCMI protocol has its own dedicated worker and dedicated queue to push
> events from the rx ISR to the worker.
> 

Could you give an example how the notification would be delivered
further to the upper layers, like hwmon driver, cpufreq or thermal?
For example, for sensor protocol which delivers event
SENSOR_TRIP_POINT_EVENT indicating a trip point was crossed.

Would it be possible for:
drivers/hwmon/scmi-hwmon.c
to get this temperature events like an interrupt?

I couldn't find it in the implementation of the registered handlers.

Regards,
Lukasz
Cristian Marussi March 23, 2020, 8:54 a.m. UTC | #2
Hi Lukasz

On 3/18/20 9:01 AM, Lukasz Luba wrote:
> Hi Cristian,
> 
> On 3/16/20 3:03 PM, Cristian Marussi wrote:
>> Hi all,
>>
>> this series wants to introduce SCMI Notification Support, built on top of
>> the standard Kernel notification chain subsystem.
>>
>> At initialization time each SCMI Protocol takes care to register with the
>> new SCMI notification core the set of its own events which it intends to
>> support.
>>
>> Using the API exposed via scmi_handle.notify_ops a Kernel user can register
>> its own notifier_t callback (via a notifier_block as usual) against any
>> registered event as identified by the tuple:
>>
>>         (proto_id, event_id, src_id)
>>
>> where src_id represents a generic source identifier which is protocol
>> dependent like domain_id, performance_id, sensor_id and so forth.
>> (users can anyway do NOT provide any src_id, and subscribe instead to ALL
>>   the existing (if any) src_id sources for that proto_id/evt_id combination)
>>
>> Each of the above tuple-specified event will be served on its own dedicated
>> blocking notification chain, dynamically allocated on-demand when at least
>> one user has shown interest on that event.
>>
>> Upon a notification delivery all the users' registered notifier_t callbacks
>> will be in turn invoked and fed with the event_id as @action param and a
>> generated custom per-event struct _report as @data param.
>> (as in include/linux/scmi_protocol.h)
>>
>> The final step of notification delivery via users' callback invocation is
>> instead delegated to a pool of deferred workers (Kernel cmwq): each
>> SCMI protocol has its own dedicated worker and dedicated queue to push
>> events from the rx ISR to the worker.
>>
> 
> Could you give an example how the notification would be delivered
> further to the upper layers, like hwmon driver, cpufreq or thermal?

Sure. I tested registering various callbacks against PERF events (since they're
what I have available on my platform implementation); I used probe() in scmi-genpd
and scmi-cpufreq to register generic testing callbacks like:

scmi_cpufrq_probe()

...
	src_id = 0x00;
	handle->notify_ops->register_event_notifier(handle, SCMI_PROTOCOL_PERF,
						    0x1, &src_id, &my_cpufreq_nb);

and removing similarly in remove().

I'll send you my debug patches that includes genpd/cpufreq callbacks registration and
an additional dummy driver that just registers perf callbacks so you can have a better idea
of the intended usage.

> For example, for sensor protocol which delivers event
> SENSOR_TRIP_POINT_EVENT indicating a trip point was crossed.
>

Regarding sensors it could be something like:


	sensor_id = 0x00;

	handle->notify_ops->register_event_notifier(handle, SCMI_PROTOCOL_SENSOR,
						    0x0, &sensor_id, &my_sensor_nb);

	handle->sensor_ops->trip_point_config(handle, sensor_id, 0, 10000);
  
Note that the notification core takes care on its own of enabling the specific events generation
when you register the first callback for a specific event as identified by (proto_id, event_id, src_id)
but in the case of the sensor protocol you'll need to explicitly setup the trip_point too.

As a result when the trip point is crossed you'll receive in your callback a sensor_report
(as in include/linux/scmi_protocol.h) as your data arg.

I just realized that being the notification enable method (trip_notify) also exposed by the ops directly,
I should probably add some sort of alert for a user...since it is not meant to be used directly when
the notification subsystem is being used....I'll think about this pitfall.

> Would it be possible for:
> drivers/hwmon/scmi-hwmon.c
> to get this temperature events like an interrupt?
> 

Sorry I did not get what you mean here.

Regards

Cristian

> I couldn't find it in the implementation of the registered handlers.
> 
> Regards,
> Lukasz