public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.19-6.12] usb: chipidea: udc: fix DMA and SG cleanup in _ep_nuke()
       [not found] <20260219020422.1539798-1-sashal@kernel.org>
@ 2026-02-19  2:03 ` Sasha Levin
  2026-02-19  2:03 ` [PATCH AUTOSEL 6.19-6.12] usb: gadget: f_fs: Fix ioctl error handling Sasha Levin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-02-19  2:03 UTC (permalink / raw)
  To: patches, stable
  Cc: Mario Peter, Xu Yang, Peter Chen, Greg Kroah-Hartman, Sasha Levin,
	linux-usb, linux-kernel

From: Mario Peter <mario.peter@leica-geosystems.com>

[ Upstream commit cea2a1257a3b5ea3e769a445b34af13e6aa5a123 ]

The ChipIdea UDC driver can encounter "not page aligned sg buffer"
errors when a USB device is reconnected after being disconnected
during an active transfer. This occurs because _ep_nuke() returns
requests to the gadget layer without properly unmapping DMA buffers
or cleaning up scatter-gather bounce buffers.

Root cause:
When a disconnect happens during a multi-segment DMA transfer, the
request's num_mapped_sgs field and sgt.sgl pointer remain set with
stale values. The request is returned to the gadget driver with status
-ESHUTDOWN but still has active DMA state. If the gadget driver reuses
this request on reconnect without reinitializing it, the stale DMA
state causes _hardware_enqueue() to skip DMA mapping (seeing non-zero
num_mapped_sgs) and attempt to use freed/invalid DMA addresses,
leading to alignment errors and potential memory corruption.

The normal completion path via _hardware_dequeue() properly calls
usb_gadget_unmap_request_by_dev() and sglist_do_debounce() before
returning the request. The _ep_nuke() path must do the same cleanup
to ensure requests are returned in a clean, reusable state.

Fix:
Add DMA unmapping and bounce buffer cleanup to _ep_nuke() to mirror
the cleanup sequence in _hardware_dequeue():
- Call usb_gadget_unmap_request_by_dev() if num_mapped_sgs is set
- Call sglist_do_debounce() with copy=false if bounce buffer exists

This ensures that when requests are returned due to endpoint shutdown,
they don't retain stale DMA mappings. The 'false' parameter to
sglist_do_debounce() prevents copying data back (appropriate for
shutdown path where transfer was aborted).

Signed-off-by: Mario Peter <mario.peter@leica-geosystems.com>
Reviewed-by: Xu Yang <xu.yang_2@nxp.com>
Acked-by: Peter Chen <peter.chen@kernel.org>
Link: https://patch.msgid.link/20260108165902.795354-1-mario.peter@leica-geosystems.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

This confirms:
1. `usb_gadget_unmap_request_by_dev()` at line 966 sets
   `req->num_mapped_sgs = 0` - this is exactly what's needed to clear
   the stale state
2. It also does `dma_unmap_sg()` at line 963 to properly unmap the DMA
   buffers
3. Without this call in `_ep_nuke()`, the request is given back with
   active DMA mappings and stale `num_mapped_sgs`

## 3. Assessment

### What the commit fixes:
- **DMA resource leak**: Requests returned via `_ep_nuke()` during
  disconnect have active DMA mappings that are never unmapped, leaking
  DMA resources
- **Stale state causing corruption**: The `num_mapped_sgs` field remains
  non-zero, causing `_hardware_enqueue()` to skip DMA mapping on reuse
  and instead use freed/invalid DMA addresses
- **Memory corruption**: Invalid DMA addresses lead to "not page aligned
  sg buffer" errors and potential memory corruption
- **Bounce buffer leak**: SG bounce buffers (`sgt.sgl`) allocated via
  `sglist_do_bounce()` are never freed, leaking memory

### Stable kernel criteria:
1. **Obviously correct**: The fix mirrors exactly what
   `_hardware_dequeue()` (line 886-891) and `ep_dequeue()` (line
   1738-1741) already do. It's a well-established pattern within the
   same file.
