Skip to contentSkip to conversation

hwmon: add Altera SoC FPGA hardware monitoring support

PATCH In review [email protected] 4 participants 12 messages Jul 23 – Jul 29

Subsystems (4)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

v8 cover hwmon: add Altera SoC FPGA hardware monitoring support

From: Tze Yee Ng <[email protected]>

This series adds hardware monitor support for Altera SoC FPGA devices. Temperature and voltage sensors are accessed through the Stratix 10 service layer and Secure Device Manager (SDM).

Patch 1 adds async HWMON SMC support to stratix10-svc and registers the socfpga-hwmon platform device.

Patch 2 adds the socfpga-hwmon driver, documentation, Kconfig, and MAINTAINERS entry.

Changes in v8:
- In Patch 2, address maintainer and Sashiko review feedback on socfpga-hwmon driver:
- Drop the extra wait_for_completion() after sync-mode timeout; on timeout report -ETIMEDOUT and call stratix10_svc_done(). The previous wait could hang forever if receive_cb is never invoked (e.g. BUSY/REJECTED for HWMON). Any leftover svc_fifo race will be handled in a separate stratix10-svc series.
- Place socfpga-hwmon alphabetically in Documentation/hwmon/index.rst
- No functional changes in Patch 1

Changes in v7:
- In Patch 2, address maintainer and Sashiko review feedback on socfpga-hwmon:
- Break out of the async poll loop on hard errors and return the real poll status instead of rewriting it to -ETIMEDOUT
- Drop redundant async poll alert/debug messages (SVC already logs)
- Replace msleep(1) with usleep_range() for short retries
- On sync-mode timeout, wait for the outstanding SVC callback before calling stratix10_svc_done() so a leftover svc_fifo request cannot desynchronize later sensor reads; still report -ETIMEDOUT
- No functional changes in Patch 1

Changes in v6:
- Rebase on torvalds/master (given “Linux 7.2-rc4”)
- No functional changes in Patch 1 and Patch 2

Changes in v5:
- Rebase on dinguyen/socfpga_svc_fixes_for_v7.2
- Address Sashiko review feedback on socfpga-hwmon (Patch 2):
- Poll async responses until HWMON_TIMEOUT instead of a fixed 3-iteration retry loop (~3 ms), fixing premature timeouts on silicon
- Add MODULE_ALIAS("platform:socfpga-hwmon")
- No functional changes in Patch 1

Changes in v4:
- Address maintainer and review feedback on socfpga-hwmon (Patch 2):
- Register devm_add_action_or_reset() before devm_hwmon_device_register_with_info() to fix devres teardown order
- Remove unreferenced completion and pre-poll wait_for_completion_io_timeout() from async reads; poll directly with a retry loop after async_send()
- No functional changes in Patch 1

Changes in v3:
- Address review feedback on socfpga-hwmon (Patch 2):
- Fix 16-bit Q8.8 temperature sign extension
- Drop unused async callback; pass NULL to stratix10_svc_async_send()
- Document and retain pre-poll wait (RSU pattern; firmware needs time before async_poll())
- Align async poll retry behaviour with RSU
- Use uninterruptible wait_for_completion_timeout() for sync reads
- Handle -EINVAL and -EOPNOTSUPP when falling back to sync mode
- Defer SVC channel cleanup via devm_add_action_or_reset()
- No functional changes in Patch 1

Changes in v2:
- Drop altr,stratix10-hwmon DT binding and intel,stratix10-svc hwmon child property
- Drop Stratix 10 SoCDK DTS hwmon node
- Register socfpga-hwmon from stratix10-svc (RSU-style)
- Replace DT channel parsing with hardcoded Stratix 10 and Agilex tables
- Rename driver/module to socfpga-hwmon (CONFIG_SENSORS_ALTERA_SOCFPGA_HWMON)
- Add Agilex channel support
- Fix SDM value conversion (Q8.8 degrees Celsius and Q16 volts to hwmon millidegrees/millivolts)
- Improve sync-mode error handling via last_err

Previous version:
https://lore.kernel.org/all/[email protected]/

Tze Yee Ng (2):
firmware: stratix10-svc: add async HWMON read commands and register
socfpga-hwmon device
hwmon: add Altera SoC FPGA hardware monitoring driver

 Documentation/hwmon/index.rst                |   1 +
 Documentation/hwmon/socfpga-hwmon.rst        |  34 ++
 MAINTAINERS                                  |   8 +
 drivers/firmware/stratix10-svc.c             |  46 +-
 drivers/hwmon/Kconfig                        |  10 +
 drivers/hwmon/Makefile                       |   1 +
 drivers/hwmon/socfpga-hwmon.c                | 579 +++++++++++++++++++
 include/linux/firmware/intel/stratix10-smc.h |  38 ++
 8 files changed, 714 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/hwmon/socfpga-hwmon.rst
 create mode 100644 drivers/hwmon/socfpga-hwmon.c
Signature · 1 line
2.43.7
author maintainer
#1 lore raw

PATCH v8 1/2 firmware: stratix10-svc: add async HWMON read commands and register socfpga-hwmon device

From: Tze Yee Ng <[email protected]>

