* Re: [PATCH v2] dt-bindings: connector: add pd-disable dependency
From: Rob Herring (Arm) @ 2026-03-31 15:46 UTC (permalink / raw)
To: Xu Yang
Cc: amitsd, kyletso, krzk+dt, badhri, conor+dt, imx, linux-usb,
linux-kernel, devicetree, gregkh
In-Reply-To: <20260330063518.719345-1-xu.yang_2@nxp.com>
On Mon, 30 Mar 2026 14:35:18 +0800, Xu Yang wrote:
> When Power Delivery is not supported, the source is unable to obtain the
> current capability from the Source PDO. As a result, typec-power-opmode
> needs to be added to advertise such capability.
>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Fixes: 7a4440bc0d86 ("dt-bindings: connector: Add pd-disable property")
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>
> ---
> Changes in v2:
> - add acked by tag
> - DTS patch has been picked by Frank
> ---
> Documentation/devicetree/bindings/connector/usb-connector.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH] usb: chipidea: udc: reject non-control requests while controller is suspended
From: Alan Stern @ 2026-03-31 15:39 UTC (permalink / raw)
To: Andreea.Popescu@aumovio.com
Cc: Peter Chen, Greg Kroah-Hartman, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <FRYP281MB261885D9851351180B1EC037EA53A@FRYP281MB2618.DEUP281.PROD.OUTLOOK.COM>
On Tue, Mar 31, 2026 at 12:21:45PM +0000, Andreea.Popescu@aumovio.com wrote:
> When Linux runtime PM autosuspends a ChipIdea UDC that is still
> enumerated by the host, the driver gates the PHY clocks and marks
> the controller as suspended (ci->in_lpm = 1) but deliberately leaves
> gadget.speed unchanged so upper-layer gadget drivers do not see a
> spurious disconnect.
>
> The problem is that those same drivers may continue to call
> usb_ep_queue() during the autosuspend window. _hardware_enqueue()
> silently adds the request to the endpoint queue and returns 0, but
> hw_ep_prime() cannot succeed with gated clocks, so the completion
> interrupt never fires. The request — and its backing buffer — is
> permanently lost. The caller sees a successful return and never
> frees the buffer.
Won't the request complete normally after the gadget is resumed, or
abnormally after a reset, disconnect, or shutdown? Either way, it
wouldn't be lost permanently.
Alan Stern
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] r8152: Add support for the RTL8157 hardware
From: Birger Koblitz @ 2026-03-31 15:38 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andrew+netdev, davem, edumazet, pabeni, linux-usb, netdev,
linux-kernel
In-Reply-To: <20260329193457.2764549-2-kuba@kernel.org>
On 3/29/26 21:34, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> r8152: Add support for the RTL8157 hardware
>
> This commit adds support for the RTL8157 chip, including its new 16-byte
> descriptor format, SRAM/ADV interfaces, and PHY setup. It reuses existing
> initialization functions where possible while branching for chip-specific
> logic.
>
>> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
>> index bab8e10e5f99..010126e6dae4 100644
>> --- a/drivers/net/usb/r8152.c
>> +++ b/drivers/net/usb/r8152.c
>> @@ -1629,6 +1862,28 @@ static u16 sram_read(struct r8152 *tp, u16 addr)
>> return ocp_reg_read(tp, OCP_SRAM_DATA);
>> }
>>
>> +static void sram_write_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
>> +{
>> + u16 data = sram_read(tp, addr);
>> +
>> + data = (data & ~clear) | set;
>> + ocp_reg_write(tp, OCP_SRAM_DATA, data);
>> +}
>
> Does this sequence write the modified data back to the wrong address?
>
> Indirect SRAM access ports typically auto-increment their internal address
> pointer upon read. Since sram_read() reads OCP_SRAM_DATA, the address
> pointer likely advances, causing the subsequent ocp_reg_write() to corrupt
> the adjacent register instead of the intended one. Should this use
> sram_write(tp, addr, data) to reset the address before writing?
>
The SRAM access does not have this alleged behaviour. This optimization is
found in the out-of-tree driver by Realtek. I also read back the values written
into the SRAM address as well as the following address, and the code is correct.
>
> Is the same auto-increment issue present here with OCP_SRAM2_DATA?
No. Again, the optimization is also found in the out-of-tree driver and I explicitly
verified the data written.
>
> [ ... ]
>
>> @@ -4080,6 +4473,14 @@ static void rtl8156_runtime_enable(struct r8152 *tp, bool enable)
>> }
>> }
>>
>> +static void rtl8157_runtime_enable(struct r8152 *tp, bool enable)
>> +{
>> + r8153_queue_wake(tp, enable);
>> + r8153b_u1u2en(tp, false);
>> + r8157_u2p3en(tp, false);
>> + rtl_runtime_suspend_enable(tp, enable);
>> +}
>
> Does this function break power management upon the first suspend and resume
> cycle? Unlike rtl8156_runtime_enable() which checks the enable parameter,
> this implementation unconditionally calls r8153b_u1u2en(tp, false) and
> r8157_u2p3en(tp, false) even when resuming.
This appears to be a bug in the out-of-tree driver. I will change this
function to follow the behaviour of rtl8156_runtime_enable() in v5. I have also traced
the PM functions while entering and leaving S3 sleep, and the behavior is now correct.
>
> [ ... ]
>
>> @@ -6693,7 +7052,7 @@ static void rtl8156_up(struct r8152 *tp)
>> r8153_aldps_en(tp, true);
>> r8153_u2p3en(tp, true);
>>
>> - if (tp->udev->speed >= USB_SPEED_SUPER)
>> + if (tp->version != RTL_VER_16 && tp->udev->speed >= USB_SPEED_SUPER)
>> r8153b_u1u2en(tp, true);
>> }
>
> Because r8153_u2p3en(tp, true) is called unconditionally just before the
> modified lines, will RTL8157 incorrectly write to the legacy USB_U2P3_CTRL
> register instead of using its specific r8157_u2p3en() function?
This was a mistake. I will fix this to call r8153_u2p3en() only for non-RTL8157
chips.
>
> [ ... ]
>
>> @@ -6706,13 +7065,13 @@ static void rtl8156_down(struct r8152 *tp)
>> return;
>> }
>>
>> - ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3);
>> - ocp_data |= PLA_MCU_SPDWN_EN;
>> - ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, ocp_data);
>> + ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, PLA_MCU_SPDWN_EN);
>>
>> r8153b_u1u2en(tp, false);
>> - r8153_u2p3en(tp, false);
>> - r8153b_power_cut_en(tp, false);
>> + if (tp->version != RTL_VER_16) {
>> + r8153_u2p3en(tp, false);
>> + r8153b_power_cut_en(tp, false);
>> + }
>> r8153_aldps_en(tp, false);
>
> While the legacy registers are correctly avoided for RTL_VER_16 here, are we
> missing calls to r8157_u2p3en(tp, false) and r8157_power_cut_en(tp, false)?
> If omitted, these power states might remain permanently enabled when the
> interface goes down.
These calls are not in the out-of-tree driver. It appears the r8157_u2p3en()
is not used in the _up and _down functions. I measured the USB power
consumption with an interposed USB dongle on the port, and the power use is
better than for the RTL8153 when the link is down.
>
> [ ... ]
>
>> @@ -8362,10 +8825,21 @@ static void r8156b_init(struct r8152 *tp)
>>
>> set_bit(GREEN_ETHERNET, &tp->flags);
>>
>> - /* rx aggregation */
>> - ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_USB_CTRL);
>> - ocp_data &= ~(RX_AGG_DISABLE | RX_ZERO_EN);
>> - ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
>> + /* rx aggregation / 16 bytes Rx descriptor */
>> + if (tp->version == RTL_VER_16)
>> + ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL, RX_AGG_DISABLE | RX_DESC_16B);
>
> Is it intended to clear the RX_DESC_16B bit for RTL_VER_16? The driver uses
> sizeof(struct rx_desc_v2) which is a 16-byte descriptor for RTL8157.
> Clearing this bit might configure the hardware to send 8-byte descriptors
> instead, leading to packet parsing corruption in rx_bottom().
The code is correct as per the out-of-tree driver. I also tested packet
transmission in various stress-tests.
>
> [ ... ]
>
>> @@ -9647,6 +10158,29 @@ static int rtl_ops_init(struct r8152 *tp)
>> tp->rx_buf_sz = 32 * 1024;
>> tp->eee_en = true;
>> tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
>> + r8152_desc_init(tp);
>> + break;
>> +
>> + case RTL_VER_16:
>> + tp->eee_en = true;
>> + tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
>> + tp->eee_adv2 = MDIO_EEE_2_5GT | MDIO_EEE_5GT;
>> + ops->init = r8156_init;
>> + ops->enable = rtl8156_enable;
>> + ops->disable = rtl8153_disable;
>> + ops->up = rtl8156_up;
>> + ops->down = rtl8156_down;
>> + ops->unload = rtl8153_unload;
>
> Will using rtl8153_unload for RTL_VER_16 result in incorrect power cut
> teardown? rtl8153_unload calls the legacy r8153_power_cut_en(tp, false).
> RTL8157 seems to require r8157_power_cut_en(tp, false) to properly clear
> USB_MISC_2 bit 1 and PCUT_STATUS on module unload.
This behaviour is also found in the out-of-tree driver by Realtek.
The out-of-tree function reads:
static void rtl8157_unload(struct r8152 *tp)
{
if (test_bit(RTL8152_UNPLUG, &tp->flags))
return;
r8153_power_cut_en(tp, false);
/* Disable Interrupt Mitigation */
if (ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xcf04,
BIT(0) | BIT(1) | BIT(2) | BIT(7)) < 0)
return;
}
However, I had forgotten to add the "Disable Interrupt Mitigation" part.
I therefore added it, depending on RTL_VER_16.
I tested unloading and re-loading the driver, again while measuring the USB power
consumption with an interposed USB dongle on the port, and the wattage
in the state where the driver is unloaded for the RTL8157 is lower than the
one for the RTL8153 (280mW vs 450mW). The driver behavior now appear
correct.
^ permalink raw reply
* Re: [PATCH net-next v4 1/2] r8152: Add support for 5Gbit Link Speeds and EEE
From: Birger Koblitz @ 2026-03-31 15:38 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andrew+netdev, davem, edumazet, pabeni, linux-usb, netdev,
linux-kernel
In-Reply-To: <20260329193449.2764517-1-kuba@kernel.org>
Hi Jakub,
thanks a lot for reviewing the patch!
On 3/29/26 21:34, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> r8152: Add support for 5Gbit Link Speeds and EEE
>
> This commit adds support for 5GBit link speeds and EEE to the RTL8157
> driver. It updates the speed and duplex settings, ethtool integration,
> and EEE advertisement parameters to handle the new 5G capabilities.
>
>> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
>> index 3b6d4252d34c..bab8e10e5f99 100644
>> --- a/drivers/net/usb/r8152.c
>> +++ b/drivers/net/usb/r8152.c
>> @@ -604,6 +604,7 @@ enum spd_duplex {
>> FORCE_100M_FULL,
>> FORCE_1000M_FULL,
>> NWAY_2500M_FULL,
>> + NWAY_5000M_FULL,
>> };
>
> Are there missing switch cases for NWAY_5000M_FULL in the hardware UPS
> flag configurations, such as r8156_ups_flags?
This question has now been raised the third time, see here:
https://lkml.org/lkml/2026/3/24/1902 and here https://lkml.org/lkml/2026/3/19/1765
However, while I share the concern, I cannot provide a better answer than
the first time or second time.
> Is it intentional that the MAC idle speed down feature (IDLE_SPDWN_EN) is
> disabled for _2500bps but remains enabled for _5000bps?
Yes, this is intentional. IDLE_SPDWN_EN is indeed only disabled for _2500bps
in the out-of-tree driver (and not for 5GBit or 10GBit).
>
>> @@ -6343,15 +6356,20 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
>> r8152_mdio_write(tp, MII_CTRL1000, new1);
>> }
>>
>> - if (tp->support_2500full) {
>> + if (tp->support_2500full || tp->support_5000full) {
>> orig = ocp_reg_read(tp, OCP_10GBT_CTRL);
>> - new1 = orig & ~MDIO_AN_10GBT_CTRL_ADV2_5G;
>> + new1 = orig & ~(MDIO_AN_10GBT_CTRL_ADV2_5G | MDIO_AN_10GBT_CTRL_ADV5G);
>>
>> if (advertising & RTL_ADVERTISED_2500_FULL) {
>> new1 |= MDIO_AN_10GBT_CTRL_ADV2_5G;
>> tp->ups_info.speed_duplex = NWAY_2500M_FULL;
>> }
>>
>> + if (advertising & RTL_ADVERTISED_5000_FULL) {
>> + new1 |= MDIO_AN_10GBT_CTRL_ADV5G;
>> + tp->ups_info.speed_duplex = NWAY_5000M_FULL;
>> + }
>> +
>
> Could this unintentionally advertise 5GbE on a 2.5GbE-only adapter if
> userspace requests it?
>
> The earlier check in this function:
>
> if (!(advertising & support))
> return -EINVAL;
>
> only ensures that at least one supported speed is requested, but it does
> not strip unsupported speeds from the advertising mask. Since this block
> is now entered when support_2500full is true, checking the advertising mask
> for RTL_ADVERTISED_5000_FULL without verifying tp->support_5000full could
> erroneously write the 5G bit to the PHY and misconfigure the hardware
> state.
I will change this to:
advertising &= support;
if (!advertising)
return -EINVAL;
in v5, to be on the safe side.
> [ ... ]
>
>> @@ -8793,6 +8814,19 @@ int rtl8152_get_link_ksettings(struct net_device *netdev,
>> cmd->base.speed = SPEED_2500;
>> }
>>
>> + if (tp->support_5000full) {
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
>> + cmd->link_modes.advertising,
>> + ocp_reg_read(tp, OCP_10GBT_CTRL) & MDIO_AN_10GBT_CTRL_ADV5G);
>> +
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
>> + cmd->link_modes.lp_advertising,
>> + ocp_reg_read(tp, OCP_10GBT_STAT) & MDIO_AN_10GBT_STAT_LP5G);
>> +
>> + if (is_speed_5000(rtl8152_get_speed(tp)))
>> + cmd->base.speed = SPEED_5000;
>> + }
>
> Since 5G adapters like the RTL8157 will have both support_2500full and
> support_5000full set, will this result in duplicate blocking USB control
> transfers?
You are right. Also in view of future 10GBit support, I will change this to:
if (tp->support_2500full || tp->support_5000full) {
u16 ocp_10bt_ctrl = ocp_reg_read(tp, OCP_10GBT_CTR)L;
u16 ocp_10bt_stat = ocp_reg_read(tp, OCP_10GBT_STAT);
if (tp->support_2500full) {
...
}
if (tp->support_5000full) {
...
}
}
> In r8153_get_eee, the reading of OCP_EEE_ADV2 and OCP_EEE_LPABLE2 is
> performed inside the if (tp->support_2500full) block that precedes this new
> code.
>
> Since those registers contain the EEE capability bits for both 2.5G
> and 5G speeds, what happens if a device has support_5000full enabled but
> support_2500full disabled? It looks like the registers would never be read,
> and the 5Gbps EEE capabilities would fail to populate correctly.
The driver supports RTL815x-chips, and all RTL815x-chips, which support 5GBit also
support 2.5gbit (the RTL8157 and the RTL8159). My understanding is, that the
802.3bz standard even requires this fallback. However, to be on the safe side and
to also support possible future chips, I will change this to:
if (tp->support_2500full || tp->support_5000full) {
val = ocp_reg_read(tp, OCP_EEE_ADV2);
mii_eee_cap2_mod_linkmode_adv_t(eee->advertised, val);
val = ocp_reg_read(tp, OCP_EEE_LPABLE2);
mii_eee_cap2_mod_linkmode_adv_t(eee->lp_advertised, val);
}
if (tp->support_2500full) {
linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, eee->supported);
if (speed & _2500bps)
linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, common);
}
if (tp->support_5000full) {
linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, eee->supported);
if (speed & _5000bps)
linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, common);
}
Cheers,
Birger
^ permalink raw reply
* Re: [RFC PATCH 1/2] xhci: prevent automatic endpoint restart after stall or error
From: stern @ 2026-03-31 15:31 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: <54121929-d775-45a0-b31d-09b783aada5d@linux.intel.com>
On Tue, Mar 31, 2026 at 12:34:54PM +0300, Mathias Nyman wrote:
> On 3/30/26 17:17, stern@rowland.harvard.edu wrote:
> > 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.
>
> The URB submit buffering in core would only take place if endpoint is halted
> with EPIPE/EPROTO.
>
> Additional latency should be limited to the time between interrupt handler
> returns a URB with EPIPE/EPROTO, and the bh workqueue finishing urb->complete()
> for that URB
>
> URBs would normally be sent to HCD directly.
>
> Yes, this might end up being quite complex, need to make sure it solves more
> issues than it creates.
Can that be done precisely? I doubt it -- there are races between the
core and the HCD and between the HCD and the hardware.
As for doing it imprecisely... I'm not sure the advantages outweigh the
complexity. Anyway, if the class driver is well behaved, this buffering
won't be needed.
> Usb core is aware of the halted endpoint before the class driver due to the bh
> workqueue giveback. To me it would make sense to make core responsible for babysitting
> the class driver urb submission for the time it withholds this information.
>
> Whole reason for this is to prevent new URB submission from HCD restarting an endpoint
> after HCD gave back a halted URB, and HCD assuming core/class drivers are aware of
> the halt when the new URB is submitted.
>
> Other option is that usb core would just refuse class driver from submitting URBs
> during this time. usb_submit_urb() would return with an error, but I think this might
> impact existing class drivers more.
How about this instead? We add a "halted" flag to the usb_host_endpoint
structure, and the core will set this flag whenever a bulk or interrupt
URB gets a status other than 0 (before putting the URB on the bh list).
If an URB has one of these statuses, when its completion handler returns
the core will unlink all the URBs queued to the same endpoint. Finally,
the "halted" flag should be cleared after a completion handler returns
if there are no more unlinked URBs still in the queue or URBs waiting on
the bh list to be given back.
The result of this is that any URB remaining in the queue when the flag
is cleared must have been submitted by the class driver _after_ the
failing URB's completion handler has run. We can assume the class
driver knows what it's doing in this case.
The endpoint queue shouldn't be restarted until the "halted" flag is
cleared. Either right away, if there are any URBs in the queue, or not
until the next URB is submitted. Doing this might require a new HCD
callback. (It would also mean the kerneldoc for usb_unlink_urb() would
need to be updated, because the endpoint might restart before all the
completion handlers for the unlinked URBs have run.)
What I'm trying to do here is come up with a single, consistent proposal
for exactly when halted endpoint queues should restart. Maybe someone
else has a better suggestion.
We also should add a special case for control endpoints because their
halt conditions can get cleared automatically when the device receives a
SETUP packet.
> > 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).
>
> My take is that endpoint should stop processing URBs, existing pending URBs
> should be retired by class/core, new URBs should restart the endpoint but new URBs
> are only permitted _after_ submitter is aware of the halt.
Yes, that's more or less what my proposal attempts to accomplish.
> A class driver testing USB specification should be able to resubmit a stalled URB
> (EPIPE) and expect it to complete with EPIPE again until it clears the halt.
Agreed.
> > 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?
>
> The disable/enable interface and set config calls during unbind/bind should,
> if I remember correctly flush pending URBs and drop and re-add the endpoint,
> clearing the xhci side halt and reset toggle.
usb_probe_interface() doesn't do any of that stuff, other than a
deferred switch to altsetting 0 if needed.
usb_unbind_interface() does call usb_enable_interface() if the interface
is already in altsetting 0, but the reset_ep argument is false so the
endpoint state doesn't get affected. Should that be changed?
Alan Stern
^ permalink raw reply
* [PATCH RFC] usb: typec: bus: Don't set state->alt when exiting altmode
From: Konrad Dybcio @ 2026-03-31 15:20 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: linux-usb, linux-kernel, usb4-upstream, Konrad Dybcio
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Currently, typec_altmode_enter/exit() and typec_remove() make a call to
typec_altmode_set_state(adev, TYPEC_STATE_SAFE, NULL) in order to
configure the chained muxes/retimers appropriately.
The drivers for those retimers and muxes however, almost unilaterally
trust that if state->alt is set, we're *entering* an alternate mode.
This may lead to NULL pointer dereferences, since most drivers would
then check the (still set) state->alt->svid and proceed to dereference
state->data. which is NULL in this call site. This on top of the fact
that the SAFE configuration wouldn't take place due to th
aforementioned assumption.
Stop sending these mixed signals.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Sending this as an RFC. The other option would be to go through all
the mux/retimer drivers and add additional if (state >= MODAL) checks.
LMK which one is preferred.
---
drivers/usb/typec/bus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
index e84b134a3381..3423f6f35b22 100644
--- a/drivers/usb/typec/bus.c
+++ b/drivers/usb/typec/bus.c
@@ -21,7 +21,7 @@ typec_altmode_set_retimer(struct altmode *alt, unsigned long conf, void *data)
if (!alt->retimer)
return 0;
- state.alt = &alt->adev;
+ state.alt = conf >= TYPEC_STATE_MODAL ? &alt->adev : NULL;
state.mode = conf;
state.data = data;
@@ -36,7 +36,7 @@ typec_altmode_set_mux(struct altmode *alt, unsigned long conf, void *data)
if (!alt->mux)
return 0;
- state.alt = &alt->adev;
+ state.alt = conf >= TYPEC_STATE_MODAL ? &alt->adev : NULL;
state.mode = conf;
state.data = data;
---
base-commit: e77a5a5cfe43b4c25bd44a3818e487033287517f
change-id: 20260331-topic-usb_mux_null-2cf93662d48e
Best regards,
--
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
^ permalink raw reply related
* Re: [PATCH] usb: xhci: use BIT macro
From: kernel test robot @ 2026-03-31 14:22 UTC (permalink / raw)
To: Oliver Neukum, mathias.nyman, gregkh, linux-usb
Cc: oe-kbuild-all, Oliver Neukum
In-Reply-To: <20260312150649.2138749-1-oneukum@suse.com>
Hi Oliver,
kernel test robot noticed the following build warnings:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus linus/master v7.0-rc6 next-20260330]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Oliver-Neukum/usb-xhci-use-BIT-macro/20260314-143843
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20260312150649.2138749-1-oneukum%40suse.com
patch subject: [PATCH] usb: xhci: use BIT macro
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20260331/202603312226.4Rkqoq6u-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260331/202603312226.4Rkqoq6u-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603312226.4Rkqoq6u-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/usb/host/xhci.c:25:
drivers/usb/host/xhci.h: In function 'xhci_decode_slot_context':
>> drivers/usb/host/xhci.h:2321:57: warning: format '%d' expects argument of type 'int', but argument 7 has type 'long unsigned int' [-Wformat=]
2321 | ret = sprintf(str, "RS %05x %s%s%s Ctx Entries %d MEL %d us Port# %d/%d",
| ~^
| |
| int
| %ld
......
2345 | (info & LAST_CTX_MASK) >> 27,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| long unsigned int
--
In file included from drivers/usb/cdns3/host.c:18:
drivers/usb/cdns3/../host/xhci.h: In function 'xhci_decode_slot_context':
>> drivers/usb/cdns3/../host/xhci.h:2321:57: warning: format '%d' expects argument of type 'int', but argument 7 has type 'long unsigned int' [-Wformat=]
2321 | ret = sprintf(str, "RS %05x %s%s%s Ctx Entries %d MEL %d us Port# %d/%d",
| ~^
| |
| int
| %ld
......
2345 | (info & LAST_CTX_MASK) >> 27,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| long unsigned int
vim +2321 drivers/usb/host/xhci.h
90d6d5731da79a Mathias Nyman 2019-04-26 2308
4843b4b5ec64b8 Mathias Nyman 2021-08-20 2309 static inline const char *xhci_decode_slot_context(char *str,
4843b4b5ec64b8 Mathias Nyman 2021-08-20 2310 u32 info, u32 info2, u32 tt_info, u32 state)
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2311 {
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2312 u32 speed;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2313 u32 hub;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2314 u32 mtt;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2315 int ret = 0;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2316
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2317 speed = info & DEV_SPEED;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2318 hub = info & DEV_HUB;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2319 mtt = info & DEV_MTT;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2320
19a7d0d65c4a81 Felipe Balbi 2017-04-07 @2321 ret = sprintf(str, "RS %05x %s%s%s Ctx Entries %d MEL %d us Port# %d/%d",
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2322 info & ROUTE_STRING_MASK,
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2323 ({ char *s;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2324 switch (speed) {
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2325 case SLOT_SPEED_FS:
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2326 s = "full-speed";
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2327 break;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2328 case SLOT_SPEED_LS:
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2329 s = "low-speed";
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2330 break;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2331 case SLOT_SPEED_HS:
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2332 s = "high-speed";
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2333 break;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2334 case SLOT_SPEED_SS:
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2335 s = "super-speed";
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2336 break;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2337 case SLOT_SPEED_SSP:
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2338 s = "super-speed plus";
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2339 break;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2340 default:
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2341 s = "UNKNOWN speed";
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2342 } s; }),
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2343 mtt ? " multi-TT" : "",
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2344 hub ? " Hub" : "",
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2345 (info & LAST_CTX_MASK) >> 27,
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2346 info2 & MAX_EXIT,
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2347 DEVINFO_TO_ROOT_HUB_PORT(info2),
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2348 DEVINFO_TO_MAX_PORTS(info2));
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2349
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2350 ret += sprintf(str + ret, " [TT Slot %d Port# %d TTT %d Intr %d] Addr %d State %s",
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2351 tt_info & TT_SLOT, (tt_info & TT_PORT) >> 8,
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2352 GET_TT_THINK_TIME(tt_info), GET_INTR_TARGET(tt_info),
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2353 state & DEV_ADDR_MASK,
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2354 xhci_slot_state_string(GET_SLOT_STATE(state)));
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2355
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2356 return str;
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2357 }
19a7d0d65c4a81 Felipe Balbi 2017-04-07 2358
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [usb:usb-linus] BUILD SUCCESS e367599529dc42578545a7f85fde517b35b3cda7
From: kernel test robot @ 2026-03-31 13:04 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-usb
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus
branch HEAD: e367599529dc42578545a7f85fde517b35b3cda7 usb: gadget: f_rndis: Fix net_device lifecycle with device_move
elapsed time: 755m
configs tested: 175
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc-15.2.0
alpha allyesconfig gcc-15.2.0
alpha defconfig gcc-15.2.0
arc allmodconfig clang-16
arc allmodconfig gcc-15.2.0
arc allnoconfig gcc-15.2.0
arc allyesconfig clang-23
arc defconfig gcc-15.2.0
arc randconfig-001-20260331 clang-23
arc randconfig-002-20260331 clang-23
arm allnoconfig gcc-15.2.0
arm allyesconfig clang-16
arm allyesconfig gcc-15.2.0
arm defconfig gcc-15.2.0
arm randconfig-001-20260331 clang-23
arm randconfig-002-20260331 clang-23
arm randconfig-003-20260331 clang-23
arm randconfig-004-20260331 clang-23
arm64 allmodconfig clang-23
arm64 allnoconfig gcc-15.2.0
arm64 defconfig gcc-15.2.0
arm64 randconfig-001-20260331 clang-18
arm64 randconfig-002-20260331 clang-18
arm64 randconfig-003-20260331 clang-18
arm64 randconfig-004-20260331 clang-18
csky allmodconfig gcc-15.2.0
csky allnoconfig gcc-15.2.0
csky defconfig gcc-15.2.0
csky randconfig-001-20260331 clang-18
csky randconfig-002-20260331 clang-18
hexagon allmodconfig clang-17
hexagon allmodconfig gcc-15.2.0
hexagon allnoconfig gcc-15.2.0
hexagon defconfig gcc-15.2.0
hexagon randconfig-001-20260331 gcc-11.5.0
hexagon randconfig-002-20260331 gcc-11.5.0
i386 allmodconfig clang-20
i386 allnoconfig gcc-15.2.0
i386 allyesconfig clang-20
i386 buildonly-randconfig-001-20260331 clang-20
i386 buildonly-randconfig-002-20260331 clang-20
i386 buildonly-randconfig-003-20260331 clang-20
i386 buildonly-randconfig-004-20260331 clang-20
i386 buildonly-randconfig-005-20260331 clang-20
i386 buildonly-randconfig-006-20260331 clang-20
i386 defconfig gcc-15.2.0
i386 randconfig-001-20260331 gcc-14
i386 randconfig-002-20260331 gcc-14
i386 randconfig-003-20260331 gcc-14
i386 randconfig-004-20260331 gcc-14
i386 randconfig-005-20260331 gcc-14
i386 randconfig-006-20260331 gcc-14
i386 randconfig-007-20260331 gcc-14
i386 randconfig-011-20260331 clang-20
i386 randconfig-012-20260331 clang-20
i386 randconfig-013-20260331 clang-20
i386 randconfig-014-20260331 clang-20
i386 randconfig-015-20260331 clang-20
i386 randconfig-016-20260331 clang-20
i386 randconfig-017-20260331 clang-20
loongarch allmodconfig clang-23
loongarch allnoconfig gcc-15.2.0
loongarch defconfig clang-19
loongarch randconfig-001-20260331 gcc-11.5.0
loongarch randconfig-002-20260331 gcc-11.5.0
m68k allmodconfig gcc-15.2.0
m68k allnoconfig gcc-15.2.0
m68k allyesconfig clang-16
m68k allyesconfig gcc-15.2.0
m68k defconfig clang-19
microblaze allnoconfig gcc-15.2.0
microblaze allyesconfig gcc-15.2.0
microblaze defconfig clang-19
mips allmodconfig gcc-15.2.0
mips allnoconfig gcc-15.2.0
mips allyesconfig gcc-15.2.0
mips ip32_defconfig clang-23
nios2 allmodconfig clang-23
nios2 allnoconfig clang-23
nios2 defconfig clang-19
nios2 randconfig-001-20260331 gcc-11.5.0
nios2 randconfig-002-20260331 gcc-11.5.0
openrisc allmodconfig clang-23
openrisc allnoconfig clang-23
openrisc defconfig gcc-15.2.0
parisc allmodconfig gcc-15.2.0
parisc allnoconfig clang-23
parisc allyesconfig clang-19
parisc defconfig gcc-15.2.0
parisc randconfig-001-20260331 clang-23
parisc randconfig-002-20260331 clang-23
parisc64 defconfig clang-19
powerpc allmodconfig gcc-15.2.0
powerpc allnoconfig clang-23
powerpc pcm030_defconfig clang-23
powerpc randconfig-001-20260331 clang-23
powerpc randconfig-002-20260331 clang-23
powerpc64 alldefconfig clang-23
powerpc64 randconfig-001-20260331 clang-23
powerpc64 randconfig-002-20260331 clang-23
riscv allmodconfig clang-23
riscv allnoconfig clang-23
riscv allyesconfig clang-16
riscv defconfig gcc-15.2.0
riscv randconfig-001-20260331 gcc-15.2.0
riscv randconfig-002-20260331 gcc-15.2.0
s390 allmodconfig clang-19
s390 allnoconfig clang-23
s390 allyesconfig gcc-15.2.0
s390 defconfig gcc-15.2.0
s390 randconfig-001-20260331 gcc-15.2.0
s390 randconfig-002-20260331 gcc-15.2.0
sh allmodconfig gcc-15.2.0
sh allnoconfig clang-23
sh allyesconfig clang-19
sh defconfig gcc-14
sh randconfig-001-20260331 gcc-15.2.0
sh randconfig-002-20260331 gcc-15.2.0
sparc allnoconfig clang-23
sparc defconfig gcc-15.2.0
sparc randconfig-001-20260331 gcc-15.2.0
sparc randconfig-002-20260331 gcc-15.2.0
sparc64 allmodconfig clang-23
sparc64 defconfig gcc-14
sparc64 randconfig-001-20260331 gcc-15.2.0
sparc64 randconfig-002-20260331 gcc-15.2.0
um allmodconfig clang-19
um allnoconfig clang-23
um allyesconfig gcc-14
um allyesconfig gcc-15.2.0
um defconfig gcc-14
um i386_defconfig gcc-14
um randconfig-001-20260331 gcc-15.2.0
um randconfig-002-20260331 gcc-15.2.0
um x86_64_defconfig gcc-14
x86_64 allmodconfig clang-20
x86_64 allnoconfig clang-23
x86_64 allyesconfig clang-20
x86_64 buildonly-randconfig-001-20260331 clang-20
x86_64 buildonly-randconfig-002-20260331 clang-20
x86_64 buildonly-randconfig-003-20260331 clang-20
x86_64 buildonly-randconfig-004-20260331 clang-20
x86_64 buildonly-randconfig-005-20260331 clang-20
x86_64 buildonly-randconfig-006-20260331 clang-20
x86_64 defconfig gcc-14
x86_64 kexec clang-20
x86_64 randconfig-001-20260331 gcc-14
x86_64 randconfig-002-20260331 gcc-14
x86_64 randconfig-003-20260331 gcc-14
x86_64 randconfig-004-20260331 gcc-14
x86_64 randconfig-005-20260331 gcc-14
x86_64 randconfig-006-20260331 gcc-14
x86_64 randconfig-011-20260331 clang-20
x86_64 randconfig-012-20260331 clang-20
x86_64 randconfig-013-20260331 clang-20
x86_64 randconfig-014-20260331 clang-20
x86_64 randconfig-015-20260331 clang-20
x86_64 randconfig-016-20260331 clang-20
x86_64 randconfig-071-20260331 clang-20
x86_64 randconfig-072-20260331 clang-20
x86_64 randconfig-073-20260331 clang-20
x86_64 randconfig-074-20260331 clang-20
x86_64 randconfig-075-20260331 clang-20
x86_64 randconfig-076-20260331 clang-20
x86_64 rhel-9.4 clang-20
x86_64 rhel-9.4-bpf gcc-14
x86_64 rhel-9.4-func clang-20
x86_64 rhel-9.4-kselftests clang-20
x86_64 rhel-9.4-kunit gcc-14
x86_64 rhel-9.4-ltp gcc-14
x86_64 rhel-9.4-rust clang-20
xtensa allnoconfig clang-23
xtensa allyesconfig clang-23
xtensa randconfig-001-20260331 gcc-15.2.0
xtensa randconfig-002-20260331 gcc-15.2.0
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v3 1/1] usb: dwc3: Add optional VBUS regulator support to SpacemiT K1
From: Chukun Pan @ 2026-03-31 13:02 UTC (permalink / raw)
To: troy.mitchell
Cc: Thinh.Nguyen, amadeus, dlan, gregkh, huang.ze, linux-kernel,
linux-riscv, linux-usb, spacemit
In-Reply-To: <5E6BDCE16DD4ADB0+acuJBfKF-SCAOu2_@kernel.org>
Hi,
> > + if (usb_get_dr_mode(dev) == USB_DR_MODE_HOST) {
> The logic looks good, but the structure can be further improved to better align with
> standard kernel coding style:
>
> if (usb_get_dr_mode(dev) != USB_DR_MODE_HOST)
> return 0;
I initially thought of writing it this way too.
> > + int ret = devm_regulator_get_enable_optional(dev, "vbus");
> Could you please move the declaration of ret to the top of the function?
Thank you for your suggestion. This patch has been merged.
Perhaps it can be revised in the future.
Thanks,
Chukun
^ permalink raw reply
* Re: [PATCH v9 3/6] mfd: max77759: add register bitmasks and modify irq configs for charger
From: Lee Jones @ 2026-03-31 12:31 UTC (permalink / raw)
To: amitsd
Cc: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
André Draszik, Greg Kroah-Hartman, Badhri Jagan Sridharan,
Heikki Krogerus, Peter Griffin, Tudor Ambarus, Alim Akhtar,
Mark Brown, Matti Vaittinen, Andrew Morton, linux-kernel,
linux-pm, devicetree, linux-usb, linux-arm-kernel,
linux-samsung-soc, RD Babiera, Kyle Tso
In-Reply-To: <20260325-max77759-charger-v9-3-4486dd297adc@google.com>
On Wed, 25 Mar 2026, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
>
> Add register bitmasks for charger function.
> In addition split the charger IRQs further such that each bit represents
> an IRQ downstream of charger regmap irq chip. In addition populate the
> ack_base to offload irq ack to the regmap irq chip framework.
>
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> Reviewed-by: André Draszik <andre.draszik@linaro.org>
> ---
> drivers/mfd/max77759.c | 95 ++++++++++++++++++++++---
> include/linux/mfd/max77759.h | 166 +++++++++++++++++++++++++++++++++++--------
> 2 files changed, 222 insertions(+), 39 deletions(-)
>
[...]
> +/*
> + * enum max77759_chgr_chgin_dtls_status - Charger Input Status
> + * @MAX77759_CHGR_CHGIN_DTLS_VBUS_UNDERVOLTAGE:
> + * Charger input voltage (Vchgin) < Under Voltage Threshold (Vuvlo)
> + * @MAX77759_CHGR_CHGIN_DTLS_VBUS_MARGINAL_VOLTAGE: Vchgin > Vuvlo and
> + * Vchgin < (Battery Voltage (Vbatt) + system voltage (Vsys))
> + * @MAX77759_CHGR_CHGIN_DTLS_VBUS_OVERVOLTAGE:
> + * Vchgin > Over Voltage threshold (Vovlo)
> + * @MAX77759_CHGR_CHGIN_DTLS_VBUS_VALID:
> + * Vchgin > Vuvlo, Vchgin < Vovlo and Vchgin > (Vsys + Vbatt)
> + */
This comment is masquerading as a kernel-doc header, but isn't actually
kernel-doc. Either change the formatting or adapt the formatting to use
/** and use W=1 to check it.
> +enum max77759_chgr_chgin_dtls_status {
> + MAX77759_CHGR_CHGIN_DTLS_VBUS_UNDERVOLTAGE,
> + MAX77759_CHGR_CHGIN_DTLS_VBUS_MARGINAL_VOLTAGE,
> + MAX77759_CHGR_CHGIN_DTLS_VBUS_OVERVOLTAGE,
> + MAX77759_CHGR_CHGIN_DTLS_VBUS_VALID,
> +};
--
Lee Jones [李琼斯]
^ permalink raw reply
* [PATCH] usb: chipidea: udc: reject non-control requests while controller is suspended
From: Andreea.Popescu @ 2026-03-31 12:21 UTC (permalink / raw)
To: Peter Chen, Greg Kroah-Hartman
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
When Linux runtime PM autosuspends a ChipIdea UDC that is still
enumerated by the host, the driver gates the PHY clocks and marks
the controller as suspended (ci->in_lpm = 1) but deliberately leaves
gadget.speed unchanged so upper-layer gadget drivers do not see a
spurious disconnect.
The problem is that those same drivers may continue to call
usb_ep_queue() during the autosuspend window. _hardware_enqueue()
silently adds the request to the endpoint queue and returns 0, but
hw_ep_prime() cannot succeed with gated clocks, so the completion
interrupt never fires. The request — and its backing buffer — is
permanently lost. The caller sees a successful return and never
frees the buffer.
The attached patch fixes this by adding an early -ESHUTDOWN return
in _ep_queue() for any non-control endpoint when ci->in_lpm is set
or when gadget.speed == USB_SPEED_UNKNOWN (cable gone / soft
disconnect). It also corrects ep_queue(), the public usb_ep_ops
wrapper, which was returning 0 instead of -ESHUTDOWN for the
USB_SPEED_UNKNOWN case, masking the same class of bug.
The guard follows the same pattern already used in ci_irq_handler(),
which exits early on ci->in_lpm, and has been verified against v6.19
ci.h (in_lpm field present) and core.c (ci_controller_suspend() sets
in_lpm without touching gadget.speed).
Please consider routing to stable; the bug has been present since
runtime PM autosuspend was introduced to this driver.
Thanks,
Andreea Popescu
---
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 64a421ae0f05..09bdd78826b6 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1100,6 +1100,27 @@ static int _ep_queue(struct usb_ep *ep, struct usb_request *req,
if (ep == NULL || req == NULL || hwep->ep.desc == NULL)
return -EINVAL;
+ /*
+ * Reject non-control submissions when the controller is suspended
+ * (ci->in_lpm set by ci_runtime_suspend) or the gadget is not yet
+ * enumerated (gadget.speed == USB_SPEED_UNKNOWN).
+ *
+ * ci_runtime_suspend gates the PHY clocks but intentionally leaves
+ * gadget.speed at its current value so that upper-layer drivers do
+ * not see a spurious disconnect. As a side-effect, callers have no
+ * way to distinguish an active gadget from a clock-gated one.
+ * _hardware_enqueue() adds the request to hwep->qh.queue and returns
+ * 0, but hw_ep_prime() is a no-op with gated clocks so the transfer
+ * completion interrupt never fires and the buffer is permanently lost.
+ *
+ * Return -ESHUTDOWN so callers can free the buffer immediately,
+ * matching the behaviour of a real hardware error completion.
+ */
+ if (hwep->type != USB_ENDPOINT_XFER_CONTROL &&
+ (ci->in_lpm || ci->gadget.speed == USB_SPEED_UNKNOWN))
+ return -ESHUTDOWN;
+
+
if (hwep->type == USB_ENDPOINT_XFER_CONTROL) {
if (req->length)
hwep = (ci->ep0_dir == RX) ?
@@ -1698,7 +1719,7 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req,
spin_lock_irqsave(hwep->lock, flags);
if (hwep->ci->gadget.speed == USB_SPEED_UNKNOWN) {
spin_unlock_irqrestore(hwep->lock, flags);
- return 0;
+ return -ESHUTDOWN;
}
retval = _ep_queue(ep, req, gfp_flags);
spin_unlock_irqrestore(hwep->lock, flags);
^ permalink raw reply related
* Re: [PATCH v2] usb: misc: usbio: Fix URB memory leak on submit failure
From: Oliver Neukum @ 2026-03-31 12:15 UTC (permalink / raw)
To: Felix Gu, Israel Cepeda, Hans de Goede, Sakari Ailus,
Greg Kroah-Hartman
Cc: linux-usb, linux-kernel
In-Reply-To: <20260331-usbio-v2-1-d8c48dad9463@gmail.com>
On 31.03.26 14:05, Felix Gu wrote:
> When usb_submit_urb() fails in usbio_probe(), the previously allocated
> URB is never freed, causing a memory leak.
>
> Fix this by jumping to err_free_urb label 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>
Reviewed-by: Oliver Neukum <oneukum@suse.com>
^ permalink raw reply
* [PATCH v2] usb: misc: usbio: Fix URB memory leak on submit failure
From: Felix Gu @ 2026-03-31 12:05 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.
Fix this by jumping to err_free_urb label 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>
---
Changes in v2:
- Fix Oliver's comment.
- Link to v1: https://lore.kernel.org/lkml/20260330-usbio-v1-1-7141b6dc612a@gmail.com
---
drivers/usb/misc/usbio.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/misc/usbio.c b/drivers/usb/misc/usbio.c
index 2e68d48a2cc0..02d1e0760f0c 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)
- return dev_err_probe(dev, ret, "Submitting usb urb\n");
+ if (ret) {
+ dev_err_probe(dev, ret, "Submitting usb urb\n");
+ goto err_free_urb;
+ }
mutex_lock(&usbio->ctrl_mutex);
@@ -663,6 +665,7 @@ static int usbio_probe(struct usb_interface *intf, const struct usb_device_id *i
err_unlock:
mutex_unlock(&usbio->ctrl_mutex);
usb_kill_urb(usbio->urb);
+err_free_urb:
usb_free_urb(usbio->urb);
return ret;
---
base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
change-id: 20260330-usbio-d70e15c97a7a
Best regards,
--
Felix Gu <ustc.gu@gmail.com>
^ permalink raw reply related
* [PATCH RFC] xhci: fix Command Aborting, diffferently
From: Michal Pecio @ 2026-03-31 11:46 UTC (permalink / raw)
To: Niklas Neronin; +Cc: mathias.nyman, linux-usb
In-Reply-To: <20260316142720.1471906-2-niklas.neronin@linux.intel.com>
OK, so here's the alternative approach I suggested: don't try to avoid
CRCR corruption but simply fix it later.
Motivation:
1. Inserting udelay() between low and high DWORD write reliably
corrupts internal CR Dequeue pointer on some HW.
2. It's unclear if such corruption happens in the field, but surely
nobody will prove that it can't.
3. Low DWORD gets corrupted even if both DWORDs are written with
their proper values on some HW.
4. It is indeed a violation of 5.1 to write high DWORD without writing
the low DWORD immediately prior. The earlier abort doesn't count.
5. Delaying high DWORD until we are sure low DWORD wasn't enough slows
us down on some HW (and currently disables IRQs for a long time).
6. Need to support 64 bit HCs on systems without writeq() somehow.
Solution:
We can't just write arbitrary deq pointer to CRCR, but we can create
a single-TRB segment which links to our arbitrary command.
TODO:
Make sure new deq and cycle are correct under all circumstances.
Don't panic if we get an event on the trampoline TRB (can it happen?)
Be sure nothing weird happens due to unexpected preexisting bugs -
that whole command management logic is a little scary.
Potential other use:
Avoid the need for wiping the command ring on suspend/resume. Allow
commands to survive the cycle and execute later. Unsure if useful.
Regards,
Michal
---
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 98ef014c8dee..2dabf295a8bc 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -486,6 +486,28 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci,
}
}
+/*
+ * CRCR doesn't support setting arbitrary Dequeue, so write it
+ * to a suitably aligned Link TRB and point the ring there.
+ * The ring must be stopped and not restarted until this returns.
+ */
+static void xhci_set_cr_deq(struct xhci_hcd *xhci, dma_addr_t deq, u32 cycle)
+{
+ u64 cr;
+ dma_addr_t trampoline_dma = xhci->dcbaa->dma +
+ offsetof(struct xhci_device_context_array, cr_trampoline);
+
+ xhci->dcbaa->cr_trampoline.segment_ptr = deq;
+ xhci->dcbaa->cr_trampoline.control = cpu_to_le32(TRB_TYPE(TRB_LINK) | cycle);
+
+ cr = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
+ cr &= CMD_RING_RSVDP;
+ cr |= trampoline_dma | cycle;
+ xhci_write_64(xhci, cr, &xhci->op_regs->cmd_ring);
+
+ xhci_info(xhci, "xhci_set_cr_deq deq %llx\n", deq);
+}
+
/* Must be called with xhci->lock held, releases and acquires lock back */
static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, unsigned long flags)
{
@@ -493,6 +515,7 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, unsigned long flags)
union xhci_trb *new_deq = xhci->cmd_ring->dequeue;
u64 crcr;
int ret;
+ u64 ns1, ns2;
xhci_dbg(xhci, "Abort command ring\n");
@@ -502,16 +525,15 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, unsigned long flags)
* The control bits like command stop, abort are located in lower
* dword of the command ring control register.
* Some controllers require all 64 bits to be written to abort the ring.
- * Make sure the upper dword is valid, pointing to the next command,
- * avoiding corrupting the command ring pointer in case the command ring
- * is stopped by the time the upper dword is written.
+ * This may corrupt *both* DWORDS of Dequeue if high DWORD write is seen
+ * after CRR=0 by some other controllers. We will fix it up.
*/
next_trb(&new_seg, &new_deq);
if (trb_is_link(new_deq))
next_trb(&new_seg, &new_deq);
-
crcr = xhci_trb_virt_to_dma(new_seg, new_deq);
- xhci_write_64(xhci, crcr | CMD_RING_ABORT, &xhci->op_regs->cmd_ring);
+
+ xhci_write_64(xhci, CMD_RING_ABORT, &xhci->op_regs->cmd_ring);
/* Section 4.6.1.2 of xHCI 1.0 spec says software should also time the
* completion of the Command Abort operation. If CRR is not negated in 5
@@ -519,14 +541,21 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, unsigned long flags)
* In the future we should distinguish between -ENODEV and -ETIMEDOUT
* and try to recover a -ETIMEDOUT with a host controller reset.
*/
+ ns1 = ktime_get_ns();
ret = xhci_handshake(&xhci->op_regs->cmd_ring,
CMD_RING_RUNNING, 0, 5 * 1000 * 1000);
+ ns2 = ktime_get_ns();
+ xhci_info(xhci, "abort took %lldns\n", ns2 - ns1);
if (ret < 0) {
xhci_err(xhci, "Abort failed to stop command ring: %d\n", ret);
xhci_halt(xhci);
xhci_hc_died(xhci);
return ret;
}
+
+ /* fixme: cycle may be wrong, not sure about crcr */
+ xhci_set_cr_deq(xhci, crcr, xhci->cmd_ring->cycle_state);
+
/*
* Writing the CMD_RING_ABORT bit should cause a cmd completion event,
* however on some host hw the CMD_RING_RUNNING bit is correctly cleared
@@ -1817,6 +1846,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
trace_xhci_handle_command(xhci->cmd_ring, &cmd_trb->generic, cmd_dma);
cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status));
+ xhci_info(xhci, "handle_cmd_completion cmd_dma %llx cmd_comp_code %d\n", cmd_dma, cmd_comp_code);
/* If CMD ring stopped we own the trbs between enqueue and dequeue */
if (cmd_comp_code == COMP_COMMAND_RING_STOPPED) {
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 32617dc155ac..40069a2c7bf9 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -193,6 +193,7 @@ struct xhci_op_regs {
#define CMD_RING_ABORT (1 << 2)
/* true: command ring is running */
#define CMD_RING_RUNNING (1 << 3)
+#define CMD_RING_RSVDP GENMASK_ULL(5, 4)
/* bits 63:6 - Command Ring pointer */
#define CMD_RING_PTR_MASK GENMASK_ULL(63, 6)
@@ -789,22 +790,6 @@ struct xhci_tt_bw_info {
};
-/**
- * struct xhci_device_context_array
- * @dev_context_ptr array of 64-bit DMA addresses for device contexts
- */
-struct xhci_device_context_array {
- /* 64-bit device addresses; we only write 32-bit addresses */
- __le64 dev_context_ptrs[MAX_HC_SLOTS];
- /* private xHCD pointers */
- dma_addr_t dma;
-};
-/*
- * TODO: change this to be dynamically sized at HC mem init time since the HC
- * might not be able to handle the maximum number of devices possible.
- */
-
-
struct xhci_transfer_event {
/* 64-bit buffer address, or immediate data */
__le64 buffer;
@@ -956,6 +941,24 @@ struct xhci_link_trb {
/* control bitfields */
#define LINK_TOGGLE (0x1<<1)
+/**
+ * struct xhci_device_context_array
+ * @dev_context_ptr array of 64-bit DMA addresses for device contexts
+ */
+struct xhci_device_context_array {
+ /* 64-bit device addresses; we only write 32-bit addresses */
+ __le64 dev_context_ptrs[MAX_HC_SLOTS];
+ /* a Link TRB to jump into arbitrary command ring slot */
+ struct xhci_link_trb cr_trampoline __aligned(sizeof(64));
+ /* private xHCD pointers */
+ dma_addr_t dma;
+};
+/*
+ * TODO: change this to be dynamically sized at HC mem init time since the HC
+ * might not be able to handle the maximum number of devices possible.
+ */
+
+
/* Command completion event TRB */
struct xhci_event_cmd {
/* Pointer to command TRB, or the value passed by the event data trb */
^ permalink raw reply related
* Re: [PATCH 9/9] usb: xhci: optimize resuming from S4 (suspend-to-disk)
From: Neronin, Niklas @ 2026-03-31 9:59 UTC (permalink / raw)
To: Michal Pecio; +Cc: mathias.nyman, linux-usb, raoxu
In-Reply-To: <20260330114510.2b1e5f05.michal.pecio@gmail.com>
On 30/03/2026 12.45, Michal Pecio wrote:
>
>>
>> - xhci_dbg(xhci, "// Disabling event ring interrupts\n");
>> - temp = readl(&xhci->op_regs->status);
>> - writel((temp & ~0x1fff) | STS_EINT, &xhci->op_regs->status);
>> - xhci_disable_interrupter(xhci, xhci->interrupters[0]);
>> + cancel_delayed_work_sync(&xhci->cmd_timer);
>> +
>> + /* Delete all remaining commands */
>> + xhci_cleanup_command_queue(xhci);
>
> Considering that xhci_suspend() clears the command ring anyway, it
> could probably do this too so we don't need to.
It makes more sense to have all clearing in one place, instead of spread
out over suspend and resume. This will be addressed in the next patch set,
i.e. remove clearing from suspend (if possible).
>
> BTW, debugfs/port_bandwidth interface queues commands and I'm not sure
> if it's synchronized in any way with power management. IOW, it might be
> possible that command are pending at suspend, but I'm not sure.
>
>> +
>> + /* Clear data which is re-initilized during runtime */
>> + xhci_for_each_ring_seg(xhci->interrupters[0]->event_ring->first_seg, seg)
>> + memset(seg->trbs, 0, sizeof(union xhci_trb) * TRBS_PER_SEGMENT);
>> +
>> + for (int i = xhci->max_slots; i > 0; i--)
>> + xhci_free_virt_devices_depth_first(xhci, i);
>
> This loop is a bit ugly, it could be a function in xhci-mem.
Will be addressed in xhci_free_virt_devices_depth_first() rework.
>
>> +
>> + xhci_rh_bw_cleanup(xhci);
>> +
>> + xhci->cmd_ring_reserved_trbs = 0;
>> + xhci_for_each_ring_seg(xhci->cmd_ring->first_seg, seg)
>> + memset(seg->trbs, 0, sizeof(union xhci_trb) * TRBS_PER_SEGMENT);
>
> This looks like a bug because it nukes link TRBs. I know that
> xhci_init() will fix this up (unless somebody changes that without
> updating here), but it looks confusing.
I would like to remove xhci_clear_command_ring() eventually,
instead have:
xhci_ring_reset()
...
xhci_set_cmd_ring_deq()
or
xhci_ring_clear()
...
xhci_ring_init()
xhci_set_cmd_ring_deq()
Then xhci_ring_*() functions can be used for all rings,
and now it is clear that the CRP is set.
Thanks,
Niklas
^ permalink raw reply
* [PATCH v3] usbip: tools: add hint when no exported devices are found
From: Zongmin Zhou @ 2026-03-31 9:58 UTC (permalink / raw)
To: skhan
Cc: gregkh, i, linux-kernel, linux-usb, min_halo, valentina.manea.m,
Zongmin Zhou
In-Reply-To: <3ff00e2e-1c45-45be-a65c-16da9b2ae5a5@linuxfoundation.org>
From: Zongmin Zhou <zhouzongmin@kylinos.cn>
When refresh_exported_devices() finds no devices, it's helpful to
inform users about potential causes. This could be due to:
1. The usbip driver module is not loaded.
2. No devices have been exported yet.
Add an informational message to guide users when ndevs == 0.
Message visibility by scenario:
- usbipd (console mode): Show on console/serial, this allows instant
visibility for debugging.
- usbipd -D (daemon mode): Message logged to syslog, can keep logs for
later traceability in production. Also can use "journalctl -f" to
trace on console.
Suggested-by: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
---
Changes in v3:
- Just add an informational message when no devices are found.
Changes in v2:
- Use system calls directly instead of checking sysfs dir.
tools/usb/usbip/libsrc/usbip_host_common.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c
index ca78aa368476..cd92baee310c 100644
--- a/tools/usb/usbip/libsrc/usbip_host_common.c
+++ b/tools/usb/usbip/libsrc/usbip_host_common.c
@@ -149,6 +149,13 @@ static int refresh_exported_devices(struct usbip_host_driver *hdriver)
}
}
+ if (hdriver->ndevs == 0) {
+ if (!strcmp(hdriver->udev_subsystem, "usb"))
+ info("Please check if %s driver is loaded or export devices.",USBIP_HOST_DRV_NAME);
+ else
+ info("Please check if %s driver is loaded or export devices.",USBIP_DEVICE_DRV_NAME);
+ }
+
return 0;
}
--
2.34.1
^ permalink raw reply related
* Re: [RFC PATCH 1/2] xhci: prevent automatic endpoint restart after stall or error
From: Mathias Nyman @ 2026-03-31 9:34 UTC (permalink / raw)
To: stern@rowland.harvard.edu
Cc: Thinh Nguyen, linux-usb@vger.kernel.org, michal.pecio@gmail.com,
oneukum@suse.com, niklas.neronin@linux.intel.com
In-Reply-To: <9e62ef5a-5b31-4fca-891b-d028f5bbfc05@rowland.harvard.edu>
On 3/30/26 17:17, stern@rowland.harvard.edu wrote:
> 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.
The URB submit buffering in core would only take place if endpoint is halted
with EPIPE/EPROTO.
Additional latency should be limited to the time between interrupt handler
returns a URB with EPIPE/EPROTO, and the bh workqueue finishing urb->complete()
for that URB
URBs would normally be sent to HCD directly.
Yes, this might end up being quite complex, need to make sure it solves more
issues than it creates.
Usb core is aware of the halted endpoint before the class driver due to the bh
workqueue giveback. To me it would make sense to make core responsible for babysitting
the class driver urb submission for the time it withholds this information.
Whole reason for this is to prevent new URB submission from HCD restarting an endpoint
after HCD gave back a halted URB, and HCD assuming core/class drivers are aware of
the halt when the new URB is submitted.
Other option is that usb core would just refuse class driver from submitting URBs
during this time. usb_submit_urb() would return with an error, but I think this might
impact existing class drivers more.
>
> 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).
My take is that endpoint should stop processing URBs, existing pending URBs
should be retired by class/core, new URBs should restart the endpoint but new URBs
are only permitted _after_ submitter is aware of the halt.
A class driver testing USB specification should be able to resubmit a stalled URB
(EPIPE) and expect it to complete with EPIPE again until it clears the halt.
>
> 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?
The disable/enable interface and set config calls during unbind/bind should,
if I remember correctly flush pending URBs and drop and re-add the endpoint,
clearing the xhci side halt and reset toggle.
>
> 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?
xhci-hcd will queue the new URB and restart the ring, if device side endpoint
remains halted due to uncleared halt then transfer stalls and URB is returned
with EPIPE.
Thanks
Mathias
^ permalink raw reply
* Re: [PATCH 3/9] usb: xhci: factor out roothub bandwidth cleanup
From: Neronin, Niklas @ 2026-03-31 9:34 UTC (permalink / raw)
To: Michal Pecio; +Cc: mathias.nyman, linux-usb, raoxu
In-Reply-To: <20260330102910.0059972c.michal.pecio@gmail.com>
On 30/03/2026 11.29, Michal Pecio wrote:
>> +/* Cleanup roothub bandwidth data */
>> +static void xhci_rh_bw_cleanup(struct xhci_hcd *xhci)
>> +{
>> + struct xhci_root_port_bw_info *rh_bw;
>> + struct xhci_tt_bw_info *tt_info, *tt_next;
>> + struct list_head *eps, *ep, *ep_next;
>> +
>> + for (int i = 0; i < xhci->max_ports; i++) {
>> + rh_bw = &xhci->rh_bw[i];
>> +
>> + /* Clear and free all TT bandwidth entries */
>> + list_for_each_entry_safe(tt_info, tt_next, &rh_bw->tts, tt_list) {
>> + list_del(&tt_info->tt_list);
>> + kfree(tt_info);
>> + }
>
> This loop is theoretically pointless, because each tt_info corresponds
> to a hub virtual device, and all this stuff should have been destroyed
> by xhci_free_virt_devices_depth_first() earlier.
>
> If anything, it seems to paper over potential memory leaks in there.
>
>> +
>> + /* Clear per-interval endpoint lists */
>> + for (int j = 0; j < XHCI_MAX_INTERVAL; j++) {
>> + eps = &rh_bw->bw_table.interval_bw[j].endpoints;
>> +
>> + list_for_each_safe(ep, ep_next, eps)
>> + list_del_init(ep);
>> + }
>
> This loop used to run before xhci_free_virt_devices_depth_first(), but
> now it will run after. It seems that the endpoints here are virt_ep
> which also should be gone already, so this loop likely does nothing
> (empty list) or writes to virtual devices after free (somebody forgot
> to unlink some endpoints from the list).
In my testing, when xhci_rh_bw_cleanup() is called after
xhci_free_virt_devices_depth_first() in xhci_resume(), all related
resources have already been freed.
That said, I have chosen to keep the existing freeing in this patch set.
Removing it would introduce an additional behavioral change and a
potential regression point, which I prefer to avoid at this stage.
>
> Do we trust xhci_free_virt_devices_depth_first() to work correctly?
> If yes then it seems this whole function is unnecessary.
I don't, mostly because it is recursive and complex. Which makes it
difficult to follow edge case issues (IMO).
>
> If not, perhaps delete this monstrosity and write a simpler cleanup:
Planning to rework xhci_free_virt_devices_depth_first() in a later patch
set, which will then result in the removal of xhci_rh_bw_cleanup().
>
> 1. for each slot_id
> - disable debugfs
> - free virt_device and child allocations but don't worry about tt_info
> 2. for each root hub port
> - free all tt_info allocations but don't worry about eps or vdevs
>
> And since this will be used by reset on resume:
>
> 3. zero out DCBAA and xhci->devs
> 4. reinitialize xhci->rb_hw
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: usb: dwc3-xilinx: Add MMI USB support on Versal Gen2 platform
From: Pandey, Radhey Shyam @ 2026-03-31 9:18 UTC (permalink / raw)
To: Krzysztof Kozlowski, Radhey Shyam Pandey
Cc: gregkh, robh, krzk+dt, conor+dt, michal.simek, Thinh.Nguyen,
p.zabel, linux-usb, devicetree, linux-arm-kernel, linux-kernel,
git
In-Reply-To: <20260331-jellyfish-of-pragmatic-prowess-a230fc@quoll>
> On Tue, Mar 31, 2026 at 12:33:01AM +0530, Radhey Shyam Pandey wrote:
>> Versal Gen2 platform multimedia integrated (MMI) module has a USB3.2 Gen
>> 2x1 Dual Role Device IP. Introduce a new compatibility string to support
>> it. The USB wrapper registers reside in the MMI UDH system-level control
>> registers (SLCR) block, so instead of a dedicated reg property, add
>> xlnx,usb-syscon phandle with four cells specifying register offsets for
>> USB2 PHY, USB3 PHY, USB DRD, and USB power configuration within the SLCR.
>>
>> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
>> ---
>> Changes for v2:
>> - Add blank line after compatible as suggested by Krzysztof.
>> - Retain the mmi suffix in the compatible string, as this USB 3.2 Gen2
>> IP from Synopsys is part of the dedicated Multimedia Interface. The
>> Versal Gen2 platform also includes a separate USB 2.0 controller,
>> and the mmi suffix uniquely distinguishes between the two USB
>> controllers. MMI is an independent subsystem particularly targeted for
>> deployment in Multi-Media related applications. The MMI block include
>> following submodules: UDH: USB3.2 Gen 2x1 Dual Role Device, DisplayPort
>> Transmit Controller, Security Module (ESM) for DisplayPort and HDMI
>> Controllers, DP AUX-I2C PHY.
>> - For MMI USB define parent address space i.e UDH block.
>> - Fix inconsistent MHz spacing to use SI convention with spaces.
>> - Move description before $ref and items in xlnx,usb-syscon property.
>> - Restore original zynqmp-dwc3 example, add new versal2-mmi-dwc3 example.
>> - Use 'usb' node name (without unit address) for versal2 example since
>> it has no reg property.
>> - Use 1/1 address/size configuration in versal2 example, use lowercase
>> hex in syscon offsets.
>> ---
>> .../devicetree/bindings/usb/dwc3-xilinx.yaml | 70 ++++++++++++++++++-
>> 1 file changed, 67 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml b/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml
>> index d6823ef5f9a7..5e31b961aff7 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml
>> +++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml
>> @@ -15,6 +15,8 @@ properties:
>> - enum:
>> - xlnx,zynqmp-dwc3
>> - xlnx,versal-dwc3
>> + - xlnx,versal2-mmi-dwc3
> I am not going to ask the same questions.
I have provided the explanation in v2 changelog.
Retain the mmi suffix in the compatible string, as this USB 3.2 Gen2
IP from Synopsys is part of the dedicated Multimedia Interface. The
Versal Gen2 platform also includes a separate USB 2.0 controller,
and the mmi suffix uniquely distinguishes between the two USB
controllers.
Let me know if you still have reservation in this approach (using
<vendor>,<soc>-<subsystem>-<ip>). Or any other alternative
based on IP version etc please suggest.
Thanks,
Radhey
^ permalink raw reply
* Re: [PATCH net-next v4 0/3] r8152: add helper functions for PLA/USB/PHY OCP registers
From: patchwork-bot+netdevbpf @ 2026-03-31 8:50 UTC (permalink / raw)
To: Chih Kai Hsu
Cc: kuba, davem, netdev, nic_swsd, linux-kernel, linux-usb, edumazet,
bjorn, pabeni
In-Reply-To: <20260326073925.32976-453-nic_swsd@realtek.com>
Hello:
This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Thu, 26 Mar 2026 15:39:22 +0800 you wrote:
> v4:
> - Add a new patch for original #1. We need to fix a typo first before
> original #1 patch.
>
> - For patch #2, replace redundant ocp_word_set_bits() with ocp_word_w0w1().
>
> v3:
> - For patch #1, fix the issue identified by the AI review.
> In v2, ocp_byte_set_bits() was used to update the USB_LPM_CTRL register.
> However, since its function only perform a bitwise OR operaion, it fails
> to clear the old bits when updating a multi-bit field like LPM_TIMER.
> Therefore, use ocp_write_byte() to maintain the original direct-write
> semantics.
>
> [...]
Here is the summary with links:
- [net-next,v4,1/3] r8152: fix incorrect register write to USB_UPHY_XTAL
https://git.kernel.org/netdev/net-next/c/48afd5124fd6
- [net-next,v4,2/3] r8152: add helper functions for PLA/USB OCP registers
https://git.kernel.org/netdev/net-next/c/d16133f177fe
- [net-next,v4,3/3] r8152: add helper functions for PHY OCP registers
https://git.kernel.org/netdev/net-next/c/7db154aa58e1
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v3 1/1] usb: dwc3: Add optional VBUS regulator support to SpacemiT K1
From: Troy Mitchell @ 2026-03-31 8:42 UTC (permalink / raw)
To: Chukun Pan, Thinh Nguyen
Cc: Yixun Lan, Ze Huang, Greg Kroah-Hartman, linux-riscv,
linux-kernel, linux-usb, spacemit, Troy Mitchell
In-Reply-To: <20260326100010.3588454-2-amadeus@jmu.edu.cn>
On Thu, Mar 26, 2026 at 18:00:10 CST, Chukun Pan wrote:
> Some SpacemiT K1 boards (like OrangePi R2S) provide USB VBUS
> through a controllable regulator. Add support for the optional
> vbus-supply property so the regulator can be properly managed
> in host mode instead of left always-on. Note that this doesn't
> apply to USB Hub downstream ports with different VBUS supplies.
>
> The enabled and disabled actions of the regulator are handled
> automatically by devm_regulator_get_enable_optional().
>
> Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
> ---
> drivers/usb/dwc3/dwc3-generic-plat.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-generic-plat.c b/drivers/usb/dwc3/dwc3-generic-plat.c
> index e846844e0023..64f5e9f20663 100644
> --- a/drivers/usb/dwc3/dwc3-generic-plat.c
> +++ b/drivers/usb/dwc3/dwc3-generic-plat.c
> @@ -12,6 +12,8 @@
> #include <linux/reset.h>
> #include <linux/regmap.h>
> #include <linux/mfd/syscon.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/usb/otg.h>
> #include "glue.h"
>
> #define EIC7700_HSP_BUS_FILTER_EN BIT(0)
> @@ -69,6 +71,20 @@ static int dwc3_eic7700_init(struct dwc3_generic *dwc3g)
> return 0;
> }
>
> +static int dwc3_spacemit_k1_init(struct dwc3_generic *dwc3g)
> +{
> + struct device *dev = dwc3g->dev;
> +
> + if (usb_get_dr_mode(dev) == USB_DR_MODE_HOST) {
The logic looks good, but the structure can be further improved to better align with
standard kernel coding style:
if (usb_get_dr_mode(dev) != USB_DR_MODE_HOST)
return 0;
> + int ret = devm_regulator_get_enable_optional(dev, "vbus");
Could you please move the declaration of ret to the top of the function?
Otherwise, LGTM.
Reviewed-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
^ permalink raw reply
* RE: [PATCH net-next v4 3/3] r8152: add helper functions for PHY OCP registers
From: Hayes Wang @ 2026-03-31 8:24 UTC (permalink / raw)
To: Chih Kai Hsu, Jakub Kicinski
Cc: davem@davemloft.net, netdev@vger.kernel.org, nic_swsd,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
edumazet@google.com, bjorn@mork.no, pabeni@redhat.com
In-Reply-To: <0fb2fd8f26334b578c299ee71958202c@realtek.com>
> -----Original Message-----
> From: Chih Kai Hsu <hsu.chih.kai@realtek.com>
> Sent: Tuesday, March 31, 2026 1:46 PM
> To: Jakub Kicinski <kuba@kernel.org>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; nic_swsd
> <nic_swsd@realtek.com>; linux-kernel@vger.kernel.org;
> linux-usb@vger.kernel.org; edumazet@google.com; bjorn@mork.no;
> pabeni@redhat.com
> Subject: RE: [PATCH net-next v4 3/3] r8152: add helper functions for PHY OCP
> registers
>
> On 31/03/26 10:02 am, Jakub Kicinski wrote:
> >
> > > diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index
> > > 32a4e8d423115..8747c55e0a484 100644
> > > --- a/drivers/net/usb/r8152.c
> > > +++ b/drivers/net/usb/r8152.c
> > [ ... ]
> > > +static void sram_write_w0w1(struct r8152 *tp, u16 addr, u16 clear,
> > > +u16 set) {
> > > + u16 data;
> > > +
> > > + data = sram_read(tp, addr);
> > > + data = (data & ~clear) | set;
> > > + ocp_reg_write(tp, OCP_SRAM_DATA, data);
> >
> > AI flags:
> >
> > Does writing directly to OCP_SRAM_DATA here actually write to the
> > intended
> > address?
> Yes, this actually writes to the intended address.
>
> >
> > Because hardware indirect SRAM access ports typically auto-increment
> their
> > internal address pointer upon access, the sram_read() call will advance
> the
> > pointer. By writing directly to OCP_SRAM_DATA instead of using
> > sram_write(tp, addr, data) to re-establish the address, the updated value
> > would be written to the next address (addr + 1) rather than the original
> addr.
> >
> > Could this corrupt the PHY configuration memory across the
> > initialization paths
> > that now use sram_clr_bits() and sram_set_bits()?
> >
> > Can you confirm if this is legit? Not sure whether this device /
> > register auto-increments or where AI got it from.
> The address pointer, OCP_SRAM_ADDR, is not auto-incremented after
> sram_read(tp, addr).
> However, the address pointer is incremented automatically after
> ocp_reg_write(tp, OCP_SRAM_DATA, data).
That's strange - I didn't receive the previous email from Jakub Kicinski.
Also, the answer above is correct.
Best Regards,
Hayes
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: usb: dwc3-xilinx: Add MMI USB support on Versal Gen2 platform
From: Krzysztof Kozlowski @ 2026-03-31 8:25 UTC (permalink / raw)
To: Radhey Shyam Pandey
Cc: gregkh, robh, krzk+dt, conor+dt, michal.simek, Thinh.Nguyen,
p.zabel, linux-usb, devicetree, linux-arm-kernel, linux-kernel,
git
In-Reply-To: <20260330190304.1841593-2-radhey.shyam.pandey@amd.com>
On Tue, Mar 31, 2026 at 12:33:01AM +0530, Radhey Shyam Pandey wrote:
> Versal Gen2 platform multimedia integrated (MMI) module has a USB3.2 Gen
> 2x1 Dual Role Device IP. Introduce a new compatibility string to support
> it. The USB wrapper registers reside in the MMI UDH system-level control
> registers (SLCR) block, so instead of a dedicated reg property, add
> xlnx,usb-syscon phandle with four cells specifying register offsets for
> USB2 PHY, USB3 PHY, USB DRD, and USB power configuration within the SLCR.
>
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> Changes for v2:
> - Add blank line after compatible as suggested by Krzysztof.
> - Retain the mmi suffix in the compatible string, as this USB 3.2 Gen2
> IP from Synopsys is part of the dedicated Multimedia Interface. The
> Versal Gen2 platform also includes a separate USB 2.0 controller,
> and the mmi suffix uniquely distinguishes between the two USB
> controllers. MMI is an independent subsystem particularly targeted for
> deployment in Multi-Media related applications. The MMI block include
> following submodules: UDH: USB3.2 Gen 2x1 Dual Role Device, DisplayPort
> Transmit Controller, Security Module (ESM) for DisplayPort and HDMI
> Controllers, DP AUX-I2C PHY.
> - For MMI USB define parent address space i.e UDH block.
> - Fix inconsistent MHz spacing to use SI convention with spaces.
> - Move description before $ref and items in xlnx,usb-syscon property.
> - Restore original zynqmp-dwc3 example, add new versal2-mmi-dwc3 example.
> - Use 'usb' node name (without unit address) for versal2 example since
> it has no reg property.
> - Use 1/1 address/size configuration in versal2 example, use lowercase
> hex in syscon offsets.
> ---
> .../devicetree/bindings/usb/dwc3-xilinx.yaml | 70 ++++++++++++++++++-
> 1 file changed, 67 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml b/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml
> index d6823ef5f9a7..5e31b961aff7 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml
> +++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml
> @@ -15,6 +15,8 @@ properties:
> - enum:
> - xlnx,zynqmp-dwc3
> - xlnx,versal-dwc3
> + - xlnx,versal2-mmi-dwc3
I am not going to ask the same questions.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH] usb: cdnsp: Add support for device-only configuration
From: Pawel Laszczak via B4 Relay @ 2026-03-31 8:19 UTC (permalink / raw)
To: Peter Chen, Roger Quadros, Greg Kroah-Hartman, Bjorn Helgaas
Cc: linux-usb, linux-kernel, linux-pci, pawell
From: Pawel Laszczak <pawell@cadence.com>
This patch introduces support for operating the Cadence USBSSP (cdnsp)
controller in a peripheral-only mode, bypassing the Dual-Role Device (DRD)
logic.
The change in BAR indexing (from BAR 2 to BAR 1) is a direct
consequence of switching from 64-bit to 32-bit addressing in the
Peripheral-only configuration.
Tested on PCI platform with Device-only configuration. Platform-side
changes are included to support the PCI glue layer's property injection.
Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
NOTE: This patch depends on Peter Chen's series "usb: cdns3: Add
USBSSP platform driver support", which was recently accepted by
Greg into the usb-testing branch (commit 6076388ca1ed).
---
drivers/usb/cdns3/cdns3-plat.c | 24 ++++++++++++---------
drivers/usb/cdns3/cdnsp-pci.c | 47 ++++++++++++++++++++++++++++++++++--------
drivers/usb/cdns3/core.c | 3 ++-
drivers/usb/cdns3/core.h | 5 ++++-
drivers/usb/cdns3/drd.c | 16 ++++++++++++--
include/linux/pci_ids.h | 1 +
6 files changed, 73 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
index 71c612e27b73..33746e672cda 100644
--- a/drivers/usb/cdns3/cdns3-plat.c
+++ b/drivers/usb/cdns3/cdns3-plat.c
@@ -75,6 +75,7 @@ static int cdns3_plat_probe(struct platform_device *pdev)
if (cdns->pdata && cdns->pdata->override_apb_timeout)
cdns->override_apb_timeout = cdns->pdata->override_apb_timeout;
+ cdns->no_drd = device_property_read_bool(dev, "no_drd");
platform_set_drvdata(pdev, cdns);
ret = platform_get_irq_byname(pdev, "host");
@@ -107,21 +108,23 @@ static int cdns3_plat_probe(struct platform_device *pdev)
cdns->dev_regs = regs;
- cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
- if (cdns->otg_irq < 0)
- return dev_err_probe(dev, cdns->otg_irq,
- "Failed to get otg IRQ\n");
+ if (!cdns->no_drd) {
+ cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
+ if (cdns->otg_irq < 0)
+ return dev_err_probe(dev, cdns->otg_irq,
+ "Failed to get otg IRQ\n");
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
- if (!res) {
- dev_err(dev, "couldn't get otg resource\n");
- return -ENXIO;
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
+ if (!res) {
+ dev_err(dev, "couldn't get otg resource\n");
+ return -ENXIO;
+ }
+
+ cdns->otg_res = *res;
}
cdns->phyrst_a_enable = device_property_read_bool(dev, "cdns,phyrst-a-enable");
- cdns->otg_res = *res;
-
cdns->wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup");
if (cdns->wakeup_irq == -EPROBE_DEFER)
return cdns->wakeup_irq;
@@ -158,6 +161,7 @@ static int cdns3_plat_probe(struct platform_device *pdev)
goto err_cdns_init;
cdns->gadget_init = cdns3_plat_gadget_init;
+
ret = cdns_core_init_role(cdns);
if (ret)
goto err_cdns_init;
diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
index 432007cfe695..e20c59ceb8a4 100644
--- a/drivers/usb/cdns3/cdnsp-pci.c
+++ b/drivers/usb/cdns3/cdnsp-pci.c
@@ -19,6 +19,7 @@
struct cdnsp_wrap {
struct platform_device *plat_dev;
+ struct property_entry prop[3];
struct resource dev_res[6];
int devfn;
};
@@ -29,10 +30,15 @@ struct cdnsp_wrap {
#define RES_HOST_ID 3
#define RES_DEV_ID 4
#define RES_DRD_ID 5
-
+/* DRD PCI configuration - 64-bit addressing */
+/* First PCI function */
#define PCI_BAR_HOST 0
-#define PCI_BAR_OTG 0
#define PCI_BAR_DEV 2
+/* Second PCI function */
+#define PCI_BAR_OTG 0
+/* Device only PCI configuration - 32-bit addressing */
+/* First PCI function */
+#define PCI_BAR_ONLY_DEV 1
#define PCI_DEV_FN_HOST_DEVICE 0
#define PCI_DEV_FN_OTG 1
@@ -65,6 +71,7 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
struct cdnsp_wrap *wrap;
struct resource *res;
struct pci_dev *func;
+ bool no_drd = false;
int ret = 0;
/*
@@ -75,11 +82,14 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
pdev->devfn != PCI_DEV_FN_OTG))
return -EINVAL;
+ if (pdev->device == PCI_DEVICE_ID_CDNS_UDC_USBSSP)
+ no_drd = true;
+
func = cdnsp_get_second_fun(pdev);
- if (!func)
+ if (!func && !no_drd)
return -EINVAL;
- if (func->class == PCI_CLASS_SERIAL_USB_XHCI ||
+ if ((func && func->class == PCI_CLASS_SERIAL_USB_XHCI) ||
pdev->class == PCI_CLASS_SERIAL_USB_XHCI) {
ret = -EINVAL;
goto put_pci;
@@ -93,7 +103,7 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
pci_set_master(pdev);
- if (pci_is_enabled(func)) {
+ if (func && pci_is_enabled(func)) {
wrap = pci_get_drvdata(func);
} else {
wrap = kzalloc_obj(*wrap);
@@ -106,10 +116,13 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
res = wrap->dev_res;
if (pdev->devfn == PCI_DEV_FN_HOST_DEVICE) {
+ int bar_dev = no_drd ? PCI_BAR_ONLY_DEV : PCI_BAR_DEV;
+
/* Function 0: host(BAR_0) + device(BAR_2). */
dev_dbg(&pdev->dev, "Initialize Device resources\n");
- res[RES_DEV_ID].start = pci_resource_start(pdev, PCI_BAR_DEV);
- res[RES_DEV_ID].end = pci_resource_end(pdev, PCI_BAR_DEV);
+
+ res[RES_DEV_ID].start = pci_resource_start(pdev, bar_dev);
+ res[RES_DEV_ID].end = pci_resource_end(pdev, bar_dev);
res[RES_DEV_ID].name = "dev";
res[RES_DEV_ID].flags = IORESOURCE_MEM;
dev_dbg(&pdev->dev, "USBSSP-DEV physical base addr: %pa\n",
@@ -145,9 +158,20 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
wrap->dev_res[RES_IRQ_OTG_ID].flags = IORESOURCE_IRQ;
}
- if (pci_is_enabled(func)) {
+ if (no_drd || pci_is_enabled(func)) {
+ u8 idx = 0;
+
/* set up platform device info */
pdata.override_apb_timeout = CHICKEN_APB_TIMEOUT_VALUE;
+ if (no_drd) {
+ wrap->prop[idx++] = PROPERTY_ENTRY_STRING("dr_mode", "peripheral");
+ wrap->prop[idx++] = PROPERTY_ENTRY_BOOL("no_drd");
+ } else {
+ wrap->prop[idx++] = PROPERTY_ENTRY_STRING("dr_mode", "otg");
+ wrap->prop[idx++] = PROPERTY_ENTRY_BOOL("usb-role-switch");
+ }
+
+ wrap->prop[idx] = (struct property_entry){ };
memset(&plat_info, 0, sizeof(plat_info));
plat_info.parent = &pdev->dev;
plat_info.fwnode = pdev->dev.fwnode;
@@ -158,6 +182,7 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
plat_info.dma_mask = pdev->dma_mask;
plat_info.data = &pdata;
plat_info.size_data = sizeof(pdata);
+ plat_info.properties = wrap->prop;
wrap->devfn = pdev->devfn;
/* register platform device */
wrap->plat_dev = platform_device_register_full(&plat_info);
@@ -185,13 +210,17 @@ static void cdnsp_pci_remove(struct pci_dev *pdev)
if (wrap->devfn == pdev->devfn)
platform_device_unregister(wrap->plat_dev);
- if (!pci_is_enabled(func))
+ if (!func || !pci_is_enabled(func))
kfree(wrap);
pci_dev_put(func);
}
static const struct pci_device_id cdnsp_pci_ids[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_UDC_USBSSP),
+ .class = PCI_CLASS_SERIAL_USB_DEVICE },
+ { PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_UDC_USBSSP),
+ .class = PCI_CLASS_SERIAL_USB_CDNS },
{ PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_USBSSP),
.class = PCI_CLASS_SERIAL_USB_DEVICE },
{ PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_USBSSP),
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index 10f00b6c3c83..72f7acba6258 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -71,7 +71,8 @@ static void cdns_role_stop(struct cdns *cdns)
static void cdns_exit_roles(struct cdns *cdns)
{
cdns_role_stop(cdns);
- cdns_drd_exit(cdns);
+ if (!cdns->no_drd)
+ cdns_drd_exit(cdns);
}
/**
diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
index dc8c4137de15..6abe231f4559 100644
--- a/drivers/usb/cdns3/core.h
+++ b/drivers/usb/cdns3/core.h
@@ -80,9 +80,11 @@ struct cdns3_platform_data {
* @pdata: platform data from glue layer
* @lock: spinlock structure
* @xhci_plat_data: xhci private data structure pointer
+ * @gadget_init: pointer to gadget initialization function
* @override_apb_timeout: hold value of APB timeout. For value 0 the default
* value in CHICKEN_BITS_3 will be preserved.
- * @gadget_init: pointer to gadget initialization function
+ * @no_drd: DRD register block is inaccessible - driver handles only
+ * device mode.
*/
struct cdns {
struct device *dev;
@@ -122,6 +124,7 @@ struct cdns {
struct xhci_plat_priv *xhci_plat_data;
int (*gadget_init)(struct cdns *cdns);
u32 override_apb_timeout;
+ bool no_drd;
};
int cdns_hw_role_switch(struct cdns *cdns);
diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
index 84fb38a5723a..38f3051c2188 100644
--- a/drivers/usb/cdns3/drd.c
+++ b/drivers/usb/cdns3/drd.c
@@ -107,7 +107,7 @@ void cdns_clear_vbus(struct cdns *cdns)
{
u32 reg;
- if (cdns->version != CDNSP_CONTROLLER_V2)
+ if (cdns->version != CDNSP_CONTROLLER_V2 || cdns->no_drd)
return;
reg = readl(&cdns->otg_cdnsp_regs->override);
@@ -120,7 +120,7 @@ void cdns_set_vbus(struct cdns *cdns)
{
u32 reg;
- if (cdns->version != CDNSP_CONTROLLER_V2)
+ if (cdns->version != CDNSP_CONTROLLER_V2 || cdns->no_drd)
return;
reg = readl(&cdns->otg_cdnsp_regs->override);
@@ -234,6 +234,9 @@ int cdns_drd_gadget_on(struct cdns *cdns)
u32 ready_bit;
int ret, val;
+ if (cdns->no_drd)
+ return 0;
+
/* switch OTG core */
writel(OTGCMD_DEV_BUS_REQ | reg, &cdns->otg_regs->cmd);
@@ -265,6 +268,9 @@ void cdns_drd_gadget_off(struct cdns *cdns)
{
u32 val;
+ if (cdns->no_drd)
+ return;
+
/*
* Driver should wait at least 10us after disabling Device
* before turning-off Device (DEV_BUS_DROP).
@@ -392,6 +398,12 @@ int cdns_drd_init(struct cdns *cdns)
u32 state, reg;
int ret;
+ if (cdns->no_drd) {
+ cdns->version = CDNSP_CONTROLLER_V2;
+ cdns->dr_mode = USB_DR_MODE_PERIPHERAL;
+ return 0;
+ }
+
regs = devm_ioremap_resource(cdns->dev, &cdns->otg_res);
if (IS_ERR(regs))
return PTR_ERR(regs);
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 406abf629be2..a931fb201402 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2424,6 +2424,7 @@
#define PCI_DEVICE_ID_CDNS_USBSS 0x0100
#define PCI_DEVICE_ID_CDNS_USB 0x0120
#define PCI_DEVICE_ID_CDNS_USBSSP 0x0200
+#define PCI_DEVICE_ID_CDNS_UDC_USBSSP 0x0400
#define PCI_VENDOR_ID_ARECA 0x17d3
#define PCI_DEVICE_ID_ARECA_1110 0x1110
---
base-commit: 5196f2c98340297e1fdd36555285e991ceddf776
change-id: 20260331-device_only-192d539430b5
Best regards,
--
Pawel Laszczak <pawell@cadence.com>
^ permalink raw reply related
* RE: [PATCH net-next v4 3/3] r8152: add helper functions for PHY OCP registers
From: Chih Kai Hsu @ 2026-03-31 5:45 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem@davemloft.net, netdev@vger.kernel.org, nic_swsd,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
edumazet@google.com, bjorn@mork.no, pabeni@redhat.com
In-Reply-To: <20260331020156.3518006-2-kuba@kernel.org>
On 31/03/26 10:02 am, Jakub Kicinski wrote:
>
> > diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index
> > 32a4e8d423115..8747c55e0a484 100644
> > --- a/drivers/net/usb/r8152.c
> > +++ b/drivers/net/usb/r8152.c
> [ ... ]
> > +static void sram_write_w0w1(struct r8152 *tp, u16 addr, u16 clear,
> > +u16 set) {
> > + u16 data;
> > +
> > + data = sram_read(tp, addr);
> > + data = (data & ~clear) | set;
> > + ocp_reg_write(tp, OCP_SRAM_DATA, data);
>
> AI flags:
>
> Does writing directly to OCP_SRAM_DATA here actually write to the
> intended
> address?
Yes, this actually writes to the intended address.
>
> Because hardware indirect SRAM access ports typically auto-increment their
> internal address pointer upon access, the sram_read() call will advance the
> pointer. By writing directly to OCP_SRAM_DATA instead of using
> sram_write(tp, addr, data) to re-establish the address, the updated value
> would be written to the next address (addr + 1) rather than the original addr.
>
> Could this corrupt the PHY configuration memory across the initialization
> paths
> that now use sram_clr_bits() and sram_set_bits()?
>
> Can you confirm if this is legit? Not sure whether this device / register
> auto-increments or where AI got it from.
The address pointer, OCP_SRAM_ADDR, is not auto-incremented after sram_read(tp, addr).
However, the address pointer is incremented automatically after ocp_reg_write(tp, OCP_SRAM_DATA, data).
Best Regards,
Chih-Kai
^ 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