2. **Fixes a real bug**: DMA corruption and resource leaks on USB
   disconnect during active transfer.
3. **Important issue**: Memory corruption, stale DMA addresses,
   potential data corruption.
4. **Small and contained**: Only 6 lines added, all within a single
   function in a single file.
5. **No new features**: Pure bug fix.

### Risk assessment:
- **Very low risk**: The added code calls well-known cleanup functions
  that are already called in the two other completion paths
  (`_hardware_dequeue` and `ep_dequeue`). The `false` parameter to
  `sglist_do_debounce` is appropriate as no data needs to be copied back
  during shutdown.
- The `usb_gadget_unmap_request_by_dev()` call is safe even if no DMA
  was mapped (it checks `req->length == 0`, `sg_was_mapped`, and
  `num_mapped_sgs` before doing anything).

### Dependency consideration:
- The `sglist_do_debounce()` function was introduced in v6.13 (commit
  edfcc455c85cc). For stable trees older than 6.13, only the
  `usb_gadget_unmap_request_by_dev()` line would apply (the
  `sglist_do_debounce` part would need to be dropped). However, the DMA
  unmap portion is independently valuable.
- The `usb_gadget_unmap_request_by_dev()` function has existed for a
  long time and is available in all active stable trees.

### Reviews/Acks:
- Reviewed-by: Xu Yang (NXP engineer, author of the bounce buffer code)
- Acked-by: Peter Chen (ChipIdea UDC subsystem maintainer)
- Applied by: Greg Kroah-Hartman (USB maintainer)

## Verification

- **Verified** `_hardware_dequeue()` at lines 886-891 performs the same
  cleanup (DMA unmap + sglist debounce) - confirmed by reading the file
- **Verified** `ep_dequeue()` at lines 1738-1741 also performs the same
  cleanup pattern - confirmed by reading the file
- **Verified** `usb_gadget_unmap_request_by_dev()` in
  `drivers/usb/gadget/udc/core.c:956-972` clears `num_mapped_sgs` to 0
  (line 966) and unmaps DMA - confirmed by reading the source
- **Verified** `_ep_nuke()` is called from multiple paths: disconnect
  (line 1108), endpoint reset (lines 1326-1327), endpoint disable (line
  1615) - confirmed by grep
- **Verified** `sglist_do_debounce()` was introduced in commit
  `edfcc455c85cc` which first appeared in v6.13 - confirmed via `git tag
  --contains`
- **Verified** the commit is self-contained: no other patches needed for
  the fix to work in trees with v6.13+
- **Verified** commit has proper review: Reviewed-by (Xu Yang, bounce
  buffer author), Acked-by (Peter Chen, subsystem maintainer)
- **Verified** `_hardware_enqueue()` at line 708 checks `num_mapped_sgs`
  to decide whether to skip DMA mapping - this is the code path that
  breaks when stale state remains

## Conclusion

This is a textbook stable backport candidate:
- Fixes a real, user-triggerable bug (USB disconnect during active
  transfer)
- Causes memory corruption and DMA errors
- Small, surgical fix (6 lines added)
- Mirrors existing cleanup patterns already in the same file
- Reviewed and acked by subsystem experts
- Very low regression risk
- Affects real hardware users (ChipIdea UDC is used in NXP i.MX SoCs,
  common in embedded/IoT)

**YES**

 drivers/usb/chipidea/udc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 64a421ae0f05b..c8d931d9d4330 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -931,6 +931,13 @@ __acquires(hwep->lock)
 		list_del_init(&hwreq->queue);
 		hwreq->req.status = -ESHUTDOWN;
 
