| Message ID | 1463761907-15439-1-git-send-email-clg@kaod.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Cédric, On Fri, 2016-05-20 at 18:31 +0200, Cédric Le Goater wrote: > Largely inspired by the TMP105 temperature sensor, this patch brings > to Qemu a model for TMP42{1,2,3} temperature sensors. > > Specs can be found here : > > http://www.ti.com/lit/gpn/tmp421 > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/misc/Makefile.objs | 1 + > hw/misc/tmp421.c | 395 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 396 insertions(+) > create mode 100644 hw/misc/tmp421.c > > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs > index 1faf163c59f3..e50596965b03 100644 > --- a/hw/misc/Makefile.objs > +++ b/hw/misc/Makefile.objs > @@ -1,6 +1,7 @@ > common-obj-$(CONFIG_APPLESMC) += applesmc.o > common-obj-$(CONFIG_MAX111X) += max111x.o > common-obj-$(CONFIG_TMP105) += tmp105.o > +common-obj-$(CONFIG_ASPEED_SOC) += tmp421.o > common-obj-$(CONFIG_ISA_DEBUG) += debugexit.o > common-obj-$(CONFIG_SGA) += sga.o > common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o > diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c > new file mode 100644 > index 000000000000..e385ce053f5e > --- /dev/null > +++ b/hw/misc/tmp421.c > @@ -0,0 +1,395 @@ > +/* > + * Texas Instruments TMP421 temperature sensor. > + * > + * Copyright (c) 2016 IBM Corporation. > + * > + * Largely inspired by : > + * > + * Texas Instruments TMP105 temperature sensor. > + * > + * Copyright (C) 2008 Nokia Corporation > + * Written by Andrzej Zaborowski <andrew@openedhand.com> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 or > + * (at your option) version 3 of the License. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include "qemu/osdep.h" > +#include "hw/hw.h" > +#include "hw/i2c/i2c.h" > +#include "qapi/error.h" > +#include "qapi/visitor.h" > + > +/* Manufacturer / Device ID's */ > +#define TMP421_MANUFACTURER_ID 0x55 > +#define TMP421_DEVICE_ID 0x21 > +#define TMP422_DEVICE_ID 0x22 > +#define TMP423_DEVICE_ID 0x23 > + > +typedef struct DeviceInfo { > + int model; > + const char *name; > +} DeviceInfo; > + > +static const DeviceInfo devices[] = { > + { TMP421_DEVICE_ID, "tmp421" }, > + { TMP422_DEVICE_ID, "tmp422" }, > + { TMP423_DEVICE_ID, "tmp423" }, > +}; > + > +typedef struct TMP421State { > + /*< private >*/ > + I2CSlave i2c; > + /*< public >*/ > + > + int16_t temperature[4]; > + > + uint8_t status; > + uint8_t config[2]; > + uint8_t rate; > + > + uint8_t len; Given the starting point of the tmp105 code the patch looks okay, but I was a bit thrown by the use of the 'len' member as what I'd consider an index. For instance we reset len to zero in tmp421_event() after populating buf, and then the data in buf is presumably sent out on a recv transaction which again starts incrementing len. len is also incremented when we don't interact with buf, e.g. when we instead assign to pointer. It feels like it could be prone to bugs, and 'cb5ef3fa1871 tmp105: Fix I2C protocol bug' suggests that might not be an unreasonable feeling. But given the code already exists in tmp105 maybe it's fine? Cheers, Andrew > + uint8_t buf[2]; > + uint8_t pointer; > + > +} TMP421State; > + > +typedef struct TMP421Class { > + I2CSlaveClass parent_class; > + DeviceInfo *dev; > +} TMP421Class; > + > +#define TYPE_TMP421 "tmp421-generic" > +#define TMP421(obj) OBJECT_CHECK(TMP421State, (obj), TYPE_TMP421) > + > +#define TMP421_CLASS(klass) \ > + OBJECT_CLASS_CHECK(TMP421Class, (klass), TYPE_TMP421) > +#define TMP421_GET_CLASS(obj) \ > + OBJECT_GET_CLASS(TMP421Class, (obj), TYPE_TMP421) > + > +/* the TMP421 registers */ > +#define TMP421_STATUS_REG 0x08 > +#define TMP421_STATUS_BUSY (1 << 7) > +#define TMP421_CONFIG_REG_1 0x09 > +#define TMP421_CONFIG_RANGE (1 << 2) > +#define TMP421_CONFIG_SHUTDOWN (1 << 6) > +#define TMP421_CONFIG_REG_2 0x0A > +#define TMP421_CONFIG_RC (1 << 2) > +#define TMP421_CONFIG_LEN (1 << 3) > +#define TMP421_CONFIG_REN (1 << 4) > +#define TMP421_CONFIG_REN2 (1 << 5) > +#define TMP421_CONFIG_REN3 (1 << 6) > + > +#define TMP421_CONVERSION_RATE_REG 0x0B > +#define TMP421_ONE_SHOT 0x0F > + > +#define TMP421_RESET 0xFC > +#define TMP421_MANUFACTURER_ID_REG 0xFE > +#define TMP421_DEVICE_ID_REG 0xFF > + > +#define TMP421_TEMP_MSB0 0x00 > +#define TMP421_TEMP_MSB1 0x01 > +#define TMP421_TEMP_MSB2 0x02 > +#define TMP421_TEMP_MSB3 0x03 > +#define TMP421_TEMP_LSB0 0x10 > +#define TMP421_TEMP_LSB1 0x11 > +#define TMP421_TEMP_LSB2 0x12 > +#define TMP421_TEMP_LSB3 0x13 > + > +static const int32_t mins[2] = { -40000, -55000 }; > +static const int32_t maxs[2] = { 127000, 150000 }; > + > +static void tmp421_get_temperature(Object *obj, Visitor *v, const char *name, > + void *opaque, Error **errp) > +{ > + TMP421State *s = TMP421(obj); > + bool ext_range = (s->config[0] & TMP421_CONFIG_RANGE); > + int offset = ext_range * 64 * 256; > + int64_t value; > + int tempid; > + > + if (sscanf(name, "temperature%d", &tempid) != 1) { > + error_setg(errp, "error reading %s: %m", name); > + return; > + } > + > + if (tempid >= 4 || tempid < 0) { > + error_setg(errp, "error reading %s", name); > + return; > + } > + > + value = ((s->temperature[tempid] - offset) * 1000 + 128) / 256; > + > + visit_type_int(v, name, &value, errp); > +} > + > +/* Units are 0.001 centigrades relative to 0 C. s->temperature is 8.8 > + * fixed point, so units are 1/256 centigrades. A simple ratio will do. > + */ > +static void tmp421_set_temperature(Object *obj, Visitor *v, const char *name, > + void *opaque, Error **errp) > +{ > + TMP421State *s = TMP421(obj); > + Error *local_err = NULL; > + int64_t temp; > + bool ext_range = (s->config[0] & TMP421_CONFIG_RANGE); > + int offset = ext_range * 64 * 256; > + int tempid; > + > + visit_type_int(v, name, &temp, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + > + if (temp >= maxs[ext_range] || temp < mins[ext_range]) { > + error_setg(errp, "value %" PRId64 ".%03" PRIu64 " °C is out of range", > + temp / 1000, temp % 1000); > + return; > + } > + > + if (sscanf(name, "temperature%d", &tempid) != 1) { > + error_setg(errp, "error reading %s: %m", name); > + return; > + } > + > + if (tempid >= 4 || tempid < 0) { > + error_setg(errp, "error reading %s", name); > + return; > + } > + > + s->temperature[tempid] = (int16_t) ((temp * 256 - 128) / 1000) + offset; > +} > + > +static void tmp421_read(TMP421State *s) > +{ > + TMP421Class *sc = TMP421_GET_CLASS(s); > + > + s->len = 0; > + > + switch (s->pointer) { > + case TMP421_MANUFACTURER_ID_REG: > + s->buf[s->len++] = TMP421_MANUFACTURER_ID; > + break; > + case TMP421_DEVICE_ID_REG: > + s->buf[s->len++] = sc->dev->model; > + break; > + case TMP421_CONFIG_REG_1: > + s->buf[s->len++] = s->config[0]; > + break; > + case TMP421_CONFIG_REG_2: > + s->buf[s->len++] = s->config[1]; > + break; > + case TMP421_CONVERSION_RATE_REG: > + s->buf[s->len++] = s->rate; > + break; > + case TMP421_STATUS_REG: > + s->buf[s->len++] = s->status; > + break; > + > + /* FIXME: check for channel enablement in config registers */ > + case TMP421_TEMP_MSB0: > + s->buf[s->len++] = (((uint16_t) s->temperature[0]) >> 8); > + break; > + case TMP421_TEMP_MSB1: > + s->buf[s->len++] = (((uint16_t) s->temperature[1]) >> 8); > + break; > + case TMP421_TEMP_MSB2: > + s->buf[s->len++] = (((uint16_t) s->temperature[2]) >> 8); > + break; > + case TMP421_TEMP_MSB3: > + s->buf[s->len++] = (((uint16_t) s->temperature[3]) >> 8); > + break; > + case TMP421_TEMP_LSB0: > + s->buf[s->len++] = (((uint16_t) s->temperature[0]) >> 0) & 0xf0; > + break; > + case TMP421_TEMP_LSB1: > + s->buf[s->len++] = (((uint16_t) s->temperature[1]) >> 0) & 0xf0; > + break; > + case TMP421_TEMP_LSB2: > + s->buf[s->len++] = (((uint16_t) s->temperature[2]) >> 0) & 0xf0; > + break; > + case TMP421_TEMP_LSB3: > + s->buf[s->len++] = (((uint16_t) s->temperature[3]) >> 0) & 0xf0; > + break; > + } > +} > + > +static void tmp421_reset(I2CSlave *i2c); > + > +static void tmp421_write(TMP421State *s) > +{ > + switch (s->pointer) { > + case TMP421_CONVERSION_RATE_REG: > + s->rate = s->buf[0]; > + break; > + case TMP421_CONFIG_REG_1: > + s->config[0] = s->buf[0]; > + break; > + case TMP421_CONFIG_REG_2: > + s->config[1] = s->buf[0]; > + break; > + case TMP421_RESET: > + tmp421_reset(I2C_SLAVE(s)); > + break; > + } > +} > + > +static int tmp421_rx(I2CSlave *i2c) > +{ > + TMP421State *s = TMP421(i2c); > + > + if (s->len < 2) { > + return s->buf[s->len++]; > + } else { > + return 0xff; > + } > +} > + > +static int tmp421_tx(I2CSlave *i2c, uint8_t data) > +{ > + TMP421State *s = TMP421(i2c); > + > + if (s->len == 0) { > + s->pointer = data; > + s->len++; > + } else { > + if (s->len <= 2) { > + s->buf[s->len - 1] = data; > + } > + s->len++; > + tmp421_write(s); > + } > + > + return 0; > +} > + > +static void tmp421_event(I2CSlave *i2c, enum i2c_event event) > +{ > + TMP421State *s = TMP421(i2c); > + > + if (event == I2C_START_RECV) { > + tmp421_read(s); > + } > + > + s->len = 0; > +} > + > +static const VMStateDescription vmstate_tmp421 = { > + .name = "TMP421", > + .version_id = 0, > + .minimum_version_id = 0, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8(len, TMP421State), > + VMSTATE_UINT8_ARRAY(buf, TMP421State, 2), > + VMSTATE_UINT8(pointer, TMP421State), > + VMSTATE_UINT8_ARRAY(config, TMP421State, 2), > + VMSTATE_UINT8(status, TMP421State), > + VMSTATE_UINT8(rate, TMP421State), > + VMSTATE_INT16_ARRAY(temperature, TMP421State, 4), > + VMSTATE_I2C_SLAVE(i2c, TMP421State), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static void tmp421_reset(I2CSlave *i2c) > +{ > + TMP421State *s = TMP421(i2c); > + TMP421Class *sc = TMP421_GET_CLASS(s); > + > + memset(s->temperature, 0, sizeof(s->temperature)); > + s->pointer = 0; > + > + s->config[0] = 0; /* TMP421_CONFIG_RANGE */ > + > + /* resistance correction and channel enablement */ > + switch (sc->dev->model) { > + case TMP421_DEVICE_ID: > + s->config[1] = 0x1c; > + break; > + case TMP422_DEVICE_ID: > + s->config[1] = 0x3c; > + break; > + case TMP423_DEVICE_ID: > + s->config[1] = 0x7c; > + break; > + } > + > + s->rate = 0x7; /* 8Hz */ > + s->status = 0; > +} > + > +static int tmp421_init(I2CSlave *i2c) > +{ > + TMP421State *s = TMP421(i2c); > + > + tmp421_reset(&s->i2c); > + > + return 0; > +} > + > +static void tmp421_initfn(Object *obj) > +{ > + object_property_add(obj, "temperature0", "int", > + tmp421_get_temperature, > + tmp421_set_temperature, NULL, NULL, NULL); > + object_property_add(obj, "temperature1", "int", > + tmp421_get_temperature, > + tmp421_set_temperature, NULL, NULL, NULL); > + object_property_add(obj, "temperature2", "int", > + tmp421_get_temperature, > + tmp421_set_temperature, NULL, NULL, NULL); > + object_property_add(obj, "temperature3", "int", > + tmp421_get_temperature, > + tmp421_set_temperature, NULL, NULL, NULL); > +} > + > +static void tmp421_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass); > + TMP421Class *sc = TMP421_CLASS(klass); > + > + k->init = tmp421_init; > + k->event = tmp421_event; > + k->recv = tmp421_rx; > + k->send = tmp421_tx; > + dc->vmsd = &vmstate_tmp421; > + sc->dev = (DeviceInfo *) data; > +} > + > +static const TypeInfo tmp421_info = { > + .name = TYPE_TMP421, > + .parent = TYPE_I2C_SLAVE, > + .instance_size = sizeof(TMP421State), > + .instance_init = tmp421_initfn, > + .class_init = tmp421_class_init, > +}; > + > +static void tmp421_register_types(void) > +{ > + int i; > + > + type_register_static(&tmp421_info); > + for (i = 0; i < ARRAY_SIZE(devices); ++i) { > + TypeInfo ti = { > + .name = devices[i].name, > + .parent = TYPE_TMP421, > + .class_init = tmp421_class_init, > + .class_data = (void *) &devices[i], > + }; > + type_register(&ti); > + } > +} > + > +type_init(tmp421_register_types)
On 05/23/2016 05:53 AM, Andrew Jeffery wrote: > Hi Cédric, > > On Fri, 2016-05-20 at 18:31 +0200, Cédric Le Goater wrote: >> Largely inspired by the TMP105 temperature sensor, this patch brings >> to Qemu a model for TMP42{1,2,3} temperature sensors. >> >> Specs can be found here : >> >> http://www.ti.com/lit/gpn/tmp421 >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/misc/Makefile.objs | 1 + >> hw/misc/tmp421.c | 395 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 396 insertions(+) >> create mode 100644 hw/misc/tmp421.c >> >> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs >> index 1faf163c59f3..e50596965b03 100644 >> --- a/hw/misc/Makefile.objs >> +++ b/hw/misc/Makefile.objs >> @@ -1,6 +1,7 @@ >> common-obj-$(CONFIG_APPLESMC) += applesmc.o >> common-obj-$(CONFIG_MAX111X) += max111x.o >> common-obj-$(CONFIG_TMP105) += tmp105.o >> +common-obj-$(CONFIG_ASPEED_SOC) += tmp421.o >> common-obj-$(CONFIG_ISA_DEBUG) += debugexit.o >> common-obj-$(CONFIG_SGA) += sga.o >> common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o >> diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c >> new file mode 100644 >> index 000000000000..e385ce053f5e >> --- /dev/null >> +++ b/hw/misc/tmp421.c >> @@ -0,0 +1,395 @@ >> +/* >> + * Texas Instruments TMP421 temperature sensor. >> + * >> + * Copyright (c) 2016 IBM Corporation. >> + * >> + * Largely inspired by : >> + * >> + * Texas Instruments TMP105 temperature sensor. >> + * >> + * Copyright (C) 2008 Nokia Corporation >> + * Written by Andrzej Zaborowski <andrew@openedhand.com> >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License as >> + * published by the Free Software Foundation; either version 2 or >> + * (at your option) version 3 of the License. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License along >> + * with this program; if not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "hw/hw.h" >> +#include "hw/i2c/i2c.h" >> +#include "qapi/error.h" >> +#include "qapi/visitor.h" >> + >> +/* Manufacturer / Device ID's */ >> +#define TMP421_MANUFACTURER_ID 0x55 >> +#define TMP421_DEVICE_ID 0x21 >> +#define TMP422_DEVICE_ID 0x22 >> +#define TMP423_DEVICE_ID 0x23 >> + >> +typedef struct DeviceInfo { >> + int model; >> + const char *name; >> +} DeviceInfo; >> + >> +static const DeviceInfo devices[] = { >> + { TMP421_DEVICE_ID, "tmp421" }, >> + { TMP422_DEVICE_ID, "tmp422" }, >> + { TMP423_DEVICE_ID, "tmp423" }, >> +}; >> + >> +typedef struct TMP421State { >> + /*< private >*/ >> + I2CSlave i2c; >> + /*< public >*/ >> + >> + int16_t temperature[4]; >> + >> + uint8_t status; >> + uint8_t config[2]; >> + uint8_t rate; >> + >> + uint8_t len; > > Given the starting point of the tmp105 code the patch looks okay, but I > was a bit thrown by the use of the 'len' member as what I'd consider an > index. For instance we reset len to zero in tmp421_event() after > populating buf, and then the data in buf is presumably sent out on a > recv transaction which again starts incrementing len. len is also > incremented when we don't interact with buf, e.g. when we instead > assign to pointer. It feels like it could be prone to bugs, and > 'cb5ef3fa1871 tmp105: Fix I2C protocol bug' suggests that might not be > an unreasonable feeling. I will take a look. Anyhow I wanted to change this config option : +common-obj-$(CONFIG_ASPEED_SOC) += tmp421.o as there is no reason not to export the device to other platforms. Thanks C. > But given the code already exists in tmp105 maybe it's fine? > > Cheers, > > Andrew > >> + uint8_t buf[2]; >> + uint8_t pointer; >> + >> +} TMP421State; >> + >> +typedef struct TMP421Class { >> + I2CSlaveClass parent_class; >> + DeviceInfo *dev; >> +} TMP421Class; >> + >> +#define TYPE_TMP421 "tmp421-generic" >> +#define TMP421(obj) OBJECT_CHECK(TMP421State, (obj), TYPE_TMP421) >> + >> +#define TMP421_CLASS(klass) \ >> + OBJECT_CLASS_CHECK(TMP421Class, (klass), TYPE_TMP421) >> +#define TMP421_GET_CLASS(obj) \ >> + OBJECT_GET_CLASS(TMP421Class, (obj), TYPE_TMP421) >> + >> +/* the TMP421 registers */ >> +#define TMP421_STATUS_REG 0x08 >> +#define TMP421_STATUS_BUSY (1 << 7) >> +#define TMP421_CONFIG_REG_1 0x09 >> +#define TMP421_CONFIG_RANGE (1 << 2) >> +#define TMP421_CONFIG_SHUTDOWN (1 << 6) >> +#define TMP421_CONFIG_REG_2 0x0A >> +#define TMP421_CONFIG_RC (1 << 2) >> +#define TMP421_CONFIG_LEN (1 << 3) >> +#define TMP421_CONFIG_REN (1 << 4) >> +#define TMP421_CONFIG_REN2 (1 << 5) >> +#define TMP421_CONFIG_REN3 (1 << 6) >> + >> +#define TMP421_CONVERSION_RATE_REG 0x0B >> +#define TMP421_ONE_SHOT 0x0F >> + >> +#define TMP421_RESET 0xFC >> +#define TMP421_MANUFACTURER_ID_REG 0xFE >> +#define TMP421_DEVICE_ID_REG 0xFF >> + >> +#define TMP421_TEMP_MSB0 0x00 >> +#define TMP421_TEMP_MSB1 0x01 >> +#define TMP421_TEMP_MSB2 0x02 >> +#define TMP421_TEMP_MSB3 0x03 >> +#define TMP421_TEMP_LSB0 0x10 >> +#define TMP421_TEMP_LSB1 0x11 >> +#define TMP421_TEMP_LSB2 0x12 >> +#define TMP421_TEMP_LSB3 0x13 >> + >> +static const int32_t mins[2] = { -40000, -55000 }; >> +static const int32_t maxs[2] = { 127000, 150000 }; >> + >> +static void tmp421_get_temperature(Object *obj, Visitor *v, const char *name, >> + void *opaque, Error **errp) >> +{ >> + TMP421State *s = TMP421(obj); >> + bool ext_range = (s->config[0] & TMP421_CONFIG_RANGE); >> + int offset = ext_range * 64 * 256; >> + int64_t value; >> + int tempid; >> + >> + if (sscanf(name, "temperature%d", &tempid) != 1) { >> + error_setg(errp, "error reading %s: %m", name); >> + return; >> + } >> + >> + if (tempid >= 4 || tempid < 0) { >> + error_setg(errp, "error reading %s", name); >> + return; >> + } >> + >> + value = ((s->temperature[tempid] - offset) * 1000 + 128) / 256; >> + >> + visit_type_int(v, name, &value, errp); >> +} >> + >> +/* Units are 0.001 centigrades relative to 0 C. s->temperature is 8.8 >> + * fixed point, so units are 1/256 centigrades. A simple ratio will do. >> + */ >> +static void tmp421_set_temperature(Object *obj, Visitor *v, const char *name, >> + void *opaque, Error **errp) >> +{ >> + TMP421State *s = TMP421(obj); >> + Error *local_err = NULL; >> + int64_t temp; >> + bool ext_range = (s->config[0] & TMP421_CONFIG_RANGE); >> + int offset = ext_range * 64 * 256; >> + int tempid; >> + >> + visit_type_int(v, name, &temp, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return; >> + } >> + >> + if (temp >= maxs[ext_range] || temp < mins[ext_range]) { >> + error_setg(errp, "value %" PRId64 ".%03" PRIu64 " °C is out of range", >> + temp / 1000, temp % 1000); >> + return; >> + } >> + >> + if (sscanf(name, "temperature%d", &tempid) != 1) { >> + error_setg(errp, "error reading %s: %m", name); >> + return; >> + } >> + >> + if (tempid >= 4 || tempid < 0) { >> + error_setg(errp, "error reading %s", name); >> + return; >> + } >> + >> + s->temperature[tempid] = (int16_t) ((temp * 256 - 128) / 1000) + offset; >> +} >> + >> +static void tmp421_read(TMP421State *s) >> +{ >> + TMP421Class *sc = TMP421_GET_CLASS(s); >> + >> + s->len = 0; >> + >> + switch (s->pointer) { >> + case TMP421_MANUFACTURER_ID_REG: >> + s->buf[s->len++] = TMP421_MANUFACTURER_ID; >> + break; >> + case TMP421_DEVICE_ID_REG: >> + s->buf[s->len++] = sc->dev->model; >> + break; >> + case TMP421_CONFIG_REG_1: >> + s->buf[s->len++] = s->config[0]; >> + break; >> + case TMP421_CONFIG_REG_2: >> + s->buf[s->len++] = s->config[1]; >> + break; >> + case TMP421_CONVERSION_RATE_REG: >> + s->buf[s->len++] = s->rate; >> + break; >> + case TMP421_STATUS_REG: >> + s->buf[s->len++] = s->status; >> + break; >> + >> + /* FIXME: check for channel enablement in config registers */ >> + case TMP421_TEMP_MSB0: >> + s->buf[s->len++] = (((uint16_t) s->temperature[0]) >> 8); >> + break; >> + case TMP421_TEMP_MSB1: >> + s->buf[s->len++] = (((uint16_t) s->temperature[1]) >> 8); >> + break; >> + case TMP421_TEMP_MSB2: >> + s->buf[s->len++] = (((uint16_t) s->temperature[2]) >> 8); >> + break; >> + case TMP421_TEMP_MSB3: >> + s->buf[s->len++] = (((uint16_t) s->temperature[3]) >> 8); >> + break; >> + case TMP421_TEMP_LSB0: >> + s->buf[s->len++] = (((uint16_t) s->temperature[0]) >> 0) & 0xf0; >> + break; >> + case TMP421_TEMP_LSB1: >> + s->buf[s->len++] = (((uint16_t) s->temperature[1]) >> 0) & 0xf0; >> + break; >> + case TMP421_TEMP_LSB2: >> + s->buf[s->len++] = (((uint16_t) s->temperature[2]) >> 0) & 0xf0; >> + break; >> + case TMP421_TEMP_LSB3: >> + s->buf[s->len++] = (((uint16_t) s->temperature[3]) >> 0) & 0xf0; >> + break; >> + } >> +} >> + >> +static void tmp421_reset(I2CSlave *i2c); >> + >> +static void tmp421_write(TMP421State *s) >> +{ >> + switch (s->pointer) { >> + case TMP421_CONVERSION_RATE_REG: >> + s->rate = s->buf[0]; >> + break; >> + case TMP421_CONFIG_REG_1: >> + s->config[0] = s->buf[0]; >> + break; >> + case TMP421_CONFIG_REG_2: >> + s->config[1] = s->buf[0]; >> + break; >> + case TMP421_RESET: >> + tmp421_reset(I2C_SLAVE(s)); >> + break; >> + } >> +} >> + >> +static int tmp421_rx(I2CSlave *i2c) >> +{ >> + TMP421State *s = TMP421(i2c); >> + >> + if (s->len < 2) { >> + return s->buf[s->len++]; >> + } else { >> + return 0xff; >> + } >> +} >> + >> +static int tmp421_tx(I2CSlave *i2c, uint8_t data) >> +{ >> + TMP421State *s = TMP421(i2c); >> + >> + if (s->len == 0) { >> + s->pointer = data; >> + s->len++; >> + } else { >> + if (s->len <= 2) { >> + s->buf[s->len - 1] = data; >> + } >> + s->len++; >> + tmp421_write(s); >> + } >> + >> + return 0; >> +} >> + >> +static void tmp421_event(I2CSlave *i2c, enum i2c_event event) >> +{ >> + TMP421State *s = TMP421(i2c); >> + >> + if (event == I2C_START_RECV) { >> + tmp421_read(s); >> + } >> + >> + s->len = 0; >> +} >> + >> +static const VMStateDescription vmstate_tmp421 = { >> + .name = "TMP421", >> + .version_id = 0, >> + .minimum_version_id = 0, >> + .fields = (VMStateField[]) { >> + VMSTATE_UINT8(len, TMP421State), >> + VMSTATE_UINT8_ARRAY(buf, TMP421State, 2), >> + VMSTATE_UINT8(pointer, TMP421State), >> + VMSTATE_UINT8_ARRAY(config, TMP421State, 2), >> + VMSTATE_UINT8(status, TMP421State), >> + VMSTATE_UINT8(rate, TMP421State), >> + VMSTATE_INT16_ARRAY(temperature, TMP421State, 4), >> + VMSTATE_I2C_SLAVE(i2c, TMP421State), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +static void tmp421_reset(I2CSlave *i2c) >> +{ >> + TMP421State *s = TMP421(i2c); >> + TMP421Class *sc = TMP421_GET_CLASS(s); >> + >> + memset(s->temperature, 0, sizeof(s->temperature)); >> + s->pointer = 0; >> + >> + s->config[0] = 0; /* TMP421_CONFIG_RANGE */ >> + >> + /* resistance correction and channel enablement */ >> + switch (sc->dev->model) { >> + case TMP421_DEVICE_ID: >> + s->config[1] = 0x1c; >> + break; >> + case TMP422_DEVICE_ID: >> + s->config[1] = 0x3c; >> + break; >> + case TMP423_DEVICE_ID: >> + s->config[1] = 0x7c; >> + break; >> + } >> + >> + s->rate = 0x7; /* 8Hz */ >> + s->status = 0; >> +} >> + >> +static int tmp421_init(I2CSlave *i2c) >> +{ >> + TMP421State *s = TMP421(i2c); >> + >> + tmp421_reset(&s->i2c); >> + >> + return 0; >> +} >> + >> +static void tmp421_initfn(Object *obj) >> +{ >> + object_property_add(obj, "temperature0", "int", >> + tmp421_get_temperature, >> + tmp421_set_temperature, NULL, NULL, NULL); >> + object_property_add(obj, "temperature1", "int", >> + tmp421_get_temperature, >> + tmp421_set_temperature, NULL, NULL, NULL); >> + object_property_add(obj, "temperature2", "int", >> + tmp421_get_temperature, >> + tmp421_set_temperature, NULL, NULL, NULL); >> + object_property_add(obj, "temperature3", "int", >> + tmp421_get_temperature, >> + tmp421_set_temperature, NULL, NULL, NULL); >> +} >> + >> +static void tmp421_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass); >> + TMP421Class *sc = TMP421_CLASS(klass); >> + >> + k->init = tmp421_init; >> + k->event = tmp421_event; >> + k->recv = tmp421_rx; >> + k->send = tmp421_tx; >> + dc->vmsd = &vmstate_tmp421; >> + sc->dev = (DeviceInfo *) data; >> +} >> + >> +static const TypeInfo tmp421_info = { >> + .name = TYPE_TMP421, >> + .parent = TYPE_I2C_SLAVE, >> + .instance_size = sizeof(TMP421State), >> + .instance_init = tmp421_initfn, >> + .class_init = tmp421_class_init, >> +}; >> + >> +static void tmp421_register_types(void) >> +{ >> + int i; >> + >> + type_register_static(&tmp421_info); >> + for (i = 0; i < ARRAY_SIZE(devices); ++i) { >> + TypeInfo ti = { >> + .name = devices[i].name, >> + .parent = TYPE_TMP421, >> + .class_init = tmp421_class_init, >> + .class_data = (void *) &devices[i], >> + }; >> + type_register(&ti); >> + } >> +} >> + >> +type_init(tmp421_register_types)
On 05/23/2016 05:53 AM, Andrew Jeffery wrote: > Hi Cédric, > > On Fri, 2016-05-20 at 18:31 +0200, Cédric Le Goater wrote: >> Largely inspired by the TMP105 temperature sensor, this patch brings >> to Qemu a model for TMP42{1,2,3} temperature sensors. >> >> Specs can be found here : >> >> http://www.ti.com/lit/gpn/tmp421 >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/misc/Makefile.objs | 1 + >> hw/misc/tmp421.c | 395 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 396 insertions(+) >> create mode 100644 hw/misc/tmp421.c >> >> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs >> index 1faf163c59f3..e50596965b03 100644 >> --- a/hw/misc/Makefile.objs >> +++ b/hw/misc/Makefile.objs >> @@ -1,6 +1,7 @@ >> common-obj-$(CONFIG_APPLESMC) += applesmc.o >> common-obj-$(CONFIG_MAX111X) += max111x.o >> common-obj-$(CONFIG_TMP105) += tmp105.o >> +common-obj-$(CONFIG_ASPEED_SOC) += tmp421.o >> common-obj-$(CONFIG_ISA_DEBUG) += debugexit.o >> common-obj-$(CONFIG_SGA) += sga.o >> common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o >> diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c >> new file mode 100644 >> index 000000000000..e385ce053f5e >> --- /dev/null >> +++ b/hw/misc/tmp421.c >> @@ -0,0 +1,395 @@ >> +/* >> + * Texas Instruments TMP421 temperature sensor. >> + * >> + * Copyright (c) 2016 IBM Corporation. >> + * >> + * Largely inspired by : >> + * >> + * Texas Instruments TMP105 temperature sensor. >> + * >> + * Copyright (C) 2008 Nokia Corporation >> + * Written by Andrzej Zaborowski <andrew@openedhand.com> >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License as >> + * published by the Free Software Foundation; either version 2 or >> + * (at your option) version 3 of the License. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License along >> + * with this program; if not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "hw/hw.h" >> +#include "hw/i2c/i2c.h" >> +#include "qapi/error.h" >> +#include "qapi/visitor.h" >> + >> +/* Manufacturer / Device ID's */ >> +#define TMP421_MANUFACTURER_ID 0x55 >> +#define TMP421_DEVICE_ID 0x21 >> +#define TMP422_DEVICE_ID 0x22 >> +#define TMP423_DEVICE_ID 0x23 >> + >> +typedef struct DeviceInfo { >> + int model; >> + const char *name; >> +} DeviceInfo; >> + >> +static const DeviceInfo devices[] = { >> + { TMP421_DEVICE_ID, "tmp421" }, >> + { TMP422_DEVICE_ID, "tmp422" }, >> + { TMP423_DEVICE_ID, "tmp423" }, >> +}; >> + >> +typedef struct TMP421State { >> + /*< private >*/ >> + I2CSlave i2c; >> + /*< public >*/ >> + >> + int16_t temperature[4]; >> + >> + uint8_t status; >> + uint8_t config[2]; >> + uint8_t rate; >> + >> + uint8_t len; > > Given the starting point of the tmp105 code the patch looks okay, but I > was a bit thrown by the use of the 'len' member as what I'd consider an > index. For instance we reset len to zero in tmp421_event() after > populating buf, and then the data in buf is presumably sent out on a > recv transaction which again starts incrementing len. len is also > incremented when we don't interact with buf, e.g. when we instead > assign to pointer. It feels like it could be prone to bugs, and > 'cb5ef3fa1871 tmp105: Fix I2C protocol bug' suggests that might not be > an unreasonable feeling. > > But given the code already exists in tmp105 maybe it's fine? So, I took my time to check this but yes, I think the code is fine. However, tmp421 does not need to support 2 bytes writes so we can simplify tmp421_tx() : static int tmp421_tx(I2CSlave *i2c, uint8_t data) { TMP421State *s = TMP421(i2c); if (s->len == 0) { /* first byte is the register pointer for a read or write * operation */ s->pointer = data; s->len++; } else if (s->len == 1) { /* second byte is the data to write. The device only supports * one byte writes */ s->buf[0] = data; tmp421_write(s); } return 0; } and tmp421 needs to support 2 bytes reads, so we need to extend a bit tmp421_read() when the temperatures are are. Linux does not use it so I guess we should use a command line tool to test. I will send an updated patch for the TMP42{1,2,3} device with a larger patchset I am working on for Aspeed support. That is for 2.9. Thanks, C. > Cheers, > > Andrew > >> + uint8_t buf[2]; >> + uint8_t pointer; >> + >> +} TMP421State; >> + >> +typedef struct TMP421Class { >> + I2CSlaveClass parent_class; >> + DeviceInfo *dev; >> +} TMP421Class; >> + >> +#define TYPE_TMP421 "tmp421-generic" >> +#define TMP421(obj) OBJECT_CHECK(TMP421State, (obj), TYPE_TMP421) >> + >> +#define TMP421_CLASS(klass) \ >> + OBJECT_CLASS_CHECK(TMP421Class, (klass), TYPE_TMP421) >> +#define TMP421_GET_CLASS(obj) \ >> + OBJECT_GET_CLASS(TMP421Class, (obj), TYPE_TMP421) >> + >> +/* the TMP421 registers */ >> +#define TMP421_STATUS_REG 0x08 >> +#define TMP421_STATUS_BUSY (1 << 7) >> +#define TMP421_CONFIG_REG_1 0x09 >> +#define TMP421_CONFIG_RANGE (1 << 2) >> +#define TMP421_CONFIG_SHUTDOWN (1 << 6) >> +#define TMP421_CONFIG_REG_2 0x0A >> +#define TMP421_CONFIG_RC (1 << 2) >> +#define TMP421_CONFIG_LEN (1 << 3) >> +#define TMP421_CONFIG_REN (1 << 4) >> +#define TMP421_CONFIG_REN2 (1 << 5) >> +#define TMP421_CONFIG_REN3 (1 << 6) >> + >> +#define TMP421_CONVERSION_RATE_REG 0x0B >> +#define TMP421_ONE_SHOT 0x0F >> + >> +#define TMP421_RESET 0xFC >> +#define TMP421_MANUFACTURER_ID_REG 0xFE >> +#define TMP421_DEVICE_ID_REG 0xFF >> + >> +#define TMP421_TEMP_MSB0 0x00 >> +#define TMP421_TEMP_MSB1 0x01 >> +#define TMP421_TEMP_MSB2 0x02 >> +#define TMP421_TEMP_MSB3 0x03 >> +#define TMP421_TEMP_LSB0 0x10 >> +#define TMP421_TEMP_LSB1 0x11 >> +#define TMP421_TEMP_LSB2 0x12 >> +#define TMP421_TEMP_LSB3 0x13 >> + >> +static const int32_t mins[2] = { -40000, -55000 }; >> +static const int32_t maxs[2] = { 127000, 150000 }; >> + >> +static void tmp421_get_temperature(Object *obj, Visitor *v, const char *name, >> + void *opaque, Error **errp) >> +{ >> + TMP421State *s = TMP421(obj); >> + bool ext_range = (s->config[0] & TMP421_CONFIG_RANGE); >> + int offset = ext_range * 64 * 256; >> + int64_t value; >> + int tempid; >> + >> + if (sscanf(name, "temperature%d", &tempid) != 1) { >> + error_setg(errp, "error reading %s: %m", name); >> + return; >> + } >> + >> + if (tempid >= 4 || tempid < 0) { >> + error_setg(errp, "error reading %s", name); >> + return; >> + } >> + >> + value = ((s->temperature[tempid] - offset) * 1000 + 128) / 256; >> + >> + visit_type_int(v, name, &value, errp); >> +} >> + >> +/* Units are 0.001 centigrades relative to 0 C. s->temperature is 8.8 >> + * fixed point, so units are 1/256 centigrades. A simple ratio will do. >> + */ >> +static void tmp421_set_temperature(Object *obj, Visitor *v, const char *name, >> + void *opaque, Error **errp) >> +{ >> + TMP421State *s = TMP421(obj); >> + Error *local_err = NULL; >> + int64_t temp; >> + bool ext_range = (s->config[0] & TMP421_CONFIG_RANGE); >> + int offset = ext_range * 64 * 256; >> + int tempid; >> + >> + visit_type_int(v, name, &temp, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return; >> + } >> + >> + if (temp >= maxs[ext_range] || temp < mins[ext_range]) { >> + error_setg(errp, "value %" PRId64 ".%03" PRIu64 " °C is out of range", >> + temp / 1000, temp % 1000); >> + return; >> + } >> + >> + if (sscanf(name, "temperature%d", &tempid) != 1) { >> + error_setg(errp, "error reading %s: %m", name); >> + return; >> + } >> + >> + if (tempid >= 4 || tempid < 0) { >> + error_setg(errp, "error reading %s", name); >> + return; >> + } >> + >> + s->temperature[tempid] = (int16_t) ((temp * 256 - 128) / 1000) + offset; >> +} >> + >> +static void tmp421_read(TMP421State *s) >> +{ >> + TMP421Class *sc = TMP421_GET_CLASS(s); >> + >> + s->len = 0; >> + >> + switch (s->pointer) { >> + case TMP421_MANUFACTURER_ID_REG: >> + s->buf[s->len++] = TMP421_MANUFACTURER_ID; >> + break; >> + case TMP421_DEVICE_ID_REG: >> + s->buf[s->len++] = sc->dev->model; >> + break; >> + case TMP421_CONFIG_REG_1: >> + s->buf[s->len++] = s->config[0]; >> + break; >> + case TMP421_CONFIG_REG_2: >> + s->buf[s->len++] = s->config[1]; >> + break; >> + case TMP421_CONVERSION_RATE_REG: >> + s->buf[s->len++] = s->rate; >> + break; >> + case TMP421_STATUS_REG: >> + s->buf[s->len++] = s->status; >> + break; >> + >> + /* FIXME: check for channel enablement in config registers */ >> + case TMP421_TEMP_MSB0: >> + s->buf[s->len++] = (((uint16_t) s->temperature[0]) >> 8); >> + break; >> + case TMP421_TEMP_MSB1: >> + s->buf[s->len++] = (((uint16_t) s->temperature[1]) >> 8); >> + break; >> + case TMP421_TEMP_MSB2: >> + s->buf[s->len++] = (((uint16_t) s->temperature[2]) >> 8); >> + break; >> + case TMP421_TEMP_MSB3: >> + s->buf[s->len++] = (((uint16_t) s->temperature[3]) >> 8); >> + break; >> + case TMP421_TEMP_LSB0: >> + s->buf[s->len++] = (((uint16_t) s->temperature[0]) >> 0) & 0xf0; >> + break; >> + case TMP421_TEMP_LSB1: >> + s->buf[s->len++] = (((uint16_t) s->temperature[1]) >> 0) & 0xf0; >> + break; >> + case TMP421_TEMP_LSB2: >> + s->buf[s->len++] = (((uint16_t) s->temperature[2]) >> 0) & 0xf0; >> + break; >> + case TMP421_TEMP_LSB3: >> + s->buf[s->len++] = (((uint16_t) s->temperature[3]) >> 0) & 0xf0; >> + break; >> + } >> +} >> + >> +static void tmp421_reset(I2CSlave *i2c); >> + >> +static void tmp421_write(TMP421State *s) >> +{ >> + switch (s->pointer) { >> + case TMP421_CONVERSION_RATE_REG: >> + s->rate = s->buf[0]; >> + break; >> + case TMP421_CONFIG_REG_1: >> + s->config[0] = s->buf[0]; >> + break; >> + case TMP421_CONFIG_REG_2: >> + s->config[1] = s->buf[0]; >> + break; >> + case TMP421_RESET: >> + tmp421_reset(I2C_SLAVE(s)); >> + break; >> + } >> +} >> + >> +static int tmp421_rx(I2CSlave *i2c) >> +{ >> + TMP421State *s = TMP421(i2c); >> + >> + if (s->len < 2) { >> + return s->buf[s->len++]; >> + } else { >> + return 0xff; >> + } >> +} >> + >> +static int tmp421_tx(I2CSlave *i2c, uint8_t data) >> +{ >> + TMP421State *s = TMP421(i2c); >> + >> + if (s->len == 0) { >> + s->pointer = data; >> + s->len++; >> + } else { >> + if (s->len <= 2) { >> + s->buf[s->len - 1] = data; >> + } >> + s->len++; >> + tmp421_write(s); >> + } >> + >> + return 0; >> +} >> + >> +static void tmp421_event(I2CSlave *i2c, enum i2c_event event) >> +{ >> + TMP421State *s = TMP421(i2c); >> + >> + if (event == I2C_START_RECV) { >> + tmp421_read(s); >> + } >> + >> + s->len = 0; >> +} >> + >> +static const VMStateDescription vmstate_tmp421 = { >> + .name = "TMP421", >> + .version_id = 0, >> + .minimum_version_id = 0, >> + .fields = (VMStateField[]) { >> + VMSTATE_UINT8(len, TMP421State), >> + VMSTATE_UINT8_ARRAY(buf, TMP421State, 2), >> + VMSTATE_UINT8(pointer, TMP421State), >> + VMSTATE_UINT8_ARRAY(config, TMP421State, 2), >> + VMSTATE_UINT8(status, TMP421State), >> + VMSTATE_UINT8(rate, TMP421State), >> + VMSTATE_INT16_ARRAY(temperature, TMP421State, 4), >> + VMSTATE_I2C_SLAVE(i2c, TMP421State), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +static void tmp421_reset(I2CSlave *i2c) >> +{ >> + TMP421State *s = TMP421(i2c); >> + TMP421Class *sc = TMP421_GET_CLASS(s); >> + >> + memset(s->temperature, 0, sizeof(s->temperature)); >> + s->pointer = 0; >> + >> + s->config[0] = 0; /* TMP421_CONFIG_RANGE */ >> + >> + /* resistance correction and channel enablement */ >> + switch (sc->dev->model) { >> + case TMP421_DEVICE_ID: >> + s->config[1] = 0x1c; >> + break; >> + case TMP422_DEVICE_ID: >> + s->config[1] = 0x3c; >> + break; >> + case TMP423_DEVICE_ID: >> + s->config[1] = 0x7c; >> + break; >> + } >> + >> + s->rate = 0x7; /* 8Hz */ >> + s->status = 0; >> +} >> + >> +static int tmp421_init(I2CSlave *i2c) >> +{ >> + TMP421State *s = TMP421(i2c); >> + >> + tmp421_reset(&s->i2c); >> + >> + return 0; >> +} >> + >> +static void tmp421_initfn(Object *obj) >> +{ >> + object_property_add(obj, "temperature0", "int", >> + tmp421_get_temperature, >> + tmp421_set_temperature, NULL, NULL, NULL); >> + object_property_add(obj, "temperature1", "int", >> + tmp421_get_temperature, >> + tmp421_set_temperature, NULL, NULL, NULL); >> + object_property_add(obj, "temperature2", "int", >> + tmp421_get_temperature, >> + tmp421_set_temperature, NULL, NULL, NULL); >> + object_property_add(obj, "temperature3", "int", >> + tmp421_get_temperature, >> + tmp421_set_temperature, NULL, NULL, NULL); >> +} >> + >> +static void tmp421_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass); >> + TMP421Class *sc = TMP421_CLASS(klass); >> + >> + k->init = tmp421_init; >> + k->event = tmp421_event; >> + k->recv = tmp421_rx; >> + k->send = tmp421_tx; >> + dc->vmsd = &vmstate_tmp421; >> + sc->dev = (DeviceInfo *) data; >> +} >> + >> +static const TypeInfo tmp421_info = { >> + .name = TYPE_TMP421, >> + .parent = TYPE_I2C_SLAVE, >> + .instance_size = sizeof(TMP421State), >> + .instance_init = tmp421_initfn, >> + .class_init = tmp421_class_init, >> +}; >> + >> +static void tmp421_register_types(void) >> +{ >> + int i; >> + >> + type_register_static(&tmp421_info); >> + for (i = 0; i < ARRAY_SIZE(devices); ++i) { >> + TypeInfo ti = { >> + .name = devices[i].name, >> + .parent = TYPE_TMP421, >> + .class_init = tmp421_class_init, >> + .class_data = (void *) &devices[i], >> + }; >> + type_register(&ti); >> + } >> +} >> + >> +type_init(tmp421_register_types)
On Mon, 2016-11-14 at 08:14 +0100, Cédric Le Goater wrote: > > Given the starting point of the tmp105 code the patch looks okay, but I > > was a bit thrown by the use of the 'len' member as what I'd consider an > > index. For instance we reset len to zero in tmp421_event() after > > populating buf, and then the data in buf is presumably sent out on a > > recv transaction which again starts incrementing len. len is also > > incremented when we don't interact with buf, e.g. when we instead > > assign to pointer. It feels like it could be prone to bugs, and > > 'cb5ef3fa1871 tmp105: Fix I2C protocol bug' suggests that might not be > > an unreasonable feeling. > > > > But given the code already exists in tmp105 maybe it's fine? > > So, I took my time to check this but yes, I think the code is fine. Yes, from memory it was okay, just not as obvious as I'd hoped it to be. > > However, tmp421 does not need to support 2 bytes writes so we can > simplify tmp421_tx() : > > static int tmp421_tx(I2CSlave *i2c, uint8_t data) > { > TMP421State *s = TMP421(i2c); > > if (s->len == 0) { > /* first byte is the register pointer for a read or write > * operation */ > s->pointer = data; > s->len++; > } else if (s->len == 1) { > /* second byte is the data to write. The device only supports > * one byte writes */ > s->buf[0] = data; > tmp421_write(s); > } > > return 0; > } > > and tmp421 needs to support 2 bytes reads, so we need to extend a bit > tmp421_read() when the temperatures are are. Linux does not use it > so I guess we should use a command line tool to test. Okay. > > I will send an updated patch for the TMP42{1,2,3} device with a larger > patchset I am working on for Aspeed support. That is for 2.9. > Okay, I'll review again then. Thanks, Andrew > Thanks, > > C.
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs index 1faf163c59f3..e50596965b03 100644 --- a/hw/misc/Makefile.objs +++ b/hw/misc/Makefile.objs @@ -1,6 +1,7 @@ common-obj-$(CONFIG_APPLESMC) += applesmc.o common-obj-$(CONFIG_MAX111X) += max111x.o common-obj-$(CONFIG_TMP105) += tmp105.o +common-obj-$(CONFIG_ASPEED_SOC) += tmp421.o common-obj-$(CONFIG_ISA_DEBUG) += debugexit.o common-obj-$(CONFIG_SGA) += sga.o common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c new file mode 100644 index 000000000000..e385ce053f5e --- /dev/null +++ b/hw/misc/tmp421.c @@ -0,0 +1,395 @@ +/* + * Texas Instruments TMP421 temperature sensor. + * + * Copyright (c) 2016 IBM Corporation. + * + * Largely inspired by : + * + * Texas Instruments TMP105 temperature sensor. + * + * Copyright (C) 2008 Nokia Corporation + * Written by Andrzej Zaborowski <andrew@openedhand.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 or + * (at your option) version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, see <http://www.gnu.org/licenses/>. + */ + +#include "qemu/osdep.h" +#include "hw/hw.h" +#include "hw/i2c/i2c.h" +#include "qapi/error.h" +#include "qapi/visitor.h" + +/* Manufacturer / Device ID's */ +#define TMP421_MANUFACTURER_ID 0x55 +#define TMP421_DEVICE_ID 0x21 +#define TMP422_DEVICE_ID 0x22 +#define TMP423_DEVICE_ID 0x23 + +typedef struct DeviceInfo { + int model; + const char *name; +} DeviceInfo; + +static const DeviceInfo devices[] = { + { TMP421_DEVICE_ID, "tmp421" }, + { TMP422_DEVICE_ID, "tmp422" }, + { TMP423_DEVICE_ID, "tmp423" }, +}; + +typedef struct TMP421State { + /*< private >*/ + I2CSlave i2c; + /*< public >*/ + + int16_t temperature[4]; + + uint8_t status; + uint8_t config[2]; + uint8_t rate; + + uint8_t len; + uint8_t buf[2]; + uint8_t pointer; + +} TMP421State; + +typedef struct TMP421Class { + I2CSlaveClass parent_class; + DeviceInfo *dev; +} TMP421Class; + +#define TYPE_TMP421 "tmp421-generic" +#define TMP421(obj) OBJECT_CHECK(TMP421State, (obj), TYPE_TMP421) + +#define TMP421_CLASS(klass) \ + OBJECT_CLASS_CHECK(TMP421Class, (klass), TYPE_TMP421) +#define TMP421_GET_CLASS(obj) \ + OBJECT_GET_CLASS(TMP421Class, (obj), TYPE_TMP421) + +/* the TMP421 registers */ +#define TMP421_STATUS_REG 0x08 +#define TMP421_STATUS_BUSY (1 << 7) +#define TMP421_CONFIG_REG_1 0x09 +#define TMP421_CONFIG_RANGE (1 << 2) +#define TMP421_CONFIG_SHUTDOWN (1 << 6) +#define TMP421_CONFIG_REG_2 0x0A +#define TMP421_CONFIG_RC (1 << 2) +#define TMP421_CONFIG_LEN (1 << 3) +#define TMP421_CONFIG_REN (1 << 4) +#define TMP421_CONFIG_REN2 (1 << 5) +#define TMP421_CONFIG_REN3 (1 << 6) + +#define TMP421_CONVERSION_RATE_REG 0x0B +#define TMP421_ONE_SHOT 0x0F + +#define TMP421_RESET 0xFC +#define TMP421_MANUFACTURER_ID_REG 0xFE +#define TMP421_DEVICE_ID_REG 0xFF + +#define TMP421_TEMP_MSB0 0x00 +#define TMP421_TEMP_MSB1 0x01 +#define TMP421_TEMP_MSB2 0x02 +#define TMP421_TEMP_MSB3 0x03 +#define TMP421_TEMP_LSB0 0x10 +#define TMP421_TEMP_LSB1 0x11 +#define TMP421_TEMP_LSB2 0x12 +#define TMP421_TEMP_LSB3 0x13 + +static const int32_t mins[2] = { -40000, -55000 }; +static const int32_t maxs[2] = { 127000, 150000 }; + +static void tmp421_get_temperature(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) +{ + TMP421State *s = TMP421(obj); + bool ext_range = (s->config[0] & TMP421_CONFIG_RANGE); + int offset = ext_range * 64 * 256; + int64_t value; + int tempid; + + if (sscanf(name, "temperature%d", &tempid) != 1) { + error_setg(errp, "error reading %s: %m", name); + return; + } + + if (tempid >= 4 || tempid < 0) { + error_setg(errp, "error reading %s", name); + return; + } + + value = ((s->temperature[tempid] - offset) * 1000 + 128) / 256; + + visit_type_int(v, name, &value, errp); +} + +/* Units are 0.001 centigrades relative to 0 C. s->temperature is 8.8 + * fixed point, so units are 1/256 centigrades. A simple ratio will do. + */ +static void tmp421_set_temperature(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) +{ + TMP421State *s = TMP421(obj); + Error *local_err = NULL; + int64_t temp; + bool ext_range = (s->config[0] & TMP421_CONFIG_RANGE); + int offset = ext_range * 64 * 256; + int tempid; + + visit_type_int(v, name, &temp, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + + if (temp >= maxs[ext_range] || temp < mins[ext_range]) { + error_setg(errp, "value %" PRId64 ".%03" PRIu64 " °C is out of range", + temp / 1000, temp % 1000); + return; + } + + if (sscanf(name, "temperature%d", &tempid) != 1) { + error_setg(errp, "error reading %s: %m", name); + return; + } + + if (tempid >= 4 || tempid < 0) { + error_setg(errp, "error reading %s", name); + return; + } + + s->temperature[tempid] = (int16_t) ((temp * 256 - 128) / 1000) + offset; +} + +static void tmp421_read(TMP421State *s) +{ + TMP421Class *sc = TMP421_GET_CLASS(s); + + s->len = 0; + + switch (s->pointer) { + case TMP421_MANUFACTURER_ID_REG: + s->buf[s->len++] = TMP421_MANUFACTURER_ID; + break; + case TMP421_DEVICE_ID_REG: + s->buf[s->len++] = sc->dev->model; + break; + case TMP421_CONFIG_REG_1: + s->buf[s->len++] = s->config[0]; + break; + case TMP421_CONFIG_REG_2: + s->buf[s->len++] = s->config[1]; + break; + case TMP421_CONVERSION_RATE_REG: + s->buf[s->len++] = s->rate; + break; + case TMP421_STATUS_REG: + s->buf[s->len++] = s->status; + break; + + /* FIXME: check for channel enablement in config registers */ + case TMP421_TEMP_MSB0: + s->buf[s->len++] = (((uint16_t) s->temperature[0]) >> 8); + break; + case TMP421_TEMP_MSB1: + s->buf[s->len++] = (((uint16_t) s->temperature[1]) >> 8); + break; + case TMP421_TEMP_MSB2: + s->buf[s->len++] = (((uint16_t) s->temperature[2]) >> 8); + break; + case TMP421_TEMP_MSB3: + s->buf[s->len++] = (((uint16_t) s->temperature[3]) >> 8); + break; + case TMP421_TEMP_LSB0: + s->buf[s->len++] = (((uint16_t) s->temperature[0]) >> 0) & 0xf0; + break; + case TMP421_TEMP_LSB1: + s->buf[s->len++] = (((uint16_t) s->temperature[1]) >> 0) & 0xf0; + break; + case TMP421_TEMP_LSB2: + s->buf[s->len++] = (((uint16_t) s->temperature[2]) >> 0) & 0xf0; + break; + case TMP421_TEMP_LSB3: + s->buf[s->len++] = (((uint16_t) s->temperature[3]) >> 0) & 0xf0; + break; + } +} + +static void tmp421_reset(I2CSlave *i2c); + +static void tmp421_write(TMP421State *s) +{ + switch (s->pointer) { + case TMP421_CONVERSION_RATE_REG: + s->rate = s->buf[0]; + break; + case TMP421_CONFIG_REG_1: + s->config[0] = s->buf[0]; + break; + case TMP421_CONFIG_REG_2: + s->config[1] = s->buf[0]; + break; + case TMP421_RESET: + tmp421_reset(I2C_SLAVE(s)); + break; + } +} + +static int tmp421_rx(I2CSlave *i2c) +{ + TMP421State *s = TMP421(i2c); + + if (s->len < 2) { + return s->buf[s->len++]; + } else { + return 0xff; + } +} + +static int tmp421_tx(I2CSlave *i2c, uint8_t data) +{ + TMP421State *s = TMP421(i2c); + + if (s->len == 0) { + s->pointer = data; + s->len++; + } else { + if (s->len <= 2) { + s->buf[s->len - 1] = data; + } + s->len++; + tmp421_write(s); + } + + return 0; +} + +static void tmp421_event(I2CSlave *i2c, enum i2c_event event) +{ + TMP421State *s = TMP421(i2c); + + if (event == I2C_START_RECV) { + tmp421_read(s); + } + + s->len = 0; +} + +static const VMStateDescription vmstate_tmp421 = { + .name = "TMP421", + .version_id = 0, + .minimum_version_id = 0, + .fields = (VMStateField[]) { + VMSTATE_UINT8(len, TMP421State), + VMSTATE_UINT8_ARRAY(buf, TMP421State, 2), + VMSTATE_UINT8(pointer, TMP421State), + VMSTATE_UINT8_ARRAY(config, TMP421State, 2), + VMSTATE_UINT8(status, TMP421State), + VMSTATE_UINT8(rate, TMP421State), + VMSTATE_INT16_ARRAY(temperature, TMP421State, 4), + VMSTATE_I2C_SLAVE(i2c, TMP421State), + VMSTATE_END_OF_LIST() + } +}; + +static void tmp421_reset(I2CSlave *i2c) +{ + TMP421State *s = TMP421(i2c); + TMP421Class *sc = TMP421_GET_CLASS(s); + + memset(s->temperature, 0, sizeof(s->temperature)); + s->pointer = 0; + + s->config[0] = 0; /* TMP421_CONFIG_RANGE */ + + /* resistance correction and channel enablement */ + switch (sc->dev->model) { + case TMP421_DEVICE_ID: + s->config[1] = 0x1c; + break; + case TMP422_DEVICE_ID: + s->config[1] = 0x3c; + break; + case TMP423_DEVICE_ID: + s->config[1] = 0x7c; + break; + } + + s->rate = 0x7; /* 8Hz */ + s->status = 0; +} + +static int tmp421_init(I2CSlave *i2c) +{ + TMP421State *s = TMP421(i2c); + + tmp421_reset(&s->i2c); + + return 0; +} + +static void tmp421_initfn(Object *obj) +{ + object_property_add(obj, "temperature0", "int", + tmp421_get_temperature, + tmp421_set_temperature, NULL, NULL, NULL); + object_property_add(obj, "temperature1", "int", + tmp421_get_temperature, + tmp421_set_temperature, NULL, NULL, NULL); + object_property_add(obj, "temperature2", "int", + tmp421_get_temperature, + tmp421_set_temperature, NULL, NULL, NULL); + object_property_add(obj, "temperature3", "int", + tmp421_get_temperature, + tmp421_set_temperature, NULL, NULL, NULL); +} + +static void tmp421_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass); + TMP421Class *sc = TMP421_CLASS(klass); + + k->init = tmp421_init; + k->event = tmp421_event; + k->recv = tmp421_rx; + k->send = tmp421_tx; + dc->vmsd = &vmstate_tmp421; + sc->dev = (DeviceInfo *) data; +} + +static const TypeInfo tmp421_info = { + .name = TYPE_TMP421, + .parent = TYPE_I2C_SLAVE, + .instance_size = sizeof(TMP421State), + .instance_init = tmp421_initfn, + .class_init = tmp421_class_init, +}; + +static void tmp421_register_types(void) +{ + int i; + + type_register_static(&tmp421_info); + for (i = 0; i < ARRAY_SIZE(devices); ++i) { + TypeInfo ti = { + .name = devices[i].name, + .parent = TYPE_TMP421, + .class_init = tmp421_class_init, + .class_data = (void *) &devices[i], + }; + type_register(&ti); + } +} + +type_init(tmp421_register_types)
Largely inspired by the TMP105 temperature sensor, this patch brings to Qemu a model for TMP42{1,2,3} temperature sensors. Specs can be found here : http://www.ti.com/lit/gpn/tmp421 Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/misc/Makefile.objs | 1 + hw/misc/tmp421.c | 395 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 396 insertions(+) create mode 100644 hw/misc/tmp421.c