lkml.org 
[lkml]   [2005]   [Jan]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: Ticket #1851 - PATCH (take 2) for adm1026.c, kernel 2.6.10-bk6
Date
From
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/

\
 
 \ /
  Last update: 2005-03-22 14:09    [W:0.097 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site