Add asynchronous Stratix 10 service layer support for hardware monitor temperature and voltage read commands in stratix10_svc_async_send() and stratix10_svc_async_prepare_response().

Register a socfpga-hwmon platform device from the service layer driver when hardware monitor support is enabled, similar to the RSU device.

---
Changes in v8:
- No functional changes from v7

Changes in v7:
- No functional changes from v6

Changes in v6:
- No functional changes from v5

Changes in v5:
- No functional changes from v4

Changes in v3:
- No functional changes from v2

Changes in v2:
- Extend patch scope beyond async SMC support: register socfpga-hwmon platform device from stratix10-svc when CONFIG_SENSORS_ALTERA_SOCFPGA_HWMON is enabled
- Follow RSU-style registration; RSU probe error handling is unchanged
- Add err_unregister_clients to unregister hwmon and RSU on populate failure
- Unregister hwmon platform device in stratix10-svc remove()

---
 drivers/firmware/stratix10-svc.c             | 46 ++++++++++++++++++--
 include/linux/firmware/intel/stratix10-smc.h | 38 ++++++++++++++++
 2 files changed, 81 insertions(+), 3 deletions(-)
drivers/firmware/stratix10-svc.c +43 -3
index c24ca5823078..fc38afed5b7f 100644
@@ -45,6 +45,7 @@
 
 /* stratix10 service layer clients */
 #define STRATIX10_RSU				"stratix10-rsu"
+#define SOCFPGA_HWMON				"socfpga-hwmon"
 
 /* Maximum number of SDM client IDs. */
 #define MAX_SDM_CLIENT_IDS			16
@@ -104,9 +105,11 @@ struct stratix10_svc_chan;
 /**
  * struct stratix10_svc - svc private data
  * @stratix10_svc_rsu: pointer to stratix10 RSU device
+ * @stratix10_svc_hwmon: pointer to stratix10 HWMON device
  */
 struct stratix10_svc {
 	struct platform_device *stratix10_svc_rsu;
+	struct platform_device *stratix10_svc_hwmon;
 };
 
 /**
@@ -1329,6 +1332,14 @@ int stratix10_svc_async_send(struct stratix10_svc_chan *chan, void *msg,
 		args.a0 = INTEL_SIP_SMC_ASYNC_RSU_NOTIFY;
 		args.a2 = p_msg->arg[0];
 		break;
+	case COMMAND_HWMON_READTEMP:
+		args.a0 = INTEL_SIP_SMC_ASYNC_HWMON_READTEMP;
+		args.a2 = p_msg->arg[0];
+		break;
+	case COMMAND_HWMON_READVOLT:
+		args.a0 = INTEL_SIP_SMC_ASYNC_HWMON_READVOLT;
+		args.a2 = p_msg->arg[0];
+		break;
 	default:
 		dev_err(ctrl->dev, "Invalid command ,%d\n", p_msg->command);
 		ret = -EINVAL;
@@ -1422,6 +1433,10 @@ static int stratix10_svc_async_prepare_response(struct stratix10_svc_chan *chan,
 		 */
 		data->kaddr1 = (void *)&handle->res;
 		break;
+	case COMMAND_HWMON_READTEMP:
+	case COMMAND_HWMON_READVOLT:
+		data->kaddr1 = (void *)&handle->res.a2;
+		break;
 
 	default:
 		dev_alert(ctrl->dev, "Invalid command\n ,%d", p_msg->command);
@@ -2013,16 +2028,38 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_put_device;
 
+	if (IS_ENABLED(CONFIG_SENSORS_ALTERA_SOCFPGA_HWMON)) {
+		svc->stratix10_svc_hwmon =
+			platform_device_alloc(SOCFPGA_HWMON, 0);
+		if (!svc->stratix10_svc_hwmon) {
+			dev_err(dev, "failed to allocate %s device\n",
+				SOCFPGA_HWMON);
+		} else {
+			svc->stratix10_svc_hwmon->dev.parent = dev;
+
+			ret = platform_device_add(svc->stratix10_svc_hwmon);
+			if (ret) {
+				dev_err(dev, "failed to add %s device: %d\n",
+					SOCFPGA_HWMON, ret);
+				platform_device_put(svc->stratix10_svc_hwmon);
+				svc->stratix10_svc_hwmon = NULL;
+			}
+		}
+	}
+
 	ret = of_platform_default_populate(dev_of_node(dev), NULL, dev);
 	if (ret)
-		goto err_unregister_rsu_dev;
+		goto err_unregister_clients;
 
 	pr_info("Intel Service Layer Driver Initialized\n");
 
 	return 0;
 
-err_unregister_rsu_dev:
-	platform_device_unregister(svc->stratix10_svc_rsu);
+err_unregister_clients:
+	if (svc->stratix10_svc_hwmon)
+		platform_device_unregister(svc->stratix10_svc_hwmon);
+	if (svc->stratix10_svc_rsu)
+		platform_device_unregister(svc->stratix10_svc_rsu);
 	goto err_free_fifos;
 err_put_device:
 	platform_device_put(svc->stratix10_svc_rsu);
@@ -2046,6 +2083,9 @@ static void stratix10_svc_drv_remove(struct platform_device *pdev)
 	struct stratix10_svc_controller *ctrl = platform_get_drvdata(pdev);
 	struct stratix10_svc *svc = ctrl->svc;
 