+		/* Unmap DMA and clean up bounce buffers before giving back */
+		usb_gadget_unmap_request_by_dev(hwep->ci->dev->parent,
+					&hwreq->req, hwep->dir);
+
+		if (hwreq->sgt.sgl)
+			sglist_do_debounce(hwreq, false);
+
 		if (hwreq->req.complete != NULL) {
 			spin_unlock(hwep->lock);
 			usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH AUTOSEL 6.19-6.12] usb: gadget: f_fs: Fix ioctl error handling
       [not found] <20260219020422.1539798-1-sashal@kernel.org>
  2026-02-19  2:03 ` [PATCH AUTOSEL 6.19-6.12] usb: chipidea: udc: fix DMA and SG cleanup in _ep_nuke() Sasha Levin
@ 2026-02-19  2:03 ` Sasha Levin
  2026-02-19  2:04 ` [PATCH AUTOSEL 6.19-6.12] usb: gadget: f_fs: fix DMA-BUF OUT queues Sasha Levin
  2026-02-19  2:04 ` [PATCH AUTOSEL 6.19-6.1] usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs Sasha Levin
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-02-19  2:03 UTC (permalink / raw)
  To: patches, stable
  Cc: Sam Day, Greg Kroah-Hartman, Sasha Levin, linux-usb, linux-kernel

From: Sam Day <me@samcday.com>

[ Upstream commit 8e4c1d06183c25022f6b0002a5cab84979ca6337 ]

When ffs_epfile_ioctl handles FUNCTIONFS_DMABUF_* ioctls, it's currently
falling through when copy_from_user fails.

However, this fallthrough isn't being checked properly, so the handler
continues executing further than it should. It then tries the secondary
dispatch where it ultimately gives up and returns -ENOTTY.

The end result is invalid ioctl invocations will yield a -ENOTTY rather
than an -EFAULT.

It's a common pattern elsewhere in the kernel code to directly return
-EFAULT when copy_from_user fails. So we update ffs_epfile_ioctl to do
the same and fix this issue.

Signed-off-by: Sam Day <me@samcday.com>
Link: https://patch.msgid.link/20260108-ffs-dmabuf-ioctl-fix-v1-1-e51633891a81@samcday.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Good. So 6.12.y exists and would contain this DMABUF code (introduced in
6.9).

### Stable Criteria Assessment

1. **Obviously correct and tested**: The fix is trivially correct —
   returning `-EFAULT` immediately on `copy_from_user` failure is the
   standard kernel pattern. The commit was accepted by Greg KH (USB
   maintainer and stable tree maintainer).

2. **Fixes a real bug**: Yes — wrong error code returned to userspace
   and unnecessary blocking call on error path. This affects userspace
   programs that depend on correct error codes for their error handling.

3. **Small and contained**: Yes — only 3 error paths changed in a single
   function, net -6 lines. No structural changes.

4. **No new features**: Correct — purely fixes error handling.

5. **Risk assessment**: Extremely low risk. The change only affects
   error paths (when `copy_from_user` fails). The non-error paths
   (successful `copy_from_user`) are completely unchanged.

### Additional Concerns

- **Unnecessary blocking**: The current buggy code causes the function
  to call `ffs_epfile_wait_ep()` when `copy_from_user` fails. This is a
  wait function that could block, causing unnecessary delays for the
  failing ioctl. The fix eliminates this.
- **Dependencies**: No dependencies on other patches needed. The fix is
  self-contained.
- **Affected versions**: Only kernels v6.9+ that contain the DMABUF
  ioctl support.

### Verification

- **git log** confirmed that DMABUF ioctls were introduced in commit
  `7b07a2a7ca02a` (v6.9)
- **Read of f_fs.c lines 1730-1835** confirmed the bug mechanism: after
  `break` from first switch, code falls through to
  `ffs_epfile_wait_ep()` then second switch where default case returns
  `-ENOTTY`, overwriting `-EFAULT`
- **git tag** confirmed the code exists in stable tree 6.12.y (which
  includes all code from v6.9+)
- **Commit message** clearly explains the bug and fix mechanism
- **Signed-off-by Greg KH** confirms the USB maintainer accepted this
  fix
- Could NOT independently verify whether userspace programs have been
  affected (unverified, but the wrong error code is a definite API
  contract violation)

### Summary

This is a clear, small, correct bug fix for error handling in the USB
gadget FunctionFS DMABUF ioctl handler. The bug causes:
1. Wrong error code (`-ENOTTY` instead of `-EFAULT`) returned to
   userspace
