I'll send a single patch for the minimal bug fix for the rc and I'll send the rest of the changes as a patch set for 4.3. Thanks Al On Thu, Jul 23, 2015 at 4:05 PM, Felipe Balbi <balbi@xxxxxx> wrote: > On Thu, Jul 23, 2015 at 03:23:26PM -0400, Alan Cooper wrote: >> On Wed, Jul 22, 2015 at 10:03 PM, Felipe Balbi <balbi@xxxxxx> wrote: >> > Hi, >> > >> > On Wed, Jul 22, 2015 at 06:27:29PM -0400, Alan Cooper wrote: >> >> On Wed, Jul 22, 2015 at 5:29 PM, Felipe Balbi <balbi@xxxxxx> wrote: >> >> > On Wed, Jul 22, 2015 at 04:26:57PM -0500, Felipe Balbi wrote: >> >> >> On Wed, Jul 22, 2015 at 04:58:07PM -0400, Al Cooper wrote: >> >> >> > V2 - Fix a compiler bug that happend when the config options >> >> >> > CONFIG_USB_GADGET_DEBUG and CONFIG_USB_GADGET_VERBOSE >> >> >> > were enabled. >> >> >> > >> >> >> > ep_dequeue() in bdc_ep.c was capturing the hw dequeue pointer >> >> >> > incorrectly by reading the wrong register for the upper 32 bits. >> >> >> > The header file defining the registers was incorrect. >> >> >> >> >> >> btw, the header file was really "incorrect" as long as you passed 0 to >> >> >> the argument :-p >> >> > >> >> > in fact, the minimal fix for this bug would be the one below: >> >> > >> >> > diff --git a/drivers/usb/gadget/udc/bdc/bdc_ep.c b/drivers/usb/gadget/udc/bdc/bdc_ep.c >> >> > index b04980cf6dc4..1efa61265d8d 100644 >> >> > --- a/drivers/usb/gadget/udc/bdc/bdc_ep.c >> >> > +++ b/drivers/usb/gadget/udc/bdc/bdc_ep.c >> >> > @@ -779,7 +779,7 @@ static int ep_dequeue(struct bdc_ep *ep, struct bdc_req *req) >> >> > /* The current hw dequeue pointer */ >> >> > tmp_32 = bdc_readl(bdc->regs, BDC_EPSTS0(0)); >> >> > deq_ptr_64 = tmp_32; >> >> > - tmp_32 = bdc_readl(bdc->regs, BDC_EPSTS0(1)); >> >> > + tmp_32 = bdc_readl(bdc->regs, BDC_EPSTS1(0)); >> >> > deq_ptr_64 |= ((u64)tmp_32 << 32); >> >> > >> >> > /* we have the dma addr of next bd that will be fetched by hardware */ >> >> > >> >> > And $subject becomes a cleanup patch for v4.3. Can you make these >> >> > changes, please ? >> >> >> >> Yes, I can make these changes, but first let me explain why I did it this way. >> >> The 8 End Point Status registers are 32 bit consecutive registers, so >> >> defining them as: >> >> #define BDC_EPSTS0(n) (0x60 + (n * 0x10)) >> >> #define BDC_EPSTS1(n) (0x64 + (n * 0x10)) >> >> #define BDC_EPSTS2(n) (0x68 + (n * 0x10)) >> >> #define BDC_EPSTS3(n) (0x6c + (n * 0x10)) >> >> #define BDC_EPSTS4(n) (0x70 + (n * 0x10)) >> >> #define BDC_EPSTS5(n) (0x74 + (n * 0x10)) >> >> #define BDC_EPSTS6(n) (0x78 + (n * 0x10)) >> >> #define BDC_EPSTS7(n) (0x7c + (n * 0x10)) >> >> seems misleading, does not reflect the hardware and using anything >> >> other than (0) would get you to some other unexpected register and >> >> should be considered a coding error. >> > >> > it sure is, but the minimal patch for -rc is what I sent above :-) As >> > long as you pass 0 as parameters, all your offsets are correct, so >> > removing the parameter (which must be always zero) is, actually, >> > refactoring happening. >> > >> >> OK, I think I understand. You're asking me to split the bug fix patch >> into two patches, one is a minimal bug fix patch for -rc and the other >> is a cleanup patch (fix the header file) that will go into 4.3. Is >> that correct? > > correct. > >> >> I think the original hardware spec had each End Point Status as a >> >> block of registers but the first silicon had them as single registers >> >> and they are expected to stay that way. >> > >> > I believe they wanted to have: >> > >> > #define BDC_EPSTS(n) (0x60 + (n * 0x4)) >> > >> > and that way you can access each EP by passing the correct argument to >> > that macro. >> > >> >> Actually, the 8 EPSTS registers are all different and together contain >> the info for 1 end point. The original plan was to have 4 32bit >> registers per end point and enough sets of these registers for all >> endpoints, but this turned out to be too expensive in silicon so they >> changed it to 1 set of 8 registers that are updated when an end point >> is stopped. > > oh ok. thanks > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
