Linux Hardware Monitor development
 help / color / mirror / Atom feed
* [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class
@ 2026-05-08 23:33 Abdurrahman Hussain via B4 Relay
  2026-05-08 23:33 ` [PATCH 1/7] hwmon: (pmbus/adm1266) use wall-clock seconds for SET_RTC Abdurrahman Hussain via B4 Relay
                   ` (7 more replies)
  0 siblings, 8 replies; 19+ messages in thread
From: Abdurrahman Hussain via B4 Relay @ 2026-05-08 23:33 UTC (permalink / raw)
  To: Guenter Roeck, Alexandru Tachici
  Cc: linux-hwmon, linux-kernel, Abdurrahman Hussain

This series fixes a few issues in the adm1266 driver and fills in
some gaps, as listed below.

Patch 1 is a wall-clock RTC bug fix: the driver currently seeds the
device's timestamp counter with CLOCK_MONOTONIC seconds, which makes
blackbox record timestamps non-comparable across reboots. It carries
a Fixes: tag.

Patch 2 writes the 16-bit fractional-seconds field of SET_RTC that
the driver was zeroing, restoring sub-second resolution on records.

Patch 3 exposes the firmware revision (IC_DEVICE_REV, 0xAE) via
debugfs alongside the existing sequencer_state entry.

Patch 4 adds a write-only clear_blackbox debugfs file. Devices
configured for single-recording mode (BLACKBOX_CONFIG[0] = 0) need
an explicit clear once the 32-record buffer fills; the documented
sub-command ({0xFE, 0x00} block-write to 0xDE) wasn't reachable
from userspace.

Patch 5 exposes the non-volatile POWERUP_COUNTER (0xE4) via debugfs.
The same value is embedded in every blackbox record, so the live
value lets userspace match a captured record back to the boot it
came from when correlating logs.

Patch 6 registers an rtc_class device backed by SET_RTC. The data
sheet (Rev. D, p. 22) explicitly recommends "frequently send the
time stamp to the ADM1266 to synchronize the UNIX time and reduce
the time from drifting" when running on the internal oscillator
(no external 32.768 kHz crystal). With this patch userspace can
drive the re-sync via standard tooling (hwclock, chrony,
systemd-timesyncd) against /dev/rtcN, with no driver-specific
sysfs ABI.

Patch 7 disambiguates GPIO line labels on platforms that fit two
ADM1266 devices on different I2C buses at the same slave address.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
Abdurrahman Hussain (7):
      hwmon: (pmbus/adm1266) use wall-clock seconds for SET_RTC
      hwmon: (pmbus/adm1266) write fractional-seconds field of SET_RTC
      hwmon: (pmbus/adm1266) add firmware_revision debugfs entry
      hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry
      hwmon: (pmbus/adm1266) add powerup_counter debugfs entry
      hwmon: (pmbus/adm1266) register rtc_class device backed by SET_RTC
      hwmon: (pmbus/adm1266) include adapter number in GPIO line label

 drivers/hwmon/pmbus/adm1266.c | 177 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 167 insertions(+), 10 deletions(-)
---
base-commit: 917719c412c48687d4a176965d1fa35320ec457c
change-id: 20260507-adm1266-cf3af42dc3d2

Best regards,
--  
Abdurrahman Hussain <abdurrahman@nexthop.ai>



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

* [PATCH 1/7] hwmon: (pmbus/adm1266) use wall-clock seconds for SET_RTC
  2026-05-08 23:33 [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Abdurrahman Hussain via B4 Relay
@ 2026-05-08 23:33 ` Abdurrahman Hussain via B4 Relay
  2026-05-08 23:48   ` sashiko-bot
  2026-05-08 23:33 ` [PATCH 2/7] hwmon: (pmbus/adm1266) write fractional-seconds field of SET_RTC Abdurrahman Hussain via B4 Relay
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Abdurrahman Hussain via B4 Relay @ 2026-05-08 23:33 UTC (permalink / raw)
  To: Guenter Roeck, Alexandru Tachici
  Cc: linux-hwmon, linux-kernel, Abdurrahman Hussain

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

The ADM1266 RTC seeds the timestamp embedded in every blackbox record.
The driver currently sets it from ktime_get_seconds(), which returns
CLOCK_MONOTONIC seconds (i.e. seconds since boot). The intent of the
register, per the data sheet (Rev. D, "Setting UNIX Time Using
SET_RTC"), is the elapsed time since 1970-01-01 UTC.

As a result, blackbox records are stamped with "seconds since this
boot," making cross-reboot correlation against host syslog impossible
and making the timestamps in two records taken on different boots
non-comparable.

Switch to ktime_get_real_seconds() so the device is seeded with
wall-clock time, as the data sheet specifies. The internal RTC will
drift from this seed (the data sheet recommends periodic re-sync from
the host); a follow-up patch exposes the device via rtc_class so
standard userspace time-sync tooling can keep it aligned.

Fixes: 15609d189302 ("hwmon: (pmbus/adm1266) read blackbox")
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index d90f8f80be8e..a86666c73a5e 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -432,7 +432,7 @@ static int adm1266_set_rtc(struct adm1266_data *data)
 	char write_buf[6];
 	int i;
 
-	kt = ktime_get_seconds();
+	kt = ktime_get_real_seconds();
 
 	memset(write_buf, 0, sizeof(write_buf));
 

-- 
2.53.0



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

* [PATCH 2/7] hwmon: (pmbus/adm1266) write fractional-seconds field of SET_RTC
  2026-05-08 23:33 [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Abdurrahman Hussain via B4 Relay
  2026-05-08 23:33 ` [PATCH 1/7] hwmon: (pmbus/adm1266) use wall-clock seconds for SET_RTC Abdurrahman Hussain via B4 Relay
@ 2026-05-08 23:33 ` Abdurrahman Hussain via B4 Relay
  2026-05-09  0:06   ` sashiko-bot
  2026-05-08 23:33 ` [PATCH 3/7] hwmon: (pmbus/adm1266) add firmware_revision debugfs entry Abdurrahman Hussain via B4 Relay
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Abdurrahman Hussain via B4 Relay @ 2026-05-08 23:33 UTC (permalink / raw)
  To: Guenter Roeck, Alexandru Tachici
  Cc: linux-hwmon, linux-kernel, Abdurrahman Hussain

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

The SET_RTC register is six bytes wide: bytes [1:0] hold fractional
seconds (LSB = 1/65536 s) and bytes [5:2] hold whole seconds. The
driver was zeroing the fractional bytes, which loses sub-second
resolution when timestamping blackbox records.

Sample wall-clock time as a struct timespec64 and convert tv_nsec to
the device's 16-bit fractional representation. The conversion uses
64-bit arithmetic to avoid overflow on the (nsec << 16) intermediate.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index a86666c73a5e..ba8a02509528 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -428,16 +428,23 @@ static int adm1266_config_nvmem(struct adm1266_data *data)
 
 static int adm1266_set_rtc(struct adm1266_data *data)
 {
-	time64_t kt;
+	struct timespec64 ts;
 	char write_buf[6];
+	u16 frac;
 	int i;
 
-	kt = ktime_get_real_seconds();
-
-	memset(write_buf, 0, sizeof(write_buf));
+	ktime_get_real_ts64(&ts);
 
+	/*
+	 * SET_RTC frame layout (datasheet Rev. D, Table 84):
+	 *   bytes [1:0] = fractional seconds, LSB = 1/65536 s
+	 *   bytes [5:2] = seconds since 1970-01-01 UTC
+	 */
+	frac = (u16)(((u64)ts.tv_nsec << 16) / NSEC_PER_SEC);
+	for (i = 0; i < 2; i++)
+		write_buf[i] = (frac >> (i * 8)) & 0xFF;
 	for (i = 0; i < 4; i++)
-		write_buf[2 + i] = (kt >> (i * 8)) & 0xFF;
+		write_buf[2 + i] = (ts.tv_sec >> (i * 8)) & 0xFF;
 
 	return i2c_smbus_write_block_data(data->client, ADM1266_SET_RTC, sizeof(write_buf),
 					  write_buf);

-- 
2.53.0



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

* [PATCH 3/7] hwmon: (pmbus/adm1266) add firmware_revision debugfs entry
  2026-05-08 23:33 [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Abdurrahman Hussain via B4 Relay
  2026-05-08 23:33 ` [PATCH 1/7] hwmon: (pmbus/adm1266) use wall-clock seconds for SET_RTC Abdurrahman Hussain via B4 Relay
  2026-05-08 23:33 ` [PATCH 2/7] hwmon: (pmbus/adm1266) write fractional-seconds field of SET_RTC Abdurrahman Hussain via B4 Relay
@ 2026-05-08 23:33 ` Abdurrahman Hussain via B4 Relay
  2026-05-09  0:38   ` sashiko-bot
  2026-05-08 23:33 ` [PATCH 4/7] hwmon: (pmbus/adm1266) add clear_blackbox " Abdurrahman Hussain via B4 Relay
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Abdurrahman Hussain via B4 Relay @ 2026-05-08 23:33 UTC (permalink / raw)
  To: Guenter Roeck, Alexandru Tachici
  Cc: linux-hwmon, linux-kernel, Abdurrahman Hussain

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

The ADM1266 reports its firmware revision via the IC_DEVICE_REV
manufacturer-specific block-read command (0xAE, datasheet Rev. D
Table 80). The first three returned bytes are the firmware
major.minor.patch fields. This is useful when correlating field
behaviour against ADI release notes; expose it through debugfs
alongside the existing sequencer_state entry.

The standard PMBus MFR_REVISION (0x9B) register is already exposed
by pmbus_core's debugfs auto-create path and reports the
manufacturer revision, which is a separate thing from the firmware
running on the device.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index ba8a02509528..5cfca00a4468 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <linux/timekeeping.h>
 
+#define ADM1266_IC_DEVICE_REV	0xAE
 #define ADM1266_BLACKBOX_CONFIG	0xD3
 #define ADM1266_PDIO_CONFIG	0xD4
 #define ADM1266_READ_STATE	0xD9
@@ -331,6 +332,30 @@ static int adm1266_state_read(struct seq_file *s, void *pdata)
 	return 0;
 }
 
+/*
+ * IC_DEVICE_REV (0xAE) returns an 8-byte block (datasheet Rev. D, Table 80):
+ *   [2:0] firmware revision  major.minor.patch
+ *   [5:3] bootloader revision major.minor.patch
+ *   [7:6] silicon revision    two ASCII characters
+ */
+static int adm1266_firmware_revision_read(struct seq_file *s, void *pdata)
+{
+	struct device *dev = s->private;
+	struct i2c_client *client = to_i2c_client(dev);
+	u8 buf[I2C_SMBUS_BLOCK_MAX];
+	int ret;
+
+	ret = i2c_smbus_read_block_data(client, ADM1266_IC_DEVICE_REV, buf);
+	if (ret < 0)
+		return ret;
+	if (ret < 3)
+		return -EIO;
+
+	seq_printf(s, "%u.%u.%u\n", buf[0], buf[1], buf[2]);
+
+	return 0;
+}
+
 static void adm1266_init_debugfs(struct adm1266_data *data)
 {
 	struct dentry *root;
@@ -343,6 +368,8 @@ static void adm1266_init_debugfs(struct adm1266_data *data)
 
 	debugfs_create_devm_seqfile(&data->client->dev, "sequencer_state", data->debugfs_dir,
 				    adm1266_state_read);
+	debugfs_create_devm_seqfile(&data->client->dev, "firmware_revision", data->debugfs_dir,
+				    adm1266_firmware_revision_read);
 }
 
 static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)