2. Unnecessary blocking in `ffs_epfile_wait_ep()` on error paths

The fix is minimal (3 error paths changed to return directly), obviously
correct (follows standard kernel patterns), has zero risk to non-error
paths, and was accepted by Greg KH. It meets all stable kernel criteria.

**YES**

 drivers/usb/gadget/function/f_fs.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 928f51fddc64e..e75d5d8b5ac91 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1744,10 +1744,8 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code,
 	{
 		int fd;
 
-		if (copy_from_user(&fd, (void __user *)value, sizeof(fd))) {
-			ret = -EFAULT;
-			break;
-		}
+		if (copy_from_user(&fd, (void __user *)value, sizeof(fd)))
+			return -EFAULT;
 
 		return ffs_dmabuf_attach(file, fd);
 	}
@@ -1755,10 +1753,8 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code,
 	{
 		int fd;
 
-		if (copy_from_user(&fd, (void __user *)value, sizeof(fd))) {
-			ret = -EFAULT;
-			break;
-		}
+		if (copy_from_user(&fd, (void __user *)value, sizeof(fd)))
+			return -EFAULT;
 
 		return ffs_dmabuf_detach(file, fd);
 	}
@@ -1766,10 +1762,8 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code,
 	{
 		struct usb_ffs_dmabuf_transfer_req req;
 
-		if (copy_from_user(&req, (void __user *)value, sizeof(req))) {
-			ret = -EFAULT;
-			break;
-		}
+		if (copy_from_user(&req, (void __user *)value, sizeof(req)))
+			return -EFAULT;
 
 		return ffs_dmabuf_transfer(file, &req);
 	}
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH AUTOSEL 6.19-6.12] usb: gadget: f_fs: fix DMA-BUF OUT queues
       [not found] <20260219020422.1539798-1-sashal@kernel.org>
  2026-02-19  2:03 ` [PATCH AUTOSEL 6.19-6.12] usb: chipidea: udc: fix DMA and SG cleanup in _ep_nuke() Sasha Levin
  2026-02-19  2:03 ` [PATCH AUTOSEL 6.19-6.12] usb: gadget: f_fs: Fix ioctl error handling Sasha Levin
@ 2026-02-19  2:04 ` Sasha Levin
  2026-02-19  2:04 ` [PATCH AUTOSEL 6.19-6.1] usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs Sasha Levin
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-02-19  2:04 UTC (permalink / raw)
  To: patches, stable
  Cc: Sam Day, David Heidelberg, Greg Kroah-Hartman, Sasha Levin,
	linux-usb, linux-kernel

From: Sam Day <me@samcday.com>

[ Upstream commit 0145e7acd29855dfba4a2f387d455b5d9a520f0e ]

Currently, DMA_FROM_DEVICE is used when attaching DMABUFs to IN
endpoints and DMA_TO_DEVICE for OUT endpoints. This is inverted from
how it should be.

The result is IOMMU read-only mappings placed on OUT queues,
triggering arm-smmu write faults.

Put differently, OUT endpoints flow data from host -> gadget, meaning
the UDC peripheral needs to have write access to the buffer to fill it
with the incoming data.

This commit flips the directions and updates the implicit-sync helpers
so IN endpoints act as readers and OUT endpoints as writers.

Signed-off-by: Sam Day <me@samcday.com>
Tested-by: David Heidelberg <david@ixit.cz>  # OnePlus 6T on sdm845-next-20251119
Link: https://patch.msgid.link/20260108-ffs-dmabuf-ioctl-fix-v1-2-e51633891a81@samcday.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Wait — I see the code at line 1512 still shows the **old** (buggy)
direction. This means the fix commit hasn't been applied yet to this
tree. Let me verify this is the pre-fix state, and confirm the
correctness of the fix.

The key insight about USB gadget directions:

- **IN endpoint** (`epfile->in == true`): Data flows from gadget **to**
  host. The UDC reads from the buffer → `DMA_TO_DEVICE` (device reads
  the memory)
