From 804ef58a9e4ac65cb8168db7abde7a96b0da9e27 Mon Sep 17 00:00:00 2001 From: Niklas Neronin Date: Wed, 6 Nov 2024 12:14:56 +0200 Subject: [PATCH 01/16] usb: xhci: remove irrelevant comment The code which it is referencing does not exist in the same function, or the file for that matter. Since it was added [1], the Interrupter Moderation Interval can be changed within xhci addon, e.g. PCI xhci_pci_setup(). [1], commit 0ebbab374223 ("USB: xhci: Ring allocation and initialization.") Signed-off-by: Niklas Neronin Signed-off-by: Mathias Nyman Link: https://lore.kernel.org/r/20241106101459.775897-31-mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/xhci-mem.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 4295e9a4de50..15db90c54a45 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -2512,11 +2512,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) ir->isoc_bei_interval = AVOID_BEI_INTERVAL_MAX; - /* - * XXX: Might need to set the Interrupter Moderation Register to - * something other than the default (~1ms minimum between interrupts). - * See section 5.5.1.2. - */ for (i = 0; i < MAX_HC_SLOTS; i++) xhci->devs[i] = NULL; -- 2.51.0 From 42b7581376015c1bbcbe5831f043cd0ac119d028 Mon Sep 17 00:00:00 2001 From: Michal Pecio Date: Wed, 6 Nov 2024 12:14:57 +0200 Subject: [PATCH 02/16] usb: xhci: Limit Stop Endpoint retries Some host controllers fail to atomically transition an endpoint to the Running state on a doorbell ring and enter a hidden "Restarting" state, which looks very much like Stopped, with the important difference that it will spontaneously transition to Running anytime soon. A Stop Endpoint command queued in the Restarting state typically fails with Context State Error and the completion handler sees the Endpoint Context State as either still Stopped or already Running. Even a case of Halted was observed, when an error occurred right after the restart. The Halted state is already recovered from by resetting the endpoint. The Running state is handled by retrying Stop Endpoint. The Stopped state was recognized as a problem on NEC controllers and worked around also by retrying, because the endpoint soon restarts and then stops for good. But there is a risk: the command may fail if the endpoint is "stopped for good" already, and retries will fail forever. The possibility of this was not realized at the time, but a number of cases were discovered later and reproduced. Some proved difficult to deal with, and it is outright impossible to predict if an endpoint may fail to ever start at all due to a hardware bug. One such bug (albeit on ASM3142, not on NEC) was found to be reliably triggered simply by toggling an AX88179 NIC up/down in a tight loop for a few seconds. An endless retries storm is quite nasty. Besides putting needless load on the xHC and CPU, it causes URBs never to be given back, paralyzing the device and connection/disconnection logic for the whole bus if the device is unplugged. User processes waiting for URBs become unkillable, drivers and kworker threads lock up and xhci_hcd cannot be reloaded. For peace of mind, impose a timeout on Stop Endpoint retries in this case. If they don't succeed in 100ms, consider the endpoint stopped permanently for some reason and just give back the unlinked URBs. This failure case is rare already and work is under way to make it rarer. Start this work today by also handling one simple case of race with Reset Endpoint, because it costs just two lines to implement. Fixes: fd9d55d190c0 ("xhci: retry Stop Endpoint on buggy NEC controllers") CC: stable@vger.kernel.org Signed-off-by: Michal Pecio Signed-off-by: Mathias Nyman Link: https://lore.kernel.org/r/20241106101459.775897-32-mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/xhci-ring.c | 28 ++++++++++++++++++++++++---- drivers/usb/host/xhci.c | 2 ++ drivers/usb/host/xhci.h | 1 + 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index c9c0c4a7588a..dd23596ccd84 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -52,6 +52,7 @@ * endpoint rings; it generates events on the event ring for these. */ +#include #include #include #include @@ -1158,16 +1159,35 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, return; case EP_STATE_STOPPED: /* - * NEC uPD720200 sometimes sets this state and fails with - * Context Error while continuing to process TRBs. - * Be conservative and trust EP_CTX_STATE on other chips. + * Per xHCI 4.6.9, Stop Endpoint command on a Stopped + * EP is a Context State Error, and EP stays Stopped. + * + * But maybe it failed on Halted, and somebody ran Reset + * Endpoint later. EP state is now Stopped and EP_HALTED + * still set because Reset EP handler will run after us. + */ + if (ep->ep_state & EP_HALTED) + break; + /* + * On some HCs EP state remains Stopped for some tens of + * us to a few ms or more after a doorbell ring, and any + * new Stop Endpoint fails without aborting the restart. + * This handler may run quickly enough to still see this + * Stopped state, but it will soon change to Running. + * + * Assume this bug on unexpected Stop Endpoint failures. + * Keep retrying until the EP starts and stops again, on + * chips where this is known to help. Wait for 100ms. */ if (!(xhci->quirks & XHCI_NEC_HOST)) break; + if (time_is_before_jiffies(ep->stop_time + msecs_to_jiffies(100))) + break; fallthrough; case EP_STATE_RUNNING: /* Race, HW handled stop ep cmd before ep was running */ - xhci_dbg(xhci, "Stop ep completion ctx error, ep is running\n"); + xhci_dbg(xhci, "Stop ep completion ctx error, ctx_state %d\n", + GET_EP_CTX_STATE(ep_ctx)); command = xhci_alloc_command(xhci, false, GFP_ATOMIC); if (!command) { diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index bc477cf99805..4977ada0a19e 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -8,6 +8,7 @@ * Some code borrowed from the Linux EHCI driver. */ +#include #include #include #include @@ -1764,6 +1765,7 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) ret = -ENOMEM; goto done; } + ep->stop_time = jiffies; ep->ep_state |= EP_STOP_CMD_PENDING; xhci_queue_stop_endpoint(xhci, command, urb->dev->slot_id, ep_index, 0); diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index a0e992c3db0d..6dd3138b2380 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -691,6 +691,7 @@ struct xhci_virt_ep { /* Bandwidth checking storage */ struct xhci_bw_info bw_info; struct list_head bw_endpoint_list; + unsigned long stop_time; /* Isoch Frame ID checking storage */ int next_frame_id; /* Use new Isoch TRB layout needed for extended TBC support */ -- 2.51.0 From 484c3bab2d5dfa13ff659a51a06e9a393141eefc Mon Sep 17 00:00:00 2001 From: Michal Pecio Date: Wed, 6 Nov 2024 12:14:58 +0200 Subject: [PATCH 03/16] usb: xhci: Fix TD invalidation under pending Set TR Dequeue xhci_invalidate_cancelled_tds() may not work correctly if the hardware is modifying endpoint or stream contexts at the same time by executing a Set TR Dequeue command. And even if it worked, it would be unable to queue Set TR Dequeue for the next stream, failing to clear xHC cache. On stream endpoints, a chain of Set TR Dequeue commands may take some time to execute and we may want to cancel more TDs during this time. Currently this leads to Stop Endpoint completion handler calling this function without testing for SET_DEQ_PENDING, which will trigger the aforementioned problems when it happens. On all endpoints, a halt condition causes Reset Endpoint to be queued and an error status given to the class driver, which may unlink more URBs in response. Stop Endpoint is queued and its handler may execute concurrently with Set TR Dequeue queued by Reset Endpoint handler. (Reset Endpoint handler calls this function too, but there seems to be no possibility of it running concurrently with Set TR Dequeue). Fix xhci_invalidate_cancelled_tds() to work correctly under a pending Set TR Dequeue. Bail out of the function when SET_DEQ_PENDING is set, then make the completion handler call the function again and also call xhci_giveback_invalidated_tds(), which needs to be called next. This seems to fix another potential bug, where the handler would call xhci_invalidate_cancelled_tds(), which may clear some deferred TDs if a sanity check fails, and the TDs wouldn't be given back promptly. Said sanity check seems to be wrong and prone to false positives when the endpoint halts, but fixing it is beyond the scope of this change, besides ensuring that cleared TDs are given back properly. Fixes: 5ceac4402f5d ("xhci: Handle TD clearing for multiple streams case") CC: stable@vger.kernel.org Signed-off-by: Michal Pecio Signed-off-by: Mathias Nyman Link: https://lore.kernel.org/r/20241106101459.775897-33-mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/xhci-ring.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index dd23596ccd84..55be03be2374 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -980,6 +980,13 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) unsigned int slot_id = ep->vdev->slot_id; int err; + /* + * This is not going to work if the hardware is changing its dequeue + * pointers as we look at them. Completion handler will call us later. + */ + if (ep->ep_state & SET_DEQ_PENDING) + return 0; + xhci = ep->xhci; list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, cancelled_td_list) { @@ -1367,7 +1374,6 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, struct xhci_slot_ctx *slot_ctx; struct xhci_stream_ctx *stream_ctx; struct xhci_td *td, *tmp_td; - bool deferred = false; ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3])); stream_id = TRB_TO_STREAM_ID(le32_to_cpu(trb->generic.field[2])); @@ -1471,8 +1477,6 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, xhci_dbg(ep->xhci, "%s: Giveback cancelled URB %p TD\n", __func__, td->urb); xhci_td_cleanup(ep->xhci, td, ep_ring, td->status); - } else if (td->cancel_status == TD_CLEARING_CACHE_DEFERRED) { - deferred = true; } else { xhci_dbg(ep->xhci, "%s: Keep cancelled URB %p TD as cancel_status is %d\n", __func__, td->urb, td->cancel_status); @@ -1483,11 +1487,15 @@ cleanup: ep->queued_deq_seg = NULL; ep->queued_deq_ptr = NULL; - if (deferred) { - /* We have more streams to clear */ + /* Check for deferred or newly cancelled TDs */ + if (!list_empty(&ep->cancelled_td_list)) { xhci_dbg(ep->xhci, "%s: Pending TDs to clear, continuing with invalidation\n", __func__); xhci_invalidate_cancelled_tds(ep); + /* Try to restart the endpoint if all is done */ + ring_doorbell_for_active_rings(xhci, slot_id, ep_index); + /* Start giving back any TDs invalidated above */ + xhci_giveback_invalidated_tds(ep); } else { /* Restart any rings with pending URBs */ xhci_dbg(ep->xhci, "%s: All TDs cleared, ring doorbell\n", __func__); -- 2.51.0 From 474538b8dd1cd9c666e56cfe8ef60fbb0fb513f4 Mon Sep 17 00:00:00 2001 From: Michal Pecio Date: Wed, 6 Nov 2024 12:14:59 +0200 Subject: [PATCH 04/16] usb: xhci: Avoid queuing redundant Stop Endpoint commands Stop Endpoint command on an already stopped endpoint fails and may be misinterpreted as a known hardware bug by the completion handler. This results in an unnecessary delay with repeated retries of the command. Avoid queuing this command when endpoint state flags indicate that it's stopped or halted and the command will fail. If commands are pending on the endpoint, their completion handlers will process cancelled TDs so it's done. In case of waiting for external operations like clearing TT buffer, the endpoint is stopped and cancelled TDs can be processed now. This eliminates practically all unnecessary retries because an endpoint with pending URBs is maintained in Running state by the driver, unless aforementioned commands or other operations are pending on it. This is guaranteed by xhci_ring_ep_doorbell() and by the fact that it is called every time any of those operations completes. The only known exceptions are hardware bugs (the endpoint never starts at all) and Stream Protocol errors not associated with any TRB, which cause an endpoint reset not followed by restart. Sounds like a bug. Generally, these retries are only expected to happen when the endpoint fails to start for unknown/no reason, which is a worse problem itself, and fixing the bug eliminates the retries too. All cases were tested and found to work as expected. SET_DEQ_PENDING was produced by patching uvcvideo to unlink URBs in 100us intervals, which then runs into this case very often. EP_HALTED was produced by restarting 'cat /dev/ttyUSB0' on a serial dongle with broken cable. EP_CLEARING_TT by the same, with the dongle on an external hub. Fixes: fd9d55d190c0 ("xhci: retry Stop Endpoint on buggy NEC controllers") CC: stable@vger.kernel.org Signed-off-by: Michal Pecio Signed-off-by: Mathias Nyman Link: https://lore.kernel.org/r/20241106101459.775897-34-mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/xhci-ring.c | 13 +++++++++++++ drivers/usb/host/xhci.c | 19 +++++++++++++++---- drivers/usb/host/xhci.h | 1 + 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 55be03be2374..4cf5363875c7 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1076,6 +1076,19 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) return 0; } +/* + * Erase queued TDs from transfer ring(s) and give back those the xHC didn't + * stop on. If necessary, queue commands to move the xHC off cancelled TDs it + * stopped on. Those will be given back later when the commands complete. + * + * Call under xhci->lock on a stopped endpoint. + */ +void xhci_process_cancelled_tds(struct xhci_virt_ep *ep) +{ + xhci_invalidate_cancelled_tds(ep); + xhci_giveback_invalidated_tds(ep); +} + /* * Returns the TD the endpoint ring halted on. * Only call for non-running rings without streams. diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 4977ada0a19e..5ebde8cae4fc 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1756,10 +1756,21 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) } } - /* Queue a stop endpoint command, but only if this is - * the first cancellation to be handled. - */ - if (!(ep->ep_state & EP_STOP_CMD_PENDING)) { + /* These completion handlers will sort out cancelled TDs for us */ + if (ep->ep_state & (EP_STOP_CMD_PENDING | EP_HALTED | SET_DEQ_PENDING)) { + xhci_dbg(xhci, "Not queuing Stop Endpoint on slot %d ep %d in state 0x%x\n", + urb->dev->slot_id, ep_index, ep->ep_state); + goto done; + } + + /* In this case no commands are pending but the endpoint is stopped */ + if (ep->ep_state & EP_CLEARING_TT) { + /* and cancelled TDs can be given back right away */ + xhci_dbg(xhci, "Invalidating TDs instantly on slot %d ep %d in state 0x%x\n", + urb->dev->slot_id, ep_index, ep->ep_state); + xhci_process_cancelled_tds(ep); + } else { + /* Otherwise, queue a new Stop Endpoint command */ command = xhci_alloc_command(xhci, false, GFP_ATOMIC); if (!command) { ret = -ENOMEM; diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 6dd3138b2380..4914f0a10cff 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1922,6 +1922,7 @@ void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring); unsigned int count_trbs(u64 addr, u64 len); int xhci_stop_endpoint_sync(struct xhci_hcd *xhci, struct xhci_virt_ep *ep, int suspend, gfp_t gfp_flags); +void xhci_process_cancelled_tds(struct xhci_virt_ep *ep); /* xHCI roothub code */ void xhci_set_link_state(struct xhci_hcd *xhci, struct xhci_port *port, -- 2.51.0 From 226ff2e681d006eada59a9693aa1976d4c15a7d4 Mon Sep 17 00:00:00 2001 From: Heikki Krogerus Date: Wed, 6 Nov 2024 17:06:05 +0200 Subject: [PATCH 05/16] usb: typec: ucsi: Convert connector specific commands to bitmaps That allows the fields in those command data structures to be easily validated. If an unsupported field is accessed, a warning is generated. This will not force UCSI version checks to be made in every place where these data structures are accessed, but it will make it easier to pinpoint issues that are caused by the unconditional accesses to those fields, and perhaps more importantly, allow those issues to be noticed immediately. Signed-off-by: Heikki Krogerus Link: https://lore.kernel.org/r/20241106150605.1017744-1-heikki.krogerus@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/typec/ucsi/psy.c | 28 ++-- drivers/usb/typec/ucsi/trace.h | 28 ++-- drivers/usb/typec/ucsi/ucsi.c | 121 +++++++------- drivers/usb/typec/ucsi/ucsi.h | 252 +++++++++++++++++------------ drivers/usb/typec/ucsi/ucsi_acpi.c | 7 +- 5 files changed, 240 insertions(+), 196 deletions(-) diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c index 1c631c7855a9..62ac69730405 100644 --- a/drivers/usb/typec/ucsi/psy.c +++ b/drivers/usb/typec/ucsi/psy.c @@ -55,8 +55,8 @@ static int ucsi_psy_get_online(struct ucsi_connector *con, union power_supply_propval *val) { val->intval = UCSI_PSY_OFFLINE; - if (con->status.flags & UCSI_CONSTAT_CONNECTED && - (con->status.flags & UCSI_CONSTAT_PWR_DIR) == TYPEC_SINK) + if (UCSI_CONSTAT(con, CONNECTED) && + (UCSI_CONSTAT(con, PWR_DIR) == TYPEC_SINK)) val->intval = UCSI_PSY_FIXED_ONLINE; return 0; } @@ -66,7 +66,7 @@ static int ucsi_psy_get_voltage_min(struct ucsi_connector *con, { u32 pdo; - switch (UCSI_CONSTAT_PWR_OPMODE(con->status.flags)) { + switch (UCSI_CONSTAT(con, PWR_OPMODE)) { case UCSI_CONSTAT_PWR_OPMODE_PD: pdo = con->src_pdos[0]; val->intval = pdo_fixed_voltage(pdo) * 1000; @@ -89,7 +89,7 @@ static int ucsi_psy_get_voltage_max(struct ucsi_connector *con, { u32 pdo; - switch (UCSI_CONSTAT_PWR_OPMODE(con->status.flags)) { + switch (UCSI_CONSTAT(con, PWR_OPMODE)) { case UCSI_CONSTAT_PWR_OPMODE_PD: if (con->num_pdos > 0) { pdo = con->src_pdos[con->num_pdos - 1]; @@ -117,7 +117,7 @@ static int ucsi_psy_get_voltage_now(struct ucsi_connector *con, int index; u32 pdo; - switch (UCSI_CONSTAT_PWR_OPMODE(con->status.flags)) { + switch (UCSI_CONSTAT(con, PWR_OPMODE)) { case UCSI_CONSTAT_PWR_OPMODE_PD: index = rdo_index(con->rdo); if (index > 0) { @@ -145,7 +145,7 @@ static int ucsi_psy_get_current_max(struct ucsi_connector *con, { u32 pdo; - switch (UCSI_CONSTAT_PWR_OPMODE(con->status.flags)) { + switch (UCSI_CONSTAT(con, PWR_OPMODE)) { case UCSI_CONSTAT_PWR_OPMODE_PD: if (con->num_pdos > 0) { pdo = con->src_pdos[con->num_pdos - 1]; @@ -173,9 +173,7 @@ static int ucsi_psy_get_current_max(struct ucsi_connector *con, static int ucsi_psy_get_current_now(struct ucsi_connector *con, union power_supply_propval *val) { - u16 flags = con->status.flags; - - if (UCSI_CONSTAT_PWR_OPMODE(flags) == UCSI_CONSTAT_PWR_OPMODE_PD) + if (UCSI_CONSTAT(con, PWR_OPMODE) == UCSI_CONSTAT_PWR_OPMODE_PD) val->intval = rdo_op_current(con->rdo) * 1000; else val->intval = 0; @@ -185,11 +183,9 @@ static int ucsi_psy_get_current_now(struct ucsi_connector *con, static int ucsi_psy_get_usb_type(struct ucsi_connector *con, union power_supply_propval *val) { - u16 flags = con->status.flags; - val->intval = POWER_SUPPLY_USB_TYPE_C; - if (flags & UCSI_CONSTAT_CONNECTED && - UCSI_CONSTAT_PWR_OPMODE(flags) == UCSI_CONSTAT_PWR_OPMODE_PD) + if (UCSI_CONSTAT(con, CONNECTED) && + UCSI_CONSTAT(con, PWR_OPMODE) == UCSI_CONSTAT_PWR_OPMODE_PD) val->intval = POWER_SUPPLY_USB_TYPE_PD; return 0; @@ -197,18 +193,18 @@ static int ucsi_psy_get_usb_type(struct ucsi_connector *con, static int ucsi_psy_get_charge_type(struct ucsi_connector *con, union power_supply_propval *val) { - if (!(con->status.flags & UCSI_CONSTAT_CONNECTED)) { + if (!(UCSI_CONSTAT(con, CONNECTED))) { val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE; return 0; } /* The Battery Charging Cabability Status field is only valid in sink role. */ - if ((con->status.flags & UCSI_CONSTAT_PWR_DIR) != TYPEC_SINK) { + if (UCSI_CONSTAT(con, PWR_DIR) != TYPEC_SINK) { val->intval = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN; return 0; } - switch (UCSI_CONSTAT_BC_STATUS(con->status.pwr_status)) { + switch (UCSI_CONSTAT(con, BC_STATUS)) { case UCSI_CONSTAT_BC_NOMINAL_CHARGING: val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD; break; diff --git a/drivers/usb/typec/ucsi/trace.h b/drivers/usb/typec/ucsi/trace.h index a0d3a934d3d9..41701dee7056 100644 --- a/drivers/usb/typec/ucsi/trace.h +++ b/drivers/usb/typec/ucsi/trace.h @@ -40,8 +40,8 @@ DEFINE_EVENT(ucsi_log_command, ucsi_reset_ppm, ); DECLARE_EVENT_CLASS(ucsi_log_connector_status, - TP_PROTO(int port, struct ucsi_connector_status *status), - TP_ARGS(port, status), + TP_PROTO(int port, struct ucsi_connector *con), + TP_ARGS(port, con), TP_STRUCT__entry( __field(int, port) __field(u16, change) @@ -55,14 +55,14 @@ DECLARE_EVENT_CLASS(ucsi_log_connector_status, ), TP_fast_assign( __entry->port = port - 1; - __entry->change = status->change; - __entry->opmode = UCSI_CONSTAT_PWR_OPMODE(status->flags); - __entry->connected = !!(status->flags & UCSI_CONSTAT_CONNECTED); - __entry->pwr_dir = !!(status->flags & UCSI_CONSTAT_PWR_DIR); - __entry->partner_flags = UCSI_CONSTAT_PARTNER_FLAGS(status->flags); - __entry->partner_type = UCSI_CONSTAT_PARTNER_TYPE(status->flags); - __entry->request_data_obj = status->request_data_obj; - __entry->bc_status = UCSI_CONSTAT_BC_STATUS(status->pwr_status); + __entry->change = UCSI_CONSTAT(con, CHANGE); + __entry->opmode = UCSI_CONSTAT(con, PWR_OPMODE); + __entry->connected = UCSI_CONSTAT(con, CONNECTED); + __entry->pwr_dir = UCSI_CONSTAT(con, PWR_DIR); + __entry->partner_flags = UCSI_CONSTAT(con, PARTNER_FLAGS); + __entry->partner_type = UCSI_CONSTAT(con, PARTNER_TYPE); + __entry->request_data_obj = UCSI_CONSTAT(con, RDO); + __entry->bc_status = UCSI_CONSTAT(con, BC_STATUS); ), TP_printk("port%d status: change=%04x, opmode=%x, connected=%d, " "sourcing=%d, partner_flags=%x, partner_type=%x, " @@ -73,13 +73,13 @@ DECLARE_EVENT_CLASS(ucsi_log_connector_status, ); DEFINE_EVENT(ucsi_log_connector_status, ucsi_connector_change, - TP_PROTO(int port, struct ucsi_connector_status *status), - TP_ARGS(port, status) + TP_PROTO(int port, struct ucsi_connector *con), + TP_ARGS(port, con) ); DEFINE_EVENT(ucsi_log_connector_status, ucsi_register_port, - TP_PROTO(int port, struct ucsi_connector_status *status), - TP_ARGS(port, status) + TP_PROTO(int port, struct ucsi_connector *con), + TP_ARGS(port, con) ); DECLARE_EVENT_CLASS(ucsi_log_register_altmode, diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index e430a0ca4a2b..974a441155e1 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -651,10 +651,11 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient) static int ucsi_get_connector_status(struct ucsi_connector *con, bool conn_ack) { u64 command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num); - struct ucsi *ucsi = con->ucsi; + size_t size = min(UCSI_GET_CONNECTOR_STATUS_SIZE, UCSI_MAX_DATA_LENGTH(con->ucsi)); int ret; - ret = ucsi_send_command_common(ucsi, command, &con->status, sizeof(con->status), conn_ack); + ret = ucsi_send_command_common(con->ucsi, command, &con->status, size, conn_ack); + return ret < 0 ? ret : 0; } @@ -668,8 +669,7 @@ static int ucsi_read_pdos(struct ucsi_connector *con, if (is_partner && ucsi->quirks & UCSI_NO_PARTNER_PDOS && - ((con->status.flags & UCSI_CONSTAT_PWR_DIR) || - !is_source(role))) + (UCSI_CONSTAT(con, PWR_DIR) || !is_source(role))) return 0; command = UCSI_COMMAND(UCSI_GET_PDOS) | UCSI_CONNECTOR_NUMBER(con->num); @@ -983,6 +983,7 @@ static void ucsi_unregister_cable(struct ucsi_connector *con) static int ucsi_check_connector_capability(struct ucsi_connector *con) { + u64 pd_revision; u64 command; int ret; @@ -996,17 +997,17 @@ static int ucsi_check_connector_capability(struct ucsi_connector *con) return ret; } - typec_partner_set_pd_revision(con->partner, - UCSI_CONCAP_FLAG_PARTNER_PD_MAJOR_REV_AS_BCD(con->cap.flags)); + pd_revision = UCSI_CONCAP(con, PARTNER_PD_REVISION_V2_1); + typec_partner_set_pd_revision(con->partner, UCSI_SPEC_REVISION_TO_BCD(pd_revision)); return ret; } static void ucsi_pwr_opmode_change(struct ucsi_connector *con) { - switch (UCSI_CONSTAT_PWR_OPMODE(con->status.flags)) { + switch (UCSI_CONSTAT(con, PWR_OPMODE)) { case UCSI_CONSTAT_PWR_OPMODE_PD: - con->rdo = con->status.request_data_obj; + con->rdo = UCSI_CONSTAT(con, RDO); typec_set_pwr_opmode(con->port, TYPEC_PWR_MODE_PD); ucsi_partner_task(con, ucsi_get_src_pdos, 30, 0); ucsi_partner_task(con, ucsi_check_altmodes, 30, HZ); @@ -1030,7 +1031,7 @@ static void ucsi_pwr_opmode_change(struct ucsi_connector *con) static int ucsi_register_partner(struct ucsi_connector *con) { - u8 pwr_opmode = UCSI_CONSTAT_PWR_OPMODE(con->status.flags); + u8 pwr_opmode = UCSI_CONSTAT(con, PWR_OPMODE); struct typec_partner_desc desc; struct typec_partner *partner; @@ -1039,7 +1040,7 @@ static int ucsi_register_partner(struct ucsi_connector *con) memset(&desc, 0, sizeof(desc)); - switch (UCSI_CONSTAT_PARTNER_TYPE(con->status.flags)) { + switch (UCSI_CONSTAT(con, PARTNER_TYPE)) { case UCSI_CONSTAT_PARTNER_TYPE_DEBUG: desc.accessory = TYPEC_ACCESSORY_DEBUG; break; @@ -1057,6 +1058,11 @@ static int ucsi_register_partner(struct ucsi_connector *con) desc.identity = &con->partner_identity; desc.usb_pd = pwr_opmode == UCSI_CONSTAT_PWR_OPMODE_PD; + if (con->ucsi->version >= UCSI_VERSION_2_1) { + u64 pd_revision = UCSI_CONCAP(con, PARTNER_PD_REVISION_V2_1); + desc.pd_revision = UCSI_SPEC_REVISION_TO_BCD(pd_revision); + } + partner = typec_register_partner(con->port, &desc); if (IS_ERR(partner)) { dev_err(con->ucsi->dev, @@ -1067,13 +1073,11 @@ static int ucsi_register_partner(struct ucsi_connector *con) con->partner = partner; - if ((con->ucsi->version >= UCSI_VERSION_3_0) && - (UCSI_CONSTAT_PARTNER_FLAGS(con->status.flags) & - UCSI_CONSTAT_PARTNER_FLAG_USB4_GEN4)) + if (con->ucsi->version >= UCSI_VERSION_3_0 && + UCSI_CONSTAT(con, PARTNER_FLAG_USB4_GEN4)) typec_partner_set_usb_mode(partner, USB_MODE_USB4); - else if ((con->ucsi->version >= UCSI_VERSION_2_0) && - (UCSI_CONSTAT_PARTNER_FLAGS(con->status.flags) & - UCSI_CONSTAT_PARTNER_FLAG_USB4_GEN3)) + else if (con->ucsi->version >= UCSI_VERSION_2_0 && + UCSI_CONSTAT(con, PARTNER_FLAG_USB4_GEN3)) typec_partner_set_usb_mode(partner, USB_MODE_USB4); return 0; @@ -1100,7 +1104,7 @@ static void ucsi_partner_change(struct ucsi_connector *con) enum usb_role u_role = USB_ROLE_NONE; int ret; - switch (UCSI_CONSTAT_PARTNER_TYPE(con->status.flags)) { + switch (UCSI_CONSTAT(con, PARTNER_TYPE)) { case UCSI_CONSTAT_PARTNER_TYPE_UFP: case UCSI_CONSTAT_PARTNER_TYPE_CABLE_AND_UFP: u_role = USB_ROLE_HOST; @@ -1116,8 +1120,8 @@ static void ucsi_partner_change(struct ucsi_connector *con) break; } - if (con->status.flags & UCSI_CONSTAT_CONNECTED) { - switch (UCSI_CONSTAT_PARTNER_TYPE(con->status.flags)) { + if (UCSI_CONSTAT(con, CONNECTED)) { + switch (UCSI_CONSTAT(con, PARTNER_TYPE)) { case UCSI_CONSTAT_PARTNER_TYPE_DEBUG: typec_set_mode(con->port, TYPEC_MODE_DEBUG); break; @@ -1125,14 +1129,13 @@ static void ucsi_partner_change(struct ucsi_connector *con) typec_set_mode(con->port, TYPEC_MODE_AUDIO); break; default: - if (UCSI_CONSTAT_PARTNER_FLAGS(con->status.flags) == - UCSI_CONSTAT_PARTNER_FLAG_USB) + if (UCSI_CONSTAT(con, PARTNER_FLAG_USB)) typec_set_mode(con->port, TYPEC_STATE_USB); } } /* Only notify USB controller if partner supports USB data */ - if (!(UCSI_CONSTAT_PARTNER_FLAGS(con->status.flags) & UCSI_CONSTAT_PARTNER_FLAG_USB)) + if (!(UCSI_CONSTAT(con, PARTNER_FLAG_USB))) u_role = USB_ROLE_NONE; ret = usb_role_switch_set_role(con->usb_role_sw, u_role); @@ -1143,7 +1146,7 @@ static void ucsi_partner_change(struct ucsi_connector *con) static int ucsi_check_connection(struct ucsi_connector *con) { - u8 prev_flags = con->status.flags; + u8 prev_state = UCSI_CONSTAT(con, CONNECTED); int ret; ret = ucsi_get_connector_status(con, false); @@ -1152,10 +1155,9 @@ static int ucsi_check_connection(struct ucsi_connector *con) return ret; } - if (con->status.flags == prev_flags) - return 0; - - if (con->status.flags & UCSI_CONSTAT_CONNECTED) { + if (UCSI_CONSTAT(con, CONNECTED)) { + if (prev_state) + return 0; ucsi_register_partner(con); ucsi_pwr_opmode_change(con); ucsi_partner_change(con); @@ -1211,6 +1213,7 @@ static void ucsi_handle_connector_change(struct work_struct *work) work); struct ucsi *ucsi = con->ucsi; enum typec_role role; + u16 change; int ret; mutex_lock(&con->lock); @@ -1227,14 +1230,15 @@ static void ucsi_handle_connector_change(struct work_struct *work) goto out_unlock; } - trace_ucsi_connector_change(con->num, &con->status); + trace_ucsi_connector_change(con->num, con); if (ucsi->ops->connector_status) ucsi->ops->connector_status(con); - role = !!(con->status.flags & UCSI_CONSTAT_PWR_DIR); + change = UCSI_CONSTAT(con, CHANGE); + role = UCSI_CONSTAT(con, PWR_DIR); - if (con->status.change & UCSI_CONSTAT_POWER_DIR_CHANGE) { + if (change & UCSI_CONSTAT_POWER_DIR_CHANGE) { typec_set_pwr_role(con->port, role); /* Complete pending power role swap */ @@ -1242,12 +1246,12 @@ static void ucsi_handle_connector_change(struct work_struct *work) complete(&con->complete); } - if (con->status.change & UCSI_CONSTAT_CONNECT_CHANGE) { + if (change & UCSI_CONSTAT_CONNECT_CHANGE) { typec_set_pwr_role(con->port, role); ucsi_port_psy_changed(con); ucsi_partner_change(con); - if (con->status.flags & UCSI_CONSTAT_CONNECTED) { + if (UCSI_CONSTAT(con, CONNECTED)) { ucsi_register_partner(con); ucsi_partner_task(con, ucsi_check_connection, 1, HZ); if (con->ucsi->cap.features & UCSI_CAP_GET_PD_MESSAGE) @@ -1255,8 +1259,7 @@ static void ucsi_handle_connector_change(struct work_struct *work) if (con->ucsi->cap.features & UCSI_CAP_CABLE_DETAILS) ucsi_partner_task(con, ucsi_check_cable, 1, HZ); - if (UCSI_CONSTAT_PWR_OPMODE(con->status.flags) == - UCSI_CONSTAT_PWR_OPMODE_PD) { + if (UCSI_CONSTAT(con, PWR_OPMODE) == UCSI_CONSTAT_PWR_OPMODE_PD) { ucsi_partner_task(con, ucsi_register_partner_pdos, 1, HZ); ucsi_partner_task(con, ucsi_check_connector_capability, 1, HZ); } @@ -1265,11 +1268,10 @@ static void ucsi_handle_connector_change(struct work_struct *work) } } - if (con->status.change & UCSI_CONSTAT_POWER_OPMODE_CHANGE || - con->status.change & UCSI_CONSTAT_POWER_LEVEL_CHANGE) + if (change & (UCSI_CONSTAT_POWER_OPMODE_CHANGE | UCSI_CONSTAT_POWER_LEVEL_CHANGE)) ucsi_pwr_opmode_change(con); - if (con->partner && con->status.change & UCSI_CONSTAT_PARTNER_CHANGE) { + if (con->partner && (change & UCSI_CONSTAT_PARTNER_CHANGE)) { ucsi_partner_change(con); /* Complete pending data role swap */ @@ -1277,10 +1279,10 @@ static void ucsi_handle_connector_change(struct work_struct *work) complete(&con->complete); } - if (con->status.change & UCSI_CONSTAT_CAM_CHANGE) + if (change & UCSI_CONSTAT_CAM_CHANGE) ucsi_partner_task(con, ucsi_check_altmodes, 1, HZ); - if (con->status.change & UCSI_CONSTAT_BC_CHANGE) + if (change & UCSI_CONSTAT_BC_CHANGE) ucsi_port_psy_changed(con); out_unlock: @@ -1440,7 +1442,7 @@ static int ucsi_dr_swap(struct typec_port *port, enum typec_data_role role) goto out_unlock; } - partner_type = UCSI_CONSTAT_PARTNER_TYPE(con->status.flags); + partner_type = UCSI_CONSTAT(con, PARTNER_TYPE); if ((partner_type == UCSI_CONSTAT_PARTNER_TYPE_DFP && role == TYPEC_DEVICE) || (partner_type == UCSI_CONSTAT_PARTNER_TYPE_UFP && @@ -1484,7 +1486,7 @@ static int ucsi_pr_swap(struct typec_port *port, enum typec_role role) goto out_unlock; } - cur_role = !!(con->status.flags & UCSI_CONSTAT_PWR_DIR); + cur_role = UCSI_CONSTAT(con, PWR_DIR); if (cur_role == role) goto out_unlock; @@ -1507,8 +1509,7 @@ static int ucsi_pr_swap(struct typec_port *port, enum typec_role role) mutex_lock(&con->lock); /* Something has gone wrong while swapping the role */ - if (UCSI_CONSTAT_PWR_OPMODE(con->status.flags) != - UCSI_CONSTAT_PWR_OPMODE_PD) { + if (UCSI_CONSTAT(con, PWR_OPMODE) != UCSI_CONSTAT_PWR_OPMODE_PD) { ucsi_reset_connector(con, true); ret = -EPROTO; } @@ -1576,19 +1577,18 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con) if (ret < 0) goto out_unlock; - if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DRP) + if (UCSI_CONCAP(con, OPMODE_DRP)) cap->data = TYPEC_PORT_DRD; - else if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DFP) + else if (UCSI_CONCAP(con, OPMODE_DFP)) cap->data = TYPEC_PORT_DFP; - else if (con->cap.op_mode & UCSI_CONCAP_OPMODE_UFP) + else if (UCSI_CONCAP(con, OPMODE_UFP)) cap->data = TYPEC_PORT_UFP; - if ((con->cap.flags & UCSI_CONCAP_FLAG_PROVIDER) && - (con->cap.flags & UCSI_CONCAP_FLAG_CONSUMER)) + if (UCSI_CONCAP(con, PROVIDER) && UCSI_CONCAP(con, CONSUMER)) cap->type = TYPEC_PORT_DRP; - else if (con->cap.flags & UCSI_CONCAP_FLAG_PROVIDER) + else if (UCSI_CONCAP(con, PROVIDER)) cap->type = TYPEC_PORT_SRC; - else if (con->cap.flags & UCSI_CONCAP_FLAG_CONSUMER) + else if (UCSI_CONCAP(con, CONSUMER)) cap->type = TYPEC_PORT_SNK; cap->revision = ucsi->cap.typec_version; @@ -1596,9 +1596,9 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con) cap->svdm_version = SVDM_VER_2_0; cap->prefer_role = TYPEC_NO_PREFERRED_ROLE; - if (con->cap.op_mode & UCSI_CONCAP_OPMODE_AUDIO_ACCESSORY) + if (UCSI_CONCAP(con, OPMODE_AUDIO_ACCESSORY)) *accessory++ = TYPEC_ACCESSORY_AUDIO; - if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DEBUG_ACCESSORY) + if (UCSI_CONCAP(con, OPMODE_DEBUG_ACCESSORY)) *accessory = TYPEC_ACCESSORY_DEBUG; if (UCSI_CONCAP_USB2_SUPPORT(con)) @@ -1646,7 +1646,7 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con) if (ucsi->ops->connector_status) ucsi->ops->connector_status(con); - switch (UCSI_CONSTAT_PARTNER_TYPE(con->status.flags)) { + switch (UCSI_CONSTAT(con, PARTNER_TYPE)) { case UCSI_CONSTAT_PARTNER_TYPE_UFP: case UCSI_CONSTAT_PARTNER_TYPE_CABLE_AND_UFP: u_role = USB_ROLE_HOST; @@ -1663,9 +1663,8 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con) } /* Check if there is already something connected */ - if (con->status.flags & UCSI_CONSTAT_CONNECTED) { - typec_set_pwr_role(con->port, - !!(con->status.flags & UCSI_CONSTAT_PWR_DIR)); + if (UCSI_CONSTAT(con, CONNECTED)) { + typec_set_pwr_role(con->port, UCSI_CONSTAT(con, PWR_DIR)); ucsi_register_partner(con); ucsi_pwr_opmode_change(con); ucsi_port_psy_changed(con); @@ -1676,7 +1675,7 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con) } /* Only notify USB controller if partner supports USB data */ - if (!(UCSI_CONSTAT_PARTNER_FLAGS(con->status.flags) & UCSI_CONSTAT_PARTNER_FLAG_USB)) + if (!(UCSI_CONSTAT(con, PARTNER_FLAG_USB))) u_role = USB_ROLE_NONE; ret = usb_role_switch_set_role(con->usb_role_sw, u_role); @@ -1686,16 +1685,14 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con) ret = 0; } - if (con->partner && - UCSI_CONSTAT_PWR_OPMODE(con->status.flags) == - UCSI_CONSTAT_PWR_OPMODE_PD) { + if (con->partner && UCSI_CONSTAT(con, PWR_OPMODE) == UCSI_CONSTAT_PWR_OPMODE_PD) { ucsi_register_device_pdos(con); ucsi_get_src_pdos(con); ucsi_check_altmodes(con); ucsi_check_connector_capability(con); } - trace_ucsi_register_port(con->num, &con->status); + trace_ucsi_register_port(con->num, con); out: fwnode_handle_put(cap->fwnode); @@ -1778,7 +1775,7 @@ static int ucsi_init(struct ucsi *ucsi) /* Get PPM capabilities */ command = UCSI_GET_CAPABILITY; - ret = ucsi_send_command(ucsi, command, &ucsi->cap, sizeof(ucsi->cap)); + ret = ucsi_send_command(ucsi, command, &ucsi->cap, UCSI_GET_CAPABILITY_SIZE); if (ret < 0) goto err_reset; diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h index b82dc4c16a0d..5ff369c24a2f 100644 --- a/drivers/usb/typec/ucsi/ucsi.h +++ b/drivers/usb/typec/ucsi/ucsi.h @@ -4,6 +4,7 @@ #define __DRIVER_USB_TYPEC_UCSI_H #include +#include #include #include #include @@ -95,27 +96,31 @@ void ucsi_connector_change(struct ucsi *ucsi, u8 num); /* -------------------------------------------------------------------------- */ /* Commands */ -#define UCSI_PPM_RESET 0x01 -#define UCSI_CANCEL 0x02 -#define UCSI_CONNECTOR_RESET 0x03 -#define UCSI_ACK_CC_CI 0x04 -#define UCSI_SET_NOTIFICATION_ENABLE 0x05 -#define UCSI_GET_CAPABILITY 0x06 -#define UCSI_GET_CONNECTOR_CAPABILITY 0x07 -#define UCSI_SET_UOM 0x08 -#define UCSI_SET_UOR 0x09 -#define UCSI_SET_PDM 0x0a -#define UCSI_SET_PDR 0x0b -#define UCSI_GET_ALTERNATE_MODES 0x0c -#define UCSI_GET_CAM_SUPPORTED 0x0d -#define UCSI_GET_CURRENT_CAM 0x0e -#define UCSI_SET_NEW_CAM 0x0f -#define UCSI_GET_PDOS 0x10 -#define UCSI_GET_CABLE_PROPERTY 0x11 -#define UCSI_GET_CONNECTOR_STATUS 0x12 -#define UCSI_GET_ERROR_STATUS 0x13 -#define UCSI_GET_PD_MESSAGE 0x15 -#define UCSI_SET_SINK_PATH 0x1c +#define UCSI_PPM_RESET 0x01 +#define UCSI_CANCEL 0x02 +#define UCSI_CONNECTOR_RESET 0x03 +#define UCSI_ACK_CC_CI 0x04 +#define UCSI_SET_NOTIFICATION_ENABLE 0x05 +#define UCSI_GET_CAPABILITY 0x06 +#define UCSI_GET_CAPABILITY_SIZE 128 +#define UCSI_GET_CONNECTOR_CAPABILITY 0x07 +#define UCSI_GET_CONNECTOR_CAPABILITY_SIZE 32 +#define UCSI_SET_UOM 0x08 +#define UCSI_SET_UOR 0x09 +#define UCSI_SET_PDM 0x0a +#define UCSI_SET_PDR 0x0b +#define UCSI_GET_ALTERNATE_MODES 0x0c +#define UCSI_GET_CAM_SUPPORTED 0x0d +#define UCSI_GET_CURRENT_CAM 0x0e +#define UCSI_SET_NEW_CAM 0x0f +#define UCSI_GET_PDOS 0x10 +#define UCSI_GET_CABLE_PROPERTY 0x11 +#define UCSI_GET_CABLE_PROPERTY_SIZE 64 +#define UCSI_GET_CONNECTOR_STATUS 0x12 +#define UCSI_GET_CONNECTOR_STATUS_SIZE 152 +#define UCSI_GET_ERROR_STATUS 0x13 +#define UCSI_GET_PD_MESSAGE 0x15 +#define UCSI_SET_SINK_PATH 0x1c #define UCSI_CONNECTOR_NUMBER(_num_) ((u64)(_num_) << 16) #define UCSI_COMMAND(_cmd_) ((_cmd_) & 0xff) @@ -127,7 +132,6 @@ void ucsi_connector_change(struct ucsi *ucsi, u8 num); #define UCSI_CONNECTOR_RESET_HARD_VER_1_0 BIT(23) /* Deprecated in v1.1 */ #define UCSI_CONNECTOR_RESET_DATA_VER_2_0 BIT(23) /* Redefined in v2.0 */ - /* ACK_CC_CI bits */ #define UCSI_ACK_CONNECTOR_CHANGE BIT(16) #define UCSI_ACK_COMMAND_COMPLETE BIT(17) @@ -251,50 +255,6 @@ struct ucsi_capability { u16 typec_version; } __packed; -/* Data structure filled by PPM in response to GET_CONNECTOR_CAPABILITY cmd. */ -struct ucsi_connector_capability { - u8 op_mode; -#define UCSI_CONCAP_OPMODE_DFP BIT(0) -#define UCSI_CONCAP_OPMODE_UFP BIT(1) -#define UCSI_CONCAP_OPMODE_DRP BIT(2) -#define UCSI_CONCAP_OPMODE_AUDIO_ACCESSORY BIT(3) -#define UCSI_CONCAP_OPMODE_DEBUG_ACCESSORY BIT(4) -#define UCSI_CONCAP_OPMODE_USB2 BIT(5) -#define UCSI_CONCAP_OPMODE_USB3 BIT(6) -#define UCSI_CONCAP_OPMODE_ALT_MODE BIT(7) - u32 flags; -#define UCSI_CONCAP_FLAG_PROVIDER BIT(0) -#define UCSI_CONCAP_FLAG_CONSUMER BIT(1) -#define UCSI_CONCAP_FLAG_SWAP_TO_DFP BIT(2) -#define UCSI_CONCAP_FLAG_SWAP_TO_UFP BIT(3) -#define UCSI_CONCAP_FLAG_SWAP_TO_SRC BIT(4) -#define UCSI_CONCAP_FLAG_SWAP_TO_SINK BIT(5) -#define UCSI_CONCAP_FLAG_EX_OP_MODE(_f_) \ - (((_f_) & GENMASK(13, 6)) >> 6) -#define UCSI_CONCAP_EX_OP_MODE_USB4_GEN2 BIT(0) -#define UCSI_CONCAP_EX_OP_MODE_EPR_SRC BIT(1) -#define UCSI_CONCAP_EX_OP_MODE_EPR_SINK BIT(2) -#define UCSI_CONCAP_EX_OP_MODE_USB4_GEN3 BIT(3) -#define UCSI_CONCAP_EX_OP_MODE_USB4_GEN4 BIT(4) -#define UCSI_CONCAP_FLAG_MISC_CAPS(_f_) \ - (((_f_) & GENMASK(17, 14)) >> 14) -#define UCSI_CONCAP_MISC_CAP_FW_UPDATE BIT(0) -#define UCSI_CONCAP_MISC_CAP_SECURITY BIT(1) -#define UCSI_CONCAP_FLAG_REV_CURR_PROT_SUPPORT BIT(18) -#define UCSI_CONCAP_FLAG_PARTNER_PD_MAJOR_REV(_f_) \ - (((_f_) & GENMASK(20, 19)) >> 19) -#define UCSI_CONCAP_FLAG_PARTNER_PD_MAJOR_REV_AS_BCD(_f_) \ - UCSI_SPEC_REVISION_TO_BCD(UCSI_CONCAP_FLAG_PARTNER_PD_MAJOR_REV(_f_)) -} __packed; - -#define UCSI_CONCAP_USB2_SUPPORT(_con_) ((_con_)->cap.op_mode & UCSI_CONCAP_OPMODE_USB2) -#define UCSI_CONCAP_USB3_SUPPORT(_con_) ((_con_)->cap.op_mode & UCSI_CONCAP_OPMODE_USB3) -#define UCSI_CONCAP_USB4_SUPPORT(_con_) \ - ((_con_)->ucsi->version >= UCSI_VERSION_2_0 && \ - ((_con_)->cap.flags & (UCSI_CONCAP_EX_OP_MODE_USB4_GEN2 | \ - UCSI_CONCAP_EX_OP_MODE_USB4_GEN3 | \ - UCSI_CONCAP_EX_OP_MODE_USB4_GEN4))) - struct ucsi_altmode { u16 svid; u32 mid; @@ -320,51 +280,143 @@ struct ucsi_cable_property { u8 latency; } __packed; -/* Data structure filled by PPM in response to GET_CONNECTOR_STATUS command. */ -struct ucsi_connector_status { - u16 change; -#define UCSI_CONSTAT_EXT_SUPPLY_CHANGE BIT(1) -#define UCSI_CONSTAT_POWER_OPMODE_CHANGE BIT(2) -#define UCSI_CONSTAT_PDOS_CHANGE BIT(5) -#define UCSI_CONSTAT_POWER_LEVEL_CHANGE BIT(6) -#define UCSI_CONSTAT_PD_RESET_COMPLETE BIT(7) -#define UCSI_CONSTAT_CAM_CHANGE BIT(8) -#define UCSI_CONSTAT_BC_CHANGE BIT(9) -#define UCSI_CONSTAT_PARTNER_CHANGE BIT(11) -#define UCSI_CONSTAT_POWER_DIR_CHANGE BIT(12) -#define UCSI_CONSTAT_CONNECT_CHANGE BIT(14) -#define UCSI_CONSTAT_ERROR BIT(15) - u16 flags; -#define UCSI_CONSTAT_PWR_OPMODE(_f_) ((_f_) & GENMASK(2, 0)) +/* Get Connector Capability Fields. */ +#define UCSI_CONCAP_OPMODE UCSI_DECLARE_BITFIELD(0, 0, 8) +#define UCSI_CONCAP_OPMODE_DFP UCSI_DECLARE_BITFIELD(0, 0, 1) +#define UCSI_CONCAP_OPMODE_UFP UCSI_DECLARE_BITFIELD(0, 1, 1) +#define UCSI_CONCAP_OPMODE_DRP UCSI_DECLARE_BITFIELD(0, 2, 1) +#define UCSI_CONCAP_OPMODE_AUDIO_ACCESSORY UCSI_DECLARE_BITFIELD(0, 3, 1) +#define UCSI_CONCAP_OPMODE_DEBUG_ACCESSORY UCSI_DECLARE_BITFIELD(0, 4, 1) +#define UCSI_CONCAP_OPMODE_USB2 UCSI_DECLARE_BITFIELD(0, 5, 1) +#define UCSI_CONCAP_OPMODE_USB3 UCSI_DECLARE_BITFIELD(0, 6, 1) +#define UCSI_CONCAP_OPMODE_ALT_MODE UCSI_DECLARE_BITFIELD(0, 7, 1) +#define UCSI_CONCAP_PROVIDER UCSI_DECLARE_BITFIELD(0, 8, 1) +#define UCSI_CONCAP_CONSUMER UCSI_DECLARE_BITFIELD(0, 9, 1) +#define UCSI_CONCAP_SWAP_TO_DFP_V1_1 UCSI_DECLARE_BITFIELD_V1_1(10, 1) +#define UCSI_CONCAP_SWAP_TO_UFP_V1_1 UCSI_DECLARE_BITFIELD_V1_1(11, 1) +#define UCSI_CONCAP_SWAP_TO_SRC_V1_1 UCSI_DECLARE_BITFIELD_V1_1(12, 1) +#define UCSI_CONCAP_SWAP_TO_SNK_V1_1 UCSI_DECLARE_BITFIELD_V1_1(13, 1) +#define UCSI_CONCAP_EXT_OPMODE_V2_0 UCSI_DECLARE_BITFIELD_V2_0(14, 8) +#define UCSI_CONCAP_EXT_OPMODE_USB4_GEN2_V2_0 UCSI_DECLARE_BITFIELD_V2_0(14, 1) +#define UCSI_CONCAP_EXT_OPMODE_EPR_SRC_V2_0 UCSI_DECLARE_BITFIELD_V2_0(15, 1) +#define UCSI_CONCAP_EXT_OPMODE_EPR_SINK_V2_0 UCSI_DECLARE_BITFIELD_V2_0(16, 1) +#define UCSI_CONCAP_EXT_OPMODE_USB4_GEN3_V2_0 UCSI_DECLARE_BITFIELD_V2_0(17, 1) +#define UCSI_CONCAP_EXT_OPMODE_USB4_GEN4_V2_0 UCSI_DECLARE_BITFIELD_V2_0(18, 1) +#define UCSI_CONCAP_MISC_V2_0 UCSI_DECLARE_BITFIELD_V2_0(22, 4) +#define UCSI_CONCAP_MISC_FW_UPDATE_V2_0 UCSI_DECLARE_BITFIELD_V2_0(22, 1) +#define UCSI_CONCAP_MISC_SECURITY_V2_0 UCSI_DECLARE_BITFIELD_V2_0(23, 1) +#define UCSI_CONCAP_REV_CURR_PROT_SUPPORT_V2_0 UCSI_DECLARE_BITFIELD_V2_0(26, 1) +#define UCSI_CONCAP_PARTNER_PD_REVISION_V2_1 UCSI_DECLARE_BITFIELD_V2_1(27, 2) + +/* Helpers for USB capability checks. */ +#define UCSI_CONCAP_USB2_SUPPORT(_con_) UCSI_CONCAP((_con_), OPMODE_USB2) +#define UCSI_CONCAP_USB3_SUPPORT(_con_) UCSI_CONCAP((_con_), OPMODE_USB3) +#define UCSI_CONCAP_USB4_SUPPORT(_con_) \ + ((_con_)->ucsi->version >= UCSI_VERSION_2_0 && \ + (UCSI_CONCAP((_con_), EXT_OPMODE_USB4_GEN2_V2_0) | \ + UCSI_CONCAP((_con_), EXT_OPMODE_USB4_GEN3_V2_0) | \ + UCSI_CONCAP((_con_), EXT_OPMODE_USB4_GEN4_V2_0))) + +/* Get Connector Status Fields. */ +#define UCSI_CONSTAT_CHANGE UCSI_DECLARE_BITFIELD(0, 0, 16) +#define UCSI_CONSTAT_PWR_OPMODE UCSI_DECLARE_BITFIELD(0, 16, 3) #define UCSI_CONSTAT_PWR_OPMODE_NONE 0 #define UCSI_CONSTAT_PWR_OPMODE_DEFAULT 1 #define UCSI_CONSTAT_PWR_OPMODE_BC 2 #define UCSI_CONSTAT_PWR_OPMODE_PD 3 #define UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5 4 #define UCSI_CONSTAT_PWR_OPMODE_TYPEC3_0 5 -#define UCSI_CONSTAT_CONNECTED BIT(3) -#define UCSI_CONSTAT_PWR_DIR BIT(4) -#define UCSI_CONSTAT_PARTNER_FLAGS(_f_) (((_f_) & GENMASK(12, 5)) >> 5) -#define UCSI_CONSTAT_PARTNER_FLAG_USB 1 -#define UCSI_CONSTAT_PARTNER_FLAG_ALT_MODE 2 -#define UCSI_CONSTAT_PARTNER_FLAG_USB4_GEN3 4 -#define UCSI_CONSTAT_PARTNER_FLAG_USB4_GEN4 8 -#define UCSI_CONSTAT_PARTNER_TYPE(_f_) (((_f_) & GENMASK(15, 13)) >> 13) +#define UCSI_CONSTAT_CONNECTED UCSI_DECLARE_BITFIELD(0, 19, 1) +#define UCSI_CONSTAT_PWR_DIR UCSI_DECLARE_BITFIELD(0, 20, 1) +#define UCSI_CONSTAT_PARTNER_FLAGS UCSI_DECLARE_BITFIELD(0, 21, 8) +#define UCSI_CONSTAT_PARTNER_FLAG_USB UCSI_DECLARE_BITFIELD(0, 21, 1) +#define UCSI_CONSTAT_PARTNER_FLAG_ALT_MODE UCSI_DECLARE_BITFIELD(0, 22, 1) +#define UCSI_CONSTAT_PARTNER_FLAG_USB4_GEN3 UCSI_DECLARE_BITFIELD(0, 23, 1) +#define UCSI_CONSTAT_PARTNER_FLAG_USB4_GEN4 UCSI_DECLARE_BITFIELD(0, 24, 1) +#define UCSI_CONSTAT_PARTNER_TYPE UCSI_DECLARE_BITFIELD(0, 29, 3) #define UCSI_CONSTAT_PARTNER_TYPE_DFP 1 #define UCSI_CONSTAT_PARTNER_TYPE_UFP 2 -#define UCSI_CONSTAT_PARTNER_TYPE_CABLE 3 /* Powered Cable */ -#define UCSI_CONSTAT_PARTNER_TYPE_CABLE_AND_UFP 4 /* Powered Cable */ +#define UCSI_CONSTAT_PARTNER_TYPE_CABLE 3 /* Powered Cable */ +#define UCSI_CONSTAT_PARTNER_TYPE_CABLE_AND_UFP 4 /* Powered Cable */ #define UCSI_CONSTAT_PARTNER_TYPE_DEBUG 5 #define UCSI_CONSTAT_PARTNER_TYPE_AUDIO 6 - u32 request_data_obj; - - u8 pwr_status; -#define UCSI_CONSTAT_BC_STATUS(_p_) ((_p_) & GENMASK(1, 0)) +#define UCSI_CONSTAT_RDO UCSI_DECLARE_BITFIELD(0, 32, 32) +#define UCSI_CONSTAT_BC_STATUS UCSI_DECLARE_BITFIELD(0, 64, 2) #define UCSI_CONSTAT_BC_NOT_CHARGING 0 #define UCSI_CONSTAT_BC_NOMINAL_CHARGING 1 #define UCSI_CONSTAT_BC_SLOW_CHARGING 2 #define UCSI_CONSTAT_BC_TRICKLE_CHARGING 3 -} __packed; +#define UCSI_CONSTAT_PD_VERSION_V1_2 UCSI_DECLARE_BITFIELD_V1_2(70, 16) + +/* Connector Status Change Bits. */ +#define UCSI_CONSTAT_EXT_SUPPLY_CHANGE BIT(1) +#define UCSI_CONSTAT_POWER_OPMODE_CHANGE BIT(2) +#define UCSI_CONSTAT_PDOS_CHANGE BIT(5) +#define UCSI_CONSTAT_POWER_LEVEL_CHANGE BIT(6) +#define UCSI_CONSTAT_PD_RESET_COMPLETE BIT(7) +#define UCSI_CONSTAT_CAM_CHANGE BIT(8) +#define UCSI_CONSTAT_BC_CHANGE BIT(9) +#define UCSI_CONSTAT_PARTNER_CHANGE BIT(11) +#define UCSI_CONSTAT_POWER_DIR_CHANGE BIT(12) +#define UCSI_CONSTAT_CONNECT_CHANGE BIT(14) +#define UCSI_CONSTAT_ERROR BIT(15) + +#define UCSI_DECLARE_BITFIELD_V1_1(_offset_, _size_) \ + UCSI_DECLARE_BITFIELD(UCSI_VERSION_1_1, (_offset_), (_size_)) +#define UCSI_DECLARE_BITFIELD_V1_2(_offset_, _size_) \ + UCSI_DECLARE_BITFIELD(UCSI_VERSION_1_2, (_offset_), (_size_)) +#define UCSI_DECLARE_BITFIELD_V2_0(_offset_, _size_) \ + UCSI_DECLARE_BITFIELD(UCSI_VERSION_2_0, (_offset_), (_size_)) +#define UCSI_DECLARE_BITFIELD_V2_1(_offset_, _size_) \ + UCSI_DECLARE_BITFIELD(UCSI_VERSION_2_1, (_offset_), (_size_)) +#define UCSI_DECLARE_BITFIELD_V3_0(_offset_, _size_) \ + UCSI_DECLARE_BITFIELD(UCSI_VERSION_3_0, (_offset_), (_size_)) + +#define UCSI_DECLARE_BITFIELD(_ver_, _offset_, _size_) \ +(struct ucsi_bitfield) { \ + .version = _ver_, \ + .offset = _offset_, \ + .size = _size_, \ +} + +struct ucsi_bitfield { + const u16 version; + const u8 offset; + const u8 size; +}; + +/** + * ucsi_bitfield_read - Read a field from UCSI command response + * @_map_: UCSI command response + * @_field_: The field offset in the response data structure + * @_ver_: UCSI version where the field was introduced + * + * Reads the fields in the command responses by first checking that the field is + * valid with the UCSI interface version that is used in the system. + * @_ver_ is the minimum UCSI version for the @_field_. If the UCSI interface is + * older than @_ver_, a warning is generated. + * + * Caveats: + * - Removed fields are not checked - @_ver_ is just the minimum UCSI version. + * + * Returns the value of @_field_, or 0 when the UCSI interface is older than + * @_ver_. + */ +#define ucsi_bitfield_read(_map_, _field_, _ver_) \ +({ \ + struct ucsi_bitfield f = (_field_); \ + WARN((_ver_) < f.version, \ + "Access to unsupported field at offset 0x%x (need version %04x)", \ + f.offset, f.version) ? 0 : \ + bitmap_read((_map_), f.offset, f.size); \ +}) + +/* Helpers to access cached command responses. */ +#define UCSI_CONCAP(_con_, _field_) \ + ucsi_bitfield_read((_con_)->cap, UCSI_CONCAP_##_field_, (_con_)->ucsi->version) + +#define UCSI_CONSTAT(_con_, _field_) \ + ucsi_bitfield_read((_con_)->status, UCSI_CONSTAT_##_field_, (_con_)->ucsi->version) /* -------------------------------------------------------------------------- */ @@ -444,8 +496,10 @@ struct ucsi_connector { struct typec_capability typec_cap; - struct ucsi_connector_status status; - struct ucsi_connector_capability cap; + /* Cached command responses. */ + DECLARE_BITMAP(cap, UCSI_GET_CONNECTOR_CAPABILITY_SIZE); + DECLARE_BITMAP(status, UCSI_GET_CONNECTOR_STATUS_SIZE); + struct power_supply *psy; struct power_supply_desc psy_desc; u32 rdo; diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c index 789f67dd9f81..5c5515551963 100644 --- a/drivers/usb/typec/ucsi/ucsi_acpi.c +++ b/drivers/usb/typec/ucsi/ucsi_acpi.c @@ -104,7 +104,6 @@ static int ucsi_gram_read_message_in(struct ucsi *ucsi, void *val, size_t val_le u16 bogus_change = UCSI_CONSTAT_POWER_LEVEL_CHANGE | UCSI_CONSTAT_PDOS_CHANGE; struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi); - struct ucsi_connector_status *status; int ret; ret = ucsi_acpi_read_message_in(ucsi, val, val_len); @@ -113,11 +112,9 @@ static int ucsi_gram_read_message_in(struct ucsi *ucsi, void *val, size_t val_le if (UCSI_COMMAND(ua->cmd) == UCSI_GET_CONNECTOR_STATUS && ua->check_bogus_event) { - status = (struct ucsi_connector_status *)val; - /* Clear the bogus change */ - if (status->change == bogus_change) - status->change = 0; + if (*(u16 *)val == bogus_change) + *(u16 *)val = 0; ua->check_bogus_event = false; } -- 2.51.0 From 40aeea50444793ed106997a49c7083b656bccfa7 Mon Sep 17 00:00:00 2001 From: Philipp Stanner Date: Tue, 5 Nov 2024 12:11:33 +0100 Subject: [PATCH 06/16] thunderbolt: Replace deprecated PCI functions pcim_iomap_table() and pcim_request_regions() have been deprecated in commit e354bb84a4c1 ("PCI: Deprecate pcim_iomap_table(), pcim_iomap_regions_request_all()") and commit d140f80f60358 ("PCI: Deprecate pcim_iomap_regions() in favor of pcim_iomap_region()"). Replace these functions with pcim_iomap_region(). Signed-off-by: Philipp Stanner Signed-off-by: Mika Westerberg --- drivers/thunderbolt/nhi.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c index 7af2642b97cb..1257dd3ce7e6 100644 --- a/drivers/thunderbolt/nhi.c +++ b/drivers/thunderbolt/nhi.c @@ -1340,18 +1340,18 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id) if (res) return dev_err_probe(dev, res, "cannot enable PCI device, aborting\n"); - res = pcim_iomap_regions(pdev, 1 << 0, "thunderbolt"); - if (res) - return dev_err_probe(dev, res, "cannot obtain PCI resources, aborting\n"); - nhi = devm_kzalloc(&pdev->dev, sizeof(*nhi), GFP_KERNEL); if (!nhi) return -ENOMEM; nhi->pdev = pdev; nhi->ops = (const struct tb_nhi_ops *)id->driver_data; - /* cannot fail - table is allocated in pcim_iomap_regions */ - nhi->iobase = pcim_iomap_table(pdev)[0]; + + nhi->iobase = pcim_iomap_region(pdev, 0, "thunderbolt"); + res = PTR_ERR_OR_ZERO(nhi->iobase); + if (res) + return dev_err_probe(dev, res, "cannot obtain PCI resources, aborting\n"); + nhi->hop_count = ioread32(nhi->iobase + REG_CAPS) & 0x3ff; dev_dbg(dev, "total paths: %d\n", nhi->hop_count); -- 2.51.0 From 7f72d17359e5916eb1abb416c7ac57f7eeb8aa51 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Thu, 7 Nov 2024 13:33:48 +0000 Subject: [PATCH 07/16] usb: gadget: function: remove redundant else statement After an initial range change on the insigned int alt being > 1 the only possible values for alt are 0 or 1. Therefore the else statement for values other than 0 or 1 is redundant and can be removed. Replace the else if (all == 1) check with just an else. Signed-off-by: Colin Ian King Reviewed-by: Takashi Iwai Link: https://lore.kernel.org/5f54ffd0-b5fe-4203-a626-c166becad362@gmail.com Link: https://lore.kernel.org/r/20241107133348.22762-1-colin.i.king@gmail.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/gadget/function/f_midi2.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/gadget/function/f_midi2.c b/drivers/usb/gadget/function/f_midi2.c index 8285df9ed6fd..ee3d9e3578f7 100644 --- a/drivers/usb/gadget/function/f_midi2.c +++ b/drivers/usb/gadget/function/f_midi2.c @@ -1285,10 +1285,8 @@ static int f_midi2_set_alt(struct usb_function *fn, unsigned int intf, if (alt == 0) op_mode = MIDI_OP_MODE_MIDI1; - else if (alt == 1) - op_mode = MIDI_OP_MODE_MIDI2; else - op_mode = MIDI_OP_MODE_UNSET; + op_mode = MIDI_OP_MODE_MIDI2; if (midi2->operation_mode == op_mode) return 0; -- 2.51.0 From 4a22918810980897393fa1776ea3877e4baf8cca Mon Sep 17 00:00:00 2001 From: Dmitry Baryshkov Date: Sat, 9 Nov 2024 02:04:14 +0200 Subject: [PATCH 08/16] usb: typec: ucsi: glink: fix off-by-one in connector_status UCSI connector's indices start from 1 up to 3, PMIC_GLINK_MAX_PORTS. Correct the condition in the pmic_glink_ucsi_connector_status() callback, fixing Type-C orientation reporting for the third USB-C connector. Fixes: 76716fd5bf09 ("usb: typec: ucsi: glink: move GPIO reading into connector_status callback") Cc: stable@vger.kernel.org Reported-by: Abel Vesa Reviewed-by: Neil Armstrong Reviewed-by: Johan Hovold Tested-by: Johan Hovold Signed-off-by: Dmitry Baryshkov Link: https://lore.kernel.org/r/20241109-ucsi-glue-fixes-v2-1-8b21ff4f9fbe@linaro.org Signed-off-by: Greg Kroah-Hartman --- 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 3e4d88ab338e..2e12758000a7 100644 --- a/drivers/usb/typec/ucsi/ucsi_glink.c +++ b/drivers/usb/typec/ucsi/ucsi_glink.c @@ -185,7 +185,7 @@ static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con) struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi); int orientation; - if (con->num >= PMIC_GLINK_MAX_PORTS || + if (con->num > PMIC_GLINK_MAX_PORTS || !ucsi->port_orientation[con->num - 1]) return; -- 2.51.0 From de9df030ccb5d3e31ee0c715d74cd77c619748f8 Mon Sep 17 00:00:00 2001 From: Dmitry Baryshkov Date: Sat, 9 Nov 2024 02:04:15 +0200 Subject: [PATCH 09/16] usb: typec: ucsi: glink: be more precise on orientation-aware ports Instead of checking if any of the USB-C ports have orientation GPIO and thus is orientation-aware, check for the GPIO for the port being registered. There are no boards that are affected by this change at this moment, so the patch is not marked as a fix, but it might affect other boards in future. Reviewed-by: Abel Vesa Reviewed-by: Neil Armstrong Reviewed-by: Johan Hovold Tested-by: Johan Hovold Signed-off-by: Dmitry Baryshkov Link: https://lore.kernel.org/r/20241109-ucsi-glue-fixes-v2-2-8b21ff4f9fbe@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/usb/typec/ucsi/ucsi_glink.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c index 2e12758000a7..90948cd6d297 100644 --- a/drivers/usb/typec/ucsi/ucsi_glink.c +++ b/drivers/usb/typec/ucsi/ucsi_glink.c @@ -172,12 +172,12 @@ static int pmic_glink_ucsi_async_control(struct ucsi *__ucsi, u64 command) static void pmic_glink_ucsi_update_connector(struct ucsi_connector *con) { struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi); - int i; - for (i = 0; i < PMIC_GLINK_MAX_PORTS; i++) { - if (ucsi->port_orientation[i]) - con->typec_cap.orientation_aware = true; - } + if (con->num > PMIC_GLINK_MAX_PORTS || + !ucsi->port_orientation[con->num - 1]) + return; + + con->typec_cap.orientation_aware = true; } static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con) -- 2.51.0 From 3fc137386c4620305bbc2a216868c53f9245670a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Hubert=20Wi=C5=9Bniewski?= Date: Sun, 10 Nov 2024 18:21:48 +0100 Subject: [PATCH 10/16] usb: musb: Fix hardware lockup on first Rx endpoint request MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit There is a possibility that a request's callback could be invoked from usb_ep_queue() (call trace below, supplemented with missing calls): req->complete from usb_gadget_giveback_request (drivers/usb/gadget/udc/core.c:999) usb_gadget_giveback_request from musb_g_giveback (drivers/usb/musb/musb_gadget.c:147) musb_g_giveback from rxstate (drivers/usb/musb/musb_gadget.c:784) rxstate from musb_ep_restart (drivers/usb/musb/musb_gadget.c:1169) musb_ep_restart from musb_ep_restart_resume_work (drivers/usb/musb/musb_gadget.c:1176) musb_ep_restart_resume_work from musb_queue_resume_work (drivers/usb/musb/musb_core.c:2279) musb_queue_resume_work from musb_gadget_queue (drivers/usb/musb/musb_gadget.c:1241) musb_gadget_queue from usb_ep_queue (drivers/usb/gadget/udc/core.c:300) According to the docstring of usb_ep_queue(), this should not happen: "Note that @req's ->complete() callback must never be called from within usb_ep_queue() as that can create deadlock situations." In fact, a hardware lockup might occur in the following sequence: 1. The gadget is initialized using musb_gadget_enable(). 2. Meanwhile, a packet arrives, and the RXPKTRDY flag is set, raising an interrupt. 3. If IRQs are enabled, the interrupt is handled, but musb_g_rx() finds an empty queue (next_request() returns NULL). The interrupt flag has already been cleared by the glue layer handler, but the RXPKTRDY flag remains set. 4. The first request is enqueued using usb_ep_queue(), leading to the call of req->complete(), as shown in the call trace above. 5. If the callback enables IRQs and another packet is waiting, step (3) repeats. The request queue is empty because usb_g_giveback() removes the request before invoking the callback. 6. The endpoint remains locked up, as the interrupt triggered by hardware setting the RXPKTRDY flag has been handled, but the flag itself remains set. For this scenario to occur, it is only necessary for IRQs to be enabled at some point during the complete callback. This happens with the USB Ethernet gadget, whose rx_complete() callback calls netif_rx(). If called in the task context, netif_rx() disables the bottom halves (BHs). When the BHs are re-enabled, IRQs are also enabled to allow soft IRQs to be processed. The gadget itself is initialized at module load (or at boot if built-in), but the first request is enqueued when the network interface is brought up, triggering rx_complete() in the task context via ioctl(). If a packet arrives while the interface is down, it can prevent the interface from receiving any further packets from the USB host. The situation is quite complicated with many parties involved. This particular issue can be resolved in several possible ways: 1. Ensure that callbacks never enable IRQs. This would be difficult to enforce, as discovering how netif_rx() interacts with interrupts was already quite challenging and u_ether is not the only function driver. Similar "bugs" could be hidden in other drivers as well. 2. Disable MUSB interrupts in musb_g_giveback() before calling the callback and re-enable them afterwars (by calling musb_{dis,en}able_interrupts(), for example). This would ensure that MUSB interrupts are not handled during the callback, even if IRQs are enabled. In fact, it would allow IRQs to be enabled when releasing the lock. However, this feels like an inelegant hack. 3. Modify the interrupt handler to clear the RXPKTRDY flag if the request queue is empty. While this approach also feels like a hack, it wastes CPU time by attempting to handle incoming packets when the software is not ready to process them. 4. Flush the Rx FIFO instead of calling rxstate() in musb_ep_restart(). This ensures that the hardware can receive packets when there is at least one request in the queue. Once IRQs are enabled, the interrupt handler will be able to correctly process the next incoming packet (eventually calling rxstate()). This approach may cause one or two packets to be dropped (two if double buffering is enabled), but this seems to be a minor issue, as packet loss can occur when the software is not yet ready to process them. Additionally, this solution makes the gadget driver compliant with the rule mentioned in the docstring of usb_ep_queue(). There may be additional solutions, but from these four, the last one has been chosen as it seems to be the most appropriate, as it addresses the "bad" behavior of the driver. Fixes: baebdf48c360 ("net: dev: Makes sure netif_rx() can be invoked in any context.") Cc: stable@vger.kernel.org Signed-off-by: Hubert Wiśniewski Link: https://lore.kernel.org/r/4ee1ead4525f78fb5909a8cbf99513ad0082ad21.camel@gmail.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/musb/musb_gadget.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c index bdf13911a1e5..c6076df0d50c 100644 --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -1161,12 +1161,19 @@ void musb_free_request(struct usb_ep *ep, struct usb_request *req) */ void musb_ep_restart(struct musb *musb, struct musb_request *req) { + u16 csr; + void __iomem *epio = req->ep->hw_ep->regs; + trace_musb_req_start(req); musb_ep_select(musb->mregs, req->epnum); - if (req->tx) + if (req->tx) { txstate(musb, req); - else - rxstate(musb, req); + } else { + csr = musb_readw(epio, MUSB_RXCSR); + csr |= MUSB_RXCSR_FLUSHFIFO | MUSB_RXCSR_P_WZC_BITS; + musb_writew(epio, MUSB_RXCSR, csr); + musb_writew(epio, MUSB_RXCSR, csr); + } } static int musb_ep_restart_resume_work(struct musb *musb, void *data) -- 2.51.0 From 65c4c9447bfc5b80b88e4d354d09c8fb86fad07f Mon Sep 17 00:00:00 2001 From: Heikki Krogerus Date: Mon, 11 Nov 2024 12:02:20 +0200 Subject: [PATCH 11/16] usb: typec: ucsi: Fix a missing bits to bytes conversion in ucsi_init() The GET_CAPABILITY size is wrong. The definitions for the command sizes are for bitfieds and therefore in bits, not bytes. This fixes an issue that prevents the interface from being registered with UCSI versions older than 2.0 because the command size exceeds the MESSAGE_IN field size. Fixes: 226ff2e681d0 ("usb: typec: ucsi: Convert connector specific commands to bitmaps") Reported-by: Abel Vesa Closes: https://lore.kernel.org/linux-usb/Zy864W7sysWZbCTd@linaro.org/ Signed-off-by: Heikki Krogerus Reviewed-by: Abel Vesa Tested-by: Abel Vesa Link: https://lore.kernel.org/r/20241111100220.1743872-1-heikki.krogerus@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/typec/ucsi/ucsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index 974a441155e1..c435c0835744 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -1775,7 +1775,8 @@ static int ucsi_init(struct ucsi *ucsi) /* Get PPM capabilities */ command = UCSI_GET_CAPABILITY; - ret = ucsi_send_command(ucsi, command, &ucsi->cap, UCSI_GET_CAPABILITY_SIZE); + ret = ucsi_send_command(ucsi, command, &ucsi->cap, + BITS_TO_BYTES(UCSI_GET_CAPABILITY_SIZE)); if (ret < 0) goto err_reset; -- 2.51.0 From 3339aff5feac899b27038cc1d54fb48c0ddd94f2 Mon Sep 17 00:00:00 2001 From: Xu Yang Date: Mon, 11 Nov 2024 17:09:16 +0800 Subject: [PATCH 12/16] usb: chipidea: imx: add imx8ulp support The dtbinding have imx7ulp and imx8ulp compatible with imx7d before. And then the dtb follow the dtbinding. However, the driver doesn't add imx8ulp compatible now. To make imx8ulp work well, this will add support for it. Signed-off-by: Xu Yang Acked-by: Peter Chen Link: https://lore.kernel.org/r/20241111090916.1534047-1-xu.yang_2@nxp.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/chipidea/usbmisc_imx.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 173c78afd502..1394881fde5f 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -1285,6 +1285,10 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = { .compatible = "fsl,imx7ulp-usbmisc", .data = &imx7ulp_usbmisc_ops, }, + { + .compatible = "fsl,imx8ulp-usbmisc", + .data = &imx7ulp_usbmisc_ops, + }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids); -- 2.51.0 From 6ea8fa9c2faf699d6599158df8ea2185fca4667b Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 11 Nov 2024 01:30:27 +0000 Subject: [PATCH 13/16] dt-bindings: usb: sunxi-musb: add Allwinner A523 compatible string The Allwinner A523/T527 SoCs have a MUSB controller fully compatible to the D1 (and ultimately the A33), with five endpoints. Add the new name to the list of compatible strings. Signed-off-by: Andre Przywara Acked-by: Conor Dooley Link: https://lore.kernel.org/r/20241111013033.22793-9-andre.przywara@arm.com Signed-off-by: Greg Kroah-Hartman --- .../devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml b/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml index f972ce976e86..57c17d5a62d6 100644 --- a/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml +++ b/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml @@ -24,6 +24,7 @@ properties: - allwinner,sun8i-a83t-musb - allwinner,sun20i-d1-musb - allwinner,sun50i-h6-musb + - allwinner,sun55i-a523-musb - const: allwinner,sun8i-a33-musb - items: - const: allwinner,sun50i-h616-musb -- 2.51.0 From 1d062ff3034866c94658d40d5e26f7bd7ef9d83c Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 11 Nov 2024 01:30:28 +0000 Subject: [PATCH 14/16] dt-bindings: usb: add A523 compatible string for EHCI and OCHI The Allwinner A523/T527 feature generic EHCI and OHCI compatible USB-2.0 host controllers (in addition to an MUSB and an XHCI controller). Add the new name to the list of supported compatible strings. Signed-off-by: Andre Przywara Acked-by: Conor Dooley Link: https://lore.kernel.org/r/20241111013033.22793-10-andre.przywara@arm.com Signed-off-by: Greg Kroah-Hartman --- Documentation/devicetree/bindings/usb/generic-ehci.yaml | 1 + Documentation/devicetree/bindings/usb/generic-ohci.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/generic-ehci.yaml b/Documentation/devicetree/bindings/usb/generic-ehci.yaml index 2ed178f16a78..9c5884c1e7c5 100644 --- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml +++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml @@ -31,6 +31,7 @@ properties: - allwinner,sun50i-a64-ehci - allwinner,sun50i-h6-ehci - allwinner,sun50i-h616-ehci + - allwinner,sun55i-a523-ehci - allwinner,sun5i-a13-ehci - allwinner,sun6i-a31-ehci - allwinner,sun7i-a20-ehci diff --git a/Documentation/devicetree/bindings/usb/generic-ohci.yaml b/Documentation/devicetree/bindings/usb/generic-ohci.yaml index b9576015736b..f1ae45aa4c86 100644 --- a/Documentation/devicetree/bindings/usb/generic-ohci.yaml +++ b/Documentation/devicetree/bindings/usb/generic-ohci.yaml @@ -18,6 +18,7 @@ properties: - allwinner,sun50i-a64-ohci - allwinner,sun50i-h6-ohci - allwinner,sun50i-h616-ohci + - allwinner,sun55i-a523-ohci - allwinner,sun5i-a13-ohci - allwinner,sun6i-a31-ohci - allwinner,sun7i-a20-ohci -- 2.51.0 From 5c5d8eb8af06df615e8b1dc88e5847196c846045 Mon Sep 17 00:00:00 2001 From: Stanislaw Gruszka Date: Tue, 12 Nov 2024 08:55:12 +0100 Subject: [PATCH 15/16] usb: misc: ljca: move usb_autopm_put_interface() after wait for response Do not mark interface as ready to suspend when we are still waiting for response messages from the device. Fixes: acd6199f195d ("usb: Add support for Intel LJCA device") Cc: stable@vger.kernel.org Reviewed-by: Hans de Goede Tested-by: Hans de Goede # ThinkPad X1 Yoga Gen 8, ov2740 Acked-by: Sakari Ailus Signed-off-by: Stanislaw Gruszka Link: https://lore.kernel.org/r/20241112075514.680712-1-stanislaw.gruszka@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/misc/usb-ljca.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c index 01ceafc4ab78..8056c65e4548 100644 --- a/drivers/usb/misc/usb-ljca.c +++ b/drivers/usb/misc/usb-ljca.c @@ -332,14 +332,11 @@ static int ljca_send(struct ljca_adapter *adap, u8 type, u8 cmd, ret = usb_bulk_msg(adap->usb_dev, adap->tx_pipe, header, msg_len, &transferred, LJCA_WRITE_TIMEOUT_MS); - - usb_autopm_put_interface(adap->intf); - if (ret < 0) - goto out; + goto out_put; if (transferred != msg_len) { ret = -EIO; - goto out; + goto out_put; } if (ack) { @@ -347,11 +344,14 @@ static int ljca_send(struct ljca_adapter *adap, u8 type, u8 cmd, timeout); if (!ret) { ret = -ETIMEDOUT; - goto out; + goto out_put; } } ret = adap->actual_length; +out_put: + usb_autopm_put_interface(adap->intf); + out: spin_lock_irqsave(&adap->lock, flags); adap->ex_buf = NULL; -- 2.51.0 From 2481af79671a6603fce201cbbc48f31e488e9fae Mon Sep 17 00:00:00 2001 From: Stanislaw Gruszka Date: Tue, 12 Nov 2024 08:55:13 +0100 Subject: [PATCH 16/16] usb: misc: ljca: set small runtime autosuspend delay On some Lenovo platforms, the patch works around problems with ov2740 sensor initialization, which manifest themself like below: [ 4.540476] ov2740 i2c-INT3474:01: error -EIO: failed to find sensor [ 4.542066] ov2740 i2c-INT3474:01: probe with driver ov2740 failed with error -5 or [ 7.742633] ov2740 i2c-INT3474:01: chip id mismatch: 2740 != 0 [ 7.742638] ov2740 i2c-INT3474:01: error -ENXIO: failed to find sensor and also by random failures of video stream start. Issue can be reproduced by this script: n=0 k=0 while [ $n -lt 50 ] ; do sudo modprobe -r ov2740 sleep `expr $RANDOM % 5` sudo modprobe ov2740 if media-ctl -p | grep -q ov2740 ; then let k++ fi let n++ done echo Success rate $k/$n Without the patch, success rate is approximately 15 or 50 tries. With the patch it does not fail. This problem is some hardware or firmware malfunction, that can not be easy debug and fix. While setting small autosuspend delay is not perfect workaround as user can configure it to any value, it will prevent the failures by default. Additionally setting small autosuspend delay should have positive effect on power consumption as for most ljca workloads device is used for just a few milliseconds flowed by long periods of at least 100ms of inactivity (usually more). Fixes: acd6199f195d ("usb: Add support for Intel LJCA device") Cc: stable@vger.kernel.org Reviewed-by: Hans de Goede Tested-by: Hans de Goede # ThinkPad X1 Yoga Gen 8, ov2740 Acked-by: Sakari Ailus Signed-off-by: Stanislaw Gruszka Link: https://lore.kernel.org/r/20241112075514.680712-2-stanislaw.gruszka@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/misc/usb-ljca.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c index 8056c65e4548..d9c21f783055 100644 --- a/drivers/usb/misc/usb-ljca.c +++ b/drivers/usb/misc/usb-ljca.c @@ -811,6 +811,14 @@ static int ljca_probe(struct usb_interface *interface, if (ret) goto err_free; + /* + * This works around problems with ov2740 initialization on some + * Lenovo platforms. The autosuspend delay, has to be smaller than + * the delay after setting the reset_gpio line in ov2740_resume(). + * Otherwise the sensor randomly fails to initialize. + */ + pm_runtime_set_autosuspend_delay(&usb_dev->dev, 10); + usb_enable_autosuspend(usb_dev); return 0; -- 2.51.0