Re: [PATCH] powernow-k8 max speed sanity check

[Pavel Machek <pavel@suse.cz>]

Hi!

> Here's some comments after trying out your 2 patches. 
> 
> First, the PCMCIA patch worked great, I can now plug/unplug power cord with
> yenta_socket loaded :) Maybe now I can _haul_ this laptop to a cafe and use 
> the WLAN, hehe. In case anybody else needs them, I'll put the
> patches 

Good.

> The powernow-k8.c patch did not work, as my numpst is 8, not 3. So why not
> just ignore the numpst, as it is not used?

What are the BIOS people smoking? I thought they have left some old
tables there, but they keep updating them...


> Maybe replace this
> 
>  		if (psb->numpst != 1) {
>  			printk(KERN_ERR BFX "numpst must be 1\n");
> -			return -ENODEV;
> +			if (psb->numpst == 3) {
> +				printk(KERN_INFO PFX "assuming arima notebug\n");
> +				arima = 1;
> +			} else
> +				return -ENODEV;
>  		}
> 
> With this instead:
>  
>  		dprintk(KERN_DEBUG PFX "numpst: 0x%x\n", psb->numpst);
>  		if (psb->numpst != 1) {
> -			printk(KERN_ERR BFX "numpst must be 1\n");
> -			return -ENODEV;
> +			printk(KERN_WARNING BFX "numpst listed as %i "
> +			       "should be 1. Using 1.\n", psb->numpst);
>  		}

Well, I wanted some way to detect exactly this broken machine. You
might want to simply put == 8 there.. but proper solution is DMI blacklist. 

> I see a little problem with hardcoding the values:
> 
> +		if (arima) {
> +			ppst[1].fid = 0x8;
> +			ppst[1].vid = 0x6;
> +#ifdef THREE
> +			ppst[2].fid = 0xa;
> +			ppst[2].vid = 0x2;
> +#endif
>  		}
> 
> This would fail if I upgraded my CPU, right? 

Yes.

> What do you think about using module options maxfid and maxvid?

Well, the original BIOS has not only maximum values wrong, but also
1600MHz wrong, as far as I can tell...

Something like /proc/frequencies file would be needed where you could

echo "0 0xa; 0x8 0x6; 0xa 0x2" > /proc/frequencies to override. You
need to override all of them, not just top one.
							Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]