- **OUT endpoint** (`epfile->in == false`): Data flows from host **to**
  gadget. The UDC writes to the buffer → `DMA_FROM_DEVICE` (device
  writes to memory)

The current code has it backwards:
- `epfile->in ? DMA_FROM_DEVICE : DMA_TO_DEVICE` — **WRONG**
- Should be: `epfile->in ? DMA_TO_DEVICE : DMA_FROM_DEVICE` —
  **CORRECT**

This is consistent with the DMA-BUF perspective where DMA direction
refers to the device's perspective relative to memory.

Similarly for the reservation fences:
- IN endpoint (read from buffer to send to host) → the fence should be
  `DMA_RESV_USAGE_READ` (reading from the buffer)
- OUT endpoint (write incoming data to buffer) → the fence should be
  `DMA_RESV_USAGE_WRITE` (writing to the buffer)

The current code has:
- `epfile->in ? DMA_RESV_USAGE_WRITE : DMA_RESV_USAGE_READ` — **WRONG**
- Should be: `epfile->in ? DMA_RESV_USAGE_READ : DMA_RESV_USAGE_WRITE` —
  **CORRECT**

And for the `dma_resv_wait_timeout`:
- `dma_resv_usage_rw(epfile->in)` means: if IN, wait for both
  read+write; if OUT, wait for write only
- Should be `dma_resv_usage_rw(!epfile->in)`: if OUT, wait for both
  read+write; if IN, wait for write only

This logic is also inverted and the fix correctly flips it.

## Summary Assessment

### What problem does this commit solve?

This fixes an inverted DMA direction mapping for DMA-BUF operations in
the USB gadget FunctionFS driver. The bug causes:
1. **IOMMU write faults** (arm-smmu) on OUT endpoints because buffers
   are mapped read-only when they need write access
2. Incorrect implicit-sync fence semantics (readers treated as writers
   and vice versa)

This is a **correctness bug** that makes DMA-BUF OUT transfers
completely non-functional on systems with an IOMMU (which includes most
modern ARM SoCs).

### Does it meet stable kernel rules?

1. **Obviously correct**: Yes — the DMA direction semantics are well-
   defined and the fix is a straightforward inversion of three boolean
   conditions
2. **Fixes a real bug**: Yes — DMA-BUF OUT transfers cause IOMMU faults
   on ARM platforms
3. **Important issue**: Yes — hardware fault/crash-level issue that
   prevents a feature from working
4. **Small and contained**: Yes — 3 lines changed in a single file
5. **No new features**: Correct — pure bug fix
6. **Tested**: Yes — "Tested-by: David Heidelberg" on OnePlus 6T
   (sdm845)

### Risk vs Benefit

- **Risk**: Very low — the change is mechanical (flip three booleans),
  touches only DMA-BUF code paths in f_fs, and cannot affect non-DMA-BUF
  USB operations
- **Benefit**: High — fixes broken DMA-BUF OUT transfers that cause
  IOMMU faults

### Concerns

- **Affected versions**: The DMA-BUF support was introduced in commit
  `7b07a2a7ca02a` during the v6.9 development cycle. Only stable trees
  from v6.9 onward need this fix (the code doesn't exist in older
  kernels).
- **Dependencies**: None — the fix is self-contained and touches only
  the three lines with the bug
- **Backport risk**: Clean application expected since the surrounding
  code has not changed significantly

### Verification

- `git log -S "ffs_dmabuf_attach" master` confirmed the DMA-BUF
  interface was introduced in commit `7b07a2a7ca02a` (v6.9 cycle,
  2024-01-30)
- `git merge-base --is-ancestor 7b07a2a7ca02a master` confirmed the
  buggy code is in master
- Read of `f_fs.c` at lines 1490-1549 verified the current (pre-fix)
  code shows the inverted `DMA_FROM_DEVICE`/`DMA_TO_DEVICE` direction
- Grep for `epfile->in` confirmed three locations needing the fix (lines
  1512, 1642, 1687) matching the patch
