t;. In theory that's a reasonable suggestion, right? USB is a
> > probable bus. We turn on power to the USB hub (and the regulator to
> > turn on power is the same no matter which hub is stuffed) and then we
> > can just check which device got enumerated. It's likely that both
> > hubs would behave the same from a software point of view, but they
> > would have different VID/PID.
>
> A 2nd compatible string solves this. Or the s/w needs to tolerate a
> mismatch in VID/PID. Pre-probe matches on compatible string and real
> probe matches on VID/PID and there doesn't have to be any relationship
> between the 2.
>
> If you have another way to power the device other than just 'Vbus' or
> self-powered, then you aren't really USB compliant.
That statement is questionable. After all, "self-powered" really
means nothing more than "not bus-powered" (apart from borderline cases
of devices that take part of their power from the bus and part from
somewhere else).
Alan Stern
ess
> both pre-probe functions could be called and (since there can be
> multiple users of a regulator) it'd be OK, but if we get into reset
> lines it's not much fun. However, describing the device more like
> Matthias has done it will be nicely compatible with second sourcing.
Can the matching be done purely by port number under the controller's root
hub without regard to the VID/PID?
Alan Stern
ally during
> system suspend the driver (whether it's a platform driver or something else)
> needs know which USB (hub) devices correspond to it. This is a real world
> problem, on hardware that might see wide distribution.
>
> There were several attempts to solve
gt; device_release_driver
> device_release_driver_internal
> __device_release_driver
> devres_release_all
>
> Anyway, if you prefer I can change the driver to use kmalloc/kfree.
No, that's fine. I just wasn't sure about this and wanted to check.
Alan Stern
e or when the device is unregistered? And if the
latter, what happens if you have multiple sysfs attribute groups going
at the same time?
Apart from those worries and the typo, this looks pretty good to me.
Acked-by: Alan Stern
Alan Stern
. spin_unlock_irq
Rest of handler runs
spin_unlock_irq
That's why we have synchronize_irq(). The usual pattern is something
like this:
spin_lock_irq(>lock);
priv->disconnected = true;
my_disable_irq(priv);
spin_unlock_irq(>lock);
synchronize_irq(priv->irq);
And of course this has to be done in a context that can sleep.
Does this answer your question?
Alan Stern
ub_usbdev_probe(struct usb_device *udev)
> +{
> + struct device *dev = >dev;
> + struct onboard_hub *hub;
> +
> + /* ignore supported hubs without device tree node */
> + if (!dev->of_node)
> + return -ENODEV;
> +
> + hub = _find_onboard_hub(dev);
> + if (IS_ERR(hub))
> + return PTR_ERR(dev);
hub, not dev.
Alan Stern
ng freed too soon
if you make onboard_hub_remove unbind the associated USB hub devices.
But there would still be a danger of those devices somehow getting
rebound again at the wrong time; this suggests that you should add a
flag to the onboard_hub structure saying that the platform device is
about to go away.
Alan Stern
On Mon, Sep 21, 2020 at 02:24:52PM +0200, Oliver Neukum wrote:
> Am Sonntag, den 20.09.2020, 10:44 +0200 schrieb Pavel Machek:
> > Move prepare to wait around, so that normal GFP_KERNEL allocation can
> > be used.
> >
> > Signed-off-by: Pavel Machek (CIP)
> &g
complicated.
Alan Stern
Index: usb-devel/drivers/usb/core/hub.c
===
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -2705,11 +2705,18 @@ static unsigned hub_is_wusb(struct usb_h
}
d hubs without device tree node */
> + if (!dev->of_node)
> + return -ENODEV;
> +
> + hub = _find_onboard_hub(dev);
> + if (IS_ERR(hub))
> + return PTR_ERR(dev);
> +
> + dev_set_drvdata(dev, hub);
> +
> + onboard_hub_add_usbdev(hub, udev);
Ignoring the return code? Then why does that routine return int rather
than void?
> +
> + return 0;
> +}
> +
> +static void onboard_hub_usbdev_disconnect(struct usb_device *udev)
> +{
> + struct onboard_hub *hub = dev_get_drvdata(>dev);
> +
> + onboard_hub_remove_usbdev(hub, udev);
Ditto.
> +
> + put_device(hub->dev);
Is there a matching get_device somewhere (like in _find_onboard_hub)?
If so, I didn't see it. And I don't see any reason for it.
Alan Stern
ost/ehci-hub.c b/drivers/usb/host/ehci-hub.c
> > > index ce0eaf7d7c12..087402aec5cb 100644
> > > --- a/drivers/usb/host/ehci-hub.c
> > > +++ b/drivers/usb/host/ehci-hub.c
> > > @@ -14,7 +14,6 @@
> > > */
> > >
> > >
> > > /*-*/
> > > -#include
> > >
> > > #define PORT_WAKE_BITS (PORT_WKOC_E|PORT_WKDISC_E|PORT_WKCONN_E)
> >
> > Thanks for root-causing this issue. genksyms is a fragile beast, good
> > luck trying to resolve that!
> >
> > I'll go queue this up and mark it for stable kernels, thanks.
>
> I'll queue it up after I get Alan's review first :)
It's fine with me.
Acked-by: Alan Stern
Alan Stern
is
> enabled on the controller of a device.
The host controller's sysfs wakeup setting should always be correct. If
it isn't, that indicates there is a bug in the host controller driver or
the corresponding platform-specific code. What driver does your system
use?
Alan Stern
On Tue, Sep 15, 2020 at 01:01:11PM +0200, Greg KH wrote:
> On Tue, Sep 15, 2020 at 11:45:31AM +0200, Eugeniu Rosca wrote:
> > Dear Alan,
> > Dear Greg,
> >
> > On Fri, Sep 11, 2020 at 11:12:28AM -0400, Alan Stern wrote:
> >
> > > The thing is, I'm a
On Tue, Sep 15, 2020 at 11:45:31AM +0200, Eugeniu Rosca wrote:
> Dear Alan,
> Dear Greg,
>
> On Fri, Sep 11, 2020 at 11:12:28AM -0400, Alan Stern wrote:
>
> > The thing is, I'm afraid that without these retry loops, some devices
> > will stop working. If that h
led autosuspend support
See https://marc.info/?l=linux-usb=159914635920888=2 and the
accompanying submissions. You'll probably want to include those updates
in your driver.
Alan Stern
On Mon, Sep 14, 2020 at 05:37:49PM +0200, Greg Kroah-Hartman wrote:
> There are a few calls to usb_control_msg() that can be converted to use
> usb_control_msg_send() instead, so do that in order to make the error
> checking a bit simpler and the code smaller.
>
> Cc: Alan Ster
tchdog = 0;
> +
> + if (of_device_is_compatible(pdev->dev.of_node, "brcm,xgs-iproc-ehci"))
> + ehci_writel(ehci, BCM_USB_FIFO_THRESHOLD,
> + >regs->bcm_iproc_insnreg01);
> +
> return 0;
> }
Acked-by: Alan Stern
For quirk OHCI_QUIRK_AMD756 or OHCI_QUIRK_HUB_POWER power switching
> remains at none, while over-current protection is now guaranteed to be
> set to per-port rather than the previous behaviour where it was either
> none or global over-current protection depending on the value
On Fri, Sep 11, 2020 at 09:25:12AM +1200, Hamish Martin wrote:
> The 'distrust_firmware' module parameter dates from 2004 and the USB
> subsystem is a lot more mature and reliable now than it was then.
> Alter the default to false now.
>
> Suggested-by: Alan Stern
> Signed-off
ice_is_compatible(dev->dev.of_node, "brcm,xgs-iproc-ehci"))
> + ehci_writel(ehci, BCM_USB_FIFO_THRESHOLD,
> + ehci->regs->bcm_iproc_insnreg01);
In theory, this should go before the usb_add_hcd() call because
afterward the controller is
On Wed, Sep 09, 2020 at 09:44:05PM +0800, YueHaibing wrote:
> commit b35c5009bbf6 ("USB: EHCI: create per-TT bandwidth tables")
> left behind this, remove it.
>
> Signed-off-by: YueHaibing
Acked-by: Alan Stern
e previous behaviour where it was either
> none or global over-current protection depending on the value at
> function entry.
Also consider renaming OHCI_QUIRK_HUB_POWER to something like
OHCI_QUIRK_PORT_POWER_ALWAYS_ON.
> Suggested-by: Alan Stern
> Signed-off-by: Hamish Martin
&
On Mon, Sep 07, 2020 at 04:16:34PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Sep 03, 2020 at 09:32:30AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 02, 2020 at 08:45:53PM -0400, Alan Stern wrote:
> > > Since this routine is used in only one place in the entire kernel
On Mon, Sep 07, 2020 at 04:51:01PM +0200, Greg Kroah-Hartman wrote:
> There are a few calls to usb_control_msg() that can be converted to use
> usb_control_msg_send() instead, so do that in order to make the error
> checking a bit simpler and the code smaller.
>
> Cc: Alan Ster
On Thu, Sep 03, 2020 at 04:45:07PM -0700, Paul E. McKenney wrote:
> The hope was to have a good version of them completed some weeks ago,
> but life intervened.
>
> My current thought is to move these three patches out of my queue for
> v5.10 to try again in v5.11:
>
> 0b8c06b75ea1
right, for two reasons. First, isn't per-port
overcurrent protection the default?
Second, RH_A_OCPM doesn't do anything unless RH_A_NOCP is clear.
Alan Stern
> ohci_writel (ohci, RH_HS_LPSC, >regs->roothub.status);
> ohci_writel (ohci, (val & RH_A_NPS) ? 0 : RH_B_PPCM
ci-platform",
> - .pm = _platform_pm_ops,
> + .pm = pm_ptr(_platform_pm_ops),
> .of_match_table = vt8500_ehci_ids,
> .acpi_match_table = ACPI_PTR(ehci_acpi_match),
> }
> --
> 2.28.0
For patches 2 - 4:
Acked-by: Alan Stern
OHCI_BIG_ENDIAN_DESC not set\n");
> err = -EINVAL;
> goto err_reset;
> }
> -#endif
>
> pm_runtime_set_active(>dev);
> pm_runtime_enable(>dev);
The changes above don't seem to have any connection with the patch
description.
=
> + USB_QUIRK_IGNORE_REMOTE_WAKEUP },
> +
> /* Realtek hub in Dell WD19 (Type-C) */
> { USB_DEVICE(0x0bda, 0x0487), .driver_info = USB_QUIRK_NO_LPM },
Please follow the instructions at the start of the file about keeping
the entries sorted by vendor ID and product ID.
Alan Stern
ch the existing
> usb_urb_ep_type_check() call, which now uses this function.
>
> Cc: Jaroslav Kysela
> Cc: Takashi Iwai
> Cc: "Gustavo A. R. Silva"
> Cc: Eli Billauer
> Cc: Emiliano Ingrassia
> Cc: Alan Stern
> Cc: Alexander Tsoy
> Cc: "Geoffrey D. Benn
o?
Sigh. That never occurred to me, but of course it is right.
Acked-by: Alan Stern
Alan Stern
> Signed-off-by: Pavel Machek (CIP)
>
> diff --git a/drivers/usb/misc/yurex.c b/drivers/usb/misc/yurex.c
> index 785080f79073..5fbbb57e6e95 100644
> --- a/drivers/usb/misc/yurex
On Wed, Sep 02, 2020 at 01:01:06PM +0200, Greg Kroah-Hartman wrote:
> There are a few calls to usb_control_msg() that can be converted to use
> usb_control_msg_send() instead, so do that in order to make the error
> checking a bit simpler and the code smaller.
>
> Cc: Alan Ster
n the driver somewhere and should be solved.
Correction: You're using two different drivers. Although it's not
impossible, it seems very unlikely that they both contain the same bug.
Alan Stern
On Tue, Sep 01, 2020 at 10:04:21AM -0700, Paul E. McKenney wrote:
> On Mon, Aug 31, 2020 at 09:45:04PM -0400, Alan Stern wrote:
> > The question is, what are you trying to accomplish in this section? Are
> > you trying to demonstrate that it isn't safe to allow arbitrary co
On Tue, Sep 01, 2020 at 11:00:16AM -0600, Khalid Aziz wrote:
> On 9/1/20 10:36 AM, Alan Stern wrote:
> > On Tue, Sep 01, 2020 at 09:15:46AM -0700, Khalid Aziz wrote:
> >> On 8/31/20 8:31 PM, Alan Stern wrote:
> >>> Can you collect a usbmon trace show
On Tue, Sep 01, 2020 at 09:15:46AM -0700, Khalid Aziz wrote:
> On 8/31/20 8:31 PM, Alan Stern wrote:
> > Can you collect a usbmon trace showing an example of this problem?
> >
>
> I have attached usbmon traces for when USB hub with keyboards and mouse
> is plugged in
t will help clear this up.
Or maybe the hubs you are testing don't work right. That's the only
reason I can think of for the failures you see with the USB-3
controller; the way it operates is very different from the way EHCI
does.
Alan Stern
it you referenced above specifically
mentions that when MMF is set and the PID code is IN then it is not a
STALL.
> Removing the code that returns EPROTO for such case solves the
> problem on my machine (as in the RFC patch)
It certainly can't solve the problem for any USB-3 connections, because
the patch doesn't touch any of the USB-3 driver code.
> but that probably is not
> the right solution. I do not understand USB protocol well enough to
> propose a better solution. Does anyone have a better idea?
Can you collect a usbmon trace showing an example of this problem?
One possibility is to introduce a special quirk for the NEC uPD72010x
EHCI controller. But we should hold off on that until we know exactly
what is happening.
Alan Stern
On Mon, Aug 31, 2020 at 02:47:38PM -0700, Paul E. McKenney wrote:
> On Mon, Aug 31, 2020 at 04:17:01PM -0400, Alan Stern wrote:
> > Is this discussion perhaps overkill?
> >
> > Let's put it this way: Suppose we have the following code:
> >
On Mon, Aug 31, 2020 at 11:20:36AM -0700, paul...@kernel.org wrote:
> From: "Paul E. McKenney"
>
> The Linux kernel has a number of categories of ordering primitives, which
> are recorded in the LKMM implementation and hinted at by cheatsheet.txt.
> But there is no overview of these categories,
On Mon, Aug 31, 2020 at 11:20:37AM -0700, paul...@kernel.org wrote:
> +No Roach-Motel Locking!
> +---
> +
> +This example requires familiarity with the herd7 "filter" clause, so
> +please read up on that topic in litmus-tests.txt.
> +
> +It is tempting to allow memory-reference
a %s req %p\n", ep->ep.name, req);
> + BUG_ON(!req);
There's no point in adding this. If the function goes on to dereference
a NULL pointer, you'll get the same effect in any case -- an oops.
If you want to make the point that req had better not be NULL, just get
rid of the "if" test entirely. You could even change the list_entry to
list_first_entry.
Alan Stern
On Sat, Aug 29, 2020 at 11:50:03AM +0200, Salvatore Bonaccorso wrote:
> Hi Alan,
>
> I'm following up on this thread because a user in Debian (Dirk, Cc'ed)
> as well encountered the same/similar issue:
>
> On Tue, Jul 21, 2020 at 10:33:25AM -0400, Alan Stern wrote:
> > On
. Neither of those calls
do_proc_bulk() or usb_bulk_msg(), so how did those routines end up in
the stack trace?
In fact, do_proc_bulk() is called only for USBDEVFS_BULK. But the test
doesn't use that ioctl!
What's going on? Am I missing part of the test?
Alan Stern
ncfg = dev->descriptor.bNumConfigurations;
...
if (ncfg > USB_MAXCONFIG) {
dev_warn(ddev, "too many configurations: %d, "
"using maximum allowed: %d\n", ncfg, USB_MAXCONFIG);
dev->descriptor.bNumConfigurations = ncfg = USB_MAXCONFIG;
}
If you want to fix the bug, you need to figure out why this isn't
working.
Alan Stern
converting everything
> over to a sane, and checkable, api and remove a bunch of wrapper
> functions as well.
Suggestion: _read and _send are not a natural pair. Consider instead
_read and _write. _recv and _send don't feel right either, because it
both cases the host sends the control message -- the difference lies
in who sends the data.
Alan Stern
>
> > In my experience it's generally frowned upon for functions to store
> > results in error paths.
>
> Then maybe v1 is more appropriate.
>
> Else i can spin a v3.
>
> My preference is v1 as it doesn't add any runtime if-checks.
If you really want to get rid of the runtime check (both the one you
added and the one already present), you can audit all the callers of
this routine to make certain that none of them pass a NULL pointer for
act_len.
Alan Stern
static analyzer objects
to.
> In my experience it's generally frowned upon for functions to store
> results in error paths.
I don't see any reason for such an attitude, at least not here. It
makes perfectly good sense, if an error prevents transmission of an
entire data buffer, to store the amount of data that did get
transmitted.
Alan Stern
sure that its runtime-resume callbacks will not be confused by that
> + * change in case they are invoked going forward.
> */
> - if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
> - pm_wakeup_event(dev, 0);
> + pm_runtime_barrier(dev);
>
> if (pm_wakeup_pending()) {
> dev->power.direct_complete = false;
Acked-by: Alan Stern
I'm going to re-spin the patch with this code simplification
> and updated changelog.
>
> > Will this fix the reported bug?
>
> I think so.
Okay, we'll see!
Alan Stern
usb_stor_bulk_transfer_sglist(): Make it store 0 to *act_len at the
start.
That way you change only one localized piece of code, instead of
changing multiple callers and leaving a possibility of more errors being
added in the future.
Alan Stern
t:
pm_runtime_barrier(dev);
Will this fix the reported bug? It seems likely to me that the actual
problem with the failure scenario in the patch description was that
turning on an ACPI power resource causes runtime-resume requests to be
queued for all devices sharing that resource. Wouldn't it make more
sense to resume only the device that requested it and leave the others
in runtime suspend?
Alan Stern
right thing without constant checking.
Alan Stern
On Wed, Aug 19, 2020 at 08:41:05PM +0800, Chunfeng Yun wrote:
> Use readl_poll_timeout_atomic() to simplify code
>
> Cc: Alan Stern
> Cc: Felipe Balbi
> Signed-off-by: Chunfeng Yun
> ---
> drivers/usb/gadget/udc/net2280.c | 21 ++---
> 1 file chang
On Tue, Aug 18, 2020 at 11:54:51AM +0100, Jim Baxter wrote:
> On 17/08/2020 19:47, Alan Stern wrote:
> >
> > Unplugging a R/W USB drive without unmounting it first is a great way to
> > corrupt the data.
> >
> Thank you, post development we will only mount the U
re
workqueues. Workqueues are allowed to spawn multiple threads; they are
supposed to resize themselves dynamically as required.
Alan Stern
the block and filesystem layers.)
> I guess it may be waiting for a time-out during the operation without the
> unmount.
That seems very unlikely. When a USB device gets unplugged the system
realizes it. Any I/O meant for that device is immediately cancelled;
there are no timeouts.
(Okay, not strictly true; there is a fraction-of-a-second timeout during
which the system waits to see whether the disconnect was permanent or
just a temporary glitch. But you're talking about 6-second long
delays.)
Alan Stern
drivers. They must make sure that their disconnect
routines block until all outstanding calls to usb_set_interface return
(in fact, until all outstanding device accesses have finished).
For instance, in the log extract you showed, it's obvious that the
uvc_start_streaming routine was running after the disconnect routine had
returned, which looks like a bug in itself: Once the disconnect routine
returns, the driver is not supposed to try to access the device at all
because some other driver may now be bound to it.
We can't just call usb_lock_device from within usb_set_interface,
because usb_set_interface is often called with that lock already held.
Alan Stern
On Tue, Aug 11, 2020 at 11:54:28AM -0700, Tom Rix wrote:
>
> On 8/11/20 10:53 AM, Alan Stern wrote:
> >>> Instead of changing all these call sites, wouldn't it be a lot easier
> >>> just to change rts51x_read_mem() to make it always return a negative
> >
On Tue, Aug 11, 2020 at 10:29:29AM -0700, Tom Rix wrote:
>
> On 8/11/20 9:03 AM, Alan Stern wrote:
> > On Tue, Aug 11, 2020 at 08:15:05AM -0700, t...@redhat.com wrote:
> >> From: Tom Rix
> >>
> >> clang static analysis reports this representative proble
return -EIO;
Instead of changing all these call sites, wouldn't it be a lot easier
just to change rts51x_read_mem() to make it always return a negative
value (such as -EIO) when there's an error?
Alan Stern
On Tue, Aug 11, 2020 at 09:55:54AM +0200, Martin Kepplinger wrote:
> On 10.08.20 16:13, Alan Stern wrote:
> > This may not matter... but it's worth pointing out that
> > expecting_media_change doesn't get cleared if ++scmd->retries >
> > scmd->allowed.
>
> a
of better names, I'm hopeless haha)
How about usb_gadget_enable_endpoints()?
Alan Stern
_write+0x3ea/0x820 drivers/usb/misc/yurex.c:495
This patch changes the call to use GFP_ATOMIC instead of GFP_KERNEL.
Reported-and-tested-by: syzbot+c2c3302f9c601a4b1...@syzkaller.appspotmail.com
Signed-off-by: Alan Stern
CC:
---
[as1940]
drivers/usb/misc/yurex.c |2 +-
1 file changed, 1 ins
ot;);
> + usb_stor_dbg(us, " Not removable media, just report
> okay\n");
> srb->result = SAM_STAT_GOOD;
> sendToTransport = 0;
> }
> --
Acked-by: Alan Stern
b+0xb4e/0x13e0 drivers/usb/core/urb.c:570
> yurex_write+0x3ea/0x820 drivers/usb/misc/yurex.c:495
The yurex driver shouldn't use GFP_KERNEL in a context where the current
state isn't TASK_RUNNING.
Alan Stern
#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
449dc8c9
I
On Mon, Aug 10, 2020 at 02:03:17PM +0200, Martin Kepplinger wrote:
> On 09.08.20 17:26, Alan Stern wrote:
> > This is a somewhat fragile approach. You don't know for certain that
> > scsi_noretry_cmd will be called. Also, scsi_noretry_cmd can be called
> > from other plac
er places.
It would be better to clear the expecting_media_change flag just before
returning from scsi_decide_disposition. That way its use is localized
to one routine, not spread out between two.
Alan Stern
On Sat, Aug 08, 2020 at 08:59:09AM +0200, Martin Kepplinger wrote:
> On 07.08.20 16:30, Alan Stern wrote:
> > On Fri, Aug 07, 2020 at 11:51:21AM +0200, Martin Kepplinger wrote:
> >> it's really strange: below is the change I'm trying. Of course that's
> >> only fo
Are you saying that scmd->allowed is set to 0? Or is scsi_notry_cmd()
returning a nonzero value? Whichever is true, why does it happen that
way?
What is the failing command? Is it a READ(10)?
> How can this be? What am I missing?
It's kind of hard to tell without seeing the error messages or
22 [inline]
> __wake_up+0xb8/0x150 kernel/sched/wait.c:142
> __usb_hcd_giveback_urb+0x340/0x4b0 drivers/usb/core/hcd.c:1653
It looks like xpad_disconnect() fails to call xpad_stop_input() if the
hardware isn't an Xbox 360W.
Alan Stern
vior.
>
> Some of them are similar to each other with subtle differences only
> and the behavior of some of them may appear as counter-intuitive, so
> clarify all that to avoid confusion.
>
> Signed-off-by: Rafael J. Wysocki
Acked-by: Alan Stern
releasing the IRQ line. When synchronize_irq() returns, you'll know any
IRQ handlers have finished running, so you won't receive any more
disconnect notifications. Then it will be safe to acquire the spinlock
and set mtu->gadget_driver to NULL.
Alan Stern
On Thu, Jul 30, 2020 at 10:05:50AM +0200, Martin Kepplinger wrote:
> On 29.06.20 18:15, Alan Stern wrote:
> > On Mon, Jun 29, 2020 at 11:42:59AM +0200, Martin Kepplinger wrote:
> >>
> >>
> >> On 26.06.20 17:44, Alan Stern wrote:
> >>> Martin's b
unsigned set_dbd_for_ms:1; /* Set "DBD" field in mode sense */
That's pretty much what James was suggesting, except for one thing: You
must not set sdkp->device->expecting_media_change to 1 for all devices
in sd_runtime_resume(). Only for devices which may generate a spurious
Unit Attention following runtime resume -- and maybe not even for all of
them, depending on what the user wants.
Alan Stern
n Wed, 2020-07-29 at 07:46 -0700, James Bottomley wrote:
> >> > > On Wed, 2020-07-29 at 10:32 -0400, Alan Stern wrote:
> >[...]
> >> > > > This error report comes from the SCSI layer, not the block
> >> > > > layer.
> >> > >
On Wed, Jul 29, 2020 at 08:49:34AM -0700, James Bottomley wrote:
> On Wed, 2020-07-29 at 11:40 -0400, Alan Stern wrote:
> > On Wed, Jul 29, 2020 at 07:53:52AM -0700, James Bottomley wrote:
> > > On Wed, 2020-07-29 at 07:46 -0700, James Bottomley wrote:
> [...]
> >
On Wed, Jul 29, 2020 at 07:53:52AM -0700, James Bottomley wrote:
> On Wed, 2020-07-29 at 07:46 -0700, James Bottomley wrote:
> > On Wed, 2020-07-29 at 10:32 -0400, Alan Stern wrote:
> > > On Wed, Jul 29, 2020 at 04:12:22PM +0200, Martin Kepplinger wrote:
> > > > On 28
ice telling us that the media (SD card?) has changed.
Ah yes, thank you. I knew that SK=6 ASC=0x28 meant "Not Ready to Ready
Transition", but I had forgotten the "(Media May Have Changed)" part.
This makes sense and is a reasonable thing to see, since many SD card
readers lose track of whether or not the card has been changed when they
go into suspend.
Alan Stern
On Wed, Jul 29, 2020 at 04:12:22PM +0200, Martin Kepplinger wrote:
> On 28.07.20 22:02, Alan Stern wrote:
> > On Tue, Jul 28, 2020 at 09:02:44AM +0200, Martin Kepplinger wrote:
> >> Hi Alan,
> >>
> >> Any API cleanup is of course welcome. I just wanted to
you would
expect.
> As we need to have that working at some point, I might look into it, but
> someone who has experience in the block layer can surely do it more
> efficiently.
I suspect that any problems you still face are caused by something else.
Alan Stern
On Mon, Jul 27, 2020 at 05:59:17PM +0100, Matthew Wilcox wrote:
> On Mon, Jul 27, 2020 at 12:31:49PM -0400, Alan Stern wrote:
> > On Mon, Jul 27, 2020 at 04:28:27PM +0100, Matthew Wilcox wrote:
> > > On Mon, Jul 27, 2020 at 11:17:46AM -0400, Alan Stern wrote:
> > > >
On Mon, Jul 27, 2020 at 04:28:27PM +0100, Matthew Wilcox wrote:
> On Mon, Jul 27, 2020 at 11:17:46AM -0400, Alan Stern wrote:
> > Given a type "T", an object x of type pointer-to-T, and a function
> > "func" that takes various arguments and returns a pointer-to-
ies
on a single mutex for all the different possible x's, it might lead to
locking conflicts (if func had to call once_func() recursively, for
example). In most reasonable situations such conflicts would not
arise.
Alan Stern
it seems reasonable
to leave this as a "gentlemen's agreement" in userspace for the time
being instead of adding it to the kernel.
Alan Stern
hat they're doing when they change driver binding,
> that's a pretty subtle interaction with no validation. Thanks,
It's worse than that. We're not just dealing with a software
interaction issue -- the _hardware_ for these devices also interacts.
That's the real reason why the driver for the device on one slot has to
be aware of the driver for the device on a different slot.
Adding a mechanism for software registration or validation won't fix the
hardware issues. Relying on a protocol that requires all the devices to
be unbound before any of them are bound to a new class of drivers, on
the other hand, will fix the problem.
Alan Stern
> FYI. It looks like I'm likely to resume my work on that driver in the
> next few weeks in which case I could probably look into these Alan.
That probably won't be needed, although thanks for the offer. I'll CC
you on any patches when they are submitted.
Alan Stern
way, one can consider both issues independently on the one fixed by
> the patch.
Yes. I'll write and submit a series of patches.
Alan Stern
lly, I imagine.
The only way to make this work at all is to unbind both uhci-hcd and
ehci-hcd first. Then after both are finished you can safely bind
vfio-pci to the EHCI controller and the UHCI controllers (in that
order).
Alan Stern
ng this memset? It's hard to
imagine that it does any good.
Similarly, net2280_remove() calls usb_del_gadget_udc(>gadget) at
its start, and so dev must be a stale pointer for the entire remainder
of the routine. But it gets used repeatedly. Surely we ought to have
a device_get() and device_put() in there.
Alan Stern
On Tue, Jul 21, 2020 at 06:00:25PM +0200, Michal Hocko wrote:
> On Tue 21-07-20 10:33:25, Alan Stern wrote:
> [...]
>
> Thanks a lot for your analysis. The laptop is slowly dying so this can
> be related.
>
> > So yes, this looks like a hardware design error. Turn
2a0 -- the same as what the debugging log says.
So yes, this looks like a hardware design error. Turning off
autosuspend by writing to the sysfs power/control file is probably the
best way to handle the problem.
Alan Stern
On Mon, Jul 20, 2020 at 08:16:05PM +0200, Michal Hocko wrote:
> On Mon 20-07-20 13:48:12, Alan Stern wrote:
> > On Mon, Jul 20, 2020 at 07:45:30PM +0200, Michal Hocko wrote:
> > > On Mon 20-07-20 13:38:07, Alan Stern wrote:
> > > > On Mon, Jul 20, 2020 at 06:33:
On Mon, Jul 20, 2020 at 07:45:30PM +0200, Michal Hocko wrote:
> On Mon 20-07-20 13:38:07, Alan Stern wrote:
> > On Mon, Jul 20, 2020 at 06:33:55PM +0200, Michal Hocko wrote:
> > > On Mon 20-07-20 11:12:55, Alan Stern wrote:
> > > [...]
> > > >
On Mon, Jul 20, 2020 at 06:33:55PM +0200, Michal Hocko wrote:
> On Mon 20-07-20 11:12:55, Alan Stern wrote:
> [...]
> > sudo echo 'module usbcore =p' >/debug/dynamic_debug/control
> >
> > Then wait long enough for some interesting messages to appear in the
> >
say "unlock semantics"?
It's not as bad as all that; people do talk about acquiring and
releasing locks, and presumably you don't have any trouble understanding
those terms. In fact this usage is quite common -- and I believe it's
where the names "acquire semantics" and "release semantics" came from
originally.
Alan Stern
On Mon, Jul 20, 2020 at 04:32:13PM +0200, Michal Hocko wrote:
> On Mon 20-07-20 09:58:57, Alan Stern wrote:
> [...]
> > Can you provide the contents of /sys/kernel/debug/usb/devices and also a
>
> attached.
It looks like you've got just two devices, only one of which is
On Mon, Jul 20, 2020 at 11:33:20AM +1000, Dave Chinner wrote:
> On Sat, Jul 18, 2020 at 10:08:11AM -0400, Alan Stern wrote:
> > > This is one of the reasons that the LKMM documetnation is so damn
> > > difficult to read and understand: just understanding the vocabulary
&
81
> [<0>] process_one_work+0x1bd/0x2c6
> [<0>] worker_thread+0x19c/0x240
> [<0>] kthread+0x11b/0x123
> [<0>] ret_from_fork+0x22/0x30
>
> Is this something known or something I can give more information about?
> From a very quick look into the code it sounds as if the system wanted
> to suspend an USB device/controller but that keeps failing again and
> again.
Yes, that's what it looks like to me too. Or maybe the suspend succeeds
but then the device/controller immediately gets woken up again.
Can you provide the contents of /sys/kernel/debug/usb/devices and also a
usbmon trace showing a few rounds of this recurring activity? Perhaps a
section of the dmesg log with dynamic debugging enabled for the usbcore
module, as well.
Alan Stern
201 - 300 of 5703 matches
Mail list logo
