Re: [PATCH] ACPI CMOS region support rev. 6
On 06/25/2015 10:33 PM, Moore, Robert wrote: > How do you handle the fact that there are three different CMOS/RTC devices > defined in the ACPI specification? (There are three different _HIDs). Right, each of the three CMOS/RTC devices in the ACPI spec has a different ACPI ID (PNP0B00, PNP0B01, PNP0B02). FreeBSD's CMOS RTC driver only attaches to PNP0B00; if a different CMOS device is present, there will be no CMOS/RTC driver (since we don't know how to talk to it). Should be a no-op, like for any unsupported device. If I had the specs for each of those devices, I could probably cobble together support for the other two, but that wasn't my original mission... Anthony > >> -Original Message- >> From: owner-freebsd-a...@freebsd.org [mailto:owner-freebsd- >> a...@freebsd.org] On Behalf Of Anthony Jenkins >> Sent: Thursday, June 25, 2015 6:16 PM >> To: Ian Smith; Warner Losh >> Cc: freebsd-acpi@freebsd.org >> Subject: Re: [PATCH] ACPI CMOS region support rev. 6 >> >> ...and of course I forget the attachment... >> >> On 06/25/15 21:14, Anthony Jenkins wrote: >>> Sooo here's the new and improved rev. 6, "new and improved" because it >>> increases the diff line count by 332%. >>>> [ajenkins@ajenkins-hplaptop /usr/src]$ wc -l atrtc_c_rev5.diff >>>> atrtc_rev6.diff >>>> 220 atrtc_c_rev5.diff >>>> 731 atrtc_rev6.diff >>> This is to satisfy the request to split the atrtc.c driver into a "core" >>> part and "bus" parts: >>>> Looking at patch 5: >>>> >>>> You need to rework this so there's an atrtc_acpi.c. Put all the ACPI >> attachment in there. You should also split off the little bit that's ISA- >> specific into atrtc_isa. Once you do that, we can talk. >>>> Warner >>> I actually finished this patch a couple months ago and have been >>> running it on my laptop, I just don't see the point...maybe Warner >>> could elaborate on the rationale for his request? At the very least, >>> this should be two commits - the functional change and the >>> refactorization request. >>> >>> I may have also added Ian's request for verbosity tweaks, but it's >>> been a while... I don't see any CMOS logging noise FWIW. >>> >>> Thanks, >>> Anthony >>> >>> > ___ > freebsd-acpi@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-acpi > To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org" ___ freebsd-acpi@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
RE: [PATCH] ACPI CMOS region support rev. 6
Ok, thanks. Just thought I should mention it. > -Original Message- > From: Anthony Jenkins [mailto:anthony.b.jenk...@att.net] > Sent: Friday, June 26, 2015 7:46 AM > To: Moore, Robert; Ian Smith; Warner Losh > Cc: freebsd-acpi@freebsd.org > Subject: Re: [PATCH] ACPI CMOS region support rev. 6 > > On 06/25/2015 10:33 PM, Moore, Robert wrote: > > How do you handle the fact that there are three different CMOS/RTC > devices defined in the ACPI specification? (There are three different > _HIDs). > Right, each of the three CMOS/RTC devices in the ACPI spec has a different > ACPI ID (PNP0B00, PNP0B01, PNP0B02). FreeBSD's CMOS RTC driver only > attaches to PNP0B00; if a different CMOS device is present, there will be > no CMOS/RTC driver (since we don't know how to talk to it). Should be a > no-op, like for any unsupported device. > > If I had the specs for each of those devices, I could probably cobble > together support for the other two, but that wasn't my original mission... > > Anthony > > > > >> -Original Message- > >> From: owner-freebsd-a...@freebsd.org [mailto:owner-freebsd- > >> a...@freebsd.org] On Behalf Of Anthony Jenkins > >> Sent: Thursday, June 25, 2015 6:16 PM > >> To: Ian Smith; Warner Losh > >> Cc: freebsd-acpi@freebsd.org > >> Subject: Re: [PATCH] ACPI CMOS region support rev. 6 > >> > >> ...and of course I forget the attachment... > >> > >> On 06/25/15 21:14, Anthony Jenkins wrote: > >>> Sooo here's the new and improved rev. 6, "new and improved" because > >>> it increases the diff line count by 332%. > >>>> [ajenkins@ajenkins-hplaptop /usr/src]$ wc -l atrtc_c_rev5.diff > >>>> atrtc_rev6.diff > >>>> 220 atrtc_c_rev5.diff > >>>> 731 atrtc_rev6.diff > >>> This is to satisfy the request to split the atrtc.c driver into a > "core" > >>> part and "bus" parts: > >>>> Looking at patch 5: > >>>> > >>>> You need to rework this so there's an atrtc_acpi.c. Put all the > >>>> ACPI > >> attachment in there. You should also split off the little bit that's > >> ISA- specific into atrtc_isa. Once you do that, we can talk. > >>>> Warner > >>> I actually finished this patch a couple months ago and have been > >>> running it on my laptop, I just don't see the point...maybe Warner > >>> could elaborate on the rationale for his request? At the very > >>> least, this should be two commits - the functional change and the > >>> refactorization request. > >>> > >>> I may have also added Ian's request for verbosity tweaks, but it's > >>> been a while... I don't see any CMOS logging noise FWIW. > >>> > >>> Thanks, > >>> Anthony > >>> > >>> > > ___ > > freebsd-acpi@freebsd.org mailing list > > http://lists.freebsd.org/mailman/listinfo/freebsd-acpi > > To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org" ___ freebsd-acpi@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
RE: [PATCH] ACPI CMOS region support rev. 6
How do you handle the fact that there are three different CMOS/RTC devices defined in the ACPI specification? (There are three different _HIDs). > -Original Message- > From: owner-freebsd-a...@freebsd.org [mailto:owner-freebsd- > a...@freebsd.org] On Behalf Of Anthony Jenkins > Sent: Thursday, June 25, 2015 6:16 PM > To: Ian Smith; Warner Losh > Cc: freebsd-acpi@freebsd.org > Subject: Re: [PATCH] ACPI CMOS region support rev. 6 > > ...and of course I forget the attachment... > > On 06/25/15 21:14, Anthony Jenkins wrote: > > Sooo here's the new and improved rev. 6, "new and improved" because it > > increases the diff line count by 332%. > >> [ajenkins@ajenkins-hplaptop /usr/src]$ wc -l atrtc_c_rev5.diff > >> atrtc_rev6.diff > >> 220 atrtc_c_rev5.diff > >> 731 atrtc_rev6.diff > > This is to satisfy the request to split the atrtc.c driver into a "core" > > part and "bus" parts: > >> Looking at patch 5: > >> > >> You need to rework this so there's an atrtc_acpi.c. Put all the ACPI > attachment in there. You should also split off the little bit that's ISA- > specific into atrtc_isa. Once you do that, we can talk. > >> > >> Warner > > I actually finished this patch a couple months ago and have been > > running it on my laptop, I just don't see the point...maybe Warner > > could elaborate on the rationale for his request? At the very least, > > this should be two commits - the functional change and the > > refactorization request. > > > > I may have also added Ian's request for verbosity tweaks, but it's > > been a while... I don't see any CMOS logging noise FWIW. > > > > Thanks, > > Anthony > > > > ___ freebsd-acpi@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
Re: [PATCH] ACPI CMOS region support rev. 6
...and of course I forget the attachment...
On 06/25/15 21:14, Anthony Jenkins wrote:
> Sooo here's the new and improved rev. 6, "new and improved" because it
> increases the diff line count by 332%.
>> [ajenkins@ajenkins-hplaptop /usr/src]$ wc -l atrtc_c_rev5.diff
>> atrtc_rev6.diff
>> 220 atrtc_c_rev5.diff
>> 731 atrtc_rev6.diff
> This is to satisfy the request to split the atrtc.c driver into a "core"
> part and "bus" parts:
>> Looking at patch 5:
>>
>> You need to rework this so there’s an atrtc_acpi.c. Put all the ACPI
>> attachment in there. You should also split off the little bit that’s
>> ISA-specific into atrtc_isa. Once you do that, we can talk.
>>
>> Warner
> I actually finished this patch a couple months ago and have been running
> it on my laptop, I just don't see the point...maybe Warner could
> elaborate on the rationale for his request? At the very least, this
> should be two commits - the functional change and the refactorization
> request.
>
> I may have also added Ian's request for verbosity tweaks, but it's been
> a while... I don't see any CMOS logging noise FWIW.
>
> Thanks,
> Anthony
>
>
Index: sys/conf/files.i386
===
--- sys/conf/files.i386 (revision 284815)
+++ sys/conf/files.i386 (working copy)
@@ -577,8 +577,10 @@
x86/iommu/intel_quirks.c optional acpi acpi_dmar pci
x86/iommu/intel_utils.c optional acpi acpi_dmar pci
x86/isa/atpic.c optional atpic
-x86/isa/atrtc.c standard
-x86/isa/clock.c standard
+x86/isa/atrtc.c standard native
+x86/isa/atrtc_isa.c standard isa atrtc
+#x86/isa/atrtc_acpi.c optional atrtc acpi
+x86/isa/clock.c optional native
x86/isa/elcr.c optional atpic | apic
x86/isa/isa.c optional isa
x86/isa/isa_dma.c optional isa
Index: sys/conf/files.amd64
===
--- sys/conf/files.amd64 (revision 284815)
+++ sys/conf/files.amd64 (working copy)
@@ -583,6 +583,8 @@
x86/iommu/intel_utils.c optional acpi acpi_dmar pci
x86/isa/atpic.c optional atpic isa
x86/isa/atrtc.c standard
+x86/isa/atrtc_isa.c standard
+x86/isa/atrtc_acpi.c standard
x86/isa/clock.c standard
x86/isa/elcr.c optional atpic isa | mptable
x86/isa/isa.c standard
Index: sys/x86/isa/atrtc.c
===
--- sys/x86/isa/atrtc.c (revision 284815)
+++ sys/x86/isa/atrtc.c (working copy)
@@ -42,21 +42,24 @@
#include
#include
#include
-#include
#include
#include
+#include
+#include "clock_if.h"
+#include "atrtcvar.h"
+
#ifdef DEV_ISA
#include
#include
#endif
-#include
-#include "clock_if.h"
#define RTC_LOCK do { if (!kdb_active) mtx_lock_spin(&clock_lock); } while (0)
#define RTC_UNLOCK do { if (!kdb_active) mtx_unlock_spin(&clock_lock); } while (0)
-int atrtcclock_disable = 0;
+#define IO_DELAY() (void)inb(0x84)
+#define IO_RTC_ADDR (IO_RTC + 0)
+#define IO_RTC_DATA (IO_RTC + 1)
static int rtc_reg = -1;
static u_char rtc_statusa = RTCSA_DIVIDER | RTCSA_NOPROF;
@@ -73,10 +76,10 @@
RTC_LOCK;
if (rtc_reg != reg) {
- inb(0x84);
+ IO_DELAY();
outb(IO_RTC, reg);
rtc_reg = reg;
- inb(0x84);
+ IO_DELAY();
}
val = inb(IO_RTC + 1);
RTC_UNLOCK;
@@ -89,13 +92,13 @@
RTC_LOCK;
if (rtc_reg != reg) {
- inb(0x84);
+ IO_DELAY();
outb(IO_RTC, reg);
rtc_reg = reg;
- inb(0x84);
+ IO_DELAY();
}
outb(IO_RTC + 1, val);
- inb(0x84);
+ IO_DELAY();
RTC_UNLOCK;
}
@@ -105,7 +108,7 @@
return(bcd2bin(rtcin(port)));
}
-static void
+void
atrtc_start(void)
{
@@ -155,15 +158,7 @@
* RTC driver for subr_rtc
*/
-struct atrtc_softc {
- int port_rid, intr_rid;
- struct resource *port_res;
- struct resource *intr_res;
- void *intr_handler;
- struct eventtimer et;
-};
-
-static int
+int
rtc_start(struct eventtimer *et, sbintime_t first, sbintime_t period)
{
@@ -172,7 +167,7 @@
return (0);
}
-static int
+int
rtc_stop(struct eventtimer *et)
{
@@ -201,7 +196,7 @@
* Stat clock ticks can still be lost, causing minor loss of accuracy
* in the statistics, but the stat clock will no longer stop.
*/
-static int
+int
rtc_intr(void *arg)
{
struct atrtc_softc *sc = (struct atrtc_softc *)arg;
@@ -215,86 +210,7 @@
return(flag ? FILTER_HANDLED : FILTER_STRAY);
}
-/*
- * Attach to the ISA PnP descriptors for the timer and realtime clock.
- */
-static struct isa_pnp_id atrtc_ids[] = {
- { 0x000bd041 /* PNP0B00 */, "AT realtime clock" },
- { 0 }
-};
-
-static int
-atrtc_probe(device_t dev)
-{
- int result;
-
- result = ISA_PNP_PROBE(device_get_parent(dev), dev, atrtc_ids);
- /* ENOENT means no PnP-ID, device is hinted. */
- if (result == ENOENT) {
- device_set_desc(dev, "AT realtime clock");
- return (BUS_PROBE_LOW_PRIORITY);
- }
- return (result);
-}
-
-static int
-atrtc_attach(device_t dev)
-{
- struct atrtc_softc *sc;
- u_long s;
- int i;
-
- sc = device_get_softc(dev);
- sc->port_res = bus_alloc_resource(dev, SYS_