+	if (svc->stratix10_svc_hwmon)
+		platform_device_unregister(svc->stratix10_svc_hwmon);
+
 	stratix10_svc_async_exit(ctrl);
 
 	of_platform_depopulate(ctrl->dev);
include/linux/firmware/intel/stratix10-smc.h +38 -0
index 9116512169dc..18ac6fe96d9d 100644
@@ -695,6 +695,44 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
 #define INTEL_SIP_SMC_ASYNC_POLL \
 	INTEL_SIP_SMC_ASYNC_VAL(INTEL_SIP_SMC_ASYNC_FUNC_ID_POLL)
 
+/**
+ * Request INTEL_SIP_SMC_ASYNC_HWMON_READTEMP
+ * Async call to request temperature
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_ASYNC_HWMON_READTEMP
+ * a1 transaction job id
+ * a2 Temperature Channel
+ * a3-a17 not used
+ *
+ * Return status
+ * a0 INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_STATUS_REJECTED
+ * or INTEL_SIP_SMC_STATUS_BUSY
+ * a1-a17 not used
+ */
+#define INTEL_SIP_SMC_ASYNC_FUNC_ID_HWMON_READTEMP	0xE8
+#define INTEL_SIP_SMC_ASYNC_HWMON_READTEMP \
+	INTEL_SIP_SMC_ASYNC_VAL(INTEL_SIP_SMC_ASYNC_FUNC_ID_HWMON_READTEMP)
+
+/**
+ * Request INTEL_SIP_SMC_ASYNC_HWMON_READVOLT
+ * Async call to request voltage
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_ASYNC_HWMON_READVOLT
+ * a1 transaction job id
+ * a2 Voltage Channel
+ * a3-a17 not used
+ *
+ * Return status
+ * a0 INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_STATUS_REJECTED
+ * or INTEL_SIP_SMC_STATUS_BUSY
+ * a1-a17 not used
+ */
+#define INTEL_SIP_SMC_ASYNC_FUNC_ID_HWMON_READVOLT	0xE9
+#define INTEL_SIP_SMC_ASYNC_HWMON_READVOLT \
+	INTEL_SIP_SMC_ASYNC_VAL(INTEL_SIP_SMC_ASYNC_FUNC_ID_HWMON_READVOLT)
+
 /**
  * Request INTEL_SIP_SMC_ASYNC_RSU_GET_SPT
  * Async call to get RSU SPT from SDM.
Signature · 1 line
2.43.7
#2 lore raw

Re: [PATCH v8 1/2] firmware: stratix10-svc: add async HWMON read commands and register socfpga-hwmon device

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Hardware monitoring functionality is improperly registered as a platform device instead of an auxiliary device.

Pre-existing issues:
- [High] Pre-existing Use-After-Free race condition during module removal due to improper teardown ordering of child devices. --

--- Patch [1]: [PATCH v8 1/2] firmware: stratix10-svc: add async HWMON read commands and register socfpga-hwmon device --- commit 5ba2e4fb155f074de568280aeede81883f87924e
Author: Tze Yee Ng <[email protected]>

firmware: stratix10-svc: add async HWMON read commands and register socfpga-hwmon device

Add asynchronous Stratix 10 service layer support for hardware monitor temperature and voltage read commands. Register a socfpga-hwmon platform device from the service layer driver when hardware monitor support is enabled.

Quoted 4 lines
drivers/firmware/stratix10-svc.c +0 -0
index c24ca5823078f..fc38afed5b7fe 100644

[ ... ]

drivers/firmware/stratix10-svc.c static int stratix10_svc_drv_probe(struct platform_device *pdev) · line 2033

Quoted 7 lines
drivers/firmware/stratix10-svc.c +3 -0
@@ -2013,16 +2028,38 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_put_device;
 
+	if (IS_ENABLED(CONFIG_SENSORS_ALTERA_SOCFPGA_HWMON)) {
+		svc->stratix10_svc_hwmon =
+			platform_device_alloc(SOCFPGA_HWMON, 0);

[Severity: Medium]
According to the hardware monitoring subsystem guidelines, if the main functionality of a chip is not hardware monitoring, its secondary hardware monitoring functionality should be implemented as an auxiliary device driver.

Since the primary purpose of this module is the firmware service layer, does registering the hardware monitoring functionality as a platform device violate the subsystem API layering? Could this be implemented via the auxiliary bus instead of using platform_device_alloc()?

Quoted 17 lines
drivers/firmware/stratix10-svc.c +16 -0
+		if (!svc->stratix10_svc_hwmon) {
+			dev_err(dev, "failed to allocate %s device\n",
+				SOCFPGA_HWMON);
+		} else {
+			svc->stratix10_svc_hwmon->dev.parent = dev;
+
+			ret = platform_device_add(svc->stratix10_svc_hwmon);
+			if (ret) {
+				dev_err(dev, "failed to add %s device: %d\n",
+					SOCFPGA_HWMON, ret);
+				platform_device_put(svc->stratix10_svc_hwmon);
+				svc->stratix10_svc_hwmon = NULL;
+			}
+		}
+	}
+
 	ret = of_platform_default_populate(dev_of_node(dev), NULL, dev);

[ ... ]

drivers/firmware/stratix10-svc.c static void stratix10_svc_drv_remove(struct platform_device *pdev) · line 2091

Quoted 10 lines
drivers/firmware/stratix10-svc.c +3 -0
@@ -2046,6 +2083,9 @@ static void stratix10_svc_drv_remove(struct platform_device *pdev)
 	struct stratix10_svc_controller *ctrl = platform_get_drvdata(pdev);
 	struct stratix10_svc *svc = ctrl->svc;
 
+	if (svc->stratix10_svc_hwmon)
+		platform_device_unregister(svc->stratix10_svc_hwmon);
+
 	stratix10_svc_async_exit(ctrl);
 
 	of_platform_depopulate(ctrl->dev);

[Severity: High]
This is a pre-existing issue, but does this teardown order create a use-after-free race condition during module removal?

The stratix10_svc_drv_remove() function calls stratix10_svc_async_exit() before unregistering the remaining OF child devices and the RSU child device. The async exit function destroys the async structures and frees the asynchronous channels and handlers.

If userspace concurrently accesses sysfs attributes for the child devices while the module unloads, could it invoke stratix10_svc_async_send() or stratix10_svc_async_poll() after the async channels have been freed?

For example, stratix10_svc_async_send() performs lockless reads of achan:

stratix10_svc_async_send() {
	...
	achan = chan->async_chan;
	ctrl = chan->ctrl;
	actrl = &ctrl->actrl;

	if (!actrl->initialized) {
	...
}

Would this lead to memory corruption if the structures are already deallocated? Should the child devices be fully unregistered before calling stratix10_svc_async_exit() to ensure no concurrent accesses are possible?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
author maintainer
#3 lore raw

Re: [PATCH v8 1/2] firmware: stratix10-svc: add async HWMON read commands and register socfpga-hwmon device

Quoted 5 lines · On 23/7/2026 2:14 pm, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Hardware monitoring functionality is improperly registered as a platform device instead of an auxiliary device.

I'll keep the platform device registration for this series. I’m happy to look at converting socfpga-hwmon to an auxiliary device in a follow-up if preferred.

Quoted 2 lines

Pre-existing issues:
- [High] Pre-existing Use-After-Free race condition during module removal due to improper teardown ordering of child devices.

It is fixed in
https://lore.kernel.org/all/6630a1568f162d9455e0580f7ccadc262db4718e.1784007275.git.adrian.ho.yin.ng@altera.com/

Quoted 97 lines

--

--- Patch [1]: [PATCH v8 1/2] firmware: stratix10-svc: add async HWMON read commands and register socfpga-hwmon device --- commit 5ba2e4fb155f074de568280aeede81883f87924e
Author: Tze Yee Ng <[email protected]>

firmware: stratix10-svc: add async HWMON read commands and register socfpga-hwmon device

Add asynchronous Stratix 10 service layer support for hardware monitor temperature and voltage read commands. Register a socfpga-hwmon platform device from the service layer driver when hardware monitor support is enabled.

Quoted 4 lines
drivers/firmware/stratix10-svc.c +0 -0
index c24ca5823078f..fc38afed5b7fe 100644

[ ... ]

drivers/firmware/stratix10-svc.c static int stratix10_svc_drv_probe(struct platform_device *pdev) · line 2033

Quoted 7 lines
drivers/firmware/stratix10-svc.c +3 -0
@@ -2013,16 +2028,38 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
  	if (ret)
  		goto err_put_device;
 
+	if (IS_ENABLED(CONFIG_SENSORS_ALTERA_SOCFPGA_HWMON)) {
+		svc->stratix10_svc_hwmon =
+			platform_device_alloc(SOCFPGA_HWMON, 0);

[Severity: Medium]
According to the hardware monitoring subsystem guidelines, if the main functionality of a chip is not hardware monitoring, its secondary hardware monitoring functionality should be implemented as an auxiliary device driver.

Since the primary purpose of this module is the firmware service layer, does registering the hardware monitoring functionality as a platform device violate the subsystem API layering? Could this be implemented via the auxiliary bus instead of using platform_device_alloc()?

Quoted 17 lines
drivers/firmware/stratix10-svc.c +16 -0
+		if (!svc->stratix10_svc_hwmon) {
+			dev_err(dev, "failed to allocate %s device\n",
+				SOCFPGA_HWMON);
+		} else {
+			svc->stratix10_svc_hwmon->dev.parent = dev;
+
+			ret = platform_device_add(svc->stratix10_svc_hwmon);
+			if (ret) {
+				dev_err(dev, "failed to add %s device: %d\n",
+					SOCFPGA_HWMON, ret);
+				platform_device_put(svc->stratix10_svc_hwmon);
+				svc->stratix10_svc_hwmon = NULL;
+			}
+		}
+	}
+
  	ret = of_platform_default_populate(dev_of_node(dev), NULL, dev);

[ ... ]

drivers/firmware/stratix10-svc.c static void stratix10_svc_drv_remove(struct platform_device *pdev) · line 2091

Quoted 10 lines
drivers/firmware/stratix10-svc.c +3 -0
@@ -2046,6 +2083,9 @@ static void stratix10_svc_drv_remove(struct platform_device *pdev)
  	struct stratix10_svc_controller *ctrl = platform_get_drvdata(pdev);
  	struct stratix10_svc *svc = ctrl->svc;
 
+	if (svc->stratix10_svc_hwmon)
+		platform_device_unregister(svc->stratix10_svc_hwmon);
+
  	stratix10_svc_async_exit(ctrl);
 
  	of_platform_depopulate(ctrl->dev);

[Severity: High]
This is a pre-existing issue, but does this teardown order create a use-after-free race condition during module removal?

The stratix10_svc_drv_remove() function calls stratix10_svc_async_exit() before unregistering the remaining OF child devices and the RSU child device. The async exit function destroys the async structures and frees the asynchronous channels and handlers.

If userspace concurrently accesses sysfs attributes for the child devices while the module unloads, could it invoke stratix10_svc_async_send() or stratix10_svc_async_poll() after the async channels have been freed?

For example, stratix10_svc_async_send() performs lockless reads of achan:

stratix10_svc_async_send() {
	...
	achan = chan->async_chan;
	ctrl = chan->ctrl;
	actrl = &ctrl->actrl;

	if (!actrl->initialized) {
	...
}

Would this lead to memory corruption if the structures are already deallocated? Should the child devices be fully unregistered before calling stratix10_svc_async_exit() to ensure no concurrent accesses are possible?

maintainer
#4 lore raw

Re: [PATCH v8 1/2] firmware: stratix10-svc: add async HWMON read commands and register socfpga-hwmon device

Quoted 11 lines · TZE YEE
Quoted 5 lines · On 23/7/2026 2:14 pm, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Hardware monitoring functionality is improperly registered as a platform device instead of an auxiliary device.

I'll keep the platform device registration for this series. I’m happy to look at converting socfpga-hwmon to an auxiliary device in a follow-up if preferred.

Agreed. Just ignore this report for this series.

Thanks,
Guenter

maintainer
#5 lore raw

Re: [PATCH v8 1/2] firmware: stratix10-svc: add async HWMON read commands and register socfpga-hwmon device

In reply to [email protected]

Hi Guenter,

Quoted 6 lines · On 7/23/26 01:02, [email protected] wrote:

From: Tze Yee Ng <[email protected]>

Add asynchronous Stratix 10 service layer support for hardware monitor temperature and voltage read commands in stratix10_svc_async_send() and stratix10_svc_async_prepare_response().

Quoted 6 lines

Register a socfpga-hwmon platform device from the service layer driver when hardware monitor support is enabled, similar to the RSU device.

---

I've been taking these patches through Greg KH's char-misc tree. I'll wait on your feedback on patch 2 to take this patch.

Thanks,
Dinh

maintainer
#6 lore raw

Re: [PATCH v8 1/2] firmware: stratix10-svc: add async HWMON read commands and register socfpga-hwmon device

Quoted 9 lines · Dinh Nguyen

Hi Guenter,

Quoted 6 lines · On 7/23/26 01:02, [email protected] wrote:

From: Tze Yee Ng <[email protected]>

Add asynchronous Stratix 10 service layer support for hardware monitor temperature and voltage read commands in stratix10_svc_async_send() and stratix10_svc_async_prepare_response().

Quoted 8 lines
Quoted 6 lines

Register a socfpga-hwmon platform device from the service layer driver when hardware monitor support is enabled, similar to the RSU device.

---

I've been taking these patches through Greg KH's char-misc tree. I'll wait on your feedback on patch 2 to take this patch.

If with "your" you refer to me - I can not determine if Sashiko's feedback on the second patch is correct or not. To me it sounds like real problems, but then I don't know the code well enough to be sure, and I don't have the time to analyze it myself. So you won't get any feedback from me unless someone declares that Sashiko's feedback can be ignored.

Thanks,
Guenter

author maintainer
#7 lore raw

PATCH v8 2/2 hwmon: add Altera SoC FPGA hardware monitoring driver

In reply to [email protected]

From: Tze Yee Ng <[email protected]>

Add a hardware monitor driver for Altera SoC FPGA devices using the Stratix 10 service layer. Sensor channels are selected based on the service layer compatible string.

---
Changes in v8:
- Drop the extra wait_for_completion() after sync-mode timeout; on timeout report -ETIMEDOUT and call stratix10_svc_done()
- Place socfpga-hwmon alphabetically in Documentation/hwmon/index.rst

Changes in v7:
- Break out of the async poll loop on hard errors; return the real poll status instead of -ETIMEDOUT
- Drop redundant async poll alert and -EAGAIN debug messages
- Replace msleep(1) with usleep_range() for short retries
- On sync-mode timeout, wait for the outstanding SVC callback before stratix10_svc_done() to avoid leftover svc_fifo entries desynchronizing later reads; still report -ETIMEDOUT to userspace

Changes in v6:
- No functional changes from v5

Changes in v5:
- Poll async responses until HWMON_TIMEOUT (2 s) instead of a fixed 3-iteration retry loop (~3 ms), fixing premature timeouts observed on silicon
- Add MODULE_ALIAS("platform:socfpga-hwmon")

Changes in v4:
- Register devm_add_action_or_reset() before devm_hwmon_device_register_with_info(); drop manual channel cleanup on hwmon registration failure
- Remove unreferenced async completion and pre-poll wait_for_completion_io_timeout(); poll directly after async_send() with the existing retry loop

Changes in v3:
- Fix 16-bit signed Q8.8 temperature conversion (cast through s16)
- Remove unused async callback; pass NULL to stratix10_svc_async_send()
- Keep wait_for_completion_io_timeout() before polling with comment explaining the service layer never invokes the callback but firmware needs time to complete the transaction (RSU pattern)
- Align async poll loop with RSU (retry on failure instead of aborting)
- Use wait_for_completion_timeout() for synchronous reads
- Handle -EINVAL and -EOPNOTSUPP when async client registration fails
- Defer SVC channel/async cleanup via devm_add_action_or_reset(); drop .remove()

Changes in v2:
- Drop altr,stratix10-hwmon OF compatible and DT channel parsing
- Select channels from hardcoded tables using parent SVC compatible (intel,stratix10-svc or intel,agilex-svc)
- Rename driver from stratix10-hwmon to socfpga-hwmon
- Rename Kconfig symbol to CONFIG_SENSORS_ALTERA_SOCFPGA_HWMON
- Add Agilex voltage and temperature channel tables
- Convert SDM Q8.8 degrees Celsius to hwmon millidegrees
- Convert SDM Q16 volts to hwmon millivolts
- Use socfpga_hwmon as hwmon sysfs device name
- Add last_err for synchronous SVC read error propagation
- Update Documentation/hwmon and MAINTAINERS accordingly

---
 Documentation/hwmon/index.rst         |   1 +
 Documentation/hwmon/socfpga-hwmon.rst |  34 ++
 MAINTAINERS                           |   8 +
 drivers/hwmon/Kconfig                 |  10 +
 drivers/hwmon/Makefile                |   1 +
 drivers/hwmon/socfpga-hwmon.c         | 579 ++++++++++++++++++++++++++
 6 files changed, 633 insertions(+)
 create mode 100644 Documentation/hwmon/socfpga-hwmon.rst
 create mode 100644 drivers/hwmon/socfpga-hwmon.c
Documentation/hwmon/index.rst +1 -0
index 29130df44d12..dfab6fec09bb 100644
@@ -251,6 +251,7 @@ Hardware Monitoring Kernel Drivers
    smsc47b397
    smsc47m192
    smsc47m1
+   socfpga-hwmon
    sparx5-temp
    spd5118
    stpddc60
Documentation/hwmon/socfpga-hwmon.rst new file +34 -0
new file mode 100644
index 000000000000..e5da42556a62
@@ -0,0 +1,34 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver socfpga-hwmon
+=============================
+
+Supported chips:
+
+ * Altera Stratix 10 SoC FPGA
+ * Altera Agilex SoC FPGA
+
+Authors:
+      - Nazim Amirul <[email protected]>
+      - Tze Yee Ng <[email protected]>
+
+Description
+-----------
+
+This driver supports hardware monitoring for Altera SoC
+FPGA devices through the Secure Device Manager and Stratix 10 service layer.
+
+The following sensor types are supported:
+
+  * temperature
+  * voltage
+
+Usage Notes
+-----------
+
+The stratix10-svc driver registers a socfpga-hwmon platform device when
+hardware monitor support is enabled. Sensor channels are selected in the
+driver based on the service layer compatible string:
+
+  * intel,stratix10-svc
+  * intel,agilex-svc
MAINTAINERS +8 -0
index a674e36529f7..ab6cb215a02f 100644
@@ -965,6 +965,14 @@ L:	[email protected]
 S:	Maintained
 F:	drivers/gpio/gpio-altera.c
 
+ALTERA SoC FPGA HWMON DRIVER
+M:	Nazim Amirul <[email protected]>
+M:	Tze Yee Ng <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	Documentation/hwmon/socfpga-hwmon.rst
+F:	drivers/hwmon/socfpga-hwmon.c
+
 ALTERA TRIPLE SPEED ETHERNET DRIVER
 M:	Boon Khai Ng <[email protected]>
 L:	[email protected]
drivers/hwmon/Kconfig +10 -0
index 2bfbcc033d59..8e768661fec2 100644
@@ -2160,6 +2160,16 @@ config SENSORS_SMSC47M192
 	  This driver can also be built as a module. If so, the module
 	  will be called smsc47m192.
 
+config SENSORS_ALTERA_SOCFPGA_HWMON
+	tristate "Altera SoC FPGA hardware monitoring features"
+	depends on INTEL_STRATIX10_SERVICE
+	help
+	  If you say yes here you get support for the temperature and
+	  voltage sensors of Altera SoC FPGA devices.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called socfpga-hwmon.
+
 config SENSORS_SMSC47B397
 	tristate "SMSC LPC47B397-NC"
 	depends on HAS_IOPORT
drivers/hwmon/Makefile +1 -0
index 63effc0ab8d1..aeedee80e1f0 100644
@@ -221,6 +221,7 @@ obj-$(CONFIG_SENSORS_SMPRO)	+= smpro-hwmon.o
 obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
 obj-$(CONFIG_SENSORS_SMSC47M1)	+= smsc47m1.o
 obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
+obj-$(CONFIG_SENSORS_ALTERA_SOCFPGA_HWMON)	+= socfpga-hwmon.o
 obj-$(CONFIG_SENSORS_SPARX5)	+= sparx5-temp.o
 obj-$(CONFIG_SENSORS_SPD5118)	+= spd5118.o
 obj-$(CONFIG_SENSORS_STTS751)	+= stts751.o
drivers/hwmon/socfpga-hwmon.c new file +579 -0

579 lines not on this page · raw message

Signature · 1 line
2.43.7
#8 lore raw

Re: [PATCH v8 2/2] hwmon: add Altera SoC FPGA hardware monitoring driver

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Timeouts in both synchronous and asynchronous read paths abandon pending requests without flushing…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Timeouts in both synchronous and asynchronous read paths abandon pending requests without flushing queues or canceling them, causing permanent data desynchronization or hangs for subsequent reads. --

--- Patch [2]: [PATCH v8 2/2] hwmon: add Altera SoC FPGA hardware monitoring driver --- commit 467543c9f9e3893d8c84995c5e7bc8bab31dfb20
Author: Tze Yee Ng <[email protected]>

hwmon: add Altera SoC FPGA hardware monitoring driver

This commit adds a hardware monitoring driver for Altera SoC FPGA devices using the Stratix 10 service layer. It supports temperature and voltage sensors based on the service layer compatible string.

Quoted 4 lines
drivers/hwmon/socfpga-hwmon.c new file +0 -0
new file mode 100644

[ ... ]

Quoted 4 lines
drivers/hwmon/socfpga-hwmon.c new file +4 -0
+static int socfpga_hwmon_async_read(struct device *dev,
+				    enum hwmon_sensor_types type,
+				    struct stratix10_svc_client_msg *msg)
+{

[ ... ]

Quoted 19 lines
drivers/hwmon/socfpga-hwmon.c new file +19 -0
+	ret = -ETIMEDOUT;
+	while (!time_after(jiffies, deadline)) {
+		status = stratix10_svc_async_poll(priv->chan, handle, &data);
+		if (status == -EAGAIN) {
+			/* still in progress */
+		} else if (status < 0) {
+			ret = status;
+			break;
+		} else if (status == 0) {
+			ret = 0;
+			break;
+		}
+		usleep_range(HWMON_RETRY_SLEEP_US, HWMON_RETRY_SLEEP_US * 2);
+	}
+
+	if (ret) {
+		dev_err(dev, "Failed to get async response\n");
+		goto done;
+	}

