Re: [PATCH] hwmon : Fix W83627THF VID reading

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

 



Hi Yuan,

> This patch fixes the VID reading; no cpu0_vid and vrm files created if
> the chip is w83627thf and GPIO5 not enabled.
> Please check.

Looks good to me. I had to fix the patch manually this time again though.

I have done three minor edits to your patch, I hope you won't mind:

1* I think the following test:
+	if (kind != w83697hf &&
+		!(kind == w83627thf && data->vid == 0xff)) {
Can be simplified to:
+	if (kind != w83697hf && data->vid != 0xff)) {
Because data->vid can only have value 0xff for type w83627thf anyway.

2* In w83627thf_read_gpio5, I did:
-	sel = superio_inb(W83627THF_GPIO5_IOSR);
+	sel = superio_inb(W83627THF_GPIO5_IOSR) & 0x3f;
The datasheet suggests that the two unused bits of register CRF4
(W83627THF_GPIO5_DR) are always zero, but better safe than sorry.

3* I dropped "default" in that comment:
        /* Convert VID to voltage based on default VRM */
It's not more a default value, it's deduced from the CPU model.

I'll stack your patch and will get it into -mm soon so that it gets some
testing. It should then go into mainline for 2.6.16. Thanks for your
contribution!

While checking the W83627THF datasheet, I noticed that there is an
additional GPIO configuration register, CRF5, named "inversion
register". Wouldn't it make sense to check the value of this register
and make sure all pins supposedly used as VID inputs are not inverted?
Mark, can you please confirm that your system which does use GPIO5 for
VID would pass that test? Proposed patch attached, comments and testers
welcome.

Thanks,
--
Jean Delvare
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: hwmon-w83627thf-improve-gpio5-test.patch.txt
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20051115/54a22370/attachment.txt 


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

  Powered by Linux