Subject: Re: CVS commit: src
To: Juan RP <juan@xtrarom.org>
From: Quentin Garnier <cube@cubidou.net>
List: source-changes
Date: 08/06/2006 20:53:12
--mjeOt6n4R71vn6wN
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Aug 06, 2006 at 08:26:59PM +0200, Juan RP wrote:
> On Sun, 6 Aug 2006 19:38:41 +0200
> Quentin Garnier <cube@cubidou.net> wrote:
>=20
> > Hmm, why do I get thanked?  Sure I made a few comments after a quick
> > glance at the code but you ignored them apparently.
> >=20
> > I expect this to break on EM64T, but I told you that already...
>=20
> Can you please explain why EM64T won't work?

Because CPUID(0x8???????) is brand-specific...

> > Also, why didn't you try adding support for that on i386, too?  I
> > haven't seen anything in the spec that restricts Cool'n'Quiet to long
> > mode.  And I still feel bitmask_snprintf would be more suitable than
> > your own equivalent.
>=20
> Do we want to have another i386/i386/powernow_k8.c? my goal is
> to share most of the code between drivers.

Well, you committed in amd64.

> You are right that bitmask_snprintf was made for this kind of things,
> but it's not very critical.

Same can be said for a lot of things.

[...]
> >  - you don't free freq_names if k8pnow_current_state is NULL (what if
> >    the PSB doesn't list the current state?  you know vendors...  I
> >    certainly expect one to ship a laptop with a broken PSB but a correct
> >    ACPI table)
>=20
> Fixed.

No.  As I said, Coverity will tell you.

> >  - the spec indicates that FID and VID changes of a dual-core processor
> >    must me co-ordinated.
>=20
> I'll take a look at linux driver.

Or at least find a way to explain the user why the frequency isn't
changing until the other core was changed (when frequency is going
down), and such.  I'm not sure if it might make the code think it was
stuck, though.

> > Glad I could help.  Please have your code reviewed on tech-kern before
> > you commit.
>=20
> Yes, I try to do that... but sometimes you wait for a review that never h=
appens.

*Then* it means you can commit.  What's one week of waiting for such a
change?  There's no hurry, especially when we're in the middle of
intending to branch or something...

--=20
Quentin Garnier - cube@cubidou.net - cube@NetBSD.org
"When I find the controls, I'll go where I like, I'll know where I want
to be, but maybe for now I'll stay right here on a silent sea."
KT Tunstall, Silent Sea, Eye to the Telescope, 2004.

--mjeOt6n4R71vn6wN
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (NetBSD)

iQEVAwUBRNY6mNgoQloHrPnoAQKx8wgAqFViwixcmFN6b/gHt9gnxO99mnxNSAPz
plc0I1d7GUnZFHQEF1lX8KK9XZppRjZmH96bMb8xGVk+YUtQZtdi8GGfe3MtwpWk
dbc12u4uMYzUjIQCL2SmEuESuMHwol4d+HrBCw/YlSDEa8Gv2WFbtmW8U1QqSoy0
0H0Y6paepYZ8edFocyt8K+no239Zv9C6WOviTf7aFDUjE6uqwpJV3ccU0QS1PQV6
imy2UvttaenmpAMMGrm10YrxgNq4eVGCX45LFtnzbFNS2RNqA8pzRBKzPyzO+dFw
dOt3ClIDhwYMy2E0idJ7Gx0hdxZn3tjYVmFGPiQ81e/W97QyCR4Kig==
=R2sN
-----END PGP SIGNATURE-----

--mjeOt6n4R71vn6wN--