public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] staging: nvec_power: quiesce EC queries for system suspend
@ 2026-03-12 21:11 Gustavo Arantes
  2026-03-12 21:11 ` [PATCH 1/2] staging: nvec_power: make EC queries synchronous Gustavo Arantes
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Gustavo Arantes @ 2026-03-12 21:11 UTC (permalink / raw)
  To: gregkh, marvin24
  Cc: linux-staging, ac100, linux-tegra, linux-kernel, dev.gustavoa

nvec_power still issues EC queries asynchronously and keeps work queued
across system sleep. This series first serializes the driver's EC
requests, then adds PM hooks so the remaining work items are quiesced
while the parent NVEC controller is suspended.

Patch 1 exports a small core parser helper, moves the battery metadata
queries into a worker, and switches nvec_power over to synchronous EC
requests.

Patch 2 tracks the suspend state per instance, prevents new work from
being queued during suspend, and restarts the polling and metadata work
on resume.

Gustavo Arantes (2):
  staging: nvec_power: make EC queries synchronous
  staging: nvec_power: stop EC queries during system suspend

 drivers/staging/nvec/nvec.c       |   7 +-
 drivers/staging/nvec/nvec.h       |   2 +
 drivers/staging/nvec/nvec_power.c | 122 +++++++++++++++++++++++++-----
 3 files changed, 111 insertions(+), 20 deletions(-)


base-commit: ad6bb64332bb4297110950769ad5af52791e33a2
-- 
2.53.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] staging: nvec_power: make EC queries synchronous
  2026-03-12 21:11 [PATCH 0/2] staging: nvec_power: quiesce EC queries for system suspend Gustavo Arantes
@ 2026-03-12 21:11 ` Gustavo Arantes
  2026-03-12 21:11 ` [PATCH 2/2] staging: nvec_power: stop EC queries during system suspend Gustavo Arantes
  2026-03-15 20:48 ` [PATCH 0/2] staging: nvec_power: quiesce EC queries for " Marc Dietrich
  2 siblings, 0 replies; 5+ messages in thread
From: Gustavo Arantes @ 2026-03-12 21:11 UTC (permalink / raw)
  To: gregkh, marvin24
  Cc: linux-staging, ac100, linux-tegra, linux-kernel, dev.gustavoa

The nvec power driver still submits its periodic AC and battery
queries, as well as the battery metadata requests, asynchronously.
That leaves it with several independent request sources and makes it
hard to quiesce the driver cleanly before system sleep.

Add a small core helper to feed a synchronous reply back through the
normal NVEC message parser, then switch nvec_power over to synchronous
queries. Move the battery metadata requests out of the notifier
callback into a worker so they can use the same synchronous path.

This keeps the existing notifier-based state updates intact while
serializing the driver's EC traffic.

Signed-off-by: Gustavo Arantes <dev.gustavoa@gmail.com>
---
 drivers/staging/nvec/nvec.c       |  7 ++--
 drivers/staging/nvec/nvec.h       |  2 +
 drivers/staging/nvec/nvec_power.c | 70 ++++++++++++++++++++++++-------
 3 files changed, 61 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index 952c5a849a56..cb1d92769941 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -408,14 +408,14 @@ static void nvec_request_master(struct work_struct *work)
 }
 
 /**
- * parse_msg - Print some information and call the notifiers on an RX message
+ * nvec_msg_parse - Print some information and call the notifiers on an RX message
  * @nvec: A &struct nvec_chip
  * @msg: A message received by @nvec
  *
  * Paarse some pieces of the message and then call the chain of notifiers
  * registered via nvec_register_notifier.
  */
