2013-08-05 08:07:31

by Du, ChangbinX

[permalink] [raw]
Subject: About TRB_TO_EP_INDEX() macro using

Recently when I check xHCI code, find that some functions try to get EP index
from a Command Completion Event TRB via TRB_TO_EP_INDEX() macro.

This is totally wrong. The macro definition is:

#define TRB_TO_EP_INDEX(p) ((((p) & (0x1f << 16)) >> 16) - 1)

TRB_TO_EP_INDEX only applies to Transfer Event TRB. But for Command
Completion Event TRB, there is no Endpoint ID field. So, we cannot directly
get EP index from these TRBs, but we can find it by the TRB Pointer.

Here list two functions for you to check:
handle_stopped_endpoint()
handle_reset_ep_completion()

Regards & Thanks!
Changbin


2013-08-05 08:33:34

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: About TRB_TO_EP_INDEX() macro using

On Mon, Aug 05, 2013 at 08:07:25AM +0000, Du, ChangbinX wrote:
> Recently when I check xHCI code, find that some functions try to get EP index
> from a Command Completion Event TRB via TRB_TO_EP_INDEX() macro.
>
> This is totally wrong. The macro definition is:
>
> #define TRB_TO_EP_INDEX(p) ((((p) & (0x1f << 16)) >> 16) - 1)
>
> TRB_TO_EP_INDEX only applies to Transfer Event TRB. But for Command
> Completion Event TRB, there is no Endpoint ID field. So, we cannot directly
> get EP index from these TRBs, but we can find it by the TRB Pointer.
>
> Here list two functions for you to check:
> handle_stopped_endpoint()
> handle_reset_ep_completion()

Care to send a patch showing how you would change this if it is
incorrect?

thanks,

greg k-h

2013-08-06 08:59:37

by Du, ChangbinX

[permalink] [raw]
Subject: RE: About TRB_TO_EP_INDEX() macro using

> On Mon, Aug 05, 2013 at 08:07:25AM +0000, Du, ChangbinX wrote:
> > Recently when I check xHCI code, find that some functions try to get EP index
> > from a Command Completion Event TRB via TRB_TO_EP_INDEX() macro.
> >
> > This is totally wrong. The macro definition is:
> >
> > #define TRB_TO_EP_INDEX(p) ((((p) & (0x1f << 16)) >> 16) - 1)
> >
> > TRB_TO_EP_INDEX only applies to Transfer Event TRB. But for Command
> > Completion Event TRB, there is no Endpoint ID field. So, we cannot directly
> > get EP index from these TRBs, but we can find it by the TRB Pointer.
> >
> > Here list two functions for you to check:
> > handle_stopped_endpoint()
> > handle_reset_ep_completion()
>
> Care to send a patch showing how you would change this if it is
> incorrect?
>
> thanks,
>
> greg k-h
> --

Hello, Greg k-h. I am not very sure about this issue. If this is true, kernel will panic when
invoking above functions. I want someone help to confirm if I miss something. If it's really
a bug, I will work out a patch to fix it.

2013-08-06 09:05:38

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: About TRB_TO_EP_INDEX() macro using

On Tue, Aug 06, 2013 at 08:59:32AM +0000, Du, ChangbinX wrote:
> > On Mon, Aug 05, 2013 at 08:07:25AM +0000, Du, ChangbinX wrote:
> > > Recently when I check xHCI code, find that some functions try to get EP index
> > > from a Command Completion Event TRB via TRB_TO_EP_INDEX() macro.
> > >
> > > This is totally wrong. The macro definition is:
> > >
> > > #define TRB_TO_EP_INDEX(p) ((((p) & (0x1f << 16)) >> 16) - 1)
> > >
> > > TRB_TO_EP_INDEX only applies to Transfer Event TRB. But for Command
> > > Completion Event TRB, there is no Endpoint ID field. So, we cannot directly
> > > get EP index from these TRBs, but we can find it by the TRB Pointer.
> > >
> > > Here list two functions for you to check:
> > > handle_stopped_endpoint()
> > > handle_reset_ep_completion()
> >
> > Care to send a patch showing how you would change this if it is
> > incorrect?
> >
> > thanks,
> >
> > greg k-h
> > --
>
> Hello, Greg k-h. I am not very sure about this issue. If this is true, kernel will panic when
> invoking above functions. I want someone help to confirm if I miss something. If it's really
> a bug, I will work out a patch to fix it.

That's not how kernel development works, sorry. If you think this is an
issue, try to reproduce it and then create a patch.

thanks,

greg k-h