svn commit: r213737 - head/sys/dev/acpica

Andriy Gapon avg at freebsd.org
Tue Oct 12 17:56:58 UTC 2010


on 12/10/2010 20:53 Andriy Gapon said the following:
> Author: avg
> Date: Tue Oct 12 17:53:01 2010
> New Revision: 213737
> URL: http://svn.freebsd.org/changeset/base/213737
> 
> Log:
>   acpi_ec: changes in communication with hardware
>   
>   Short description of the changes:
>   - attempt to retry some commands for which it is possible (read, query)
>   - always make a short sleep before checking EC status in polled mode
>   - periodically poll EC status in interrupt mode
>   - change logic for detecting broken interrupt delivery and falling back
>     to polled mode
>   - check that EC is ready for input before starting a new command, wait
>     if necessary
>   
>   This commit is based on the original patch by David Naylor.
>   
>   PR:		kern/150517
>   Submitted by:	David Naylor <naylor.b.david at gmail.com>

There also should have been:
Tested by:	David Naylor <naylor.b.david at gmail.com>,
		kuba guzik <kuba.g4 at gmail.com>,
		Nicolas <nicolas.alcouffe at orange.fr>

>   Reviewed by:	jkim
>   MFC after:	3 weeks
> 
> Modified:
>   head/sys/dev/acpica/acpi_ec.c
> 
> Modified: head/sys/dev/acpica/acpi_ec.c
> ==============================================================================
> --- head/sys/dev/acpica/acpi_ec.c	Tue Oct 12 17:40:45 2010	(r213736)
> +++ head/sys/dev/acpica/acpi_ec.c	Tue Oct 12 17:53:01 2010	(r213737)
> @@ -153,7 +153,7 @@ struct acpi_ec_softc {
>      int			ec_glkhandle;
>      int			ec_burstactive;
>      int			ec_sci_pend;
> -    u_int		ec_gencount;
> +    volatile u_int	ec_gencount;
>      int			ec_suspending;
>  };
>  
> @@ -165,7 +165,7 @@ struct acpi_ec_softc {
>  #define EC_LOCK_TIMEOUT	1000
>  
>  /* Default delay in microseconds between each run of the status polling loop. */
> -#define EC_POLL_DELAY	5
> +#define EC_POLL_DELAY	50
>  
>  /* Total time in ms spent waiting for a response from EC. */
>  #define EC_TIMEOUT	750
> @@ -599,12 +599,32 @@ acpi_ec_write_method(device_t dev, u_int
>      return (0);
>  }
>  
> +static ACPI_STATUS
> +EcCheckStatus(struct acpi_ec_softc *sc, const char *msg, EC_EVENT event)
> +{
> +    ACPI_STATUS status;
> +    EC_STATUS ec_status;
> +
> +    status = AE_NO_HARDWARE_RESPONSE;
> +    ec_status = EC_GET_CSR(sc);
> +    if (sc->ec_burstactive && !(ec_status & EC_FLAG_BURST_MODE)) {
> +	CTR1(KTR_ACPI, "ec burst disabled in waitevent (%s)", msg);
> +	sc->ec_burstactive = FALSE;
> +    }
> +    if (EVENT_READY(event, ec_status)) {
> +	CTR2(KTR_ACPI, "ec %s wait ready, status %#x", msg, ec_status);
> +	status = AE_OK;
> +    }
> +    return (status);
> +}
> +
>  static void
>  EcGpeQueryHandler(void *Context)
>  {
>      struct acpi_ec_softc	*sc = (struct acpi_ec_softc *)Context;
>      UINT8			Data;
>      ACPI_STATUS			Status;
> +    int				retry;
>      char			qxx[5];
>  
>      ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
> @@ -625,7 +645,16 @@ EcGpeQueryHandler(void *Context)
>       * that may arise from running the query from causing another query
>       * to be queued, we clear the pending flag only after running it.
>       */
> -    Status = EcCommand(sc, EC_COMMAND_QUERY);
> +    for (retry = 0; retry < 2; retry++) {
> +	Status = EcCommand(sc, EC_COMMAND_QUERY);
> +	if (ACPI_SUCCESS(Status))
> +	    break;
> +	if (EcCheckStatus(sc, "retr_check",
> +	    EC_EVENT_INPUT_BUFFER_EMPTY) == AE_OK)
> +	    continue;
> +	else
> +	    break;
> +    }
>      sc->ec_sci_pend = FALSE;
>      if (ACPI_FAILURE(Status)) {
>  	EcUnlock(sc);
> @@ -678,7 +707,7 @@ EcGpeHandler(void *Context)
>       * address and then data values.)
>       */
>      atomic_add_int(&sc->ec_gencount, 1);
> -    wakeup(&sc->ec_gencount);
> +    wakeup(&sc);
>  
>      /*
>       * If the EC_SCI bit of the status register is set, queue a query handler.
> @@ -789,68 +818,27 @@ EcSpaceHandler(UINT32 Function, ACPI_PHY
>  }
>  
>  static ACPI_STATUS
> -EcCheckStatus(struct acpi_ec_softc *sc, const char *msg, EC_EVENT event)
> -{
> -    ACPI_STATUS status;
> -    EC_STATUS ec_status;
> -
> -    status = AE_NO_HARDWARE_RESPONSE;
> -    ec_status = EC_GET_CSR(sc);
> -    if (sc->ec_burstactive && !(ec_status & EC_FLAG_BURST_MODE)) {
> -	CTR1(KTR_ACPI, "ec burst disabled in waitevent (%s)", msg);
> -	sc->ec_burstactive = FALSE;
> -    }
> -    if (EVENT_READY(event, ec_status)) {
> -	CTR2(KTR_ACPI, "ec %s wait ready, status %#x", msg, ec_status);
> -	status = AE_OK;
> -    }
> -    return (status);
> -}
> -
> -static ACPI_STATUS
>  EcWaitEvent(struct acpi_ec_softc *sc, EC_EVENT Event, u_int gen_count)
>  {
> +    static int	no_intr = 0;
>      ACPI_STATUS	Status;
> -    int		count, i, slp_ival;
> +    int		count, i, need_poll, slp_ival;
>  
>      ACPI_SERIAL_ASSERT(ec);
>      Status = AE_NO_HARDWARE_RESPONSE;
> -    int need_poll = cold || rebooting || ec_polled_mode || sc->ec_suspending;
> -    /*
> -     * The main CPU should be much faster than the EC.  So the status should
> -     * be "not ready" when we start waiting.  But if the main CPU is really
> -     * slow, it's possible we see the current "ready" response.  Since that
> -     * can't be distinguished from the previous response in polled mode,
> -     * this is a potential issue.  We really should have interrupts enabled
> -     * during boot so there is no ambiguity in polled mode.
> -     *
> -     * If this occurs, we add an additional delay before actually entering
> -     * the status checking loop, hopefully to allow the EC to go to work
> -     * and produce a non-stale status.
> -     */
> -    if (need_poll) {
> -	static int	once;
> -
> -	if (EcCheckStatus(sc, "pre-check", Event) == AE_OK) {
> -	    if (!once) {
> -		device_printf(sc->ec_dev,
> -		    "warning: EC done before starting event wait\n");
> -		once = 1;
> -	    }
> -	    AcpiOsStall(10);
> -	}
> -    }
> +    need_poll = cold || rebooting || ec_polled_mode || sc->ec_suspending;
>  
>      /* Wait for event by polling or GPE (interrupt). */
>      if (need_poll) {
>  	count = (ec_timeout * 1000) / EC_POLL_DELAY;
>  	if (count == 0)
>  	    count = 1;
> +	DELAY(10);
>  	for (i = 0; i < count; i++) {
>  	    Status = EcCheckStatus(sc, "poll", Event);
>  	    if (Status == AE_OK)
>  		break;
> -	    AcpiOsStall(EC_POLL_DELAY);
> +	    DELAY(EC_POLL_DELAY);
>  	}
>      } else {
>  	slp_ival = hz / 1000;
> @@ -869,34 +857,37 @@ EcWaitEvent(struct acpi_ec_softc *sc, EC
>  	 * EC query).
>  	 */
>  	for (i = 0; i < count; i++) {
> -	    if (gen_count != sc->ec_gencount) {
> -		/*
> -		 * Record new generation count.  It's possible the GPE was
> -		 * just to notify us that a query is needed and we need to
> -		 * wait for a second GPE to signal the completion of the
> -		 * event we are actually waiting for.
> -		 */
> -		gen_count = sc->ec_gencount;
> -		Status = EcCheckStatus(sc, "sleep", Event);
> -		if (Status == AE_OK)
> -		    break;
> +	    if (gen_count == sc->ec_gencount)
> +		tsleep(&sc, 0, "ecgpe", slp_ival);
> +	    /*
> +	     * Record new generation count.  It's possible the GPE was
> +	     * just to notify us that a query is needed and we need to
> +	     * wait for a second GPE to signal the completion of the
> +	     * event we are actually waiting for.
> +	     */
> +	    Status = EcCheckStatus(sc, "sleep", Event);
> +	    if (Status == AE_OK) {
> +		if (gen_count == sc->ec_gencount)
> +		    no_intr++;
> +		else
> +		    no_intr = 0;
> +		break;
>  	    }
> -	    tsleep(&sc->ec_gencount, PZERO, "ecgpe", slp_ival);
> +	    gen_count = sc->ec_gencount;
>  	}
>  
>  	/*
>  	 * We finished waiting for the GPE and it never arrived.  Try to
>  	 * read the register once and trust whatever value we got.  This is
> -	 * the best we can do at this point.  Then, force polled mode on
> -	 * since this system doesn't appear to generate GPEs.
> +	 * the best we can do at this point.
>  	 */
> -	if (Status != AE_OK) {
> +	if (Status != AE_OK)
>  	    Status = EcCheckStatus(sc, "sleep_end", Event);
> -	    device_printf(sc->ec_dev,
> -		"wait timed out (%sresponse), forcing polled mode\n",
> -		Status == AE_OK ? "" : "no ");
> -	    ec_polled_mode = TRUE;
> -	}
> +    }
> +    if (!need_poll && no_intr > 10) {
> +	device_printf(sc->ec_dev,
> +	    "not getting interrupts, switched to polled mode\n");
> +	ec_polled_mode = 1;
>      }
>      if (Status != AE_OK)
>  	    CTR0(KTR_ACPI, "error: ec wait timed out");
> @@ -933,6 +924,14 @@ EcCommand(struct acpi_ec_softc *sc, EC_C
>  	return (AE_BAD_PARAMETER);
>      }
>  
> +    /*
> +     * Ensure empty input buffer before issuing command.
> +     * Use generation count of zero to force a quick check.
> +     */
> +    status = EcWaitEvent(sc, EC_EVENT_INPUT_BUFFER_EMPTY, 0);
> +    if (ACPI_FAILURE(status))
> +	return (status);
> +
>      /* Run the command and wait for the chosen event. */
>      CTR1(KTR_ACPI, "ec running command %#x", cmd);
>      gen_count = sc->ec_gencount;
> @@ -955,24 +954,31 @@ EcRead(struct acpi_ec_softc *sc, UINT8 A
>  {
>      ACPI_STATUS	status;
>      u_int gen_count;
> +    int retry;
>  
>      ACPI_SERIAL_ASSERT(ec);
>      CTR1(KTR_ACPI, "ec read from %#x", Address);
>  
> -    status = EcCommand(sc, EC_COMMAND_READ);
> -    if (ACPI_FAILURE(status))
> -	return (status);
> +    for (retry = 0; retry < 2; retry++) {
> +	status = EcCommand(sc, EC_COMMAND_READ);
> +	if (ACPI_FAILURE(status))
> +	    return (status);
>  
> -    gen_count = sc->ec_gencount;
> -    EC_SET_DATA(sc, Address);
> -    status = EcWaitEvent(sc, EC_EVENT_OUTPUT_BUFFER_FULL, gen_count);
> -    if (ACPI_FAILURE(status)) {
> -	device_printf(sc->ec_dev, "EcRead: failed waiting to get data\n");
> -	return (status);
> +	gen_count = sc->ec_gencount;
> +	EC_SET_DATA(sc, Address);
> +	status = EcWaitEvent(sc, EC_EVENT_OUTPUT_BUFFER_FULL, gen_count);
> +	if (ACPI_FAILURE(status)) {
> +	    if (EcCheckStatus(sc, "retr_check",
> +		EC_EVENT_INPUT_BUFFER_EMPTY) == AE_OK)
> +		continue;
> +	    else
> +		break;
> +	}
> +	*Data = EC_GET_DATA(sc);
> +	return (AE_OK);
>      }
> -    *Data = EC_GET_DATA(sc);
> -
> -    return (AE_OK);
> +    device_printf(sc->ec_dev, "EcRead: failed waiting to get data\n");
> +    return (status);
>  }
>  
>  static ACPI_STATUS
> @@ -992,7 +998,7 @@ EcWrite(struct acpi_ec_softc *sc, UINT8 
>      EC_SET_DATA(sc, Address);
>      status = EcWaitEvent(sc, EC_EVENT_INPUT_BUFFER_EMPTY, gen_count);
>      if (ACPI_FAILURE(status)) {
> -	device_printf(sc->ec_dev, "EcRead: failed waiting for sent address\n");
> +	device_printf(sc->ec_dev, "EcWrite: failed waiting for sent address\n");
>  	return (status);
>      }
>  


-- 
Andriy Gapon


More information about the svn-src-all mailing list