Page MenuHomeFreeBSD

Add ACPI support for RTC/CMOS device
ClosedPublic

Authored by wulf on Feb 24 2019, 12:40 AM.
Tags
None
Referenced Files
F82072640: D19314.diff
Thu, Apr 25, 6:29 AM
Unknown Object (File)
Sat, Apr 20, 2:31 PM
Unknown Object (File)
Tue, Apr 9, 2:05 PM
Unknown Object (File)
Thu, Mar 28, 5:26 PM
Unknown Object (File)
Mar 2 2024, 7:39 PM
Unknown Object (File)
Feb 15 2024, 11:00 PM
Unknown Object (File)
Dec 20 2023, 5:16 AM
Unknown Object (File)
Dec 12 2023, 10:36 PM

Details

Summary

This is an attempt to resurrect PR/207419 by Anthony Jenkins [1]

Implementation of ACPI RTC/CMOS interface.

FreeBSD base system does not provide an ACPI handler for the PC/AT RTC/CMOS device with PnP ID PNP0B00; on recent HP laptops, the absence of this handler causes suspend/resume and poweroff(8) to hang/fail, emitting e.g. [2], [3]

ACPI Error: No handler for Region [RCM0] (0xfffff80005796480) [SystemCMOS] (20150818/evregion-176)
ACPI Error: Region SystemCMOS (ID=5) has no handler (20150818/exfldio-317)

These HP laptops have BIOSes which attempt to query the RTC date/time registers via ACPI before suspending/powering off.

ACPI-5.0 specification section 9.15 describes the ACPI interface to the RTC/CMOS device (atrtc(4) in FreeBSD).

Original patch has been discussed in several threads on acpi@ mailing list in 2013-2015 e.g. [4] that finally led to fixing most of concerns. Now I am trying to fix remaining ones.
Improvements over PR version of the patch:

  • ACPI and ISA atrtc(4) drivers do not share common device_t structure anymore so they got different attach/detach methods and ACPI CMOS handler is installed from ACPI driver only. That should help to avoid any problems with ACPI-less installs;
  • ACPI - related code is compiled out if ACPI support is disabled in kernel config. ACPI version of atrtc(4) is compiled out in that case too;
  • Added a knob to enable/disable CMOS handler in ACPI subsystem-friendly way via setting of debug.acpi.disabled loader tunable;
  • CMOS reads and writes are logged through ACPI_VPRINT facility so they do not generate spam on ttyv0 terminal screen by default;

[1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=207419
[2]. https://wiki.freebsd.org/Laptops/HP_Envy_6Z-1100
[3]. https://wiki.freebsd.org/Laptops/HP_Notebook_15-af104ur
[4] http://freebsd.1045724.x6.nabble.com/No-acpi-dell-4-td5991030.html
[5] https://github.com/torvalds/linux/blob/master/drivers/acpi/acpi_cmos_rtc.c

Test Plan

Check that affected HP laptops are able to suspend/resume
Check that 'ACPI Error: No handler for Region [SystemCMOS]' terminal messages are disappeared

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I don't know much about acpi, but from the viewpoint of what I know about the atrtc driver, this looks okay. Using rtcin/out_locked() as I mentioned above would be a small performance enhancement. I don't have a ton of free time these days and may not get around to reviewing a revised diff. I'm okay with making the changes I noted above or not; either way is fine with me.

sys/x86/isa/atrtc.c
271

This probably should be...

mtx_lock_spin(&atrtc_lock);
while (len-- > 0)
    *ptr++ = rtcin_locked(reg++);
mtx_unlock_spin(&atrtc_lock);

Likewise in rtcout_region() below using rtcout_locked().

326

Why does this check bitwidth > 32? That seems to arbitrarily limit cmos access to a max of 4 bytes at a time. The linux driver cited in the comments with this change doens't have any such check. (FWIW, the bios's access to the cmos at susupend/resume time is likely limited to reading a single byte containing the century number to extend the 2-digit value in the rtc regs.)

This revision is now accepted and ready to land.Feb 24 2019, 7:33 PM
wulf added inline comments.
sys/x86/isa/atrtc.c
271

Locking is intentionally de-optimized to raise interrupt handler and ntpd priorities. ACPI is a second class citizen here.

326

Hmmm. I followed all the discussions on this patch on acpi@ list threads dated back to 2014-2015 and was not able find exact origin of this limit. Fortunately original author is subscribed to this review so I can forward this question to him:

Hi Anthony, is it necessary to check "bitwidth > 32" here? What is the reason behind this?

sys/x86/isa/atrtc.c
326

I honestly don't recall...vaguely remember seeing a 32-bit limit somewhere (I think I had originally put it in to simplify debug printf()s), but if there is no technical reason for a limit, then I'm happy to lose it.

This revision was automatically updated to reflect the committed changes.
wulf marked an inline comment as done.