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-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"

Reply via email to