On Thu, Nov 03, 2016 at 09:09:46AM +0200, Felipe Balbi wrote: > > Hi, > > Bin Liu <b-liu@xxxxxx> writes: > >> > On Mon, Oct 31, 2016 at 12:47:52PM +0200, Felipe Balbi wrote: > >> >> Hi guys, > >> >> > >> >> Sorry for the patch bomb, but I wanted to make sure everyoby knows which > >> >> patches are already queued up for the next window. They are still > >> >> sitting in my testing/next branch, so I can still change any of them. > >> >> > >> >> Please make sure to go through each one of them. This very branch has > >> >> been tested on Intel SKL and I couldn't find any problems so far. > >> > > >> > It seems you missed my comments in the RFC set, basically the set breaks > >> > MUSB and other controllers. > >> > >> Okay, I'll go look for the original comments. > > > > I also commented in the RFC for two more issues. > > > > - some udc drivers use 0-based mult, but the new > > usb_endpoint_maxp_mult() makes it 1-based. > > no problems there. Some will use the value as is while some will > decrement 1. I see the issue is that some patches in this set only replaces usb_endpoint_maxp() to usb_endpoint_maxp_mult(), but doesn't decrease the result by 1. Please check patch 15, 16, 18, 19, and 21 in this set. I commented the same in the original RFC set. > > > - some udc drivers keeps bit 11 & 12 when calling the old > > usb_endpoint_maxp(), but the new version masked bit 11 & 12 already. > > those have been updated to use usb_endpoint_maxp_mult(). Are there any > missing? Please check patch 39, 40, 41, I am not sure how the controller works, but at least I can tell these patches change how the variable 'max' is used. (I guess I should put this comments in the those patches directly, now hard to follow...) If you don't see any issue with these changes, please ignore my comments here. Regards, -Bin. > > > And here is my comment in RFC/PATCH 01/45. > > > > " > > It seems the instances which use 1 based value is less than those use 0 > > based. To me it seems make sense to just return 0 based value here. > > well, we have to add decrement, sure. But we can use this to get rid of > unnecessary branches. > > IOW, this: > > size = usb_endpoint_maxp(desc); > > if (usb_endpoint_xfer_isoc(desc)) > size *= usb_endpoint_maxp_mult(desc); > > Can be written as: > > size = usb_endpoint_maxp(desc) * usb_endpoint_maxp_mult(desc); > > If this isn't valid, you have corrupt registers in your gadget driver > anyway. > > > Some controllers like musb writes the 0 based value to a register. > > " > > And MUSB still writes 0-based value to the register because we're > decrementing 1 from the value returned by usb_endpoint_maxp_mult() > > -- > 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
