On Tue, Oct 01, 2013 at 10:05:17AM +0300, Dan Carpenter wrote:
> On Tue, Oct 01, 2013 at 01:21:28AM +, Paul Zimmerman wrote:
> > > From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> > > Sent: Monday, September 30, 2013 6:09 PM
> > >
> > > Yeah. I guess it's fine... I was going to sugges
Hi Dinh,
> >Somehow I assumed that was fixed by the hardware, but I see now that you
> >are right. Yes, making the definition larger is better than moving the
> >+ 1.
> This was my original fix to the problem, but I thought that it would
> be confusing when reading the code. I also thought about
Hey Dan, Dinh,
> > [resend]: previous reply didn't include Matthijs
> >
>
> He sets his Mail-Followup-To: so that we don't CC him on replies. I
> assume it's deliberate because he only wants the copy from the mailing
> list?
Exactly, I just set that for whatever mailing list I subscribe to.
How
instead of 4.
Before this commit, hardware with 16 host channels would overflow the
field, making it appear as 0 channels.
This bug was introduced in commit 9badec2 (staging: dwc2: interpret all
hwcfg and related register at init time).
Reported-by: Dinh Nguyen
Signed-off-by: Matthijs Kooijman
Hey Dinh,
> >Reported-by: Dinh Nguyen
> Can you please use:
> Dinh Nguyen
Sorry, I just used your sender address, should have checked your patch
instead.
Paul, if you can ack this patch, I'll resend it with the proper tag and
include your acked-by as well.
Gr.
Matthijs
signature.asc
Descrip
Hi Paul,
> By the way, it looks like 'num_dev_ep' would have the same problem,
I don't think so, since the hardware doesn't do the off-by-one trick
there (presumably because having 0 endpoints make sense, but 0 host
channels doesn't):
hw->num_dev_ep = (hwcfg2 & GHWCFG2_NUM_DEV_EP_MASK) >>
instead of 4.
Before this commit, hardware with 16 host channels would overflow the
field, making it appear as 0 channels.
This bug was introduced in commit 9badec2 (staging: dwc2: interpret all
hwcfg and related register at init time).
Reported-by: Dinh Nguyen
Signed-off-by: Matthijs Kooijman
fault value.
> (Stephen?)
I previously submitted a patch to load everything from DT, which might
serve as inspiration: http://article.gmane.org/gmane.linux.usb.general/85086
(but note this reply: http://article.gmane.org/gmane.linux.usb.general/85104)
> Signed-off-by: Paul Zimmerman
Revie
Hi Paul,
> The transfer scheduler in the dwc2 driver is pretty basic, not to
> mention buggy. It works fairly well with just a couple of devices
> plugged in, but if you add, say, multiple devices with periodic
> endpoints, the scheduler breaks down and can't even enumerate all
> the devices.
Thi
Hey Paul,
> > > diff --git a/drivers/staging/dwc2/core.h b/drivers/staging/dwc2/core.h
> > > index fc075a7..e771e40 100644
> > > --- a/drivers/staging/dwc2/core.h
> > > +++ b/drivers/staging/dwc2/core.h
> > > @@ -150,10 +150,11 @@ enum dwc2_lx_state {
> > > * are enabled
> >
Hey Paul,
one more thing:
> > > > + * -1 - GAHBCFG value will not be
> > > > overridden
This seems incorrect: If it is set to -1, GAHBCFG will be set to 0x06
(INCR4), it is not left unchanged. I'll also include this in my
documentation patch.
Gr.
Matthijs
___
Hi Paul,
> > Furthermore, I wonder about how this scheduler works exactly. What I see
> > is:
> > - the number usecs needed for a single transfer in a periodic qh is
> >calculated
> > - When the qh is scheduled, the first of the 8 microframes with enough
> >usecs available is picked for
Hi Paul,
> Add the NAK holdoff patch from the downstream Raspberry Pi kernel.
> This allows the transfer scheduler to better handle "cheeky devices
> that just hold off using NAKs".
> @@ -365,6 +366,7 @@ struct dwc2_hsotg {
> u8 otg_port;
> u32 *frame_list;
> dma_addr_t frame_li
Hi Paul & Dom,
I haven't got time to look closely right now, but at first glance most
of my comments have been resolved. One thing that is still in there, is
this piece of code for which I'm not sure if it is really related to the
topic of the patch:
> @@ -780,6 +784,10 @@ static void dwc2_assig
Hi Paul,
> Matthijs' concern about periodic endpoints with bInterval=1 seems
> to be unfounded. I tried a webcam, which uses a bInterval=1 isoc
> endpoint, on my PCI-based dev board, and it still works fine with
> this patch applied.
For the record, I still think this concern actually exists, but
Hi Paul,
> > > @@ -365,6 +366,7 @@ struct dwc2_hsotg {
> > > u8 otg_port;
> > > u32 *frame_list;
> > > dma_addr_t frame_list_dma;
> > > + int next_sched_frame;
> >
> > This variable is still not really used, I think. Most of the mentions in
> > the patch are assignments, except for these tw
16 matches
Mail list logo