-- 
2.53.0



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

* [PATCH 4/7] hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry
  2026-05-08 23:33 [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Abdurrahman Hussain via B4 Relay
                   ` (2 preceding siblings ...)
  2026-05-08 23:33 ` [PATCH 3/7] hwmon: (pmbus/adm1266) add firmware_revision debugfs entry Abdurrahman Hussain via B4 Relay
@ 2026-05-08 23:33 ` Abdurrahman Hussain via B4 Relay
  2026-05-09  1:10   ` sashiko-bot
  2026-05-08 23:33 ` [PATCH 5/7] hwmon: (pmbus/adm1266) add powerup_counter " Abdurrahman Hussain via B4 Relay
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Abdurrahman Hussain via B4 Relay @ 2026-05-08 23:33 UTC (permalink / raw)
  To: Guenter Roeck, Alexandru Tachici
  Cc: linux-hwmon, linux-kernel, Abdurrahman Hussain

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

The ADM1266 blackbox can be configured in two recording modes via
BLACKBOX_CONFIG[0]: cyclic, where the device overwrites the oldest
record once the 32-record buffer fills, and single, where it stops
recording until the buffer is cleared. Deployments that need to
preserve the full record history across multiple fault episodes
typically run in single mode and need a way to clear the buffer
after the records have been collected.

Expose a write-only debugfs file alongside sequencer_state. Writing
any data to it issues the documented clear-blackbox sub-command:
a 2-byte block-write to READ_BLACKBOX (0xDE) with payload
{0xFE, 0x00} (datasheet Rev. D).

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 5cfca00a4468..4c8d2e9e07dd 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -356,6 +356,36 @@ static int adm1266_firmware_revision_read(struct seq_file *s, void *pdata)
 	return 0;
 }
 
+/*
+ * Clearing the blackbox is required when the device is configured in
+ * single-recording mode (BLACKBOX_CONFIG[0] = 0): once the 32-record
+ * buffer is full the device stops recording until cleared.
+ *
+ * The clear is issued as a 2-byte block-write to READ_BLACKBOX with
+ * payload {0xFE, 0x00} per the datasheet.
+ */
+static ssize_t adm1266_clear_blackbox_write(struct file *file, const char __user *ubuf,
+					    size_t count, loff_t *ppos)
+{
+	struct i2c_client *client = file->private_data;
+	u8 payload[2] = { 0xFE, 0x00 };
+	int ret;
+
+	ret = i2c_smbus_write_block_data(client, ADM1266_READ_BLACKBOX,
+					 sizeof(payload), payload);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static const struct file_operations adm1266_clear_blackbox_fops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.write = adm1266_clear_blackbox_write,
+	.llseek = noop_llseek,
+};
+
 static void adm1266_init_debugfs(struct adm1266_data *data)
 {
 	struct dentry *root;
@@ -370,6 +400,8 @@ static void adm1266_init_debugfs(struct adm1266_data *data)
 				    adm1266_state_read);
 	debugfs_create_devm_seqfile(&data->client->dev, "firmware_revision", data->debugfs_dir,
 				    adm1266_firmware_revision_read);
+	debugfs_create_file("clear_blackbox", 0200, data->debugfs_dir, data->client,
+			    &adm1266_clear_blackbox_fops);
 }
 
 static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)

-- 
2.53.0



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

* [PATCH 5/7] hwmon: (pmbus/adm1266) add powerup_counter debugfs entry
  2026-05-08 23:33 [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Abdurrahman Hussain via B4 Relay
                   ` (3 preceding siblings ...)
  2026-05-08 23:33 ` [PATCH 4/7] hwmon: (pmbus/adm1266) add clear_blackbox " Abdurrahman Hussain via B4 Relay
@ 2026-05-08 23:33 ` Abdurrahman Hussain via B4 Relay
  2026-05-09  1:48   ` sashiko-bot
  2026-05-08 23:33 ` [PATCH 6/7] hwmon: (pmbus/adm1266) register rtc_class device backed by SET_RTC Abdurrahman Hussain via B4 Relay
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Abdurrahman Hussain via B4 Relay @ 2026-05-08 23:33 UTC (permalink / raw)
  To: Guenter Roeck, Alexandru Tachici
  Cc: linux-hwmon, linux-kernel, Abdurrahman Hussain

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

The ADM1266 maintains a 16-bit non-volatile POWERUP_COUNTER register
(0xE4, datasheet Rev. D, Table 93) that increments on every power
cycle and cannot be reset by the host. Each blackbox record already
embeds the counter at record time, so the standalone live value is
primarily useful for matching a captured record back to the boot it
came from when correlating logs.

Expose it as a read-only debugfs file alongside firmware_revision.
The block-read returns two payload bytes in little-endian order.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 4c8d2e9e07dd..a058616d4215 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -28,6 +28,7 @@
 #define ADM1266_READ_BLACKBOX	0xDE
 #define ADM1266_SET_RTC		0xDF
 #define ADM1266_GPIO_CONFIG	0xE1
+#define ADM1266_POWERUP_COUNTER	0xE4
 #define ADM1266_BLACKBOX_INFO	0xE6
 #define ADM1266_PDIO_STATUS	0xE9
 #define ADM1266_GPIO_STATUS	0xEA
@@ -356,6 +357,31 @@ static int adm1266_firmware_revision_read(struct seq_file *s, void *pdata)
 	return 0;
 }
 
