From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Subject: Re: [lm-sensors] [PATCH] hwmon: (thmc50) Convert to a new-style i2c driver Date: Wed, 16 Jul 2008 23:04:11 +0200 Message-ID: <487E624B.4060908@hhs.nl> References: <20080716173316.1e8c5c42@hyperion.delvare> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20080716173316.1e8c5c42-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: i2c-bounces-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org Errors-To: i2c-bounces-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org To: Jean Delvare Cc: Krzysztof Helt , Linux I2C , LM Sensors List-Id: linux-i2c@vger.kernel.org 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 > Cc: Krzysztof Helt > @@ -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 i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org http://lists.lm-sensors.org/mailman/listinfo/i2c