[ ... ]

drivers/hwmon/socfpga-hwmon.c line 248

Quoted 4 lines
drivers/hwmon/socfpga-hwmon.c new file +4 -0
+done:
+	stratix10_svc_async_done(priv->chan, handle);
+	return ret;
+}

[Severity: High]
If the polling loop times out, this calls stratix10_svc_async_done() which immediately frees the job ID via ida_free().

Since the firmware might still be processing this request, could the next async read reuse this job ID and intercept the delayed completion of this timed-out request?

[ ... ]

Quoted 4 lines
drivers/hwmon/socfpga-hwmon.c new file +4 -0
+static int socfpga_hwmon_sync_read(struct device *dev,
+				   enum hwmon_sensor_types type,
+				   struct stratix10_svc_client_msg *msg)
+{

[ ... ]

drivers/hwmon/socfpga-hwmon.c line 280

Quoted 13 lines
drivers/hwmon/socfpga-hwmon.c new file +13 -0
+	ret = wait_for_completion_timeout(&priv->completion, HWMON_TIMEOUT);
+	if (!ret) {
+		dev_err(priv->client.dev, "timeout waiting for SMC call\n");
+		ret = -ETIMEDOUT;
+		goto status_done;
+	}
+
+	ret = priv->last_err;
+
+status_done:
+	stratix10_svc_done(priv->chan);
+	return ret;
+}