- Line 2442 shows `epfile->in = usb_endpoint_dir_in(ep->ep->desc)`
  confirming `epfile->in` is true for IN endpoints (data to host)
- USB DMA direction convention verified: IN endpoint = device reads
  buffer = `DMA_TO_DEVICE`; OUT endpoint = device writes buffer =
  `DMA_FROM_DEVICE`. The original code has these inverted.
- `git tag --contains 7b07a2a7ca02a` shows the commit is in `p-6.12`,
  `p-6.15`, `p-6.16` branches, confirming the buggy code exists in
  multiple stable trees
- Commit has "Tested-by: David Heidelberg" and was merged by Greg Kroah-
  Hartman (USB maintainer), indicating quality review

This is a clear, small, well-tested fix for a real hardware fault that
makes DMA-BUF OUT transfers completely broken on IOMMU-enabled
platforms. It meets all stable kernel criteria.

**YES**

 drivers/usb/gadget/function/f_fs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index fa467a40949d2..928f51fddc64e 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1509,7 +1509,7 @@ static int ffs_dmabuf_attach(struct file *file, int fd)
 		goto err_dmabuf_detach;
 	}
 
-	dir = epfile->in ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	dir = epfile->in ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 
 	err = ffs_dma_resv_lock(dmabuf, nonblock);
 	if (err)
