drivers: sensor: mcp9808: fix various problems and improve test

drivers/sensor/mcp9808/mcp9808.c

@@ -1,6 +1,5 @@
-/* sensor_mcp9808.c - Driver for MCP9808 temperature sensor */
-
 /*
+ * Copyright (c) 2019 Peter Bigot Consulting, LLC
  * Copyright (c) 2016 Intel Corporation
  *
  * SPDX-License-Identifier: Apache-2.0
@@ -19,9 +18,11 @@
 
 LOG_MODULE_REGISTER(MCP9808, CONFIG_SENSOR_LOG_LEVEL);
 
-int mcp9808_reg_read(struct mcp9808_data *data, u8_t reg, u16_t *val)
+int mcp9808_reg_read(struct device *dev, u8_t reg, u16_t *val)
 {
-	int rc = i2c_write_read(data->i2c_master, data->i2c_slave_addr,
+	const struct mcp9808_data *data = dev->driver_data;
+	const struct mcp9808_config *cfg = dev->config->config_info;
+	int rc = i2c_write_read(data->i2c_master, cfg->i2c_addr,
 				&reg, sizeof(reg),
 				val, sizeof(*val));
 
@@ -38,56 +39,64 @@ static int mcp9808_sample_fetch(struct device *dev, enum sensor_channel chan)
 
 	__ASSERT_NO_MSG(chan == SENSOR_CHAN_ALL || chan == SENSOR_CHAN_AMBIENT_TEMP);
 
-	return mcp9808_reg_read(data, MCP9808_REG_TEMP_AMB, &data->reg_val);
+	return mcp9808_reg_read(dev, MCP9808_REG_TEMP_AMB, &data->reg_val);
 }
 
 static int mcp9808_channel_get(struct device *dev,
 			       enum sensor_channel chan,
 			       struct sensor_value *val)
 {
-	struct mcp9808_data *data = dev->driver_data;
+	const struct mcp9808_data *data = dev->driver_data;
+	int temp = mcp9808_temp_signed_from_reg(data->reg_val);
 
 	__ASSERT_NO_MSG(chan == SENSOR_CHAN_AMBIENT_TEMP);
 
-	val->val1 = (data->reg_val & MCP9808_TEMP_INT_MASK) >>
-		     MCP9808_TEMP_INT_SHIFT;
-	val->val2 = (data->reg_val & MCP9808_TEMP_FRAC_MASK) * 62500U;
-
-	if (data->reg_val & MCP9808_SIGN_BIT) {
-		val->val1 -= 256;
-	}
+	val->val1 = temp / MCP9808_TEMP_SCALE_CEL;
+	temp -= val->val1 * MCP9808_TEMP_SCALE_CEL;
+	val->val2 = (temp * 1000000) / MCP9808_TEMP_SCALE_CEL;
 
 	return 0;
 }
 
 static const struct sensor_driver_api mcp9808_api_funcs = {
 	.sample_fetch = mcp9808_sample_fetch,
 	.channel_get = mcp9808_channel_get,
+#ifdef CONFIG_MCP9808_TRIGGER
 	.attr_set = mcp9808_attr_set,
 	.trigger_set = mcp9808_trigger_set,
+#endif /* CONFIG_MCP9808_TRIGGER */
 };
 
 int mcp9808_init(struct device *dev)
 {
 	struct mcp9808_data *data = dev->driver_data;
+	const struct mcp9808_config *cfg = dev->config->config_info;
+	int rc = 0;
 
-	data->i2c_master =
-	       device_get_binding(DT_INST_0_MICROCHIP_MCP9808_BUS_NAME);
+	data->i2c_master = device_get_binding(cfg->i2c_bus);
 	if (!data->i2c_master) {
-		LOG_DBG("mcp9808: i2c master not found: %s",
-		    DT_INST_0_MICROCHIP_MCP9808_BUS_NAME);
+		LOG_DBG("mcp9808: i2c master not found: %s", cfg->i2c_bus);
 		return -EINVAL;
 	}
 
-	data->i2c_slave_addr = DT_INST_0_MICROCHIP_MCP9808_BASE_ADDRESS;
+#ifdef CONFIG_MCP9808_TRIGGER
+	rc = mcp9808_setup_interrupt(dev);
+#endif /* CONFIG_MCP9808_TRIGGER */
 
-	mcp9808_setup_interrupt(dev);
-
-	return 0;
+	return rc;
 }
 
