On Tue, Nov 30, 2010 at 1:17 AM, Jean Delvare <khali@xxxxxxxxxxxx> wrote:
I humbly suggest that you are forgetting something. With your recommendation, the REG_TO_DIV function would be called many orders of magnitute more times (thousands? millions?) than the DIV_TO_REG function here. REG_TO_DIV would have to be called every time the fan speed is _read_ but DIV_TO_REG only needs to be called the one or two times that the fan_div is _written_ So while the macros might differ in complexity, the number of times they are called swamps any economy gained by changing the fan_div from divisor to register value.
The code above is clear in and appropriate in my opinion.
Phil P.
> + mutex_lock(&data->update_lock);
> + orig_div = data->fan_div[nr];
> + data->fan_div[nr] = DIV_FROM_REG(new_div);
> +
> + if (nr < 4) { /* 0 <= nr < 4 */
> + adm1026_write_value(client, ADM1026_REG_FAN_DIV_0_3,
> + (DIV_TO_REG(data->fan_div[0]) << 0) |
> + (DIV_TO_REG(data->fan_div[1]) << 2) |
> + (DIV_TO_REG(data->fan_div[2]) << 4) |
> + (DIV_TO_REG(data->fan_div[3]) << 6) );
> + } else { /* 3 < nr < 8 */
> + adm1026_write_value(client, ADM1026_REG_FAN_DIV_4_7,
> + (DIV_TO_REG(data->fan_div[4]) << 0) |
> + (DIV_TO_REG(data->fan_div[5]) << 2) |
> + (DIV_TO_REG(data->fan_div[6]) << 4) |
> + (DIV_TO_REG(data->fan_div[7]) << 6) );
> + }
Note: this is horribly inefficient. In my opinion, data->fan_div should
hold split but un-decoded register values. Calling DIV_FROM_REG() is
cheap, calling DIV_TO_REG() is expensive. In fact DIV_TO_REG()
shouldn't exist in the first place, as the conversion should happen in
a single place. Again, this is material for a separate patch.
I humbly suggest that you are forgetting something. With your recommendation, the REG_TO_DIV function would be called many orders of magnitute more times (thousands? millions?) than the DIV_TO_REG function here. REG_TO_DIV would have to be called every time the fan speed is _read_ but DIV_TO_REG only needs to be called the one or two times that the fan_div is _written_ So while the macros might differ in complexity, the number of times they are called swamps any economy gained by changing the fan_div from divisor to register value.
The code above is clear in and appropriate in my opinion.
Phil P.
_______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
