Messages in this thread |  | | Date | Sun, 1 Jun 2003 05:16:11 +0300 | From | Pasi Savolainen <> | Subject | Re: [PATCH] amd76x_pm port to 2.5.70 |
| |
* Dave Jones <davej@codemonkey.org.uk>: > On Sat, May 31, 2003 at 09:33:21PM +0300, Pasi Savolainen wrote: > > + /* Clear W4SG, and set PMIOEN, if using a 765/766 set STPGNT as well. > > + * AMD-766: C3A41; page 59 in AMD-766 doc > > + * AMD-768: DevB:3x41C; page 94 in AMD-768 doc */ > > + pci_read_config_byte(pdev_sb, 0x41, ®byte); > > + if(enable) { > > + regbyte |= ((0 << 0) | (is_766?1:0 << 1) | (1 << 7)); > ^^^^^^ > > This looks totally bogus. If you want that bit clearing, you need > to AND its inverse, not OR it. The second statement also looks a > bit funny to the eye.
AFAICT, it's 'for future expansion', NMC. I'll trim it down. Either way, that enable seems to work, at least on my machine. As I don't know much about it, I can't say what's right/wrong. (I don't have the spec. Only code that does desired thing)
> > +static void > > +amd76x_smp_idle(void) > > +{ > > + /* > > + * Exit idle mode immediately if the CPU does not change. > > + * Usually that means that we have some load on another CPU. > > + */ > > + > > + if (prs[0].idle && prs[1].idle && amd76x_pm_cfg.last_pr == smp_processor_id()) { > > + prs[0].idle = 0; > > + prs[1].idle = 0; > > + /* This looks redundent as it was just checked in the if() */ > > + /* amd76x_pm_cfg.last_pr = smp_processor_id(); */ > > except with preemption, it may have changed. This needs to be fixed, > as noted in the function header. Either get_cpu(),put_cpu() or explicit > preempt_enable/disable()
This is idle loop, like we're not doing anything, and if somebody is to preempt us, then it sure means we're not supposed to idle.
Second point is (I'm _really_ on thin ground here..) that if we get_cpu() here, and other CPU is sleeping, then it'd be woken up sooner, for cache will need syncing.
> > +/* > > + * Info exported through "/proc/driver/amd76x_pm" > > This should really be using sysfs. Adding extra junk to /proc is > somewhat frowned upon, and with the other PM stuff now living in > sysfs, that seems to be the way forward.
done.
> > > +static void __exit > > +amd76x_pm_cleanup(void) > > +{ > > +#ifndef AMD76X_NTH > > + pm_idle = amd76x_pm_cfg.orig_idle; > > + wmb(); > > + //__asm__ __volatile__ ("wbinvd;"); // propagate through SMP > > Not sure why this was there, is it noted in the 766 spec to flush > the cache ? if so, it needs to be reenabled, and used with on_each_cpu()
Zwane told me to use synchronize_kernel(); it cured my crashes on rmmod amd76x_pm. -> hack removed.
> Sounds like possible candidate for something that should be exposed > via sysfs.
done.
Iteration n at <http://varg.dyndns.org/psi/files/misc/amd76x_pm-2.5.70.patch.bz2>
/sys/devices/pci0/00:00.0/C2_cnt (ro) number of C2 calls made. /sys/devices/pci0/00:00.0/lazy_idle (rw) number of successfull idle calls to be made before entering C2.
-- Psi -- <http://www.iki.fi/pasi.savolainen> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |