* Re: [lm-sensors] [PATCH v2] hwmon (ds1621): Remove detection function
2013-05-21 0:29 [lm-sensors] [PATCH v2] hwmon (ds1621): Remove detection function Robert Coulson
@ 2013-05-21 0:46 ` Guenter Roeck
2013-05-21 6:26 ` Jean Delvare
2013-05-22 15:37 ` Robert Coulson
2 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2013-05-21 0:46 UTC (permalink / raw)
To: lm-sensors
On Mon, May 20, 2013 at 05:29:29PM -0700, Robert Coulson wrote:
> v2:
> - removed detect function references in ds1621 documentation
> - replaced deprecated with removed and fixed a spelling error
>
> Due to a lack of device and vendor identification registers, the
> Dallas/Maxim DS16xx devices cannot be uniquely detected, sometimes
> resulting in false positives. Therefore, the detection function is
> being removed in favor of explicit device instantiation.
>
> These changes are based on the 3.9 source base and intended for
> hwmon-next.
>
> Signed-off-by: Robert Coulson <rob.coulson@gmail.com>
Applied.
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [lm-sensors] [PATCH v2] hwmon (ds1621): Remove detection function
2013-05-21 0:29 [lm-sensors] [PATCH v2] hwmon (ds1621): Remove detection function Robert Coulson
2013-05-21 0:46 ` Guenter Roeck
@ 2013-05-21 6:26 ` Jean Delvare
2013-05-22 15:37 ` Robert Coulson
2 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2013-05-21 6:26 UTC (permalink / raw)
To: lm-sensors
Hi Robert,
On Mon, 20 May 2013 17:29:29 -0700, Robert Coulson wrote:
> v2:
> - removed detect function references in ds1621 documentation
> - replaced deprecated with removed and fixed a spelling error
Note: the patch changes history should go below, between the "---" and
the diffstat, so that it doesn't make it to git.
Almost there, see my comments inline.
>
> Due to a lack of device and vendor identification registers, the
> Dallas/Maxim DS16xx devices cannot be uniquely detected, sometimes
> resulting in false positives. Therefore, the detection function is
> being removed in favor of explicit device instantiation.
>
> These changes are based on the 3.9 source base and intended for
> hwmon-next.
>
> Signed-off-by: Robert Coulson <rob.coulson@gmail.com>
> ---
> Documentation/hwmon/ds1621 | 13 ++++---------
> drivers/hwmon/ds1621.c | 43 -------------------------------------------
> 2 files changed, 4 insertions(+), 52 deletions(-)
>
> diff --git a/Documentation/hwmon/ds1621 b/Documentation/hwmon/ds1621
> index 140eab5..009aabb 100644
> --- a/Documentation/hwmon/ds1621
> +++ b/Documentation/hwmon/ds1621
> @@ -8,9 +8,7 @@ Supported chips:
> Datasheet: Publicly available from www.maximintegrated.com
>
> * Dallas Semiconductor DS1625
> - Prefix:
> - 'ds1621' - if binding via _detect function
> - 'ds1625' - explicit instantiation
> + Prefix: 'ds1625'
> Addresses scanned: I2C 0x48 - 0x4f
You much change these lists to "none", as the driver will no longer
scan any address.
> Datasheet: Publicly available from www.datasheetarchive.com
>
> @@ -77,12 +75,9 @@ The DS1625 is pin compatible and functionally equivalent with the DS1621,
> but the DS1621 is meant to replace it. The DS1631 and DS1721 are also
> pin compatible with the DS1621, but provide multi-resolution support.
>
> -Since there is no version register, there is no unique identification
> -for these devices. In addition, the DS1631 and DS1721 will emulate a
> -DS1621 device, if not explicitly instantiated (why? because the detect
> -function compares the temperature register values bits and checks for a
> -9-bit resolution). Therefore, for correct device identification and
> -functionality, explicit device instantiation is required.
> +Since there is no version or vendor identification register, there is
> +no unique identification for these devices. Therefore, explicit device
> +instantiation is required for correct device identification and functionality.
>
> The DS1721 is pin compatible with the DS1621, has an accuracy of +/- 1.0
> degree Celius over a -10 to +85 degree range, a minimum/maximum alarm
> diff --git a/drivers/hwmon/ds1621.c b/drivers/hwmon/ds1621.c
> index d4f02a8..124fc83 100644
> --- a/drivers/hwmon/ds1621.c
> +++ b/drivers/hwmon/ds1621.c
> @@ -360,48 +360,6 @@ static const struct attribute_group ds1621_group = {
> .is_visible = ds1621_attribute_visible
> };
>
> -
> -/* Return 0 if detection is successful, -ENODEV otherwise */
> -static int ds1621_detect(struct i2c_client *client,
> - struct i2c_board_info *info)
> -{
> - struct i2c_adapter *adapter = client->adapter;
> - int conf, temp;
> - int i;
> -
> - if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA
> - | I2C_FUNC_SMBUS_WORD_DATA
> - | I2C_FUNC_SMBUS_WRITE_BYTE))
> - return -ENODEV;
> -
> - /*
> - * Now, we do the remaining detection. It is lousy.
> - *
> - * The NVB bit should be low if no EEPROM write has been requested
> - * during the latest 10ms, which is highly improbable in our case.
> - */
> - conf = i2c_smbus_read_byte_data(client, DS1621_REG_CONF);
> - if (conf < 0 || conf & DS1621_REG_CONFIG_NVB)
> - return -ENODEV;
> - /*
> - * The ds1621 & ds1625 use 9-bit resolution, so the 7 lowest bits
> - * of the temperature should always be 0 (NOTE: The other chips
> - * have multi-resolution support, so if they have 9-bit resolution
> - * configured and the min/max temperature values set accordingly,
> - * then if not explicitly instantiated, they *will* appear as and
> - * emulate a ds1621 device).
> - */
> - for (i = 0; i < ARRAY_SIZE(DS1621_REG_TEMP); i++) {
> - temp = i2c_smbus_read_word_data(client, DS1621_REG_TEMP[i]);
> - if (temp < 0 || (temp & 0x7f00))
> - return -ENODEV;
> - }
> -
> - strlcpy(info->type, "ds1621", I2C_NAME_SIZE);
> -
> - return 0;
> -}
> -
> static int ds1621_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -467,7 +425,6 @@ static struct i2c_driver ds1621_driver = {
> .probe = ds1621_probe,
> .remove = ds1621_remove,
> .id_table = ds1621_id,
> - .detect = ds1621_detect,
> .address_list = normal_i2c,
This address list can be removed as well, as it is only used to
determine when the detect callback should be called.
> };
>
Thanks,
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [lm-sensors] [PATCH v2] hwmon (ds1621): Remove detection function
2013-05-21 0:29 [lm-sensors] [PATCH v2] hwmon (ds1621): Remove detection function Robert Coulson
2013-05-21 0:46 ` Guenter Roeck
2013-05-21 6:26 ` Jean Delvare
@ 2013-05-22 15:37 ` Robert Coulson
2 siblings, 0 replies; 4+ messages in thread
From: Robert Coulson @ 2013-05-22 15:37 UTC (permalink / raw)
To: lm-sensors
[-- Attachment #1.1: Type: text/plain, Size: 5680 bytes --]
On Mon, May 20, 2013 at 11:26 PM, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Robert,
>
Good morning Jean,
>
> On Mon, 20 May 2013 17:29:29 -0700, Robert Coulson wrote:
> > v2:
> > - removed detect function references in ds1621 documentation
> > - replaced deprecated with removed and fixed a spelling error
>
> Note: the patch changes history should go below, between the "---" and
> the diffstat, so that it doesn't make it to git.
Thank you for the info, I appreciate it and will do so in the future.
>
> Almost there, see my comments inline.
>
> >
> > Due to a lack of device and vendor identification registers, the
> > Dallas/Maxim DS16xx devices cannot be uniquely detected, sometimes
> > resulting in false positives. Therefore, the detection function is
> > being removed in favor of explicit device instantiation.
> >
> > These changes are based on the 3.9 source base and intended for
> > hwmon-next.
> >
> > Signed-off-by: Robert Coulson <rob.coulson@gmail.com>
> > ---
> > Documentation/hwmon/ds1621 | 13 ++++---------
> > drivers/hwmon/ds1621.c | 43
> -------------------------------------------
> > 2 files changed, 4 insertions(+), 52 deletions(-)
> >
> > diff --git a/Documentation/hwmon/ds1621 b/Documentation/hwmon/ds1621
> > index 140eab5..009aabb 100644
> > --- a/Documentation/hwmon/ds1621
> > +++ b/Documentation/hwmon/ds1621
> > @@ -8,9 +8,7 @@ Supported chips:
> > Datasheet: Publicly available from www.maximintegrated.com
> >
> > * Dallas Semiconductor DS1625
> > - Prefix:
> > - 'ds1621' - if binding via _detect function
> > - 'ds1625' - explicit instantiation
> > + Prefix: 'ds1625'
> > Addresses scanned: I2C 0x48 - 0x4f
>
> You much change these lists to "none", as the driver will no longer
> scan any address.
>
agreed.
>
> > Datasheet: Publicly available from www.datasheetarchive.com
> >
> > @@ -77,12 +75,9 @@ The DS1625 is pin compatible and functionally
> equivalent with the DS1621,
> > but the DS1621 is meant to replace it. The DS1631 and DS1721 are also
> > pin compatible with the DS1621, but provide multi-resolution support.
> >
> > -Since there is no version register, there is no unique identification
> > -for these devices. In addition, the DS1631 and DS1721 will emulate a
> > -DS1621 device, if not explicitly instantiated (why? because the detect
> > -function compares the temperature register values bits and checks for a
> > -9-bit resolution). Therefore, for correct device identification and
> > -functionality, explicit device instantiation is required.
> > +Since there is no version or vendor identification register, there is
> > +no unique identification for these devices. Therefore, explicit device
> > +instantiation is required for correct device identification and
> functionality.
> >
> > The DS1721 is pin compatible with the DS1621, has an accuracy of +/- 1.0
> > degree Celius over a -10 to +85 degree range, a minimum/maximum alarm
> > diff --git a/drivers/hwmon/ds1621.c b/drivers/hwmon/ds1621.c
> > index d4f02a8..124fc83 100644
> > --- a/drivers/hwmon/ds1621.c
> > +++ b/drivers/hwmon/ds1621.c
> > @@ -360,48 +360,6 @@ static const struct attribute_group ds1621_group = {
> > .is_visible = ds1621_attribute_visible
> > };
> >
> > -
> > -/* Return 0 if detection is successful, -ENODEV otherwise */
> > -static int ds1621_detect(struct i2c_client *client,
> > - struct i2c_board_info *info)
> > -{
> > - struct i2c_adapter *adapter = client->adapter;
> > - int conf, temp;
> > - int i;
> > -
> > - if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA
> > - | I2C_FUNC_SMBUS_WORD_DATA
> > - | I2C_FUNC_SMBUS_WRITE_BYTE))
> > - return -ENODEV;
> > -
> > - /*
> > - * Now, we do the remaining detection. It is lousy.
> > - *
> > - * The NVB bit should be low if no EEPROM write has been requested
> > - * during the latest 10ms, which is highly improbable in our case.
> > - */
> > - conf = i2c_smbus_read_byte_data(client, DS1621_REG_CONF);
> > - if (conf < 0 || conf & DS1621_REG_CONFIG_NVB)
> > - return -ENODEV;
> > - /*
> > - * The ds1621 & ds1625 use 9-bit resolution, so the 7 lowest bits
> > - * of the temperature should always be 0 (NOTE: The other chips
> > - * have multi-resolution support, so if they have 9-bit resolution
> > - * configured and the min/max temperature values set accordingly,
> > - * then if not explicitly instantiated, they *will* appear as and
> > - * emulate a ds1621 device).
> > - */
> > - for (i = 0; i < ARRAY_SIZE(DS1621_REG_TEMP); i++) {
> > - temp = i2c_smbus_read_word_data(client,
> DS1621_REG_TEMP[i]);
> > - if (temp < 0 || (temp & 0x7f00))
> > - return -ENODEV;
> > - }
> > -
> > - strlcpy(info->type, "ds1621", I2C_NAME_SIZE);
> > -
> > - return 0;
> > -}
> > -
> > static int ds1621_probe(struct i2c_client *client,
> > const struct i2c_device_id *id)
> > {
> > @@ -467,7 +425,6 @@ static struct i2c_driver ds1621_driver = {
> > .probe = ds1621_probe,
> > .remove = ds1621_remove,
> > .id_table = ds1621_id,
> > - .detect = ds1621_detect,
> > .address_list = normal_i2c,
>
> This address list can be removed as well, as it is only used to
> determine when the detect callback should be called.
>
agreed and nice catch.
>
> > };
> >
>
> Thanks,
> --
> Jean Delvare
>
Your welcome.
I'll send an updated patch set shortly.
Rob
[-- Attachment #1.2: Type: text/html, Size: 7973 bytes --]
[-- Attachment #2: Type: text/plain, Size: 153 bytes --]
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 4+ messages in thread