Messages in this thread Patch in this message | | | Subject | Re: Ticket #1851 - PATCH (take 2) for adm1026.c, kernel 2.6.10-bk6 | Date | Mon, 3 Jan 2005 13:37:07 -0800 | From | Justin Thiessen <> |
| |
On Mon, Jan 03, 2005 at 08:10:56PM +0100, Jean Delvare wrote: > Hi Justin, >
<patch snipped>
> Mmm, I don't like this fix. > > For one thing, it still leaves some room for someone to call a sysfs > callback function before the values are all intialized (because you > create the sysfs files interface first, then intialize all the values). > > For another, this change will significantly increase the driver loading > time. Just check it! SMBus is slow and the ADM1026 has a lot of > registers.
Good points.
<snilp>
> Alternatives I can think of are: > > 1* Only intialize the few values that actually can be needed before > the update function was ever called. > > 2* Call the update function from the write sysfs callback functions > where needed.
Ick. Let's go with (1). A quick review of the adm1026_set_* functions doesn't seem to turn up any other situations where unextracted hardware defaults should cause any problems. In all other cases where a set function depends on data->* values that may not have been set, the data->* value default of 0 results in the desired behavior.
So let's just have the adm1026_init_client() function read the hardware fan divisor values and store them in fan*_div.
<snip>
> ...That said, the relevant code could be moved to a > separate function, called by both the detect/init and update functions, > so that no slowdown occurs (except for the extra function call, but > that's nothing compared to the SMBus slowness) and the code is still not > duplicated.
The amount of duplicated code is only a few lines, and I think the result is clearer if it is not extracted into a separate function. See the following patch.
Signed-off-by: Justin Thiessen <jthiessen@penguincomputing.com>
-------------------
--- linux-2.6.10/drivers/i2c/chips/adm1026.c.orig 2005-01-02 15:21:58.000000000 -0800 +++ linux-2.6.10/drivers/i2c/chips/adm1026.c 2005-01-02 18:27:40.695689832 -0800 @@ -452,6 +452,15 @@ void adm1026_init_client(struct i2c_clie client->id, value); data->config1 = value; adm1026_write_value(client, ADM1026_REG_CONFIG1, value); + + /* initialize fan_div[] to hardware defaults */ + value = adm1026_read_value(client, ADM1026_REG_FAN_DIV_0_3) + | (adm1026_read_value(client, ADM1026_REG_FAN_DIV_4_7) + << 8); + for (i = 0;i <= 7;++i) { + data->fan_div[i] = DIV_FROM_REG(value & 0x03); + value >>= 2; + } } void adm1026_print_gpio(struct i2c_client *client) - 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/
| |