Typically FPGA devices are configured with CoreConsultant parameter
DWC_USB3x_EN_LOG_PHYS_EP_SUPT=0 to reduce gate count and improve timing.
This means that the number of INs equals to OUTs endpoints. But
typically non-FPGA devices enable this CoreConsultant parameter to
support flexible endpoint mapping and potentially may have unequal
number of INs to OUTs physical endpoints.
The driver must check how many physical endpoints are available for each
direction and initialize them properly.
Cc: stable(a)vger.kernel.org
Fixes: 47d3946ea220 ("usb: dwc3: refactor gadget endpoint count calculation")
Signed-off-by: Thinh Nguyen <Thinh.Nguyen(a)synopsys.com>
---
drivers/usb/dwc3/core.c | 1 +
drivers/usb/dwc3/core.h | 2 ++
drivers/usb/dwc3/gadget.c | 19 ++++++++++++-------
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 841daec70b6e..1084aa8623c2 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -529,6 +529,7 @@ static void dwc3_core_num_eps(struct dwc3 *dwc)
struct dwc3_hwparams *parms = &dwc->hwparams;
dwc->num_eps = DWC3_NUM_EPS(parms);
+ dwc->num_in_eps = DWC3_NUM_IN_EPS(parms);
}
static void dwc3_cache_hwparams(struct dwc3 *dwc)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 1b241f937d8f..1295dac019f9 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -990,6 +990,7 @@ struct dwc3_scratchpad_array {
* @u1sel: parameter from Set SEL request.
* @u1pel: parameter from Set SEL request.
* @num_eps: number of endpoints
+ * @num_in_eps: number of IN endpoints
* @ep0_next_event: hold the next expected event
* @ep0state: state of endpoint zero
* @link_state: link state
@@ -1193,6 +1194,7 @@ struct dwc3 {
u8 speed;
u8 num_eps;
+ u8 num_in_eps;
struct dwc3_hwparams hwparams;
struct dentry *root;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 25f654b79e48..8a38ee10c00b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2025,7 +2025,7 @@ static void dwc3_stop_active_transfers(struct dwc3 *dwc)
{
u32 epnum;
- for (epnum = 2; epnum < dwc->num_eps; epnum++) {
+ for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
struct dwc3_ep *dep;
dep = dwc->eps[epnum];
@@ -2628,16 +2628,21 @@ static int dwc3_gadget_init_endpoint(struct dwc3 *dwc, u8 epnum)
return 0;
}
-static int dwc3_gadget_init_endpoints(struct dwc3 *dwc, u8 total)
+static int dwc3_gadget_init_endpoints(struct dwc3 *dwc)
{
- u8 epnum;
+ u8 i;
+ int ret;
INIT_LIST_HEAD(&dwc->gadget->ep_list);
- for (epnum = 0; epnum < total; epnum++) {
- int ret;
+ for (i = 0; i < dwc->num_in_eps; i++) {
+ ret = dwc3_gadget_init_endpoint(dwc, i * 2 + 1);
+ if (ret)
+ return ret;
+ }
- ret = dwc3_gadget_init_endpoint(dwc, epnum);
+ for (i = 0; i < dwc->num_eps - dwc->num_in_eps; i++) {
+ ret = dwc3_gadget_init_endpoint(dwc, i * 2);
if (ret)
return ret;
}
@@ -3863,7 +3868,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
* sure we're starting from a well known location.
*/
- ret = dwc3_gadget_init_endpoints(dwc, dwc->num_eps);
+ ret = dwc3_gadget_init_endpoints(dwc);
if (ret)
goto err4;
base-commit: 96ebc9c871d8a28fb22aa758dd9188a4732df482
--
2.28.0
So this set has grown further than I expected.
This addresses most reviews from Paul and also consolidates the nocb
timers code.
Please mind the very first patch that is a stable bugfix.
git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
rcu/dev
HEAD: 75991420c246c26f598602da1a70947b5bdf77b6
Thanks,
Frederic
---
Frederic Weisbecker (16):
rcu/nocb: Fix potential missed nocb_timer rearm
rcu/nocb: Comment the reason behind BH disablement on batch processing
rcu/nocb: Forbid NOCB toggling on offline CPUs
rcu/nocb: Only (re-)initialize segcblist when needed on CPU up
rcu/nocb: Disable bypass when CPU isn't completely offloaded
rcu/nocb: Avoid confusing double write of rdp->nocb_cb_sleep
rcu/nocb: Rename nocb_gp_update_state to nocb_gp_update_state_deoffloading
rcu/nocb: Move trace_rcu_nocb_wake() calls outside nocb_lock when possible
rcu/nocb: Merge nocb_timer to the rdp leader
rcu/nocb: Directly call __wake_nocb_gp() from bypass timer
rcu/nocb: Allow de-offloading rdp leader
rcu/nocb: Cancel nocb_timer upon nocb_gp wakeup
rcu/nocb: Delete bypass_timer upon nocb_gp wakeup
rcu/nocb: Only cancel nocb timer if not polling
rcu/nocb: Prepare for finegrained deferred wakeup
rcu/nocb: Unify timers
include/linux/rcu_segcblist.h | 7 +-
include/trace/events/rcu.h | 1 +
kernel/rcu/tree.c | 12 +-
kernel/rcu/tree.h | 9 +-
kernel/rcu/tree_plugin.h | 280 ++++++++++++++++++++++--------------------
5 files changed, 163 insertions(+), 146 deletions(-)
On Thu, Jan 28, 2021 at 11:12:28AM -0800, Paul E. McKenney wrote:
> On Thu, Jan 28, 2021 at 06:12:10PM +0100, Frederic Weisbecker wrote:
> > Simply checking if the segcblist is enabled is enough to know if we
> > need to initialize it or not. It's safe to check within hotplug
> > machine.
> >
> > Signed-off-by: Frederic Weisbecker <frederic(a)kernel.org>
> > Cc: Josh Triplett <josh(a)joshtriplett.org>
> > Cc: Lai Jiangshan <jiangshanlai(a)gmail.com>
> > Cc: Joel Fernandes <joel(a)joelfernandes.org>
> > Cc: Neeraj Upadhyay <neeraju(a)codeaurora.org>
> > Cc: Boqun Feng <boqun.feng(a)gmail.com>
>
> Hmmm...
>
> At the start of a CPU-hotplug operation, an incoming CPU's callback
> list can be in a number of states:
>
> 1. Disabled and empty. This is the case when the boot CPU has
> not done call_rcu(), when a non-boot CPU first comes online,
> and when a non-offloaded CPU comes back online. In this case,
> it is permissible to initialize ->cblist. Because either the
> CPU is currently running with interrupts disabled (boot CPU)
> or is not yet running at all (other CPUs), it is not necessary
> to acquire ->nocb_lock.
>
> 2. Disabled and non-empty. This is the case when the boot CPU has
> done call_rcu(). It is not permissible to initialize ->cblist
> because doing so will leak any callbacks posted by early boot
> invocations of call_rcu().
I don't think that's possible. In this case __call_rcu() has called
rcu_segcblist_init() and has enabled the segcblist.
>
> Test for the possibility of leaking by building with
> CONFIG_PROVE_RCU=y and booting with rcupdate.rcu_self_test=1.
>
> 3. Enabled, whether empty or not. This is the case when an
> offloaded CPU comes back online. This is the only case where
> the ->nocb_lock must be held to modify ->cblist. However,
> it is not necessarily to modify ->cblist because the rcuoc
> kthread is on the job.
>
> So I believe that it is necessary to check for both disabled and empty.
> But don't take my word for it! Build with CONFIG_PROVE_RCU=y and boot
> with rcupdate.rcu_self_test=1. ;-)
I'm trying that :-)