-struct mcp9808_data mcp9808_data;
+static struct mcp9808_data mcp9808_data;
+static const struct mcp9808_config mcp9808_cfg = {
+	.i2c_bus = DT_INST_0_MICROCHIP_MCP9808_BUS_NAME,
+	.i2c_addr = DT_INST_0_MICROCHIP_MCP9808_BASE_ADDRESS,
+#ifdef CONFIG_MCP9808_TRIGGER
+	.alert_pin = DT_INST_0_MICROCHIP_MCP9808_INT_GPIOS_PIN,
+	.alert_flags = DT_INST_0_MICROCHIP_MCP9808_INT_GPIOS_FLAGS,
+	.alert_controller = DT_INST_0_MICROCHIP_MCP9808_INT_GPIOS_CONTROLLER,
+#endif /* CONFIG_MCP9808_TRIGGER */
+};
 
 DEVICE_AND_API_INIT(mcp9808, DT_INST_0_MICROCHIP_MCP9808_LABEL, mcp9808_init,
-		    &mcp9808_data, NULL, POST_KERNEL,
+		    &mcp9808_data, &mcp9808_cfg, POST_KERNEL,
 		    CONFIG_SENSOR_INIT_PRIORITY, &mcp9808_api_funcs);

drivers/sensor/mcp9808/mcp9808.h

@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2019 Peter Bigot Consulting, LLC
  * Copyright (c) 2016 Intel Corporation
  *
  * SPDX-License-Identifier: Apache-2.0
@@ -21,41 +22,74 @@
 #define MCP9808_REG_CRITICAL		0x04
 #define MCP9808_REG_TEMP_AMB		0x05
 
-#define MCP9808_ALERT_INT		BIT(0)
-#define MCP9808_ALERT_CNT		BIT(3)
-#define MCP9808_INT_CLEAR		BIT(5)
-
-#define MCP9808_SIGN_BIT		BIT(12)
-#define MCP9808_TEMP_INT_MASK		0x0ff0
-#define MCP9808_TEMP_INT_SHIFT		4
-#define MCP9808_TEMP_FRAC_MASK		0x000f
-
-#define MCP9808_TEMP_MAX		0xffc
+/* 16 bits control configuration and state.
+ *
+ * * Bit 0 controls alert signal output mode
+ * * Bit 1 controls interrupt polarity
+ * * Bit 2 disables upper and lower threshold checking
+ * * Bit 3 enables alert signal output
+ * * Bit 4 records alert status
+ * * Bit 5 records interrupt status
+ * * Bit 6 locks the upper/lower window registers
+ * * Bit 7 locks the critical register
+ * * Bit 8 enters shutdown mode
+ * * Bits 9-10 control threshold hysteresis
+ */
+#define MCP9808_CFG_ALERT_MODE_INT	BIT(0)
+#define MCP9808_CFG_ALERT_ENA		BIT(3)
+#define MCP9808_CFG_ALERT_STATE		BIT(4)
+#define MCP9808_CFG_INT_CLEAR		BIT(5)
+
+/* 16 bits are used for temperature and state encoding:
+ * * Bits 0..11 encode the temperature in a 2s complement signed value
+ *   in Celsius with 1/16 Cel resolution
+ * * Bit 12 is set to indicate a negative temperature
+ * * Bit 13 is set to indicate a temperature below the lower threshold
+ * * Bit 14 is set to indicate a temperature above the upper threshold
+ * * Bit 15 is set to indicate a temperature above the critical threshold
+ */
+#define MCP9808_TEMP_SCALE_CEL		16 /* signed */
+#define MCP9808_TEMP_SIGN_BIT		BIT(12)
+#define MCP9808_TEMP_ABS_MASK		((u16_t)(MCP9808_TEMP_SIGN_BIT - 1U))
+#define MCP9808_TEMP_LWR_BIT		BIT(13)
+#define MCP9808_TEMP_UPR_BIT		BIT(14)
+#define MCP9808_TEMP_CRT_BIT		BIT(15)
 
 struct mcp9808_data {
 	struct device *i2c_master;
-	u16_t i2c_slave_addr;
 
 	u16_t reg_val;
 
-	struct gpio_callback gpio_cb;
+#ifdef CONFIG_MCP9808_TRIGGER
+	struct device *alert_gpio;
+	struct gpio_callback alert_cb;
+
+	struct device *dev;
+
+	struct sensor_trigger trig;
+	sensor_trigger_handler_t trigger_handler;
+#endif
 
 #ifdef CONFIG_MCP9808_TRIGGER_OWN_THREAD
 	struct k_sem sem;
 #endif
 
 #ifdef CONFIG_MCP9808_TRIGGER_GLOBAL_THREAD
 	struct k_work work;
-	struct device *dev;
 #endif
+};
 