[Severity: High]
If wait_for_completion_timeout() times out, this calls stratix10_svc_done(), which stops the worker thread via kthread_stop() without flushing the pending message from the channel's svc_fifo.

If the worker thread evaluates kthread_should_stop() before dequeuing the message, will the stale message stay in the queue?

If so, could the next read spawn a new thread that dequeues the stale message and returns wrong data (like returning a voltage reading for a temperature request)?

Also, if the worker thread is stuck inside the SMC call, won't kthread_stop() block indefinitely, hanging the reader process?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
author maintainer
#9 lore raw

Re: [PATCH v8 2/2] hwmon: add Altera SoC FPGA hardware monitoring driver

Quoted 52 lines · On 23/7/2026 2:15 pm, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Timeouts in both synchronous and asynchronous read paths abandon pending requests without flushing queues or canceling them, causing permanent data desynchronization or hangs for subsequent reads. -->
--- Patch [2]: [PATCH v8 2/2] hwmon: add Altera SoC FPGA hardware monitoring driver ---
commit 467543c9f9e3893d8c84995c5e7bc8bab31dfb20
Author: Tze Yee Ng <[email protected]>

hwmon: add Altera SoC FPGA hardware monitoring driver

This commit adds a hardware monitoring driver for Altera SoC FPGA devices using the Stratix 10 service layer. It supports temperature and voltage sensors based on the service layer compatible string.