+/*
+ * POWERUP_COUNTER (0xE4) is a 2-byte little-endian non-volatile counter
+ * that increments on every device power cycle (datasheet Rev. D, Table
+ * 93). It saturates at 65535 and cannot be reset by the host. Each
+ * blackbox record embeds the counter value at record time, so this live
+ * read is mainly useful for matching a record back to its boot.
+ */
+static int adm1266_powerup_counter_read(struct seq_file *s, void *pdata)
+{
+	struct device *dev = s->private;
+	struct i2c_client *client = to_i2c_client(dev);
+	u8 buf[I2C_SMBUS_BLOCK_MAX];
+	int ret;
+
+	ret = i2c_smbus_read_block_data(client, ADM1266_POWERUP_COUNTER, buf);
+	if (ret < 0)
+		return ret;
+	if (ret != 2)
+		return -EIO;
+
+	seq_printf(s, "%u\n", buf[0] | (buf[1] << 8));
+
+	return 0;
+}
+
 /*
  * Clearing the blackbox is required when the device is configured in
  * single-recording mode (BLACKBOX_CONFIG[0] = 0): once the 32-record
@@ -400,6 +426,8 @@ static void adm1266_init_debugfs(struct adm1266_data *data)
 				    adm1266_state_read);
 	debugfs_create_devm_seqfile(&data->client->dev, "firmware_revision", data->debugfs_dir,
 				    adm1266_firmware_revision_read);
+	debugfs_create_devm_seqfile(&data->client->dev, "powerup_counter", data->debugfs_dir,
+				    adm1266_powerup_counter_read);
 	debugfs_create_file("clear_blackbox", 0200, data->debugfs_dir, data->client,
 			    &adm1266_clear_blackbox_fops);
 }

-- 
2.53.0



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

* [PATCH 6/7] hwmon: (pmbus/adm1266) register rtc_class device backed by SET_RTC
  2026-05-08 23:33 [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Abdurrahman Hussain via B4 Relay
                   ` (4 preceding siblings ...)
  2026-05-08 23:33 ` [PATCH 5/7] hwmon: (pmbus/adm1266) add powerup_counter " Abdurrahman Hussain via B4 Relay
@ 2026-05-08 23:33 ` Abdurrahman Hussain via B4 Relay
  2026-05-09  2:26   ` sashiko-bot
  2026-05-08 23:33 ` [PATCH 7/7] hwmon: (pmbus/adm1266) include adapter number in GPIO line label Abdurrahman Hussain via B4 Relay
  2026-05-09 14:14 ` [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Guenter Roeck
  7 siblings, 1 reply; 19+ messages in thread
From: Abdurrahman Hussain via B4 Relay @ 2026-05-08 23:33 UTC (permalink / raw)
  To: Guenter Roeck, Alexandru Tachici
  Cc: linux-hwmon, linux-kernel, Abdurrahman Hussain

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

The ADM1266 internal oscillator is not specified for accurate time
keeping. Per the datasheet Rev. D, page 22, host software should
"frequently send the time stamp to the ADM1266 to synchronize the
UNIX time and reduce the time from drifting." Today the driver
seeds the RTC once at probe and provides no way for userspace to
re-seed it later, so blackbox record timestamps drift unboundedly
on long-uptime systems.

Register an rtc_class device backed by the SET_RTC block-R/W
register (0xDF, datasheet Rev. D, Table 84). This creates a
standard /dev/rtcN device whose ->set_time and ->read_time write
and read the same 6-byte SET_RTC frame (32-bit seconds since
1970-01-01 UTC plus a 16-bit fractional-seconds field). Standard
userspace tooling (hwclock --systohc, chrony, systemd-timesyncd)
can now drive the periodic re-sync the data sheet recommends, with
no driver-specific sysfs ABI.

The probe-time wall-clock seed is preserved so the device has a
sane RTC even before userspace runs.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 90 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 76 insertions(+), 14 deletions(-)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index a058616d4215..07c20746f083 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -18,6 +18,7 @@
 #include <linux/nvmem-consumer.h>
 #include <linux/nvmem-provider.h>
 #include "pmbus.h"
+#include <linux/rtc.h>
 #include <linux/slab.h>
 #include <linux/timekeeping.h>
 
@@ -513,28 +514,85 @@ static int adm1266_config_nvmem(struct adm1266_data *data)
 	return 0;
 }
 
-static int adm1266_set_rtc(struct adm1266_data *data)
+/*
+ * SET_RTC frame layout (datasheet Rev. D, Table 84):
+ *   bytes [1:0] = fractional seconds, LSB = 1/65536 s
+ *   bytes [5:2] = seconds since 1970-01-01 UTC
+ */
+static int adm1266_write_rtc(struct i2c_client *client, const struct timespec64 *ts)
 {
-	struct timespec64 ts;
-	char write_buf[6];
+	u8 buf[6];
 	u16 frac;
 	int i;
 
+	frac = (u16)(((u64)ts->tv_nsec << 16) / NSEC_PER_SEC);
+	for (i = 0; i < 2; i++)
+		buf[i] = (frac >> (i * 8)) & 0xFF;
+	for (i = 0; i < 4; i++)
+		buf[2 + i] = (ts->tv_sec >> (i * 8)) & 0xFF;
+
+	return i2c_smbus_write_block_data(client, ADM1266_SET_RTC, sizeof(buf), buf);
+}
+
+static int adm1266_set_rtc(struct adm1266_data *data)
+{
+	struct timespec64 ts;
+
 	ktime_get_real_ts64(&ts);
+	return adm1266_write_rtc(data->client, &ts);
+}
 
