On Wed, Jan 22, 2025 at 01:44:02AM +0000, Thinh Nguyen wrote:
> On Fri, Jan 17, 2025, Andy Shevchenko wrote:
> > On Thu, Jan 16, 2025 at 11:35:19PM +0000, Thinh Nguyen wrote:
> > > On Thu, Jan 16, 2025, Andy Shevchenko wrote:
...
> > > > for (epnum = 0; epnum < total; epnum++) {
> > > > - int ret;
> > > > + for (num = 0; num < count; num++) {
> > > > + if (epnum == eps[num])
> > > > + break;
> > > > + }
> > > > + if (num < count)
> > > > + continue;
> > >
> > > You can probably rewrite this logic better.
> >
> > Any suggestions?
> >
> > Thanks for the review!
>
> From the first look, is the list sorted? If so, you don't need another
> for-loop.
Even if it's sorted it's not 1:1 mapped by indices. I do not understand how we
can avoid the second loop. The only possibility is indeed to sort the list and
sparse it in accordance to the endpoint numbers, but if we are going this way,
it's much easier to switch to bitmap and the respective bitops.
> Also, we loop over the number of endpoints throughout the driver, but
> you only skip it here during init. Please double check for invalid
> accessing of endpoints in other places.
>
> Perhaps set the dwc->eps[reserved_ep] to ERR_PTR(-EINVAL) or something
> when you parse the reserved endpoints so you can skip them in your loop.
Note, this is only for UDC, in USB host these are okay to be used.
Does your suggestion imply that?
--
With Best Regards,
Andy Shevchenko