@@ -1639,7 +1639,7 @@ static int ffs_dmabuf_transfer(struct file *file,
 	/* Make sure we don't have writers */
 	timeout = nonblock ? 0 : msecs_to_jiffies(DMABUF_ENQUEUE_TIMEOUT_MS);
 	retl = dma_resv_wait_timeout(dmabuf->resv,
-				     dma_resv_usage_rw(epfile->in),
+				     dma_resv_usage_rw(!epfile->in),
 				     true, timeout);
 	if (retl == 0)
 		retl = -EBUSY;
@@ -1684,7 +1684,7 @@ static int ffs_dmabuf_transfer(struct file *file,
 	dma_fence_init(&fence->base, &ffs_dmabuf_fence_ops,
 		       &priv->lock, priv->context, seqno);
 
-	resv_dir = epfile->in ? DMA_RESV_USAGE_WRITE : DMA_RESV_USAGE_READ;
+	resv_dir = epfile->in ? DMA_RESV_USAGE_READ : DMA_RESV_USAGE_WRITE;
 
 	dma_resv_add_fence(dmabuf->resv, &fence->base, resv_dir);
 	dma_resv_unlock(dmabuf->resv);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH AUTOSEL 6.19-6.1] usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs
       [not found] <20260219020422.1539798-1-sashal@kernel.org>
                   ` (2 preceding siblings ...)
  2026-02-19  2:04 ` [PATCH AUTOSEL 6.19-6.12] usb: gadget: f_fs: fix DMA-BUF OUT queues Sasha Levin
@ 2026-02-19  2:04 ` Sasha Levin
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-02-19  2:04 UTC (permalink / raw)
  To: patches, stable
  Cc: Benson Leung, Heikki Krogerus, Greg Kroah-Hartman, Sasha Levin,
	linux-usb, linux-kernel

From: Benson Leung <bleung@chromium.org>

[ Upstream commit 6811e0a08bdce6b2767414caf17fda24c2e4e032 ]

ucsi_psy_get_voltage_max and ucsi_psy_get_current_max are calculated
using whichever pdo is in the last position of the src_pdos array, presuming
it to be a fixed pdo, so the pdo_fixed_voltage or pdo_max_current
helpers are used on that last pdo.

However, non-Fixed PDOs such as Battery PDOs, Augmented PDOs (used for AVS and
for PPS) may exist, and are always at the end of the array if they do.
In the event one of these more advanced chargers are attached the helpers for
fixed return mangled values.

Here's an example case of a Google Pixel Flex Dual Port 67W USB-C Fast Charger
with PPS support:
POWER_SUPPLY_NAME=ucsi-source-psy-cros_ec_ucsi.4.auto2
POWER_SUPPLY_TYPE=USB
POWER_SUPPLY_CHARGE_TYPE=Standard
POWER_SUPPLY_USB_TYPE=C [PD] PD_PPS PD_DRP
POWER_SUPPLY_ONLINE=1
POWER_SUPPLY_VOLTAGE_MIN=5000000
POWER_SUPPLY_VOLTAGE_MAX=13400000
POWER_SUPPLY_VOLTAGE_NOW=20000000
POWER_SUPPLY_CURRENT_MAX=5790000
POWER_SUPPLY_CURRENT_NOW=3250000

Voltage Max is reading as 13.4V, but that's an incorrect decode of the PPS
APDO in the last position. Same goes for CURRENT_MAX. 5.79A is incorrect.

Instead, enumerate through the src_pdos and filter just for Fixed PDOs for
now, and find the one with the highest voltage and current respectively.

After, from the same charger:
POWER_SUPPLY_NAME=ucsi-source-psy-cros_ec_ucsi.4.auto2
POWER_SUPPLY_TYPE=USB
POWER_SUPPLY_CHARGE_TYPE=Standard
POWER_SUPPLY_USB_TYPE=C [PD] PD_PPS PD_DRP
POWER_SUPPLY_ONLINE=1
POWER_SUPPLY_VOLTAGE_MIN=5000000
POWER_SUPPLY_VOLTAGE_MAX=20000000
POWER_SUPPLY_VOLTAGE_NOW=20000000
POWER_SUPPLY_CURRENT_MAX=4000000
POWER_SUPPLY_CURRENT_NOW=3250000

Signed-off-by: Benson Leung <bleung@chromium.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Link: https://patch.msgid.link/20251208174918.289394-3-bleung@chromium.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

This confirms the bug mechanism: `pdo_fixed_voltage()` and
`pdo_max_current()` extract specific bit fields from a PDO. When a non-
Fixed PDO (like an APDO for PPS) is passed to these functions, the bit
fields have completely different meanings, leading to garbage values
being reported.

### Bug Assessment

**What the bug is:** When a USB PD charger advertises non-Fixed PDOs
(Battery, PPS/AVS APDOs), the UCSI power supply driver incorrectly
applies Fixed PDO bit-extraction helpers to these non-Fixed PDOs,
resulting in wrong voltage_max and current_max values reported to
userspace.

**Who is affected:** Any USB Type-C user connected to a PPS or AVS
charger. This is increasingly common with modern fast chargers (the
example is a Google Pixel charger). The incorrect values are exposed via
the power supply sysfs interface and could confuse userspace power
management tools or battery monitoring.

**The fix:** Instead of blindly using the last PDO, iterate through all
PDOs and only consider Fixed type PDOs, finding the maximum voltage and
current among them. This is correct because Fixed PDOs are always
present per the USB PD spec and represent the baseline capabilities.

### Risk Assessment

**Low risk:**
- The change is small and contained (two functions modified in one file)
- The logic is straightforward: loop over PDOs, filter by type, find max
- It only changes behavior for the PD case and only when non-Fixed PDOs
  are present
- Reviewed by Heikki Krogerus (the UCSI maintainer)
- Committed by Greg KH (USB maintainer)

**Correctness considerations:**
- The new code correctly handles `num_pdos == 0`
  (max_voltage/max_current stays 0)
- The type check `pdo_type(pdo) == PDO_TYPE_FIXED` correctly filters out
  Battery, Variable, and APDO types
- Using max() logic to find the highest fixed voltage/current is correct
  per USB PD spec where fixed PDOs increase in voltage

### Stable Kernel Criteria

1. **Obviously correct and tested:** Yes - clear logic, tested with real
   hardware (Google Pixel charger), reviewed by subsystem maintainer
2. **Fixes a real bug:** Yes - incorrect power supply values reported to
   userspace
3. **Important issue:** Moderate - incorrect power data can confuse
   userspace charging tools; not a crash but a data correctness issue
   affecting real hardware
4. **Small and contained:** Yes - about 30 lines changed in a single
   file
5. **No new features:** Correct - this only fixes the calculation logic
6. **Applies cleanly:** Likely needs checking against specific stable
   trees, but the file has been stable

### Verification

- `git log --follow --diff-filter=A -- drivers/usb/typec/ucsi/psy.c`
  confirmed the file was added in commit 992a60ed0d5e3 (v5.8 timeframe),
  present in all active stable trees
- Read `include/linux/usb/pd.h:337-355` confirmed `pdo_fixed_voltage()`
  and `pdo_max_current()` extract Fixed PDO-specific bit fields that
  produce garbage for non-Fixed PDO types
- Read the current `psy.c` and confirmed the buggy code at lines 118-123
  (voltage_max) and 179-184 (current_max) uses
  `con->src_pdos[con->num_pdos - 1]` which could be a non-Fixed PDO
- The `pdo_type()` function at line 332 of pd.h confirms the fix's type-
  checking approach is correct
- Reviewed-by from Heikki Krogerus (UCSI maintainer) and committed by
  Greg KH (USB maintainer) verified in commit message
- `git log` on the file shows 11 prior commits, indicating a mature and
  actively maintained file

**YES**

 drivers/usb/typec/ucsi/psy.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c
index 3abe9370ffaaf..62160c4191718 100644
--- a/drivers/usb/typec/ucsi/psy.c
+++ b/drivers/usb/typec/ucsi/psy.c
@@ -112,15 +112,20 @@ static int ucsi_psy_get_voltage_max(struct ucsi_connector *con,
 				    union power_supply_propval *val)
 {
 	u32 pdo;
+	int max_voltage = 0;
 
 	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];
-			val->intval = pdo_fixed_voltage(pdo) * 1000;
-		} else {
-			val->intval = 0;
+		for (int i = 0; i < con->num_pdos; i++) {
+			int pdo_voltage = 0;
+
+			pdo = con->src_pdos[i];
+			if (pdo_type(pdo) == PDO_TYPE_FIXED)
+				pdo_voltage = pdo_fixed_voltage(pdo) * 1000;
+			max_voltage = (pdo_voltage > max_voltage) ? pdo_voltage
+								  : max_voltage;
 		}
+		val->intval = max_voltage;
 		break;
 	case UCSI_CONSTAT_PWR_OPMODE_TYPEC3_0:
 	case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5:
@@ -168,6 +173,7 @@ static int ucsi_psy_get_current_max(struct ucsi_connector *con,
 				    union power_supply_propval *val)
 {
 	u32 pdo;
+	int max_current = 0;
 
 	if (!UCSI_CONSTAT(con, CONNECTED)) {
 		val->intval = 0;
@@ -176,12 +182,16 @@ static int ucsi_psy_get_current_max(struct ucsi_connector *con,
 
 	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];
-			val->intval = pdo_max_current(pdo) * 1000;
-		} else {
-			val->intval = 0;
+		for (int i = 0; i < con->num_pdos; i++) {
+			int pdo_current = 0;
+
+			pdo = con->src_pdos[i];
+			if (pdo_type(pdo) == PDO_TYPE_FIXED)
+				pdo_current = pdo_max_current(pdo) * 1000;
+			max_current = (pdo_current > max_current) ? pdo_current
+								  : max_current;
 		}
+		val->intval = max_current;
 		break;
 	case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5:
 		val->intval = UCSI_TYPEC_1_5_CURRENT * 1000;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-02-19  2:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260219020422.1539798-1-sashal@kernel.org>
2026-02-19  2:03 ` [PATCH AUTOSEL 6.19-6.12] usb: chipidea: udc: fix DMA and SG cleanup in _ep_nuke() Sasha Levin
2026-02-19  2:03 ` [PATCH AUTOSEL 6.19-6.12] usb: gadget: f_fs: Fix ioctl error handling Sasha Levin
2026-02-19  2:04 ` [PATCH AUTOSEL 6.19-6.12] usb: gadget: f_fs: fix DMA-BUF OUT queues Sasha Levin
2026-02-19  2:04 ` [PATCH AUTOSEL 6.19-6.1] usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox