On Mon, Sep 05, 2022 at 04:36:16PM +0800, Ray Chi wrote: > On Sat, Sep 3, 2022 at 1:07 AM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > I don't understand. If you don't know whether the accessory is broken, > > how do you know whether to set the quirk? > > > > On the other hand, if you always set the quirk even before you know > > whether the accessory is broken, why make it a quirk at all? Why not > > make it the normal behavior of the driver? > > > > Since our device has a watchdog mechanism, when the device connects to > a broken accessory, the kernel panic will happen. This problem didn't happen > in all USB Hosts, so I want to use the quirk to fix this problem for those hosts > with a watchdog mechanism. Okay. So this shouldn't be a quirk; it should apply all the time to any hub where the host controller has this watchdog mechanism. > > Why not set CONFIG_USB_FEW_INIT_RETRIES instead? > > > > https://source.android.com/docs/core/architecture/kernel/android-common > According to Android Common Kernel, I can't only add this config to one project. > In addition, it can't stop enumeration so that the timeout problem > still happens. This is the first time you have mentioned either the watchdog mechanism or the fact that this is intended for Android. It would have been a lot better if both of these facts were included in the initial patch description. You can't expect people to evaluate a new patch properly if they don't have a clear picture of what it was meant for. > > might describe in detail a situation where the quirk is needed, > > explaining what sort of behavior of the system would lead you to set the > > quirk, and why. > > > > There is a kernel panic when the device connects to the broken accessory. > I tried to modify the initial_descriptor_timeout. When the accessory is not > working, the total time is 6.5s (get descriptor retry) + 5*2 seconds > (set address of xhci timeout). > The time is so long to cause kernel panic for the device. This is why I want to > stop enumeration instead reducing the retries or timeout. It sounds like what you need is a "quick initialization" option that will limit the timeout lengths and the numbers of retries, and will cause the system to ignore connections on a port once an initialization has failed. There should also be a way to make the system stop ignoring a port, perhaps by writing to a sysfs file. In addition, there should be an automatic algorithm to determine which hub ports this option will apply to. I don't think you want it to be based on a quirk, because you shouldn't need to wait for a kernel panic before realizing that the quirk is needed -- that's why the algorithm has to be automatic. Can you write a new patch that works more like this? Alan Stern