Quoted 4 lines
drivers/hwmon/socfpga-hwmon.c new file +0 -0
new file mode 100644

[ ... ]

Quoted 4 lines
drivers/hwmon/socfpga-hwmon.c new file +4 -0
+static int socfpga_hwmon_async_read(struct device *dev,
+				    enum hwmon_sensor_types type,
+				    struct stratix10_svc_client_msg *msg)
+{

[ ... ]

Quoted 19 lines
drivers/hwmon/socfpga-hwmon.c new file +19 -0
+	ret = -ETIMEDOUT;
+	while (!time_after(jiffies, deadline)) {
+		status = stratix10_svc_async_poll(priv->chan, handle, &data);
+		if (status == -EAGAIN) {
+			/* still in progress */
+		} else if (status < 0) {
+			ret = status;
+			break;
+		} else if (status == 0) {
+			ret = 0;
+			break;
+		}
+		usleep_range(HWMON_RETRY_SLEEP_US, HWMON_RETRY_SLEEP_US * 2);
+	}
+
+	if (ret) {
+		dev_err(dev, "Failed to get async response\n");
+		goto done;
+	}

[ ... ]

drivers/hwmon/socfpga-hwmon.c line 248

Quoted 4 lines
drivers/hwmon/socfpga-hwmon.c new file +4 -0
+done:
+	stratix10_svc_async_done(priv->chan, handle);
+	return ret;
+}

