transport: Fix event handling

Events were not quite being handled as per the intent of the recent
refactor: The protocol layer was meant to record the raw set of events
and provide the protocol-version-specific mask to the transport layer,
which the transport layer would then use to flush out the state in
accordance with its implementation-specific requirements.

What was going wrong was that the transport implementations were
overwriting the raw set of events with the protocol-specific masked set
of events, meaning that we'd lose the raw state and provide an
incomplete BMC state value on protocol upgrade.

Rework the event handling to make sure the responsibilities are properly
split between the layers.

Change-Id: Iace6615a121e4ce7dcca690d9adf62e5ab9ccee2
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

4 files changed, 20 insertions, 19 deletions

diff --git a/protocol.c b/protocol.c
index 2c97f92..616af3d 100644
--- a/protocol.c
+++ b/protocol.c
@@ -40,7 +40,7 @@ int protocol_events_set(struct mbox_context *context, uint8_t bmc_event)
 	 */
 	context->bmc_events |= bmc_event;
 
-	return context->transport->set_events(context, (bmc_event & mask));
+	return context->transport->set_events(context, bmc_event, mask);
 }
 
 /*
@@ -56,7 +56,7 @@ int protocol_events_clear(struct mbox_context *context, uint8_t bmc_event)
 
 	context->bmc_events &= ~bmc_event;
 
-	return context->transport->clear_events(context, (bmc_event & mask));
+	return context->transport->clear_events(context, bmc_event, mask);
 }
 
 int protocol_v1_reset(struct mbox_context *context)
diff --git a/transport.h b/transport.h
index 8d63b27..0566a85 100644
--- a/transport.h
+++ b/transport.h
@@ -7,8 +7,10 @@
 struct mbox_context;
 
 struct transport_ops {
-	int (*set_events)(struct mbox_context *context, uint8_t events);
-	int (*clear_events)(struct mbox_context *context, uint8_t events);
+	int (*set_events)(struct mbox_context *context, uint8_t events,
+			  uint8_t mask);
+	int (*clear_events)(struct mbox_context *context, uint8_t events,
+			    uint8_t mask);
 };
 
 #endif /* TRANSPORT_H */
diff --git a/transport_dbus.c b/transport_dbus.c
index 3ad0dad..c21abda 100644
--- a/transport_dbus.c
+++ b/transport_dbus.c
@@ -19,6 +19,7 @@ static int transport_dbus_property_update(struct mbox_context *context,
 	/* Two properties plus a terminating NULL */
 	char *props[3] = { 0 };
 	int i = 0;
+	int rc;
 
 	if (events & BMC_EVENT_FLASH_CTRL_LOST) {
 		props[i++] = "FlashControlLost";
@@ -28,19 +29,21 @@ static int transport_dbus_property_update(struct mbox_context *context,
 		props[i++] = "DaemonReady";
 	}
 
-	return sd_bus_emit_properties_changed_strv(context->bus,
+	rc = sd_bus_emit_properties_changed_strv(context->bus,
 						 MBOX_DBUS_OBJECT,
 						 /* FIXME: Hard-coding v2 */
 						 MBOX_DBUS_PROTOCOL_IFACE_V2,
 						 props);
+
+	return (rc < 0) ? rc : 0;
 }
 
 static int transport_dbus_set_events(struct mbox_context *context,
-				     uint8_t events)
+				     uint8_t events, uint8_t mask)
 {
 	int rc;
 
-	rc = transport_dbus_property_update(context, events);
+	rc = transport_dbus_property_update(context, events & mask);
 	if (rc < 0) {
 		return rc;
 	}
@@ -89,10 +92,10 @@ static int transport_dbus_set_events(struct mbox_context *context,
 }
 
 static int transport_dbus_clear_events(struct mbox_context *context,
-				       uint8_t events)
+				       uint8_t events, uint8_t mask)
 {
 	/* No need to emit signals for ackable events on clear */
-	return transport_dbus_property_update(context, events);
+	return transport_dbus_property_update(context, events & mask);
 }
 
 static const struct transport_ops transport_dbus_ops = {
diff --git a/transport_mbox.c b/transport_mbox.c
index 340a526..3c2727a 100644
--- a/transport_mbox.c
+++ b/transport_mbox.c
@@ -90,7 +90,7 @@ static inline int mbox_xlate_errno(struct mbox_context *context,
  *
  * Return:	0 on success otherwise negative error code
  */
-static int transport_mbox_flush_events(struct mbox_context *context)
+static int transport_mbox_flush_events(struct mbox_context *context, uint8_t events)
 {
 	int rc;
 
@@ -103,7 +103,7 @@ static int transport_mbox_flush_events(struct mbox_context *context)
 	}
 
 	/* Write to mbox status register */
-	rc = write(context->fds[MBOX_FD].fd, &context->bmc_events, 1);
+	rc = write(context->fds[MBOX_FD].fd, &events, 1);
 	if (rc != 1) {
 		MSG_ERR("Couldn't write to BMC status reg: %s\n",
 				strerror(errno));
@@ -122,19 +122,15 @@ static int transport_mbox_flush_events(struct mbox_context *context)
 }
 
 static int transport_mbox_set_events(struct mbox_context *context,
-				     uint8_t events)
+				     uint8_t events, uint8_t mask)
 {
-	context->bmc_events |= events;
-
-	return transport_mbox_flush_events(context);
+	return transport_mbox_flush_events(context, context->bmc_events & mask);
 }
 
 static int transport_mbox_clear_events(struct mbox_context *context,
-				       uint8_t events)
+				       uint8_t events, uint8_t mask)
 {
-	context->bmc_events &= ~events;
-
-	return transport_mbox_flush_events(context);
+	return transport_mbox_flush_events(context, context->bmc_events & mask);
 }
 
 static const struct transport_ops transport_mbox_ops = {