-	/*
-	 * SET_RTC frame layout (datasheet Rev. D, Table 84):
-	 *   bytes [1:0] = fractional seconds, LSB = 1/65536 s
-	 *   bytes [5:2] = seconds since 1970-01-01 UTC
-	 */
-	frac = (u16)(((u64)ts.tv_nsec << 16) / NSEC_PER_SEC);
-	for (i = 0; i < 2; i++)
-		write_buf[i] = (frac >> (i * 8)) & 0xFF;
+static int adm1266_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	u8 buf[I2C_SMBUS_BLOCK_MAX];
+	u32 secs;
+	int ret;
+	int i;
+
+	ret = i2c_smbus_read_block_data(client, ADM1266_SET_RTC, buf);
+	if (ret < 0)
+		return ret;
+	if (ret != 6)
+		return -EIO;
+
+	secs = 0;
 	for (i = 0; i < 4; i++)
-		write_buf[2 + i] = (ts.tv_sec >> (i * 8)) & 0xFF;
+		secs |= (u32)buf[2 + i] << (i * 8);
 
-	return i2c_smbus_write_block_data(data->client, ADM1266_SET_RTC, sizeof(write_buf),
-					  write_buf);
+	rtc_time64_to_tm(secs, tm);
+	return 0;
+}
+
+static int adm1266_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct timespec64 ts = {
+		.tv_sec = rtc_tm_to_time64(tm),
+		.tv_nsec = 0,
+	};
+
+	return adm1266_write_rtc(client, &ts);
+}
+
+static const struct rtc_class_ops adm1266_rtc_ops = {
+	.read_time = adm1266_rtc_read_time,
+	.set_time = adm1266_rtc_set_time,
+};
+
+static int adm1266_register_rtc(struct adm1266_data *data)
+{
+	struct rtc_device *rtc;
+
+	rtc = devm_rtc_allocate_device(&data->client->dev);
+	if (IS_ERR(rtc))
+		return PTR_ERR(rtc);
+
+	rtc->ops = &adm1266_rtc_ops;
+	rtc->range_min = 0;
+	rtc->range_max = U32_MAX;
+
+	return devm_rtc_register_device(rtc);
 }
 
 static int adm1266_probe(struct i2c_client *client)
@@ -564,6 +622,10 @@ static int adm1266_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
+	ret = adm1266_register_rtc(data);
+	if (ret < 0)
+		return ret;
+
 	ret = adm1266_config_nvmem(data);
 	if (ret < 0)
 		return ret;

-- 
2.53.0



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