+struct mcp9808_config {
+	const char *i2c_bus;
+	u16_t i2c_addr;
 #ifdef CONFIG_MCP9808_TRIGGER
-	struct sensor_trigger trig;
-	sensor_trigger_handler_t trigger_handler;
-#endif
+	u8_t alert_pin;
+	u8_t alert_flags;
+	const char *alert_controller;
+#endif /* CONFIG_MCP9808_TRIGGER */
 };
 
-int mcp9808_reg_read(struct mcp9808_data *data, u8_t reg, u16_t *val);
+int mcp9808_reg_read(struct device *dev, u8_t reg, u16_t *val);
 
 #ifdef CONFIG_MCP9808_TRIGGER
 int mcp9808_attr_set(struct device *dev, enum sensor_channel chan,
@@ -64,26 +98,37 @@ int mcp9808_attr_set(struct device *dev, enum sensor_channel chan,
 int mcp9808_trigger_set(struct device *dev,
 			const struct sensor_trigger *trig,
 			sensor_trigger_handler_t handler);
-void mcp9808_setup_interrupt(struct device *dev);
-#else
-static inline int mcp9808_attr_set(struct device *dev,
-				   enum sensor_channel chan,
-				   enum sensor_attribute attr,
-				   const struct sensor_value *val)
-{
-	return -ENOTSUP;
-}
+int mcp9808_setup_interrupt(struct device *dev);
+#endif /* CONFIG_MCP9808_TRIGGER */
 
-static inline int mcp9808_trigger_set(struct device *dev,
-				      const struct sensor_trigger *trig,
-				      sensor_trigger_handler_t handler)
+/* Encode a signed temperature in scaled Celsius to the format used in
+ * register values.
+ */
+static inline u16_t mcp9808_temp_reg_from_signed(int temp)
 {
-	return -ENOTSUP;
+	/* Get the 12-bit 2s complement value */
+	u16_t rv = temp & MCP9808_TEMP_ABS_MASK;
+
+	if (temp < 0) {
+		rv |= MCP9808_TEMP_SIGN_BIT;
+	}
+	return rv;
 }
 
-static void mcp9808_setup_interrupt(struct device *dev)
+/* Decode a register temperature value to a signed temperature in
+ * scaled Celsius.
+ */
+static inline int mcp9808_temp_signed_from_reg(u16_t reg)
 {
+	int rv = reg & MCP9808_TEMP_ABS_MASK;
+
+	if (reg & MCP9808_TEMP_SIGN_BIT) {
+		/* Convert 12-bit 2s complement to signed negative
+		 * value.
+		 */
+		rv = -(1U + (rv ^ MCP9808_TEMP_ABS_MASK));
+	}
+	return rv;
 }
-#endif /* CONFIG_MCP9808_TRIGGER */
 
 #endif /* ZEPHYR_DRIVERS_SENSOR_MCP9808_MCP9808_H_ */