Date: Fri, 06 Mar 2015 11:37:58 -0500 From: Anthony Jenkins <Anthony.B.Jenkins@att.net> To: Ian Smith <smithi@nimnet.asn.au> Cc: freebsd-acpi@freebsd.org Subject: Re: [PATCH] ACPI CMOS region support rev. 3 [was Re: No acpi_dell(4)] Message-ID: <54F9D7E6.4050807@att.net> In-Reply-To: <20150306025800.U46361@sola.nimnet.asn.au> References: <20150222180817.GD27984@strugglingcoder.info> <54EB8C21.2080600@att.net> <2401337.2oUs7iAbtB@ralph.baldwin.cx> <54EF3D5D.4010106@att.net> <20150227222203.P38620@sola.nimnet.asn.au> <20150228125857.D1277@besplex.bde.org> <54F14368.4020807@att.net> <20150302002647.W42658@sola.nimnet.asn.au> <54F5E53D.1090601@att.net> <20150306025800.U46361@sola.nimnet.asn.au>
next in thread | previous in thread | raw e-mail | index | archive | help
On 03/06/2015 06:49 AM, Ian Smith wrote:
> On Tue, 3 Mar 2015 11:45:49 -0500, Anthony Jenkins wrote:
> > On 03/01/2015 09:29 AM, Ian Smith wrote:
> [..]
> Regarding systems without ACPI loaded, or active: what happens when the
> below AcpiInstallAddressSpaceHandler() call fails, but returns 0? Would
> not that prevent rtc_start() from running atrtc_start() etc for non-ACPI
> clock initialisation and registration?
Good catch, there's technically no reason to bail on rtc_start() if I
fail to register the ACPI CMOS handler; it'll just never get called
(same as old behaviour). I'll change "Error" to "Warning" and remove
the return 0.
> I suppose there's a global kernel variable for acpi_is_active ono?
>
> > +static int
> > rtc_start(struct eventtimer *et, sbintime_t first, sbintime_t period)
> > {
> >
> > @@ -245,10 +323,17 @@
> > int i;
> >
> > sc = device_get_softc(dev);
> > + sc->acpi_handle = acpi_get_handle(dev);
> > sc->port_res = bus_alloc_resource(dev, SYS_RES_IOPORT, &sc->port_rid,
> > IO_RTC, IO_RTC + 1, 2, RF_ACTIVE);
> > if (sc->port_res == NULL)
> > device_printf(dev, "Warning: Couldn't map I/O.\n");
> > + if (ACPI_FAILURE(AcpiInstallAddressSpaceHandler(sc->acpi_handle,
> > + ACPI_ADR_SPACE_CMOS, acpi_rtc_cmos_handler, NULL, sc)))
> > + {
> > + device_printf(dev, "Error registering ACPI CMOS address space handler.\n");
> > + return 0;
> > + }
> > atrtc_start();
> > clock_register(dev, 1000000);
> > bzero(&sc->et, sizeof(struct eventtimer));
> > @@ -286,6 +371,15 @@
> > return(0);
> > }
>
> Might that not matter for detach, as you're not testing for return code?
Yeah I'd prefer to remember the failure to install the handler and
conditionally uninstall it in the detach method.
I'll fix up the logging and add these suggestions this weekend.
Thanks,
Anthony
> > +static int atrtc_detach(device_t dev)
> > +{
> > + struct atrtc_softc *sc;
> > +
> > + sc = device_get_softc(dev);
> > + AcpiRemoveAddressSpaceHandler(sc->acpi_handle,
> > ACPI_ADR_SPACE_CMOS, acpi_rtc_cmos_handler);
> > + return bus_generic_detach(dev);
> > +}
>
> cheers, Ian
> _______________________________________________
> freebsd-acpi@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
> To unsubscribe, send any mail to "freebsd-acpi-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54F9D7E6.4050807>