* [PATCH 7/7] hwmon: (pmbus/adm1266) include adapter number in GPIO line label
  2026-05-08 23:33 [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Abdurrahman Hussain via B4 Relay
                   ` (5 preceding siblings ...)
  2026-05-08 23:33 ` [PATCH 6/7] hwmon: (pmbus/adm1266) register rtc_class device backed by SET_RTC Abdurrahman Hussain via B4 Relay
@ 2026-05-08 23:33 ` Abdurrahman Hussain via B4 Relay
  2026-05-09  2:39   ` sashiko-bot
  2026-05-09 14:14 ` [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Guenter Roeck
  7 siblings, 1 reply; 19+ messages in thread
From: Abdurrahman Hussain via B4 Relay @ 2026-05-08 23:33 UTC (permalink / raw)
  To: Guenter Roeck, Alexandru Tachici
  Cc: linux-hwmon, linux-kernel, Abdurrahman Hussain

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

Platforms that fit more than one ADM1266 on different I2C buses at
the same 7-bit slave address (a common shelf-management pattern,
e.g. one device per power domain) end up with duplicate GPIO line
labels because the existing format only includes the slave address.
Including the adapter number disambiguates them.

The label is purely informational (visible via gpioinfo and the
gpiochip /sys/class/gpio name); no DT or ABI consumer parses it.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 07c20746f083..ec6a2c9167d8 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -293,8 +293,9 @@ static int adm1266_config_gpio(struct adm1266_data *data)
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(data->gpio_names); i++) {
-		gpio_name = devm_kasprintf(&data->client->dev, GFP_KERNEL, "adm1266-%x-%s",
-					   data->client->addr, adm1266_names[i]);
+		gpio_name = devm_kasprintf(&data->client->dev, GFP_KERNEL, "adm1266-%x-%x-%s",
+					   data->client->adapter->nr, data->client->addr,
+					   adm1266_names[i]);
 		if (!gpio_name)
 			return -ENOMEM;
 

-- 
2.53.0



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

* Re: [PATCH 1/7] hwmon: (pmbus/adm1266) use wall-clock seconds for SET_RTC
  2026-05-08 23:33 ` [PATCH 1/7] hwmon: (pmbus/adm1266) use wall-clock seconds for SET_RTC Abdurrahman Hussain via B4 Relay
@ 2026-05-08 23:48   ` sashiko-bot
  0 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-05-08 23:48 UTC (permalink / raw)
  To: Abdurrahman Hussain via B4 Relay; +Cc: linux-hwmon

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Silent truncation of a 64-bit real-time timestamp causes Y2038 arithmetic overflow.
- [High] Unconditionally overwriting the hardware RTC during probe destroys retained state and defeats cross-reboot correlation.
--

commit 260e18847f33c3f0022668e0092b4f93049b2ea6
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

hwmon: (pmbus/adm1266) use wall-clock seconds for SET_RTC

This patch modifies the ADM1266 driver to seed the device internal RTC
with wall-clock time instead of monotonic time since boot. This change is
intended to allow cross-reboot correlation of blackbox records.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index d90f8f80be8e0..a86666c73a5e9 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
> @@ -432,7 +432,7 @@ static int adm1266_set_rtc(struct adm1266_data *data)
>  	char write_buf[6];
>  	int i;
>  
> -	kt = ktime_get_seconds();
> +	kt = ktime_get_real_seconds();

Can this lead to an arithmetic overflow? ktime_get_real_seconds returns
a 64-bit time64_t, but the subsequent serialization loop only processes
4 bytes:

    for (i = 0; i < 4; i++)
        write_buf[2 + i] = (kt >> (i * 8)) & 0xFF;

This silently truncates the upper 32 bits of the timestamp. Should the value
be clamped or bounds-checked to prevent silent wrap-around?

Also, if adm1266_set_rtc is called unconditionally during early probe, will
ktime_get_real_seconds return an uninitialized time near the 1970 epoch
if the system wall-clock has not yet been synchronized via an external RTC
or userspace NTP?

Overwriting the internal hardware RTC on every probe could destroy any valid
time state retained across a warm reboot, breaking the stated goal of
cross-reboot log correlation. Does this code need to preserve the existing
RTC state rather than seeding it at probe?

>  
>  	memset(write_buf, 0, sizeof(write_buf));
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-adm1266-v1-0-ec08bf29e0ce@nexthop.ai?part=1

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

* Re: [PATCH 2/7] hwmon: (pmbus/adm1266) write fractional-seconds field of SET_RTC
  2026-05-08 23:33 ` [PATCH 2/7] hwmon: (pmbus/adm1266) write fractional-seconds field of SET_RTC Abdurrahman Hussain via B4 Relay
@ 2026-05-09  0:06   ` sashiko-bot
  0 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-05-09  0:06 UTC (permalink / raw)
  To: Abdurrahman Hussain via B4 Relay; +Cc: linux-hwmon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] 64-bit division using the `/` operator will cause build failures on 32-bit architectures.
--

commit f0fa424ad89f6e68ea279a8fd355cf480c9af8fc
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

hwmon: (pmbus/adm1266) write fractional-seconds field of SET_RTC

This commit updates the adm1266 driver to write the fractional-seconds field
of the SET_RTC register. It samples wall-clock time as a struct timespec64
and converts tv_nsec to a 16-bit fractional representation using 64-bit
arithmetic.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index a86666c73a5e9..ba8a025095287 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
[ ... ]
> @@ -428,16 +428,23 @@ static int adm1266_config_nvmem(struct adm1266_data *data)
[ ... ]
> +	/*
> +	 * SET_RTC frame layout (datasheet Rev. D, Table 84):
> +	 *   bytes [1:0] = fractional seconds, LSB = 1/65536 s
> +	 *   bytes [5:2] = seconds since 1970-01-01 UTC
> +	 */
> +	frac = (u16)(((u64)ts.tv_nsec << 16) / NSEC_PER_SEC);

Will this cause build failures on 32-bit architectures?

The compiler lacks built-in hardware support for 64-bit division on 32-bit
architectures and emits a call to an intrinsic like __udivdi3, which is not
available since the kernel does not link with libgcc.

Should this use the div_u64() helper function instead, such as
div_u64((u64)ts.tv_nsec << 16, NSEC_PER_SEC)?

> +	for (i = 0; i < 2; i++)
> +		write_buf[i] = (frac >> (i * 8)) & 0xFF;
>  	for (i = 0; i < 4; i++)
> -		write_buf[2 + i] = (kt >> (i * 8)) & 0xFF;
> +		write_buf[2 + i] = (ts.tv_sec >> (i * 8)) & 0xFF;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-adm1266-v1-0-ec08bf29e0ce@nexthop.ai?part=2

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

* Re: [PATCH 3/7] hwmon: (pmbus/adm1266) add firmware_revision debugfs entry
  2026-05-08 23:33 ` [PATCH 3/7] hwmon: (pmbus/adm1266) add firmware_revision debugfs entry Abdurrahman Hussain via B4 Relay
@ 2026-05-09  0:38   ` sashiko-bot
  0 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-05-09  0:38 UTC (permalink / raw)
  To: Abdurrahman Hussain via B4 Relay; +Cc: linux-hwmon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added `firmware_revision` debugfs attribute does not implement the required locking, violating hwmon subsystem guidelines.
--

commit f5df7ce75de0c22a0deb00195b618fb596ec7c9a
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

hwmon: (pmbus/adm1266) add firmware_revision debugfs entry

This commit adds a debugfs entry to read and report the ADM1266 firmware
revision. It fetches the data via the manufacturer-specific IC_DEVICE_REV
block-read command.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index ba8a025095287..5cfca00a44682 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c

[ ... ]

> +static int adm1266_firmware_revision_read(struct seq_file *s, void *pdata)
> +{
> +	struct device *dev = s->private;
> +	struct i2c_client *client = to_i2c_client(dev);
> +	u8 buf[I2C_SMBUS_BLOCK_MAX];
> +	int ret;
> +
> +	ret = i2c_smbus_read_block_data(client, ADM1266_IC_DEVICE_REV, buf);

Does this function need to take the hardware monitoring lock before accessing
the device?

The subsystem guidelines state that drivers must implement locking for
attributes registered by other means than the standard sysfs info parameter.
Would it be appropriate to wrap this I2C read with hwmon_lock() and
hwmon_unlock() to prevent concurrent access issues?

> +	if (ret < 0)
> +		return ret;
> +	if (ret < 3)
> +		return -EIO;
> +
> +	seq_printf(s, "%u.%u.%u\n", buf[0], buf[1], buf[2]);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-adm1266-v1-0-ec08bf29e0ce@nexthop.ai?part=3

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

* Re: [PATCH 4/7] hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry
  2026-05-08 23:33 ` [PATCH 4/7] hwmon: (pmbus/adm1266) add clear_blackbox " Abdurrahman Hussain via B4 Relay
@ 2026-05-09  1:10   ` sashiko-bot
  0 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-05-09  1:10 UTC (permalink / raw)
  To: Abdurrahman Hussain via B4 Relay; +Cc: linux-hwmon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing locking in the debugfs clear_blackbox attribute handler creates a race condition with nvmem blackbox reads and violates hwmon subsystem guidelines.
--

commit bc35a67bf6371f1a9ee8f0d0c2436676ca57954d
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry

This commit exposes a write-only debugfs file alongside sequencer_state to
clear the ADM1266 blackbox. Writing any data to this file issues a 2-byte
block-write command to the READ_BLACKBOX register.

> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
[ ... ]
> +static ssize_t adm1266_clear_blackbox_write(struct file *file, const char __user *ubuf,
> +					    size_t count, loff_t *ppos)
> +{
> +	struct i2c_client *client = file->private_data;
> +	u8 payload[2] = { 0xFE, 0x00 };
> +	int ret;
> +
> +	ret = i2c_smbus_write_block_data(client, ADM1266_READ_BLACKBOX,
> +					 sizeof(payload), payload);

Could this clear command race with concurrent reads of the blackbox?

Looking at adm1266_nvmem_read_blackbox(), it performs a multi-transaction
sequence where it first reads the record_count, then loops to read individual
records:

drivers/hwmon/pmbus/adm1266.c:adm1266_nvmem_read_blackbox() {
    ...
    ret = i2c_smbus_read_block_data(data->client, ADM1266_BLACKBOX_INFO, buf);
    ...
    record_count = buf[3];

    for (index = 0; index < record_count; index++) {
        ret = adm1266_pmbus_block_xfer(data, ADM1266_READ_BLACKBOX, 1,
                                       &index, read_buff);
    ...
}

If this write runs concurrently, it seems it could clear the hardware
blackbox mid-read since the core I2C bus lock only serializes individual
transactions. This might cause the remaining nvmem reads to fetch corrupted
data or fail with I/O errors.

The hwmon subsystem guidelines also mention that drivers must implement
locking for attributes registered outside the core subsystem. Should this
write be protected to serialize it against other hardware accesses?

> +	if (ret < 0)
> +		return ret;
> +
> +	return count;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-adm1266-v1-0-ec08bf29e0ce@nexthop.ai?part=4

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

* Re: [PATCH 5/7] hwmon: (pmbus/adm1266) add powerup_counter debugfs entry
  2026-05-08 23:33 ` [PATCH 5/7] hwmon: (pmbus/adm1266) add powerup_counter " Abdurrahman Hussain via B4 Relay
@ 2026-05-09  1:48   ` sashiko-bot
  0 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-05-09  1:48 UTC (permalink / raw)
  To: Abdurrahman Hussain via B4 Relay; +Cc: linux-hwmon

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Use-after-free (UAF) during device removal due to conflicting devm and I2C core debugfs cleanup.
- [High] The debugfs attribute read function does not use hwmon_lock() to protect the hardware transaction.
--

commit b318aa9245d6d04f4ea8a9408b54190c4f30c67d
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

hwmon: (pmbus/adm1266) add powerup_counter debugfs entry

The ADM1266 maintains a 16-bit non-volatile POWERUP_COUNTER register
that increments on every power cycle and cannot be reset by the host.
This patch exposes it as a read-only debugfs file.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
[ ... ]
> +static int adm1266_powerup_counter_read(struct seq_file *s, void *pdata)
> +{
> +	struct device *dev = s->private;
> +	struct i2c_client *client = to_i2c_client(dev);
> +	u8 buf[I2C_SMBUS_BLOCK_MAX];
> +	int ret;
> +
> +	ret = i2c_smbus_read_block_data(client, ADM1266_POWERUP_COUNTER, buf);

The hwmon subsystem guidelines mention that drivers should use hwmon_lock()
and hwmon_unlock() for attributes registered outside of the core hwmon sysfs
operations.

Since this debugfs read issues an I2C transaction, should hwmon_lock() be
acquired before calling i2c_smbus_read_block_data() to avoid racing with
concurrent sysfs or internal device operations?

> +	if (ret < 0)
> +		return ret;
> +	if (ret != 2)
> +		return -EIO;
> +
> +	seq_printf(s, "%u\n", buf[0] | (buf[1] << 8));
> +
> +	return 0;
> +}
[ ... ]
> @@ -400,6 +426,8 @@ static void adm1266_init_debugfs(struct adm1266_data *data)
>  				    adm1266_state_read);
>  	debugfs_create_devm_seqfile(&data->client->dev, "firmware_revision", data->debugfs_dir,
>  				    adm1266_firmware_revision_read);
> +	debugfs_create_devm_seqfile(&data->client->dev, "powerup_counter", data->debugfs_dir,
> +				    adm1266_powerup_counter_read);

Can this sequence lead to a use-after-free during device removal?

According to the I2C subsystem rules, the I2C core cleans up debugfs entries
attached to the client after the driver's remove function finishes, but
before releasing devres resources.

If the I2C core removes the debugfs tree and frees the dentries first,
wouldn't the subsequent devm cleanup for this seqfile try to call
debugfs_remove() using an already-freed dentry pointer?

>  	debugfs_create_file("clear_blackbox", 0200, data->debugfs_dir, data->client,
>  			    &adm1266_clear_blackbox_fops);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-adm1266-v1-0-ec08bf29e0ce@nexthop.ai?part=5

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

* Re: [PATCH 6/7] hwmon: (pmbus/adm1266) register rtc_class device backed by SET_RTC
  2026-05-08 23:33 ` [PATCH 6/7] hwmon: (pmbus/adm1266) register rtc_class device backed by SET_RTC Abdurrahman Hussain via B4 Relay
@ 2026-05-09  2:26   ` sashiko-bot
  0 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-05-09  2:26 UTC (permalink / raw)
  To: Abdurrahman Hussain via B4 Relay; +Cc: linux-hwmon

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] 64-bit division using the `/` operator will cause a build failure on 32-bit architectures.
- [High] RTC operations bypass mandatory subsystem locking rules.
- [Medium] Unchecked arithmetic truncation of 64-bit system time during device probe.
--

commit 8d9788960851cdc09e1d770587afd7363eaefe26
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

hwmon: (pmbus/adm1266) register rtc_class device backed by SET_RTC

This commit registers an rtc_class device backed by the SET_RTC block-R/W
register to allow userspace to frequently synchronize the ADM1266 UNIX
time. It maps the standard /dev/rtcN ->set_time and ->read_time operations
to the 6-byte SET_RTC frame. The probe-time wall-clock seed is also
preserved.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index a058616d42156..07c20746f0831 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
> @@ -513,28 +514,85 @@ static int adm1266_config_nvmem(struct adm1266_data *data)
>  	return 0;
>  }
>  
> -static int adm1266_set_rtc(struct adm1266_data *data)
> +/*
> + * SET_RTC frame layout (datasheet Rev. D, Table 84):
> + *   bytes [1:0] = fractional seconds, LSB = 1/65536 s
> + *   bytes [5:2] = seconds since 1970-01-01 UTC
> + */
> +static int adm1266_write_rtc(struct i2c_client *client, const struct timespec64 *ts)
>  {
> -	struct timespec64 ts;
> -	char write_buf[6];
> +	u8 buf[6];
>  	u16 frac;
>  	int i;
>  
> +	frac = (u16)(((u64)ts->tv_nsec << 16) / NSEC_PER_SEC);

Will this 64-bit division cause a link error on 32-bit architectures?

Using the / operator on a 64-bit integer can lead to undefined references to
__udivdi3 on 32-bit builds. Could this use div_u64() instead?

> +	for (i = 0; i < 2; i++)
> +		buf[i] = (frac >> (i * 8)) & 0xFF;
> +	for (i = 0; i < 4; i++)
> +		buf[2 + i] = (ts->tv_sec >> (i * 8)) & 0xFF;

Since ts->tv_sec is a 64-bit value, does this loop silently truncate values
exceeding 32 bits when called during probe?

adm1266_probe()
  adm1266_set_rtc()
    adm1266_write_rtc()

The hwmon subsystem guidelines ask to check for overflows in arithmetic
calculations.

> +
> +	return i2c_smbus_write_block_data(client, ADM1266_SET_RTC, sizeof(buf), buf);
> +}

[ ... ]

> +static int adm1266_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	u8 buf[I2C_SMBUS_BLOCK_MAX];
> +	u32 secs;
> +	int ret;
> +	int i;
> +
> +	ret = i2c_smbus_read_block_data(client, ADM1266_SET_RTC, buf);

The hardware monitoring subsystem guidelines state that drivers must implement
locking for attributes registered by any other means.

Are these callbacks missing a call to hwmon_lock() (or pmbus_lock())?

If an RTC class device implicitly exposes time attributes to userspace, could
these concurrent I2C accesses risk disrupting stateful hardware transactions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-adm1266-v1-0-ec08bf29e0ce@nexthop.ai?part=6

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

* Re: [PATCH 7/7] hwmon: (pmbus/adm1266) include adapter number in GPIO line label
  2026-05-08 23:33 ` [PATCH 7/7] hwmon: (pmbus/adm1266) include adapter number in GPIO line label Abdurrahman Hussain via B4 Relay
@ 2026-05-09  2:39   ` sashiko-bot
  0 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-05-09  2:39 UTC (permalink / raw)
  To: Abdurrahman Hussain via B4 Relay; +Cc: linux-hwmon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The I2C adapter number is incorrectly formatted as hexadecimal instead of decimal in the GPIO line label.
--

commit 79ede8d83182247b49e64822f8f1d3ea0161b1cd
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

hwmon: (pmbus/adm1266) include adapter number in GPIO line label

This commit updates the GPIO line labels in the ADM1266 driver to include
the I2C adapter number. This change disambiguates the labels when multiple
devices reside at the same I2C address on different buses.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 07c20746f0831..ec6a2c9167d82 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
> @@ -293,8 +293,9 @@ static int adm1266_config_gpio(struct adm1266_data *data)
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(data->gpio_names); i++) {
> -		gpio_name = devm_kasprintf(&data->client->dev, GFP_KERNEL, "adm1266-%x-%s",
> -					   data->client->addr, adm1266_names[i]);
> +		gpio_name = devm_kasprintf(&data->client->dev, GFP_KERNEL, "adm1266-%x-%x-%s",
> +					   data->client->adapter->nr, data->client->addr,

Is the use of %x intended for the I2C adapter number here?

I2C adapter numbers are conventionally represented as decimal integers in
Linux (e.g., i2c-10 or 10-0040). Formatting the adapter number as hexadecimal
means that bus numbers of 10 or greater will be displayed with letters (e.g.,
bus 10 becomes a, resulting in adm1266-a-40-...).

Would using %d for the adapter number better match standard I2C identifiers?

> +					   adm1266_names[i]);
>  		if (!gpio_name)
>  			return -ENOMEM;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-adm1266-v1-0-ec08bf29e0ce@nexthop.ai?part=7

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

* Re: [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class
  2026-05-08 23:33 [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Abdurrahman Hussain via B4 Relay
                   ` (6 preceding siblings ...)
  2026-05-08 23:33 ` [PATCH 7/7] hwmon: (pmbus/adm1266) include adapter number in GPIO line label Abdurrahman Hussain via B4 Relay
@ 2026-05-09 14:14 ` Guenter Roeck
  2026-05-09 21:58   ` Abdurrahman Hussain
  7 siblings, 1 reply; 19+ messages in thread
From: Guenter Roeck @ 2026-05-09 14:14 UTC (permalink / raw)
  To: abdurrahman, Alexandru Tachici; +Cc: linux-hwmon, linux-kernel

On 5/8/26 16:33, Abdurrahman Hussain via B4 Relay wrote:
> This series fixes a few issues in the adm1266 driver and fills in
> some gaps, as listed below.
> 
> Patch 1 is a wall-clock RTC bug fix: the driver currently seeds the
> device's timestamp counter with CLOCK_MONOTONIC seconds, which makes
> blackbox record timestamps non-comparable across reboots. It carries
> a Fixes: tag.
> 
> Patch 2 writes the 16-bit fractional-seconds field of SET_RTC that
> the driver was zeroing, restoring sub-second resolution on records.
> 
> Patch 3 exposes the firmware revision (IC_DEVICE_REV, 0xAE) via
> debugfs alongside the existing sequencer_state entry.
> 
> Patch 4 adds a write-only clear_blackbox debugfs file. Devices
> configured for single-recording mode (BLACKBOX_CONFIG[0] = 0) need
> an explicit clear once the 32-record buffer fills; the documented
> sub-command ({0xFE, 0x00} block-write to 0xDE) wasn't reachable
> from userspace.
> 
> Patch 5 exposes the non-volatile POWERUP_COUNTER (0xE4) via debugfs.
> The same value is embedded in every blackbox record, so the live
> value lets userspace match a captured record back to the boot it
> came from when correlating logs.
> 
> Patch 6 registers an rtc_class device backed by SET_RTC. The data
> sheet (Rev. D, p. 22) explicitly recommends "frequently send the
> time stamp to the ADM1266 to synchronize the UNIX time and reduce
> the time from drifting" when running on the internal oscillator
> (no external 32.768 kHz crystal). With this patch userspace can
> drive the re-sync via standard tooling (hwclock, chrony,
> systemd-timesyncd) against /dev/rtcN, with no driver-specific
> sysfs ABI.
> 
> Patch 7 disambiguates GPIO line labels on platforms that fit two
> ADM1266 devices on different I2C buses at the same slave address.
> 
> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>

Please address Sashiko's feedback.

Thanks,
Guenter


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

* Re: [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class
  2026-05-09 14:14 ` [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Guenter Roeck
@ 2026-05-09 21:58   ` Abdurrahman Hussain
  2026-05-09 23:49     ` Guenter Roeck
  0 siblings, 1 reply; 19+ messages in thread
From: Abdurrahman Hussain @ 2026-05-09 21:58 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Alexandru Tachici, linux-hwmon, linux-kernel

Hi Guenter,

I didn't see any feedback. Checked my spam folder and nothing
shows up on the mailing list either.

Regards,
Abdurrahman


On Sat, May 9, 2026 at 7:14 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 5/8/26 16:33, Abdurrahman Hussain via B4 Relay wrote:
> > This series fixes a few issues in the adm1266 driver and fills in
> > some gaps, as listed below.
> >
> > Patch 1 is a wall-clock RTC bug fix: the driver currently seeds the
> > device's timestamp counter with CLOCK_MONOTONIC seconds, which makes
> > blackbox record timestamps non-comparable across reboots. It carries
> > a Fixes: tag.
> >
> > Patch 2 writes the 16-bit fractional-seconds field of SET_RTC that
> > the driver was zeroing, restoring sub-second resolution on records.
> >
> > Patch 3 exposes the firmware revision (IC_DEVICE_REV, 0xAE) via
> > debugfs alongside the existing sequencer_state entry.
> >
> > Patch 4 adds a write-only clear_blackbox debugfs file. Devices
> > configured for single-recording mode (BLACKBOX_CONFIG[0] = 0) need
> > an explicit clear once the 32-record buffer fills; the documented
> > sub-command ({0xFE, 0x00} block-write to 0xDE) wasn't reachable
> > from userspace.
> >
> > Patch 5 exposes the non-volatile POWERUP_COUNTER (0xE4) via debugfs.
> > The same value is embedded in every blackbox record, so the live
> > value lets userspace match a captured record back to the boot it
> > came from when correlating logs.
> >
> > Patch 6 registers an rtc_class device backed by SET_RTC. The data
> > sheet (Rev. D, p. 22) explicitly recommends "frequently send the
> > time stamp to the ADM1266 to synchronize the UNIX time and reduce
> > the time from drifting" when running on the internal oscillator
> > (no external 32.768 kHz crystal). With this patch userspace can
> > drive the re-sync via standard tooling (hwclock, chrony,
> > systemd-timesyncd) against /dev/rtcN, with no driver-specific
> > sysfs ABI.
> >
> > Patch 7 disambiguates GPIO line labels on platforms that fit two
> > ADM1266 devices on different I2C buses at the same slave address.
> >
> > Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>
> Please address Sashiko's feedback.
>
> Thanks,
> Guenter
>

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

* Re: [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class
  2026-05-09 21:58   ` Abdurrahman Hussain
@ 2026-05-09 23:49     ` Guenter Roeck
  2026-05-11  3:46       ` Abdurrahman Hussain
  0 siblings, 1 reply; 19+ messages in thread
From: Guenter Roeck @ 2026-05-09 23:49 UTC (permalink / raw)
  To: Abdurrahman Hussain; +Cc: Alexandru Tachici, linux-hwmon, linux-kernel

On 5/9/26 14:58, Abdurrahman Hussain wrote:
> Hi Guenter,
> 
> I didn't see any feedback. Checked my spam folder and nothing
> shows up on the mailing list either.
> 

Your return address is apparently

Abdurrahman Hussain via B4 Relay <devnull+abdurrahman.nexthop.ai@kernel.org>

which I guess ends up in /dev/null. I don't know why the e-mail to the
mailing list gets filtered out.

Here is the feedback:

https://sashiko.dev/#/patchset/20260508-adm1266-v1-0-ec08bf29e0ce%40nexthop.ai

Guenter

> Regards,
> Abdurrahman
> 
> 
> On Sat, May 9, 2026 at 7:14 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 5/8/26 16:33, Abdurrahman Hussain via B4 Relay wrote:
>>> This series fixes a few issues in the adm1266 driver and fills in
>>> some gaps, as listed below.
>>>
>>> Patch 1 is a wall-clock RTC bug fix: the driver currently seeds the
>>> device's timestamp counter with CLOCK_MONOTONIC seconds, which makes
>>> blackbox record timestamps non-comparable across reboots. It carries
>>> a Fixes: tag.
>>>
>>> Patch 2 writes the 16-bit fractional-seconds field of SET_RTC that
>>> the driver was zeroing, restoring sub-second resolution on records.
>>>
>>> Patch 3 exposes the firmware revision (IC_DEVICE_REV, 0xAE) via
>>> debugfs alongside the existing sequencer_state entry.
>>>
>>> Patch 4 adds a write-only clear_blackbox debugfs file. Devices
>>> configured for single-recording mode (BLACKBOX_CONFIG[0] = 0) need
>>> an explicit clear once the 32-record buffer fills; the documented
>>> sub-command ({0xFE, 0x00} block-write to 0xDE) wasn't reachable
>>> from userspace.
>>>
>>> Patch 5 exposes the non-volatile POWERUP_COUNTER (0xE4) via debugfs.
>>> The same value is embedded in every blackbox record, so the live
>>> value lets userspace match a captured record back to the boot it
>>> came from when correlating logs.
>>>
>>> Patch 6 registers an rtc_class device backed by SET_RTC. The data
>>> sheet (Rev. D, p. 22) explicitly recommends "frequently send the
>>> time stamp to the ADM1266 to synchronize the UNIX time and reduce
>>> the time from drifting" when running on the internal oscillator
>>> (no external 32.768 kHz crystal). With this patch userspace can
>>> drive the re-sync via standard tooling (hwclock, chrony,
>>> systemd-timesyncd) against /dev/rtcN, with no driver-specific
>>> sysfs ABI.
>>>
>>> Patch 7 disambiguates GPIO line labels on platforms that fit two
>>> ADM1266 devices on different I2C buses at the same slave address.
>>>
>>> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>>
>> Please address Sashiko's feedback.
>>
>> Thanks,
>> Guenter
>>


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

* Re: [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class
  2026-05-09 23:49     ` Guenter Roeck
@ 2026-05-11  3:46       ` Abdurrahman Hussain
  0 siblings, 0 replies; 19+ messages in thread
From: Abdurrahman Hussain @ 2026-05-11  3:46 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Alexandru Tachici, linux-hwmon, linux-kernel

Hi Guenter,

Thank you for the link.

I am not sure why the replies are being filtered;
it may be related to using the b4 relay web endpoint.

I will address the comments on Sashiko and submit v2 shortly.
I'll send the next version directly to avoid further delivery issues.

Best regards,
Abdurrahman Hussain

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

end of thread, other threads:[~2026-05-11  3:46 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-08 23:33 [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Abdurrahman Hussain via B4 Relay
2026-05-08 23:33 ` [PATCH 1/7] hwmon: (pmbus/adm1266) use wall-clock seconds for SET_RTC Abdurrahman Hussain via B4 Relay
2026-05-08 23:48   ` sashiko-bot
2026-05-08 23:33 ` [PATCH 2/7] hwmon: (pmbus/adm1266) write fractional-seconds field of SET_RTC Abdurrahman Hussain via B4 Relay
2026-05-09  0:06   ` sashiko-bot
2026-05-08 23:33 ` [PATCH 3/7] hwmon: (pmbus/adm1266) add firmware_revision debugfs entry Abdurrahman Hussain via B4 Relay
2026-05-09  0:38   ` sashiko-bot
2026-05-08 23:33 ` [PATCH 4/7] hwmon: (pmbus/adm1266) add clear_blackbox " Abdurrahman Hussain via B4 Relay
2026-05-09  1:10   ` sashiko-bot
2026-05-08 23:33 ` [PATCH 5/7] hwmon: (pmbus/adm1266) add powerup_counter " Abdurrahman Hussain via B4 Relay
2026-05-09  1:48   ` sashiko-bot
2026-05-08 23:33 ` [PATCH 6/7] hwmon: (pmbus/adm1266) register rtc_class device backed by SET_RTC Abdurrahman Hussain via B4 Relay
2026-05-09  2:26   ` sashiko-bot
2026-05-08 23:33 ` [PATCH 7/7] hwmon: (pmbus/adm1266) include adapter number in GPIO line label Abdurrahman Hussain via B4 Relay
2026-05-09  2:39   ` sashiko-bot
2026-05-09 14:14 ` [PATCH 0/7] hwmon: (pmbus/adm1266) RTC fix, blackbox, FW rev, rtc_class Guenter Roeck
2026-05-09 21:58   ` Abdurrahman Hussain
2026-05-09 23:49     ` Guenter Roeck
2026-05-11  3:46       ` Abdurrahman Hussain

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