* Re: [RFC PATCH 1/2] xhci: prevent automatic endpoint restart after stall or error
From: Mathias Nyman @ 2026-03-30 12:51 UTC (permalink / raw)
To: Thinh Nguyen
Cc: linux-usb@vger.kernel.org, stern@rowland.harvard.edu,
michal.pecio@gmail.com, oneukum@suse.com,
niklas.neronin@linux.intel.com
In-Reply-To: <20260326232400.zkplsxflhykhayyb@synopsys.com>
On 3/27/26 01:24, Thinh Nguyen wrote:
> On Thu, Mar 26, 2026, Mathias Nyman wrote:
>> On 3/26/26 03:19, Thinh Nguyen wrote:
>>> On Wed, Mar 25, 2026, Mathias Nyman wrote:
>>>> On 3/25/26 03:52, Thinh Nguyen wrote:
>>>>> On Mon, Mar 23, 2026, Mathias Nyman wrote:
>>>>>> Avoid automatically restarting bulk or interrupt transfers after a
>>>>>> URB is given back due to stall or error.
>>>>>>
>>>>>> Introduce a 'TD_TAINTED' state for pending TDs queued on a endpoint when
>>>>>> it halted. The actual TD the endpoint halted on is marked TD_HALTED,
>>>>>> and its URB is given back with proper EPROTO or EPIPE error code.
>>>>>>
>>>>>> Don't automatically restart an endpoint if the next queued TD after
>>>>>> the TD_HALTED one is marked tainted.
>>>>>
>>>>> Sounds good for -EPROTO, but will a clear-halt ring the corresponding
>>>>> the endpoint's doorbell for STALL endpoint?
>>>>>
>>>>
>>>> With this change xhci would not restart the stalled endpoint after a clear-halt
>>>> request. The first usb_enqueue() call after clear-halt would start it.
>>>>
>>>> Could make sense to restart the endpoint after a clear-halt, and just add a small
>>>> debug message if the next queued URB is marked 'tainted'.
>>>>
>>>
>>> The -EPROTO should be handled differently than -EPIPE. A STALL endpoint
>>> is part of a normal usb flow. Should the class driver submit a new URB
>>> while the endpoint is STALL, we would always expect a STALL error
>>> response after the endpoint is restarted. That's not the case with
>>> -EPROTO where the data may be corrupted and/or the host and device are
>>> out of sync. We should not continue until the class driver do some
>>> recovery. IMHO, we can keep the handling of -EPIPE how it was before.
>>> Let the xHC tell whether the STALL error still persists instead of
>>> managing it by the xhci driver.
>>>
>> I agree that that we should restart the endpoint if class/core enqueues a new
>> URB _after_ xhci gave back an URB with EPIPE after endpoint STALL.
>>
>> But I don't think we should restart the ring to continue processing URBs that
>> were queued before the endpoint stalled. This would prevent the class/core
>> from even attempting to retire the pending URBs, something USB2.0 spec,
>> '5.8.5 Bulk Transfer Data Sequences' requires:
>>
>> "If a halt condition is detected on a bulk pipe due to transmission errors or
>> a STALL handshake being returned from the endpoint, all pending IRPs are
>> retired. Removal of the halt condition is achieved via software intervention
>> through a separate control pipe."
>>
>
> Fair point. Then the core will need to track the endpoint's STALL state
> and parse the clear-halt request to know which endpoint and when to
> clear the STALL before it can accept new URB. So the first usb_enqueue()
> call after clear-halt can start the endpoint again. The xhci will also
> need to have access to this state.
Ideally xhci driver would return the URB with EPIPE after STALL, and not continue
processing URBs before a clear halt is sent, or a new URB is enqueued.
USB core would hold off submitting URBs to xhci, buffering URBs enqueued for this
STALLED endpoint until class driver urb->complete() finishes for that EPIPE URB.
Usb core could flag this endpoint as "halt_pending" before adding the EPIPE URB to
the bh workqueue. Then after urb->complete() work is called and core is sure class
driver is aware of the EPIPE, then core would clear the flag and flush the buffered
URBs to the host controller drivers, restarting the ring
Class driver urb->complete() would most likely retire/cancel the pending URBs, both the
earlier queued 'tainted' URBs, and the most recent 'buffered' URBs in usb core.
Class driver should clear the halt, but is free to do whatever it wants.
It could choose to send a new URB without clearing the halt,
have it processed, trigger STALL again, and get URB returned with EPIPE status.
I don't think most class/usb device drivers really handle stall that well.
Above might be wishful thinking.
>
> Currently you have the xhci driver to "retire" the halted URBs. However,
> you also noted that class/core may attempt to retire the pending URBs.
> Who's expected to handle the retirement here?
Maybe we should let core retire the pending URBS, and only fix the xhci driver
'automatic endpoint restart after stall' part after that core change is done.
Should cause less regression.
>
> On a separate note, will you plan to implement the clear-halt for EPROTO
> in xhci?
I don't think this should be part of xhci driver. Decision to send control requests
to the device should be done by core or class drivers.
Thanks
Mathias
^ permalink raw reply
* Re: [PATCH] USB: cdc-acm: Add support for second ACM channel on more Nokia phones
From: gregkh @ 2026-03-30 13:13 UTC (permalink / raw)
To: Рыжов Фёдор
Cc: linux-kernel@vger.kernel.org, Oliver Neukum,
linux-usb@vger.kernel.org
In-Reply-To: <a8008c91-52c1-4a5c-ac96-dd45f109c68c@outlook.com>
On Thu, Mar 26, 2026 at 02:37:41PM +0000, Рыжов Фёдор wrote:
> On Nokia Series60 phones, second ACM channel is used by phone
> applications to communicate with PC. It reports "vendor-specific
> protocol" so kernel does not expose it as tty by default.
>
> This patch extends the whitelist with all S60v3.x and S60v5.x phones,
> supported by official "Nokia PC Suite" driver for Windows. Device IDs
> are taken from its installation configuration file. Older S60v2.x and
> S60v1.x devices were not added because I have no hardware to test.
>
> Signed-off-by: Ansel Hayashi <symansel@outlook.com>
Does not match the From: line :(
^ permalink raw reply
* Re: [PATCH v5 1/4] usb: port: track the disabled state
From: Greg Kroah-Hartman @ 2026-03-30 13:15 UTC (permalink / raw)
To: Marco Felsch
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Fabio Estevam,
Matthias Kaehlcke, Liam Girdwood, Mark Brown, linux-usb,
linux-kernel, devicetree, kernel
In-Reply-To: <mavc4ului42mdozzfwxd6zfa5b664umlqe5dal2r6knnilbi5a@ubnxauwhnoue>
On Fri, Mar 20, 2026 at 11:16:43PM +0100, Marco Felsch wrote:
> On 26-03-11, Greg Kroah-Hartman wrote:
> > On Mon, Feb 23, 2026 at 12:27:34PM +0100, Marco Felsch wrote:
> > > The disable state isn't tracked at the moment, instead the state is
> > > directly passed to the hub driver. Change this behavior to only trigger
> > > the hub if a state change happened. Exit early in case of no state
> > > changes but don't return an error.
> > >
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>
> ...
>
> > > #define to_usb_port(_dev) \
> > > diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> > > index 44e38f922bc553adee64b35c536dfd4154a42d8a..86e9d6d0c0f505782569565fde8e4a46b06b8b4d 100644
> > > --- a/drivers/usb/core/port.c
> > > +++ b/drivers/usb/core/port.c
> > > @@ -117,6 +117,10 @@ static ssize_t disable_store(struct device *dev, struct device_attribute *attr,
> > > if (rc)
> > > return rc;
> > >
> > > + /* Early quit if no change was detected */
> > > + if (port_dev->disabled == disabled)
> > > + return count;
> > > +
> >
> > This will change behavior where someone tells the port to be enabled
> > again, when it already is. Is that ok?
>
> That's the whole purpose of this patch. Can you please elaborate why
> someone wants to enable or disbale a port more than once in a row?
I have given up trying to understand why users do what users do :)
^ permalink raw reply
* Re: [PATCH 0/3] usb: typec: ucsi: Add support for SET_PDOS command
From: Greg KH @ 2026-03-30 13:17 UTC (permalink / raw)
To: Pooja Katiyar
Cc: linux-usb, heikki.krogerus, dmitry.baryshkov, johan,
asutosh.pathak
In-Reply-To: <cover.1773090896.git.pooja.katiyar@intel.com>
On Wed, Mar 18, 2026 at 03:25:28PM -0700, Pooja Katiyar wrote:
> This series implements support for UCSI SET PDOS command. It provides
> interface to send message out data and update source or sink
> capabilities PDOs on a connector over debugfs interface.
>
> Pooja Katiyar (3):
> usb: typec: ucsi: Add support for message_out data structure
> usb: typec: ucsi: Enable debugfs for message_out data structure
> usb: typec: ucsi: Add support for SET_PDOS command
>
> drivers/usb/typec/ucsi/cros_ec_ucsi.c | 6 ++-
> drivers/usb/typec/ucsi/debugfs.c | 31 +++++++++++++++
> drivers/usb/typec/ucsi/ucsi.c | 52 ++++++++++++++++++++-----
> drivers/usb/typec/ucsi/ucsi.h | 20 +++++++++-
> drivers/usb/typec/ucsi/ucsi_acpi.c | 22 ++++++++++-
> drivers/usb/typec/ucsi/ucsi_ccg.c | 6 ++-
> drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 6 ++-
> 7 files changed, 123 insertions(+), 20 deletions(-)
>
> --
> 2.43.0
>
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/process/submitting-patches.rst for what
needs to be done here to properly describe this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
^ permalink raw reply
* Re: [PATCH v6 phy-next 03/28] usb: add missing headers transitively included by <linux/phy/phy.h>
From: Greg Kroah-Hartman @ 2026-03-30 13:19 UTC (permalink / raw)
To: Vladimir Oltean
Cc: linux-phy, Vinod Koul, Neil Armstrong, dri-devel, freedreno,
linux-arm-kernel, linux-arm-msm, linux-can, linux-gpio, linux-ide,
linux-kernel, linux-media, linux-pci, linux-renesas-soc,
linux-riscv, linux-rockchip, linux-samsung-soc, linux-scsi,
linux-sunxi, linux-tegra, linux-usb, netdev, spacemit,
UNGLinuxDriver, Thinh Nguyen, Peter Chen, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
In-Reply-To: <20260327184706.1600329-4-vladimir.oltean@nxp.com>
On Fri, Mar 27, 2026 at 08:46:41PM +0200, Vladimir Oltean wrote:
> The chipidea ci_hdrc_imx driver uses regulator consumer API like
> regulator_enable() but does not include <linux/regulator/consumer.h>.
>
> The core USB HCD driver calls invalidate_kernel_vmap_range() and
> flush_kernel_vmap_range(), but does not include <linux/highmem.h>.
>
> The DWC3 gadget driver calls:
> - device_property_present()
> - device_property_count_u8()
> - device_property_read_u8_array()
> but does not include <linux/property.h>
>
> The dwc3-generic-plat driver uses of_device_get_match_data() but does
> not include <linux/of.h>.
>
> In all these cases, the necessary includes were still provided somehow,
> directly or indirectly, through <linux/phy/phy.h>. The latter header
> wants to drop those includes, so fill in the required headers to avoid
> any breakage.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> # dwc3
> ---
> Cc: Peter Chen <peter.chen@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Frank Li <Frank.Li@nxp.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>
> v2->v6: none
> v1->v2: collect tag
> ---
> drivers/usb/chipidea/ci_hdrc_imx.c | 1 +
> drivers/usb/core/hcd.c | 1 +
> drivers/usb/dwc3/dwc3-generic-plat.c | 1 +
> drivers/usb/dwc3/gadget.c | 1 +
> 4 files changed, 4 insertions(+)
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: [PATCH] usb: cdns3: gadget: fix NULL pointer dereference in ep_queue
From: Sergey Shtylyov @ 2026-03-30 13:30 UTC (permalink / raw)
To: Peter Chen (CIX), Yongchao Wu
Cc: Pawel Laszczak, Roger Quadros, Greg Kroah-Hartman, linux-usb,
linux-kernel
In-Reply-To: <acpAmimXvxD+WXA1@nchen-desktop>
On 3/30/26 12:21 PM, Peter Chen (CIX) wrote:
[...]
>> When the gadget endpoint is disabled or not yet configured, the ep->desc
>> pointer can be NULL. This leads to a NULL pointer dereference when
>> __cdns3_gadget_ep_queue() is called, causing a kernel crash.
>>
>> Add a check to return -ESHUTDOWN if ep->desc is NULL, which is the
>> standard return code for unconfigured endpoints.
>>
>> This prevents potential crashes when ep_queue is called on endpoints
>> that are not ready.
>>
>> Signed-off-by: Yongchao Wu <yongchao.wu@autochips.com>
>
> Add Fixed-by tag and Cc to stable tree please, others:
I think you meant the Fixes tag. :-)
[...]
MBR, Sergey
^ permalink raw reply
* [PATCH net-next 0/3] Add OATC10 Sleep Wake-up support and LAN867x Rev.D0 handling
From: Parthiban Veerasooran @ 2026-03-30 13:42 UTC (permalink / raw)
To: parthiban.veerasooran, piergiorgio.beruto, andrew, hkallweit1,
linux, davem, edumazet, kuba, pabeni, steve.glendinning,
UNGLinuxDriver
Cc: netdev, linux-usb
This patch series adds support for 10BASE‑T1S Open Alliance TC10 (OATC10)
Sleep/Wake‑up to the generic Clause 45 PHY layer and integrates it with
the Microchip LAN867x Rev.D0 T1S PHY driver. It also ensures that PHY
suspend is properly invoked from the smsc95xx USB Ethernet driver so that
low‑power states are entered during system suspend.
Patch Summary:
1. add generic OATC10 suspend/resume helpers
- Add TC10 Sleep/Wake‑up MDIO register definitions
- Implement genphy_c45_oatc10_suspend() and genphy_c45_oatc10_resume()
- Cache PLCA configuration in struct phy_device for restore after wake
2. add LAN867x Rev.D0 TC10 suspend and Wake‑on‑PHY support
- Enable TC10 low‑power entry via generic helpers
- Restore configuration correctly on resume
- Add ethtool WoL support using PHY wake‑up pulse (WUP)
- Mark driver with PHY_ALWAYS_CALL_SUSPEND
3. suspend attached PHY on system suspend
- Call phy_suspend() before USB suspend
- Allows TC10‑capable PHYs to enter low‑power state
Testing:
Tested on EVB-LAN8670-USB Rev.D0 with suspend/resume cycles and verified:
- PLCA configuration is preserved across low-power transitions.
- Wake-up Pulse triggers proper resume.
- ethtool WOL operations function as expected.
Reference:
Open Alliance TC10 10BASE-T1S Sleep/Wake-up Specification:
https://opensig.org/wp-content/uploads/2024/01/TC14_TC10_JWG_10BASE-T1S-Sleep-Wake-up-Specification_1.0_final.pdf
Parthiban Veerasooran (3):
net: phy: phy-c45: add OATC10 Sleep/Wakeup support in 10BASE-T1S PHYs
net: phy: microchip_t1s: add suspend and WOL support for LAN867x
Rev.D0
net: usb: smsc95xx: suspend PHY during USB suspend
drivers/net/phy/mdio-open-alliance.h | 13 ++++
drivers/net/phy/microchip_t1s.c | 67 ++++++++++++++++++++-
drivers/net/phy/phy-c45.c | 82 ++++++++++++++++++++++++++
drivers/net/usb/smsc95xx.c | 6 ++
include/linux/phy.h | 88 +++++++++++++++-------------
5 files changed, 211 insertions(+), 45 deletions(-)
base-commit: cf0d9080c6f795bc6be08babbffa29b62c06e9b0
--
2.34.1
^ permalink raw reply
* [PATCH net-next 1/3] net: phy: phy-c45: add OATC10 Sleep/Wakeup support in 10BASE-T1S PHYs
From: Parthiban Veerasooran @ 2026-03-30 13:42 UTC (permalink / raw)
To: parthiban.veerasooran, piergiorgio.beruto, andrew, hkallweit1,
linux, davem, edumazet, kuba, pabeni, steve.glendinning,
UNGLinuxDriver
Cc: netdev, linux-usb
In-Reply-To: <20260330134222.67597-1-parthiban.veerasooran@microchip.com>
Add generic Clause 45 helpers to support the Open Alliance TC10
10BASE-T1S Sleep/Wake-up specification. This patch introduces
register definitions for the TC10 sleep/wake status and control
registers and adds generic suspend/resume helpers for
OATC10-compliant PHYs.
The new genphy_c45_oatc10_suspend() helper verifies low-power
capability, caches the current PLCA configuration, and requests entry
into the low-power sleep state. Since all PHY configuration is lost
while sleeping, the PLCA configuration is stored in the phy_device
structure for restoration after wake-up.
The corresponding genphy_c45_oatc10_resume() helper reinitializes the
PHY after wake-up and restores the cached PLCA configuration using
driver callbacks.
Additionally, the PLCA configuration structure is moved into
struct phy_device to allow persistent storage across suspend/resume
cycles.
These helpers allow PHY drivers for TC10 10BASE-T1S devices to
implement sleep and wake-up handling in a consistent way without
duplicating common logic.
Open Alliance TC10 10BASE-T1S Sleep/Wake-up Specification ref:
https://opensig.org/wp-content/uploads/2024/01/TC14_TC10_JWG_10BASE-T1S-Sleep-Wake-up-Specification_1.0_final.pdf
Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
---
drivers/net/phy/mdio-open-alliance.h | 13 ++++
drivers/net/phy/phy-c45.c | 82 ++++++++++++++++++++++++++
include/linux/phy.h | 88 +++++++++++++++-------------
3 files changed, 141 insertions(+), 42 deletions(-)
diff --git a/drivers/net/phy/mdio-open-alliance.h b/drivers/net/phy/mdio-open-alliance.h
index 449d0fb67093..62946be9fb78 100644
--- a/drivers/net/phy/mdio-open-alliance.h
+++ b/drivers/net/phy/mdio-open-alliance.h
@@ -78,6 +78,19 @@
/* SQI is supported using 3 bits means 8 levels (0-7) */
#define OATC14_SQI_MAX_LEVEL 7
+/* Open Alliance 10BASE-T1S Sleep/Wake-up Registers
+ * Specification:
+ * "10BASE-T1S Sleep/Wake-up Specification"
+ * https://opensig.org/wp-content/uploads/2024/01/TC14_TC10_JWG_10BASE-T1S-Sleep-Wake-up-Specification_1.0_final.pdf
+ */
+/* Sleep/Wake-up Status Register */
+#define MDIO_OATC10_WS_STATUS 0xd000
+#define OATC10_WS_STATUS_LPCAP BIT(15) /* PM client capability */
+
+/* Sleep/Wake-up Control Register */
+#define MDIO_OATC10_WS_CONTROL 0xd001
+#define OATC10_WS_CONTROL_LPREQ BIT(15) /* Request low power */
+
/* Bus Short/Open Status:
* 0 0 - no fault; everything is ok. (Default)
* 0 1 - detected as an open or missing termination(s)
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index d48aa7231b37..627eaae9e60f 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -1832,3 +1832,85 @@ int genphy_c45_oatc14_get_sqi(struct phy_device *phydev)
return ret & OATC14_DCQ_SQI_VALUE;
}
EXPORT_SYMBOL(genphy_c45_oatc14_get_sqi);
+
+/**
+ * genphy_c45_oatc10_suspend - Suspend OATC10 PHY into low power state
+ * @phydev: PHY device to suspend
+ *
+ * Puts an OATC10 PHY into low power sleep state.
+ *
+ * The function performs the following steps:
+ * 1. Verify low power capability is supported
+ * 2. Cache current PLCA configuration for restoration on wake
+ * 3. Set the low power request bit to enter sleep state
+ *
+ * Return:
+ * * 0 on successful entry to low power state
+ * * -EOPNOTSUPP if PHY doesn't support low power capability
+ * * Negative error code on register read/write failures
+ */
+int genphy_c45_oatc10_suspend(struct phy_device *phydev)
+{
+ int ret;
+
+ /* Check for Low Power capability */
+ ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC10_WS_STATUS);
+ if (ret < 0)
+ return ret;
+
+ if (!(ret & OATC10_WS_STATUS_LPCAP))
+ return -EOPNOTSUPP;
+
+ /* Cache PLCA settings for later use. These values must be restored when
+ * the PHY wakes up from the low-power sleep state, as all configured
+ * settings are lost.
+ */
+ ret = genphy_c45_plca_get_cfg(phydev, &phydev->plca_cfg);
+ if (ret)
+ return ret;
+
+ phydev->plca_cfg.version = -1;
+
+ if (phydev->state == PHY_UP)
+ /* Put the PHY into low power sleep state */
+ return phy_set_bits_mmd(phydev, MDIO_MMD_VEND2,
+ MDIO_OATC10_WS_CONTROL,
+ OATC10_WS_CONTROL_LPREQ);
+
+ return 0;
+}
+EXPORT_SYMBOL(genphy_c45_oatc10_suspend);
+
+/**
+ * genphy_c45_oatc10_resume - Resume OATC10 PHY from low-power sleep state
+ * @phydev: PHY device to resume
+ *
+ * Resume a PHY from suspend state. When the PHY wakes up from the low-power
+ * sleep state, all configured settings are lost. This function reinitializes
+ * the PHY configuration settings.
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+int genphy_c45_oatc10_resume(struct phy_device *phydev)
+{
+ int ret;
+
+ if (!phydev->suspended)
+ return 0;
+
+ /* When the PHY wakes up from the low-power sleep state, it needs to be
+ * reinitialized as all configured settings are lost.
+ */
+ if (phydev->drv->config_init) {
+ ret = phydev->drv->config_init(phydev);
+ if (ret)
+ return ret;
+ }
+
+ /* Reconfigure the PHY with cached PLCA settings */
+ if (phydev->drv->set_plca_cfg)
+ return phydev->drv->set_plca_cfg(phydev, &phydev->plca_cfg);
+
+ return 0;
+}
+EXPORT_SYMBOL(genphy_c45_oatc10_resume);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5de4b172cd0b..9bee520ac421 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -555,6 +555,48 @@ struct phy_oatc14_sqi_capability {
u8 sqiplus_bits;
};
+/**
+ * struct phy_plca_cfg - Configuration of the PLCA (Physical Layer Collision
+ * Avoidance) Reconciliation Sublayer.
+ *
+ * @version: read-only PLCA register map version. -1 = not available. Ignored
+ * when setting the configuration. Format is the same as reported by the PLCA
+ * IDVER register (31.CA00). -1 = not available.
+ * @enabled: PLCA configured mode (enabled/disabled). -1 = not available / don't
+ * set. 0 = disabled, anything else = enabled.
+ * @node_id: the PLCA local node identifier. -1 = not available / don't set.
+ * Allowed values [0 .. 254]. 255 = node disabled.
+ * @node_cnt: the PLCA node count (maximum number of nodes having a TO). Only
+ * meaningful for the coordinator (node_id = 0). -1 = not available / don't
+ * set. Allowed values [1 .. 255].
+ * @to_tmr: The value of the PLCA to_timer in bit-times, which determines the
+ * PLCA transmit opportunity window opening. See IEEE802.3 Clause 148 for
+ * more details. The to_timer shall be set equal over all nodes.
+ * -1 = not available / don't set. Allowed values [0 .. 255].
+ * @burst_cnt: controls how many additional frames a node is allowed to send in
+ * single transmit opportunity (TO). The default value of 0 means that the
+ * node is allowed exactly one frame per TO. A value of 1 allows two frames
+ * per TO, and so on. -1 = not available / don't set.
+ * Allowed values [0 .. 255].
+ * @burst_tmr: controls how many bit times to wait for the MAC to send a new
+ * frame before interrupting the burst. This value should be set to a value
+ * greater than the MAC inter-packet gap (which is typically 96 bits).
+ * -1 = not available / don't set. Allowed values [0 .. 255].
+ *
+ * A structure containing configuration parameters for setting/getting the PLCA
+ * RS configuration. The driver does not need to implement all the parameters,
+ * but should report what is actually used.
+ */
+struct phy_plca_cfg {
+ int version;
+ int enabled;
+ int node_id;
+ int node_cnt;
+ int to_tmr;
+ int burst_cnt;
+ int burst_tmr;
+};
+
/**
* struct phy_device - An instance of a PHY
*
@@ -655,6 +697,7 @@ struct phy_oatc14_sqi_capability {
* @shared: Pointer to private data shared by phys in one package
* @priv: Pointer to driver private data
* @oatc14_sqi_capability: SQI capability information for OATC14 10Base-T1S PHY
+ * @plca_cfg: Cache PLCA configuration for OATC10 compliance 10Base-T1S PHY
*
* interrupts currently only supports enabled or disabled,
* but could be changed in the future to support enabling
@@ -807,6 +850,7 @@ struct phy_device {
#endif
struct phy_oatc14_sqi_capability oatc14_sqi_capability;
+ struct phy_plca_cfg plca_cfg;
};
/* Generic phy_device::dev_flags */
@@ -858,48 +902,6 @@ enum link_inband_signalling {
LINK_INBAND_BYPASS = BIT(2),
};
-/**
- * struct phy_plca_cfg - Configuration of the PLCA (Physical Layer Collision
- * Avoidance) Reconciliation Sublayer.
- *
- * @version: read-only PLCA register map version. -1 = not available. Ignored
- * when setting the configuration. Format is the same as reported by the PLCA
- * IDVER register (31.CA00). -1 = not available.
- * @enabled: PLCA configured mode (enabled/disabled). -1 = not available / don't
- * set. 0 = disabled, anything else = enabled.
- * @node_id: the PLCA local node identifier. -1 = not available / don't set.
- * Allowed values [0 .. 254]. 255 = node disabled.
- * @node_cnt: the PLCA node count (maximum number of nodes having a TO). Only
- * meaningful for the coordinator (node_id = 0). -1 = not available / don't
- * set. Allowed values [1 .. 255].
- * @to_tmr: The value of the PLCA to_timer in bit-times, which determines the
- * PLCA transmit opportunity window opening. See IEEE802.3 Clause 148 for
- * more details. The to_timer shall be set equal over all nodes.
- * -1 = not available / don't set. Allowed values [0 .. 255].
- * @burst_cnt: controls how many additional frames a node is allowed to send in
- * single transmit opportunity (TO). The default value of 0 means that the
- * node is allowed exactly one frame per TO. A value of 1 allows two frames
- * per TO, and so on. -1 = not available / don't set.
- * Allowed values [0 .. 255].
- * @burst_tmr: controls how many bit times to wait for the MAC to send a new
- * frame before interrupting the burst. This value should be set to a value
- * greater than the MAC inter-packet gap (which is typically 96 bits).
- * -1 = not available / don't set. Allowed values [0 .. 255].
- *
- * A structure containing configuration parameters for setting/getting the PLCA
- * RS configuration. The driver does not need to implement all the parameters,
- * but should report what is actually used.
- */
-struct phy_plca_cfg {
- int version;
- int enabled;
- int node_id;
- int node_cnt;
- int to_tmr;
- int burst_cnt;
- int burst_tmr;
-};
-
/**
* struct phy_plca_status - Status of the PLCA (Physical Layer Collision
* Avoidance) Reconciliation Sublayer.
@@ -2333,6 +2335,8 @@ int genphy_c45_oatc14_cable_test_get_status(struct phy_device *phydev,
bool *finished);
int genphy_c45_oatc14_get_sqi_max(struct phy_device *phydev);
int genphy_c45_oatc14_get_sqi(struct phy_device *phydev);
+int genphy_c45_oatc10_suspend(struct phy_device *phydev);
+int genphy_c45_oatc10_resume(struct phy_device *phydev);
/* The gen10g_* functions are the old Clause 45 stub */
int gen10g_config_aneg(struct phy_device *phydev);
--
2.34.1
^ permalink raw reply related
* [PATCH net-next 3/3] net: usb: smsc95xx: suspend PHY during USB suspend
From: Parthiban Veerasooran @ 2026-03-30 13:42 UTC (permalink / raw)
To: parthiban.veerasooran, piergiorgio.beruto, andrew, hkallweit1,
linux, davem, edumazet, kuba, pabeni, steve.glendinning,
UNGLinuxDriver
Cc: netdev, linux-usb
In-Reply-To: <20260330134222.67597-1-parthiban.veerasooran@microchip.com>
The smsc95xx driver registers a PHY device but does not currently
propagate suspend events to it when the USB interface is suspended.
Call phy_suspend() from the driver's suspend callback so the attached
PHY can properly enter low-power state during system or runtime
suspend. This aligns smsc95xx suspend handling with other network
drivers that manage an external or integrated PHY.
Without this, the PHY may remain active and fail to execute its own
suspend procedure, leading to unnecessary power consumption or
incorrect resume behavior.
This change is also required for the EVB-LAN8670-USB Rev.D0 device to
support OATC10-compliant sleep and wake functionality.
Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
---
drivers/net/usb/smsc95xx.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 42e4048b574b..3a6e03b7410a 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1550,6 +1550,12 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
pdata->pm_task = current;
+ if (pdata->phydev) {
+ ret = phy_suspend(pdata->phydev);
+ if (ret)
+ return ret;
+ }
+
ret = usbnet_suspend(intf, message);
if (ret < 0) {
netdev_warn(dev->net, "usbnet_suspend error\n");
--
2.34.1
^ permalink raw reply related
* [PATCH net-next 2/3] net: phy: microchip_t1s: add suspend and WOL support for LAN867x Rev.D0
From: Parthiban Veerasooran @ 2026-03-30 13:42 UTC (permalink / raw)
To: parthiban.veerasooran, piergiorgio.beruto, andrew, hkallweit1,
linux, davem, edumazet, kuba, pabeni, steve.glendinning,
UNGLinuxDriver
Cc: netdev, linux-usb
In-Reply-To: <20260330134222.67597-1-parthiban.veerasooran@microchip.com>
The LAN867x Rev.D0 PHY supports OATC10-compliant sleep and wake
functionality, which can be used for system suspend and wake-up via
MDI Wake-up Pulse (WUP).
Add suspend/resume handling and basic Wake-on-LAN support using
WAKE_PHY, enabling WUP as a wake source via the Sleep Control 0
register. The driver configures MDI wake before entering suspend
and reports/enables the wake source through ethtool WOL callbacks.
Also adjust the Rev.D0 configuration path to avoid checking the reset
completion status when resuming from suspend, as the reset status is
only valid after an explicit PHY reset and not after sleep.
This allows LAN867x Rev.D0 systems to reliably wake from suspend via
MDI activity while preserving correct initialization behavior.
Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
---
drivers/net/phy/microchip_t1s.c | 67 +++++++++++++++++++++++++++++++--
1 file changed, 64 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
index e601d56b2507..6b158ae5fb16 100644
--- a/drivers/net/phy/microchip_t1s.c
+++ b/drivers/net/phy/microchip_t1s.c
@@ -38,6 +38,10 @@
#define LINK_STATUS_CONFIGURATION GENMASK(12, 11)
#define LINK_STATUS_SEMAPHORE BIT(0)
+/* Sleep Control 0 Register */
+#define LAN867X_REG_SLEEP_CTRL0 0x0080
+#define SLEEP_CTRL0_MDI_WAKEUP_EN BIT(14)
+
/* Link Status Configuration */
#define LINK_STATUS_CONFIG_PLCA_STATUS 0x1
#define LINK_STATUS_CONFIG_SEMAPHORE 0x2
@@ -472,9 +476,14 @@ static int lan867x_revd0_config_init(struct phy_device *phydev)
{
int ret;
- ret = lan867x_check_reset_complete(phydev);
- if (ret)
- return ret;
+ /* Reset status is only valid after an explicit PHY reset, it is not set
+ * when the PHY resumes from suspend/sleep state.
+ */
+ if (!phydev->suspended) {
+ ret = lan867x_check_reset_complete(phydev);
+ if (ret)
+ return ret;
+ }
for (int i = 0; i < ARRAY_SIZE(lan867x_revd0_fixup_regs); i++) {
ret = phy_write_mmd(phydev, MDIO_MMD_VEND2,
@@ -491,6 +500,53 @@ static int lan867x_revd0_config_init(struct phy_device *phydev)
return lan867x_revd0_link_active_selection(phydev, false);
}
+static int lan867x_revd0_suspend(struct phy_device *phydev)
+{
+ int ret;
+
+ /* Configure WUP as wake source for system suspend */
+ ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2,
+ LAN867X_REG_SLEEP_CTRL0,
+ SLEEP_CTRL0_MDI_WAKEUP_EN);
+ if (ret)
+ return ret;
+
+ return genphy_c45_oatc10_suspend(phydev);
+}
+
+static int lan867x_revd0_set_wol(struct phy_device *phydev,
+ struct ethtool_wolinfo *wol)
+{
+ /* Only support WAKE_PHY via WUP (Wake-up Pulse) */
+ if (wol->wolopts & ~WAKE_PHY)
+ return -EOPNOTSUPP;
+
+ if (wol->wolopts & WAKE_PHY) {
+ return phy_set_bits_mmd(phydev, MDIO_MMD_VEND2,
+ LAN867X_REG_SLEEP_CTRL0,
+ SLEEP_CTRL0_MDI_WAKEUP_EN);
+ } else {
+ /* Disable WUP wake source */
+ return phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2,
+ LAN867X_REG_SLEEP_CTRL0,
+ SLEEP_CTRL0_MDI_WAKEUP_EN);
+ }
+}
+
+static void lan867x_revd0_get_wol(struct phy_device *phydev,
+ struct ethtool_wolinfo *wol)
+{
+ int ret;
+
+ wol->supported = WAKE_PHY;
+ wol->wolopts = 0;
+
+ /* Check if WUP wake source is currently enabled */
+ ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, LAN867X_REG_SLEEP_CTRL0);
+ if (ret >= 0 && (ret & SLEEP_CTRL0_MDI_WAKEUP_EN))
+ wol->wolopts = WAKE_PHY;
+}
+
static int lan86xx_read_status(struct phy_device *phydev)
{
/* The phy has some limitations, namely:
@@ -569,6 +625,7 @@ static struct phy_driver microchip_t1s_driver[] = {
PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVD0),
.name = "LAN867X Rev.D0",
.features = PHY_BASIC_T1S_P2MP_FEATURES,
+ .flags = PHY_ALWAYS_CALL_SUSPEND,
.config_init = lan867x_revd0_config_init,
.get_plca_cfg = genphy_c45_plca_get_cfg,
.set_plca_cfg = lan86xx_plca_set_cfg,
@@ -577,6 +634,10 @@ static struct phy_driver microchip_t1s_driver[] = {
.cable_test_get_status = genphy_c45_oatc14_cable_test_get_status,
.get_sqi = genphy_c45_oatc14_get_sqi,
.get_sqi_max = genphy_c45_oatc14_get_sqi_max,
+ .suspend = lan867x_revd0_suspend,
+ .resume = genphy_c45_oatc10_resume,
+ .get_wol = lan867x_revd0_get_wol,
+ .set_wol = lan867x_revd0_set_wol,
},
{
PHY_ID_MATCH_EXACT(PHY_ID_LAN865X_REVB),
--
2.34.1
^ permalink raw reply related
* Re: [RFC PATCH 1/2] xhci: prevent automatic endpoint restart after stall or error
From: stern @ 2026-03-30 14:17 UTC (permalink / raw)
To: Mathias Nyman
Cc: Thinh Nguyen, linux-usb@vger.kernel.org, michal.pecio@gmail.com,
oneukum@suse.com, niklas.neronin@linux.intel.com
In-Reply-To: <2604e951-01e8-44d0-a11e-be63b0849c23@linux.intel.com>
On Mon, Mar 30, 2026 at 03:51:46PM +0300, Mathias Nyman wrote:
> Ideally xhci driver would return the URB with EPIPE after STALL, and not continue
> processing URBs before a clear halt is sent, or a new URB is enqueued.
> USB core would hold off submitting URBs to xhci, buffering URBs enqueued for this
> STALLED endpoint until class driver urb->complete() finishes for that EPIPE URB.
>
> Usb core could flag this endpoint as "halt_pending" before adding the EPIPE URB to
> the bh workqueue. Then after urb->complete() work is called and core is sure class
> driver is aware of the EPIPE, then core would clear the flag and flush the buffered
> URBs to the host controller drivers, restarting the ring
This is not practical; it would result in a big slowdown for large bulk
transfers. Doing this would mean the core could not send an URB to the
HCD until all the previous URBs for that endpoint had completed and the
interrupt handler had run, which would add significant latency to
transfers.
Exactly what should happen to URBs coming after one that completes with
-EPIPE is not immediately obvious. If the HCD did try to send them to
the device right away then they would also be stalled, which is
obviously non-productive. The only guarantee the kernel makes
about this situation is that the endpoint queue won't restart
until all completed or unlinked URBs have been given back (likewise for
-EPROTO errors).
The only safe course available to class drivers is to unlink all the
pending URBs. In theory the core could do this for all drivers
automatically, but at present it doesn't.
> Class driver urb->complete() would most likely retire/cancel the pending URBs, both the
> earlier queued 'tainted' URBs, and the most recent 'buffered' URBs in usb core.
> Class driver should clear the halt, but is free to do whatever it wants.
> It could choose to send a new URB without clearing the halt,
> have it processed, trigger STALL again, and get URB returned with EPIPE status.
>
> I don't think most class/usb device drivers really handle stall that well.
> Above might be wishful thinking.
Indeed. We can make life a little easier for drivers, but clearing the
endpoint halt is up to them.
> > Currently you have the xhci driver to "retire" the halted URBs. However,
> > you also noted that class/core may attempt to retire the pending URBs.
> > Who's expected to handle the retirement here?
>
> Maybe we should let core retire the pending URBS, and only fix the xhci driver
> 'automatic endpoint restart after stall' part after that core change is done.
>
> Should cause less regression.
>
> >
> > On a separate note, will you plan to implement the clear-halt for EPROTO
> > in xhci?
>
> I don't think this should be part of xhci driver. Decision to send control requests
> to the device should be done by core or class drivers.
Here's a troubling consequence for people to consider: Suppose an
endpoint queue stops with -EPROTO or -EPIPE, and before the class driver
gets around to calling usb_clear_halt(), it is unbound. What happens
the next time a driver binds to the device and tries to use the
endpoint?
In particular: What does xhci-hcd do if an URB is submitted for an
endpoint whose queue is currently stopped? Does it reject the
submission with some sort of error code, or does it go ahead and add the
URB to the end of the non-advancing queue? If the latter is true, how
will a newly bound driver ever discover that the queue is stopped?
Alan Stern
^ permalink raw reply
* Re: [PATCH] usb: phy: mxs: manually reset phy regs after a warm reset
From: Greg KH @ 2026-03-30 14:19 UTC (permalink / raw)
To: Xu Yang
Cc: Frank.Li, s.hauer, kernel, festevam, linux-usb, imx,
linux-arm-kernel, linux-kernel
In-Reply-To: <20260330093133.973785-1-xu.yang_2@nxp.com>
On Mon, Mar 30, 2026 at 05:31:33PM +0800, Xu Yang wrote:
> The usb phy registers are not fully reset on warm reset under stress
> conditions. We need to manually reset those (CTRL, PWD, DEBUG, PLL_SIC)
> regs after a warm reset. This will reset DEBUG and PLL_SIC registers.
> CTRL and PWD register are handled by "SFT" bit in stmp_reset_block().
>
> ERR051269: USB PHY registers not fully resetting on warm reset under
> stress conditions
>
> The following USB PHY registers must be written by SW to restore the reset
> value after a warm reset:
>
> Reg: ctrl Addr: 0x29910030 Data: 0xc000_0000
> Reg: pwd Addr: 0x29910000 Data: 0x001e_1c00
> Reg: debug0 Addr: 0x29910050 Data: 0x7f18_0000
> Reg: pll_sic Addr: 0x299100a0 Data: 0x00d1_2000
>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
> drivers/usb/phy/phy-mxs-usb.c | 32 +++++++++++++++++++++++++++++---
> 1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index 7069dd3f4d0d..dd42db8a0829 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -209,6 +209,9 @@ static const struct mxs_phy_data imx6ul_phy_data = {
> static const struct mxs_phy_data imx7ulp_phy_data = {
> };
>
> +static const struct mxs_phy_data imx8ulp_phy_data = {
> +};
> +
> static const struct of_device_id mxs_phy_dt_ids[] = {
> { .compatible = "fsl,imx6sx-usbphy", .data = &imx6sx_phy_data, },
> { .compatible = "fsl,imx6sl-usbphy", .data = &imx6sl_phy_data, },
> @@ -217,6 +220,7 @@ static const struct of_device_id mxs_phy_dt_ids[] = {
> { .compatible = "fsl,vf610-usbphy", .data = &vf610_phy_data, },
> { .compatible = "fsl,imx6ul-usbphy", .data = &imx6ul_phy_data, },
> { .compatible = "fsl,imx7ulp-usbphy", .data = &imx7ulp_phy_data, },
> + { .compatible = "fsl,imx8ulp-usbphy", .data = &imx8ulp_phy_data, },
Why can't you use &imx7ulp_phy_data here as it's all just empty?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH RFC net-next] usb: gadget: u_ether: fix eth_get_drvinfo() oops
From: Greg Kroah-Hartman @ 2026-03-30 14:21 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Guilherme G. Piccoli, Kees Cook, linux-usb, netdev, Tony Luck
In-Reply-To: <E1w6oNQ-0000000ER1E-43NU@rmk-PC.armlinux.org.uk>
On Sun, Mar 29, 2026 at 12:32:20PM +0100, Russell King (Oracle) wrote:
> The following oops was observed on the nVidia Jetson Xavier NX1
> platform with 7.0.0-rc5, which is caused by dev->gadget becoming NULL
> while the u_ether device is still visible to userspace. Adding some
> debug reveals that we see this sequence:
>
> net usb1: gadget detached
> tegra-xudc 3550000.usb: ep 0 disabled
> net usb1: no gadget for drvinfo
> usb1 (unregistering): left allmulticast mode
> usb1 (unregistering): left promiscuous mode
> l4tbr0: port 2(usb1) entered disabled state
>
> Put a sticky plaster over this (it's racy!) I suspect
> gether_detach_gadget() needs to at least take the RTNL as well to
> prevent ethtool ops running concurrently with the gadget being
> detached.
>
> Unable to handle kernel NULL pointer dereference at virtual address 000000000000
> 00d0
> Mem abort info:
> ESR = 0x0000000096000004
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x04: level 0 translation fault
> Data abort info:
> ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> user pgtable: 4k pages, 48-bit VAs, pgdp=00000001086b1000
> [00000000000000d0] pgd=0000000000000000, p4d=0000000000000000
> Internal error: Oops: 0000000096000004 [#1] SMP
> Modules linked in: snd_soc_tegra210_mvc snd_soc_tegra186_asrc snd_soc_tegra210_m
> ixer snd_soc_tegra210_admaif snd_soc_tegra_pcm snd_soc_tegra210_ope snd_soc_tegr
> a186_dspk snd_soc_tegra210_i2s snd_soc_simple_card_utils snd_soc_tegra210_dmic s
> nd_soc_tegra210_amx snd_soc_tegra210_adx snd_soc_tegra210_sfc rtw88_8822ce rtw88
> _8822c rtw88_pci rtw88_core mac80211 libarc4 sha256 cfg80211 snd_soc_tegra210_ah
> ub snd_soc_core snd_pcm_dmaengine snd_pcm snd_timer tegra210_adma zram tegra_drm
> zsmalloc usb_f_ncm syscopyarea usb_f_mass_storage drm_client_lib sysfillrect sy
> simgblt ramoops fb_sys_fops reed_solomon nls_iso8859_1 drm_display_helper nls_cp437 vfat drm_kms_helper fb fat cec drm_dp_aux_bus tegra_aconnect usb_f_acm u_serial usb_f_rndis u_ether max77620_wdt realtek tegra_xudc phy_package snd phy_tegra194_p2u dwmac_dwc_qos_eth soundcore stmmac_platform pwm_tegra rtc_efi tegra186_gpc_dma virt_dma tegra_bpmp_thermal host1x pcie_tegra194 libcomposite udc_core drm drm_panel_orientation_quirks backlight efivarfs ip_tables
> x_tables
> CPU: 1 UID: 0 PID: 4181 Comm: systemd-udevd Not tainted 7.0.0-rc5-net-next+ #640 PREEMPT
> Hardware name: NVIDIA NVIDIA Jetson Xavier NX Developer Kit/Jetson, BIOS 6.0-37391689 08/28/2024
> pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : eth_get_drvinfo+0x4c/0x80 [u_ether]
> lr : eth_get_drvinfo+0x40/0x80 [u_ether]
> sp : ffff80008484bb60
> x29: ffff80008484bb60 x28: ffff000083de18c0 x27: 0000000000000000
> x26: 0000000000000000 x25: ffff0000947da0a8 x24: 0000000000004000
> x23: ffff0000a38a5600 x22: 0000000000000000 x21: ffff80007a3a5558
> x20: ffff0000947da000 x19: ffff0000a38a5608 x18: 0000000000000000
> x17: 0000000000000000 x16: 0000000000000000 x15: 0000ffffe09af760
> x14: 0000000000000000 x13: 0000000000000020 x12: 0101010101010101
> x11: 7f7f7f7f7f7f7f7f x10: 00007ffff0071c50 x9 : 0000000000000020
> x8 : 0101010101010101 x7 : ffff0000a38a5634 x6 : fefefefefefefeff
> x5 : ffff0000a38a562c x4 : 0000000000383030 x3 : 0000000000383030
> x2 : 0000000000000020 x1 : 0000000000000000 x0 : ffff0000a38a564c
> Call trace:
> eth_get_drvinfo+0x4c/0x80 [u_ether] (P)
> ethtool_get_drvinfo+0x54/0x1e0
> __dev_ethtool+0x698/0x2000
> dev_ethtool+0x90/0x1a0
> dev_ioctl+0x37c/0x594
> sock_ioctl+0x390/0x5bc
> __arm64_sys_ioctl+0x408/0xeb4
> invoke_syscall.constprop.0+0x50/0xe0
> do_el0_svc+0x40/0xc0
> el0_svc+0x48/0x2a0
> el0t_64_sync_handler+0xa0/0xe4
> el0t_64_sync+0x19c/0x1a0
> Code: 95a68811 f9468a81 d2800402 91011260 (f9406821)
> ---[ end trace 0000000000000000 ]---
>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>
> 1. get_maintainers.pl is picking up on the PSTORE folk because of the
> mention of "ramoops" in the commit message... even though this has
> nothing to do with them.
>
> 2. I think there's more bugs here as mentioned in the commit message,
> since setting dev->gadget to NULL without taking any locking while
> the u_ether device is still published to userspace looks mightily
> dangerous. Seems to be another kernel driver that fails to obey the
> basics discussed in OLS some twenty years ago concerning the order
> of setup/publish/unpublish/teardown... and we still don't seem to
> be able to get it right. :(
>
> 3. Patch generated against the net-next tree, because that's what I'm
> working with being focussed on stmmac at the moment - but I would like
> my test platform not to oops and reboot itself when I instead want it
> to power off.
>
> drivers/usb/gadget/function/u_ether.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
> index 1a9e7c495e2e..d1ae91889537 100644
> --- a/drivers/usb/gadget/function/u_ether.c
> +++ b/drivers/usb/gadget/function/u_ether.c
> @@ -113,6 +113,10 @@ static void eth_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *p)
>
> strscpy(p->driver, "g_ether", sizeof(p->driver));
> strscpy(p->version, UETH__VERSION, sizeof(p->version));
> +
> + if (!dev->gadget)
> + return;
> +
> strscpy(p->fw_version, dev->gadget->name, sizeof(p->fw_version));
> strscpy(p->bus_info, dev_name(&dev->gadget->dev), sizeof(p->bus_info));
> }
Looks semi-sane, I'll pick this up through the USB tree now.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2] usb: gadget: rndis: validate query and set message buffers
From: Greg Kroah-Hartman @ 2026-03-30 14:23 UTC (permalink / raw)
To: Pengpeng Hou; +Cc: linux-usb, linux-kernel, Kees Cook
In-Reply-To: <20260323080845.40045-1-pengpeng@iscas.ac.cn>
On Mon, Mar 23, 2026 at 04:08:45PM +0800, Pengpeng Hou wrote:
> rndis_set_response() already checks the host-controlled
> InformationBufferOffset/InformationBufferLength pair before using it,
> but the QUERY path still passes the same fields straight into
> gen_ndis_query_resp(). The parser also does not verify that MsgLength
> fits the actual EP0 request buffer before dispatching the message.
>
> Pass the actual request size into rndis_msg_parser(), reject messages
> whose MsgLength exceeds the received buffer, and apply the same offset
> and length validation to QUERY and SET requests before dereferencing the
> embedded information buffer.
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> v2:
> - add commit message context and fix rationale
> - no code changes
Have you tested this? I remember lots of issues like this in the
protocol, so this might not be the only one in here. I really just want
to delete this code entirely, but some people really like to talk to old
obsolete Windows systems :(
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] usb: phy: mxs: manually reset phy regs after a warm reset
From: Frank Li @ 2026-03-30 14:33 UTC (permalink / raw)
To: Xu Yang
Cc: gregkh, s.hauer, kernel, festevam, linux-usb, imx,
linux-arm-kernel, linux-kernel
In-Reply-To: <20260330093133.973785-1-xu.yang_2@nxp.com>
On Mon, Mar 30, 2026 at 05:31:33PM +0800, Xu Yang wrote:
> The usb phy registers are not fully reset on warm reset under stress
> conditions. We need to manually reset those (CTRL, PWD, DEBUG, PLL_SIC)
Avoid the words "we ..."
So need manually reset CTRL, PWD, DEBUG, PLL_SIC ...
> regs after a warm reset. This will reset DEBUG and PLL_SIC registers.
> CTRL and PWD register are handled by "SFT" bit in stmp_reset_block().
>
> ERR051269: USB PHY registers not fully resetting on warm reset under
> stress conditions
>
> The following USB PHY registers must be written by SW to restore the reset
> value after a warm reset:
>
> Reg: ctrl Addr: 0x29910030 Data: 0xc000_0000
> Reg: pwd Addr: 0x29910000 Data: 0x001e_1c00
> Reg: debug0 Addr: 0x29910050 Data: 0x7f18_0000
> Reg: pll_sic Addr: 0x299100a0 Data: 0x00d1_2000
>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
> drivers/usb/phy/phy-mxs-usb.c | 32 +++++++++++++++++++++++++++++---
> 1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index 7069dd3f4d0d..dd42db8a0829 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -209,6 +209,9 @@ static const struct mxs_phy_data imx6ul_phy_data = {
> static const struct mxs_phy_data imx7ulp_phy_data = {
> };
>
> +static const struct mxs_phy_data imx8ulp_phy_data = {
> +};
> +
> static const struct of_device_id mxs_phy_dt_ids[] = {
> { .compatible = "fsl,imx6sx-usbphy", .data = &imx6sx_phy_data, },
> { .compatible = "fsl,imx6sl-usbphy", .data = &imx6sl_phy_data, },
> @@ -217,6 +220,7 @@ static const struct of_device_id mxs_phy_dt_ids[] = {
> { .compatible = "fsl,vf610-usbphy", .data = &vf610_phy_data, },
> { .compatible = "fsl,imx6ul-usbphy", .data = &imx6ul_phy_data, },
> { .compatible = "fsl,imx7ulp-usbphy", .data = &imx7ulp_phy_data, },
> + { .compatible = "fsl,imx8ulp-usbphy", .data = &imx8ulp_phy_data, },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, mxs_phy_dt_ids);
> @@ -248,6 +252,11 @@ static inline bool is_imx7ulp_phy(struct mxs_phy *mxs_phy)
> return mxs_phy->data == &imx7ulp_phy_data;
> }
>
> +static inline bool is_imx8ulp_phy(struct mxs_phy *mxs_phy)
> +{
> + return mxs_phy->data == &imx8ulp_phy_data;
don't use this kind check.
Add field 'need_reset_reg' in mxs_phy_data
imx8ulp_phy_data = {
.need_reset_reg = true;
}
if (mxs->data->need_reset_reg)
...
The same logic for
if (is_imx7ulp_phy(mxs_phy) || is_imx8ulp_phy(mxs_phy))
mxs_phy_pll_enable(phy->io_priv, false);
add 'need_phy_pull_enable' in mxs_phy_data. (new patch for it)
set it true at both imx7ulp_phy_data and imx8ulp_phy_data.
Frank
> +}
> +
> static inline bool is_imx6ul_phy(struct mxs_phy *mxs_phy)
> {
> return mxs_phy->data == &imx6ul_phy_data;
> @@ -305,12 +314,29 @@ static int mxs_phy_pll_enable(void __iomem *base, bool enable)
> return ret;
> }
>
> +/*
> + * The imx8ulp phy registers are not properly reset after a warm
> + * reset (ERR051269). Using the following steps to reset DEBUG and
> + * PLL_SIC regs. CTRL and PWD regs are reset by "SFT" bit in
> + * stmp_reset_block().
> + */
> +static void mxs_phy_regs_reset(void __iomem *base)
> +{
> + writel(0x7f180000, base + HW_USBPHY_DEBUG_SET);
> + writel(~0x7f180000, base + HW_USBPHY_DEBUG_CLR);
> + writel(0x00d12000, base + HW_USBPHY_PLL_SIC_SET);
> + writel(~0x00d12000, base + HW_USBPHY_PLL_SIC_CLR);
> +}
> +
> static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
> {
> int ret;
> void __iomem *base = mxs_phy->phy.io_priv;
>
> - if (is_imx7ulp_phy(mxs_phy)) {
> + if (is_imx8ulp_phy(mxs_phy))
> + mxs_phy_regs_reset(base);
> +
> + if (is_imx7ulp_phy(mxs_phy) || is_imx8ulp_phy(mxs_phy)) {
> ret = mxs_phy_pll_enable(base, true);
> if (ret)
> return ret;
> @@ -368,7 +394,7 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
> return 0;
>
> disable_pll:
> - if (is_imx7ulp_phy(mxs_phy))
> + if (is_imx7ulp_phy(mxs_phy) || is_imx8ulp_phy(mxs_phy))
> mxs_phy_pll_enable(base, false);
> return ret;
> }
> @@ -487,7 +513,7 @@ static void mxs_phy_shutdown(struct usb_phy *phy)
> writel(BM_USBPHY_CTRL_CLKGATE,
> phy->io_priv + HW_USBPHY_CTRL_SET);
>
> - if (is_imx7ulp_phy(mxs_phy))
> + if (is_imx7ulp_phy(mxs_phy) || is_imx8ulp_phy(mxs_phy))
> mxs_phy_pll_enable(phy->io_priv, false);
>
> if (mxs_phy->phy_3p0)
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: phy: phy-c45: add OATC10 Sleep/Wakeup support in 10BASE-T1S PHYs
From: Andrew Lunn @ 2026-03-30 14:46 UTC (permalink / raw)
To: Parthiban Veerasooran
Cc: piergiorgio.beruto, hkallweit1, linux, davem, edumazet, kuba,
pabeni, steve.glendinning, UNGLinuxDriver, netdev, linux-usb
In-Reply-To: <20260330134222.67597-2-parthiban.veerasooran@microchip.com>
> + /* Cache PLCA settings for later use. These values must be restored when
> + * the PHY wakes up from the low-power sleep state, as all configured
> + * settings are lost.
> + */
Does the standard define that this configuration is lost in low power
mode? If the standard says this, then fine. But if not, it should be
the PHY driver which saves its state during suspend, and restores it
during resume.
Andrew
^ permalink raw reply
* Re: [PATCH 6/6] usbip: vhci_sysfs: Use safer strscpy() instead of strcpy()
From: gregkh @ 2026-03-30 14:49 UTC (permalink / raw)
To: Shuah Khan
Cc: 艾超, b-liu, johan, badhri, heikki.krogerus,
valentina.manea.m, shuah, i, tiwai, kees, christophe.jaillet,
prashanth.k, khtsai, tglx, mingo, linux-usb, linux-kernel,
Dan Carpenter
In-Reply-To: <b3a6543a-9375-4e52-960e-3f8dc4be5526@linuxfoundation.org>
On Thu, Mar 26, 2026 at 12:19:24PM -0600, Shuah Khan wrote:
> On 3/25/26 04:57, 艾超 wrote:
> > Dear Shuah:
> >
> > Test Report for [usbip: vhci_sysfs: Use safer strscpy() instead of strcpy()]
> > Environment:
> > Kernel Version: v6.12.76+ (with patch applied)
> > Hardware/QEMU: Thinkpad E15
>
>
> > Conclusion:
> > The longth of the name is MAX_STATUS_NAME+1 = 17, used the strcpy copy "status-abcdefghijklmnopqrstuvwxyz1234567890" in name and used strscpy copy "status-abcdefghi" in name.
> > Tested-by: Aichao@kylinos.cn
> >
>
> Thank you for the report.
>
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
>
> Greg, Please pick this patch up.
Can someone resend it please, I can't seem to find it anywhere on its
own.
thanks,
greg k-h
^ permalink raw reply
* [PATCH] usb: core: avoid double use of 'usb3-phy'
From: Gabor Juhos @ 2026-03-30 15:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Stanley Chang
Cc: Andrew Lunn, Gregory Clement, Sebastian Hesselbarth,
Miquel Raynal, linux-usb, linux-kernel, linux-arm-kernel,
Gabor Juhos
Commit 53a2d95df836 ("usb: core: add phy notify connect and disconnect")
causes double use of the 'usb3-phy' in certain cases.
Since that commit, if a generic PHY named 'usb3-phy' is specified in
the device tree, that is getting added to the 'phy_roothub' list of the
secondary HCD by the usb_phy_roothub_alloc_usb3_phy() function. However,
that PHY is getting added also to the primary HCD's 'phy_roothub' list
by usb_phy_roothub_alloc() if there is no generic PHY specified with
'usb2-phy' name.
This causes that the usb_add_hcd() function executes each phy operations
twice on the 'usb3-phy'. Once when the primary HCD is added, then once
again when the secondary HCD is added.
The issue affects the Marvell Armada 3700 platform at least, where a
custom name is used for the USB2 PHY:
$ git grep 'phy-names.*usb3' arch/arm64/boot/dts/marvell/armada-37xx.dtsi | tr '\t' ' '
arch/arm64/boot/dts/marvell/armada-37xx.dtsi: phy-names = "usb3-phy", "usb2-utmi-otg-phy";
Extend the usb_phy_roothub_alloc_usb3_phy() function to skip adding the
'usb3-phy' to the 'phy_roothub' list of the secondary HCD when 'usb2-phy'
is not specified in the device tree to avoid the double use.
Fixes: 53a2d95df836 ("usb: core: add phy notify connect and disconnect")
Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
drivers/usb/core/phy.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
index 4bba1c2757406a35bf19eb7984a2807212374d18..4d966cc9cdc9510147041df7875ad9e48a9ea3af 100644
--- a/drivers/usb/core/phy.c
+++ b/drivers/usb/core/phy.c
@@ -114,7 +114,7 @@ EXPORT_SYMBOL_GPL(usb_phy_roothub_alloc);
struct usb_phy_roothub *usb_phy_roothub_alloc_usb3_phy(struct device *dev)
{
struct usb_phy_roothub *phy_roothub;
- int num_phys;
+ int num_phys, usb2_phy_index;
if (!IS_ENABLED(CONFIG_GENERIC_PHY))
return NULL;
@@ -124,6 +124,16 @@ struct usb_phy_roothub *usb_phy_roothub_alloc_usb3_phy(struct device *dev)
if (num_phys <= 0)
return NULL;
+ /*
+ * If 'usb2-phy' is not present, usb_phy_roothub_alloc() added
+ * all PHYs to the primary HCD's phy_roothub already, so skip
+ * adding 'usb3-phy' here to avoid double use of that.
+ */
+ usb2_phy_index = of_property_match_string(dev->of_node, "phy-names",
+ "usb2-phy");
+ if (usb2_phy_index < 0)
+ return NULL;
+
phy_roothub = devm_kzalloc(dev, sizeof(*phy_roothub), GFP_KERNEL);
if (!phy_roothub)
return ERR_PTR(-ENOMEM);
---
base-commit: f50200dd44125e445a6164e88c217472fa79cdbc
change-id: 20260329-usb-avoid-usb3-phy-double-use-7e498373073c
Best regards,
--
Gabor Juhos <j4g8y7@gmail.com>
^ permalink raw reply related
* Re: [PATCH RFC net-next] usb: gadget: u_ether: fix eth_get_drvinfo() oops
From: Greg Kroah-Hartman @ 2026-03-30 15:21 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Guilherme G. Piccoli, Kees Cook, linux-usb, netdev, Tony Luck
In-Reply-To: <E1w6oNQ-0000000ER1E-43NU@rmk-PC.armlinux.org.uk>
On Sun, Mar 29, 2026 at 12:32:20PM +0100, Russell King (Oracle) wrote:
> The following oops was observed on the nVidia Jetson Xavier NX1
> platform with 7.0.0-rc5, which is caused by dev->gadget becoming NULL
> while the u_ether device is still visible to userspace. Adding some
> debug reveals that we see this sequence:
>
> net usb1: gadget detached
> tegra-xudc 3550000.usb: ep 0 disabled
> net usb1: no gadget for drvinfo
> usb1 (unregistering): left allmulticast mode
> usb1 (unregistering): left promiscuous mode
> l4tbr0: port 2(usb1) entered disabled state
>
> Put a sticky plaster over this (it's racy!) I suspect
> gether_detach_gadget() needs to at least take the RTNL as well to
> prevent ethtool ops running concurrently with the gadget being
> detached.
>
> Unable to handle kernel NULL pointer dereference at virtual address 000000000000
> 00d0
> Mem abort info:
> ESR = 0x0000000096000004
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x04: level 0 translation fault
> Data abort info:
> ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> user pgtable: 4k pages, 48-bit VAs, pgdp=00000001086b1000
> [00000000000000d0] pgd=0000000000000000, p4d=0000000000000000
> Internal error: Oops: 0000000096000004 [#1] SMP
> Modules linked in: snd_soc_tegra210_mvc snd_soc_tegra186_asrc snd_soc_tegra210_m
> ixer snd_soc_tegra210_admaif snd_soc_tegra_pcm snd_soc_tegra210_ope snd_soc_tegr
> a186_dspk snd_soc_tegra210_i2s snd_soc_simple_card_utils snd_soc_tegra210_dmic s
> nd_soc_tegra210_amx snd_soc_tegra210_adx snd_soc_tegra210_sfc rtw88_8822ce rtw88
> _8822c rtw88_pci rtw88_core mac80211 libarc4 sha256 cfg80211 snd_soc_tegra210_ah
> ub snd_soc_core snd_pcm_dmaengine snd_pcm snd_timer tegra210_adma zram tegra_drm
> zsmalloc usb_f_ncm syscopyarea usb_f_mass_storage drm_client_lib sysfillrect sy
> simgblt ramoops fb_sys_fops reed_solomon nls_iso8859_1 drm_display_helper nls_cp437 vfat drm_kms_helper fb fat cec drm_dp_aux_bus tegra_aconnect usb_f_acm u_serial usb_f_rndis u_ether max77620_wdt realtek tegra_xudc phy_package snd phy_tegra194_p2u dwmac_dwc_qos_eth soundcore stmmac_platform pwm_tegra rtc_efi tegra186_gpc_dma virt_dma tegra_bpmp_thermal host1x pcie_tegra194 libcomposite udc_core drm drm_panel_orientation_quirks backlight efivarfs ip_tables
> x_tables
> CPU: 1 UID: 0 PID: 4181 Comm: systemd-udevd Not tainted 7.0.0-rc5-net-next+ #640 PREEMPT
> Hardware name: NVIDIA NVIDIA Jetson Xavier NX Developer Kit/Jetson, BIOS 6.0-37391689 08/28/2024
> pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : eth_get_drvinfo+0x4c/0x80 [u_ether]
> lr : eth_get_drvinfo+0x40/0x80 [u_ether]
> sp : ffff80008484bb60
> x29: ffff80008484bb60 x28: ffff000083de18c0 x27: 0000000000000000
> x26: 0000000000000000 x25: ffff0000947da0a8 x24: 0000000000004000
> x23: ffff0000a38a5600 x22: 0000000000000000 x21: ffff80007a3a5558
> x20: ffff0000947da000 x19: ffff0000a38a5608 x18: 0000000000000000
> x17: 0000000000000000 x16: 0000000000000000 x15: 0000ffffe09af760
> x14: 0000000000000000 x13: 0000000000000020 x12: 0101010101010101
> x11: 7f7f7f7f7f7f7f7f x10: 00007ffff0071c50 x9 : 0000000000000020
> x8 : 0101010101010101 x7 : ffff0000a38a5634 x6 : fefefefefefefeff
> x5 : ffff0000a38a562c x4 : 0000000000383030 x3 : 0000000000383030
> x2 : 0000000000000020 x1 : 0000000000000000 x0 : ffff0000a38a564c
> Call trace:
> eth_get_drvinfo+0x4c/0x80 [u_ether] (P)
> ethtool_get_drvinfo+0x54/0x1e0
> __dev_ethtool+0x698/0x2000
> dev_ethtool+0x90/0x1a0
> dev_ioctl+0x37c/0x594
> sock_ioctl+0x390/0x5bc
> __arm64_sys_ioctl+0x408/0xeb4
> invoke_syscall.constprop.0+0x50/0xe0
> do_el0_svc+0x40/0xc0
> el0_svc+0x48/0x2a0
> el0t_64_sync_handler+0xa0/0xe4
> el0t_64_sync+0x19c/0x1a0
> Code: 95a68811 f9468a81 d2800402 91011260 (f9406821)
> ---[ end trace 0000000000000000 ]---
>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>
> 1. get_maintainers.pl is picking up on the PSTORE folk because of the
> mention of "ramoops" in the commit message... even though this has
> nothing to do with them.
>
> 2. I think there's more bugs here as mentioned in the commit message,
> since setting dev->gadget to NULL without taking any locking while
> the u_ether device is still published to userspace looks mightily
> dangerous. Seems to be another kernel driver that fails to obey the
> basics discussed in OLS some twenty years ago concerning the order
> of setup/publish/unpublish/teardown... and we still don't seem to
> be able to get it right. :(
>
> 3. Patch generated against the net-next tree, because that's what I'm
> working with being focussed on stmmac at the moment - but I would like
> my test platform not to oops and reboot itself when I instead want it
> to power off.
>
> drivers/usb/gadget/function/u_ether.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
> index 1a9e7c495e2e..d1ae91889537 100644
> --- a/drivers/usb/gadget/function/u_ether.c
> +++ b/drivers/usb/gadget/function/u_ether.c
> @@ -113,6 +113,10 @@ static void eth_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *p)
>
> strscpy(p->driver, "g_ether", sizeof(p->driver));
> strscpy(p->version, UETH__VERSION, sizeof(p->version));
> +
> + if (!dev->gadget)
> + return;
> +
> strscpy(p->fw_version, dev->gadget->name, sizeof(p->fw_version));
> strscpy(p->bus_info, dev_name(&dev->gadget->dev), sizeof(p->bus_info));
> }
> --
> 2.47.3
This is fixed much the same way in a patch that was sent before you, so
I'll take that one and get it into 7.0-final so that you don't trip over
this anymore.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] usb: cdns3: gadget: fix resource leak on gadget init failure
From: 吴永超 @ 2026-03-30 15:25 UTC (permalink / raw)
To: 'Peter Chen (CIX)'
Cc: 'Pawel Laszczak', 'Roger Quadros',
'Greg Kroah-Hartman', linux-usb, linux-kernel
On 26-03-30, Peter Chen wrote:
> I have two questions:
> - What exactly resource is leaked?
> - Do you call cdns_drd_host_on before setup host?
For the first question:
The DRD hardware is left in gadget mode while software state remains
INACTIVE, creating hardware/software state inconsistency.
I will update the commit title to "usb: cdns3: gadget: fix state
inconsistency on gadget init failure" to better reflect this.
For the second question:
When gadget start fails, the role state is not set to
CDNS_ROLE_STATE_ACTIVE. When switching to host via:
echo host > /sys/class/usb_role/13180000.usb-role-switch/role
The flow is:
cdns_role_set()
-> cdns_role_stop()
if (cdns->roles[role]->state == CDNS_ROLE_STATE_INACTIVE)
return; // Skips cleanup because state is still INACTIVE
This violates the DRD controller design specification (Figure22),
which requires returning to idle state before switching roles.
Error logs:
[ 516.440698][ T4992] configfs-gadget 13180000.usb: failed to start g1: -19
[ 516.442035][ T4992] cdns-usb3 13180000.usb: Failed to add gadget
[ 516.443278][ T4992] cdns-usb3 13180000.usb: set role 2 has failed
...
echo host > /sys/class/usb_role/13180000.usb-role-switch/role
[ 1301.375722][ T5000] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller
[ 1301.377716][ T5000] Internal error: synchronous external abort: 96000010 [#1] PREEMPT SMP
...
I will prepare v2 with Fixed-by tag and Cc to stable tree.
-----邮件原件-----
发件人: Peter Chen (CIX) <peter.chen@kernel.org>
发送时间: 2026年3月30日 13:52
收件人: Yongchao Wu <yongchao.wu@autochips.com>
抄送: Pawel Laszczak <pawell@cadence.com>; Roger Quadros <rogerq@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
主题: Re: [PATCH] usb: cdns3: gadget: fix resource leak on gadget init failure
On 26-03-28 23:15:50, Yongchao Wu wrote:
> When cdns3_gadget_start() fails, the DRD gadget is left in an
> initialized state, causing a resource leak. When switching to host
> mode, the stale state triggers a synchronous external abort in
> xhci_gen_setup(), leading to kernel panic:
>
> [ 1301.377716][ T5000][E] Internal error: synchronous external abort:
> 96000010 [#1] PREEMPT SMP [ 1301.382485][ T5000][I] pc :
> xhci_gen_setup+0xa4/0x408 [ 1301.393391][ T5000][I] backtrace:
> ...
> xhci_gen_setup+0xa4/0x408 <-- CRASH
> xhci_plat_setup+0x44/0x58
> usb_add_hcd+0x284/0x678
> ...
> cdns_role_set+0x9c/0xbc <-- Role switch
>
> Fix by calling cdns_drd_gadget_off() in the error path to properly
> clean up the DRD gadget state. This prevents both the resource leak
> and the kernel panic during role switching.
>
Good catch.
I have two questions:
- What exactly resource is leaked?
- Do you call cdns_drd_host_on before setup host?
Besides, you may add Fixed-by tag and Cc to stable tree.
Peter
> Signed-off-by: Yongchao Wu <yongchao.wu@autochips.com>
> ---
> drivers/usb/cdns3/cdns3-gadget.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/cdns3/cdns3-gadget.c
> b/drivers/usb/cdns3/cdns3-gadget.c
> index d59a60a16..4cc3f2ffb 100644
> --- a/drivers/usb/cdns3/cdns3-gadget.c
> +++ b/drivers/usb/cdns3/cdns3-gadget.c
> @@ -3428,6 +3428,7 @@ static int __cdns3_gadget_init(struct cdns *cdns)
> ret = cdns3_gadget_start(cdns);
> if (ret) {
> pm_runtime_put_sync(cdns->dev);
> + cdns_drd_gadget_off(cdns);
> return ret;
> }
>
> --
> 2.43.0
>
>
--
Best regards,
Peter
^ permalink raw reply
* Re: [PATCH v3 1/4] media: uvcvideo: Import standard controls from uvcdynctrl
From: Hans de Goede @ 2026-03-30 15:44 UTC (permalink / raw)
To: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab,
Greg Kroah-Hartman
Cc: linux-media, linux-kernel, linux-usb, Manav Gautama, Martin Rubli
In-Reply-To: <20260316-uvcdynctrl-v3-1-19cd4657e1f3@chromium.org>
Hi,
On 16-Mar-26 14:34, Ricardo Ribalda wrote:
> The uvcdynctrl tool from libwebcam:
> https://sourceforge.net/projects/libwebcam/
> maps proprietary controls into v4l2 controls using the UVCIOC_CTRL_MAP
> ioctl.
>
> The tool has not been updated for 10+ years now, and there is no reason
> for the UVC driver to not do the mapping by itself.
>
> This patch adds the mappings from the uvcdynctrl into the driver. Hopefully
> this effort can help in deprecating the UVCIOC_CTRL_MAP ioctl.
>
> Some background about UVCIOC_CTRL_MAP (thanks Laurent for the context):
>
> ```
> this was envisioned as the base of a vibrant ecosystem where a large
> number of vendors would submit XML files that describe their XU control
> mappings, at a pace faster than could be supported by adding XU mappings
> to the driver. This vision failed to materialize and the tool has not
> been updated for 10+ years now. There is no reason to believe the
> situation will change.
> ```
Thanks, I've applied this to my local uvc/for-next branch and will
push this out to the public uvc repo for-next branch soon.
Regards,
Hans
>
> During the porting, the following mappings where NOT imported because
> they were not using standard v4l2 IDs. It is recommended that userspace
> moves to UVCIOC_CTRL_QUERY for non standard controls.
>
> {
> .id = V4L2_CID_FLASH_MODE,
> .entity = UVC_GUID_SIS_LED_HW_CONTROL,
> .selector = 4,
> .size = 4,
> .offset = 0,
> .v4l2_type = V4L2_CTRL_TYPE_MENU,
> .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> .menu_mask = 0x3,
> .menu_mapping = { 0x20, 0x22 },
> .menu_names = { "Off", "On" },
>
> },
> {
> .id = V4L2_CID_FLASH_FREQUENCY,
> .entity = UVC_GUID_SIS_LED_HW_CONTROL,
> .selector = 4,
> .size = 8,
> .offset = 16,
> .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> },
> {
> .id = V4L2_CID_LED1_MODE,
> .entity = UVC_GUID_LOGITECH_USER_HW_CONTROL_V1,
> .selector = 1,
> .size = 8,
> .offset = 0,
> .v4l2_type = V4L2_CTRL_TYPE_MENU,
> .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> .menu_mask = 0xF,
> .menu_mapping = { 0, 1, 2, 3 },
> .menu_names = { "Off", "On", "Blinking", "Auto" },
>
> },
> {
> .id = V4L2_CID_LED1_FREQUENCY,
> .entity = UVC_GUID_LOGITECH_USER_HW_CONTROL_V1,
> .selector = 1,
> .size = 8,
> .offset = 16,
> .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> },
> {
> .id = V4L2_CID_DISABLE_PROCESSING,
> .entity = UVC_GUID_LOGITECH_VIDEO_PIPE_V1,
> .selector = 5,
> .size = 8,
> .offset = 0,
> .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
> .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
> },
> {
> .id = V4L2_CID_RAW_BITS_PER_PIXEL,
> .entity = UVC_GUID_LOGITECH_VIDEO_PIPE_V1,
> .selector = 8,
> .size = 8,
> .offset = 0,
> .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> },
> {
> .id = V4L2_CID_LED1_MODE,
> .entity = UVC_GUID_LOGITECH_PERIPHERAL,
> .selector = 0x09,
> .size = 2,
> .offset = 8,
> .v4l2_type = V4L2_CTRL_TYPE_MENU,
> .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> .menu_mask = 0xF,
> .menu_mapping = { 0, 1, 2, 3 },
> .menu_names = { "Off", "On", "Blink", "Auto" },
>
> },
> {
> .id = V4L2_CID_LED1_FREQUENCY,
> .entity = UVC_GUID_LOGITECH_PERIPHERAL,
> .selector = 0x09,
> .size = 8,
> .offset = 24,
> .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> },
>
> This script has been used to generate the mappings. They were then
> reformatted manually to follow the driver style.
>
> import sys
> import uuid
> import re
> import xml.etree.ElementTree as ET
>
> def get_namespace(root):
> return re.match(r"\{.*\}", root.tag).group(0)
>
> def get_single_guid(ns, constant):
> id = constant.find(ns + "id").text
> value = constant.find(ns + "value").text
> return (id, value)
>
> def get_constants(ns, root):
> out = dict()
> for constant in root.iter(ns + "constant"):
> attr = constant.attrib
> if attr["type"] == "integer":
> id, value = get_single_guid(ns, constant)
> if id in out:
> print(f"dupe constant {id}")
> out[id] = value
>
> return out
>
> def get_guids(ns, root):
> out = dict()
> for constant in root.iter(ns + "constant"):
> attr = constant.attrib
> if attr["type"] == "guid":
> id, value = get_single_guid(ns, constant)
> if id in out:
> print(f"dupe guid {id}")
> out[id] = value
>
> return out
>
> def get_single_control(ns, control):
> out = {}
> for id in "entity", "selector", "index", "size", "description":
> v = control.find(ns + id)
> if v is None and id == "description":
> continue
> out[id] = v.text
>
> reqs = set()
> for r in control.find(ns + "requests"):
> reqs.add(r.text)
> out["requests"] = reqs
>
> return (control.attrib["id"], out)
>
> def get_controls(ns, root):
> out = dict()
> for control in root.iter(ns + "control"):
> id, value = get_single_control(ns, control)
> if id in out:
> print(f"Dupe control id {id}")
> out[id] = value
>
> return out
>
> def get_single_mapping(ns, mapping):
> out = {}
> out["name"] = mapping.find(ns + "name").text
> uvc = mapping.find(ns + "uvc")
> for id in "size", "offset", "uvc_type":
> out[id] = uvc.find(ns + id).text
> out["control_ref"] = uvc.find(ns + "control_ref").attrib["idref"]
>
> v4l2 = mapping.find(ns + "v4l2")
> for id in "id", "v4l2_type":
> out[id] = v4l2.find(ns + id).text
>
> menu = {}
> for entry in v4l2.iter(ns + "menu_entry"):
> menu[entry.attrib["name"]] = entry.attrib["value"]
> if menu:
> out["menu"] = menu
>
> return out
>
> def get_mapping(ns, root):
> out = []
> for control in root.iter(ns + "mapping"):
> mapping = get_single_mapping(ns, control)
> out += [mapping]
>
> return out
>
> def print_guids(guids):
> for g in guids:
> print(f"#define {g} \\")
> u_bytes = uuid.UUID(guids[g]).bytes_le
> u_bytes = [f"0x{b:02x}" for b in u_bytes]
> print("\t{ " + ", ".join(u_bytes) + " }")
>
> def print_flags(flags):
> get_range = {"GET_MIN", "GET_DEF", "GET_MAX", "GET_CUR", "GET_RES"}
> if get_range.issubset(flags):
> flags -= get_range
> flags.add("GET_RANGE")
>
> flags = list(flags)
> flags.sort()
> out = ""
> for f in flags[:-1]:
> out += f"UVC_CTRL_FLAG_{f}\n\t\t\t\t| "
>
> out += f"UVC_CTRL_FLAG_{flags[-1]}"
>
> return out
>
> def print_description(desc):
> print("/*")
> for line in desc.strip().splitlines():
> print(f" * {line.strip()}")
> print("*/")
>
> def print_controls(controls, cons):
> for id in controls:
> c = controls[id]
> if "description" in c:
> print_description(c["description"])
> print(
> f"""\t{{
> \t\t.entity\t\t= {c["entity"]},
> \t\t.selector\t= {cons[c["selector"]]},
> \t\t.index\t\t= {c["index"]},
> \t\t.size\t\t= {c["size"]},
> \t\t.flags\t\t= {print_flags(c["requests"])},
> \t}},"""
> )
>
> def menu_mapping_txt(menu):
> out = f"\n\t\t.menu_mask\t= 0x{((1<<len(menu))-1):X},\n"
> out += f"\t\t.menu_mapping\t= {{ {", ".join(menu.values())} }},\n"
> out += f"\t\t.menu_names\t= {{ \"{"\", \"".join(menu.keys())}\" }},\n"
> return out
>
> def print_mappings(mappings, controls, cons):
> for m in mappings:
> c = controls[m["control_ref"]]
>
> if "menu" in m:
> menu_mapping = menu_mapping_txt(m["menu"])
> else:
> menu_mapping = ""
> print(
> f"""\t{{
> \t\t.id\t\t= {m["id"]},
> \t\t.entity\t\t= {c["entity"]},
> \t\t.selector\t= {cons[c["selector"]]},
> \t\t.size\t\t= {m["size"]},
> \t\t.offset\t\t= {m["offset"]},
> \t\t.v4l2_type\t= {m["v4l2_type"]},
> \t\t.data_type\t= {m["uvc_type"]},{menu_mapping}
> \t}},"""
> )
>
> def print_code(guids, cons, controls, mappings):
> used_controls = set()
> for m in mappings:
> used_controls.add(m["control_ref"])
>
> used_guids = set()
> for c in used_controls:
> used_guids.add(controls[c]["entity"])
>
> print("\n######GUIDs#######\n")
> print_guids({id: guids[id] for id in guids if id in used_guids})
> print("\n######CONTROLS#######\n")
> print_controls({id: controls[id] for id in controls if id in used_controls}, cons)
> print("\n######MAPPINGS#######\n")
> print_mappings(mappings, controls, cons)
> # print(guids)
> # print(used_controls)
>
> root = ET.fromstring(sys.stdin.read())
> ns = get_namespace(root)
> cons = get_constants(ns, root)
> guids = get_guids(ns, root)
> controls = get_controls(ns, root)
> mappings = get_mapping(ns, root)
> print_code(guids, cons, controls, mappings)
>
> Cc: Manav Gautama <bandwidthcrunch@gmail.com>
> Cc: Martin Rubli <martin_rubli@logitech.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 174 +++++++++++++++++++++++++++++++++++++++
> include/linux/usb/uvc.h | 6 ++
> 2 files changed, 180 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index b9a3d9257a11..b6e020b41671 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -385,6 +385,99 @@ static const struct uvc_control_info uvc_ctrls[] = {
> | UVC_CTRL_FLAG_GET_RANGE
> | UVC_CTRL_FLAG_RESTORE,
> },
> + /*
> + * Allows the control of pan/tilt motor movements for camera models
> + * that support mechanical pan/tilt.
> + *
> + * Bits 0 to 15 control pan, bits 16 to 31 control tilt.
> + * The unit of the pan/tilt values is 1/64th of a degree and the
> + * resolution is 1 degree.
> + */
> + {
> + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1,
> + .selector = 1,
> + .index = 0,
> + .size = 4,
> + .flags = UVC_CTRL_FLAG_GET_DEF
> + | UVC_CTRL_FLAG_GET_MAX
> + | UVC_CTRL_FLAG_GET_MIN
> + | UVC_CTRL_FLAG_SET_CUR,
> + },
> + /*
> + * Reset the pan/tilt motors to their original position for camera
> + * models that support mechanical pan/tilt.
> + *
> + * Setting bit 0 resets the pan position.
> + * Setting bit 1 resets the tilt position.
> + *
> + * Both bits can be set at the same time to reset both, pan and tilt,
> + * at the same time.
> + */
> + {
> + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1,
> + .selector = 2,
> + .index = 1,
> + .size = 1,
> + .flags = UVC_CTRL_FLAG_GET_DEF
> + | UVC_CTRL_FLAG_GET_MAX
> + | UVC_CTRL_FLAG_GET_MIN
> + | UVC_CTRL_FLAG_SET_CUR,
> + },
> + /*
> + * Allows the control of focus motor movements for camera models that
> + * support mechanical focus.
> + *
> + * Bits 0 to 7 allow selection of the desired lens position.
> + * There are no physical units, instead, the focus range is spread over
> + * 256 logical units with 0 representing infinity focus and 255 being
> + * macro focus.
> + */
> + {
> + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1,
> + .selector = 3,
> + .index = 2,
> + .size = 6,
> + .flags = UVC_CTRL_FLAG_GET_CUR
> + | UVC_CTRL_FLAG_GET_DEF
> + | UVC_CTRL_FLAG_GET_MAX
> + | UVC_CTRL_FLAG_GET_MIN
> + | UVC_CTRL_FLAG_SET_CUR,
> + },
> + /*
> + * Allows the control of pan/tilt motor movements for camera models
> + * that support mechanical pan/tilt.
> + *
> + * Bits 0 to 15 control pan, bits 16 to 31 control tilt.
> + */
> + {
> + .entity = UVC_GUID_LOGITECH_PERIPHERAL,
> + .selector = 1,
> + .index = 0,
> + .size = 4,
> + .flags = UVC_CTRL_FLAG_GET_DEF
> + | UVC_CTRL_FLAG_GET_MAX
> + | UVC_CTRL_FLAG_GET_MIN
> + | UVC_CTRL_FLAG_GET_RES
> + | UVC_CTRL_FLAG_SET_CUR,
> + },
> + /*
> + * Reset the pan/tilt motors to their original position for camera
> + * models that support mechanical pan/tilt.
> + *
> + * Setting bit 0 resets the pan position.
> + * Setting bit 1 resets the tilt position.
> + */
> + {
> + .entity = UVC_GUID_LOGITECH_PERIPHERAL,
> + .selector = 2,
> + .index = 1,
> + .size = 1,
> + .flags = UVC_CTRL_FLAG_GET_DEF
> + | UVC_CTRL_FLAG_GET_MAX
> + | UVC_CTRL_FLAG_GET_MIN
> + | UVC_CTRL_FLAG_GET_RES
> + | UVC_CTRL_FLAG_SET_CUR,
> + },
> };
>
> static const u32 uvc_control_classes[] = {
> @@ -1009,6 +1102,87 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> .menu_mask = BIT(V4L2_COLORFX_VIVID) |
> BIT(V4L2_COLORFX_NONE),
> },
> + {
> + .id = V4L2_CID_PAN_RELATIVE,
> + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1,
> + .selector = 1,
> + .size = 16,
> + .offset = 0,
> + .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> + .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> + },
> + {
> + .id = V4L2_CID_TILT_RELATIVE,
> + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1,
> + .selector = 1,
> + .size = 16,
> + .offset = 16,
> + .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> + .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> + },
> + {
> + .id = V4L2_CID_PAN_RESET,
> + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1,
> + .selector = 2,
> + .size = 1,
> + .offset = 0,
> + .v4l2_type = V4L2_CTRL_TYPE_BUTTON,
> + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> + },
> + {
> + .id = V4L2_CID_TILT_RESET,
> + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1,
> + .selector = 2,
> + .size = 1,
> + .offset = 1,
> + .v4l2_type = V4L2_CTRL_TYPE_BUTTON,
> + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> + },
> + {
> + .id = V4L2_CID_PAN_RELATIVE,
> + .entity = UVC_GUID_LOGITECH_PERIPHERAL,
> + .selector = 1,
> + .size = 16,
> + .offset = 0,
> + .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> + .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> + },
> + {
> + .id = V4L2_CID_TILT_RELATIVE,
> + .entity = UVC_GUID_LOGITECH_PERIPHERAL,
> + .selector = 1,
> + .size = 16,
> + .offset = 16,
> + .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> + .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> + },
> + {
> + .id = V4L2_CID_PAN_RESET,
> + .entity = UVC_GUID_LOGITECH_PERIPHERAL,
> + .selector = 2,
> + .size = 1,
> + .offset = 0,
> + .v4l2_type = V4L2_CTRL_TYPE_BUTTON,
> + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> + },
> + {
> + .id = V4L2_CID_TILT_RESET,
> + .entity = UVC_GUID_LOGITECH_PERIPHERAL,
> + .selector = 2,
> + .size = 1,
> + .offset = 1,
> + .v4l2_type = V4L2_CTRL_TYPE_BUTTON,
> + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> + },
> + {
> + .id = V4L2_CID_FOCUS_ABSOLUTE,
> + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1,
> + .selector = 3,
> + .size = 8,
> + .offset = 0,
> + .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> + },
> };
>
> /* ------------------------------------------------------------------------
> diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> index ea92ac623a45..dea23aabbad4 100644
> --- a/include/linux/usb/uvc.h
> +++ b/include/linux/usb/uvc.h
> @@ -43,6 +43,12 @@
> #define UVC_GUID_MSXU_1_5 \
> {0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
> 0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
> +#define UVC_GUID_LOGITECH_MOTOR_CONTROL_V1 \
> + {0x82, 0x06, 0x61, 0x63, 0x70, 0x50, 0xab, 0x49, \
> + 0xb8, 0xcc, 0xb3, 0x85, 0x5e, 0x8d, 0x22, 0x56 }
> +#define UVC_GUID_LOGITECH_PERIPHERAL \
> + {0x21, 0x2d, 0xe5, 0xff, 0x30, 0x80, 0x2c, 0x4e, \
> + 0x82, 0xd9, 0xf5, 0x87, 0xd0, 0x05, 0x40, 0xbd }
>
> /* https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls */
> #define UVC_MSXU_CONTROL_FOCUS 0x01
>
^ permalink raw reply
* Re: [PATCH v3 2/4] media: uvcvideo: Announce deprecation intentions for UVCIOC_CTRL_MAP
From: Hans de Goede @ 2026-03-30 15:44 UTC (permalink / raw)
To: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab,
Greg Kroah-Hartman
Cc: linux-media, linux-kernel, linux-usb
In-Reply-To: <20260316-uvcdynctrl-v3-2-19cd4657e1f3@chromium.org>
Hi,
On 16-Mar-26 14:34, Ricardo Ribalda wrote:
> The UVCIOC_CTRL_MAP lets userspace create a mapping for a custom
> control.
>
> This mapping is usually created by the uvcdynctrl userspace utility. We
> would like to get the mappings into the driver instead.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Thanks, I've applied this to my local uvc/for-next branch and will
push this out to the public uvc repo for-next branch soon.
Regards,
Hans
> ---
> Documentation/userspace-api/media/drivers/uvcvideo.rst | 2 ++
> drivers/media/usb/uvc/uvc_v4l2.c | 4 ++++
> 2 files changed, 6 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/drivers/uvcvideo.rst b/Documentation/userspace-api/media/drivers/uvcvideo.rst
> index dbb30ad389ae..b09d2f8ba66e 100644
> --- a/Documentation/userspace-api/media/drivers/uvcvideo.rst
> +++ b/Documentation/userspace-api/media/drivers/uvcvideo.rst
> @@ -109,6 +109,8 @@ IOCTL reference
> UVCIOC_CTRL_MAP - Map a UVC control to a V4L2 control
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> +**This IOCTL is deprecated and will be eventually removed**
> +
> Argument: struct uvc_xu_control_mapping
>
> **Description**:
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index d5860661c115..f9049e9c0d3a 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -1046,6 +1046,8 @@ static long uvc_ioctl_default(struct file *file, void *priv, bool valid_prio,
> switch (cmd) {
> /* Dynamic controls. */
> case UVCIOC_CTRL_MAP:
> + pr_warn_once("uvcvideo: " DEPRECATED
> + "UVCIOC_CTRL_MAP ioctl will be eventually removed.\n");
> return uvc_ioctl_xu_ctrl_map(chain, arg);
>
> case UVCIOC_CTRL_QUERY:
> @@ -1160,6 +1162,8 @@ static long uvc_v4l2_compat_ioctl32(struct file *file,
>
> switch (cmd) {
> case UVCIOC_CTRL_MAP32:
> + pr_warn_once("uvcvideo: " DEPRECATED
> + "UVCIOC_CTRL_MAP32 ioctl will be eventually removed.\n");
> ret = uvc_v4l2_get_xu_mapping(&karg.xmap, up);
> if (ret)
> break;
>
^ permalink raw reply
* [PATCH] usb: misc: usbio: Fix URB memory leak on submit failure
From: Felix Gu @ 2026-03-30 15:47 UTC (permalink / raw)
To: Israel Cepeda, Hans de Goede, Sakari Ailus, Greg Kroah-Hartman
Cc: linux-usb, linux-kernel, Felix Gu
When usb_submit_urb() fails in usbio_probe(), the previously allocated
URB is never freed, causing a memory leak.
Add usb_free_urb() call to properly release the URB on the error path.
Fixes: 121a0f839dbb ("usb: misc: Add Intel USBIO bridge driver")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
drivers/usb/misc/usbio.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/misc/usbio.c b/drivers/usb/misc/usbio.c
index 2e68d48a2cc0..d4eb3c7bafc4 100644
--- a/drivers/usb/misc/usbio.c
+++ b/drivers/usb/misc/usbio.c
@@ -614,8 +614,10 @@ static int usbio_probe(struct usb_interface *intf, const struct usb_device_id *i
usb_fill_bulk_urb(usbio->urb, udev, usbio->rx_pipe, usbio->rxbuf,
usbio->rxbuf_len, usbio_bulk_recv, usbio);
ret = usb_submit_urb(usbio->urb, GFP_KERNEL);
- if (ret)
+ if (ret) {
+ usb_free_urb(usbio->urb);
return dev_err_probe(dev, ret, "Submitting usb urb\n");
+ }
mutex_lock(&usbio->ctrl_mutex);
---
base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
change-id: 20260330-usbio-d70e15c97a7a
Best regards,
--
Felix Gu <ustc.gu@gmail.com>
^ permalink raw reply related
* Re: [PATCH v3 3/4] media: uvcvideo: Introduce allow_privacy_override module parameter
From: Hans de Goede @ 2026-03-30 16:02 UTC (permalink / raw)
To: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab,
Greg Kroah-Hartman
Cc: linux-media, linux-kernel, linux-usb
In-Reply-To: <20260316-uvcdynctrl-v3-3-19cd4657e1f3@chromium.org>
Hi,
On 16-Mar-26 14:34, Ricardo Ribalda wrote:
> Some camera modules have XU controls that can configure the behaviour of
> the privacy LED.
>
> Block mapping of those controls, unless the module is configured with
> a new parameter: allow_privacy_override.
>
> This is just an interim solution. Based on the users feedback, we will
> either put the privacy controls behind a CONFIG option, or completely
> block them.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
I realize this patch is not without controversy, but I do believe
that this is an important step to safeguard the privacy of Logitech
webcam users.
Discussion in this thread has mentioned libusb / usbfs access as
a workaround. But that can be used to directly access usb-storage
devices directly without going through a filesystem and those
pesky filesystem permission checks, yet we do bother with those.
As with all things related to information security it is all
about defenense in depth and this just makes it that bit harder
for spyware to disable the privacy LED.
I do believe that the RFC Kconfig option likely is a bridge
too far. As discussed in the thread it will likely be at least
a year before many stable distro users actually see the change
adding this module parameter, so switching to permanently
disabling this by default through Kconfig in a year seems much
too soon and depending on the feedback we may end up sticking
with the module parameter and never permanently disabling this.
As such while merging this I've removed this paragraph from the commit
message:
"This is just an interim solution. Based on the users feedback, we will
either put the privacy controls behind a CONFIG option, or completely
block them."
and I've also removed the deprecation warning turning this into
a regular bool module parameter.
I've pushed this to the uvc git repo for-next branch now,
with the discussed changes squashed in.
Regards,
Hans
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 38 ++++++++++++++++++++++++++++++++++++++
> drivers/media/usb/uvc/uvc_driver.c | 20 ++++++++++++++++++++
> drivers/media/usb/uvc/uvc_v4l2.c | 7 +++++++
> drivers/media/usb/uvc/uvcvideo.h | 2 ++
> include/linux/usb/uvc.h | 4 ++++
> 5 files changed, 71 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index b6e020b41671..3ca108b83f1d 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -3001,6 +3001,35 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device *dev,
> return ret;
> }
>
> +bool uvc_ctrl_is_privacy_control(u8 entity[16], u8 selector)
> +{
> + /*
> + * This list is not exhaustive, it is a best effort to block access to
> + * non documented controls that can affect user's privacy.
> + */
> + struct privacy_control {
> + u8 entity[16];
> + u8 selector;
> + } privacy_control[] = {
> + {
> + .entity = UVC_GUID_LOGITECH_USER_HW_CONTROL_V1,
> + .selector = 1,
> + },
> + {
> + .entity = UVC_GUID_LOGITECH_PERIPHERAL,
> + .selector = 9,
> + },
> + };
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(privacy_control); i++)
> + if (!memcmp(entity, privacy_control[i].entity, 16) &&
> + selector == privacy_control[i].selector)
> + return true;
> +
> + return false;
> +}
> +
> int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
> struct uvc_xu_control_query *xqry)
> {
> @@ -3045,6 +3074,15 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
> return -ENOENT;
> }
>
> + if (uvc_ctrl_is_privacy_control(entity->guid, xqry->selector) &&
> + !uvc_allow_privacy_override_param) {
> + dev_warn_once(&chain->dev->intf->dev,
> + "Privacy related controls can only be accessed if module parameter allow_privacy_override is true\n");
> + uvc_dbg(chain->dev, CONTROL, "Blocking access to privacy related Control %pUl/%u\n",
> + entity->guid, xqry->selector);
> + return -EACCES;
> + }
> +
> if (mutex_lock_interruptible(&chain->ctrl_mutex))
> return -ERESTARTSYS;
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index b0ca81d924b6..74c9dea29d36 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -36,6 +36,7 @@ unsigned int uvc_no_drop_param = 1;
> static unsigned int uvc_quirks_param = -1;
> unsigned int uvc_dbg_param;
> unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT;
> +bool uvc_allow_privacy_override_param;
>
> static struct usb_driver uvc_driver;
>
> @@ -2505,6 +2506,25 @@ MODULE_PARM_DESC(trace, "Trace level bitmask");
> module_param_named(timeout, uvc_timeout_param, uint, 0644);
> MODULE_PARM_DESC(timeout, "Streaming control requests timeout");
>
> +static int param_set_privacy(const char *val, const struct kernel_param *kp)
> +{
> + pr_warn_once("uvcvideo: " DEPRECATED
> + "allow_privacy_override parameter will be eventually removed.\n");
> + return param_set_bool(val, kp);
> +}
> +
> +static const struct kernel_param_ops param_ops_privacy = {
> + .set = param_set_privacy,
> + .get = param_get_bool,
> +};
> +
> +param_check_bool(allow_privacy_override, &uvc_allow_privacy_override_param);
> +module_param_cb(allow_privacy_override, ¶m_ops_privacy,
> + &uvc_allow_privacy_override_param, 0644);
> +__MODULE_PARM_TYPE(allow_privacy_override, "bool");
> +MODULE_PARM_DESC(allow_privacy_override,
> + "Allow access to privacy related controls");
> +
> /* ------------------------------------------------------------------------
> * Driver initialization and cleanup
> */
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index f9049e9c0d3a..6d4f027c8402 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -133,6 +133,13 @@ static int uvc_ioctl_xu_ctrl_map(struct uvc_video_chain *chain,
> return -EINVAL;
> }
>
> + if (uvc_ctrl_is_privacy_control(xmap->entity, xmap->selector) &&
> + !uvc_allow_privacy_override_param) {
> + dev_warn_once(&chain->dev->intf->dev,
> + "Privacy related controls can only be mapped if module parameter allow_privacy_override is true\n");
> + return -EACCES;
> + }
> +
> map = kzalloc_obj(*map);
> if (map == NULL)
> return -ENOMEM;
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 8480d65ecb85..362110d58ca3 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -664,6 +664,7 @@ extern unsigned int uvc_no_drop_param;
> extern unsigned int uvc_dbg_param;
> extern unsigned int uvc_timeout_param;
> extern unsigned int uvc_hw_timestamps_param;
> +extern bool uvc_allow_privacy_override_param;
>
> #define uvc_dbg(_dev, flag, fmt, ...) \
> do { \
> @@ -794,6 +795,7 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
> struct uvc_xu_control_query *xqry);
>
> void uvc_ctrl_cleanup_fh(struct uvc_fh *handle);
> +bool uvc_ctrl_is_privacy_control(u8 entity[16], u8 selector);
>
> /* Utility functions */
> struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts,
> diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> index dea23aabbad4..70c2a7d25236 100644
> --- a/include/linux/usb/uvc.h
> +++ b/include/linux/usb/uvc.h
> @@ -49,6 +49,10 @@
> #define UVC_GUID_LOGITECH_PERIPHERAL \
> {0x21, 0x2d, 0xe5, 0xff, 0x30, 0x80, 0x2c, 0x4e, \
> 0x82, 0xd9, 0xf5, 0x87, 0xd0, 0x05, 0x40, 0xbd }
> +#define UVC_GUID_LOGITECH_USER_HW_CONTROL_V1 \
> + {0x82, 0x06, 0x61, 0x63, 0x70, 0x50, 0xab, 0x49, \
> + 0xb8, 0xcc, 0xb3, 0x85, 0x5e, 0x8d, 0x22, 0x1f }
> +
>
> /* https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls */
> #define UVC_MSXU_CONTROL_FOCUS 0x01
>
^ permalink raw reply
* Re: [PATCH v2] usbip: tools: Add usbip host driver availability check
From: Shuah Khan @ 2026-03-30 16:44 UTC (permalink / raw)
To: Zongmin Zhou
Cc: i, linux-kernel, linux-usb, valentina.manea.m, Zongmin Zhou,
Greg KH, Shuah Khan
In-Reply-To: <1667ddf5-e36c-4cba-87b4-9cd3d34032e1@163.com>
On 3/29/26 21:10, Zongmin Zhou wrote:
>
> On 2026/3/28 02:29, Shuah Khan wrote:
>> On 3/27/26 11:51, Shuah Khan wrote:
>>> On 3/27/26 02:39, Zongmin Zhou wrote:
>>>>
>>>> On 2026/3/27 02:43, Shuah Khan wrote:
>>>>> On 3/26/26 02:43, Greg KH wrote:
>>>
>>> [removed text]
>>>
>>>>>
>>>>> The problem Zongmin is trying fix ish when usbipd starts, it looks for
>>>>> exported if any - if it doesn't find any it assumes there aren't any
>>>>> exported and doesn't detect that usbip_host driver isn't loaded.
>>>>>
>>>>> refresh_exported_devices() doesn't have the logic and it shouldn't
>>>>> include that logic because this hook is called periodically to
>>>>> refresh the list of exported devices. Starting usbipd and loading
>>>>> the driver are distinct steps and without any dependencies.
>>>>>
>>>>> This patch he is trying to add that detection so a message can be printed
>>>>> to say "load the driver".
>>>>>
>>>>> A message can be added easily to cover two cases:
>>>>>
>>>>> 1. usbip_host driver isn't loaded
>>>>> 2. There are no exported devices.
>>>>>
>>>>> refresh_exported_devices() will not find any devices in both
>>>>> of the above scenarios. It isn't an error if it can't find
>>>>> any exported devices.
>>>>>
>>>>> An informational message when refresh_exported_devices()
>>>>> when it doesn't find any devices could help users.
>>>>>
>>>>> Zongmin,
>>>>>
>>>>> Would adding a message that says
>>>>> "Check if usbip_host driver is loaded or export devices"
>>>>> solve the problem of hard to debug problem you are addressing here?
>>>>>
>>>> Shuah,
>>>>
>>>> Your suggestion makes sense.
>>>> Adding an informational message when no devices are found would be a simple
>>>> and clean solution that helps users debug without being intrusive.
>>>>
>>>> However, I plan to add the info() message in usbip_generic_driver_open() instead of
>>>> refresh_exported_devices(), because:
>>>> - usbip_generic_driver_open() is called only once at initialization.
>>>> - refresh_exported_devices() is called periodically to refresh the exported device list.
>>>
>>> refresh_exported_devices() isn't called periodically - it is called
>>> from usbip_host_driver op: refresh_device_list and it will be called
>>> whenever usbipd (host side) calls it whenever it receives a request from
>>> user-space via process_request()
>>>
>>> For example running "usbip list -l" command will trigger a run of
>>> refresh_exported_devices() via usbip_host_driver op: refresh_device_list
>>>
>>> I don't think it will that noisy to add a message to refresh_exported_devices()
>>> when device list is zero. Currently the logic doesn't detect device list zero.
>>> You have add that condition to print informational message.
>>>
>>>
>>>> - When the server has no exported devices, having zero devices
>>>> is normal and not worth frequent info messages.
>>>
>>> That is correct. Zero exported devices isn't an error and this could
>>> persist until devices are exported.
>>>
>>>>
>>>> Theoretically, we only need to prompt once at startup. Is my understanding correct?> > I'll also remove the existing error messages like below,
>>>> since they cannot accurately determine whether the driver is loaded:
>>>>
>>>> if (ret)
>>>> err("please load " USBIP_CORE_MOD_NAME ".ko and "
>>>> USBIP_HOST_DRV_NAME ".ko!");
>>>
>>> Leave this one alone, because it gets called from a couple of places.
> In usbip_generic_driver_open(), the only path that triggers this message is a failure of udev_new().
> This function fails due to system-level issues like memory exhaustion, not because usbip driver module is missing.
>
> Furthermore, since refresh_exported_devices() doesn't practically return an error here,
> the message is never seen during actual driver loading failures.
> So I think it’s better to remove this inaccurate hint to avoid confusing the users.
>
> This is the reason why I previously wanted to remove it.
>>>
>>
>> Better yet, why not change the usbip instead - usbip_list for example.
>> This is the one that prints the device list and the change can be made
>> there when the list is zero to say, "Check if driver is loaded and
>> exported devices"
> I think placing the check/message in refresh_exported_devices() would be more effective.
> This function covers all scenarios where the device list is refreshed, including:
> usbipd startup, usbip list -r, and usbip attach operations.
Right changing refresh_exported_devices() covers all cases. Think about the
following scenarios:
- Where will this message get printed when usbipd is started on the console and in
background?
- Where will this message get printed when user runs various usbip list commands?
Ideally these commands should tell the user what happened without looking
at daemon logs.
thanks,
-- Shuah
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox