RE: [PATCH v2] usb: host: Implement workaround for Erratum A-009611
-Original Message- From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com] Sent: Monday, December 11, 2017 4:52 PM To: Yinbo Zhu ; Greg Kroah-Hartman Cc: Mathias Nyman ; open list:DESIGNWARE USB3 DRD IP DRIVER ; open list:DESIGNWARE USB3 DRD IP DRIVER ; open list ; Xiaobo Xie ; Jerry Huang ; Ran Wang Subject: RE: [PATCH v2] usb: host: Implement workaround for Erratum A-009611 >>Hi, >>(please break your lines at 80-characters) >>Yinbo Zhu writes: I had check it. Every line is less than 80-characters. >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index >>> 5cb3f6795b0b..071e7cea8cbb 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -1106,6 +1106,9 @@ static void dwc3_get_properties(struct dwc3 >>> *dwc) >>> >>> dwc->quirk_reverse_in_out = device_property_read_bool(dev, >>> "snps,quirk_reverse_in_out"); > >>This was generated on vendor tree. This quirk doesn't exist in dwc3. >>Also, > >update your tree and review MAINTAINERS file. It has been almost 2 > years since I left TI :-) > >>-- >>Balbi > > Hi Balbi, > > The quirk that I had add it in dwc3. Your meaning is that I can't use > quirk to enable or disable the erratum, isn't it? The tree is > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git, I had > updated it. >-*- mode: grep; default-directory: "~/workspace/linux/" -*- Grep started at >Mon Dec 11 10:50:47 >git --no-pager grep --color -nH -e quirk_reverse_in_out >Grep finished with no matches found at Mon Dec 11 10:50:48 >-- >balbi Hi Balbi, You can't find the quirk that it is normal. There's no one in the previous code. The quirk that I added to control the new erratum Please you note. Thanks. Yinbo. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Dual-role behavior with USB-C?
Hello. Thank you very much for your comment. > Since USB OTG FSM has not been accepted by industry during last ten years, we > decide > to give up maintaining OTG FSM at Linux kernel. For role switch use case, > please use > /sys/../role instead, see below commit for detail: > > > https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git/commit/drivers/usb/chipidea/core.c?h=ci-for-usb- next&id=a932a8041ff9941a244619555f1c75ecf299f662 I am still learning so just point me to existing memo / notes which I should look into, before go furher. 1) HNP aware devices That means it does not work (or will not work) with HNP-aware device anymore, or I can expect that it still works when I enable CONFIG_USB_OTG and CONFIG_USB_OTG_FSM? 2) role I/F. As for the role I/F. I tried like below and confirmed that the role value changes gadget <-> host. And new bus is added can see on lsusb output, when I change the role from gadget to host. echo host > /sys/bus/platform/devices/ci_hdrc.0/role However, I am still confused since it does not more. Say, I have two boards A and B connected with C-C cable. And by command like above, I set A host role and B gadget role, and also load mass storage driver on B. I expect to see on B is enumerated as mass storage device attached to the newly added bus on A, but I cannot see it on lsusb output. Not sure I am missing / skipping steps. 3) USB-C lines As for USB-C receptacle on my board, I can see the following. In my case I am just trying to use it in place of old one (micro-A/B receptacle), but I wonder if I further need kernel configuration specific to TYPEC. board cable USB_OTG_DP - Dp1/Dp2 USB_OTG_DN - Dn1/Dn2 USB_OTG_ID - CC1/CC2 Peter -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG] drivers/usb/host/isp116x-hcd: a possible sleep-in-atomic bug in isp116x_start
According to drivers/usb/host/isp116x-hcd.c, the kernel module may sleep under a spinlock. The function call path is: isp116x_start (acquire the spinlock) device_init_wakeup device_wakeup_enable wakeup_source_register wakeup_source_create kmalloc(GFP_KERNEL) --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: chipidea: msm: fix ulpi-node lookup
On Mon, Nov 13, 2017 at 11:12:58AM +0100, Johan Hovold wrote:
> Fix child-node lookup during probe, which ended up searching the whole
> device tree depth-first starting at the parent rather than just matching
> on its children.
>
> Note that the original premature free of the parent node has already
> been fixed separately, but that fix was apparently never backported to
> stable.
>
> Fixes: 47654a162081 ("usb: chipidea: msm: Restore wrapper settings after
> reset")
> Fixes: b74c43156c0c ("usb: chipidea: msm: ci_hdrc_msm_probe() missing
> of_node_get()")
> Cc: stable # 4.10: b74c43156c0c
> Cc: Stephen Boyd
> Cc: Frank Rowand
> Signed-off-by: Johan Hovold
> ---
> drivers/usb/chipidea/ci_hdrc_msm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c
> b/drivers/usb/chipidea/ci_hdrc_msm.c
> index 3593ce0ec641..880009987460 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -247,7 +247,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> if (ret)
> goto err_mux;
>
> - ulpi_node = of_find_node_by_name(of_node_get(pdev->dev.of_node),
> "ulpi");
> + ulpi_node = of_get_child_by_name(pdev->dev.of_node, "ulpi");
Stephen, would you comment on it? I am afraid I can't find the benefit
for this change.
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 2/2] usb: chipidea: tegra: Select Tegra's PHY in Kconfig
On Mon, Dec 11, 2017 at 04:09:44PM +0300, Dmitry Osipenko wrote: > On 11.12.2017 13:04, Thierry Reding wrote: > > On Mon, Dec 11, 2017 at 02:10:00AM +0300, Dmitry Osipenko wrote: > >> UDC driver won't probe without Tegra's PHY, hence select it in the > >> Kconfig. > >> > >> Signed-off-by: Dmitry Osipenko > >> --- > >> drivers/usb/chipidea/Kconfig | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig > >> index 785f0ed037f7..2ef3b27ea72b 100644 > >> --- a/drivers/usb/chipidea/Kconfig > >> +++ b/drivers/usb/chipidea/Kconfig > >> @@ -27,6 +27,7 @@ config USB_CHIPIDEA_PCI > >> config USB_CHIPIDEA_UDC > >>bool "ChipIdea device controller" > >>depends on USB_GADGET > >> + select USB_TEGRA_PHY if ARCH_TEGRA > > > > This is kind of pointless given that USB_TEGRA_PHY originally was > > automatically enabled if ARCH_TEGRA was enabled. > > Again, please take a closer look at the patches. USB_TEGRA_PHY was enabled if > USB_EHCI_TEGRA was and not ARCH_TEGRA. > > > What do we gain by these two patches, other than maybe make the driver > > buildable as a module? > > Firstly, tegra-phy is built only if ehci-tegra is built. > > Secondly, I think we need to enforce Tegra PHY to be compiled as built-in if > one > of ehci-tegra or chipidea drivers is built-in and the other is compiled as a > module. You may not bind controller driver with PHY driver in Kconfig, we need to make sure the controller driver has no build error if the PHY driver is not select. And if the PHY driver is not loaded, the controller driver should return -EPROBE_DEFER for it. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Dual-role behavior with USB-C?
> > Hello. > > Just in case if you already have pointer or experience with this, any > suggestion is > highly appreciated. > > Some of the recent i.MX boards has USB-C socket for USB-OTG port, and I wonder > how I can try the following steps (Dual-role behavior testing) with USB-C > sockets. > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel. > org%2Fdoc%2FDocumentation%2Fusb%2Fchipidea.txt&data=02%7C01%7Cpeter.c > hen%40nxp.com%7Cc2f36f5be257466d78b608d53e8e2d57%7C686ea1d3bc2b4c6f > a92cd99c5c301635%7C0%7C0%7C636483702729739829&sdata=6slKCmxytnXl% > 2BP4J73NlXm6CPtVSq3GGsXqSXO%2Bn5fY%3D&reserved=0 > > The steps involves micro-A and micro-B plugs, but for my board only USB-C plug > can be inserted. > (I tried C-C cable to connect two boards, but nothing happened - both of the > boards > stayed in gadget role, neither becoming the host). > > I have confirmed my board can act as host (can detect a device attached and > shown > on lsusb output) or gadget (by loading mass storage driver, it can be shown as > external disk from my PC). > > (Well, however these behaviors are not stable yet with my local build image.) > > Anyway: > > - How can I conduct test steps outlined in chipidea.txt memo. Since USB OTG FSM has not been accepted by industry during last ten years, we decide to give up maintaining OTG FSM at Linux kernel. For role switch use case, please use /sys/../role instead, see below commit for detail: https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git/commit/drivers/usb/chipidea/core.c?h=ci-for-usb-next&id=a932a8041ff9941a244619555f1c75ecf299f662 Peter -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4.4 13/49] usb: dwc2: Fix UDC state tracking
On Sat, Dec 09, 2017 at 06:12:56PM +0100, Greg Kroah-Hartman wrote:
>On Fri, Dec 08, 2017 at 03:37:17AM +, Ben Hutchings wrote:
>> On Thu, 2017-12-07 at 14:07 +0100, Greg Kroah-Hartman wrote:
>> > 4.4-stable review patch. If anyone has any objections, please let me
>> > know.
>> >
>> > --
>> >
>> > From: John Stultz
>> >
>> >
>> > [ Upstream commit ce2b21a4e5ce042c0a42c9db8fa9e0f849427d5e ]
>> >
>> > It has been noticed that the dwc2 udc state reporting doesn't
>> > seem to work (at least on HiKey boards). Where after the initial
>> > setup, the sysfs /sys/class/udc/f72c.usb/state file would
>> > report "configured" no matter the state of the OTG port.
>> >
>> > This patch adds a call so that we report to the UDC layer when
>> > the gadget device is disconnected.
>> >
>> > This patch does depend on the previous patch ("usb: dwc2:
>> > Improve gadget state disconnection handling") in this patch set
>> > in order to properly work.
>>
>> Then you should add that (commit d2471d4a24df).
>
>Ah, but that patch doesn't apply :(
>
>So, I've dropped this one, and the one after this one (which depended on
>this one), so all should be back to how things were.
>
>Sasha, can you fix this up and submit all 3 for the 4.4, 4.9 and 4.14
>trees sometime in the future?
Sure, I'll resend this.
--
Thanks,
Sasha--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray
On Mon, Dec 11, 2017 at 02:12:28PM -0800, Joe Perches wrote: > On Tue, 2017-12-12 at 08:43 +1100, Dave Chinner wrote: > > On Sat, Dec 09, 2017 at 09:00:18AM -0800, Joe Perches wrote: > > > On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote: > > > > 1. Using lockdep_set_novalidate_class() for anything other > > > > than device->mutex will throw checkpatch warnings. Nice. (*) > > > [] > > > > (*) checkpatch.pl is considered mostly harmful round here, too, > > > > but that's another rant > > > > > > How so? > > > > Short story is that it barfs all over the slightly non-standard > > coding style used in XFS. > [] > > This sort of stuff is just lowest-common-denominator noise - great > > for new code and/or inexperienced developers, but not for working > > with large bodies of existing code with slightly non-standard > > conventions. > > Completely reasonable. Thanks. > > Do you get many checkpatch submitters for fs/xfs? We used to. Not recently, though. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray
On Mon, 2017-12-11 at 14:43 -0800, Matthew Wilcox wrote:
> On Mon, Dec 11, 2017 at 02:12:28PM -0800, Joe Perches wrote:
> > Completely reasonable. Thanks.
>
> If we're doing "completely reasonable" complaints, then ...
>
> - I don't understand why plain 'unsigned' is deemed bad.
That was a David Miller preference.
> - The rule about all function parameters in prototypes having a name
>doesn't make sense. Example:
>
> int ida_get_new_above(struct ida *ida, int starting_id, int *p_id);
Improvements to regex welcomed.
> - Forcing a blank line after variable declarations sometimes makes for
>some weird-looking code.
True. I don't care for this one myself.
>Constructively, I think this warning can be suppressed for blocks
>that are under, say, 8 lines.
Not easy to do as checkpatch works on patches.
> 6) Functions
>
>
> Functions should be short and sweet, and do just one thing. They should
> fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
> as we all know), and do one thing and do that well.
>
>I'm not expecting you to be able to write a perl script that checks
>the first line, but we have way too many 200-plus line functions in
>the kernel. I'd like a warning on anything over 200 lines (a factor
>of 4 over Linus's stated goal).
Maybe reasonable.
Some declaration blocks for things like:
void foo(void)
{
static const struct foobar array[] = {
{ long count of lines... };
[body]
}
might make that warning unreasonable though.
> - I don't understand the error for xa_head here:
>
> struct xarray {
> spinlock_t xa_lock;
> gfp_t xa_flags;
> void __rcu *xa_head;
> };
>
>Do people really think that:
>
> struct xarray {
> spinlock_t xa_lock;
> gfp_t xa_flags;
> void __rcu*xa_head;
> };
>
>is more aesthetically pleasing? And not just that, but it's an *error*
>so the former is *RIGHT* and this is *WRONG*. And not just a matter
>of taste?
No opinion really.
That's from Andy Whitcroft's original implementation.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: serial: qcserial: add Sierra Wireless EM7565
On Mon, Dec 11, 2017 at 09:48:00PM +0100, Bjørn Mork wrote: > Reinhard Speyerer writes: > > > Sierra Wireless EM7565 devices use the DEVICE_SWI layout for their > > serial ports > > > > T: Bus=01 Lev=03 Prnt=29 Port=01 Cnt=02 Dev#= 31 Spd=480 MxCh= 0 > > D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 > > P: Vendor=1199 ProdID=9091 Rev= 0.06 > > S: Manufacturer=Sierra Wireless, Incorporated > > S: Product=Sierra Wireless EM7565 Qualcomm Snapdragon X16 LTE-A > > S: SerialNumber= > > C:* #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=500mA > > I:* If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial > > E: Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > > E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > > I:* If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=qcserial > > E: Ad=83(I) Atr=03(Int.) MxPS= 10 Ivl=32ms > > E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > > E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > > I:* If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=qcserial > > E: Ad=85(I) Atr=03(Int.) MxPS= 10 Ivl=32ms > > E: Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > > E: Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > > I:* If#= 8 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan > > E: Ad=86(I) Atr=03(Int.) MxPS= 8 Ivl=32ms > > E: Ad=8e(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > > E: Ad=0f(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > > > > but need sendsetup = true for the GPS port to make it work properly. > > Add a new DEVICE_SW2 layout variant and use it for the EM7565 PID 0x9091. > > Add a DEVICE_SWI entry for the EM7565 QDL PID 0x9090. > > I wonder if the new variant is strictly necessary? I got a feeling that > the old behaviour is arbitrary, and that older Sierra devices might work > both with and without sendsetup too. > > Did you try sendsetup on the NMEA port with any older Sierra device? > > I just did a quick test on the EM7455 in my laptop, and it doesn't seem > to mind at least. > Hi Bjørn, I did a quick test with a MC7304 running firmware 05.05.67.00 and the NMEA port works too with sendsetup enabled. The introduction of the QCSERIAL_SW2 layout was an attempt to avoid potential regressions with other devices as my testing possibilities are limited to MC77xx/MC73xx/MC74xx devices. If you and Johan consider it safe to always use sendsetup on the NMEA port for the QCSERIAL_SWI layout I could submit this version as v2 of the patch. Regards, Reinhard -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 08/73] xarray: Add documentation
On 12/05/2017 04:40 PM, Matthew Wilcox wrote: > From: Matthew Wilcox > > This is documentation on how to use the XArray, not details about its > internal implementation. > > Signed-off-by: Matthew Wilcox > --- > Documentation/core-api/index.rst | 1 + > Documentation/core-api/xarray.rst | 281 > ++ > 2 files changed, 282 insertions(+) > create mode 100644 Documentation/core-api/xarray.rst > > diff --git a/Documentation/core-api/xarray.rst > b/Documentation/core-api/xarray.rst > new file mode 100644 > index ..871161539242 > --- /dev/null > +++ b/Documentation/core-api/xarray.rst > @@ -0,0 +1,281 @@ > +== > +XArray > +== > + > +Overview > + > + > +The XArray is an abstract data type which behaves like a very large array > +of pointers. It meets many of the same needs as a hash or a conventional > +resizable array. Unlike a hash, it allows you to sensibly go to the > +next or previous entry in a cache-efficient manner. In contrast to > +a resizable array, there is no need for copying data or changing MMU > +mappings in order to grow the array. It is more memory-efficient, > +parallelisable and cache friendly than a doubly-linked list. It takes > +advantage of RCU to perform lookups without locking. > + > +The XArray implementation is efficient when the indices used are > +densely clustered; hashing the object and using the hash as the index > +will not perform well. The XArray is optimised for small indices, > +but still has good performance with large indices. If your index is > +larger than ULONG_MAX then the XArray is not the data type for you. > +The most important user of the XArray is the page cache. > + > +A freshly-initialised XArray contains a ``NULL`` pointer at every index. > +Each non-``NULL`` entry in the array has three bits associated with > +it called tags. Each tag may be flipped on or off independently of > +the others. You can search for entries with a given tag set. Only tags that are set, or search for entries with some tag(s) cleared? Or is that like a mathematical set? > +Normal pointers may be stored in the XArray directly. They must be 4-byte > +aligned, which is true for any pointer returned from :c:func:`kmalloc` and > +:c:func:`alloc_page`. It isn't true for arbitrary user-space pointers, > +nor for function pointers. You can store pointers to statically allocated > +objects, as long as those objects have an alignment of at least 4. This (above) is due to the internal usage of low bits for flags? > +The XArray does not support storing :c:func:`IS_ERR` pointers; some > +conflict with data values and others conflict with entries the XArray > +uses for its own purposes. If you need to store special values which > +cannot be confused with real kernel pointers, the values 4, 8, ... 4092 > +are available. or if I know that they values are errno-range values, I can just shift them left by 2 to store them and then shift them right by 2 to use them? oh, or use the following function? > +You can also store integers between 0 and ``LONG_MAX`` in the XArray. > +You must first convert it into an entry using :c:func:`xa_mk_value`. > +When you retrieve an entry from the XArray, you can check whether it is > +a data value by calling :c:func:`xa_is_value`, and convert it back to > +an integer by calling :c:func:`xa_to_value`. > + > +An unusual feature of the XArray is the ability to create entries which > +occupy a range of indices. Once stored to, looking up any index in > +the range will return the same entry as looking up any other index in > +the range. Setting a tag on one index will set it on all of them. > +Storing to any index will store to all of them. Multi-index entries can > +be explicitly split into smaller entries, or storing ``NULL`` into any > +entry will cause the XArray to forget about the range. > + > +Normal API > +== > + > +Start by initialising an XArray, either with :c:func:`DEFINE_XARRAY` > +for statically allocated XArrays or :c:func:`xa_init` for dynamically > +allocated ones. > + > +You can then set entries using :c:func:`xa_store` and get entries > +using :c:func:`xa_load`. xa_store will overwrite any entry with the > +new entry and return the previous entry stored at that index. If you > +store %NULL, the XArray does not need to allocate memory. You can call > +:c:func:`xa_erase` to avoid inventing a GFP flags value. There is no > +difference between an entry that has never been stored to and one that > +has most recently had %NULL stored to it. > + > +You can conditionally replace an entry at an index by using > +:c:func:`xa_cmpxchg`. Like :c:func:`cmpxchg`, it will only succeed if > +the entry at that index has the 'old' value. It also returns the entry > +which was at that index; if it returns the same entry which was passed as > +'old', then :c:func:`xa_cmpxchg` succeeded. > + > +You can enquire whether a tag is set on an entry by using > +:c:func:`xa_ge
Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray
On Mon, Dec 11, 2017 at 02:12:28PM -0800, Joe Perches wrote:
> Completely reasonable. Thanks.
If we're doing "completely reasonable" complaints, then ...
- I don't understand why plain 'unsigned' is deemed bad.
- The rule about all function parameters in prototypes having a name
doesn't make sense. Example:
int ida_get_new_above(struct ida *ida, int starting_id, int *p_id);
There is zero additional value in naming 'ida'. I know it's an ida.
The struct name tells me that. If there're two struct ida pointers
in the prototype, then sure, I want to name them so I know which is
which (maybe 'src' and 'dst'). Having an unadorned 'int' parameter
to a function should be a firable offence. But there's no need to
call 'gfp_t' anything. We know it's a gfp_t. Adding 'gfp_mask'
after it doesn't tell us anything extra.
- Forcing a blank line after variable declarations sometimes makes for
some weird-looking code. For example, there is no problem with this
code (from a checkpatch PoV):
if (xa_is_sibling(entry)) {
offset = xa_to_sibling(entry);
entry = xa_entry(xas->xa, node, offset);
/* Move xa_index to the first index of this entry */
xas->xa_index = (((xas->xa_index >> node->shift) &
~XA_CHUNK_MASK) | offset) << node->shift;
}
but if I decide I don't need 'offset' outside this block, and I want
to move the declaration inside, it looks like this:
if (xa_is_sibling(entry)) {
unsigned int offset = xa_to_sibling(entry);
entry = xa_entry(xas->xa, node, offset);
/* Move xa_index to the first index of this entry */
xas->xa_index = (((xas->xa_index >> node->shift) &
~XA_CHUNK_MASK) | offset) << node->shift;
}
Does that blank line really add anything to your comprehension of the
block? It upsets my train of thought.
Constructively, I think this warning can be suppressed for blocks
that are under, say, 8 lines. Or maybe indented blocks is where I don't
want this warning. Not sure.
Here's another example where I don't think the blank line adds anything:
static inline int xa_store_empty(struct xarray *xa, unsigned long index,
void *entry, gfp_t gfp, int errno)
{
void *curr = xa_cmpxchg(xa, index, NULL, entry, gfp);
if (!curr)
return 0;
if (xa_is_err(curr))
return xa_err(curr);
return errno;
}
So line count definitely has something to do with it.
- There's no warning for the first paragraph of section 6:
6) Functions
Functions should be short and sweet, and do just one thing. They should
fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
as we all know), and do one thing and do that well.
I'm not expecting you to be able to write a perl script that checks
the first line, but we have way too many 200-plus line functions in
the kernel. I'd like a warning on anything over 200 lines (a factor
of 4 over Linus's stated goal).
- I don't understand the error for xa_head here:
struct xarray {
spinlock_t xa_lock;
gfp_t xa_flags;
void __rcu *xa_head;
};
Do people really think that:
struct xarray {
spinlock_t xa_lock;
gfp_t xa_flags;
void __rcu *xa_head;
};
is more aesthetically pleasing? And not just that, but it's an *error*
so the former is *RIGHT* and this is *WRONG*. And not just a matter
of taste?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray
On Tue, 2017-12-12 at 08:43 +1100, Dave Chinner wrote: > On Sat, Dec 09, 2017 at 09:00:18AM -0800, Joe Perches wrote: > > On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote: > > > 1. Using lockdep_set_novalidate_class() for anything other > > > than device->mutex will throw checkpatch warnings. Nice. (*) > > [] > > > (*) checkpatch.pl is considered mostly harmful round here, too, > > > but that's another rant > > > > How so? > > Short story is that it barfs all over the slightly non-standard > coding style used in XFS. [] > This sort of stuff is just lowest-common-denominator noise - great > for new code and/or inexperienced developers, but not for working > with large bodies of existing code with slightly non-standard > conventions. Completely reasonable. Thanks. Do you get many checkpatch submitters for fs/xfs? If so, could probably do something about adding a checkpatch file flag to the directory or equivalent. Maybe add something like: fs/xfs/.checkpatch where the contents turn off most everything -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray
On Sun, Dec 10, 2017 at 08:23:15PM -0800, Matthew Wilcox wrote:
> On Mon, Dec 11, 2017 at 10:57:45AM +1100, Dave Chinner wrote:
> > i.e. the fact the cmpxchg failed may not have anything to do with a
> > race condtion - it failed because the slot wasn't empty like we
> > expected it to be. There can be any number of reasons the slot isn't
> > empty - the API should not "document" that the reason the insert
> > failed was a race condition. It should document the case that we
> > "couldn't insert because there was an existing entry in the slot".
> > Let the surrounding code document the reason why that might have
> > happened - it's not for the API to assume reasons for failure.
> >
> > i.e. this API and potential internal implementation makes much
> > more sense:
> >
> > int
> > xa_store_iff_empty(...)
> > {
> > curr = xa_cmpxchg(&pag->pag_ici_xa, agino, NULL, ip, GFP_NOFS);
> > if (!curr)
> > return 0; /* success! */
> > if (!IS_ERR(curr))
> > return -EEXIST; /* failed - slot not empty */
> > return PTR_ERR(curr); /* failed - XA internal issue */
> > }
> >
> > as it replaces the existing preload and insert code in the XFS code
> > paths whilst letting us handle and document the "insert failed
> > because slot not empty" case however we want. It implies nothing
> > about *why* the slot wasn't empty, just that we couldn't do the
> > insert because it wasn't empty.
>
> Yeah, OK. So, over the top of the recent changes I'm looking at this:
>
> I'm not in love with xa_store_empty() as a name. I almost want
> xa_store_weak(), but after my MAP_FIXED_WEAK proposed name got shot
> down, I'm leery of it. "empty" is at least a concept we already have
> in the API with the comment for xa_init() talking about an empty array
> and xa_empty(). I also considered xa_store_null and xa_overwrite_null
> and xa_replace_null(). Naming remains hard.
>
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 941f38bb94a4..586b43836905 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -451,7 +451,7 @@ xfs_iget_cache_miss(
> int flags,
> int lock_flags)
> {
> - struct xfs_inode*ip, *curr;
> + struct xfs_inode*ip;
> int error;
> xfs_agino_t agino = XFS_INO_TO_AGINO(mp, ino);
> int iflags;
> @@ -498,8 +498,7 @@ xfs_iget_cache_miss(
> xfs_iflags_set(ip, iflags);
>
> /* insert the new inode */
> - curr = xa_cmpxchg(&pag->pag_ici_xa, agino, NULL, ip, GFP_NOFS);
> - error = __xa_race(curr, -EAGAIN);
> + error = xa_store_empty(&pag->pag_ici_xa, agino, ip, GFP_NOFS, -EAGAIN);
> if (error)
> goto out_unlock;
Can we avoid encoding the error to be returned into the function
call? No other generic/library API does this, so this seems like a
highly unusual special snowflake approach. I just don't see how
creating a whole new error specification convention is justified
for the case where it *might* save a line or two of error checking
code in a caller?
I'd much prefer that the API defines the error on failure, and let
the caller handle that specific error however they need to like
every other library interface we have...
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] doc: usb: chipidea: Fix typo in 'enumerate'
On Fri, 8 Dec 2017 11:53:37 -0200 Fabio Estevam wrote: > Fix the spelling of 'enumerate' in this document. Applied to the docs tree, thanks. jon -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: core: only clean up what we allocated
From: Andrey Konovalov
When cleaning up the configurations, make sure we only free the number
of configurations and interfaces that we could have allocated.
Reported-by: Andrey Konovalov
Cc: stable
Signed-off-by: Greg Kroah-Hartman
diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 55b198ba629b..93b38471754e 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -764,18 +764,21 @@ void usb_destroy_configuration(struct usb_device *dev)
return;
if (dev->rawdescriptors) {
- for (i = 0; i < dev->descriptor.bNumConfigurations; i++)
+ for (i = 0; i < dev->descriptor.bNumConfigurations &&
+ i < USB_MAXCONFIG; i++)
kfree(dev->rawdescriptors[i]);
kfree(dev->rawdescriptors);
dev->rawdescriptors = NULL;
}
- for (c = 0; c < dev->descriptor.bNumConfigurations; c++) {
+ for (c = 0; c < dev->descriptor.bNumConfigurations &&
+ c < USB_MAXCONFIG; c++) {
struct usb_host_config *cf = &dev->config[c];
kfree(cf->string);
- for (i = 0; i < cf->desc.bNumInterfaces; i++) {
+ for (i = 0; i < cf->desc.bNumInterfaces &&
+ i < USB_MAXINTERFACES; i++) {
if (cf->intf_cache[i])
kref_put(&cf->intf_cache[i]->ref,
usb_release_interface_cache);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] phy: rockchip-typec: Try to turn the PHY on several times
Bind / unbind stress testing of the USB controller on rk3399 found
that we'd often end up with lots of failures that looked like this:
phy phy-ff80.phy.9: phy poweron failed --> -110
dwc3 fe90.dwc3: failed to initialize core
dwc3: probe of fe90.dwc3 failed with error -110
Those errors were sometimes seen at bootup too, in which case USB
peripherals wouldn't work until unplugged and re-plugged in.
I spent some time trying to figure out why the PHY was failing to
power on but I wasn't able to. Possibly this has to do with the fact
that the PHY docs say that the USB controller "needs to be held in
reset to hold pipe power state in P2 before initializing the Type C
PHY" but that doesn't appear to be easy to do with the dwc3 driver
today. Messing around with the ordering of the reset vs. the PHY
initialization in the dwc3 driver didn't seem to fix things.
I did, however, find that if I simply retry the power on it seems to
have a good chance of working. So let's add some retries. I ran a
pretty tight bind/unbind loop overnight. When I did so, I found that
I need to retry between 1% and 2% of the time. Overnight I found only
a small handful of times where I needed 2 retries. I never found a
case where I needed 3 retries.
I'm completely aware of the fact that this is quite an ugly hack and I
wish I didn't have to resort to it, but I have no other real idea how
to make this hardware reliable. If Rockchip in the future can come up
with a solution we can always revert this hack. Until then, let's at
least have something that works.
This patch is tested atop Enric's latest dwc3 patch series ending at:
https://patchwork.kernel.org/patch/10095527/
...but it could be applied independently of that series without any
bad effects.
For some more details on this bug, you can refer to:
https://bugs.chromium.org/p/chromium/issues/detail?id=783464
Signed-off-by: Douglas Anderson
---
drivers/phy/rockchip/phy-rockchip-typec.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c
b/drivers/phy/rockchip/phy-rockchip-typec.c
index ee85fa0ca4b0..5c2157156ce1 100644
--- a/drivers/phy/rockchip/phy-rockchip-typec.c
+++ b/drivers/phy/rockchip/phy-rockchip-typec.c
@@ -349,6 +349,8 @@
#define MODE_DFP_USB BIT(1)
#define MODE_DFP_DPBIT(2)
+#define POWER_ON_TRIES 5
+
struct usb3phy_reg {
u32 offset;
u32 enable_bit;
@@ -818,9 +820,8 @@ static int tcphy_get_mode(struct rockchip_typec_phy *tcphy)
return mode;
}
-static int rockchip_usb3_phy_power_on(struct phy *phy)
+static int _rockchip_usb3_phy_power_on(struct rockchip_typec_phy *tcphy)
{
- struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
struct rockchip_usb3phy_port_cfg *cfg = &tcphy->port_cfgs;
const struct usb3phy_reg *reg = &cfg->pipe_status;
int timeout, new_mode, ret = 0;
@@ -867,6 +868,25 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
return ret;
}
+static int rockchip_usb3_phy_power_on(struct phy *phy)
+{
+ struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
+ int ret;
+ int tries;
+
+ for (tries = 0; tries < POWER_ON_TRIES; tries++) {
+ ret = _rockchip_usb3_phy_power_on(tcphy);
+ if (!ret)
+ break;
+ }
+
+ if (tries && !ret)
+ dev_info(tcphy->dev, "Needed %d loops to turn on\n", tries);
+
+ return ret;
+}
+
+
static int rockchip_usb3_phy_power_off(struct phy *phy)
{
struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
--
2.15.1.424.g9478a66081-goog
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray
On Sat, Dec 09, 2017 at 09:00:18AM -0800, Joe Perches wrote: > On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote: > > 1. Using lockdep_set_novalidate_class() for anything other > > than device->mutex will throw checkpatch warnings. Nice. (*) > [] > > (*) checkpatch.pl is considered mostly harmful round here, too, > > but that's another rant > > How so? Short story is that it barfs all over the slightly non-standard coding style used in XFS. It generates enough noise on incidental things we aren't important that it complicates simple things. e.g. I just moved a block of defines from one header to another, and checkpatch throws about 10 warnings on that because of whitespace. I'm just moving code - I don't want to change it and it doesn't need to be modified because it is neat and easy to read and is obviously correct. A bunch of prototypes I added another parameter to gets warnings because it uses "unsigned" for an existing parameter that I'm not changing. And so on. This sort of stuff is just lowest-common-denominator noise - great for new code and/or inexperienced developers, but not for working with large bodies of existing code with slightly non-standard conventions. Churning *lots* of code we otherwise wouldn't touch just to shut up checkpatch warnings takes time, risks regressions and makes it harder to trace the history of the code when we are trying to track down bugs. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net,stable] net: qmi_wwan: add Sierra EM7565 1199:9091
From: Sebastian Sjoholm
Sierra Wireless EM7565 is an Qualcomm MDM9x50 based M.2 modem.
The USB id is added to qmi_wwan.c to allow QMI communication
with the EM7565.
Signed-off-by: Sebastian Sjoholm
Acked-by: Bjørn Mork
---
[The corresponding qcserial patch will be submitted by Reinhard Speyerer.]
---
drivers/net/usb/qmi_wwan.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 304ec6555cd8..3cebd6683938 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1204,6 +1204,7 @@ static const struct usb_device_id products[] = {
{QMI_FIXED_INTF(0x1199, 0x9079, 10)}, /* Sierra Wireless EM74xx */
{QMI_FIXED_INTF(0x1199, 0x907b, 8)},/* Sierra Wireless EM74xx */
{QMI_FIXED_INTF(0x1199, 0x907b, 10)}, /* Sierra Wireless EM74xx */
+ {QMI_FIXED_INTF(0x1199, 0x9091, 8)},/* Sierra Wireless EM7565 */
{QMI_FIXED_INTF(0x1bbb, 0x011e, 4)},/* Telekom Speedstick LTE II
(Alcatel One Touch L100V LTE) */
{QMI_FIXED_INTF(0x1bbb, 0x0203, 2)},/* Alcatel L800MA */
{QMI_FIXED_INTF(0x2357, 0x0201, 4)},/* TP-LINK HSUPA Modem MA180 */
--
2.14.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: serial: qcserial: add Sierra Wireless EM7565
Reinhard Speyerer writes: > Sierra Wireless EM7565 devices use the DEVICE_SWI layout for their > serial ports > > T: Bus=01 Lev=03 Prnt=29 Port=01 Cnt=02 Dev#= 31 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 > P: Vendor=1199 ProdID=9091 Rev= 0.06 > S: Manufacturer=Sierra Wireless, Incorporated > S: Product=Sierra Wireless EM7565 Qualcomm Snapdragon X16 LTE-A > S: SerialNumber= > C:* #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=500mA > I:* If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=qcserial > E: Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > I:* If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=qcserial > E: Ad=83(I) Atr=03(Int.) MxPS= 10 Ivl=32ms > E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > I:* If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=qcserial > E: Ad=85(I) Atr=03(Int.) MxPS= 10 Ivl=32ms > E: Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > E: Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > I:* If#= 8 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan > E: Ad=86(I) Atr=03(Int.) MxPS= 8 Ivl=32ms > E: Ad=8e(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > E: Ad=0f(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > > but need sendsetup = true for the GPS port to make it work properly. > Add a new DEVICE_SW2 layout variant and use it for the EM7565 PID 0x9091. > Add a DEVICE_SWI entry for the EM7565 QDL PID 0x9090. I wonder if the new variant is strictly necessary? I got a feeling that the old behaviour is arbitrary, and that older Sierra devices might work both with and without sendsetup too. Did you try sendsetup on the NMEA port with any older Sierra device? I just did a quick test on the EM7455 in my laptop, and it doesn't seem to mind at least. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net,stable] net: qmi_wwan: add Sierra EM7565 1199:9091
ssjoh...@mac.com writes:
> From: Sebastian Sjoholm
>
> From: Sebastian Sjoholm
>
> Sierra Wireless EM7565 is an Qualcomm MDM9x50 based M.2 modem.
> The USB id is added to qmi_wwan.c to allow QMI communication with the EM7565.
>
> Signed-off-by: Sebastian Sjoholm
> ---
> [The corresponding qcserial patch will be submitted by Reinhard Speyerer.]
>
> ---
> drivers/net/usb/qmi_wwan.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index 304ec6555cd8..3cebd6683938 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -1204,6 +1204,7 @@ static const struct usb_device_id products[] = {
> {QMI_FIXED_INTF(0x1199, 0x9079, 10)}, /* Sierra Wireless EM74xx */
> {QMI_FIXED_INTF(0x1199, 0x907b, 8)},/* Sierra Wireless EM74xx */
> {QMI_FIXED_INTF(0x1199, 0x907b, 10)}, /* Sierra Wireless EM74xx */
> + {QMI_FIXED_INTF(0x1199, 0x9091, 8)},/* Sierra Wireless EM7565 */
> {QMI_FIXED_INTF(0x1bbb, 0x011e, 4)},/* Telekom Speedstick LTE II
> (Alcatel One Touch L100V LTE) */
> {QMI_FIXED_INTF(0x1bbb, 0x0203, 2)},/* Alcatel L800MA */
> {QMI_FIXED_INTF(0x2357, 0x0201, 4)},/* TP-LINK HSUPA Modem MA180 */
Looks good except for the duplicate 'From' line. Drop that and you can
add
Acked-by: Bjørn Mork
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net,stable] net: qmi_wwan: add Quectel BG96 2c7c:0296
Hi,
Sorry for the re-email of the patch below, clearly a beginners mistake of me
not to clear my tmp/ folder.
Please disregard this.
Regards,
Sebastian
> On Dec 11, 2017, at 21:12 , ssjoh...@mac.com wrote:
>
> From: Sebastian Sjoholm
>
> Quectel BG96 is an Qualcomm MDM9206 based IoT modem, supporting both
> CAT-M and NB-IoT. Tested hardware is BG96 mounted on Quectel development
> board (EVB). The USB id is added to qmi_wwan.c to allow QMI
> communication with the BG96.
>
> Signed-off-by: Sebastian Sjoholm
>
> ---
> drivers/net/usb/qmi_wwan.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index 720a3a248070..c750cf7c042b 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -1239,6 +1239,7 @@ static const struct usb_device_id products[] = {
> {QMI_FIXED_INTF(0x1e0e, 0x9001, 5)},/* SIMCom 7230E */
> {QMI_QUIRK_SET_DTR(0x2c7c, 0x0125, 4)}, /* Quectel EC25, EC20 R2.0
> Mini PCIe */
> {QMI_QUIRK_SET_DTR(0x2c7c, 0x0121, 4)}, /* Quectel EC21 Mini PCIe */
> + {QMI_FIXED_INTF(0x2c7c, 0x0296, 4)},/* Quectel BG96 */
>
> /* 4. Gobi 1000 devices */
> {QMI_GOBI1K_DEVICE(0x05c6, 0x9212)},/* Acer Gobi Modem Device */
> --
> 2.11.0 (Apple Git-81)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net,stable] net: qmi_wwan: add Sierra EM7565 1199:9091
From: Sebastian Sjoholm
From: Sebastian Sjoholm
Sierra Wireless EM7565 is an Qualcomm MDM9x50 based M.2 modem.
The USB id is added to qmi_wwan.c to allow QMI communication with the EM7565.
Signed-off-by: Sebastian Sjoholm
---
[The corresponding qcserial patch will be submitted by Reinhard Speyerer.]
---
drivers/net/usb/qmi_wwan.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 304ec6555cd8..3cebd6683938 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1204,6 +1204,7 @@ static const struct usb_device_id products[] = {
{QMI_FIXED_INTF(0x1199, 0x9079, 10)}, /* Sierra Wireless EM74xx */
{QMI_FIXED_INTF(0x1199, 0x907b, 8)},/* Sierra Wireless EM74xx */
{QMI_FIXED_INTF(0x1199, 0x907b, 10)}, /* Sierra Wireless EM74xx */
+ {QMI_FIXED_INTF(0x1199, 0x9091, 8)},/* Sierra Wireless EM7565 */
{QMI_FIXED_INTF(0x1bbb, 0x011e, 4)},/* Telekom Speedstick LTE II
(Alcatel One Touch L100V LTE) */
{QMI_FIXED_INTF(0x1bbb, 0x0203, 2)},/* Alcatel L800MA */
{QMI_FIXED_INTF(0x2357, 0x0201, 4)},/* TP-LINK HSUPA Modem MA180 */
--
2.14.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net,stable] net: qmi_wwan: add Quectel BG96 2c7c:0296
From: Sebastian Sjoholm
Quectel BG96 is an Qualcomm MDM9206 based IoT modem, supporting both
CAT-M and NB-IoT. Tested hardware is BG96 mounted on Quectel development
board (EVB). The USB id is added to qmi_wwan.c to allow QMI
communication with the BG96.
Signed-off-by: Sebastian Sjoholm
---
drivers/net/usb/qmi_wwan.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 720a3a248070..c750cf7c042b 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1239,6 +1239,7 @@ static const struct usb_device_id products[] = {
{QMI_FIXED_INTF(0x1e0e, 0x9001, 5)},/* SIMCom 7230E */
{QMI_QUIRK_SET_DTR(0x2c7c, 0x0125, 4)}, /* Quectel EC25, EC20 R2.0
Mini PCIe */
{QMI_QUIRK_SET_DTR(0x2c7c, 0x0121, 4)}, /* Quectel EC21 Mini PCIe */
+ {QMI_FIXED_INTF(0x2c7c, 0x0296, 4)},/* Quectel BG96 */
/* 4. Gobi 1000 devices */
{QMI_GOBI1K_DEVICE(0x05c6, 0x9212)},/* Acer Gobi Modem Device */
--
2.11.0 (Apple Git-81)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: remove the URB_NO_FSBR flag
On Mon, 11 Dec 2017, Shuah Khan wrote:
> On 12/11/2017 09:58 AM, Alan Stern wrote:
> > The URB_NO_FSBR flag has never really been used. It was introduced as
> > a potential way for UHCI to minimize PCI bus usage (by not attempting
> > full-speed bulk and control transfers more than once per frame), but
> > the flag was not set by any drivers.
> >
> > There's no point in keeping it around. This patch simplifies the API
> > by removing it. Unfortunately, it does have to be kept as part of the
> > usbfs ABI, but at least we can document in
> > include/uapi/linux/usbdevice_fs.h that it doesn't do anything.
> >
> > Signed-off-by: Alan Stern
> >
> > ---
> >
> > Shuah, I'm not sure if this will cause an interoperability problem for
> > usbip. In theory, a machine running an older kernel might set the
> > URB_NO_FSBR flag when communicating with a new kernel, which would
> > cause an error. But I don't know any reason why even an old kernel
> > would ever set the flag.
>
> Alan, I am not sure how the interoperability will manifest.
I was worried that a newer kernel might get an URB from an older kernel
that had the NO_FSBR bit set. The newer kernel wouldn't understand
the bit and might reject the URB because of it.
> > Index: usb-4.x/drivers/usb/host/uhci-q.c
> > ===
> > --- usb-4.x.orig/drivers/usb/host/uhci-q.c
> > +++ usb-4.x/drivers/usb/host/uhci-q.c
> > @@ -73,8 +73,7 @@ static void uhci_add_fsbr(struct uhci_hc
> > {
> > struct urb_priv *urbp = urb->hcpriv;
> >
> > - if (!(urb->transfer_flags & URB_NO_FSBR))
> > - urbp->fsbr = 1;
> > + urbp->fsbr = 1;
>
> So essentially fsbr gets set always. URB_NO_FSBR has no effect.
Correct.
> > Index: usb-4.x/drivers/usb/usbip/stub_rx.c
> > ===
> > --- usb-4.x.orig/drivers/usb/usbip/stub_rx.c
> > +++ usb-4.x/drivers/usb/usbip/stub_rx.c
> > @@ -412,9 +412,6 @@ static void masking_bogus_flags(struct u
> > if (is_out)
> > allowed |= URB_ZERO_PACKET;
> > /* FALLTHROUGH */
> > - case USB_ENDPOINT_XFER_CONTROL:
> > - allowed |= URB_NO_FSBR; /* only affects UHCI */
> > - /* FALLTHROUGH */
>
> usbip doesn't really do anything special for URB_NO_FSBR, other
> than just setting the flag here. This is followed by usb_submit_urb().
> Probably there is no need to set URB_NO_FSBR in USBIP to begin with,
> considering usb_submit_urb() handled that before this patch.
Agreed, I can't think of any reasonable way this would happen.
> I think it is safe to make the change.
>
> Acked-by: Shuah Khan
Okay, thanks.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: remove the URB_NO_FSBR flag
On 12/11/2017 09:58 AM, Alan Stern wrote:
> The URB_NO_FSBR flag has never really been used. It was introduced as
> a potential way for UHCI to minimize PCI bus usage (by not attempting
> full-speed bulk and control transfers more than once per frame), but
> the flag was not set by any drivers.
>
> There's no point in keeping it around. This patch simplifies the API
> by removing it. Unfortunately, it does have to be kept as part of the
> usbfs ABI, but at least we can document in
> include/uapi/linux/usbdevice_fs.h that it doesn't do anything.
>
> Signed-off-by: Alan Stern
>
> ---
>
> Shuah, I'm not sure if this will cause an interoperability problem for
> usbip. In theory, a machine running an older kernel might set the
> URB_NO_FSBR flag when communicating with a new kernel, which would
> cause an error. But I don't know any reason why even an old kernel
> would ever set the flag.
Alan, I am not sure how the interoperability will manifest.
> ===
> --- usb-4.x.orig/drivers/usb/core/urb.c
> +++ usb-4.x/drivers/usb/core/urb.c
> @@ -479,9 +479,6 @@ int usb_submit_urb(struct urb *urb, gfp_
> if (is_out)
> allowed |= URB_ZERO_PACKET;
> /* FALLTHROUGH */
> - case USB_ENDPOINT_XFER_CONTROL:
> - allowed |= URB_NO_FSBR; /* only affects UHCI */
> - /* FALLTHROUGH */
> default:/* all non-iso endpoints */
> if (!is_out)
> allowed |= URB_SHORT_NOT_OK;
> Index: usb-4.x/drivers/usb/host/uhci-q.c
> ===
> --- usb-4.x.orig/drivers/usb/host/uhci-q.c
> +++ usb-4.x/drivers/usb/host/uhci-q.c
> @@ -73,8 +73,7 @@ static void uhci_add_fsbr(struct uhci_hc
> {
> struct urb_priv *urbp = urb->hcpriv;
>
> - if (!(urb->transfer_flags & URB_NO_FSBR))
> - urbp->fsbr = 1;
> + urbp->fsbr = 1;
So essentially fsbr gets set always. URB_NO_FSBR has no effect.
> }
>
> static void uhci_urbp_wants_fsbr(struct uhci_hcd *uhci, struct urb_priv
> *urbp)
> Index: usb-4.x/drivers/usb/usbip/stub_rx.c
> ===
> --- usb-4.x.orig/drivers/usb/usbip/stub_rx.c
> +++ usb-4.x/drivers/usb/usbip/stub_rx.c
> @@ -412,9 +412,6 @@ static void masking_bogus_flags(struct u
> if (is_out)
> allowed |= URB_ZERO_PACKET;
> /* FALLTHROUGH */
> - case USB_ENDPOINT_XFER_CONTROL:
> - allowed |= URB_NO_FSBR; /* only affects UHCI */
> - /* FALLTHROUGH */
usbip doesn't really do anything special for URB_NO_FSBR, other
than just setting the flag here. This is followed by usb_submit_urb().
Probably there is no need to set URB_NO_FSBR in USBIP to begin with,
considering usb_submit_urb() handled that before this patch.
I think it is safe to make the change.
Acked-by: Shuah Khan
thanks,
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: xhci: fix incorrect memset()
On Mon, Dec 11, 2017 at 6:01 PM, Greg Kroah-Hartman
wrote:
> On Mon, Dec 11, 2017 at 02:59:13PM +0200, Mathias Nyman wrote:
>> On 11.12.2017 13:27, Arnd Bergmann wrote:
>> > gcc-8 warnings about the new driver using a memset with a bogus length:
>> >
>> > drivers/usb/host/xhci-dbgcap.c: In function 'xhci_dbc_eps_exit':
>> > drivers/usb/host/xhci-dbgcap.c:369:2: error: 'memset' used with length
>> > equal to number of elements without multiplication by element size
>> > [-Werror=memset-elt-size]
>> >
>> > It looks like the author meant to use sizeof() rather than ARRAY_SIZE()
>> > here, so use that.
>> >
>> > Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
>> > Signed-off-by: Arnd Bergmann
>> > ---
>>
>> Another patch to fix the same thing was sent earlier as a follow up to the
>> original series.
>> https://marc.info/?l=linux-usb&m=151298133524873&w=2
>>
>> But your patch includes the Fixes line with the commit id, which is nice.
>>
>> Both fix the problem, It doesn't matter for me which one gets applied
>
> They fix it in different ways, which is correct?
Both are correct, 'sizeof(dbc->eps)' is the same as 'sizeof(struct
dbc_ep) * ARRAY_SIZE(dbc->eps)'.
> And Arnd probably doesn't have the hardware to do so?
Right.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: xhci: fix incorrect memset()
On Mon, Dec 11, 2017 at 02:59:13PM +0200, Mathias Nyman wrote:
> On 11.12.2017 13:27, Arnd Bergmann wrote:
> > gcc-8 warnings about the new driver using a memset with a bogus length:
> >
> > drivers/usb/host/xhci-dbgcap.c: In function 'xhci_dbc_eps_exit':
> > drivers/usb/host/xhci-dbgcap.c:369:2: error: 'memset' used with length
> > equal to number of elements without multiplication by element size
> > [-Werror=memset-elt-size]
> >
> > It looks like the author meant to use sizeof() rather than ARRAY_SIZE()
> > here, so use that.
> >
> > Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
> > Signed-off-by: Arnd Bergmann
> > ---
>
> Another patch to fix the same thing was sent earlier as a follow up to the
> original series.
> https://marc.info/?l=linux-usb&m=151298133524873&w=2
>
> But your patch includes the Fixes line with the commit id, which is nice.
>
> Both fix the problem, It doesn't matter for me which one gets applied
They fix it in different ways, which is correct?
I'm guessing you tested yours? And Arnd probably doesn't have the
hardware to do so?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: remove the URB_NO_FSBR flag
The URB_NO_FSBR flag has never really been used. It was introduced as
a potential way for UHCI to minimize PCI bus usage (by not attempting
full-speed bulk and control transfers more than once per frame), but
the flag was not set by any drivers.
There's no point in keeping it around. This patch simplifies the API
by removing it. Unfortunately, it does have to be kept as part of the
usbfs ABI, but at least we can document in
include/uapi/linux/usbdevice_fs.h that it doesn't do anything.
Signed-off-by: Alan Stern
---
Shuah, I'm not sure if this will cause an interoperability problem for
usbip. In theory, a machine running an older kernel might set the
URB_NO_FSBR flag when communicating with a new kernel, which would
cause an error. But I don't know any reason why even an old kernel
would ever set the flag.
[as1854]
Documentation/usb/usbip_protocol.txt |1 -
drivers/usb/core/devio.c |2 --
drivers/usb/core/urb.c |3 ---
drivers/usb/host/uhci-q.c|3 +--
drivers/usb/usbip/stub_rx.c |3 ---
include/linux/usb.h |1 -
include/uapi/linux/usbdevice_fs.h|2 +-
7 files changed, 2 insertions(+), 13 deletions(-)
Index: usb-4.x/drivers/usb/core/devio.c
===
--- usb-4.x.orig/drivers/usb/core/devio.c
+++ usb-4.x/drivers/usb/core/devio.c
@@ -1681,8 +1681,6 @@ static int proc_do_submiturb(struct usb_
u |= URB_ISO_ASAP;
if (uurb->flags & USBDEVFS_URB_SHORT_NOT_OK && is_in)
u |= URB_SHORT_NOT_OK;
- if (uurb->flags & USBDEVFS_URB_NO_FSBR)
- u |= URB_NO_FSBR;
if (uurb->flags & USBDEVFS_URB_ZERO_PACKET)
u |= URB_ZERO_PACKET;
if (uurb->flags & USBDEVFS_URB_NO_INTERRUPT)
Index: usb-4.x/drivers/usb/core/urb.c
===
--- usb-4.x.orig/drivers/usb/core/urb.c
+++ usb-4.x/drivers/usb/core/urb.c
@@ -479,9 +479,6 @@ int usb_submit_urb(struct urb *urb, gfp_
if (is_out)
allowed |= URB_ZERO_PACKET;
/* FALLTHROUGH */
- case USB_ENDPOINT_XFER_CONTROL:
- allowed |= URB_NO_FSBR; /* only affects UHCI */
- /* FALLTHROUGH */
default:/* all non-iso endpoints */
if (!is_out)
allowed |= URB_SHORT_NOT_OK;
Index: usb-4.x/drivers/usb/host/uhci-q.c
===
--- usb-4.x.orig/drivers/usb/host/uhci-q.c
+++ usb-4.x/drivers/usb/host/uhci-q.c
@@ -73,8 +73,7 @@ static void uhci_add_fsbr(struct uhci_hc
{
struct urb_priv *urbp = urb->hcpriv;
- if (!(urb->transfer_flags & URB_NO_FSBR))
- urbp->fsbr = 1;
+ urbp->fsbr = 1;
}
static void uhci_urbp_wants_fsbr(struct uhci_hcd *uhci, struct urb_priv *urbp)
Index: usb-4.x/drivers/usb/usbip/stub_rx.c
===
--- usb-4.x.orig/drivers/usb/usbip/stub_rx.c
+++ usb-4.x/drivers/usb/usbip/stub_rx.c
@@ -412,9 +412,6 @@ static void masking_bogus_flags(struct u
if (is_out)
allowed |= URB_ZERO_PACKET;
/* FALLTHROUGH */
- case USB_ENDPOINT_XFER_CONTROL:
- allowed |= URB_NO_FSBR; /* only affects UHCI */
- /* FALLTHROUGH */
default:/* all non-iso endpoints */
if (!is_out)
allowed |= URB_SHORT_NOT_OK;
Index: usb-4.x/include/linux/usb.h
===
--- usb-4.x.orig/include/linux/usb.h
+++ usb-4.x/include/linux/usb.h
@@ -1293,7 +1293,6 @@ extern int usb_disabled(void);
#define URB_ISO_ASAP 0x0002 /* iso-only; use the first unexpired
* slot in the schedule */
#define URB_NO_TRANSFER_DMA_MAP0x0004 /* urb->transfer_dma valid on
submit */
-#define URB_NO_FSBR0x0020 /* UHCI-specific */
#define URB_ZERO_PACKET0x0040 /* Finish bulk OUT with short
packet */
#define URB_NO_INTERRUPT 0x0080 /* HINT: no non-error interrupt
* needed */
Index: usb-4.x/include/uapi/linux/usbdevice_fs.h
===
--- usb-4.x.orig/include/uapi/linux/usbdevice_fs.h
+++ usb-4.x/include/uapi/linux/usbdevice_fs.h
@@ -79,7 +79,7 @@ struct usbdevfs_connectinfo {
#define USBDEVFS_URB_SHORT_NOT_OK 0x01
#define USBDEVFS_URB_ISO_ASAP 0x02
#define USBDEVFS_URB_BULK_CONTINUATION 0x04
-#define USBDEVFS_URB_NO_FSBR 0x20
+#define USBDEVFS_URB_NO_FSBR 0x20/* Not used */
#define USBDEVFS_URB_ZERO_PACKET 0x40
#define USBDEVFS_URB_NO_INTERRUPT 0x80
Index: usb-4.x/Documentation/usb/
Re: [PATCH] Revert "usb: gadget: allow to enable legacy drivers without USB_ETH"
On Mon, Dec 11, 2017 at 5:45 PM, Bart Van Assche wrote: > Romain Izard reported the following about commit 7a9618a22aad: > > As it reached Linus' tree with v4.15-rc3, I recently noticed the > following commit that triggered a Kconfig request. I believe that this > change does not make sense. > > 7a9618a22aa usb: gadget: allow to enable legacy drivers without USB_ETH > > USB_ETH was not a dependency, but a default value for the choice. As the > choice was marked as "optional", it was possible to remove this value > when building. > > After this modification, the Kconfig choice option does not contain > anything anymore, so it is useless. It is also possible to select > multiple built-in legacy drivers. This builds, but will not work as > expected as only one legacy driver can be bound to an USB device > controller at a time. > > Hence revert commit 7a9618a22aad. > > Signed-off-by: Bart Van Assche > Cc: Romain Izard > Cc: Arnd Bergmann > Cc: Stephen Rothwell > Cc: Masahiro Yamada > Cc: Hannes Reinecke > Cc: Nicholas Bellinger > Cc: Andrzej Pietrasiewicz > Cc: linux-usb@vger.kernel.org > Cc: Felipe Balbi Ok, let's use this one instead of my patch then, Acked-by: Arnd Bergmann -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][usb-next] usb: xhci: make function xhci_dbc_free_req static
From: Colin Ian King
Function xhci_dbc_free_req is local to the source and does not need to
be in global scope, so make it static.
Cleans up sparse warning:
symbol 'xhci_dbc_free_req' was not declared. Should it be static?
Signed-off-by: Colin Ian King
---
drivers/usb/host/xhci-dbgtty.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-dbgtty.c b/drivers/usb/host/xhci-dbgtty.c
index 48811d72a94c..8d47b6fbf973 100644
--- a/drivers/usb/host/xhci-dbgtty.c
+++ b/drivers/usb/host/xhci-dbgtty.c
@@ -122,7 +122,7 @@ static void dbc_write_complete(struct xhci_hcd *xhci,
struct dbc_request *req)
spin_unlock(&port->port_lock);
}
-void xhci_dbc_free_req(struct dbc_ep *dep, struct dbc_request *req)
+static void xhci_dbc_free_req(struct dbc_ep *dep, struct dbc_request *req)
{
kfree(req->buf);
dbc_free_request(dep, req);
--
2.14.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: dra7: Disable USB metastability workaround for USB2
* Felipe Balbi [171204 11:43]: > Roger Quadros writes: > > > The metastability workaround causes Erratic errors [1] > > on the HighSpeed USB PHY which can cause upto 2 seconds > > delay in enumerating to a USB host while in Gadget mode. > > > > Disable the Run/Stop metastability workaround to avoid this > > ill effect. We are aware that this opens up the opportunity > > for Run/Stop metastability, however this issue has never been > > observed in TI releases so we think that Run/Stop metastability > > is a lesser evil than the PHY Erratic errors. So disable it. > > > > [1] USB controller trace during gadget enumeration > > irq/90-dwc3-969 [000] d...52.323145: dwc3_event: event > > (0901): Erratic Error [U0] > > irq/90-dwc3-969 [000] d...52.560646: dwc3_event: event > > (0901): Erratic Error [U0] > > irq/90-dwc3-969 [000] d...52.798144: dwc3_event: event > > (0901): Erratic Error [U0] > > > > Signed-off-by: Roger Quadros > > FWIW: > > Acked-by: Felipe Balbi > > I'm taking the dwc3 counterpart to v4.16 OK applying this into omap-for-v4.16/dt thanks. Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Revert "usb: gadget: allow to enable legacy drivers without USB_ETH"
Romain Izard reported the following about commit 7a9618a22aad: As it reached Linus' tree with v4.15-rc3, I recently noticed the following commit that triggered a Kconfig request. I believe that this change does not make sense. 7a9618a22aa usb: gadget: allow to enable legacy drivers without USB_ETH USB_ETH was not a dependency, but a default value for the choice. As the choice was marked as "optional", it was possible to remove this value when building. After this modification, the Kconfig choice option does not contain anything anymore, so it is useless. It is also possible to select multiple built-in legacy drivers. This builds, but will not work as expected as only one legacy driver can be bound to an USB device controller at a time. Hence revert commit 7a9618a22aad. Signed-off-by: Bart Van Assche Cc: Romain Izard Cc: Arnd Bergmann Cc: Stephen Rothwell Cc: Masahiro Yamada Cc: Hannes Reinecke Cc: Nicholas Bellinger Cc: Andrzej Pietrasiewicz Cc: linux-usb@vger.kernel.org Cc: Felipe Balbi diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 31cce7805eb2..0a19a76645ad 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -508,8 +508,8 @@ choice controller, and the relevant drivers for each function declared by the device. -source "drivers/usb/gadget/legacy/Kconfig" - endchoice +source "drivers/usb/gadget/legacy/Kconfig" + endif # USB_GADGET diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig index a12fb459dbd9..9570bbeced4f 100644 --- a/drivers/usb/gadget/legacy/Kconfig +++ b/drivers/usb/gadget/legacy/Kconfig @@ -13,6 +13,14 @@ # both kinds of controller can also support "USB On-the-Go" (CONFIG_USB_OTG). # +menuconfig USB_GADGET_LEGACY + bool "Legacy USB Gadget Support" + help + Legacy USB gadgets are USB gadgets that do not use the USB gadget + configfs interface. + +if USB_GADGET_LEGACY + config USB_ZERO tristate "Gadget Zero (DEVELOPMENT)" select USB_LIBCOMPOSITE @@ -490,3 +498,5 @@ config USB_G_WEBCAM Say "y" to link the driver statically, or "m" to build a dynamically linked module called "g_webcam". + +endif : --- drivers/usb/gadget/Kconfig| 4 ++-- drivers/usb/gadget/legacy/Kconfig | 10 -- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 0a19a76645ad..31cce7805eb2 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -508,8 +508,8 @@ choice controller, and the relevant drivers for each function declared by the device. -endchoice - source "drivers/usb/gadget/legacy/Kconfig" +endchoice + endif # USB_GADGET diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig index 9570bbeced4f..a12fb459dbd9 100644 --- a/drivers/usb/gadget/legacy/Kconfig +++ b/drivers/usb/gadget/legacy/Kconfig @@ -13,14 +13,6 @@ # both kinds of controller can also support "USB On-the-Go" (CONFIG_USB_OTG). # -menuconfig USB_GADGET_LEGACY - bool "Legacy USB Gadget Support" - help - Legacy USB gadgets are USB gadgets that do not use the USB gadget - configfs interface. - -if USB_GADGET_LEGACY - config USB_ZERO tristate "Gadget Zero (DEVELOPMENT)" select USB_LIBCOMPOSITE @@ -498,5 +490,3 @@ config USB_G_WEBCAM Say "y" to link the driver statically, or "m" to build a dynamically linked module called "g_webcam". - -endif -- 2.15.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with 7a9618a22aa usb: gadget: allow to enable legacy drivers without USB_ETH
2017-12-11 17:14 GMT+01:00 Bart Van Assche : > On Mon, 2017-12-11 at 12:24 +0100, Romain Izard wrote: >> As it reached Linus' tree with v4.15-rc3, I recently noticed the >> following commit that triggered a Kconfig request. I believe that this >> change does not make sense. >> >> 7a9618a22aa usb: gadget: allow to enable legacy drivers without USB_ETH >> >> USB_ETH was not a dependency, but a default value for the choice. As the >> choice was marked as "optional", it was possible to remove this value >> when building. >> >> After this modification, the Kconfig choice option does not contain >> anything anymore, so it is useless. It is also possible to select >> multiple built-in legacy drivers. This builds, but will not work as >> expected as only one legacy driver can be bound to an USB device >> controller at a time. > > Hello Romain, > > Sorry but it seams like I misread the drivers/usb/gadget/Kconfig file. Are > you OK with a revert of commit 7a9618a22aa? A revert or Arnd's patch, either of these is OK for me. Best regards, -- Romain Izard -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: gadget: restore tristate-choice for legacy gadgets
On Mon, Dec 11, 2017 at 5:09 PM, Bart Van Assche wrote:
> On Mon, 2017-12-11 at 12:30 +0100, Arnd Bergmann wrote:
>> One patch that was meant as a cleanup apparently did more than it intended,
>> allowing all combinations of legacy gadget drivers to be built into the
>> kernel, and leaving an empty 'choice' statement behind:
>>
>> drivers/usb/gadget/Kconfig:487:warning: choice default symbol 'USB_ETH' is
>> not contained in the choice
>>
>> The description of commit 7a9618a22aad ("usb: gadget: allow to enable legacy
>> drivers without USB_ETH") was a bit cryptic, as it did not change the
>> behavior of USB_ETH other than allowing it to be built into the kernel
>> alongside other legacy gadgets, which is not a valid configuration.
>>
>> As Felipe explained in the description for commit bc49d1d17dcf ("usb:
>> gadget: don't couple configfs to legacy gadgets"), the configfs based
>> gadgets can be freely configured as loadable modules or built-in
>> drivers, but the legacy gadgets can only be modules if there is more
>> than one of them, so we require the 'choice' statement here.
>>
>> This leaves the added USB_GADGET_LEGACY menuconfig symbol in place,
>> but then restores the 'choice' below it, so we can enforce the
>> single-legacy-gadget rule as before.
>
> Hello Arnd,
>
> A discussion is ongoing about whether or not commit 7a9618a22aad should be
> reverted.
> Please drop this patch until a conclusion has been reached.
Ok. I'll use a revert of 7a9618a22aad in my local test tree then.
Reverting that is probably good, I thought about suggesting that
instead, but couldn't tell whether you had a bigger plan behind that
commit.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: gadget: restore tristate-choice for legacy gadgets
On Mon, 2017-12-11 at 12:30 +0100, Arnd Bergmann wrote:
> One patch that was meant as a cleanup apparently did more than it intended,
> allowing all combinations of legacy gadget drivers to be built into the
> kernel, and leaving an empty 'choice' statement behind:
>
> drivers/usb/gadget/Kconfig:487:warning: choice default symbol 'USB_ETH' is
> not contained in the choice
>
> The description of commit 7a9618a22aad ("usb: gadget: allow to enable legacy
> drivers without USB_ETH") was a bit cryptic, as it did not change the
> behavior of USB_ETH other than allowing it to be built into the kernel
> alongside other legacy gadgets, which is not a valid configuration.
>
> As Felipe explained in the description for commit bc49d1d17dcf ("usb:
> gadget: don't couple configfs to legacy gadgets"), the configfs based
> gadgets can be freely configured as loadable modules or built-in
> drivers, but the legacy gadgets can only be modules if there is more
> than one of them, so we require the 'choice' statement here.
>
> This leaves the added USB_GADGET_LEGACY menuconfig symbol in place,
> but then restores the 'choice' below it, so we can enforce the
> single-legacy-gadget rule as before.
Hello Arnd,
A discussion is ongoing about whether or not commit 7a9618a22aad should be
reverted.
Please drop this patch until a conclusion has been reached.
Thanks,
Bart.
Re: Webcams not recognized on a Dell Latitude 5285 laptop
> > > it looks like you need the experimental driver posted here > > > https://www.spinics.net/lists/linux-media/msg123268.html > > Thanks for the information. >> So, if I understand correctly, this driver will not be included in 4.15, will > > it? > > Any idea when this will be included in a release? > I have no idea. Could you contact the original developers? > The answer is interesting, but I have no idea. It seems it will be included in the 4.16 release: https://www.mail-archive.com/linux-media@vger.kernel.org/msg122619.html Probably just a bit too late for 4.15. Frederic -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dwc2 gadget driver does not generate DISABLE event on unplugging USB cable
Hi Jun Sun, On 12/8/2017 17:33, Jun Sun wrote: > Hi, all, > > I have a raspberry pi zero w board and I'm currently configuring the > device as USB gadget and connects it to PC. > > I'm using configfs/functionfs to send a vendor specific interface with > single buik in and bulk out endpoint each. > > When I unplug the USB cable, there is no DISABLE event generated on > ep0 until next time the cable is re-plugged in again, right before the > next ENABLE event. Obviously this is not very useful as I really need > to know exactly when a unpllugging is happening. > > You can see a debugging trace below. > > After looking around, I finally got it working by the attached patch. > However, I have very little confidence in the patch itself. Would > appreciate any feedback on the issue and the correct fix. > > Cheers. > > Jun > > > > > [ 130.406932] ffs_epfile_io_complete() > [ 130.406964] == JSUN == ffs_func_disable() called > [ 130.406980] __ffs_event_add(DISABLE) > [ 130.406991] adding event 3 > [ 130.435178] ffs_epfile_release() > [ 130.435193] ffs_data_closed() > [ 130.435199] ffs_data_put() > [ 130.435325] ffs_epfile_release() > [ 130.435333] ffs_data_closed() > [ 130.435338] ffs_data_put() > [ 130.435758] ffs_ep0_read() > [ 130.461705] dwc2 2098.usb: new device is high-speed > [ 130.554583] dwc2 2098.usb: new device is high-speed > [ 130.591376] dwc2 2098.usb: new address 17 > [ 130.827451] dwc2 2098.usb: new device is high-speed > [ 130.918627] dwc2 2098.usb: new device is high-speed > [ 130.962062] dwc2 2098.usb: new address 18 > [ 131.152381] dwc2 2098.usb: new device is high-speed > [ 131.243481] dwc2 2098.usb: new device is high-speed > [ 131.285993] dwc2 2098.usb: new address 19 > [ 131.461014] configfs-gadget gadget: high-speed config #1: c > [ 131.461057] __ffs_event_add(ENABLE) > [ 131.461066] adding event 2 > [ 131.464892] ffs_epfile_open() > [ 131.464908] ffs_data_opened() > [ 131.465261] ffs_epfile_open() > [ 131.465272] ffs_data_opened() > [ 131.488760] ffs_epfile_read_iter() > [ 131.488835] ffs_ep0_read() > > > > > [ 151.214987] ffs_epfile_io_complete() > [ 151.215020] == JSUN == ffs_func_disable() called > [ 151.215038] __ffs_event_add(DISABLE) > [ 151.215048] adding event 3 > [ 151.246561] ffs_epfile_release() > [ 151.246577] ffs_data_closed() > [ 151.246583] ffs_data_put() > [ 151.247797] ffs_epfile_release() > [ 151.247811] ffs_data_closed() > [ 151.247817] ffs_data_put() > [ 151.248227] ffs_ep0_read() > [ 151.269764] dwc2 2098.usb: new device is high-speed > [ 151.363232] dwc2 2098.usb: new device is high-speed > [ 151.400091] dwc2 2098.usb: new address 20 > [ 151.638217] dwc2 2098.usb: new device is high-speed > [ 151.729145] dwc2 2098.usb: new device is high-speed > [ 151.773919] dwc2 2098.usb: new address 21 > [ 151.962998] dwc2 2098.usb: new device is high-speed > [ 152.054036] dwc2 2098.usb: new device is high-speed > [ 152.096922] dwc2 2098.usb: new address 22 > [ 152.267818] configfs-gadget gadget: high-speed config #1: c > [ 152.267863] __ffs_event_add(ENABLE) > [ 152.267874] adding event 2 > [ 152.273430] ffs_epfile_open() > [ 152.273446] ffs_data_opened() > [ 152.273811] ffs_epfile_open() > [ 152.273822] ffs_dat[ 152.296847] ffs_epfile_read_iter() > [ 152.296921] ffs_ep0_read() > [ 251.986224] random: crng init done > Please try to apply those patches. Please tell me does they fix your issue or not? [PATCH v2 0/3] dwc2 fixes for edge cases on hikey [PATCH 1/3 v2] usb: dwc2: Improve gadget state disconnection handling [PATCH 2/3 v2] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode [PATCH 3/3 v2] usb: dwc2: Fix UDC state tracking You can find this patches on lkml. Submitted by John Stultz. Thanks, Grigor. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Webcams not recognized on a Dell Latitude 5285 laptop
Am Montag, den 11.12.2017, 15:33 +0100 schrieb FRÉDÉRIC PARRENIN : > > > > Am Montag, den 11.12.2017, 15:01 +0100 schrieb FRÉDÉRIC PARRENIN : > > > > > > Thank you for your answer. > > > The information is provided below. > > > > > Hi, > > > > > it looks like you need the experimental driver posted here > > > > > https://www.spinics.net/lists/linux-media/msg123268.html > > Thanks for the information. > So, if I understand correctly, this driver will not be included in 4.15, will > it? > Any idea when this will be included in a release? > I have no idea. Could you contact the original developers? The answer is interesting, but I have no idea. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Webcams not recognized on a Dell Latitude 5285 laptop
> Am Montag, den 11.12.2017, 15:01 +0100 schrieb FRÉDÉRIC PARRENIN : > > Thank you for your answer. > > The information is provided below. > Hi, > it looks like you need the experimental driver posted here > https://www.spinics.net/lists/linux-media/msg123268.html Thanks for the information. So, if I understand correctly, this driver will not be included in 4.15, will it? Any idea when this will be included in a release? Frederic > HTH > Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Webcams not recognized on a Dell Latitude 5285 laptop
Am Montag, den 11.12.2017, 15:01 +0100 schrieb FRÉDÉRIC PARRENIN : > Thank you for your answer. > The information is provided below. Hi, it looks like you need the experimental driver posted here https://www.spinics.net/lists/linux-media/msg123268.html HTH Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] usb: phy: tegra: Increase PHY clock stabilization timeout
On 11.12.2017 12:53, Thierry Reding wrote:
> On Mon, Dec 11, 2017 at 01:55:35AM +0300, Dmitry Osipenko wrote:
>> This fixes "utmi_phy_clk_enable: timeout waiting for phy to stabilize"
>> error message.
>>
>> Signed-off-by: Dmitry Osipenko
>> ---
>> drivers/usb/phy/phy-tegra-usb.c | 13 -
>> 1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/phy/phy-tegra-usb.c
>> b/drivers/usb/phy/phy-tegra-usb.c
>> index f668bfb708d3..7d5db625f800 100644
>> --- a/drivers/usb/phy/phy-tegra-usb.c
>> +++ b/drivers/usb/phy/phy-tegra-usb.c
>> @@ -16,7 +16,7 @@
>> #include
>> #include
>> #include
>> -#include
>> +#include
>> #include
>> #include
>> #include
>> @@ -305,14 +305,9 @@ static int utmip_pad_power_off(struct tegra_usb_phy
>> *phy)
>>
>> static int utmi_wait_register(void __iomem *reg, u32 mask, u32 result)
>> {
>> -unsigned long timeout = 2000;
>> -do {
>> -if ((readl(reg) & mask) == result)
>> -return 0;
>> -udelay(1);
>> -timeout--;
>> -} while (timeout);
>> -return -1;
>> +u32 tmp;
>> +
>> +return readl_poll_timeout(reg, tmp, (tmp & mask) == result, 1, 5000);
>
> Technically I think this should be readl_poll_timeout_atomic() because
> the above used to use udelay() instead of usleep_range(). But since the
> function is never used inside atomic context, this looks fine.
>
> You may want to bump the sleep time between reads to something like 10
> or 20. usleep_range() doesn't always work well with very short values,
> and given that you already bump the timeout from 2 ms to 5 ms indicates
> to me that we're actually spending a lot of time in this loop, and
> iterating somewhere between 2000 and 5000 times isn't any good. We only
> use this function to wait for the USB_PHY_CLK_VALID bit which happens
> during clock enable and disable, which isn't going to be very often, so
> even in the best case (where the clock is immediately valid) there's no
> need to return within a microsecond.
Thank you very much for the suggestion. Given that 2ms isn't enough, it should
be fine increase sleep time even to 1-2ms.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Webcams not recognized on a Dell Latitude 5285 laptop
Thank you for your answer. The information is provided below. > > Multimedia controller: Intel Corporation Skylake Imaging Unit > Please provide at least > 1) dmesg $ dmesg [0.00] microcode: microcode updated early to revision 0x62, date = 2017-04-27 [0.00] Linux version 4.14.3-300.fc27.x86_64 (mockbu...@bkernel02.phx2.fedoraproject.org) (gcc version 7.2.1 20170915 (Red Hat 7.2.1-2) (GCC)) #1 SMP Mon Dec 4 17:18:27 UTC 2017 [0.00] Command line: BOOT_IMAGE=/vmlinuz-4.14.3-300.fc27.x86_64 root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/root rd.lvm.lv=fedora/swap rhgb quiet LANG=fr_FR.UTF-8 [0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point registers' [0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers' [0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers' [0.00] x86/fpu: Supporting XSAVE feature 0x008: 'MPX bounds registers' [0.00] x86/fpu: Supporting XSAVE feature 0x010: 'MPX CSR' [0.00] x86/fpu: xstate_offset[2]: 576, xstate_sizes[2]: 256 [0.00] x86/fpu: xstate_offset[3]: 832, xstate_sizes[3]: 64 [0.00] x86/fpu: xstate_offset[4]: 896, xstate_sizes[4]: 64 [0.00] x86/fpu: Enabled xstate features 0x1f, context size is 960 bytes, using 'compacted' format. [0.00] e820: BIOS-provided physical RAM map: [0.00] BIOS-e820: [mem 0x-0x00057fff] usable [0.00] BIOS-e820: [mem 0x00058000-0x00058fff] reserved [0.00] BIOS-e820: [mem 0x00059000-0x0009efff] usable [0.00] BIOS-e820: [mem 0x0009f000-0x000f] reserved [0.00] BIOS-e820: [mem 0x0010-0xa18b7fff] usable [0.00] BIOS-e820: [mem 0xa18b8000-0xa18b8fff] ACPI NVS [0.00] BIOS-e820: [mem 0xa18b9000-0xa18b9fff] reserved [0.00] BIOS-e820: [mem 0xa18ba000-0xaa22efff] usable [0.00] BIOS-e820: [mem 0xaa22f000-0xaa5a5fff] reserved [0.00] BIOS-e820: [mem 0xaa5a6000-0xaa5ebfff] ACPI data [0.00] BIOS-e820: [mem 0xaa5ec000-0xaaf0] ACPI NVS [0.00] BIOS-e820: [mem 0xaaf1-0xab5fefff] reserved [0.00] BIOS-e820: [mem 0xab5ff000-0xab5f] usable [0.00] BIOS-e820: [mem 0xab60-0xaf7f] reserved [0.00] BIOS-e820: [mem 0xe000-0xefff] reserved [0.00] BIOS-e820: [mem 0xfe00-0xfe010fff] reserved [0.00] BIOS-e820: [mem 0xfec0-0xfec00fff] reserved [0.00] BIOS-e820: [mem 0xfee0-0xfee00fff] reserved [0.00] BIOS-e820: [mem 0xff00-0x] reserved [0.00] BIOS-e820: [mem 0x0001-0x00044e7f] usable [0.00] NX (Execute Disable) protection: active [0.00] e820: update [mem 0x9f92b018-0x9f93b057] usable ==> usable [0.00] e820: update [mem 0x9f92b018-0x9f93b057] usable ==> usable [0.00] extended physical RAM map: [0.00] reserve setup_data: [mem 0x-0x00057fff] usable [0.00] reserve setup_data: [mem 0x00058000-0x00058fff] reserved [0.00] reserve setup_data: [mem 0x00059000-0x0009efff] usable [0.00] reserve setup_data: [mem 0x0009f000-0x000f] reserved [0.00] reserve setup_data: [mem 0x0010-0x9f92b017] usable [0.00] reserve setup_data: [mem 0x9f92b018-0x9f93b057] usable [0.00] reserve setup_data: [mem 0x9f93b058-0xa18b7fff] usable [0.00] reserve setup_data: [mem 0xa18b8000-0xa18b8fff] ACPI NVS [0.00] reserve setup_data: [mem 0xa18b9000-0xa18b9fff] reserved [0.00] reserve setup_data: [mem 0xa18ba000-0xaa22efff] usable [0.00] reserve setup_data: [mem 0xaa22f000-0xaa5a5fff] reserved [0.00] reserve setup_data: [mem 0xaa5a6000-0xaa5ebfff] ACPI data [0.00] reserve setup_data: [mem 0xaa5ec000-0xaaf0] ACPI NVS [0.00] reserve setup_data: [mem 0xaaf1-0xab5fefff] reserved [0.00] reserve setup_data: [mem 0xab5ff000-0xab5f] usable [0.00] reserve setup_data: [mem 0xab60-0xaf7f] reserved [0.00] reserve setup_data: [mem 0xe000-0xefff] reserved [0.00] reserve setup_data: [mem 0xfe00-0xfe010fff] reserved [0.00] reserve setup_data: [mem 0xfec0-0xfec00fff] reserved [0.00] reserve setup_data: [mem 0xfee0-0xfee00fff] reserved [0.00] reserve setup_data: [mem 0
Re: [PATCH] USB: serial: ti_usb_3410_5052: Avoid goto in bulk in callback
On Mon, Dec 11, 2017 at 12:52:46PM +0100, Johan Hovold wrote: > On Mon, Dec 11, 2017 at 12:32:49PM +0100, Ladislav Michl wrote: > > On Mon, Dec 11, 2017 at 11:10:35AM +0100, Johan Hovold wrote: [snip] > > > I'm afraid I don't consider this an improvement. I prefer using gotos > > > for error paths, while keeping the success path out of the status > > > switch. > > > > > > Furthermore, this isn't functionally equivalent as we'd not longer log > > > an error for -EPIPE. > > > > Yes, you are right... Now, shouldn't we react somehow to stalled endpoint? > > Tty side seems to be unaware of it. > > Recovering from a stalled endpoint is a bit involved, so for now we > typically just log an error an bail out (forcing the user to reopen the > port). This seems to work well enough as this condition should be rare. I just do not see this in code. I would expect pending tty I/O operation would fail once USB device errors out with -EPIPE, so tty side consumer gets notified about error. Either it is not there or I did not look hard enough :) > > > In fact, that -EPIPE is already on my TODO list as we really should not > > > be resubmitting URBs for a stalled endpoint in the first place. That in > > > turn should allow for some clean up of this callback. > > > > Out of curiosity, how would be stalled endpoint reported? > > Simply logging that the urb has been stopped along with the URB status > (-EPIPE) should be enough. See above. > Johan Best regards, ladis -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: typec: wcove: fix the sink capabilities
USB Power Delivery Specification (v3.0) dictates in ch.
6.4.1 - Capabilities Message - that the vSafe5V Fixed Supply
Object shall always be the first object. tcpm.c now checks
that this rule is obeyed (commit 5007e1b5db73 "typec: tcpm
Validate source and sink caps"), and that makes the
typec_wcove.c fail to probe. The voltage is higher then what
is permitted for the vSafe5V parameter.
Dropping the voltage in the first Fixed Supply object of the
sink capabilities down to 5V, and maximum current down to
500mA, making the driver probe successfully again.
Also, removing the Battery and Variable Supply objects, as
there is no need for them.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/typec_wcove.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/typec/typec_wcove.c b/drivers/usb/typec/typec_wcove.c
index a8d479eb221a..2e990e0d917d 100644
--- a/drivers/usb/typec/typec_wcove.c
+++ b/drivers/usb/typec/typec_wcove.c
@@ -556,10 +556,8 @@ static const u32 src_pdo[] = {
};
static const u32 snk_pdo[] = {
- PDO_FIXED(12000, 3000, PDO_FIXED_DUAL_ROLE | PDO_FIXED_DATA_SWAP |
+ PDO_FIXED(5000, 500, PDO_FIXED_DUAL_ROLE | PDO_FIXED_DATA_SWAP |
PDO_FIXED_USB_COMM),
- PDO_BATT(4750, 12000, 15000),
- PDO_VAR(4750, 12000, 3000),
};
static struct tcpc_config wcove_typec_config = {
--
2.15.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/2] usb: phy: tegra: Cleanup error messages
On 11.12.2017 12:37, Thierry Reding wrote: > On Mon, Dec 11, 2017 at 02:07:37AM +0300, Dmitry Osipenko wrote: >> Tegra's PHY driver has a mix of pr_err() and dev_err(), let's switch to >> dev_err() and use common errors message formatting across the driver for >> consistency. >> >> Signed-off-by: Dmitry Osipenko >> --- >> drivers/usb/phy/phy-tegra-usb.c | 72 >> + >> 1 file changed, 44 insertions(+), 28 deletions(-) > > Can we also get rid of all the function names in error messages? I see > that for some error messages you've removed them, but then for others > you added them, so you remove inconsistencies on one hand and add other > inconsistencies at the same time. =) I've removed function names where they aren't useful and added where they are. Of course we can change the error message instead of using function name to give a clue from where in the code error is originated. I'll remove function names in v2. > Other than that, I like this. Thanks ;) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 2/2] usb: tegra: Move UTMI-pads reset from ehci-tegra to tegra-phy
On 11.12.2017 13:25, Thierry Reding wrote: > On Mon, Dec 11, 2017 at 02:07:38AM +0300, Dmitry Osipenko wrote: >> UTMI pads are shared by USB controllers and reset of UTMI pads is shared >> with the reset of USB1 controller. Currently reset of UTMI pads is done by >> the EHCI driver and ChipIdea UDC works because EHCI driver always happen >> to be probed first. Move reset controls from ehci-tegra to tegra-phy in >> order to resolve the problem. >> >> Signed-off-by: Dmitry Osipenko >> --- >> drivers/usb/host/ehci-tegra.c | 87 >> ++- >> drivers/usb/phy/phy-tegra-usb.c | 46 + >> include/linux/usb/tegra_usb_phy.h | 2 + >> 3 files changed, 87 insertions(+), 48 deletions(-) > > I don't think we can do this. For one I don't think shared resets are > going to work here because you really won't ever be able to reset after > two devices have requested the same reset. Ah, indeed. Originally I had the reset being done in the probe, but then changed it in the last minute without proper testing. Good catch! I'll revert back patch to the origin. Second, utmip_pad_close() > could be called at any point and it will have the side-effect of either > not doing a reset at all (because it is shared) or resetting the USBD > controller at the same time. utmip_pad_close() is only called on tegra-phy driver removal, so it is absolutely fine. > We've been over this code a great deal over the years. I'd love it to be > simpler, but every time we tried to simplify it, things broke. Well, the current code is already broken quite severely because now we have two users of the tegra-phy: ehci-tegra and chipidea-tegra. Things brake if host driver is loaded after the UDC because host would reset the UDC. And also pads won't be reset if ehci-tegra isn't loaded at all. Shared reset seems to be a perfect solution for us and of course it requires extra carefulness. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 2/2] usb: chipidea: tegra: Select Tegra's PHY in Kconfig
On 11.12.2017 13:04, Thierry Reding wrote: > On Mon, Dec 11, 2017 at 02:10:00AM +0300, Dmitry Osipenko wrote: >> UDC driver won't probe without Tegra's PHY, hence select it in the >> Kconfig. >> >> Signed-off-by: Dmitry Osipenko >> --- >> drivers/usb/chipidea/Kconfig | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig >> index 785f0ed037f7..2ef3b27ea72b 100644 >> --- a/drivers/usb/chipidea/Kconfig >> +++ b/drivers/usb/chipidea/Kconfig >> @@ -27,6 +27,7 @@ config USB_CHIPIDEA_PCI >> config USB_CHIPIDEA_UDC >> bool "ChipIdea device controller" >> depends on USB_GADGET >> +select USB_TEGRA_PHY if ARCH_TEGRA > > This is kind of pointless given that USB_TEGRA_PHY originally was > automatically enabled if ARCH_TEGRA was enabled. Again, please take a closer look at the patches. USB_TEGRA_PHY was enabled if USB_EHCI_TEGRA was and not ARCH_TEGRA. > What do we gain by these two patches, other than maybe make the driver > buildable as a module? Firstly, tegra-phy is built only if ehci-tegra is built. Secondly, I think we need to enforce Tegra PHY to be compiled as built-in if one of ehci-tegra or chipidea drivers is built-in and the other is compiled as a module. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/2] usb: phy: Add Kconfig entry for Tegra's PHY driver
On 11.12.2017 13:02, Thierry Reding wrote: > On Mon, Dec 11, 2017 at 02:09:59AM +0300, Dmitry Osipenko wrote: >> Add Kconfig entry so that other drivers other than ehci-tegra >> (like ChipIdea) could add Tegra's PHY to build dependencies. >> >> Signed-off-by: Dmitry Osipenko >> --- >> drivers/usb/host/Kconfig | 2 +- >> drivers/usb/phy/Kconfig | 8 >> drivers/usb/phy/Makefile | 2 +- >> 3 files changed, 10 insertions(+), 2 deletions(-) > > I don't think we actually build-depend on the PHY driver from the > ChipIdea driver. In the past, we've refrained from modelling runtime > dependencies using Kconfig because in some cases (such as this) it'll > include more than necessary (ChipIdea will automatically pull in the > USB PHY driver irrespective of whether or not Tegra is enabled). Please take a closer look at the patch. Tegra PHY driver is only compiled if ehci-tegra driver is compiled. So we need to decouple build dependency in order fix it. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: xhci: fix incorrect memset()
On 11.12.2017 13:27, Arnd Bergmann wrote:
gcc-8 warnings about the new driver using a memset with a bogus length:
drivers/usb/host/xhci-dbgcap.c: In function 'xhci_dbc_eps_exit':
drivers/usb/host/xhci-dbgcap.c:369:2: error: 'memset' used with length equal to
number of elements without multiplication by element size
[-Werror=memset-elt-size]
It looks like the author meant to use sizeof() rather than ARRAY_SIZE()
here, so use that.
Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
Signed-off-by: Arnd Bergmann
---
Another patch to fix the same thing was sent earlier as a follow up to the
original series.
https://marc.info/?l=linux-usb&m=151298133524873&w=2
But your patch includes the Fixes line with the commit id, which is nice.
Both fix the problem, It doesn't matter for me which one gets applied
Thanks
-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: serial: ti_usb_3410_5052: Avoid goto in bulk in callback
On Mon, Dec 11, 2017 at 12:32:49PM +0100, Ladislav Michl wrote: > On Mon, Dec 11, 2017 at 11:10:35AM +0100, Johan Hovold wrote: > > On Mon, Dec 11, 2017 at 12:09:19AM +0100, Ladislav Michl wrote: > > > Make status handling in bulk in callback function more compact, > > > which renders goto pointless. > > > > > > Signed-off-by: Ladislav Michl > > > --- > > > drivers/usb/serial/ti_usb_3410_5052.c | 36 > > > ++- > > > 1 file changed, 14 insertions(+), 22 deletions(-) > > I'm afraid I don't consider this an improvement. I prefer using gotos > > for error paths, while keeping the success path out of the status > > switch. > > > > Furthermore, this isn't functionally equivalent as we'd not longer log > > an error for -EPIPE. > > Yes, you are right... Now, shouldn't we react somehow to stalled endpoint? > Tty side seems to be unaware of it. Recovering from a stalled endpoint is a bit involved, so for now we typically just log an error an bail out (forcing the user to reopen the port). This seems to work well enough as this condition should be rare. > > In fact, that -EPIPE is already on my TODO list as we really should not > > be resubmitting URBs for a stalled endpoint in the first place. That in > > turn should allow for some clean up of this callback. > > Out of curiosity, how would be stalled endpoint reported? Simply logging that the urb has been stopped along with the URB status (-EPIPE) should be enough. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: Issues with 7a9618a22aa usb: gadget: allow to enable legacy drivers without USB_ETH
Hi, Romain Izard writes: > Once again, with the correct mail address. > > > -- Forwarded message -- > From: Romain Izard > Date: 2017-12-11 12:24 GMT+01:00 > Subject: Issues with 7a9618a22aa usb: gadget: allow to enable legacy > drivers without USB_ETH > To: Bart Van Assche , Felipe Balbi > , Hannes Reinecke > Cc : Nicholas Bellinger , Andrzej Pietrasiewicz > > > > Hello, > > As it reached Linus' tree with v4.15-rc3, I recently noticed the > following commit that triggered a Kconfig request. I believe that this > change does not make sense. > > 7a9618a22aa usb: gadget: allow to enable legacy drivers without USB_ETH > > USB_ETH was not a dependency, but a default value for the choice. As the > choice was marked as "optional", it was possible to remove this value > when building. > > After this modification, the Kconfig choice option does not contain > anything anymore, so it is useless. It is also possible to select > multiple built-in legacy drivers. This builds, but will not work as > expected as only one legacy driver can be bound to an USB device > controller at a time. check Arnd's fix: https://marc.info/?i=2017123048.3514863-1-a...@arndb.de -- balbi signature.asc Description: PGP signature
Re: [PATCH] USB: serial: ti_usb_3410_5052: Avoid goto in bulk in callback
On Mon, Dec 11, 2017 at 11:10:35AM +0100, Johan Hovold wrote:
> On Mon, Dec 11, 2017 at 12:09:19AM +0100, Ladislav Michl wrote:
> > Make status handling in bulk in callback function more compact,
> > which renders goto pointless.
> >
> > Signed-off-by: Ladislav Michl
> > ---
> > drivers/usb/serial/ti_usb_3410_5052.c | 36
> > ++-
> > 1 file changed, 14 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/usb/serial/ti_usb_3410_5052.c
> > b/drivers/usb/serial/ti_usb_3410_5052.c
> > index 6b22857f6e52..2786ec7bbca2 100644
> > --- a/drivers/usb/serial/ti_usb_3410_5052.c
> > +++ b/drivers/usb/serial/ti_usb_3410_5052.c
> > @@ -1219,29 +1219,10 @@ static void ti_bulk_in_callback(struct urb *urb)
> >
> > switch (status) {
> > case 0:
> > - break;
> > - case -ECONNRESET:
> > - case -ENOENT:
> > - case -ESHUTDOWN:
> > - dev_dbg(dev, "%s - urb shutting down, %d\n", __func__, status);
> > - return;
> > - default:
> > - dev_err(dev, "%s - nonzero urb status, %d\n",
> > - __func__, status);
> > - }
> > -
> > - if (status == -EPIPE)
> > - goto exit;
> > -
> > - if (status) {
> > - dev_err(dev, "%s - stopping read!\n", __func__);
> > - return;
> > - }
> > -
> > - if (urb->actual_length) {
> > + if (!urb->actual_length)
> > + break;
> > usb_serial_debug_data(dev, __func__, urb->actual_length,
> > urb->transfer_buffer);
> > -
> > if (!tport->tp_is_open)
> > dev_dbg(dev, "%s - port closed, dropping data\n",
> > __func__);
> > @@ -1250,9 +1231,20 @@ static void ti_bulk_in_callback(struct urb *urb)
> > spin_lock(&tport->tp_lock);
> > port->icount.rx += urb->actual_length;
> > spin_unlock(&tport->tp_lock);
> > + break;
> > + case -ECONNRESET:
> > + case -ENOENT:
> > + case -ESHUTDOWN:
> > + dev_dbg(dev, "%s - urb shutting down, %d\n", __func__, status);
> > + return;
> > + case -EPIPE:
> > + break;
> > + default:
> > + dev_err(dev, "%s - nonzero urb status, %d\n",
> > + __func__, status);
> > + return;
>
> I'm afraid I don't consider this an improvement. I prefer using gotos
> for error paths, while keeping the success path out of the status
> switch.
>
> Furthermore, this isn't functionally equivalent as we'd not longer log
> an error for -EPIPE.
Yes, you are right... Now, shouldn't we react somehow to stalled endpoint?
Tty side seems to be unaware of it.
> In fact, that -EPIPE is already on my TODO list as we really should not
> be resubmitting URBs for a stalled endpoint in the first place. That in
> turn should allow for some clean up of this callback.
Out of curiosity, how would be stalled endpoint reported?
> Thanks,
> Johan
Thank you,
ladis
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: gadget: webcam: fix V4L2 Kconfig dependency
Configuring the USB_G_WEBCAM driver as built-in leads to a link error when CONFIG_VIDEO_V4L2 is a loadable module: drivers/usb/gadget/function/f_uvc.o: In function `uvc_function_setup': f_uvc.c:(.text+0xfe): undefined reference to `v4l2_event_queue' drivers/usb/gadget/function/f_uvc.o: In function `uvc_function_ep0_complete': f_uvc.c:(.text+0x188): undefined reference to `v4l2_event_queue' This changes the Kconfig dependency to disallow that configuration, and force it to be a module in that case as well. This is apparently a rather old bug, but very hard to trigger even in thousands of randconfig builds. Signed-off-by: Arnd Bergmann --- drivers/usb/gadget/legacy/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig index 2d80a9d1d5d9..fbd974965399 100644 --- a/drivers/usb/gadget/legacy/Kconfig +++ b/drivers/usb/gadget/legacy/Kconfig @@ -513,7 +513,7 @@ endif # or video class gadget drivers), or specific hardware, here. config USB_G_WEBCAM tristate "USB Webcam Gadget" - depends on VIDEO_DEV + depends on VIDEO_V4L2 select USB_LIBCOMPOSITE select VIDEOBUF2_VMALLOC select USB_F_UVC -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: gadget: restore tristate-choice for legacy gadgets
One patch that was meant as a cleanup apparently did more than it intended,
allowing all combinations of legacy gadget drivers to be built into the
kernel, and leaving an empty 'choice' statement behind:
drivers/usb/gadget/Kconfig:487:warning: choice default symbol 'USB_ETH' is not
contained in the choice
The description of commit 7a9618a22aad ("usb: gadget: allow to enable legacy
drivers without USB_ETH") was a bit cryptic, as it did not change the
behavior of USB_ETH other than allowing it to be built into the kernel
alongside other legacy gadgets, which is not a valid configuration.
As Felipe explained in the description for commit bc49d1d17dcf ("usb:
gadget: don't couple configfs to legacy gadgets"), the configfs based
gadgets can be freely configured as loadable modules or built-in
drivers, but the legacy gadgets can only be modules if there is more
than one of them, so we require the 'choice' statement here.
This leaves the added USB_GADGET_LEGACY menuconfig symbol in place,
but then restores the 'choice' below it, so we can enforce the
single-legacy-gadget rule as before.
Fixes: 7a9618a22aad ("usb: gadget: allow to enable legacy drivers without
USB_ETH")
Signed-off-by: Arnd Bergmann
---
drivers/usb/gadget/Kconfig| 28
drivers/usb/gadget/legacy/Kconfig | 28
2 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 0a19a76645ad..eab61f552c19 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -482,34 +482,6 @@ config USB_CONFIGFS_F_TCM
Both protocols can work on USB2.0 and USB3.0.
UAS utilizes the USB 3.0 feature called streams support.
-choice
- tristate "USB Gadget precomposed configurations"
- default USB_ETH
- optional
- help
- A Linux "Gadget Driver" talks to the USB Peripheral Controller
- driver through the abstract "gadget" API. Some other operating
- systems call these "client" drivers, of which "class drivers"
- are a subset (implementing a USB device class specification).
- A gadget driver implements one or more USB functions using
- the peripheral hardware.
-
- Gadget drivers are hardware-neutral, or "platform independent",
- except that they sometimes must understand quirks or limitations
- of the particular controllers they work with. For example, when
- a controller doesn't support alternate configurations or provide
- enough of the right types of endpoints, the gadget driver might
- not be able work with that controller, or might need to implement
- a less common variant of a device class protocol.
-
- The available choices each represent a single precomposed USB
- gadget configuration. In the device model, each option contains
- both the device instantiation as a child for a USB gadget
- controller, and the relevant drivers for each function declared
- by the device.
-
-endchoice
-
source "drivers/usb/gadget/legacy/Kconfig"
endif # USB_GADGET
diff --git a/drivers/usb/gadget/legacy/Kconfig
b/drivers/usb/gadget/legacy/Kconfig
index 9570bbeced4f..2d80a9d1d5d9 100644
--- a/drivers/usb/gadget/legacy/Kconfig
+++ b/drivers/usb/gadget/legacy/Kconfig
@@ -21,6 +21,32 @@ menuconfig USB_GADGET_LEGACY
if USB_GADGET_LEGACY
+choice
+ tristate "USB Gadget precomposed configurations"
+ default USB_ETH
+ optional
+ help
+ A Linux "Gadget Driver" talks to the USB Peripheral Controller
+ driver through the abstract "gadget" API. Some other operating
+ systems call these "client" drivers, of which "class drivers"
+ are a subset (implementing a USB device class specification).
+ A gadget driver implements one or more USB functions using
+ the peripheral hardware.
+
+ Gadget drivers are hardware-neutral, or "platform independent",
+ except that they sometimes must understand quirks or limitations
+ of the particular controllers they work with. For example, when
+ a controller doesn't support alternate configurations or provide
+ enough of the right types of endpoints, the gadget driver might
+ not be able work with that controller, or might need to implement
+ a less common variant of a device class protocol.
+
+ The available choices each represent a single precomposed USB
+ gadget configuration. In the device model, each option contains
+ both the device instantiation as a child for a USB gadget
+ controller, and the relevant drivers for each function declared
+ by the device.
+
config USB_ZERO
tristate "Gadget Zero (DEVELOPMENT)"
select USB_LIBCOMPOSITE
@@ -499,4 +525,6 @@ config USB_G_WEBCAM
Say "y" to link the driver statically, or "m" to build
[PATCH] usb: xhci: fix incorrect memset()
gcc-8 warnings about the new driver using a memset with a bogus length:
drivers/usb/host/xhci-dbgcap.c: In function 'xhci_dbc_eps_exit':
drivers/usb/host/xhci-dbgcap.c:369:2: error: 'memset' used with length equal to
number of elements without multiplication by element size
[-Werror=memset-elt-size]
It looks like the author meant to use sizeof() rather than ARRAY_SIZE()
here, so use that.
Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
Signed-off-by: Arnd Bergmann
---
drivers/usb/host/xhci-dbgcap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
index 671e5023e683..1e535bd2be01 100644
--- a/drivers/usb/host/xhci-dbgcap.c
+++ b/drivers/usb/host/xhci-dbgcap.c
@@ -366,7 +366,7 @@ static void xhci_dbc_eps_exit(struct xhci_hcd *xhci)
{
struct xhci_dbc *dbc = xhci->dbc;
- memset(dbc->eps, 0, ARRAY_SIZE(dbc->eps));
+ memset(dbc->eps, 0, sizeof(dbc->eps));
}
static int xhci_dbc_mem_init(struct xhci_hcd *xhci, gfp_t flags)
--
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: serial: Avoid goto in bulk callbacks
Hi Oliver, Johan, On Mon, Dec 11, 2017 at 11:58:21AM +0100, Oliver Neukum wrote: > Am Montag, den 11.12.2017, 00:11 +0100 schrieb Ladislav Michl: > > Refactor bulk callback functions to make use of goto pointless. > > > > > > Hi, > > you seem to assume that this is a good thing by itself which > needs no further justification. I disagree. You duplicated > code. Where did I duplicated code? Success code path is straightforward now, so while I'm not pushing at all for this patch to be included as it is side effect of looking into another drivers while debugging something else, I really do not understand your comment. On Mon, Dec 11, 2017 at 11:13:51AM +0100, Johan Hovold wrote: > So as for the ti driver, I prefer keeping the success path out of the > switch statement. Undestood, as noted above, I have no strong preference either way. Just did what seemed easier to read to me :) Best regads, ladis -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: serial: Avoid goto in bulk callbacks
Am Montag, den 11.12.2017, 00:11 +0100 schrieb Ladislav Michl: > Refactor bulk callback functions to make use of goto pointless. > > Hi, you seem to assume that this is a good thing by itself which needs no further justification. I disagree. You duplicated code. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: Change TxFIFO and RxFIFO flushing flow
Hi Minas,
Am 11.12.2017 um 11:45 schrieb Minas Harutyunyan:
> Hi Stefan,
> On 12/8/2017 8:25 PM, Stefan Wahren wrote:
>> Hi Minas,
>>
>> Am 07.12.2017 um 17:51 schrieb Stefan Wahren:
>>> Hi Minas,
>>>
>>> Am 07.12.2017 um 09:40 schrieb Stefan Wahren:
Before flushing fifos required to check AHB master state and
flush when AHB master is in IDLE state.
Signed-off-by: Minas Harutyunyan
---
drivers/usb/dwc2/core.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index dbca3b8890da..cbc7c562477f 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -670,10 +670,23 @@ void dwc2_flush_tx_fifo(struct dwc2_hsotg *hsotg,
const int num)
dev_vdbg(hsotg->dev, "Flush Tx FIFO %d\n", num);
+ /* Wait for AHB master IDLE state */
+ do {
+ udelay(1);
>>> is this delay always necessary before reading GRSTCTL?
>>>
>>> If yes please add a comment why.
>>> If no please rework the loop in order to avoid this delay in case the
>>> AHB master is already idle.
>>>
>> i've seen your Patch V2, but it isn't what i thought of. How about:
>>
>>
>> while (1) {
>> greset = dwc2_readl(hsotg->regs + GRSTCTL);
>> if (greset & GRSTCTL_AHBIDLE)
>> break;
>>
>> if (++count > 1) {
>> dev_warn(hsotg->dev,
>> "%s() HANG! AHB Idle GRSTCTL=%0x\n",
>> __func__, greset);
>> return;
>> }
> udelay(1);
>> }
>>
> I'm Ok with above loop, just think to add udelay(1) in bottom of loop.
> udelay() required to allow AHB go to idle, if not yet.
sorry my fault, i forgot the udelay in my suggestion. I'm fine with your
addition.
>
> Actually in core.c lot of similar code: to wait for set or clear some
> bits which should be fully updated. BTW, my first patch just copied from
> dwc_core_reset() function.
>
>
>> Btw: Please provide a change history, so the maintainers can keep track
>> of your changes.
>>
>> Regards
>> Stefan
>>
> Thanks,
> Minas
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Webcams not recognized on a Dell Latitude 5285 laptop
Am Montag, den 11.12.2017, 09:05 +0100 schrieb Frédéric Parrenin : > Multimedia controller: Intel Corporation Skylake Imaging Unit Please provide at least 1) dmesg 2) lsusb -v 3) lspci -vvk 4) lspci -vvnk There is no webcam shown on your USB bus. Without dmesg it is impossible to say whether this is a problem with detection or your webcam, or it is indeed attached by another method (e.g. PCI) Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: Change TxFIFO and RxFIFO flushing flow
Hi Stefan,
On 12/8/2017 8:25 PM, Stefan Wahren wrote:
> Hi Minas,
>
> Am 07.12.2017 um 17:51 schrieb Stefan Wahren:
>> Hi Minas,
>>
>> Am 07.12.2017 um 09:40 schrieb Stefan Wahren:
>>> Before flushing fifos required to check AHB master state and
>>> flush when AHB master is in IDLE state.
>>>
>>> Signed-off-by: Minas Harutyunyan
>>> ---
>>>drivers/usb/dwc2/core.c | 27 +++
>>>1 file changed, 27 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
>>> index dbca3b8890da..cbc7c562477f 100644
>>> --- a/drivers/usb/dwc2/core.c
>>> +++ b/drivers/usb/dwc2/core.c
>>> @@ -670,10 +670,23 @@ void dwc2_flush_tx_fifo(struct dwc2_hsotg *hsotg,
>>> const int num)
>>>
>>> dev_vdbg(hsotg->dev, "Flush Tx FIFO %d\n", num);
>>>
>>> + /* Wait for AHB master IDLE state */
>>> + do {
>>> + udelay(1);
>> is this delay always necessary before reading GRSTCTL?
>>
>> If yes please add a comment why.
>> If no please rework the loop in order to avoid this delay in case the
>> AHB master is already idle.
>>
>
> i've seen your Patch V2, but it isn't what i thought of. How about:
>
>
> while (1) {
> greset = dwc2_readl(hsotg->regs + GRSTCTL);
> if (greset & GRSTCTL_AHBIDLE)
> break;
>
> if (++count > 1) {
> dev_warn(hsotg->dev,
> "%s() HANG! AHB Idle GRSTCTL=%0x\n",
> __func__, greset);
> return;
> }
udelay(1);
> }
>
I'm Ok with above loop, just think to add udelay(1) in bottom of loop.
udelay() required to allow AHB go to idle, if not yet.
Actually in core.c lot of similar code: to wait for set or clear some
bits which should be fully updated. BTW, my first patch just copied from
dwc_core_reset() function.
> Btw: Please provide a change history, so the maintainers can keep track
> of your changes.
>
> Regards
> Stefan
>
Thanks,
Minas
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb-core: Fix potential null pointer dereference in xhci-debugfs.c
On 10.12.2017 00:38, Alexander Kappner wrote: Hi Mathias, thanks for the patch! The system now resumes cleanly from hibernate even with usbmuxd doing its thing. Tested-by: Alexander Kappner While testing this I hit some other issues with xhci-debugfs.c but I'll write these up in a separate bug. Thanks, Adding reported-by and tested-by tags, and pushing patch to my for-usb-linus branch -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 2/2] usb: tegra: Move UTMI-pads reset from ehci-tegra to tegra-phy
On Mon, Dec 11, 2017 at 02:07:38AM +0300, Dmitry Osipenko wrote: > UTMI pads are shared by USB controllers and reset of UTMI pads is shared > with the reset of USB1 controller. Currently reset of UTMI pads is done by > the EHCI driver and ChipIdea UDC works because EHCI driver always happen > to be probed first. Move reset controls from ehci-tegra to tegra-phy in > order to resolve the problem. > > Signed-off-by: Dmitry Osipenko > --- > drivers/usb/host/ehci-tegra.c | 87 > ++- > drivers/usb/phy/phy-tegra-usb.c | 46 + > include/linux/usb/tegra_usb_phy.h | 2 + > 3 files changed, 87 insertions(+), 48 deletions(-) I don't think we can do this. For one I don't think shared resets are going to work here because you really won't ever be able to reset after two devices have requested the same reset. Second, utmip_pad_close() could be called at any point and it will have the side-effect of either not doing a reset at all (because it is shared) or resetting the USBD controller at the same time. We've been over this code a great deal over the years. I'd love it to be simpler, but every time we tried to simplify it, things broke. Thierry signature.asc Description: PGP signature
Re: [PATCH] USB: serial: Avoid goto in bulk callbacks
On Mon, Dec 11, 2017 at 12:11:52AM +0100, Ladislav Michl wrote:
> Refactor bulk callback functions to make use of goto pointless.
>
> Signed-off-by: Ladislav Michl
> ---
> drivers/usb/serial/generic.c | 12 +---
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
> index 2274d9625f63..d1a09ba10b6f 100644
> --- a/drivers/usb/serial/generic.c
> +++ b/drivers/usb/serial/generic.c
> @@ -389,6 +389,9 @@ void usb_serial_generic_read_bulk_callback(struct urb
> *urb)
> urb->actual_length);
> switch (status) {
> case 0:
> + usb_serial_debug_data(&port->dev, __func__,
> + urb->actual_length, data);
> + port->serial->type->process_read_urb(urb);
> break;
So as for the ti driver, I prefer keeping the success path out of the
switch statement.
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: serial: ti_usb_3410_5052: Avoid goto in bulk in callback
On Mon, Dec 11, 2017 at 12:09:19AM +0100, Ladislav Michl wrote:
> Make status handling in bulk in callback function more compact,
> which renders goto pointless.
>
> Signed-off-by: Ladislav Michl
> ---
> drivers/usb/serial/ti_usb_3410_5052.c | 36
> ++-
> 1 file changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/usb/serial/ti_usb_3410_5052.c
> b/drivers/usb/serial/ti_usb_3410_5052.c
> index 6b22857f6e52..2786ec7bbca2 100644
> --- a/drivers/usb/serial/ti_usb_3410_5052.c
> +++ b/drivers/usb/serial/ti_usb_3410_5052.c
> @@ -1219,29 +1219,10 @@ static void ti_bulk_in_callback(struct urb *urb)
>
> switch (status) {
> case 0:
> - break;
> - case -ECONNRESET:
> - case -ENOENT:
> - case -ESHUTDOWN:
> - dev_dbg(dev, "%s - urb shutting down, %d\n", __func__, status);
> - return;
> - default:
> - dev_err(dev, "%s - nonzero urb status, %d\n",
> - __func__, status);
> - }
> -
> - if (status == -EPIPE)
> - goto exit;
> -
> - if (status) {
> - dev_err(dev, "%s - stopping read!\n", __func__);
> - return;
> - }
> -
> - if (urb->actual_length) {
> + if (!urb->actual_length)
> + break;
> usb_serial_debug_data(dev, __func__, urb->actual_length,
> urb->transfer_buffer);
> -
> if (!tport->tp_is_open)
> dev_dbg(dev, "%s - port closed, dropping data\n",
> __func__);
> @@ -1250,9 +1231,20 @@ static void ti_bulk_in_callback(struct urb *urb)
> spin_lock(&tport->tp_lock);
> port->icount.rx += urb->actual_length;
> spin_unlock(&tport->tp_lock);
> + break;
> + case -ECONNRESET:
> + case -ENOENT:
> + case -ESHUTDOWN:
> + dev_dbg(dev, "%s - urb shutting down, %d\n", __func__, status);
> + return;
> + case -EPIPE:
> + break;
> + default:
> + dev_err(dev, "%s - nonzero urb status, %d\n",
> + __func__, status);
> + return;
I'm afraid I don't consider this an improvement. I prefer using gotos
for error paths, while keeping the success path out of the status
switch.
Furthermore, this isn't functionally equivalent as we'd not longer log
an error for -EPIPE.
In fact, that -EPIPE is already on my TODO list as we really should not
be resubmitting URBs for a stalled endpoint in the first place. That in
turn should allow for some clean up of this callback.
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 2/2] usb: chipidea: tegra: Select Tegra's PHY in Kconfig
On Mon, Dec 11, 2017 at 02:10:00AM +0300, Dmitry Osipenko wrote: > UDC driver won't probe without Tegra's PHY, hence select it in the > Kconfig. > > Signed-off-by: Dmitry Osipenko > --- > drivers/usb/chipidea/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig > index 785f0ed037f7..2ef3b27ea72b 100644 > --- a/drivers/usb/chipidea/Kconfig > +++ b/drivers/usb/chipidea/Kconfig > @@ -27,6 +27,7 @@ config USB_CHIPIDEA_PCI > config USB_CHIPIDEA_UDC > bool "ChipIdea device controller" > depends on USB_GADGET > + select USB_TEGRA_PHY if ARCH_TEGRA This is kind of pointless given that USB_TEGRA_PHY originally was automatically enabled if ARCH_TEGRA was enabled. What do we gain by these two patches, other than maybe make the driver buildable as a module? Thierry signature.asc Description: PGP signature
Re: [PATCH v1 1/2] usb: phy: Add Kconfig entry for Tegra's PHY driver
On Mon, Dec 11, 2017 at 02:09:59AM +0300, Dmitry Osipenko wrote: > Add Kconfig entry so that other drivers other than ehci-tegra > (like ChipIdea) could add Tegra's PHY to build dependencies. > > Signed-off-by: Dmitry Osipenko > --- > drivers/usb/host/Kconfig | 2 +- > drivers/usb/phy/Kconfig | 8 > drivers/usb/phy/Makefile | 2 +- > 3 files changed, 10 insertions(+), 2 deletions(-) I don't think we actually build-depend on the PHY driver from the ChipIdea driver. In the past, we've refrained from modelling runtime dependencies using Kconfig because in some cases (such as this) it'll include more than necessary (ChipIdea will automatically pull in the USB PHY driver irrespective of whether or not Tegra is enabled). Thierry signature.asc Description: PGP signature
Re: [PATCH v1] usb: phy: tegra: Increase PHY clock stabilization timeout
On Mon, Dec 11, 2017 at 01:55:35AM +0300, Dmitry Osipenko wrote:
> This fixes "utmi_phy_clk_enable: timeout waiting for phy to stabilize"
> error message.
>
> Signed-off-by: Dmitry Osipenko
> ---
> drivers/usb/phy/phy-tegra-usb.c | 13 -
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> index f668bfb708d3..7d5db625f800 100644
> --- a/drivers/usb/phy/phy-tegra-usb.c
> +++ b/drivers/usb/phy/phy-tegra-usb.c
> @@ -16,7 +16,7 @@
> #include
> #include
> #include
> -#include
> +#include
> #include
> #include
> #include
> @@ -305,14 +305,9 @@ static int utmip_pad_power_off(struct tegra_usb_phy *phy)
>
> static int utmi_wait_register(void __iomem *reg, u32 mask, u32 result)
> {
> - unsigned long timeout = 2000;
> - do {
> - if ((readl(reg) & mask) == result)
> - return 0;
> - udelay(1);
> - timeout--;
> - } while (timeout);
> - return -1;
> + u32 tmp;
> +
> + return readl_poll_timeout(reg, tmp, (tmp & mask) == result, 1, 5000);
Technically I think this should be readl_poll_timeout_atomic() because
the above used to use udelay() instead of usleep_range(). But since the
function is never used inside atomic context, this looks fine.
You may want to bump the sleep time between reads to something like 10
or 20. usleep_range() doesn't always work well with very short values,
and given that you already bump the timeout from 2 ms to 5 ms indicates
to me that we're actually spending a lot of time in this loop, and
iterating somewhere between 2000 and 5000 times isn't any good. We only
use this function to wait for the USB_PHY_CLK_VALID bit which happens
during clock enable and disable, which isn't going to be very often, so
even in the best case (where the clock is immediately valid) there's no
need to return within a microsecond.
Thierry
signature.asc
Description: PGP signature
[RESEND PATCH v9] xhci : AMD Promontory USB disable port support
From: Joe Lee
For AMD Promontory xHCI host,although you can disable USB ports in
BIOSsettings,those ports will be enabled anyway after you remove a
device onthat port and re-plug it in again. It's a known limitation of
the chip.As a workaround we can clear the PORT_WAKE_BITS.
Signed-off-by: Joe Lee
---
v9:
- Fix multi-line comment style
v8:
- usb_amd_pt_check_port() function return a bool
v7:
- add a empty function for the case when CONFIG_USB_PCI is not
defined in pci-quirks.h
v6:
- Fix coding format.
v5:
- Add check disable port setting before set PORT_WAKE_BITS
v4:
- Remove the patch code in case USB_PORT_FEAT_REMOTE_WAKE_MASK
v3:
- Fix some checkpatch.pl
---
drivers/usb/host/pci-quirks.c | 128 ++
drivers/usb/host/pci-quirks.h | 5 ++
drivers/usb/host/xhci-hub.c | 7 +++
drivers/usb/host/xhci-pci.c | 11
drivers/usb/host/xhci.h | 2 +-
5 files changed, 152 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 6dda362..bf8354e 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -65,6 +65,23 @@
#defineAX_INDXC0x30
#defineAX_DATAC0x34
+#define PT_ADDR_INDX 0xE8
+#define PT_READ_INDX 0xE4
+#define PT_SIG_1_ADDR 0xA520
+#define PT_SIG_2_ADDR 0xA521
+#define PT_SIG_3_ADDR 0xA522
+#define PT_SIG_4_ADDR 0xA523
+#define PT_SIG_1_DATA 0x78
+#define PT_SIG_2_DATA 0x56
+#define PT_SIG_3_DATA 0x34
+#define PT_SIG_4_DATA 0x12
+#define PT4_P1_REG 0xB521
+#define PT4_P2_REG 0xB522
+#define PT2_P1_REG 0xD520
+#define PT2_P2_REG 0xD521
+#define PT1_P1_REG 0xD522
+#define PT1_P2_REG 0xD523
+
#defineNB_PCIE_INDX_ADDR 0xe0
#defineNB_PCIE_INDX_DATA 0xe4
#definePCIE_P_CNTL 0x10040
@@ -511,6 +528,117 @@ void usb_amd_dev_put(void)
}
EXPORT_SYMBOL_GPL(usb_amd_dev_put);
+bool usb_amd_pt_check_port(struct device *device, int port)
+{
+ unsigned char value;
+ struct pci_dev *pdev;
+
+ pdev = to_pci_dev(device);
+ pci_write_config_word(pdev, PT_ADDR_INDX, PT_SIG_1_ADDR);
+
+ pci_read_config_byte(pdev, PT_READ_INDX, &value);
+ if (value != PT_SIG_1_DATA)
+ return false;
+
+ pci_write_config_word(pdev, PT_ADDR_INDX, PT_SIG_2_ADDR);
+
+ pci_read_config_byte(pdev, PT_READ_INDX, &value);
+ if (value != PT_SIG_2_DATA)
+ return false;
+
+ pci_write_config_word(pdev, PT_ADDR_INDX, PT_SIG_3_ADDR);
+
+ pci_read_config_byte(pdev, PT_READ_INDX, &value);
+ if (value != PT_SIG_3_DATA)
+ return false;
+
+ pci_write_config_word(pdev, PT_ADDR_INDX, PT_SIG_4_ADDR);
+
+ pci_read_config_byte(pdev, PT_READ_INDX, &value);
+ if (value != PT_SIG_4_DATA)
+ return false;
+
+ if ((pdev->device == 0x43b9) || (pdev->device == 0x43ba)) {
+ /* device is AMD_PROMONTORYA_4(0x43b9) or
+* PROMONTORYA_3(0x43ba)
+* disable port setting
+* PT_4_P1_REG[7..1] is USB2.0 port6 to 0
+* PT4_P2_REG[6..0] is USB 13 to port 7
+* 0 : disable ;1 : enable
+*/
+ if (port > 6) {
+ pci_write_config_word(pdev, PT_ADDR_INDX,
+ PT4_P2_REG);
+
+ pci_read_config_byte(pdev, PT_READ_INDX, &value);
+ if (value & (1<<(port - 7)))
+ return false;
+ else
+ return true;
+ } else {
+ pci_write_config_word(pdev, PT_ADDR_INDX,
+ PT4_P1_REG);
+
+ pci_read_config_byte(pdev, PT_READ_INDX, &value);
+ if (value & (1<<(port + 1)))
+ return false;
+ else
+ return true;
+ }
+ } else if (pdev->device == 0x43bb) {
+ /* device is AMD_PROMONTORYA_2(0x43bb)
+* disable port setting
+* PT2_P1_REG[7..5] is USB2.0 port2 to 0
+* PT2_P2_REG[5..0] is USB 9 to port3
+* 0 : disable ;1 : enable
+*/
+ if (port > 2) {
+ pci_write_config_word(pdev, PT_ADDR_INDX, PT2_P2_REG);
+
+ pci_read_config_byte(pdev, PT_READ_INDX, &value);
+ if (value & (1<<(port - 3)))
+ return false;
+ else
+ return true;
+ } else {
+ p
Re: [PATCH v1 1/2] usb: phy: tegra: Cleanup error messages
On Mon, Dec 11, 2017 at 02:07:37AM +0300, Dmitry Osipenko wrote: > Tegra's PHY driver has a mix of pr_err() and dev_err(), let's switch to > dev_err() and use common errors message formatting across the driver for > consistency. > > Signed-off-by: Dmitry Osipenko > --- > drivers/usb/phy/phy-tegra-usb.c | 72 > + > 1 file changed, 44 insertions(+), 28 deletions(-) Can we also get rid of all the function names in error messages? I see that for some error messages you've removed them, but then for others you added them, so you remove inconsistencies on one hand and add other inconsistencies at the same time. =) Other than that, I like this. Thierry signature.asc Description: PGP signature
RE: [PATCH v2] usb: host: Implement workaround for Erratum A-009611
Hi, (please break your lines at 80-characters) Yinbo Zhu writes: >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index >>> 5cb3f6795b0b..071e7cea8cbb 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -1106,6 +1106,9 @@ static void dwc3_get_properties(struct dwc3 >>> *dwc) >>> >>> dwc->quirk_reverse_in_out = device_property_read_bool(dev, >>> "snps,quirk_reverse_in_out"); > >>This was generated on vendor tree. This quirk doesn't exist in dwc3. Also, > >update your tree and review MAINTAINERS file. It has been almost 2 years > since I left TI :-) > >>-- >>Balbi > > Hi Balbi, > > The quirk that I had add it in dwc3. Your meaning is that I can't use > quirk to enable or disable the erratum, isn't it? The tree is > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git, I had > updated it. -*- mode: grep; default-directory: "~/workspace/linux/" -*- Grep started at Mon Dec 11 10:50:47 git --no-pager grep --color -nH -e quirk_reverse_in_out Grep finished with no matches found at Mon Dec 11 10:50:48 -- balbi signature.asc Description: PGP signature
RE: [PATCH v2] usb: host: Implement workaround for Erratum A-009611
-Original Message-
From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
Sent: Friday, December 08, 2017 6:21 PM
To: Yinbo Zhu
Cc: Felipe Balbi ; Mathias Nyman ; open
list:DESIGNWARE USB3 DRD IP DRIVER ; open
list:DESIGNWARE USB3 DRD IP DRIVER ; open list
; Xiaobo Xie ; Jerry Huang
; Ran Wang
Subject: Re: [PATCH v2] usb: host: Implement workaround for Erratum A-009611
On Fri, Dec 08, 2017 at 05:49:41PM +0800, yinbo@nxp.com wrote:
> From: "yinbo.zhu"
>
> Description: This is a occasional problem where the software
>No need for a "Description:" word. That's just assumed here, right?
> issues an End Transfer command while a USB transfer is in progress,
> resulting in the TxFIFO being flushed when the lower layer is waiting
> for data,causing the super speed (SS) transmit to get blocked.
> If the End Transfer command is issued on an IN endpoint to flush out
> the pending transfers when the same IN endpoint is doing transfers on
> the USB, then depending upon the timing of the End Transfer (and the
> resulting internal FIFO flush),the lower layer (U3PTL/U3MAC) could get
> stuck waiting for data indefinitely. This blocks the transmission path
> on the SS, and no DP/ACK/ERDY/DEVNOTIF packets can be sent from the
> device.
> Impact: If this issue happens and the transmission gets blocked, then
> the USB host aborts and resets/re-enumerates the device.
> This unblocks the transmitt engine and the device functions normally.
>
> Workaround: Software must wait for all existing TRBs to complete
> before issuing End transfer command.
>
> Configs Affected:
> LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0, LS2088-48A-R1.1,
> LX2160-2120-2080A-R1.
What are these Configs? That doesn't seem to match up with anything that is in
the kernel tree that I can see.
>
> Signed-off-by: yinbo.zhu
> ---
> drivers/usb/dwc3/core.c | 3 +++
> drivers/usb/dwc3/core.h | 3 +++
> drivers/usb/dwc3/host.c | 3 +++
> drivers/usb/host/xhci-plat.c | 4
> drivers/usb/host/xhci.c | 24 ++--
> drivers/usb/host/xhci.h | 1 +
> 6 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
> 5cb3f6795b0b..071e7cea8cbb 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1106,6 +1106,9 @@ static void dwc3_get_properties(struct dwc3
> *dwc)
>
> dwc->quirk_reverse_in_out = device_property_read_bool(dev,
> "snps,quirk_reverse_in_out");
> + dwc->quirk_stop_transfer_in_block = device_property_read_bool(dev,
> + "snps,quirk_stop_transfer_in_block");
>Have you documented this new DT value somewhere?
I had add some description in drivers/usb/dwc3/core.h.
Is it okay?
" + * @quirk_stop_transfer_in_block: prevent block transmission from being
+ *interrupted."
> +
> dwc->needs_fifo_resize = of_property_read_bool(node,
> "tx-fifo-resize");
>
> dwc->configure_gfladj =
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index
> 6c530cbedf49..b2425799ecb6 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -900,6 +900,8 @@ struct dwc3_scratchpad_array {
> * 3 - Reserved
> * @disable_devinit_u1u2_quirk: disable device-initiated U1/U2 request.
> * @quirk_reverse_in_out: prevent tx fifo reverse the data direction.
> + * @quirk_stop_transfer_in_block: prevent block transmission from being
> + *interrupted.
> * @imod_interval: set the interrupt moderation interval in 250ns
> * increments or 0 to disable.
> */
> @@ -1063,6 +1065,7 @@ struct dwc3 {
> unsignedtx_de_emphasis:2;
> unsigneddisable_devinit_u1u2_quirk:1;
> unsignedquirk_reverse_in_out:1;
> + unsignedquirk_stop_transfer_in_block:1;
>
> u16 imod_interval;
> };
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c index
> 2cd48633d3fa..a9ccbf1b9871 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -110,6 +110,9 @@ int dwc3_host_init(struct dwc3 *dwc)
> if (dwc->quirk_reverse_in_out)
> props[prop_idx++].name = "quirk-reverse-in-out";
>
> + if (dwc->quirk_stop_transfer_in_block)
> + props[prop_idx++].name = "quirk-stop-transfer-in-block";
> +
> if (dwc->usb3_lpm_capable)
> props[prop_idx++].name = "usb3-lpm-capable";
>
> diff --git a/drivers/usb/host/xhci-plat.c
> b/drivers/usb/host/xhci-plat.c index d1c1e882e6d7..5721d4ece625 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -272,6 +272,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
> if (device_property_read_bool(&pdev->dev, "quirk-reverse-in-out"))
> xhci->quirks |= XH
[PATCH] xhci: fixup incorrect memset size parameter when clearing up DbC on exit.
Incorrect size was given to memset when zeroing the DbC endpoint
structures on exit. Use element size * ARRAY_SIZE to fix it
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-dbgcap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
index 671e502..452df0f 100644
--- a/drivers/usb/host/xhci-dbgcap.c
+++ b/drivers/usb/host/xhci-dbgcap.c
@@ -366,7 +366,7 @@ static void xhci_dbc_eps_exit(struct xhci_hcd *xhci)
{
struct xhci_dbc *dbc = xhci->dbc;
- memset(dbc->eps, 0, ARRAY_SIZE(dbc->eps));
+ memset(dbc->eps, 0, sizeof(struct dbc_ep) * ARRAY_SIZE(dbc->eps));
}
static int xhci_dbc_mem_init(struct xhci_hcd *xhci, gfp_t flags)
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] usb: host: Implement workaround for Erratum A-009611
-Original Message- From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] Sent: Monday, December 11, 2017 3:35 PM To: Yinbo Zhu Cc: Felipe Balbi ; Mathias Nyman ; open list:DESIGNWARE USB3 DRD IP DRIVER ; open list:DESIGNWARE USB3 DRD IP DRIVER ; open list ; Xiaobo Xie ; Jerry Huang ; Ran Wang Subject: Re: [PATCH v2] usb: host: Implement workaround for Erratum A-009611 On Mon, Dec 11, 2017 at 03:15:37AM +, Yinbo Zhu wrote: > > > -Original Message- > From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com] > Sent: Friday, December 08, 2017 6:44 PM > To: Greg Kroah-Hartman ; Yinbo Zhu > > Cc: Mathias Nyman ; open list:DESIGNWARE USB3 > DRD IP DRIVER ; open list:DESIGNWARE USB3 > DRD IP DRIVER ; open list > ; Xiaobo Xie ; Jerry > Huang ; Ran Wang > Subject: Re: [PATCH v2] usb: host: Implement workaround for Erratum > A-009611 > > > >Hi, > > >Greg Kroah-Hartman writes: > > On Fri, Dec 08, 2017 at 05:49:41PM +0800, yinbo@nxp.com wrote: > >> From: "yinbo.zhu" > >> > >> Description: This is a occasional problem where the software > > > > No need for a "Description:" word. That's just assumed here, right? > > I will remove "Description:" thanks. > >> issues an End Transfer command while a USB transfer is in progress, > >> resulting in the TxFIFO being flushed when the lower layer is > >> waiting for data,causing the super speed (SS) transmit to get blocked. > >> If the End Transfer command is issued on an IN endpoint to flush > >> out the pending transfers when the same IN endpoint is doing > >> transfers on the USB, then depending upon the timing of the End > >> Transfer (and the resulting internal FIFO flush),the lower layer > >> (U3PTL/U3MAC) could get stuck waiting for data indefinitely. This > >> blocks the transmission path on the SS, and no DP/ACK/ERDY/DEVNOTIF > >> packets can be sent from the device. > >> Impact: If this issue happens and the transmission gets blocked, > >> then the USB host aborts and resets/re-enumerates the device. > >> This unblocks the transmitt engine and the device functions normally. > >> > >> Workaround: Software must wait for all existing TRBs to complete > >> before issuing End transfer command. > >> > >> Configs Affected: > >> LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0, LS2088-48A-R1.1, > >> LX2160-2120-2080A-R1. > > > > What are these Configs? That doesn't seem to match up with anything > > that is in the kernel tree that I can see. > > These configs is soc information, I don't enable it on these platform dts. > Although the erratum issue can't be reproduced. >I do not understand what this means, please explain it a bit better. >thanks, >greg k-h Maybe I have a problem with your words, Your meaning is that you want to ask me why I didn't add an attribute in the device tree to match kernel for every platform, right? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Webcams not recognized on a Dell Latitude 5285 laptop
Dear all, I recently installed linux 4.14 on a Dell Latitude 5285 detachable laptop. The problem I have is that the webcams (both the rear one and the front one) are not recognized. There is no /dev/video* file. I am not sure what is the model of the webcam. The Dell support website for this laptop mentions a Realtek IR webcam: http://www.dell.com/support/home/fr/fr/frbsdt1/product-support/product/latitude-12-5285-2-in-1-laptop/drivers Below are my lspci and lsusb outputs, under kernel 4.14. It is not clear to me which lines refer to the webcams. Thank you for your help! Frédéric :~$ lspci 00:00.0 Host bridge: Intel Corporation Xeon E3-1200 v6/7th Gen Core Processor Host Bridge/DRAM Registers (rev 02) 00:02.0 VGA compatible controller: Intel Corporation HD Graphics 620 (rev 02) 00:04.0 Signal processing controller: Intel Corporation Skylake Processor Thermal Subsystem (rev 02) 00:05.0 Multimedia controller: Intel Corporation Skylake Imaging Unit (rev 01) 00:13.0 Non-VGA unclassified device: Intel Corporation Device 9d35 (rev 21) 00:14.0 USB controller: Intel Corporation Sunrise Point-LP USB 3.0 xHCI Controller (rev 21) 00:14.2 Signal processing controller: Intel Corporation Sunrise Point-LP Thermal subsystem (rev 21) 00:14.3 Multimedia controller: Intel Corporation Device 9d32 (rev 01) 00:15.0 Signal processing controller: Intel Corporation Sunrise Point-LP Serial IO I2C Controller #0 (rev 21) 00:15.1 Signal processing controller: Intel Corporation Sunrise Point-LP Serial IO I2C Controller #1 (rev 21) 00:15.2 Signal processing controller: Intel Corporation Sunrise Point-LP Serial IO I2C Controller #2 (rev 21) 00:16.0 Communication controller: Intel Corporation Sunrise Point-LP CSME HECI #1 (rev 21) 00:1c.0 PCI bridge: Intel Corporation Sunrise Point-LP PCI Express Root Port #5 (rev f1) 00:1c.7 PCI bridge: Intel Corporation Sunrise Point-LP PCI Express Root Port #8 (rev f1) 00:1d.0 PCI bridge: Intel Corporation Sunrise Point-LP PCI Express Root Port #9 (rev f1) 00:1f.0 ISA bridge: Intel Corporation Device 9d4e (rev 21) 00:1f.2 Memory controller: Intel Corporation Sunrise Point-LP PMC (rev 21) 00:1f.3 Audio device: Intel Corporation Device 9d71 (rev 21) 00:1f.4 SMBus: Intel Corporation Sunrise Point-LP SMBus (rev 21) 01:00.0 Non-Volatile memory controller: Toshiba America Info Systems Device 0116 02:00.0 Network controller: Intel Corporation Wireless 8265 / 8275 (rev 78) 03:00.0 Unassigned class [ff00]: Realtek Semiconductor Co., Ltd. RTS525A PCI Express Card Reader (rev 01) :~$ lsusb Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub Bus 001 Device 003: ID 8087:0a2b Intel Corp. Bus 001 Device 004: ID 044e:1218 Alps Electric Co., Ltd Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] usb: host: Implement workaround for Erratum A-007463
-Original Message- From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] Sent: Monday, December 11, 2017 3:34 PM To: Yinbo Zhu Cc: Felipe Balbi ; Mathias Nyman ; open list:DESIGNWARE USB3 DRD IP DRIVER ; open list:DESIGNWARE USB3 DRD IP DRIVER ; open list ; Xiaobo Xie ; Jerry Huang ; Ran Wang Subject: Re: [PATCH v2] usb: host: Implement workaround for Erratum A-007463 On Mon, Dec 11, 2017 at 02:26:02AM +, Yinbo Zhu wrote: > > > -Original Message- > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > Sent: Friday, December 08, 2017 6:18 PM > To: Yinbo Zhu > Cc: Felipe Balbi ; Mathias Nyman > ; open list:DESIGNWARE USB3 DRD IP DRIVER > ; open list:DESIGNWARE USB3 DRD IP DRIVER > ; open list > ; Xiaobo Xie ; Jerry > Huang ; Ran Wang > Subject: Re: [PATCH v2] usb: host: Implement workaround for Erratum > A-007463 > > On Fri, Dec 08, 2017 at 05:49:40PM +0800, yinbo@nxp.com wrote: > > From: "yinbo.zhu" > > >I need a "real name" here, I doubt you sign documents as: > > "yinbo.zhu" > >right? :) > > >Also, you sent 3 patches, yet no way to know what order to apply them in. > >Please fix that up by sending a >patch series, properly numbered. > > >thanks, > > >greg k-h > > Hi Greg Kroah-Hartman, > > "Yinbo.zhu" is my email address prefix,and it is automatically generated > through the git command. >Then this means you have not correctly configured git, please fix that :) I will remove the point in "Yinbo.zhu" Thanks. > You can follow the order of patch A-007463 A-009611 A-009668 to apply it. >That's not how we number patches in the kernel, please do so in the normal >way. > See the mailing lists for lots of examples. The kernel documentation also > describes the correct format for this. Okay, I will mark the sign for patch order. About the kernel documentation I will remove the punctuation. thanks >thanks, -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: host: Setting TOUTCAL and USBTRDTIM fields in host mode
Hi Stefan,
On 12/7/2017 9:04 PM, Stefan Wahren wrote:
> Hi Minas,
>
> Am 07.12.2017 um 17:56 schrieb Stefan Wahren:
>> Added missing GUSBCFG programming in host mode.
>>
>> These fields even if was programmed in device mode (in function
>> dwc2_hsotg_core_init_disconnected()) will be resetting to POR values
>> after core soft reset applied.
>> So, each time when switching to host mode required to set these fields
>> to correct values. It's allow fix issues with lot of transaction errors
>> due to timeouts and turnarrounds on USB bus.
>>
>> Signed-off-by: Minas Harutyunyan
>
> please add a fixes tag.
>
>> ---
>> drivers/usb/dwc2/hcd.c | 9 -
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index 614bb9603def..05e4e8c07bf1 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -2317,10 +2317,17 @@ static int dwc2_core_init(struct dwc2_hsotg *hsotg,
>> bool initial_setup)
>>*/
>> static void dwc2_core_host_init(struct dwc2_hsotg *hsotg)
>> {
>> -u32 hcfg, hfir, otgctl;
>> +u32 hcfg, hfir, otgctl, usbcfg, val;
>>
>> dev_dbg(hsotg->dev, "%s(%p)\n", __func__, hsotg);
>>
>> +/* Set HS/FS Timeout Calibration and USBTrdTim */
>> +usbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
>> +usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_USBTRDTIM_MASK);
>> +val = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
>> +usbcfg |= (GUSBCFG_TOUTCAL(7) | (val << GUSBCFG_USBTRDTIM_SHIFT));
>
> These are too much magic numbers (9,5,7). Could you please add a comment
> or even better defines for them?
>
> Thanks
>
>> +dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);
>> +
>> /* Restart the Phy Clock */
>> dwc2_writel(0, hsotg->regs + PCGCTL);
>>
>
New version of this patch submitted with this name "[PATCH] usb: dwc2:
host: Fix transaction errors in host mode". So, this one could be ignored.
Thanks,
Minas
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc2: host: Fix transaction errors in host mode
Added missing GUSBCFG programming in host mode, which fixes
transaction errors issue on HiKey and Altera Cyclone V boards.
These field even if was programmed in device mode (in function
dwc2_hsotg_core_init_disconnected()) will be resetting to POR values
after core soft reset applied.
So, each time when switching to host mode required to set this field
to correct value.
Signed-off-by: Minas Harutyunyan
---
drivers/usb/dwc2/hcd.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 614bb9603def..aa8390122408 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -2317,10 +2317,22 @@ static int dwc2_core_init(struct dwc2_hsotg *hsotg,
bool initial_setup)
*/
static void dwc2_core_host_init(struct dwc2_hsotg *hsotg)
{
- u32 hcfg, hfir, otgctl;
+ u32 hcfg, hfir, otgctl, usbcfg, val;
dev_dbg(hsotg->dev, "%s(%p)\n", __func__, hsotg);
+ /* Set HS/FS Timeout Calibration to 7 (max available value).
+* The number of PHY clocks that the application programs in
+* this field is added to the high/full speed interpacket timeout
+* duration in the core to account for any additional delays
+* introduced by the PHY. This can be required, because the delay
+* introduced by the PHY in generating the linestate condition
+* can vary from one PHY to another.
+*/
+ usbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
+ usbcfg |= GUSBCFG_TOUTCAL(7);
+ dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);
+
/* Restart the Phy Clock */
dwc2_writel(0, hsotg->regs + PCGCTL);
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