-static int parse_msg(struct nvec_chip *nvec, struct nvec_msg *msg)
+int nvec_msg_parse(struct nvec_chip *nvec, struct nvec_msg *msg)
 {
 	if ((msg->data[0] & 1 << 7) == 0 && msg->data[3]) {
 		dev_err(nvec->dev, "ec responded %*ph\n", 4, msg->data);
@@ -432,6 +432,7 @@ static int parse_msg(struct nvec_chip *nvec, struct nvec_msg *msg)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(nvec_msg_parse);
 
 /**
  * nvec_dispatch - Process messages received from the EC
@@ -459,7 +460,7 @@ static void nvec_dispatch(struct work_struct *work)
 			nvec->last_sync_msg = msg;
 			complete(&nvec->sync_write);
 		} else {
-			parse_msg(nvec, msg);
+			nvec_msg_parse(nvec, msg);
 			nvec_msg_free(nvec, msg);
 		}
 		spin_lock_irqsave(&nvec->rx_lock, flags);
diff --git a/drivers/staging/nvec/nvec.h b/drivers/staging/nvec/nvec.h
index 80c0353f141c..d6854c9d629d 100644
--- a/drivers/staging/nvec/nvec.h
+++ b/drivers/staging/nvec/nvec.h
@@ -168,6 +168,8 @@ int nvec_write_sync(struct nvec_chip *nvec,
 		    const unsigned char *data, short size,
 		    struct nvec_msg **msg);
 
+int nvec_msg_parse(struct nvec_chip *nvec, struct nvec_msg *msg);
+
 int nvec_register_notifier(struct nvec_chip *nvec,
 			   struct notifier_block *nb,
 			   unsigned int events);
diff --git a/drivers/staging/nvec/nvec_power.c b/drivers/staging/nvec/nvec_power.c
index 2faab9fdedef..6b3235f41d07 100644
--- a/drivers/staging/nvec/nvec_power.c
+++ b/drivers/staging/nvec/nvec_power.c
@@ -23,6 +23,7 @@
 struct nvec_power {
 	struct notifier_block notifier;
 	struct delayed_work poller;
+	struct work_struct bat_init;
 	struct nvec_chip *nvec;
 	int on;
 	int bat_present;
@@ -106,14 +107,38 @@ static const int bat_init[] = {
 	MANUFACTURER, MODEL, TYPE,
 };
 
-static void get_bat_mfg_data(struct nvec_power *power)
+static int nvec_power_write_sync(struct nvec_power *power,
+				 unsigned char *buf, short size)
 {
+	struct nvec_msg *msg;
+	int ret;
+
+	ret = nvec_write_sync(power->nvec, buf, size, &msg);
+	if (ret < 0)
+		return ret;
+
+	nvec_msg_parse(power->nvec, msg);
+	nvec_msg_free(power->nvec, msg);
+
+	return 0;
+}
+
+static void nvec_power_bat_init(struct work_struct *work)
+{
+	struct nvec_power *power = container_of(work, struct nvec_power,
+						 bat_init);
+	unsigned char buf[] = { NVEC_BAT, 0 };
 	int i;
-	char buf[] = { NVEC_BAT, SLOT_STATUS };
+	int ret;
 
 	for (i = 0; i < ARRAY_SIZE(bat_init); i++) {
 		buf[1] = bat_init[i];
-		nvec_write_async(power->nvec, buf, 2);
+
+		ret = nvec_power_write_sync(power, buf, sizeof(buf));
+		if (ret < 0)
+			dev_warn(power->nvec->dev,
+				 "failed to query battery data %u: %d\n",
+				 bat_init[i], ret);
 	}
 }
 
@@ -133,7 +158,7 @@ static int nvec_power_bat_notifier(struct notifier_block *nb,
 		if (res->plc[0] & 1) {
 			if (power->bat_present == 0) {
 				status_changed = 1;
-				get_bat_mfg_data(power);
+				schedule_work(&power->bat_init);
 			}
 
 			power->bat_present = 1;
@@ -347,15 +372,19 @@ static const int bat_iter[] = {
 
 static void nvec_power_poll(struct work_struct *work)
 {
-	char buf[] = { NVEC_SYS, GET_SYSTEM_STATUS };
+	unsigned char buf[] = { NVEC_SYS, GET_SYSTEM_STATUS };
 	struct nvec_power *power = container_of(work, struct nvec_power,
-						poller.work);
+							poller.work);
+	int ret;
 
 	if (counter >= ARRAY_SIZE(bat_iter))
 		counter = 0;
 
 	/* AC status via sys req */
-	nvec_write_async(power->nvec, buf, 2);
+	ret = nvec_power_write_sync(power, buf, sizeof(buf));
+	if (ret < 0)
+		dev_warn(power->nvec->dev,
+			 "failed to query AC status: %d\n", ret);
 	msleep(100);
 
 	/*
@@ -364,7 +393,10 @@ static void nvec_power_poll(struct work_struct *work)
 	 */
 	buf[0] = NVEC_BAT;
 	buf[1] = bat_iter[counter++];
-	nvec_write_async(power->nvec, buf, 2);
+	ret = nvec_power_write_sync(power, buf, sizeof(buf));
+	if (ret < 0)
+		dev_warn(power->nvec->dev,
+			 "failed to query battery status: %d\n", ret);
 
 	schedule_delayed_work(to_delayed_work(work), msecs_to_jiffies(5000));
 };
@@ -394,13 +426,13 @@ static int nvec_power_probe(struct platform_device *pdev)
 		power->notifier.notifier_call = nvec_power_notifier;
 
 		INIT_DELAYED_WORK(&power->poller, nvec_power_poll);
-		schedule_delayed_work(&power->poller, msecs_to_jiffies(5000));
 		break;
 	case BAT:
 		psy = &nvec_bat_psy;
 		psy_desc = &nvec_bat_psy_desc;
 
 		power->notifier.notifier_call = nvec_power_bat_notifier;
+		INIT_WORK(&power->bat_init, nvec_power_bat_init);
 		break;
 	default:
 		return -ENODEV;
@@ -408,25 +440,33 @@ static int nvec_power_probe(struct platform_device *pdev)
 
 	nvec_register_notifier(nvec, &power->notifier, NVEC_SYS);
 
-	if (pdev->id == BAT)
-		get_bat_mfg_data(power);
-
 	*psy = power_supply_register(&pdev->dev, psy_desc, &psy_cfg);
 
-	return PTR_ERR_OR_ZERO(*psy);
+	if (IS_ERR(*psy)) {
+		nvec_unregister_notifier(nvec, &power->notifier);
+		return PTR_ERR(*psy);
+	}
+
+	if (pdev->id == AC)
+		schedule_delayed_work(&power->poller, msecs_to_jiffies(5000));
+	else
+		schedule_work(&power->bat_init);
+
+	return 0;
 }
 
 static void nvec_power_remove(struct platform_device *pdev)
 {
 	struct nvec_power *power = platform_get_drvdata(pdev);
 
-	cancel_delayed_work_sync(&power->poller);
 	nvec_unregister_notifier(power->nvec, &power->notifier);
 	switch (pdev->id) {
 	case AC:
+		cancel_delayed_work_sync(&power->poller);
 		power_supply_unregister(nvec_psy);
 		break;
 	case BAT:
+		cancel_work_sync(&power->bat_init);
 		power_supply_unregister(nvec_bat_psy);
 	}
 }
@@ -435,7 +475,7 @@ static struct platform_driver nvec_power_driver = {
 	.probe = nvec_power_probe,
 	.remove = nvec_power_remove,
 	.driver = {
-		   .name = "nvec-power",
+		.name = "nvec-power",
 	}
 };
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] staging: nvec_power: stop EC queries during system suspend
  2026-03-12 21:11 [PATCH 0/2] staging: nvec_power: quiesce EC queries for system suspend Gustavo Arantes
  2026-03-12 21:11 ` [PATCH 1/2] staging: nvec_power: make EC queries synchronous Gustavo Arantes
@ 2026-03-12 21:11 ` Gustavo Arantes
  2026-03-15 20:48 ` [PATCH 0/2] staging: nvec_power: quiesce EC queries for " Marc Dietrich
  2 siblings, 0 replies; 5+ messages in thread
From: Gustavo Arantes @ 2026-03-12 21:11 UTC (permalink / raw)
  To: gregkh, marvin24
  Cc: linux-staging, ac100, linux-tegra, linux-kernel, dev.gustavoa

The nvec_power driver continues to schedule its polling and battery
metadata work across system sleep, even though the parent nvec core
suspends the controller.

Track when each nvec_power instance is suspended, stop scheduling new
work once suspend begins, cancel the pending work items, and restart
them on resume. This keeps the driver's EC traffic quiesced while
the parent controller is asleep.

Signed-off-by: Gustavo Arantes <dev.gustavoa@gmail.com>
---
 drivers/staging/nvec/nvec_power.c | 52 +++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/nvec/nvec_power.c b/drivers/staging/nvec/nvec_power.c
index 6b3235f41d07..54e5c5cdaf63 100644
--- a/drivers/staging/nvec/nvec_power.c
+++ b/drivers/staging/nvec/nvec_power.c
@@ -25,6 +25,7 @@ struct nvec_power {
 	struct delayed_work poller;
 	struct work_struct bat_init;
 	struct nvec_chip *nvec;
+	bool suspended;
 	int on;
 	int bat_present;
 	int bat_status;
@@ -158,7 +159,8 @@ static int nvec_power_bat_notifier(struct notifier_block *nb,
 		if (res->plc[0] & 1) {
 			if (power->bat_present == 0) {
 				status_changed = 1;
-				schedule_work(&power->bat_init);
+				if (!READ_ONCE(power->suspended))
+					schedule_work(&power->bat_init);
 			}
 
 			power->bat_present = 1;
@@ -398,7 +400,9 @@ static void nvec_power_poll(struct work_struct *work)
 		dev_warn(power->nvec->dev,
 			 "failed to query battery status: %d\n", ret);
 
-	schedule_delayed_work(to_delayed_work(work), msecs_to_jiffies(5000));
+	if (!READ_ONCE(power->suspended))
+		schedule_delayed_work(to_delayed_work(work),
+				      msecs_to_jiffies(5000));
 };
 
 static int nvec_power_probe(struct platform_device *pdev)
@@ -471,11 +475,55 @@ static void nvec_power_remove(struct platform_device *pdev)
 	}
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int nvec_power_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct nvec_power *power = dev_get_drvdata(dev);
+
+	WRITE_ONCE(power->suspended, true);
+
+	switch (pdev->id) {
+	case AC:
+		cancel_delayed_work_sync(&power->poller);
+		break;
+	case BAT:
+		cancel_work_sync(&power->bat_init);
+		break;
+	}
+
+	return 0;
+}
+
+static int nvec_power_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct nvec_power *power = dev_get_drvdata(dev);
+
+	WRITE_ONCE(power->suspended, false);
+
+	switch (pdev->id) {
+	case AC:
+		schedule_delayed_work(&power->poller, msecs_to_jiffies(5000));
+		break;
+	case BAT:
+		schedule_work(&power->bat_init);
+		break;
+	}
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(nvec_power_pm_ops, nvec_power_suspend,
+			 nvec_power_resume);
+
 static struct platform_driver nvec_power_driver = {
 	.probe = nvec_power_probe,
 	.remove = nvec_power_remove,
 	.driver = {
 		.name = "nvec-power",
+		.pm = &nvec_power_pm_ops,
 	}
 };
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] staging: nvec_power: quiesce EC queries for system suspend
  2026-03-12 21:11 [PATCH 0/2] staging: nvec_power: quiesce EC queries for system suspend Gustavo Arantes
  2026-03-12 21:11 ` [PATCH 1/2] staging: nvec_power: make EC queries synchronous Gustavo Arantes
  2026-03-12 21:11 ` [PATCH 2/2] staging: nvec_power: stop EC queries during system suspend Gustavo Arantes
@ 2026-03-15 20:48 ` Marc Dietrich
  2026-03-15 21:48   ` Gustavo Arantes
  2 siblings, 1 reply; 5+ messages in thread
From: Marc Dietrich @ 2026-03-15 20:48 UTC (permalink / raw)
  To: Gustavo Arantes; +Cc: gregkh, linux-staging, ac100, linux-tegra, linux-kernel

Hello Gustavo,

thanks for your patches.

On Thu, 12 Mar 2026, Gustavo Arantes wrote:
> nvec_power still issues EC queries asynchronously and keeps work queued
> across system sleep. This series first serializes the driver's EC
> requests, then adds PM hooks so the remaining work items are quiesced
> while the parent NVEC controller is suspended.

Reading out the battery info during boot takes some time as far as I 
remember, but I haven't tested your patches yet. Is the use of sync 
writes really required in order to realize a clean suspend?

AFAIK, the machine doesn't support suspend currently, partily due to 
missing atomic writes for the EC. Building the driver as a removable 
module may be enough to test the change though.
Are you able to test the change on real hardware?

Best wishes,

Marc

> Patch 1 exports a small core parser helper, moves the battery metadata
> queries into a worker, and switches nvec_power over to synchronous EC
> requests.
>
> Patch 2 tracks the suspend state per instance, prevents new work from
> being queued during suspend, and restarts the polling and metadata work
> on resume.
>
> Gustavo Arantes (2):
>  staging: nvec_power: make EC queries synchronous
>  staging: nvec_power: stop EC queries during system suspend
>
> drivers/staging/nvec/nvec.c       |   7 +-
> drivers/staging/nvec/nvec.h       |   2 +
> drivers/staging/nvec/nvec_power.c | 122 +++++++++++++++++++++++++-----
> 3 files changed, 111 insertions(+), 20 deletions(-)
>
>
> base-commit: ad6bb64332bb4297110950769ad5af52791e33a2
> -- 
> 2.53.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] staging: nvec_power: quiesce EC queries for system suspend
  2026-03-15 20:48 ` [PATCH 0/2] staging: nvec_power: quiesce EC queries for " Marc Dietrich
