Re: [PATCH 1/2] hwmon (ds1621): Add DS1731 chip support to ds1621 driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Tue, May 28, 2013 at 7:52 PM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
On Tue, May 28, 2013 at 04:56:01PM -0700, Robert Coulson wrote:
> These changes add DS1731 chip support to the ds1621 driver,
> Kconfig, and documentation.
>
> Signed-off-by: Robert Coulson <rob.coulson@xxxxxxxxx>

Hi Rob,

Nitpicks ...

Good afternoon Guenter,
 

> ---
>  Documentation/hwmon/ds1621 |   22 ++++++++++++++--------
>  drivers/hwmon/Kconfig      |    1 +
>  drivers/hwmon/ds1621.c     |   12 +++++++-----
>  3 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/hwmon/ds1621 b/Documentation/hwmon/ds1621
> index 2022379..64ee39f 100644
> --- a/Documentation/hwmon/ds1621
> +++ b/Documentation/hwmon/ds1621
> @@ -22,6 +22,11 @@ Supported chips:
>      Addresses scanned: none
>      Datasheet: Publicly available from www.maximintegrated.com
>
> +  * Maxim Integrated DS1731
> +    Prefix: 'ds1731'
> +    Addresses scanned: none
> +    Datasheet: Publicly available from www.maximintegrated.com
> +
>  Authors:
>          Christian W. Zuckschwerdt <zany@xxxxxxxx>
>          valuable contributions by Jan M. Sendler <sendler@xxxxxxxxxx>
> @@ -72,7 +77,7 @@ Temperature conversion of the DS1621 takes up to 1000ms; internal access to
>  non-volatile registers may last for 10ms or below.
>
>  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
> +but the DS1621 is meant to replace it. The DS1631, DS1721, & DS1731 are also
>  pin compatible with the DS1621, but provide multi-resolution support.
>
>  Since there is no version or vendor identification register, there is
> @@ -96,14 +101,15 @@ One additional note about the ds1721 is that although the data sheet says
>  the temperature flags (THF and TLF) are used internally, these flags do
>  get set and cleared as the actual temperature crosses the min or max settings.
>
> -The DS1631 is also pin compatible with the DS1621 and feature compatible with
> -the DS1721, however the DS1631 accuracy is +/- 0.5 degree Celius over the
> -same range.
> +The DS1631 and DS1731 are pin compatible with the DS1621 and feature compatible
> +with the DS1721, however, the DS1631 accuracy is +/- 0.5 degree Celius over

        DS1721. However, ...

fixed.
 

> +the 0 to +70 degrees, while the DS1731 accuracy is +/-1 degree Celius over the

"over the ... degrees" sounds weird. "over the range of", maybe ? Or at least
add "range" after "degrees".

how about 'within -10 to +85 degrees'.
 

> +-10 to +85 degree range.
>
> -Changing the DS1631/1721 resolution mode affects the conversion time and can be
> -done from userspace, via the device 'update_interval' sysfs attribute. This
> -attribute will normalize range of input values to the device maximum resolution
> -values defined in the datasheet as such:
> +Changing the DS1631, DS1721, or DS1731 resolution mode affects the conversion

Changing the resolution mode for ... affects

agreed.
 

> +time and can be done from userspace, via the device 'update_interval' sysfs
> +attribute. This attribute will normalize range of input values to the device
> +maximum resolution values defined in the datasheet as such:
>
        as follows ?

agreed.
 

>  Resolution    Conversion Time    Input Range
>   (C/LSB)       (msec)             (msec)
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 0f20eef..38c27b2 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -324,6 +324,7 @@ config SENSORS_DS1621
>         - Dallas Semiconductor DS1625
>         - Maxim Integrated DS1631
>         - Maxim Integrated DS1721
> +       - Maxim Integrated DS1731
>
>         This driver can also be built as a module.  If so, the module
>         will be called ds1621.
> diff --git a/drivers/hwmon/ds1621.c b/drivers/hwmon/ds1621.c
> index 1ecfcd0..c94495e 100644
> --- a/drivers/hwmon/ds1621.c
> +++ b/drivers/hwmon/ds1621.c
> @@ -10,8 +10,8 @@
>   * resolution, a thermal alarm output (Tout), and user-defined minimum
>   * and maximum temperature thresholds (TH & TL).
>   *
> - * The DS1625, DS1631, & DS1721 are pin compatible with the DS1621 and
> - * similar in operation, with slight variations as noted in the device
> + * The DS1625, DS1631, DS1721, & DS1731 are pin compatible with the DS1621
> + * and similar in operation, with slight variations as noted in the device
>   * datasheets (please refer to www.maximintegrated.com for specific
>   * device information).
>   *
> @@ -47,7 +47,7 @@
>  #include <linux/kernel.h>
>
>  /* Supported devices */
> -enum chips { ds1621, ds1625, ds1631, ds1721 };
> +enum chips { ds1621, ds1625, ds1631, ds1721, ds1731 };
>
>  /* Insmod parameters */
>  static int polarity = -1;
> @@ -65,7 +65,7 @@ MODULE_PARM_DESC(polarity, "Output's polarity: 0 = active high, 1 = active low")
>   *   7    6    5    4    3    2    1    0
>   * |Done|THF |TLF |NVB | 1  | 0  |POL |1SHOT|
>   *
> - * - DS1631:
> + * - DS1631, DS1731:
>   *   7    6    5    4    3    2    1    0
>   * |Done|THF |TLF |NVB | R1 | R0 |POL |1SHOT|
>   *
> @@ -140,7 +140,7 @@ static inline int DS1621_TEMP_FROM_REG(u16 reg)
>   * TEMP: 0.001C/bit (-55C to +125C)
>   * REG:
>   *  - 1621, 1625: 0.5C/bit
> - *  - 1631, 1721: 0.0625C/bit
> + *  - 1631, 1721, 1731: 0.0625C/bit
>   * Assume highest resolution and let the bits fall where they may..
>   */
>  static inline u16 DS1621_TEMP_TO_REG(long temp)
> @@ -176,6 +176,7 @@ static void ds1621_init_client(struct i2c_client *client)
>               break;
>       case ds1631:
>       case ds1721:
> +     case ds1731:
>               resol = (new_conf & DS1621_REG_CONFIG_RESOL) >>
>                        DS1621_REG_CONFIG_RESOL_SHIFT;
>               data->update_interval = ds1721_convrates[resol];
> @@ -408,6 +409,7 @@ static const struct i2c_device_id ds1621_id[] = {
>       { "ds1625", ds1625 },
>       { "ds1631", ds1631 },
>       { "ds1721", ds1721 },
> +     { "ds1731", ds1731 },
>       { }
>  };
>  MODULE_DEVICE_TABLE(i2c, ds1621_id);
> --
> 1.7.9.5
>
>
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@xxxxxxxxxxxxxx
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
>

_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux