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 <khali at linux-fr.org>
> Cc: Krzysztof Helt <krzysztof.h1 at wp.pl>
> @@ -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
