[lm-sensors] [Module] hwmon: GM/GME965 GM45 and more chips IGP thermal report

Jean Delvare khali at linux-fr.org
Thu Oct 8 22:02:15 CEST 2009


Hi Lu,

On Sat, 18 Jul 2009 11:40:36 +0000, Lu Zhihe wrote:
> Dear all,
> 
>     Attachment is the updated version gm965temp module and patched
> "sensors-detect.gm965temp".
>     For some reasons about safe consideration, gm965temp is not
> considered to merge to hwmon subsystem.
>     So, it is packaged as external module.

Can you please elaborate about it? Don't you ever plan to get your
driver included in the kernel tree?

If your driver won't ever be merged, I admit I am not interested in
reviewing it. If the plan is to merge it, then I will review it.

>     Now, it supported:
>     PM/GM/GME965, PM/GM45(need tester :-). These chips read temp from
> register TR1/RTR1 .
> 
>     Following Chip sets:
>     G33/Q33/Q35 G41 G43/Q43 G45/Q45,
>     B43_BASE and B43_SOFT_SKU.
>     They all need testers!                      These chips read temp
> from register TSTTP.RELT .
> 
>     Thanks for Tobias Hain helping me test gm965temp.
> 
>     Hope someone can help to test it too, thank you very much!

I can't test your code, I do not have any supporter piece of hardware,
sorry.

I've looked at the intel-agp driver, which binds to the same PCI
devices. It's way too big and complex to resort to the same hack we
have in sis5595 and i2c-sis5595 for example. The intel-agp driver must
remain a "real" pci driver which binds to its devices, in particular so
that power management works OK.

This leaves us with 3 alternatives.

1* Add hwmon support to the intel-agp driver directly. As this driver
   is already somewhat big (77 kB) it would probably go unnoticed. The
   hwmon part in your driver is probably about 350 lines of code.

2* Implement hwmon as a separate module, simply use pci_get_dev()
   during initialization of that module, and if a device is found,
   register a platform driver and instantiate a platform device for the
   hwmon part. This is essentially what your implementation does,
   except that your module registers the platform driver and device
   regardless of the presence of a supported chip, which is bad.

3* Let intel-agp instantiate the platform device for hwmon, and only
   register the driver in the separate module. This avoid some code
   duplication, and we can have the hwmon module auto-load. Maybe the
   best compromise code-wise, but possibly somewhat more difficult to
   maintain.

Note that driver auto-loading should also work with option #2, although
your implementation doesn't do it. It shouldn't be too hard to add.

I don't have a strong opinion between these 3 choices, but I want the
driver to auto-load as needed.

-- 
Jean Delvare




More information about the lm-sensors mailing list