Re: [PATCH v3 1/3] hwmon: tmp401: add support for TI TMP435

Re: [PATCH v3 1/3] hwmon: tmp401: add support for TI TMP435

[Guenter Roeck]

On Thu, Dec 04, 2014 at 05:45:51PM +0100, Bartosz Golaszewski wrote:
> From: Patrick Titiano <ptitiano@baylibre.com>
> 
> Signed-off-by: Patrick Titiano <ptitiano@baylibre.com>
> [Bartosz Golaszewski: prepared for submission, code review fixes]
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/hwmon/tmp401.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp401.c b/drivers/hwmon/tmp401.c
> index 7fa6e7d..ccd9938 100644
> --- a/drivers/hwmon/tmp401.c
> +++ b/drivers/hwmon/tmp401.c
> @@ -46,7 +46,7 @@
>  /* Addresses to scan */
>  static const unsigned short normal_i2c[] = { 0x4c, 0x4d, 0x4e, I2C_CLIENT_END };
>  
> -enum chips { tmp401, tmp411, tmp431, tmp432 };
> +enum chips { tmp401, tmp411, tmp431, tmp432, tmp435 };
>  
>  /*
>   * The TMP401 registers, note some registers have different addresses for
> @@ -136,6 +136,7 @@ static const u8 TMP432_STATUS_REG[] = {
>  #define TMP411C_DEVICE_ID			0x10
>  #define TMP431_DEVICE_ID			0x31
>  #define TMP432_DEVICE_ID			0x32
> +#define TMP435_DEVICE_ID			0x35
>  
>  /*
>   * Driver data (common to all clients)
> @@ -146,6 +147,7 @@ static const struct i2c_device_id tmp401_id[] = {
>  	{ "tmp411", tmp411 },
>  	{ "tmp431", tmp431 },
>  	{ "tmp432", tmp432 },
> +	{ "tmp435", tmp435 },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, tmp401_id);
> @@ -684,6 +686,11 @@ static int tmp401_detect(struct i2c_client *client,
>  			return -ENODEV;
>  		kind = tmp432;
>  		break;
> +	case TMP435_DEVICE_ID:
> +		if (client->addr != 0x4c)
> +			return -ENODEV;

On second thought, is this really correct ?

I had another look into the datasheet. It seems to me that the chip
may support up to 9 different addresses, though that is a bit vague
since the datasheet first claims that the address would be 0x4c,
only to go on and list the 9 addresses (0x48 to 0x4f as well as 0x37)
in the next paragraph.

Thanks,
Guenter

[PATCH v3 0/3] hwmon: tmp401: support for TMP435 and fix for a probe issue

[Bartosz Golaszewski]

This series adds support for the TI TMP435 chip and fixes an issue similar
to the one we observed for ina2xx - ignoring the write errors when probing the
device.

v3:
- remove the copyright notice from tmp401.c
- preinitialize status and skip an 'if'

v2:
- changed 'static const char *names' to 'static const char * const names'
  as checkpatch.pl still complained about this one,
- updated Documentation and Kconfig on TMP435,
- added missing error check in tmp401_init_client(),
- removed the redundant "Initialization failed" string since we'll get
  'tmp401: probe of X-00XX failed with error -121' anyway in dmesg,

v1:
https://www.mail-archive.com/linux-kernel%40vger.kernel.org/msg781930.html

Bartosz Golaszewski (2):
  hwmon: tmp401: add TMP435 to the list of supported chips in docs and Kconfig
  hwmon: tmp401: bail-out from tmp401_probe() in case of write errors

Patrick Titiano (1):
  hwmon: tmp401: add support for TI TMP435

 Documentation/hwmon/tmp401 |  7 +++++--
 drivers/hwmon/Kconfig      |  2 +-
 drivers/hwmon/tmp401.c     | 45 ++++++++++++++++++++++++++++++++-------------
 3 files changed, 38 insertions(+), 16 deletions(-)

-- 
2.1.3

[PATCH v3 1/3] hwmon: tmp401: add support for TI TMP435

[Bartosz Golaszewski]

From: Patrick Titiano <ptitiano@baylibre.com>

Signed-off-by: Patrick Titiano <ptitiano@baylibre.com>
[Bartosz Golaszewski: prepared for submission, code review fixes]
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/hwmon/tmp401.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/tmp401.c b/drivers/hwmon/tmp401.c
index 7fa6e7d..ccd9938 100644
--- a/drivers/hwmon/tmp401.c
+++ b/drivers/hwmon/tmp401.c
@@ -46,7 +46,7 @@
 /* Addresses to scan */
 static const unsigned short normal_i2c[] = { 0x4c, 0x4d, 0x4e, I2C_CLIENT_END };
 
-enum chips { tmp401, tmp411, tmp431, tmp432 };
+enum chips { tmp401, tmp411, tmp431, tmp432, tmp435 };
 
 /*
  * The TMP401 registers, note some registers have different addresses for
@@ -136,6 +136,7 @@ static const u8 TMP432_STATUS_REG[] = {
 #define TMP411C_DEVICE_ID			0x10
 #define TMP431_DEVICE_ID			0x31
 #define TMP432_DEVICE_ID			0x32
+#define TMP435_DEVICE_ID			0x35
 
 /*
  * Driver data (common to all clients)
@@ -146,6 +147,7 @@ static const struct i2c_device_id tmp401_id[] = {
 	{ "tmp411", tmp411 },
 	{ "tmp431", tmp431 },
 	{ "tmp432", tmp432 },
+	{ "tmp435", tmp435 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, tmp401_id);
@@ -684,6 +686,11 @@ static int tmp401_detect(struct i2c_client *client,
 			return -ENODEV;
 		kind = tmp432;
 		break;
+	case TMP435_DEVICE_ID:
+		if (client->addr != 0x4c)
+			return -ENODEV;
+		kind = tmp435;
+		break;
 	default:
 		return -ENODEV;
 	}
@@ -705,7 +712,9 @@ static int tmp401_detect(struct i2c_client *client,
 static int tmp401_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
-	const char *names[] = { "TMP401", "TMP411", "TMP431", "TMP432" };
+	static const char * const names[] = {
+		"TMP401", "TMP411", "TMP431", "TMP432", "TMP435"
+	};
 	struct device *dev = &client->dev;
 	struct device *hwmon_dev;
 	struct tmp401_data *data;
-- 
2.1.3