From 1e35511dad649e71a5aff82f1304ddd7f700dea5 Mon Sep 17 00:00:00 2001 From: Linyu Yuan Date: Wed, 4 Nov 2020 09:46:31 +0800 Subject: [PATCH 1/6] ucsi: ucsi_glink: add more flags for clients during notification Add partner_change and connect flags when notifying clients about the partner information.This would be useful for some clients to modify their state/configuration. Change-Id: I85b601b7ea60dda2ac630c51a499767f5fb70351 Signed-off-by: Linyu Yuan --- drivers/usb/typec/ucsi/ucsi_glink.c | 6 ++++++ include/linux/usb/ucsi_glink.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c index 3f49e9d69051..aa8aea7f05cb 100644 --- a/drivers/usb/typec/ucsi/ucsi_glink.c +++ b/drivers/usb/typec/ucsi/ucsi_glink.c @@ -375,6 +375,12 @@ static void ucsi_qti_notify(struct ucsi_dev *udev, unsigned int offset, udev->constat_info.partner_usb = false; udev->constat_info.partner_alternate_mode = false; + udev->constat_info.partner_change = + status->change & UCSI_CONSTAT_PARTNER_CHANGE; + + udev->constat_info.connect = + status->flags & UCSI_CONSTAT_CONNECTED; + conn_partner_type = UCSI_CONSTAT_PARTNER_TYPE(status->flags); switch (conn_partner_type) { diff --git a/include/linux/usb/ucsi_glink.h b/include/linux/usb/ucsi_glink.h index 891ba957bb7b..bdd9d1d86412 100644 --- a/include/linux/usb/ucsi_glink.h +++ b/include/linux/usb/ucsi_glink.h @@ -13,6 +13,8 @@ struct ucsi_glink_constat_info { enum typec_accessory acc; bool partner_usb; bool partner_alternate_mode; + bool partner_change; + bool connect; }; struct notifier_block; From f9c1434696075a03e6691c8749c04fbc6f3037d8 Mon Sep 17 00:00:00 2001 From: Subbaraman Narayanamurthy Date: Fri, 5 Mar 2021 10:53:25 -0800 Subject: [PATCH 2/6] ucsi: ucsi_glink: Move debug log in ucsi_qti_glink_write() Currently we print a debug log in ucsi_qti_glink_write() only after it succeeds with an ACK from PPM. To debug some timeout issues, it is helpful to see the log printed before writing and waiting for the ACK. So, move it. Change-Id: I811bc7647030d247f7082092e73d31c90944e43b Signed-off-by: Subbaraman Narayanamurthy --- drivers/usb/typec/ucsi/ucsi_glink.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c index aa8aea7f05cb..535cd3b48081 100644 --- a/drivers/usb/typec/ucsi/ucsi_glink.c +++ b/drivers/usb/typec/ucsi/ucsi_glink.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2019-2020, The Linux Foundation. All rights reserved. + * Copyright (c) 2019-2021, The Linux Foundation. All rights reserved. */ #define pr_fmt(fmt) "UCSI: %s: " fmt, __func__ @@ -285,6 +285,9 @@ static int ucsi_qti_glink_write(struct ucsi_dev *udev, unsigned int offset, reinit_completion(&udev->sync_write_ack); } + ucsi_log(sync ? "sync_write:" : "async_write:", offset, + (u8 *)val, val_len); + rc = pmic_glink_write(udev->client, &ucsi_buf, sizeof(ucsi_buf)); if (rc < 0) { @@ -314,9 +317,6 @@ static int ucsi_qti_glink_write(struct ucsi_dev *udev, unsigned int offset, } } - ucsi_log(sync ? "sync_write:" : "async_write:", offset, - (u8 *)val, val_len); - if (((u8 *)val)[0] == UCSI_GET_CONNECTOR_STATUS) { mutex_lock(&udev->notify_lock); set_bit(CONN_STAT_REQD, &udev->cmd_requested_flags); From f18f7394cd4d17026bbb943e26d3e24ef0107f7f Mon Sep 17 00:00:00 2001 From: Subbaraman Narayanamurthy Date: Tue, 6 Apr 2021 18:39:48 -0700 Subject: [PATCH 3/6] usb: typec: ucsi_glink: Fix a NULL pointer dereference During a SSR/PDR scenario, when pmic_glink state is down, ucsi_glink driver unregisters from the UCSI framework thereby setting udev->ucsi to NULL. Typically, we don't expect any notification coming from the charger firmware that runs on a remote subsystem until pmic_glink state is up and ucsi_glink driver registers again with the UCSI framework. In the case, if ucsi_glink driver receives a notification before UCSI driver finishes the registration, then don't handle it. Change-Id: Ieb792ca6026344462c7eae4d40aef87ae061ae96 Signed-off-by: Subbaraman Narayanamurthy --- drivers/usb/typec/ucsi/ucsi_glink.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c index 535cd3b48081..a2db93800c12 100644 --- a/drivers/usb/typec/ucsi/ucsi_glink.c +++ b/drivers/usb/typec/ucsi/ucsi_glink.c @@ -77,6 +77,7 @@ struct ucsi_dev { struct mutex read_lock; struct mutex write_lock; struct mutex notify_lock; + struct mutex state_lock; struct ucsi_read_buf_resp_msg rx_buf; unsigned long flags; atomic_t rx_valid; @@ -201,6 +202,19 @@ static int handle_ucsi_notify(struct ucsi_dev *udev, void *data, size_t len) return -EINVAL; } + mutex_lock(&udev->state_lock); + if (atomic_read(&udev->state) == PMIC_GLINK_STATE_DOWN) { + pr_err("glink state is down\n"); + mutex_unlock(&udev->state_lock); + return -ENOTCONN; + } + + if (!udev->ucsi) { + pr_err("ucsi is NULL\n"); + mutex_unlock(&udev->state_lock); + return -ENODEV; + } + msg_ptr = data; cci = msg_ptr->notification; ucsi_log("notify:", UCSI_CCI, (u8 *)&cci, sizeof(cci)); @@ -221,6 +235,7 @@ static int handle_ucsi_notify(struct ucsi_dev *udev, void *data, size_t len) if (con && con->ucsi) ucsi_connector_change(udev->ucsi, con_num); } + mutex_unlock(&udev->state_lock); return 0; } @@ -485,11 +500,13 @@ static int ucsi_setup(struct ucsi_dev *udev) return -EINVAL; } + mutex_lock(&udev->state_lock); udev->ucsi = ucsi_create(udev->dev, &ucsi_qti_ops); if (IS_ERR(udev->ucsi)) { rc = PTR_ERR(udev->ucsi); dev_err(udev->dev, "ucsi_create failed rc=%d\n", rc); udev->ucsi = NULL; + mutex_unlock(&udev->state_lock); return rc; } @@ -500,9 +517,11 @@ static int ucsi_setup(struct ucsi_dev *udev) dev_err(udev->dev, "ucsi_register failed rc=%d\n", rc); ucsi_destroy(udev->ucsi); udev->ucsi = NULL; + mutex_unlock(&udev->state_lock); return rc; } + mutex_unlock(&udev->state_lock); return 0; } @@ -520,12 +539,14 @@ static void ucsi_qti_state_cb(void *priv, enum pmic_glink_state state) dev_dbg(udev->dev, "state: %d\n", state); + mutex_lock(&udev->state_lock); atomic_set(&udev->state, state); switch (state) { case PMIC_GLINK_STATE_DOWN: if (!udev->ucsi) { dev_err(udev->dev, "ucsi is NULL\n"); + mutex_unlock(&udev->state_lock); return; } @@ -539,6 +560,7 @@ static void ucsi_qti_state_cb(void *priv, enum pmic_glink_state state) default: break; } + mutex_unlock(&udev->state_lock); } static int ucsi_probe(struct platform_device *pdev) @@ -557,6 +579,7 @@ static int ucsi_probe(struct platform_device *pdev) mutex_init(&udev->read_lock); mutex_init(&udev->write_lock); mutex_init(&udev->notify_lock); + mutex_init(&udev->state_lock); init_completion(&udev->read_ack); init_completion(&udev->write_ack); init_completion(&udev->sync_write_ack); From 62a30b8944765389cb4c2df0896e0d5758d36cf4 Mon Sep 17 00:00:00 2001 From: Subbaraman Narayanamurthy Date: Fri, 4 Jun 2021 12:38:28 -0700 Subject: [PATCH 4/6] usb: typec: ucsi_glink: Change dev_err to dev_dbg in ucsi_qti_state_cb() When ucsi_qti_state_cb() is invoked for a SSR event, it is called twice when PDR is enabled. Because of that, error log is printed. This is because pmic_glink driver receives PDR notification followed by SSR notification. It is fine for the clients to receive both the notifications and take action or ignore it. Change dev_err to dev_dbg in ucsi_qti_state_cb() so that it is not taken for an error condition. Change-Id: I2799e3b17920ef88d434a63c06f9cbf6fc63fff9 Signed-off-by: Subbaraman Narayanamurthy --- drivers/usb/typec/ucsi/ucsi_glink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c index a2db93800c12..f4f6a681e5c7 100644 --- a/drivers/usb/typec/ucsi/ucsi_glink.c +++ b/drivers/usb/typec/ucsi/ucsi_glink.c @@ -545,7 +545,7 @@ static void ucsi_qti_state_cb(void *priv, enum pmic_glink_state state) switch (state) { case PMIC_GLINK_STATE_DOWN: if (!udev->ucsi) { - dev_err(udev->dev, "ucsi is NULL\n"); + dev_dbg(udev->dev, "ucsi is NULL\n"); mutex_unlock(&udev->state_lock); return; } From fb407a3f0e0a6034fb6e315e65870ec70008245e Mon Sep 17 00:00:00 2001 From: Linyu Yuan Date: Fri, 18 Jun 2021 14:16:14 +0800 Subject: [PATCH 5/6] usb: typec: ucsi_glink: serialize notification to client If PPM notifies consecutive connector status in short time, in ucsi_qti_notify(), there is a chance that second connector status event will cancel first notification work before it finish, so client will not receive notification and work unexpected. This change will record each connector status info into a private buffer and send status to client one by one in ucsi_qti_notify_work(). Change-Id: I0797ce32bf33319659da3511281044ad947e6bb5 Signed-off-by: Linyu Yuan --- drivers/usb/typec/ucsi/ucsi_glink.c | 65 +++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 13 deletions(-) diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c index f4f6a681e5c7..97c81979f49a 100644 --- a/drivers/usb/typec/ucsi/ucsi_glink.c +++ b/drivers/usb/typec/ucsi/ucsi_glink.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -67,6 +68,11 @@ struct ucsi_notify_ind_msg { u32 reserved; }; +struct constat_info_entry { + struct list_head node; + struct ucsi_glink_constat_info constat_info; +}; + struct ucsi_dev { struct device *dev; struct ucsi *ucsi; @@ -82,7 +88,7 @@ struct ucsi_dev { unsigned long flags; atomic_t rx_valid; unsigned long cmd_requested_flags; - struct ucsi_glink_constat_info constat_info; + struct list_head constat_info_list; struct work_struct notify_work; struct work_struct setup_work; atomic_t state; @@ -363,12 +369,37 @@ static int ucsi_qti_sync_write(struct ucsi *ucsi, unsigned int offset, return ucsi_qti_glink_write(udev, offset, val, val_len, true); } +static void ucsi_qti_clean_notification(struct ucsi_dev *udev) +{ + struct constat_info_entry *entry, *tmp; + + mutex_lock(&udev->notify_lock); + list_for_each_entry_safe(entry, tmp, &udev->constat_info_list, node) { + list_del(&entry->node); + kfree(entry); + } + INIT_LIST_HEAD(&udev->constat_info_list); + mutex_unlock(&udev->notify_lock); +} + static void ucsi_qti_notify_work(struct work_struct *work) { struct ucsi_dev *udev = container_of(work, struct ucsi_dev, notify_work); + struct constat_info_entry *entry; - raw_notifier_call_chain(&ucsi_glink_notifier, 0, &udev->constat_info); + mutex_lock(&udev->notify_lock); + while (!list_empty(&udev->constat_info_list)) { + entry = list_first_entry(&udev->constat_info_list, + struct constat_info_entry, node); + list_del(&entry->node); + mutex_unlock(&udev->notify_lock); + raw_notifier_call_chain(&ucsi_glink_notifier, + 0, &entry->constat_info); + kfree(entry); + mutex_lock(&udev->notify_lock); + } + mutex_unlock(&udev->notify_lock); } static void ucsi_qti_notify(struct ucsi_dev *udev, unsigned int offset, @@ -376,6 +407,7 @@ static void ucsi_qti_notify(struct ucsi_dev *udev, unsigned int offset, { u8 conn_partner_type, conn_partner_flag; bool cmd_requested; + struct constat_info_entry *entry; if (len != sizeof(*status)) return; @@ -385,45 +417,49 @@ static void ucsi_qti_notify(struct ucsi_dev *udev, unsigned int offset, mutex_unlock(&udev->notify_lock); if (cmd_requested && offset == UCSI_MESSAGE_IN) { - cancel_work_sync(&udev->notify_work); + entry = kzalloc(sizeof(*entry), GFP_KERNEL); + if (!entry) + return; - udev->constat_info.partner_usb = false; - udev->constat_info.partner_alternate_mode = false; + INIT_LIST_HEAD(&entry->node); + entry->constat_info.partner_usb = false; + entry->constat_info.partner_alternate_mode = false; - udev->constat_info.partner_change = + entry->constat_info.partner_change = status->change & UCSI_CONSTAT_PARTNER_CHANGE; - udev->constat_info.connect = + entry->constat_info.connect = status->flags & UCSI_CONSTAT_CONNECTED; conn_partner_type = UCSI_CONSTAT_PARTNER_TYPE(status->flags); switch (conn_partner_type) { case UCSI_CONSTAT_PARTNER_TYPE_AUDIO: - udev->constat_info.acc = TYPEC_ACCESSORY_AUDIO; + entry->constat_info.acc = TYPEC_ACCESSORY_AUDIO; break; case UCSI_CONSTAT_PARTNER_TYPE_DEBUG: - udev->constat_info.acc = TYPEC_ACCESSORY_DEBUG; + entry->constat_info.acc = TYPEC_ACCESSORY_DEBUG; break; case UCSI_CONSTAT_PARTNER_TYPE_UFP: case UCSI_CONSTAT_PARTNER_TYPE_CABLE: case UCSI_CONSTAT_PARTNER_TYPE_CABLE_AND_UFP: case UCSI_CONSTAT_PARTNER_TYPE_DFP: - udev->constat_info.partner_usb = true; + entry->constat_info.partner_usb = true; fallthrough; default: - udev->constat_info.acc = TYPEC_ACCESSORY_NONE; + entry->constat_info.acc = TYPEC_ACCESSORY_NONE; break; } conn_partner_flag = UCSI_CONSTAT_PARTNER_FLAGS(status->flags); if (conn_partner_flag & UCSI_CONSTAT_PARTNER_FLAG_USB) - udev->constat_info.partner_usb = true; + entry->constat_info.partner_usb = true; if (conn_partner_flag & UCSI_CONSTAT_PARTNER_FLAG_ALT_MODE) - udev->constat_info.partner_alternate_mode = true; + entry->constat_info.partner_alternate_mode = true; mutex_lock(&udev->notify_lock); + list_add_tail(&entry->node, &udev->constat_info_list); clear_bit(CONN_STAT_REQD, &udev->cmd_requested_flags); mutex_unlock(&udev->notify_lock); @@ -550,6 +586,7 @@ static void ucsi_qti_state_cb(void *priv, enum pmic_glink_state state) return; } + ucsi_qti_clean_notification(udev); ucsi_unregister(udev->ucsi); ucsi_destroy(udev->ucsi); udev->ucsi = NULL; @@ -574,6 +611,7 @@ static int ucsi_probe(struct platform_device *pdev) if (!udev) return -ENOMEM; + INIT_LIST_HEAD(&udev->constat_info_list); INIT_WORK(&udev->notify_work, ucsi_qti_notify_work); INIT_WORK(&udev->setup_work, ucsi_qti_setup_work); mutex_init(&udev->read_lock); @@ -624,6 +662,7 @@ static int ucsi_remove(struct platform_device *pdev) struct ucsi_dev *udev = dev_get_drvdata(dev); int rc; + ucsi_qti_clean_notification(udev); cancel_work_sync(&udev->notify_work); ucsi_unregister(udev->ucsi); ucsi_destroy(udev->ucsi); From 1f384881ea6557a99cb725ff61b1118bd308abb2 Mon Sep 17 00:00:00 2001 From: Subbaraman Narayanamurthy Date: Tue, 29 Jun 2021 09:28:30 -0700 Subject: [PATCH 6/6] usb: typec: ucsi_glink: add a debug log in ucsi_qti_notify_work() Add a debug log to print the partner accessory information from connector status notifications from PPM that are being notified to the clients. This is useful for debugging. Change-Id: I84bcba5088fae33989a462c618ede72b4efbe1b2 Signed-off-by: Subbaraman Narayanamurthy --- drivers/usb/typec/ucsi/ucsi_glink.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c index 97c81979f49a..2cff884324bc 100644 --- a/drivers/usb/typec/ucsi/ucsi_glink.c +++ b/drivers/usb/typec/ucsi/ucsi_glink.c @@ -394,6 +394,12 @@ static void ucsi_qti_notify_work(struct work_struct *work) struct constat_info_entry, node); list_del(&entry->node); mutex_unlock(&udev->notify_lock); + pr_debug("acc: %d usb: %d alt_mode: %d change: %d connect: %d\n", + entry->constat_info.acc, + entry->constat_info.partner_usb, + entry->constat_info.partner_alternate_mode, + entry->constat_info.partner_change, + entry->constat_info.connect); raw_notifier_call_chain(&ucsi_glink_notifier, 0, &entry->constat_info); kfree(entry);