[Severity: High]
If the polling loop times out, this calls stratix10_svc_async_done() which immediately frees the job ID via ida_free().

Quoted 4 lines

Since the firmware might still be processing this request, could the next async read reuse this job ID and intercept the delayed completion of this timed-out request?

Agreed — this is a valid concern.

On async timeout we call stratix10_svc_async_done(), which frees the job ID via ida_free(). Poll is keyed by transaction_id, so if firmware is still completing the old request, a later async_send() that reuses that job ID could observe the delayed response.

A proper fix belongs in the SVC / ATF (e.g. cancel before freeing the ID, or keep the ID allocated until firmware completion). We will address that in a separate SVC series rather than papering over it in the hwmon driver.

Please let me know if you would prefer a different approach.

Quoted 28 lines

[ ... ]

Quoted 4 lines
drivers/hwmon/socfpga-hwmon.c new file +4 -0
+static int socfpga_hwmon_sync_read(struct device *dev,
+				   enum hwmon_sensor_types type,
+				   struct stratix10_svc_client_msg *msg)
+{

[ ... ]

drivers/hwmon/socfpga-hwmon.c line 280

Quoted 13 lines
drivers/hwmon/socfpga-hwmon.c new file +13 -0
+	ret = wait_for_completion_timeout(&priv->completion, HWMON_TIMEOUT);
+	if (!ret) {
+		dev_err(priv->client.dev, "timeout waiting for SMC call\n");
+		ret = -ETIMEDOUT;
+		goto status_done;
+	}
+
+	ret = priv->last_err;
+
+status_done:
+	stratix10_svc_done(priv->chan);
+	return ret;
+}

