On 3/12/20 1:51 PM, Lukasz Luba wrote:

Hi Cristian,

just one comment below...

On 3/4/20 4:25 PM, Cristian Marussi wrote:

quoted

Add core SCMI Notifications dispatch and delivery support logic which is
able, at first, to dispatch well-known received events from the RX ISR to
the dedicated deferred worker, and then, from there, to final deliver the
events to the registered users' callbacks.

Dispatch and delivery is just added here, still not enabled.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
V3 --> V4
- dispatcher now handles dequeuing of events in chunks (header+payload):
   handling of these in_flight events let us remove one unneeded memcpy
   on RX interrupt path (scmi_notify)
- deferred dispatcher now access their own per-protocol handlers' table
   reducing locking contention on the RX path
V2 --> V3
- exposing wq in sysfs via WQ_SYSFS
V1 --> V2
- splitted out of V1 patch 04
- moved from IDR maps to real HashTables to store event_handlers
- simplified delivery logic
---
  drivers/firmware/arm_scmi/notify.c | 334 ++++++++++++++++++++++++++++-
  drivers/firmware/arm_scmi/notify.h |   9 +
  2 files changed, 342 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_scmi/notify.c 

b/drivers/firmware/arm_scmi/notify.c

[snip]

quoted

+
+/**
+ * scmi_notify  - Queues a notification for further deferred processing
+ *
+ * This is called in interrupt context to queue a received event for
+ * deferred processing.
+ *
+ * @handle: The handle identifying the platform instance from which the
+ *        dispatched event is generated
+ * @proto_id: Protocol ID
+ * @evt_id: Event ID (msgID)
+ * @buf: Event Message Payload (without the header)
+ * @len: Event Message Payload size
+ * @ts: RX Timestamp in nanoseconds (boottime)
+ *
+ * Return: 0 on Success
+ */
+int scmi_notify(const struct scmi_handle *handle, u8 proto_id, u8 
evt_id,
+        const void *buf, size_t len, u64 ts)
+{
+    struct scmi_registered_event *r_evt;
+    struct scmi_event_header eh;
+    struct scmi_notify_instance *ni = handle->notify_priv;
+
+    /* Ensure atomic value is updated */
+    smp_mb__before_atomic();
+    if (unlikely(!atomic_read(&ni->enabled)))
+        return 0;
+
+    r_evt = SCMI_GET_REVT(ni, proto_id, evt_id);
+    if (unlikely(!r_evt))
+        return -EINVAL;
+
+    if (unlikely(len > r_evt->evt->max_payld_sz)) {
+        pr_err("SCMI Notifications: discard badly sized message\n");
+        return -EINVAL;
+    }
+    if (unlikely(kfifo_avail(&r_evt->proto->equeue.kfifo) <
+             sizeof(eh) + len)) {
+        pr_warn("SCMI Notifications: queue full dropping proto_id:%d  
evt_id:%d  ts:%lld\n",
+            proto_id, evt_id, ts);
+        return -ENOMEM;
+    }
+
+    eh.timestamp = ts;
+    eh.evt_id = evt_id;
+    eh.payld_sz = len;
+    kfifo_in(&r_evt->proto->equeue.kfifo, &eh, sizeof(eh));
+    kfifo_in(&r_evt->proto->equeue.kfifo, buf, len);
+    queue_work(r_evt->proto->equeue.wq,
+           &r_evt->proto->equeue.notify_work);

Is it safe to ignore the return value from the queue_work here?

and also from the kfifo_in

Regards,
Lukasz

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel