staging: greybus: bootrom: power off module on timeout

From: Aditya Chari S <[email protected]>

Previously, when a firmware size request, get firmware request, or ready-to-boot request from a module timed out, the driver would log an error and free any firmware buffer, but left the module powered on in a potentially inconsistent state.

Fix this by ejecting the module's interface via the SVC when a timeout occurs, forcing it to be power-cycled and re-detected rather than left in limbo.

---
 drivers/staging/greybus/bootrom.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

index 83921d90c..b564d2255 100644
@@ -54,8 +54,12 @@ static void gb_bootrom_timedout(struct work_struct *work)
 	struct delayed_work *dwork = to_delayed_work(work);
 	struct gb_bootrom *bootrom = container_of(dwork,
 						  struct gb_bootrom, dwork);
-	struct device *dev = &bootrom->connection->bundle->dev;
+	struct gb_connection *connection = bootrom->connection;
+	struct device *dev = &connection->bundle->dev;
+	struct gb_interface *intf = connection->bundle->intf;
+	struct gb_svc *svc = connection->hd->svc;
 	const char *reason;
+	int ret;
 
 	switch (bootrom->next_request) {
 	case NEXT_REQ_FIRMWARE_SIZE:

@@ -82,7 +86,11 @@ static void gb_bootrom_timedout(struct work_struct *work)
 	free_firmware(bootrom);
 	mutex_unlock(&bootrom->mutex);
 
-	/* TODO: Power-off Module ? */
+	/* Power-off Module */
+	ret = gb_svc_intf_eject(svc, intf->interface_id);
+	if (ret)
+		dev_err(dev, "failed to eject interface %u (%d)\n",
+			intf->interface_id, ret);
 }
 
 static void gb_bootrom_set_timeout(struct gb_bootrom *bootrom,

2.53.0

Cool, but how was this tested? What hardware needs this?

thanks,

greg k-h

Fair question. Honestly - I don't have Project Ara hardware, and as far as I can tell it's not really possible to get any at this point (the project's been dead for years and I haven't found boards floating around anywhere).

So this was build-tested only - compiles clean, checkpatch clean - but I haven't been able to actually trigger the timeout path on real hardware. I based the fix on the existing gb_svc_intf_eject() usage elsewhere in the SVC code, so the API usage should be correct, but I can't vouch for the runtime behavior beyond that.

That said, if this driver is effectively unmaintained at this point and changes without hardware validation aren't worth taking, I'd appreciate knowing that as well - happy to defer to your judgment on whether this is worth pursuing further.

Thanks,
Aditya