[Severity: High]
If wait_for_completion_timeout() times out, this calls stratix10_svc_done(), which stops the worker thread via kthread_stop() without flushing the pending message from the channel's svc_fifo.

If the worker thread evaluates kthread_should_stop() before dequeuing the message, will the stale message stay in the queue?

Quoted 7 lines

If so, could the next read spawn a new thread that dequeues the stale message and returns wrong data (like returning a voltage reading for a temperature request)?

Also, if the worker thread is stuck inside the SMC call, won't kthread_stop() block indefinitely, hanging the reader process?

v8 drops the extra wait_for_completion() after timeout, then it reports -ETIMEDOUT and calls stratix10_svc_done(). As noted in my earlier reply to Guenter [1], the remaining concern is that stratix10_svc_done() stops the worker without flushing svc_fifo, so a request dequeued only after the client has already timed out could theoretically desynchronize a later read. We plan to address that with a FIFO flush in the service layer in a separate series.

[1]
https://lore.kernel.org/all/[email protected]/

Thanks,
Tze Yee

maintainer Acked-by on v8 2/2
#10 lore raw

Re: [PATCH v8 2/2] hwmon: add Altera SoC FPGA hardware monitoring driver

In reply to [email protected]
Quoted 8 lines · On 7/22/26 23:02, [email protected] wrote:

From: Tze Yee Ng <[email protected]>

Add a hardware monitor driver for Altera SoC FPGA devices using the Stratix 10 service layer. Sensor channels are selected based on the service layer compatible string.

I assume that the series will be applied through some other maintainer tree.

Guenter

maintainer
#11 lore raw

Re: [PATCH v8 2/2] hwmon: add Altera SoC FPGA hardware monitoring driver

Quoted 7 lines · Guenter Roeck
Quoted 6 lines · On 7/22/26 23:02, [email protected] wrote:

From: Tze Yee Ng <[email protected]>

Add a hardware monitor driver for Altera SoC FPGA devices using the Stratix 10 service layer. Sensor channels are selected based on the service layer compatible string.

Quoted 7 lines
Quoted 2 lines

I assume that the series will be applied through some other maintainer tree.

I can take this series.

Thanks,
Dinh