Re: applesmc oops in 3.10/3.11

[Guenter Roeck]

On Tue, Oct 01, 2013 at 12:55:26PM +0200, Henrik Rydberg wrote:
> > >Warning message triggered with 3.12.0-0.rc3.git0.1.fc21.x86_64.
> > >
> > >[   10.886016] applesmc: key count changed from 261 to 1174405121
> > >
> > 
> > Explains the crash, but the new key count is very wrong. 1174405121 = 0x46000001.
> > Which I guess explains the subsequent memory allocation error in the log.
> > 
> > Henrik, any idea what might be going on ? Is it possible that the previous
> > command failure leaves some state machine in a bad state ?
> 
> I seem to recall a report on another similar state problem on newer
> machines, so maybe, yes. Older machines seem fine, I have never
> encountered the problem myself. Here is a patch to test that
> theory. It has been tested to be pretty harmless on two different
> generations.
> 
> I really really do not want to add an 'if (value is insane)' check ;-)
> 
Chris,

any chance you can load this patch on an affected machine so we can get
test feedback ? This one is too experimental to submit upstream without
knowing that it really fixes the problem.

Thanks,
Guenter

> Thanks,
> Henrik
> 
> From d48a9e4e6e45dcd9c7e7ad88df714b92772a62d6 Mon Sep 17 00:00:00 2001
> From: Henrik Rydberg <rydberg@xxxxxxxxxxx>
> Date: Tue, 1 Oct 2013 12:16:09 +0200
> Subject: [PATCH] applesmc remedy take 1
> 
> Conjectured problem: there are remnant bytes ready on the data line
> which corrupts the read after a failure.
> 
> Remedy: assuming bit0 is the read valid line, try to flush it before
> starting a new command.
> 
> Exception: the write-number-of-bytes-to-read command seems to differ
> between models (it may not be needed on the newest), so do not try to
> flush the data at that particular point.
> 
> Tested on a MacBookPro10,1 and a MacBookAir3,1, but the original problem
> has not been reproduced, so the actual effect of this patch is unknown.
> ---
>  drivers/hwmon/applesmc.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
> index 98814d1..f6eaf6d1 100644
> --- a/drivers/hwmon/applesmc.c
> +++ b/drivers/hwmon/applesmc.c
> @@ -186,10 +186,23 @@ static int wait_read(void)
>   * send_byte - Write to SMC port, retrying when necessary. Callers
>   * must hold applesmc_lock.
>   */
> -static int send_byte(u8 cmd, u16 port)
> +static int send_byte(u8 cmd, u16 port, bool flush)
>  {
> -	u8 status;
> -	int us;
> +	u8 status, data;
> +	int us, nskip;
> +
> +	if (flush) {
> +		/* read the data port until bit0 is cleared */
> +		for (nskip = 0; nskip < 16; nskip++) {
> +			udelay(APPLESMC_MIN_WAIT);
> +			status = inb(APPLESMC_CMD_PORT);
> +			if (!(status & 0x01))
> +				break;
> +			data = inb(APPLESMC_DATA_PORT);
> +		}
> +		if (nskip)
> +			pr_warn("flushed %d bytes\n", nskip);
> +	}
>  
>  	outb(cmd, port);
>  	for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) {
> @@ -215,7 +228,7 @@ static int send_byte(u8 cmd, u16 port)
>  
>  static int send_command(u8 cmd)
>  {
> -	return send_byte(cmd, APPLESMC_CMD_PORT);
> +	return send_byte(cmd, APPLESMC_CMD_PORT, true);
>  }
>  
>  static int send_argument(const char *key)
> @@ -223,7 +236,7 @@ static int send_argument(const char *key)
>  	int i;
>  
>  	for (i = 0; i < 4; i++)
> -		if (send_byte(key[i], APPLESMC_DATA_PORT))
> +		if (send_byte(key[i], APPLESMC_DATA_PORT, true))
>  			return -EIO;
>  	return 0;
>  }
> @@ -237,7 +250,7 @@ static int read_smc(u8 cmd, const char *key, u8 *buffer, u8 len)
>  		return -EIO;
>  	}
>  
> -	if (send_byte(len, APPLESMC_DATA_PORT)) {
> +	if (send_byte(len, APPLESMC_DATA_PORT, false)) {
>  		pr_warn("%.4s: read len fail\n", key);
>  		return -EIO;
>  	}
> @@ -262,13 +275,13 @@ static int write_smc(u8 cmd, const char *key, const u8 *buffer, u8 len)
>  		return -EIO;
>  	}
>  
> -	if (send_byte(len, APPLESMC_DATA_PORT)) {
> +	if (send_byte(len, APPLESMC_DATA_PORT, true)) {
>  		pr_warn("%.4s: write len fail\n", key);
>  		return -EIO;
>  	}
>  
>  	for (i = 0; i < len; i++) {
> -		if (send_byte(buffer[i], APPLESMC_DATA_PORT)) {
> +		if (send_byte(buffer[i], APPLESMC_DATA_PORT, true)) {
>  			pr_warn("%s: write data fail\n", key);
>  			return -EIO;
>  		}
> -- 
> 1.8.3.4
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/