@ 2026-03-15 21:48   ` Gustavo Arantes
  0 siblings, 0 replies; 5+ messages in thread
From: Gustavo Arantes @ 2026-03-15 21:48 UTC (permalink / raw)
  To: marvin24
  Cc: gregkh, linux-staging, ac100, linux-tegra, linux-kernel,
	Gustavo Arantes

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

Hello Marc,

thanks for reviewing.

On Sun, 15 Mar 2026, Marc Dietrich wrote:
> Reading out the battery info during boot takes some time as far as I
> remember, but I haven't tested your patches yet. Is the use of sync
> writes really required in order to realize a clean suspend?

You're right, I worked out some math on this, and I agree that the sync
conversion isn't strictly necessary.

The worst case with async writes during suspend is a reply arriving
after work has been cancelled. That reply would just be lost, and since
the poller refreshes everything on resume, there's no corruption or
crash — just one missed update.

The sync approach does avoid that window, but the cost is structural:
the battery metadata init serializes N queries, so the boot-time wall
time goes from roughly one EC round-trip to N * T_rt. With N = 3
(MANUFACTURER, MODEL, TYPE), the relative overhead is always 2x
regardless of how fast the EC is. Without hardware to measure T_rt
I can't tell whether that's 100ms or 2s of added latency, but either
way it's a cost with no real safety benefit.

> Are you able to test the change on real hardware?

Unfortunately I don't have access to Tegra 2 hardware, so this has
only been build-tested and reviewed by inspection. I'd appreciate it
if you or someone on the ac100 list could validate the v2 on a real
device.

If you think the lost-reply window during suspend is benign, I'd
prefer to drop patch 1 and send a v2 with just the PM hooks on top
of the existing async flow. Otherwise, I'm happy to keep the sync
conversion as-is.


Best regards,
Gustavo Arantes

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-03-15 21:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-12 21:11 [PATCH 0/2] staging: nvec_power: quiesce EC queries for system suspend Gustavo Arantes
2026-03-12 21:11 ` [PATCH 1/2] staging: nvec_power: make EC queries synchronous Gustavo Arantes
2026-03-12 21:11 ` [PATCH 2/2] staging: nvec_power: stop EC queries during system suspend Gustavo Arantes
2026-03-15 20:48 ` [PATCH 0/2] staging: nvec_power: quiesce EC queries for " Marc Dietrich
2026-03-15 21:48   ` Gustavo Arantes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox