From 790b9023f7f12273c3929e950fc8ba111ee9abe4 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Wed, 14 Dec 2022 18:35:03 +0000 Subject: [PATCH] ti-tps6598x: Use the TX identity to set the PD VID&PID to the correct values This allows us to match the firmware stream to the dock model perfectly, without relying on the oUID and CONFIG being set to specific OEM values. --- plugins/ti-tps6598x/README.md | 7 +- plugins/ti-tps6598x/fu-ti-tps6598x-common.h | 1 + plugins/ti-tps6598x/fu-ti-tps6598x-device.c | 82 ++++++++----------- .../ti-tps6598x/fu-ti-tps6598x-pd-device.c | 82 ++++++++++++------- 4 files changed, 89 insertions(+), 83 deletions(-) diff --git a/plugins/ti-tps6598x/README.md b/plugins/ti-tps6598x/README.md index f1fcf80c9..3420a848c 100644 --- a/plugins/ti-tps6598x/README.md +++ b/plugins/ti-tps6598x/README.md @@ -23,11 +23,10 @@ These devices use the standard USB DeviceInstanceId values, e.g. * `USB\VID_0451&PID_ACE1` * `USB\VID_0451` -Devices also have additional instance IDs which corresponds to the UID, oUID and OTP config, e.g. +Child devices also have an additional instance IDs which corresponds to the index, e.g. -* `USB\VID_0451&PID_ACE1&UID_50bf5616c608a6b98b4169b220d9a5b8` -* `USB\VID_0451&PID_ACE1&OUID_2200000000000000` -* `USB\VID_0451&PID_0451&CONFIG_00000000A206000030000000` +* `USB\VID_2188&PID_5988&REV_0714&PD_00 +* `USB\VID_2188&PID_5988&PD_00 ## Update Behavior diff --git a/plugins/ti-tps6598x/fu-ti-tps6598x-common.h b/plugins/ti-tps6598x/fu-ti-tps6598x-common.h index 51ec0fc01..c71a409f5 100644 --- a/plugins/ti-tps6598x/fu-ti-tps6598x-common.h +++ b/plugins/ti-tps6598x/fu-ti-tps6598x-common.h @@ -27,6 +27,7 @@ #define TI_TPS6598X_REGISTER_OTP_CONFIG 0x2D /* ro, 12 bytes */ #define TI_TPS6598X_REGISTER_BUILD_IDENTIFIER 0x2E /* ro, 64 bytes */ #define TI_TPS6598X_REGISTER_DEVICE_INFO 0x2F /* ro, 47 bytes */ +#define TI_TPS6598X_REGISTER_TX_IDENTITY 0x47 /* rw, 49 bytes */ #define TI_TPS6598X_SFWI_SUCCESS 0x0 #define TI_TPS6598X_SFWI_FAIL_FLASH_ERROR_OR_BUSY 0x4 diff --git a/plugins/ti-tps6598x/fu-ti-tps6598x-device.c b/plugins/ti-tps6598x/fu-ti-tps6598x-device.c index 0c27bcf59..e2f86400f 100644 --- a/plugins/ti-tps6598x/fu-ti-tps6598x-device.c +++ b/plugins/ti-tps6598x/fu-ti-tps6598x-device.c @@ -14,6 +14,8 @@ struct _FuTiTps6598xDevice { FuUsbDevice parent_instance; + gchar *uid; + gchar *ouid; }; G_DEFINE_TYPE(FuTiTps6598xDevice, fu_ti_tps6598x_device, FU_TYPE_USB_DEVICE) @@ -25,6 +27,14 @@ G_DEFINE_TYPE(FuTiTps6598xDevice, fu_ti_tps6598x_device, FU_TYPE_USB_DEVICE) #define TI_TPS6598X_USB_REQUEST_READ 0xFE #define TI_TPS6598X_USB_BUFFER_SIZE 8 /* bytes */ +static void +fu_ti_tps6598x_device_to_string(FuDevice *device, guint idt, GString *str) +{ + FuTiTps6598xDevice *self = FU_TI_TPS6598X_DEVICE(device); + fu_string_append(str, idt, "UID", self->uid); + fu_string_append(str, idt, "oUID", self->ouid); +} + /* read @length bytes from address @addr */ static GByteArray * fu_ti_tps6598x_device_usbep_read_raw(FuTiTps6598xDevice *self, @@ -465,61 +475,25 @@ fu_ti_tps6598x_device_ensure_mode(FuTiTps6598xDevice *self, GError **error) static gboolean fu_ti_tps6598x_device_ensure_uid(FuTiTps6598xDevice *self, GError **error) { - g_autofree gchar *str = NULL; - g_autoptr(GByteArray) buf = NULL; - - buf = fu_ti_tps6598x_device_usbep_read(self, TI_TPS6598X_REGISTER_UID, 16, error); + g_autoptr(GByteArray) buf = + fu_ti_tps6598x_device_usbep_read(self, TI_TPS6598X_REGISTER_UID, 16, error); if (buf == NULL) return FALSE; - str = fu_byte_array_to_string(buf); - fu_device_add_instance_str(FU_DEVICE(self), "UID", str); - return fu_device_build_instance_id(FU_DEVICE(self), - error, - "USB", - "VID", - "PID", - "UID", - NULL); + g_free(self->uid); + self->uid = fu_byte_array_to_string(buf); + return TRUE; } static gboolean fu_ti_tps6598x_device_ensure_ouid(FuTiTps6598xDevice *self, GError **error) { - g_autofree gchar *str = NULL; - g_autoptr(GByteArray) buf = NULL; - - buf = fu_ti_tps6598x_device_usbep_read(self, TI_TPS6598X_REGISTER_OUID, 8, error); + g_autoptr(GByteArray) buf = + fu_ti_tps6598x_device_usbep_read(self, TI_TPS6598X_REGISTER_OUID, 8, error); if (buf == NULL) return FALSE; - str = fu_byte_array_to_string(buf); - fu_device_add_instance_str(FU_DEVICE(self), "OUID", str); - return fu_device_build_instance_id(FU_DEVICE(self), - error, - "USB", - "VID", - "PID", - "OUID", - NULL); -} - -static gboolean -fu_ti_tps6598x_device_ensure_config(FuTiTps6598xDevice *self, GError **error) -{ - g_autofree gchar *str = NULL; - g_autoptr(GByteArray) buf = NULL; - - buf = fu_ti_tps6598x_device_usbep_read(self, TI_TPS6598X_REGISTER_OTP_CONFIG, 12, error); - if (buf == NULL) - return FALSE; - str = fu_byte_array_to_string(buf); - fu_device_add_instance_strup(FU_DEVICE(self), "CONFIG", str); - return fu_device_build_instance_id(FU_DEVICE(self), - error, - "USB", - "VID", - "PID", - "CONFIG", - NULL); + g_free(self->ouid); + self->ouid = fu_byte_array_to_string(buf); + return TRUE; } static gboolean @@ -558,10 +532,6 @@ fu_ti_tps6598x_device_setup(FuDevice *device, GError **error) g_prefix_error(error, "failed to read oUID: "); return FALSE; } - if (!fu_ti_tps6598x_device_ensure_config(self, error)) { - g_prefix_error(error, "failed to read OTP config: "); - return FALSE; - } /* create child PD devices */ for (guint i = 0; i < FU_TI_TPS6598X_PD_MAX; i++) { @@ -786,10 +756,22 @@ fu_ti_tps6598x_device_init(FuTiTps6598xDevice *self) fu_usb_device_add_interface(FU_USB_DEVICE(self), 0x0); } +static void +fu_ti_tps6598x_device_finalize(GObject *object) +{ + FuTiTps6598xDevice *self = FU_TI_TPS6598X_DEVICE(object); + g_free(self->uid); + g_free(self->ouid); + G_OBJECT_CLASS(fu_ti_tps6598x_device_parent_class)->finalize(object); +} + static void fu_ti_tps6598x_device_class_init(FuTiTps6598xDeviceClass *klass) { FuDeviceClass *klass_device = FU_DEVICE_CLASS(klass); + GObjectClass *object_class = G_OBJECT_CLASS(klass); + object_class->finalize = fu_ti_tps6598x_device_finalize; + klass_device->to_string = fu_ti_tps6598x_device_to_string; klass_device->write_firmware = fu_ti_tps6598x_device_write_firmware; klass_device->attach = fu_ti_tps6598x_device_attach; klass_device->setup = fu_ti_tps6598x_device_setup; diff --git a/plugins/ti-tps6598x/fu-ti-tps6598x-pd-device.c b/plugins/ti-tps6598x/fu-ti-tps6598x-pd-device.c index a69f244d2..f952bbb6d 100644 --- a/plugins/ti-tps6598x/fu-ti-tps6598x-pd-device.c +++ b/plugins/ti-tps6598x/fu-ti-tps6598x-pd-device.c @@ -22,31 +22,21 @@ static gboolean fu_ti_tps6598x_pd_device_probe(FuDevice *device, GError **error) { FuTiTps6598xPdDevice *self = FU_TI_TPS6598X_PD_DEVICE(device); - FuTiTps6598xDevice *parent = FU_TI_TPS6598X_DEVICE(fu_device_get_parent(device)); g_autofree gchar *name = g_strdup_printf("TPS6598X PD#%u", self->target); g_autofree gchar *logical_id = g_strdup_printf("PD%u", self->target); - - /* do as few register reads as possible as they are s...l...o...w... */ fu_device_set_name(device, name); fu_device_set_logical_id(device, logical_id); - - /* fake GUID */ - fu_device_add_instance_u16(device, "VID", fu_usb_device_get_vid(FU_USB_DEVICE(parent))); - fu_device_add_instance_u16(device, "PID", fu_usb_device_get_pid(FU_USB_DEVICE(parent))); fu_device_add_instance_u8(device, "PD", self->target); - return fu_device_build_instance_id(device, error, "USB", "VID", "PID", "PD", NULL); + return TRUE; } static gboolean -fu_ti_tps6598x_pd_device_setup(FuDevice *device, GError **error) +fu_ti_tps6598x_pd_device_ensure_version(FuTiTps6598xPdDevice *self, GError **error) { - FuTiTps6598xPdDevice *self = FU_TI_TPS6598X_PD_DEVICE(device); - FuTiTps6598xDevice *parent = FU_TI_TPS6598X_DEVICE(fu_device_get_parent(device)); + FuTiTps6598xDevice *parent = FU_TI_TPS6598X_DEVICE(fu_device_get_parent(FU_DEVICE(self))); g_autoptr(GByteArray) buf = NULL; - g_autofree gchar *version = NULL; - g_autofree gchar *config = NULL; + g_autofree gchar *str = NULL; - /* register reads are s...l...o...w... */ buf = fu_ti_tps6598x_device_read_target_register(parent, self->target, TI_TPS6598X_REGISTER_VERSION, @@ -54,33 +44,67 @@ fu_ti_tps6598x_pd_device_setup(FuDevice *device, GError **error) error); if (buf == NULL) return FALSE; - version = g_strdup_printf("%02X%02X.%02X.%02X", - buf->data[3], - buf->data[2], - buf->data[1], - buf->data[0]); - fu_device_set_version_format(device, FWUPD_VERSION_FORMAT_PLAIN); - fu_device_set_version(device, version); + str = g_strdup_printf("%02X%02X.%02X.%02X", + buf->data[3], + buf->data[2], + buf->data[1], + buf->data[0]); + fu_device_set_version(FU_DEVICE(self), str); + return TRUE; +} + +static gboolean +fu_ti_tps6598x_pd_device_ensure_tx_identity(FuTiTps6598xPdDevice *self, GError **error) +{ + FuTiTps6598xDevice *parent = FU_TI_TPS6598X_DEVICE(fu_device_get_parent(FU_DEVICE(self))); + guint16 val = 0; + g_autoptr(GByteArray) buf = NULL; - /* the PD OTP config should be unique enough */ buf = fu_ti_tps6598x_device_read_target_register(parent, self->target, - TI_TPS6598X_REGISTER_OTP_CONFIG, - 12, + TI_TPS6598X_REGISTER_TX_IDENTITY, + 47, error); if (buf == NULL) return FALSE; - config = fu_byte_array_to_string(buf); - fu_device_add_instance_strup(device, "CONFIG", config); + if (!fu_memread_uint16_safe(buf->data, buf->len, 0x01, &val, G_LITTLE_ENDIAN, error)) + return FALSE; + if (val != 0x0 && val != 0xFF) + fu_device_add_instance_u16(FU_DEVICE(self), "VID", val); + if (!fu_memread_uint16_safe(buf->data, buf->len, 0x0B, &val, G_LITTLE_ENDIAN, error)) + return FALSE; + if (val != 0x0 && val != 0xFF) + fu_device_add_instance_u16(FU_DEVICE(self), "PID", val); + if (!fu_memread_uint16_safe(buf->data, buf->len, 0x09, &val, G_LITTLE_ENDIAN, error)) + return FALSE; + if (val != 0x0 && val != 0xFF) + fu_device_add_instance_u16(FU_DEVICE(self), "REV", val); /* success */ - return fu_device_build_instance_id(device, + return TRUE; +} + +static gboolean +fu_ti_tps6598x_pd_device_setup(FuDevice *device, GError **error) +{ + FuTiTps6598xPdDevice *self = FU_TI_TPS6598X_PD_DEVICE(device); + + /* register reads are slow, so do as few as possible */ + if (!fu_ti_tps6598x_pd_device_ensure_version(self, error)) + return FALSE; + if (!fu_ti_tps6598x_pd_device_ensure_tx_identity(self, error)) + return FALSE; + + /* add new instance IDs */ + if (!fu_device_build_instance_id(FU_DEVICE(self), error, "USB", "VID", "PID", "PD", NULL)) + return FALSE; + return fu_device_build_instance_id(FU_DEVICE(self), error, "USB", "VID", "PID", + "REV", "PD", - "CONFIG", NULL); } @@ -93,7 +117,7 @@ fu_ti_tps6598x_pd_device_report_metadata_pre(FuDevice *device, GHashTable *metad /* this is too slow to do for each update... */ if (g_getenv("FWUPD_TI_TPS6598X_VERBOSE") == NULL) return; - for (guint i = 0; i < 0x30; i++) { + for (guint i = 0; i < 0x80; i++) { g_autoptr(GByteArray) buf = NULL; g_autoptr(GError) error_local = NULL;