Jean Delvare wrote:
> The new-style thmc50 driver implements the optional detect()
> callback to cover the use cases of the legacy driver.
> 
> Signed-off-by: Jean Delvare <[EMAIL PROTECTED]>
> Cc: Krzysztof Helt <[EMAIL PROTECTED]>

> @@ -307,21 +304,22 @@ static int thmc50_detect(struct i2c_adap
>       }
>       if (err == -ENODEV) {
>               pr_debug("thmc50: Detection of THMC50/ADM1022 failed\n");
> -             goto exit_free;
> +             return err;
>       }
> -     data->type = kind;
>  
>       if (kind == adm1022) {
>               int id = i2c_adapter_id(client->adapter);
>               int i;
>  
>               type_name = "adm1022";
> -             data->has_temp3 = (config >> 7) & 1;    /* config MSB */
>               for (i = 0; i + 1 < adm1022_temp3_num; i += 2)
>                       if (adm1022_temp3[i] == id &&
> -                         adm1022_temp3[i + 1] == address) {
> +                         adm1022_temp3[i + 1] == client->addr) {
>                               /* enable 2nd remote temp */
> -                             data->has_temp3 = 1;
> +                             config |= (1 << 7);
> +                             i2c_smbus_write_byte_data(client,
> +                                                       THMC50_REG_CONF,
> +                                                       config);
>                               break;
>                       }
>       } else {

Hmm, This does not seem right, you are _writing_ to the device in a the detect 
function changing its configuration. I understand you can no longer set 
data->has_temp3 as there is no data yet in the detect, instead the entire loop 
which checks if the device has temp3 forced through a module option should be 
moved to the probe function. Writing to a config register in the detect 
function feels wrong to me.

Regards,

Hans

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Reply via email to