Re: [PATCH] usb: dwc3: core: Add quirk for enabling AutoRetry feature in the controller
On Sat, Jul 21, 2018 at 03:58:40PM +0530, Anurag Kumar Vulisha wrote: > By default when core sees any transaction error(CRC or overflow) > it replies with terminating retry ACK (Retry=1 and Nump == 0). > Enabling this Auto Retry feature in controller, on seeing any > transaction errors makes the core to send an non-terminating ACK > transaction packet (that is, ACK TP with Retry=1 and Nump != 0). > Doing so will give controller a chance to recover from the error > condition. > > Signed-off-by: Anurag Kumar Vulisha > --- > Documentation/devicetree/bindings/usb/dwc3.txt | 5 + > drivers/usb/dwc3/core.c| 16 > drivers/usb/dwc3/core.h| 6 ++ > 3 files changed, 27 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt > b/Documentation/devicetree/bindings/usb/dwc3.txt > index 7f13ebe..2ba2bc2 100644 > --- a/Documentation/devicetree/bindings/usb/dwc3.txt > +++ b/Documentation/devicetree/bindings/usb/dwc3.txt > @@ -94,6 +94,11 @@ Optional properties: > this and tx-thr-num-pkt-prd to a valid, non-zero value > 1-16 (DWC_usb31 programming guide section 1.2.3) to > enable periodic ESS TX threshold. > + - snps,enable_auto_retry: Set to enable Auto retry Feature to make the s/_/-/ > + controller operating in Host mode on seeing transaction > + errors(CRC errors or internal overrun scenerios) on IN > + transfers to reply to the device with a non-terminating > + retry ACK (i.e, an ACK TP with Retry=1 & Nump != 0) Seems like the property is unnecessary. When would you not want this retry behavior? Why not just enable unconditionally in the driver? Rob -- 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/5] memstick: rtsx_usb_ms: Support runtime power management
On Wed, 25 Jul 2018, Kai-Heng Feng wrote: > In order to let host's parent device, rtsx_usb, to use USB remote wake > up signaling to do card detection, it needs to be suspended. Hence it's > necessary to add runtime PM support for the memstick host. > > To keep memstick host stays suspended when it's not in use, convert the > card detection function from kthread to delayed_work, which can be > scheduled when the host is resumed and can be canceled when the host is > suspended. > > Use an idle function check if there's no card and the power mode is > MEMSTICK_POWER_OFF. If both criteria are met, put the device to suspend. > > Signed-off-by: Kai-Heng Feng I'm not familiar enough with this driver to comment on specific instances, but you should carefully go through the code and make sure that you use pm_runtime_get_noresume() and pm_runtime_put_noidle() correctly. The basic idea is to use these when you know beforehand that the device is already at full power (for _get_noresume) and the usage count will not go to 0 (for _put_noidle). If you aren't certain of these requirements then you should call pm_runtime_get_sync() and pm_runtime_put(). 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 2/5] memstick: Prevent memstick host from getting runtime suspended during card detection
On Wed, 25 Jul 2018, Kai-Heng Feng wrote:
> We can use MEMSTICK_POWER_{ON,OFF} along with pm_runtime_{get,put}
> helpers to let memstick host support runtime pm.
>
> There's a small window between memstick_detect_change() and its queued
> work, memstick_check(). In this window the rpm count may go down to zero
> before the memstick host powers on, so the host can be inadvertently
> suspended.
>
> Increment rpm count before calling memstick_check(), and decrement rpm
> count afterward, as now we are sure the memstick host should be
> suspended or not.
>
> Signed-off-by: Kai-Heng Feng
> ---
> drivers/memstick/core/memstick.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/memstick/core/memstick.c
> b/drivers/memstick/core/memstick.c
> index 76382c858c35..944fe0c93fa7 100644
> --- a/drivers/memstick/core/memstick.c
> +++ b/drivers/memstick/core/memstick.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
>
> #define DRIVER_NAME "memstick"
>
> @@ -209,6 +210,7 @@ static int memstick_dummy_check(struct memstick_dev *card)
> */
> void memstick_detect_change(struct memstick_host *host)
> {
> + pm_runtime_get_noresume(host->dev.parent);
> queue_work(workqueue, >media_checker);
> }
> EXPORT_SYMBOL(memstick_detect_change);
> @@ -479,6 +481,8 @@ static void memstick_check(struct work_struct *work)
> host->set_param(host, MEMSTICK_POWER, MEMSTICK_POWER_OFF);
>
> mutex_unlock(>lock);
> +
> + pm_runtime_put_noidle(host->dev.parent);
> dev_dbg(>dev, "memstick_check finished\n");
> }
This should be pm_runtime_put(), not _put_noidle(). The reason is
because if this call causes the PM runtime usage count to drop to 0,
you do want the device to go into runtime suspend.
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 2/6] module: add support for symbol namespaces.
On Wed, Jul 25, 2018 at 8:55 AM Jessica Yu wrote:
>
> +++ Martijn Coenen [24/07/18 09:56 +0200]:
> >I did find an issue with my approach:
> >
> >On Mon, Jul 16, 2018 at 2:21 PM, Martijn Coenen wrote:
> >> The ELF symbols are renamed to include the namespace with an asm label;
> >> for example, symbol 'usb_stor_suspend' in namespace USB_STORAGE becomes
> >> 'usb_stor_suspend.USB_STORAGE'. This allows modpost to do namespace
> >> checking, without having to go through all the effort of parsing ELF and
> >> reloction records just to get to the struct kernel_symbols.
> >
> >depmod doesn't like this: if namespaced symbols are built-in to the
> >kernel, they will appear as 'symbol.NS' in the symbol table, whereas
> >modules using those symbols just depend on 'symbol'. This will cause
> >depmod to warn about unknown symbols. I didn't find this previously
> >because all the exports/imports I tested were done from modules
> >themselves.
> >
> >One way to deal with it is to change depmod, but it looks like it
> >hasn't been changed in ages, and it would also introduce a dependency
this might be because you are looking to the wrong project
(module-init-tools) rather than kmod that replaced it some years ago?
> >on userspaces to update it to avoid these warnings. So, we'd have to
> >encode the symbol namespace information in another way for modpost to
> >use. I could just write a separate post-processing tool (much like
> >genksyms), or perhaps encode the information in a discardable section.
> >Any other suggestions welcome.
>
> This seems to be because depmod uses System.map as a source for kernel
> symbols and scans for only the ones prefixed with "__ksymtab" to find
> the exported ones - and those happen to use the '.' to mark the
> namespace it belongs to. It strips that prefix and uses the remainder
> as the actual symbol name. To fix that it'd have to strip the
> namespace suffix in the symbol name as well.
>
> Just scanning the depmod source code, it seems really trivial to just
> have depmod accommodate the new symbol name format. Adding Lucas (kmod
> maintainer) to CC.
Yep, that would be easy and allow depmod to be backward compatible
since we would do nothing if the symbol doesn't have the suffix.
As for dependency on a new version, this seems trivial enough to be
backported to previous releases used on distros so even if they are
not rolling they would get a compatible depmod.
CC'ing linux-modu...@vger.kernel.org to keep track of this there
thanks
Lucas De Marchi
>
> Thanks,
>
> Jessica
>
> >
> >>
> >> On x86_64 I saw no difference in binary size (compression), but at
> >> runtime this will require a word of memory per export to hold the
> >> namespace. An alternative could be to store namespaced symbols in their
> >> own section and use a separate 'struct namespaced_kernel_symbol' for
> >> that section, at the cost of making the module loader more complex.
> >>
> >> Signed-off-by: Martijn Coenen
> >> ---
> >> include/asm-generic/export.h | 2 +-
> >> include/linux/export.h | 83 +++-
> >> include/linux/module.h | 13 ++
> >> kernel/module.c | 79 ++
> >> 4 files changed, 156 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
> >> index 68efb950a918..4c3d1afb702f 100644
> >> --- a/include/asm-generic/export.h
> >> +++ b/include/asm-generic/export.h
> >> @@ -29,7 +29,7 @@
> >> .section ___ksymtab\sec+\name,"a"
> >> .balign KSYM_ALIGN
> >> __ksymtab_\name:
> >> - __put \val, __kstrtab_\name
> >> + __put \val, __kstrtab_\name, 0
> >> .previous
> >> .section __ksymtab_strings,"a"
> >> __kstrtab_\name:
> >> diff --git a/include/linux/export.h b/include/linux/export.h
> >> index ad6b8e697b27..9f6e70eeb85f 100644
> >> --- a/include/linux/export.h
> >> +++ b/include/linux/export.h
> >> @@ -22,6 +22,11 @@ struct kernel_symbol
> >> {
> >> unsigned long value;
> >> const char *name;
> >> + const char *namespace;
> >> +};
> >> +
> >> +struct namespace_import {
> >> + const char *namespace;
> >> };
> >>
> >> #ifdef MODULE
> >> @@ -54,18 +59,28 @@ extern struct module __this_module;
> >> #define __CRC_SYMBOL(sym, sec)
> >> #endif
> >>
> >> +#define NS_SEPARATOR "."
> >> +
> >> +#define MODULE_IMPORT_NS(ns) \
> >> + static const struct namespace_import __knsimport_##ns \
> >> + asm("__knsimport_" #ns) \
> >> + __attribute__((section("__knsimport"), used)) \
> >> + = { #ns }
> >> +
> >> /* For every exported symbol, place a struct in the __ksymtab section */
> >> -#define ___EXPORT_SYMBOL(sym, sec) \
> >> +#define ___EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)
> >> \
> >>
Re: [PATCH 2/6] module: add support for symbol namespaces.
+++ Martijn Coenen [24/07/18 09:56 +0200]:
I did find an issue with my approach:
On Mon, Jul 16, 2018 at 2:21 PM, Martijn Coenen wrote:
The ELF symbols are renamed to include the namespace with an asm label;
for example, symbol 'usb_stor_suspend' in namespace USB_STORAGE becomes
'usb_stor_suspend.USB_STORAGE'. This allows modpost to do namespace
checking, without having to go through all the effort of parsing ELF and
reloction records just to get to the struct kernel_symbols.
depmod doesn't like this: if namespaced symbols are built-in to the
kernel, they will appear as 'symbol.NS' in the symbol table, whereas
modules using those symbols just depend on 'symbol'. This will cause
depmod to warn about unknown symbols. I didn't find this previously
because all the exports/imports I tested were done from modules
themselves.
One way to deal with it is to change depmod, but it looks like it
hasn't been changed in ages, and it would also introduce a dependency
on userspaces to update it to avoid these warnings. So, we'd have to
encode the symbol namespace information in another way for modpost to
use. I could just write a separate post-processing tool (much like
genksyms), or perhaps encode the information in a discardable section.
Any other suggestions welcome.
This seems to be because depmod uses System.map as a source for kernel
symbols and scans for only the ones prefixed with "__ksymtab" to find
the exported ones - and those happen to use the '.' to mark the
namespace it belongs to. It strips that prefix and uses the remainder
as the actual symbol name. To fix that it'd have to strip the
namespace suffix in the symbol name as well.
Just scanning the depmod source code, it seems really trivial to just
have depmod accommodate the new symbol name format. Adding Lucas (kmod
maintainer) to CC.
Thanks,
Jessica
On x86_64 I saw no difference in binary size (compression), but at
runtime this will require a word of memory per export to hold the
namespace. An alternative could be to store namespaced symbols in their
own section and use a separate 'struct namespaced_kernel_symbol' for
that section, at the cost of making the module loader more complex.
Signed-off-by: Martijn Coenen
---
include/asm-generic/export.h | 2 +-
include/linux/export.h | 83 +++-
include/linux/module.h | 13 ++
kernel/module.c | 79 ++
4 files changed, 156 insertions(+), 21 deletions(-)
diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
index 68efb950a918..4c3d1afb702f 100644
--- a/include/asm-generic/export.h
+++ b/include/asm-generic/export.h
@@ -29,7 +29,7 @@
.section ___ksymtab\sec+\name,"a"
.balign KSYM_ALIGN
__ksymtab_\name:
- __put \val, __kstrtab_\name
+ __put \val, __kstrtab_\name, 0
.previous
.section __ksymtab_strings,"a"
__kstrtab_\name:
diff --git a/include/linux/export.h b/include/linux/export.h
index ad6b8e697b27..9f6e70eeb85f 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -22,6 +22,11 @@ struct kernel_symbol
{
unsigned long value;
const char *name;
+ const char *namespace;
+};
+
+struct namespace_import {
+ const char *namespace;
};
#ifdef MODULE
@@ -54,18 +59,28 @@ extern struct module __this_module;
#define __CRC_SYMBOL(sym, sec)
#endif
+#define NS_SEPARATOR "."
+
+#define MODULE_IMPORT_NS(ns) \
+ static const struct namespace_import __knsimport_##ns \
+ asm("__knsimport_" #ns) \
+ __attribute__((section("__knsimport"), used)) \
+ = { #ns }
+
/* For every exported symbol, place a struct in the __ksymtab section */
-#define ___EXPORT_SYMBOL(sym, sec) \
+#define ___EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)
\
extern typeof(sym) sym; \
__CRC_SYMBOL(sym, sec) \
- static const char __kstrtab_##sym[] \
+ static const char __kstrtab_##sym##nspost[] \
__attribute__((section("__ksymtab_strings"), aligned(1))) \
= #sym; \
- static const struct kernel_symbol __ksymtab_##sym \
+ static const struct kernel_symbol __ksymtab_##sym##nspost \
+ asm("__ksymtab_" #sym nspost2) \
__used \
- __attribute__((section("___ksymtab" sec "+" #sym), used)) \
+ __attribute__((section("___ksymtab" sec "+" #sym #nspost))) \
+ __attribute__((used)) \
__attribute__((aligned(sizeof(void *
RE: [PATCH 4/8] usb: dwc3: implement stream transfer timeout
>-Original Message- >From: Andy Shevchenko [mailto:andy.shevche...@gmail.com] >Sent: Wednesday, July 25, 2018 8:55 PM >To: Anurag Kumar Vulisha >Cc: Felipe Balbi ; Greg Kroah-Hartman >; v.anuragku...@gmail.com; USB u...@vger.kernel.org>; Linux Kernel Mailing List >Subject: Re: [PATCH 4/8] usb: dwc3: implement stream transfer timeout > >On Wed, Jul 25, 2018 at 6:14 PM, Anurag Kumar Vulisha > wrote: > +/* + * Timeout value in msecs used by stream_timeout_timer when streams +are enabled */ +#define STREAM_TIMEOUT 50 >>> >>>Perhaps, STREAM_TIMEOUT_MS 50 >>> >>>Dunno about this driver, but it's a usual practice to help reader with >>>understanding >>>code on the first glance. >>> >> >> Since I have mentioned "msecs" in comment above describing the >STREAM_TIMEOUT, >> thought it would suffice. But if you feel I should change it, I will fix it >> in v2 > >But you didn't put that comment before each occurrence of this >constant in the code, right? >That's what I'm talking about. If reader looks into the code, there is >no need to understand the order of the value for timeout, since units >are part of the name. > >Of course, you may ignore this comment. > I agree with your comments , will wait for other comments on this patch series and address them altogether in v2 Thanks, Anurag Kumar Vulisha >-- >With Best Regards, >Andy Shevchenko
[PATCH 1/5] misc: rtsx_usb: Use USB remote wakeup signaling for card insertion detection
Although rtsx_usb doesn't support card removal detection, card insertion
will resume rtsx_usb by USB remote wakeup signaling.
When rtsx_usb gets resumed, also resumes its child devices,
rtsx_usb_sdmmc and rtsx_usb_ms, to notify them there's a card in its
slot.
Signed-off-by: Kai-Heng Feng
---
drivers/misc/cardreader/rtsx_usb.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/misc/cardreader/rtsx_usb.c
b/drivers/misc/cardreader/rtsx_usb.c
index b97903ff1a72..f7a66f614085 100644
--- a/drivers/misc/cardreader/rtsx_usb.c
+++ b/drivers/misc/cardreader/rtsx_usb.c
@@ -723,8 +723,15 @@ static int rtsx_usb_suspend(struct usb_interface *intf,
pm_message_t message)
return 0;
}
+static int rtsx_usb_resume_child(struct device *dev, void *data)
+{
+ pm_request_resume(dev);
+ return 0;
+}
+
static int rtsx_usb_resume(struct usb_interface *intf)
{
+ device_for_each_child(>dev, NULL, rtsx_usb_resume_child);
return 0;
}
@@ -734,6 +741,7 @@ static int rtsx_usb_reset_resume(struct usb_interface *intf)
(struct rtsx_ucr *)usb_get_intfdata(intf);
rtsx_usb_reset_chip(ucr);
+ device_for_each_child(>dev, NULL, rtsx_usb_resume_child);
return 0;
}
--
2.17.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 5/5] misc: rtsx_usb: / memstick: rtsx_usb_ms: Avoid long delay before system suspend
There's a long power-on delay at the end of rtsx_usb_ms_set_param(). This delay is noticeable right before system suspend. To prevent already suspended memstick host from getting powered on by PM core, use DPM_FLAG_SMART_SUSPEND to avoid the situation. Signed-off-by: Kai-Heng Feng --- drivers/memstick/host/rtsx_usb_ms.c | 4 drivers/misc/cardreader/rtsx_usb.c | 1 + 2 files changed, 5 insertions(+) diff --git a/drivers/memstick/host/rtsx_usb_ms.c b/drivers/memstick/host/rtsx_usb_ms.c index 126eb310980a..e3b635d1220f 100644 --- a/drivers/memstick/host/rtsx_usb_ms.c +++ b/drivers/memstick/host/rtsx_usb_ms.c @@ -763,6 +763,10 @@ static int rtsx_usb_ms_drv_probe(struct platform_device *pdev) msh->set_param = rtsx_usb_ms_set_param; msh->caps = MEMSTICK_CAP_PAR4; + /* DPM_FLAG_LEAVE_SUSPENDED is not needed, the parent device will wake +* up memstick host. +*/ + dev_pm_set_driver_flags(ms_dev(host), DPM_FLAG_SMART_SUSPEND); pm_runtime_set_active(ms_dev(host)); pm_runtime_enable(ms_dev(host)); diff --git a/drivers/misc/cardreader/rtsx_usb.c b/drivers/misc/cardreader/rtsx_usb.c index f7a66f614085..98bb878a6ade 100644 --- a/drivers/misc/cardreader/rtsx_usb.c +++ b/drivers/misc/cardreader/rtsx_usb.c @@ -671,6 +671,7 @@ static int rtsx_usb_probe(struct usb_interface *intf, goto out_init_fail; #ifdef CONFIG_PM + dev_pm_set_driver_flags(>dev, DPM_FLAG_SMART_SUSPEND | DPM_FLAG_LEAVE_SUSPENDED); intf->needs_remote_wakeup = 1; usb_enable_autosuspend(usb_dev); #endif -- 2.17.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 4/5] memstick: rtsx_usb_ms: Support runtime power management
In order to let host's parent device, rtsx_usb, to use USB remote wake
up signaling to do card detection, it needs to be suspended. Hence it's
necessary to add runtime PM support for the memstick host.
To keep memstick host stays suspended when it's not in use, convert the
card detection function from kthread to delayed_work, which can be
scheduled when the host is resumed and can be canceled when the host is
suspended.
Use an idle function check if there's no card and the power mode is
MEMSTICK_POWER_OFF. If both criteria are met, put the device to suspend.
Signed-off-by: Kai-Heng Feng
---
drivers/memstick/host/rtsx_usb_ms.c | 138 ++--
1 file changed, 71 insertions(+), 67 deletions(-)
diff --git a/drivers/memstick/host/rtsx_usb_ms.c
b/drivers/memstick/host/rtsx_usb_ms.c
index cd12f3d1c088..126eb310980a 100644
--- a/drivers/memstick/host/rtsx_usb_ms.c
+++ b/drivers/memstick/host/rtsx_usb_ms.c
@@ -40,15 +40,14 @@ struct rtsx_usb_ms {
struct mutexhost_mutex;
struct work_struct handle_req;
-
- struct task_struct *detect_ms;
- struct completion detect_ms_exit;
+ struct delayed_work poll_card;
u8 ssc_depth;
unsigned intclock;
int power_mode;
unsigned char ifmode;
booleject;
+ boolsuspend;
};
static inline struct device *ms_dev(struct rtsx_usb_ms *host)
@@ -524,7 +523,7 @@ static void rtsx_usb_ms_handle_req(struct work_struct *work)
int rc;
if (!host->req) {
- pm_runtime_get_sync(ms_dev(host));
+ pm_runtime_get_noresume(ms_dev(host));
do {
rc = memstick_next_req(msh, >req);
dev_dbg(ms_dev(host), "next req %d\n", rc);
@@ -545,7 +544,7 @@ static void rtsx_usb_ms_handle_req(struct work_struct *work)
host->req->error);
}
} while (!rc);
- pm_runtime_put(ms_dev(host));
+ pm_runtime_put_noidle(ms_dev(host));
}
}
@@ -572,7 +571,7 @@ static int rtsx_usb_ms_set_param(struct memstick_host *msh,
dev_dbg(ms_dev(host), "%s: param = %d, value = %d\n",
__func__, param, value);
- pm_runtime_get_sync(ms_dev(host));
+ pm_runtime_get_noresume(ms_dev(host));
mutex_lock(>dev_mutex);
err = rtsx_usb_card_exclusive_check(ucr, RTSX_USB_MS_CARD);
@@ -585,14 +584,14 @@ static int rtsx_usb_ms_set_param(struct memstick_host
*msh,
break;
if (value == MEMSTICK_POWER_ON) {
- pm_runtime_get_sync(ms_dev(host));
+ pm_runtime_get_noresume(ms_dev(host));
err = ms_power_on(host);
+ if (err)
+ pm_runtime_put_noidle(ms_dev(host));
} else if (value == MEMSTICK_POWER_OFF) {
err = ms_power_off(host);
- if (host->msh->card)
+ if (!err)
pm_runtime_put_noidle(ms_dev(host));
- else
- pm_runtime_put(ms_dev(host));
} else
err = -EINVAL;
if (!err)
@@ -638,7 +637,7 @@ static int rtsx_usb_ms_set_param(struct memstick_host *msh,
}
out:
mutex_unlock(>dev_mutex);
- pm_runtime_put(ms_dev(host));
+ pm_runtime_put_noidle(ms_dev(host));
/* power-on delay */
if (param == MEMSTICK_POWER && value == MEMSTICK_POWER_ON)
@@ -648,75 +647,86 @@ static int rtsx_usb_ms_set_param(struct memstick_host
*msh,
return err;
}
-#ifdef CONFIG_PM_SLEEP
-static int rtsx_usb_ms_suspend(struct device *dev)
+#ifdef CONFIG_PM
+static int rtsx_usb_ms_runtime_suspend(struct device *dev)
{
struct rtsx_usb_ms *host = dev_get_drvdata(dev);
struct memstick_host *msh = host->msh;
- dev_dbg(ms_dev(host), "--> %s\n", __func__);
-
+ host->suspend = true;
memstick_suspend_host(msh);
+
return 0;
}
-static int rtsx_usb_ms_resume(struct device *dev)
+static int rtsx_usb_ms_runtime_resume(struct device *dev)
{
struct rtsx_usb_ms *host = dev_get_drvdata(dev);
struct memstick_host *msh = host->msh;
- dev_dbg(ms_dev(host), "--> %s\n", __func__);
-
memstick_resume_host(msh);
+ host->suspend = false;
+ schedule_delayed_work(>poll_card, 100);
+
return 0;
}
-#endif /* CONFIG_PM_SLEEP */
-/*
- * Thread function of ms card slot detection. The thread starts right after
- * successful host addition. It stops while the driver removal function sets
- * host->eject true.
- */
-static int rtsx_usb_detect_ms_card(void *__host)
+static int
[PATCH 2/5] memstick: Prevent memstick host from getting runtime suspended during card detection
We can use MEMSTICK_POWER_{ON,OFF} along with pm_runtime_{get,put}
helpers to let memstick host support runtime pm.
There's a small window between memstick_detect_change() and its queued
work, memstick_check(). In this window the rpm count may go down to zero
before the memstick host powers on, so the host can be inadvertently
suspended.
Increment rpm count before calling memstick_check(), and decrement rpm
count afterward, as now we are sure the memstick host should be
suspended or not.
Signed-off-by: Kai-Heng Feng
---
drivers/memstick/core/memstick.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c
index 76382c858c35..944fe0c93fa7 100644
--- a/drivers/memstick/core/memstick.c
+++ b/drivers/memstick/core/memstick.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#define DRIVER_NAME "memstick"
@@ -209,6 +210,7 @@ static int memstick_dummy_check(struct memstick_dev *card)
*/
void memstick_detect_change(struct memstick_host *host)
{
+ pm_runtime_get_noresume(host->dev.parent);
queue_work(workqueue, >media_checker);
}
EXPORT_SYMBOL(memstick_detect_change);
@@ -479,6 +481,8 @@ static void memstick_check(struct work_struct *work)
host->set_param(host, MEMSTICK_POWER, MEMSTICK_POWER_OFF);
mutex_unlock(>lock);
+
+ pm_runtime_put_noidle(host->dev.parent);
dev_dbg(>dev, "memstick_check finished\n");
}
--
2.17.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 3/5] memstick: rtsx_usb_ms: Use ms_dev() helper
Use ms_dev() helper for consistency.
Signed-off-by: Kai-Heng Feng
---
drivers/memstick/host/rtsx_usb_ms.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/memstick/host/rtsx_usb_ms.c
b/drivers/memstick/host/rtsx_usb_ms.c
index 4f64563df7de..cd12f3d1c088 100644
--- a/drivers/memstick/host/rtsx_usb_ms.c
+++ b/drivers/memstick/host/rtsx_usb_ms.c
@@ -785,7 +785,7 @@ static int rtsx_usb_ms_drv_remove(struct platform_device
*pdev)
mutex_lock(>host_mutex);
if (host->req) {
- dev_dbg(&(pdev->dev),
+ dev_dbg(ms_dev(host),
"%s: Controller removed during transfer\n",
dev_name(>dev));
host->req->error = -ENOMEDIUM;
@@ -807,10 +807,10 @@ static int rtsx_usb_ms_drv_remove(struct platform_device
*pdev)
if (pm_runtime_active(ms_dev(host)))
pm_runtime_put(ms_dev(host));
- pm_runtime_disable(>dev);
+ pm_runtime_disable(ms_dev(host));
platform_set_drvdata(pdev, NULL);
- dev_dbg(&(pdev->dev),
+ dev_dbg(ms_dev(host),
": Realtek USB Memstick controller has been removed\n");
return 0;
--
2.17.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 0/5 v3] Keep rtsx_usb suspended when there's no card
Hi, This is based on Ulf's work [1] [2]. This patch series can keep rtsx_usb suspended, to save ~0.5W on Intel platforms and ~1.5W on AMD platforms. [1] https://patchwork.kernel.org/patch/10440583/ [2] https://patchwork.kernel.org/patch/10445725/ Kai-Heng Feng (5): misc: rtsx_usb: Use USB remote wakeup signaling for card insertion detection memstick: Prevent memstick host from getting runtime suspended during card detection memstick: rtsx_usb_ms: Use ms_dev() helper memstick: rtsx_usb_ms: Support runtime power management misc: rtsx_usb: / memstick: rtsx_usb_ms: Avoid long delay before system suspend -- v3: Skip parent device check in rtsx_usb_resume_child() . Remove dev_dbg() if it only prints function name. Use ms_dev() helper at more places. Remove const qualifier for UNIVERSAL_DEV_PM_OPS. v2: Resend to linux-usb and LKML. drivers/memstick/core/memstick.c| 4 + drivers/memstick/host/rtsx_usb_ms.c | 148 +++- drivers/misc/cardreader/rtsx_usb.c | 9 ++ 3 files changed, 91 insertions(+), 70 deletions(-) -- 2.17.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 4/8] usb: dwc3: implement stream transfer timeout
On Wed, Jul 25, 2018 at 6:14 PM, Anurag Kumar Vulisha wrote: >>> +/* >>> + * Timeout value in msecs used by stream_timeout_timer when streams >>> +are enabled */ >>> +#define STREAM_TIMEOUT 50 >> >>Perhaps, STREAM_TIMEOUT_MS 50 >> >>Dunno about this driver, but it's a usual practice to help reader with >>understanding >>code on the first glance. >> > > Since I have mentioned "msecs" in comment above describing the STREAM_TIMEOUT, > thought it would suffice. But if you feel I should change it, I will fix it > in v2 But you didn't put that comment before each occurrence of this constant in the code, right? That's what I'm talking about. If reader looks into the code, there is no need to understand the order of the value for timeout, since units are part of the name. Of course, you may ignore this comment. -- With Best Regards, Andy Shevchenko -- 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/8] usb: dwc3: implement stream transfer timeout
Hi Andy, Thanks for your review comments, please find my comments inline >-Original Message- >From: Andy Shevchenko [mailto:andy.shevche...@gmail.com] >Sent: Wednesday, July 25, 2018 8:36 PM >To: Anurag Kumar Vulisha >Cc: Felipe Balbi ; Greg Kroah-Hartman >; v.anuragku...@gmail.com; USB u...@vger.kernel.org>; Linux Kernel Mailing List >Subject: Re: [PATCH 4/8] usb: dwc3: implement stream transfer timeout > >On Wed, Jul 25, 2018 at 2:51 PM, Anurag Kumar Vulisha > wrote: >> According to dwc3 databook when streams are used, it may be possible >> for the host and device become out of sync, where device may wait for >> host to issue prime transcation and host may wait for device to issue >> erdy. To avoid such deadlock, timeout needs to be implemented. After >> timeout occurs, device will first stop transfer and restart the >> transfer again. This patch does the same. > >> +/* >> + * Timeout value in msecs used by stream_timeout_timer when streams >> +are enabled */ >> +#define STREAM_TIMEOUT 50 > >Perhaps, STREAM_TIMEOUT_MS 50 > >Dunno about this driver, but it's a usual practice to help reader with >understanding >code on the first glance. > Since I have mentioned "msecs" in comment above describing the STREAM_TIMEOUT, thought it would suffice. But if you feel I should change it, I will fix it in v2 Thanks, Anurag Kumar Vulisha >-- >With Best Regards, >Andy Shevchenko
Re: [PATCH 4/8] usb: dwc3: implement stream transfer timeout
On Wed, Jul 25, 2018 at 2:51 PM, Anurag Kumar Vulisha wrote: > According to dwc3 databook when streams are used, it may be possible > for the host and device become out of sync, where device may wait for > host to issue prime transcation and host may wait for device to issue > erdy. To avoid such deadlock, timeout needs to be implemented. After > timeout occurs, device will first stop transfer and restart the transfer > again. This patch does the same. > +/* > + * Timeout value in msecs used by stream_timeout_timer when streams are > enabled > + */ > +#define STREAM_TIMEOUT 50 Perhaps, STREAM_TIMEOUT_MS 50 Dunno about this driver, but it's a usual practice to help reader with understanding code on the first glance. -- With Best Regards, Andy Shevchenko -- 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 5/8] usb: dwc3: don't issue no-op trb for stream capable endpoints
The stream capable endpoints require stream id to be given
when issuing START TRANSFER. While issuing no-op trb the
stream id is not yet known, so don't issue no-op trb's on
stream capable endpoints.
Signed-off-by: Anurag Kumar Vulisha
---
drivers/usb/dwc3/gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index e2ccb55..af8d470 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -668,7 +668,7 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
unsigned int action)
* Issue StartTransfer here with no-op TRB so we can always rely on No
* Response Update Transfer command.
*/
- if (usb_endpoint_xfer_bulk(desc) ||
+ if ((usb_endpoint_xfer_bulk(desc) && !dep->stream_capable) ||
usb_endpoint_xfer_int(desc)) {
struct dwc3_gadget_ep_cmd_params params;
struct dwc3_trb *trb;
--
2.1.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 3/8] usb: dwc3: make controller clear transfer resources after complete
To start transfer with another stream id, controller needs to free
previously allocated transfer resource. This will be automatically
done by the controller at the time of XferComplete Event. This
patch updates the code to issue XferComplete event once all transfers
are done by setting LST bit in the ctrl field of the last TRB.
Signed-off-by: Anurag Kumar Vulisha
---
drivers/usb/dwc3/gadget.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index efc6e13..b3e9e7f 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -571,7 +571,8 @@ static int dwc3_gadget_set_ep_config(struct dwc3_ep *dep,
unsigned int action)
if (usb_ss_max_streams(comp_desc) && usb_endpoint_xfer_bulk(desc)) {
params.param1 |= DWC3_DEPCFG_STREAM_CAPABLE
- | DWC3_DEPCFG_STREAM_EVENT_EN;
+ | DWC3_DEPCFG_STREAM_EVENT_EN
+ | DWC3_DEPCFG_XFER_COMPLETE_EN;
dep->stream_capable = true;
}
@@ -999,6 +1000,15 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep,
struct dwc3_trb *trb,
if (chain)
trb->ctrl |= DWC3_TRB_CTRL_CHN;
+ /*
+* To issue start transfer on another stream, controller need to free
+* previously acquired transfer resource. Setting the LST bit in
+* last TRB makes the controller clear transfer resource for that
+* endpoint, allowing to start another stream on that endpoint.
+*/
+ else if (dep->stream_capable)
+ trb->ctrl |= DWC3_TRB_CTRL_LST;
+
if (usb_endpoint_xfer_bulk(dep->endpoint.desc) && dep->stream_capable)
trb->ctrl |= DWC3_TRB_CTRL_SID_SOFN(stream_id);
@@ -2268,7 +2278,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct
dwc3_ep *dep,
if (event->status & DEPEVT_STATUS_SHORT && !chain)
return 1;
- if (event->status & DEPEVT_STATUS_IOC)
+ if (event->status & (DEPEVT_STATUS_IOC | DEPEVT_STATUS_LST))
return 1;
return 0;
@@ -2457,6 +2467,10 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
}
switch (event->endpoint_event) {
+ case DWC3_DEPEVT_XFERCOMPLETE:
+ if (!dep->stream_capable)
+ break;
+ dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
case DWC3_DEPEVT_XFERINPROGRESS:
dwc3_gadget_endpoint_transfer_in_progress(dep, event);
break;
@@ -2472,7 +2486,6 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
}
break;
case DWC3_DEPEVT_STREAMEVT:
- case DWC3_DEPEVT_XFERCOMPLETE:
case DWC3_DEPEVT_RXTXFIFOEVT:
break;
}
--
2.1.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 4/8] usb: dwc3: implement stream transfer timeout
According to dwc3 databook when streams are used, it may be possible
for the host and device become out of sync, where device may wait for
host to issue prime transcation and host may wait for device to issue
erdy. To avoid such deadlock, timeout needs to be implemented. After
timeout occurs, device will first stop transfer and restart the transfer
again. This patch does the same.
Signed-off-by: Anurag Kumar Vulisha
---
drivers/usb/dwc3/core.h | 7 +++
drivers/usb/dwc3/gadget.c | 39 +++
2 files changed, 46 insertions(+)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 285ce0e..581f619 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -619,6 +619,11 @@ struct dwc3_event_buffer {
#define DWC3_TRB_NUM 256
+/*
+ * Timeout value in msecs used by stream_timeout_timer when streams are enabled
+ */
+#define STREAM_TIMEOUT 50
+
/**
* struct dwc3_ep - device side endpoint representation
* @endpoint: usb endpoint
@@ -642,6 +647,7 @@ struct dwc3_event_buffer {
* @name: a human readable name e.g. ep1out-bulk
* @direction: true for TX, false for RX
* @stream_capable: true when streams are enabled
+ * @stream_timeout_timer: timer used to aviod deadlock when streams are used
*/
struct dwc3_ep {
struct usb_ep endpoint;
@@ -691,6 +697,7 @@ struct dwc3_ep {
unsigneddirection:1;
unsignedstream_capable:1;
+ struct timer_list stream_timeout_timer;
};
enum dwc3_phy {
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index b3e9e7f..e2ccb55 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -254,6 +254,7 @@ int dwc3_send_gadget_generic_command(struct dwc3 *dwc,
unsigned cmd, u32 param)
}
static int __dwc3_gadget_wakeup(struct dwc3 *dwc);
+static void stream_timeout_function(struct timer_list *arg);
/**
* dwc3_send_gadget_ep_cmd - issue an endpoint command
@@ -574,6 +575,8 @@ static int dwc3_gadget_set_ep_config(struct dwc3_ep *dep,
unsigned int action)
| DWC3_DEPCFG_STREAM_EVENT_EN
| DWC3_DEPCFG_XFER_COMPLETE_EN;
dep->stream_capable = true;
+ timer_setup(>stream_timeout_timer,
+ stream_timeout_function, 0);
}
if (!usb_endpoint_xfer_control(desc))
@@ -730,6 +733,9 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
trace_dwc3_gadget_ep_disable(dep);
+ if (dep->stream_capable)
+ del_timer(>stream_timeout_timer);
+
dwc3_remove_requests(dwc, dep);
/* make sure HW endpoint isn't stalled */
@@ -1257,6 +1263,12 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep
*dep)
return ret;
}
+ if (starting && dep->stream_capable) {
+ dep->stream_timeout_timer.expires = jiffies +
+ msecs_to_jiffies(STREAM_TIMEOUT);
+ add_timer(>stream_timeout_timer);
+ }
+
return 0;
}
@@ -2403,6 +2415,13 @@ static void
dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
stop = true;
}
+ /*
+* Delete the timer that was started in __dwc3_gadget_kick_transfer()
+* for stream capable endpoints.
+*/
+ if (dep->stream_capable)
+ del_timer(>stream_timeout_timer);
+
dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
if (stop) {
@@ -2486,6 +2505,14 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
}
break;
case DWC3_DEPEVT_STREAMEVT:
+ switch (event->status) {
+ case DEPEVT_STREAMEVT_FOUND:
+ del_timer(>stream_timeout_timer);
+ break;
+ case DEPEVT_STREAMEVT_NOTFOUND:
+ default:
+ dev_err(dwc->dev, "unable to find suitable stream");
+ }
case DWC3_DEPEVT_RXTXFIFOEVT:
break;
}
@@ -2587,6 +2614,18 @@ static void dwc3_stop_active_transfer(struct dwc3_ep
*dep, bool force)
}
}
+static void stream_timeout_function(struct timer_list *arg)
+{
+ struct dwc3_ep *dep = from_timer(dep, arg, stream_timeout_timer);
+ struct dwc3 *dwc = dep->dwc;
+ unsigned long flags;
+
+ spin_lock_irqsave(>lock, flags);
+ dwc3_stop_active_transfer(dep, true);
+ __dwc3_gadget_kick_transfer(dep);
+ spin_unlock_irqrestore(>lock, flags);
+}
+
static void dwc3_clear_stall_all_ep(struct dwc3 *dwc)
{
u32 epnum;
--
2.1.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 8/8] usb: dwc3: Check MISSED ISOC bit only for ISOC endpoints
When streaming is enabled on BULK endpoints and LST bit is set
observed MISSED ISOC bit set in event->status for BULK ep. Since
this bit is only valid for isocronous endpoints, changed the code
to check for isocrnous endpoints when MISSED ISOC bit is set.
Signed-off-by: Anurag Kumar Vulisha
---
drivers/usb/dwc3/gadget.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 1e5cd4b..0293be2 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2413,7 +2413,8 @@ static void
dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
if (event->status & DEPEVT_STATUS_BUSERR)
status = -ECONNRESET;
- if (event->status & DEPEVT_STATUS_MISSED_ISOC) {
+ if ((event->status & DEPEVT_STATUS_MISSED_ISOC) &&
+ usb_endpoint_xfer_isoc(dep->endpoint.desc)) {
status = -EXDEV;
if (list_empty(>started_list))
--
2.1.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 6/8] usb: dwc3: check for requests in started list for stream capable endpoints
For stream capable endpoints, uas layer can queue mulpile requests on
single ep with different stream ids. So, there can be multiple pending
requests waiting to be transferred. This patch changes the code to check
for any pending requests waiting to be transferred on ep started_list and
calls __dwc3_gadget_kick_transfer() if any.
Signed-off-by: Anurag Kumar Vulisha
---
drivers/usb/dwc3/gadget.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index af8d470..fe1ea245 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2424,6 +2424,9 @@ static void
dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
+ if (dep->stream_capable && !list_empty(>started_list))
+ __dwc3_gadget_kick_transfer(dep);
+
if (stop) {
dwc3_stop_active_transfer(dep, true);
dep->flags = DWC3_EP_ENABLED;
--
2.1.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 7/8] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields
The present code in dwc3_gadget_ep_reclaim_completed_trb() will check for IOC/LST bit in the event->status and returns if IOC/LST bit is set. This logic doesn't work if multiple TRBs are queued per request and the IOC/LST bit is set on the last TRB of that request. Consider an example where a queued request has multiple queued TRBs and IOC/LST bit is set only for the last TRB. In this case, the Core generates XferComplete/XferInProgress events only for the last TRB (since IOC/LST are set only for the last TRB). As per the logic in dwc3_gadget_ep_reclaim_completed_trb() event->status is checked for IOC/LST bit and returns on the first TRB. This makes the remaining TRBs left unhandled. To aviod this, changed the code to check for IOC/LST bits in both event->status & TRB->ctrl. This patch does the same. Signed-off-by: Anurag Kumar Vulisha --- drivers/usb/dwc3/gadget.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index fe1ea245..1e5cd4b 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2290,7 +2290,12 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, if (event->status & DEPEVT_STATUS_SHORT && !chain) return 1; - if (event->status & (DEPEVT_STATUS_IOC | DEPEVT_STATUS_LST)) + if ((event->status & DEPEVT_STATUS_IOC) && + (trb->ctrl & DWC3_TRB_CTRL_IOC)) + return 1; + + if ((event->status & DEPEVT_STATUS_LST) && + (trb->ctrl & DWC3_TRB_CTRL_LST)) return 1; return 0; -- 2.1.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 1/8] usb: dwc3: Correct the logic for checking TRB full in __dwc3_prepare_one_trb()
In __dwc3_prepare_one_trb(), IOC bit is set if the total number of available TRB's are zero. The number of available TRBs are calculated using dwc3_calc_trbs_left(), which determines TRBs full or not based on HWO bit set in the TRB. During preparation of TRB __dwc3_prepare_one_trb() is always called with a TRB which doesn't have HWO bit set. __dwc3_prepare_one_trb() depends on dwc3_calc_trbs_left() to calculate the free available TRBs and set IOC bit if not available. Since dwc3_calc_trbs_left() determines TRB available based on HWO bit, there are chances that dwc3_calc_trbs_left() wrongly calculates the present working TRB as free TRB (since HWO bit is not yet set). This patch corrects this issue by setting HWO bit before calling dwc3_calc_trbs_left(). Signed-off-by: Anurag Kumar Vulisha --- drivers/usb/dwc3/gadget.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 69bf137..f73d219 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -990,6 +990,8 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, trb->ctrl |= DWC3_TRB_CTRL_ISP_IMI; } + trb->ctrl |= DWC3_TRB_CTRL_HWO; + if ((!no_interrupt && !chain) || (dwc3_calc_trbs_left(dep) == 0)) trb->ctrl |= DWC3_TRB_CTRL_IOC; @@ -1000,8 +1002,6 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, if (usb_endpoint_xfer_bulk(dep->endpoint.desc) && dep->stream_capable) trb->ctrl |= DWC3_TRB_CTRL_SID_SOFN(stream_id); - trb->ctrl |= DWC3_TRB_CTRL_HWO; - trace_dwc3_prepare_trb(dep, trb); } -- 2.1.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 2/8] usb: dwc3: update stream id in depcmd
For stream capable endpoints, stream id related information
needs to be updated into DEPCMD while issuing START TRANSFER.
This patch does the same.
Signed-off-by: Anurag Kumar Vulisha
---
drivers/usb/dwc3/gadget.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index f73d219..efc6e13 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1224,6 +1224,9 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep
*dep)
params.param1 = lower_32_bits(req->trb_dma);
cmd = DWC3_DEPCMD_STARTTRANSFER;
+ if (dep->stream_capable)
+ cmd |= DWC3_DEPCMD_PARAM(req->request.stream_id);
+
if (usb_endpoint_xfer_isoc(dep->endpoint.desc))
cmd |= DWC3_DEPCMD_PARAM(dep->frame_number);
} else {
--
2.1.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 0/8] fix broken BULK stream support to dwc3 gadget driver
These patch series fixes the broken BULK streaming support in dwc3 gadget driver. Anurag Kumar Vulisha (8): usb: dwc3: Correct the logic for checking TRB full in __dwc3_prepare_one_trb() usb: dwc3: update stream id in depcmd usb: dwc3: make controller clear transfer resources after complete usb: dwc3: implement stream transfer timeout usb: dwc3: don't issue no-op trb for stream capable endpoints usb: dwc3: check for requests in started list for stream capable endpoints usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields usb: dwc3: Check MISSED ISOC bit only for ISOC endpoints drivers/usb/dwc3/core.h | 7 + drivers/usb/dwc3/gadget.c | 78 ++- 2 files changed, 78 insertions(+), 7 deletions(-) -- 2.1.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] Driver for MaxLinear/Exar USB (UART) Serial Adapters
On Di, 2018-07-24 at 15:36 -0700, Patong Yang wrote:
> +static int xrusb_ctrl_msg(struct xrusb *xrusb,
> + int request, int value, void *buf, int len)
> +{
> + int rv = usb_control_msg(xrusb->dev,
> + usb_sndctrlpipe(xrusb->dev, 0),
> + request,
> + USB_RT_XRUSB,
> + value,
> + xrusb->control->altsetting[0].desc.bInterfaceNumber,
> + buf,
> + len,
> + 5000);
Please use the symbolic constant.
> + return rv < 0 ? rv : 0;
> +}
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
