Re: hwmon API update

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

 



Le 13/02/2011 18:16, Guenter Roeck a écrit :
On Sun, Feb 13, 2011 at 07:18:44AM -0500, Martin Peres wrote:
Hi,

I am working on power management on the nouveau driver and I need a way
to get data out of and send commands to the i2c drivers from the kernel
space.

We can already change the clocks of the card, but we need a way to
monitor the temperature and bump the fan speed if needed.
Another problem with letting users mess with the i2c driver by
themselves is that some cards use the i2c driver for fan management
while others don't. This is why I would like to introduce nouveau as an
hwmon driver, exporting the temperature, fan management and clock speeds
so as we can use the thermal zone to monitor the temperature and react
when needed.

So far, we use:
- w83l785ts
- w83781d
- adt7473 (most common one)
- f75375
- lm99

With the help of Matthew Garret, I updated his previous proposal for an
in-kernel API for hwmon. The patch should apply cleanly on Linux
2.6.38-rc4. This patch only provides the API, no modification to the
drivers has been completed yet.

Looking forward to your review and feedback.

Martin
 From 059b647b7b8bd98c04cf48b4062048b8ae963593 Mon Sep 17 00:00:00 2001
From: Martin Peres<martin.peres@xxxxxxxxxxxxxxx>
Date: Sun, 13 Feb 2011 11:35:17 +0100
Subject: [PATCH] hwmon API update

Original creator: Matthew Garrett<mjg@xxxxxxxxxx>

Signed-off-by: Martin Peres<martin.peres@xxxxxxxxxxxxxxx>
This is an extremely complex change just for the benefit of one driver,
with a huge potential of misuse. The changes required in each driver
to actually implement the API are substantial, and pretty much only add
complexity to each hwmon driver with no real benefit.

The cost gets even larger if one has to consider that some may want or
have to to backport drivers to earlier kernel versions. This patchset
would result in significant efforts to do such backports.

For the API itself, there are lots of functions with similar parameters,
and those parameters are needed in the drivers to determine which attribute
is affected. A single function would have accomplished the same, as the drivers
will need case statements anyway to identify the actual attribute to be read
or written. What we end up here with is a large number of functions to be
supported by each driver, all with pretty much the same set of arguments.

I don't know what current thinking is about kernel size increases, but it
looks like this patch will result in quite significant kernel size increase
(some 18*8 = 144 bytes per driver for all the pointers, plus the actual
functions, adds up to a lot). Again this would be with no benefit for most
of the users of the hwmon subsystem. Sure, one can argue that the size increases
will only occur if the drivers are actually loaded, but that is a pretty weak
argument since the code size increase will still show up in each driver.

In summary I am not in favor for this change. Maybe Jean thinks differently,
but for my part I don't plan to approve it.

Guenter
Actually, it is not completely true. This API isn't mandatory for the drivers to implement. We could only modify the drivers we need in nouveau and leave the others untouched but this is only good for as a transition from the sysfs-only interface to the new interface.

I agree that changing hwmon in the way we are asking is a big change in philosophy, but what are you suggesting? We can't just re-implement the needed i2c drivers in nouveau and the only way we can access the already existing i2c drivers is through sysfs.

The real question is why hwmon only is targeted for the userland? Another question is, why is the actual code of the drivers buried so deep inside the implementation details of the sysfs interface (this is what makes it so painful to update)?

Actually, this proposal could save space as once this interface is adopted by some drivers, all the sysfs-related code could be shared in hwmon.c.

Another proposal could be to access the drivers through sysfs, but I don't know if it is possible and I think it would be abusing the sysfs interface anyway.

I think you now understand our situation a bit better, do you have any suggestion? I really wish to find an agreement on this as not sharing the code is not an option for me.

Martin

_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors



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

  Powered by Linux