drivers/hwmon/pmbus/adm1266.c | 77 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 71 insertions(+), 6 deletions(-)
This is what remains of the v3 series after Guenter applied patches
1/5 (firmware_revision) and 5/5 (GPIO line label) to hwmon-next, and
asked for patch 4/5 (rtc_class) to be dropped.
Patch 1 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. The patch also acquires pmbus_lock at the
adm1266_nvmem_read() callback boundary so the memset of
data->dev_mem, the blackbox refill, and the memcpy to userspace run
as a single critical section -- the nvmem core does not serialize
concurrent reg_read calls.
Patch 2 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. The block-read is taken under
pmbus_lock to serialise against any pmbus_core PAGE+register
sequence on the device.
Patch 3 takes pmbus_lock in adm1266_state_read() (the pre-existing
sequencer_state debugfs handler) for the same defensive-locking
reason that motivates the new debugfs files in patches 1 and 2:
any direct device access from outside pmbus_core should be ordered
with respect to pmbus_core's own PAGE+register sequences.
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
Changes in v4:
- Drop former patches 1 (firmware_revision) and 5 (GPIO line label):
applied to hwmon-next by Guenter.
- Drop former patch 4 (rtc_class) per Guenter's review: turning the
chip into a pseudo rtc_class device and making the driver depend
on the RTC subsystem was rejected. The underlying CLOCK_MONOTONIC
-vs- wall-clock bug in the original probe-time seed is fixed
independently in the "hwmon: (pmbus/adm1266) buffer-bound and
timestamp fixes" series sent in parallel, so the seed-at-probe
behaviour becomes correct without this driver needing to depend
on RTC_CLASS.
- Patch 1 (clear_blackbox, was 2/5): take pmbus_lock at the outer
adm1266_nvmem_read() callback rather than inside
adm1266_nvmem_read_blackbox() so it covers the memset of
data->dev_mem, the refill, and the memcpy to userspace as a
single critical section. The nvmem core does not serialize
concurrent reg_read invocations, so the v3 placement still let
the memset race with another reader's memcpy to userspace
(Sashiko review of v3).
- Patch 2 (powerup_counter, was 3/5): take pmbus_lock around the
block-read so the access serialises with any pmbus_core sequence
that sets PAGE on the device (Sashiko review of v3).
- New patch 3: take pmbus_lock in adm1266_state_read() (the
sequencer_state debugfs handler). Same defensive-locking
argument as patches 1 and 2; surfaced while folding similar
fixes into the parallel "hwmon: (pmbus/adm1266) GPIO accessor
fixes" series.
- Link to v3: https://patch.msgid.link/20260512-adm1266-v3-0-a81a479b0bb0@nexthop.ai
Changes in v3:
- Patch 2: take guard(pmbus_lock)(client) in
adm1266_clear_blackbox_write() and adm1266_nvmem_read_blackbox()
to serialise the clear against the multi-record nvmem read loop
(both share command 0xDE). Also move adm1266_config_nvmem() to
after pmbus_do_probe() so the blackbox nvmem entry isn't exposed
to userspace before the PMBus mutex is initialised (Sashiko
review).
- Patch 4: take guard(pmbus_lock)(client) in the RTC
read_time/set_time callbacks to serialise against the PMBus
core's PAGE+register sequences. Move adm1266_register_rtc() to
after pmbus_do_probe() for the same reason as the config_nvmem
move; /dev/rtcN being opened by userspace (systemd-timesyncd,
udev) before probe completed would otherwise hit an
uninitialised mutex (observed on hardware, also flagged by
Sashiko).
- Link to v2: https://patch.msgid.link/20260510-adm1266-v2-0-3a22b903c2f1@nexthop.ai
Changes in v2:
- Drop the v1 "use wall-clock seconds for SET_RTC" and "write
fractional-seconds field of SET_RTC" patches. v1 patch 6 already
added an rtc_class device, so userspace now owns the time-set
policy and the probe-time seed is no longer needed - removing it
also fixes the cross-reboot blackbox-correlation regression the
seed introduced.
- In the rtc_class patch, write the SET_RTC fractional-seconds
bytes as zero rather than computing them from a timespec64. The
rtc_class API is second-precision, so the divide that v1 patch 2
was adding never produced a non-zero result anyway. This also
drops the 64-bit divide that would have failed to link on 32-bit
builds (Sashiko review).
- Use %d (decimal) for the I2C adapter number in GPIO line labels,
matching the i2c-N convention used elsewhere in Linux. v1 used
%x, which rendered bus numbers >= 10 in hexadecimal (Sashiko
review).
- Link to v1: https://patch.msgid.link/20260508-adm1266-v1-0-ec08bf29e0ce@nexthop.ai
To: Guenter Roeck <linux@roeck-us.net>
To: Alexandru Tachici <alexandru.tachici@analog.com>
Cc: linux-hwmon@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
Abdurrahman Hussain (3):
hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry
hwmon: (pmbus/adm1266) add powerup_counter debugfs entry
hwmon: (pmbus/adm1266) serialize sequencer_state debugfs read with pmbus_lock
drivers/hwmon/pmbus/adm1266.c | 77 +++++++++++++++++++++++++++++++++++++++----
1 file changed, 71 insertions(+), 6 deletions(-)
---
base-commit: 70eda68668d1476b459b64e69b8f36659fa9dfa8
change-id: 20260507-adm1266-cf3af42dc3d2
Best regards,
--
Abdurrahman Hussain <abdurrahman@nexthop.ai>
Hi,
On 5/16/26 18:18, Abdurrahman Hussain wrote:
> This is what remains of the v3 series after Guenter applied patches
> 1/5 (firmware_revision) and 5/5 (GPIO line label) to hwmon-next, and
> asked for patch 4/5 (rtc_class) to be dropped.
>
> Patch 1 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. The patch also acquires pmbus_lock at the
> adm1266_nvmem_read() callback boundary so the memset of
> data->dev_mem, the blackbox refill, and the memcpy to userspace run
> as a single critical section -- the nvmem core does not serialize
> concurrent reg_read calls.
>
> Patch 2 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. The block-read is taken under
> pmbus_lock to serialise against any pmbus_core PAGE+register
> sequence on the device.
>
> Patch 3 takes pmbus_lock in adm1266_state_read() (the pre-existing
> sequencer_state debugfs handler) for the same defensive-locking
> reason that motivates the new debugfs files in patches 1 and 2:
> any direct device access from outside pmbus_core should be ordered
> with respect to pmbus_core's own PAGE+register sequences.
>
> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
The series no longer applies after applying the bug fix series.
Please rebase it on top of the hwmon-next branch of
git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
and resubmit.
Sorry for the trouble, and thanks a lot for fixing all the problems
with the driver.
Guenter
On Wed May 20, 2026 at 7:10 AM PDT, Guenter Roeck wrote:
> Hi,
>
> On 5/16/26 18:18, Abdurrahman Hussain wrote:
>> This is what remains of the v3 series after Guenter applied patches
>> 1/5 (firmware_revision) and 5/5 (GPIO line label) to hwmon-next, and
>> asked for patch 4/5 (rtc_class) to be dropped.
>>
>> Patch 1 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. The patch also acquires pmbus_lock at the
>> adm1266_nvmem_read() callback boundary so the memset of
>> data->dev_mem, the blackbox refill, and the memcpy to userspace run
>> as a single critical section -- the nvmem core does not serialize
>> concurrent reg_read calls.
>>
>> Patch 2 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. The block-read is taken under
>> pmbus_lock to serialise against any pmbus_core PAGE+register
>> sequence on the device.
>>
>> Patch 3 takes pmbus_lock in adm1266_state_read() (the pre-existing
>> sequencer_state debugfs handler) for the same defensive-locking
>> reason that motivates the new debugfs files in patches 1 and 2:
>> any direct device access from outside pmbus_core should be ordered
>> with respect to pmbus_core's own PAGE+register sequences.
>>
>> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>
> The series no longer applies after applying the bug fix series.
> Please rebase it on top of the hwmon-next branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
> and resubmit.
>
> Sorry for the trouble, and thanks a lot for fixing all the problems
> with the driver.
>
> Guenter
Will do, thank you for your support!
Abdurrahman
On Wed May 20, 2026 at 10:10 AM PDT, Abdurrahman Hussain wrote:
> On Wed May 20, 2026 at 7:10 AM PDT, Guenter Roeck wrote:
>> Hi,
>>
>> On 5/16/26 18:18, Abdurrahman Hussain wrote:
>>> This is what remains of the v3 series after Guenter applied patches
>>> 1/5 (firmware_revision) and 5/5 (GPIO line label) to hwmon-next, and
>>> asked for patch 4/5 (rtc_class) to be dropped.
>>>
>>> Patch 1 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. The patch also acquires pmbus_lock at the
>>> adm1266_nvmem_read() callback boundary so the memset of
>>> data->dev_mem, the blackbox refill, and the memcpy to userspace run
>>> as a single critical section -- the nvmem core does not serialize
>>> concurrent reg_read calls.
>>>
>>> Patch 2 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. The block-read is taken under
>>> pmbus_lock to serialise against any pmbus_core PAGE+register
>>> sequence on the device.
>>>
>>> Patch 3 takes pmbus_lock in adm1266_state_read() (the pre-existing
>>> sequencer_state debugfs handler) for the same defensive-locking
>>> reason that motivates the new debugfs files in patches 1 and 2:
>>> any direct device access from outside pmbus_core should be ordered
>>> with respect to pmbus_core's own PAGE+register sequences.
>>>
>>> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>>
>> The series no longer applies after applying the bug fix series.
>> Please rebase it on top of the hwmon-next branch of
>> git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
>> and resubmit.
>>
>> Sorry for the trouble, and thanks a lot for fixing all the problems
>> with the driver.
>>
>> Guenter
>
> Will do, thank you for your support!
>
> Abdurrahman
Hi Guenter,
Before I send v5 of the adm1266 series, I'd like to revisit the
SET_RTC exposure question from your v3 review [1].
The use case I keep landing on is the one the datasheet itself
recommends: a userspace agent (chrony hook, systemd-timesyncd
script, or a small periodic daemon) keeps the chip's seconds
counter aligned with wall-clock so the value embedded in each
blackbox record stays correct across long uptimes. The probe-
time ktime_get_real_seconds() seed (now in hwmon-next) only fixes
this at boot; the chip's counter drifts after that.
You ruled out rtc_class and called a kernel-side periodic timer
"a bit excessive", which I agree, it is. That leaves a
userspace-triggered sync. Two debugfs shapes I'd consider, both under
pmbus/<hwmon>/adm1266/ (alongside the clear_blackbox,
powerup_counter, and sequencer_state entries v5 already adds):
A. RW since_epoch -- mirrors /sys/class/rtc/<dev>/since_epoch.
Read returns the chip's SET_RTC seconds counter, write sets
it. The read has the useful side benefit of letting
userspace measure host-vs-chip drift without writing.
B. Write-only set_rtc -- writing anything to the file makes
the driver read ktime_get_real_seconds() itself and push it
to SET_RTC. Simpler interface (kernel owns the time
source; userspace just triggers the sync), no payload to
parse, no way for userspace to pass in a wrong value.
Do either of these look right to you, or would you rather I just
leave the driver at "probe-time seed only" and skip a SET_RTC
interface entirely? v5 as it stands has no such interface and
is ready to send; a SET_RTC patch can follow separately later if
you'd like one.
[1] https://lore.kernel.org/r/20260512-adm1266-v3-0-a81a479b0bb0@nexthop.ai
Thanks,
Abdurrahman
On 5/20/26 12:40, Abdurrahman Hussain wrote:
> On Wed May 20, 2026 at 10:10 AM PDT, Abdurrahman Hussain wrote:
>> On Wed May 20, 2026 at 7:10 AM PDT, Guenter Roeck wrote:
>>> Hi,
>>>
>>> On 5/16/26 18:18, Abdurrahman Hussain wrote:
>>>> This is what remains of the v3 series after Guenter applied patches
>>>> 1/5 (firmware_revision) and 5/5 (GPIO line label) to hwmon-next, and
>>>> asked for patch 4/5 (rtc_class) to be dropped.
>>>>
>>>> Patch 1 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. The patch also acquires pmbus_lock at the
>>>> adm1266_nvmem_read() callback boundary so the memset of
>>>> data->dev_mem, the blackbox refill, and the memcpy to userspace run
>>>> as a single critical section -- the nvmem core does not serialize
>>>> concurrent reg_read calls.
>>>>
>>>> Patch 2 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. The block-read is taken under
>>>> pmbus_lock to serialise against any pmbus_core PAGE+register
>>>> sequence on the device.
>>>>
>>>> Patch 3 takes pmbus_lock in adm1266_state_read() (the pre-existing
>>>> sequencer_state debugfs handler) for the same defensive-locking
>>>> reason that motivates the new debugfs files in patches 1 and 2:
>>>> any direct device access from outside pmbus_core should be ordered
>>>> with respect to pmbus_core's own PAGE+register sequences.
>>>>
>>>> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>>>
>>> The series no longer applies after applying the bug fix series.
>>> Please rebase it on top of the hwmon-next branch of
>>> git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
>>> and resubmit.
>>>
>>> Sorry for the trouble, and thanks a lot for fixing all the problems
>>> with the driver.
>>>
>>> Guenter
>>
>> Will do, thank you for your support!
>>
>> Abdurrahman
>
> Hi Guenter,
>
> Before I send v5 of the adm1266 series, I'd like to revisit the
> SET_RTC exposure question from your v3 review [1].
>
> The use case I keep landing on is the one the datasheet itself
> recommends: a userspace agent (chrony hook, systemd-timesyncd
> script, or a small periodic daemon) keeps the chip's seconds
> counter aligned with wall-clock so the value embedded in each
> blackbox record stays correct across long uptimes. The probe-
> time ktime_get_real_seconds() seed (now in hwmon-next) only fixes
> this at boot; the chip's counter drifts after that.
>
> You ruled out rtc_class and called a kernel-side periodic timer
> "a bit excessive", which I agree, it is. That leaves a
> userspace-triggered sync. Two debugfs shapes I'd consider, both under
> pmbus/<hwmon>/adm1266/ (alongside the clear_blackbox,
> powerup_counter, and sequencer_state entries v5 already adds):
>
> A. RW since_epoch -- mirrors /sys/class/rtc/<dev>/since_epoch.
> Read returns the chip's SET_RTC seconds counter, write sets
> it. The read has the useful side benefit of letting
> userspace measure host-vs-chip drift without writing.
>
> B. Write-only set_rtc -- writing anything to the file makes
> the driver read ktime_get_real_seconds() itself and push it
> to SET_RTC. Simpler interface (kernel owns the time
> source; userspace just triggers the sync), no payload to
> parse, no way for userspace to pass in a wrong value.
>
How about a combination ? read returns the current value, anything
written synchronizes with the kernel rtc.
> Do either of these look right to you, or would you rather I just
> leave the driver at "probe-time seed only" and skip a SET_RTC
> interface entirely? v5 as it stands has no such interface and
> is ready to send; a SET_RTC patch can follow separately later if
> you'd like one.
>
I am fine either way.
Thanks,
Guenter
On Wed May 20, 2026 at 1:59 PM PDT, Guenter Roeck wrote:
> On 5/20/26 12:40, Abdurrahman Hussain wrote:
>> On Wed May 20, 2026 at 10:10 AM PDT, Abdurrahman Hussain wrote:
>>> On Wed May 20, 2026 at 7:10 AM PDT, Guenter Roeck wrote:
>>>> Hi,
>>>>
>>>> On 5/16/26 18:18, Abdurrahman Hussain wrote:
>>>>> This is what remains of the v3 series after Guenter applied patches
>>>>> 1/5 (firmware_revision) and 5/5 (GPIO line label) to hwmon-next, and
>>>>> asked for patch 4/5 (rtc_class) to be dropped.
>>>>>
>>>>> Patch 1 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. The patch also acquires pmbus_lock at the
>>>>> adm1266_nvmem_read() callback boundary so the memset of
>>>>> data->dev_mem, the blackbox refill, and the memcpy to userspace run
>>>>> as a single critical section -- the nvmem core does not serialize
>>>>> concurrent reg_read calls.
>>>>>
>>>>> Patch 2 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. The block-read is taken under
>>>>> pmbus_lock to serialise against any pmbus_core PAGE+register
>>>>> sequence on the device.
>>>>>
>>>>> Patch 3 takes pmbus_lock in adm1266_state_read() (the pre-existing
>>>>> sequencer_state debugfs handler) for the same defensive-locking
>>>>> reason that motivates the new debugfs files in patches 1 and 2:
>>>>> any direct device access from outside pmbus_core should be ordered
>>>>> with respect to pmbus_core's own PAGE+register sequences.
>>>>>
>>>>> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>>>>
>>>> The series no longer applies after applying the bug fix series.
>>>> Please rebase it on top of the hwmon-next branch of
>>>> git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
>>>> and resubmit.
>>>>
>>>> Sorry for the trouble, and thanks a lot for fixing all the problems
>>>> with the driver.
>>>>
>>>> Guenter
>>>
>>> Will do, thank you for your support!
>>>
>>> Abdurrahman
>>
>> Hi Guenter,
>>
>> Before I send v5 of the adm1266 series, I'd like to revisit the
>> SET_RTC exposure question from your v3 review [1].
>>
>> The use case I keep landing on is the one the datasheet itself
>> recommends: a userspace agent (chrony hook, systemd-timesyncd
>> script, or a small periodic daemon) keeps the chip's seconds
>> counter aligned with wall-clock so the value embedded in each
>> blackbox record stays correct across long uptimes. The probe-
>> time ktime_get_real_seconds() seed (now in hwmon-next) only fixes
>> this at boot; the chip's counter drifts after that.
>>
>> You ruled out rtc_class and called a kernel-side periodic timer
>> "a bit excessive", which I agree, it is. That leaves a
>> userspace-triggered sync. Two debugfs shapes I'd consider, both under
>> pmbus/<hwmon>/adm1266/ (alongside the clear_blackbox,
>> powerup_counter, and sequencer_state entries v5 already adds):
>>
>> A. RW since_epoch -- mirrors /sys/class/rtc/<dev>/since_epoch.
>> Read returns the chip's SET_RTC seconds counter, write sets
>> it. The read has the useful side benefit of letting
>> userspace measure host-vs-chip drift without writing.
>>
>> B. Write-only set_rtc -- writing anything to the file makes
>> the driver read ktime_get_real_seconds() itself and push it
>> to SET_RTC. Simpler interface (kernel owns the time
>> source; userspace just triggers the sync), no payload to
>> parse, no way for userspace to pass in a wrong value.
>>
>
> How about a combination ? read returns the current value, anything
> written synchronizes with the kernel rtc.
>
Going with that for v5. One naming question before I send:
- set_rtc -- matches the PMBus command name (SET_RTC = 0xDF), but
"set" reads as write-only at first glance, which is a bit
misleading for an RW file.
- since_epoch -- mirrors /sys/class/rtc/<dev>/since_epoch, but
that one is read-only in the RTC subsystem so the dual
semantic might surprise users coming from there.
- rtc_sync -- describes the write semantic directly, but leaves
the read side unnamed.
- rtc -- shortest and most neutral; doesn't bias toward either
operation.
Any preference, or should I just pick?
Thanks,
Abdurrahman
On 5/20/26 14:18, Abdurrahman Hussain wrote:
> On Wed May 20, 2026 at 1:59 PM PDT, Guenter Roeck wrote:
>> On 5/20/26 12:40, Abdurrahman Hussain wrote:
>>> On Wed May 20, 2026 at 10:10 AM PDT, Abdurrahman Hussain wrote:
>>>> On Wed May 20, 2026 at 7:10 AM PDT, Guenter Roeck wrote:
>>>>> Hi,
>>>>>
>>>>> On 5/16/26 18:18, Abdurrahman Hussain wrote:
>>>>>> This is what remains of the v3 series after Guenter applied patches
>>>>>> 1/5 (firmware_revision) and 5/5 (GPIO line label) to hwmon-next, and
>>>>>> asked for patch 4/5 (rtc_class) to be dropped.
>>>>>>
>>>>>> Patch 1 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. The patch also acquires pmbus_lock at the
>>>>>> adm1266_nvmem_read() callback boundary so the memset of
>>>>>> data->dev_mem, the blackbox refill, and the memcpy to userspace run
>>>>>> as a single critical section -- the nvmem core does not serialize
>>>>>> concurrent reg_read calls.
>>>>>>
>>>>>> Patch 2 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. The block-read is taken under
>>>>>> pmbus_lock to serialise against any pmbus_core PAGE+register
>>>>>> sequence on the device.
>>>>>>
>>>>>> Patch 3 takes pmbus_lock in adm1266_state_read() (the pre-existing
>>>>>> sequencer_state debugfs handler) for the same defensive-locking
>>>>>> reason that motivates the new debugfs files in patches 1 and 2:
>>>>>> any direct device access from outside pmbus_core should be ordered
>>>>>> with respect to pmbus_core's own PAGE+register sequences.
>>>>>>
>>>>>> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>>>>> The series no longer applies after applying the bug fix series.
>>>>> Please rebase it on top of the hwmon-next branch of
>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
>>>>> and resubmit.
>>>>>
>>>>> Sorry for the trouble, and thanks a lot for fixing all the problems
>>>>> with the driver.
>>>>>
>>>>> Guenter
>>>> Will do, thank you for your support!
>>>>
>>>> Abdurrahman
>>> Hi Guenter,
>>>
>>> Before I send v5 of the adm1266 series, I'd like to revisit the
>>> SET_RTC exposure question from your v3 review [1].
>>>
>>> The use case I keep landing on is the one the datasheet itself
>>> recommends: a userspace agent (chrony hook, systemd-timesyncd
>>> script, or a small periodic daemon) keeps the chip's seconds
>>> counter aligned with wall-clock so the value embedded in each
>>> blackbox record stays correct across long uptimes. The probe-
>>> time ktime_get_real_seconds() seed (now in hwmon-next) only fixes
>>> this at boot; the chip's counter drifts after that.
>>>
>>> You ruled out rtc_class and called a kernel-side periodic timer
>>> "a bit excessive", which I agree, it is. That leaves a
>>> userspace-triggered sync. Two debugfs shapes I'd consider, both under
>>> pmbus/<hwmon>/adm1266/ (alongside the clear_blackbox,
>>> powerup_counter, and sequencer_state entries v5 already adds):
>>>
>>> A. RW since_epoch -- mirrors /sys/class/rtc/<dev>/since_epoch.
>>> Read returns the chip's SET_RTC seconds counter, write sets
>>> it. The read has the useful side benefit of letting
>>> userspace measure host-vs-chip drift without writing.
>>>
>>> B. Write-only set_rtc -- writing anything to the file makes
>>> the driver read ktime_get_real_seconds() itself and push it
>>> to SET_RTC. Simpler interface (kernel owns the time
>>> source; userspace just triggers the sync), no payload to
>>> parse, no way for userspace to pass in a wrong value.
>>>
>> How about a combination ? read returns the current value, anything
>> written synchronizes with the kernel rtc.
>>
> Going with that for v5. One naming question before I send:
>
> - set_rtc -- matches the PMBus command name (SET_RTC = 0xDF), but
> "set" reads as write-only at first glance, which is a bit
> misleading for an RW file.
> - since_epoch -- mirrors /sys/class/rtc/<dev>/since_epoch, but
> that one is read-only in the RTC subsystem so the dual
> semantic might surprise users coming from there.
> - rtc_sync -- describes the write semantic directly, but leaves
> the read side unnamed.
> - rtc -- shortest and most neutral; doesn't bias toward either
> operation.
>
> Any preference, or should I just pick?
I like rtc_sync the best, but I'll accept your choice.
Thanks,
Guenter
© 2016 - 2026 Red Hat, Inc.