Re: Will there ever be EMC6w201 support?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 11 May 2011 16:28:04 +0200, Luca Tettamanti wrote:
> On Wed, May 11, 2011 at 04:15:45PM +0200, Jean Delvare wrote:
> > On Wed, 11 May 2011 15:48:59 +0200, Luca Tettamanti wrote:
> > > The driver uses lahf to copy the lowest byte of EFLAGS to eax, then
> > > shifts it to right and test the lowest bit (CF).
> > > Using pushf instead we get the whole EFLAGS and test the lowest bit.
> > > Does it make sense? Someone should double check the patch, I don't trust
> > > my assembly skills ;)
> > > 
> > > diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
> > > index d72433f..3554d10 100644
> > > --- a/drivers/char/i8k.c
> > > +++ b/drivers/char/i8k.c
> > > @@ -139,8 +139,8 @@ static int i8k_smm(struct smm_regs *regs)
> > >  		"movl %%edi,20(%%rax)\n\t"
> > >  		"popq %%rdx\n\t"
> > >  		"movl %%edx,0(%%rax)\n\t"
> > > -		"lahf\n\t"
> > > -		"shrl $8,%%eax\n\t"
> > > +		"pusfh\n\t"
> > 
> > I think you meant pushf, not pusfh, right?
> 
> Yeah, right... hand me that brown paper bag ;)
> 
> > 
> > > +		"popl %%eax\n\t"
> > >  		"andl $1,%%eax\n"
> > >  		:"=a"(rc)
> > >  		:    "a"(regs)
> > 
> > I am no asm expert, but I don't get why you removed the shrl
> > instruction. As I understand it, your pushf+popl replaces only lahf,
> > the bit shifting is still needed (I assume the code wants to test bit 8
> > in the flag register.)
> 
> AFAIK lahf loads the lowest byte of EFLAGS into AH which is the upper
> half of AX, hence the shift.

Yes, you're right, of course. So much for my asm skills. Please return
the brown paper bag to me when you're done ;)

> The popl is wrong though, pushf only pushes 2 bytes... so let's try
> again:
> 
> diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
> index d72433f..f71f266 100644
> --- a/drivers/char/i8k.c
> +++ b/drivers/char/i8k.c
> @@ -139,8 +139,8 @@ static int i8k_smm(struct smm_regs *regs)
>  		"movl %%edi,20(%%rax)\n\t"
>  		"popq %%rdx\n\t"
>  		"movl %%edx,0(%%rax)\n\t"
> -		"lahf\n\t"
> -		"shrl $8,%%eax\n\t"
> +		"pushf\n\t"
> +		"pop %%ax\n\t"
>  		"andl $1,%%eax\n"
>  		:"=a"(rc)
>  		:    "a"(regs)

Looks reasonable. I can't test it as I don't have the hardware, but I
have updated the driver at:
  http://khali.linux-fr.org/devel/misc/i8k/
with the above fix. Jeff, Harry, can you test?

Thanks,
-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors


[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux