Re: [PATCH v3 01/28] xhci: Add support to allocate several interrupters

[Wesley Cheng]

Hi Mathias,

On 6/26/2023 6:55 AM, Mathias Nyman wrote:
> On 24.6.2023 1.37, Wesley Cheng wrote:
> > Hi Mathias,
> >
> > On 3/13/2023 1:32 PM, Wesley Cheng wrote:
> > > Hi Mathias,
> > >
> > > On 3/10/2023 7:07 AM, Mathias Nyman wrote:
> > > > On 9.3.2023 1.57, Wesley Cheng wrote:
> > > > > From: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
> > > > >
> > > > > Introduce xHCI APIs to allow for clients to allocate and free
> > > > > interrupters.  This allocates an array of interrupters, which is 
> > > > > based on
> > > > > the max_interrupters parameter.  The primary interrupter is set as the
> > > > > first entry in the array, and secondary interrupters following after.
> > > >
> > > > I'm thinking about changing this offloading xHCI API
> > > > xhci should be aware and keep track of which devices and endpoints that
> > > > are offloaded to avoid device getting offloaded twice, avoid xhci 
> > > > driver
> > > > from queuing anything itself for these, and act properly if the 
> > > > offloaded
> > > > device or entire host is removed.
> > > >
> > > > So first thing audio side would need to do do is register/create an
> > > > offload entry for the device using the API:
> > > >
> > > > struct xhci_sideband *xhci_sideband_register(struct usb_device *udev)
> > > >
> > > > (xHCI specs calls offload sideband)
> > > > Then endpoints and interrupters can be added and removed from this
> > > > offload entry
> > > >
> > > > I have some early thoughts written as non-compiling code in:
> > > >
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git 
> > > > feature_interrupters
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters 
> > > >
> > > >
> > > > Let me know what you think about this.
> > > >
> > > > > Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
> > > > > Signed-off-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx>
> > > >
> > > > My Signed-off-by tag is being misused here.
> > > >
> > > > I wrote a chunk of the code in this patch as PoC that I shared in a 
> > > > separate topic branch.
> > > > It was incomplete and not intended for upstream yet. (lacked 
> > > > locking, several fixme parts, etc..)
> > > > The rest of the code in this patch is completely new to me.
> > >
> > > Sorry about this.  I cherry picked the change directly from your 
> > > branch, so it carried your signed off tag with it.  Will make to 
> > > include them properly next time.
> >
> > I'm about ready to submit the next revision for this set of changes, 
> > and I was wondering how we should handle the changes you made on:
> > https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters 
> >
> >
> > I did make some modifications to some of the interrupter fixme tags 
> > you had, and also updated the xhci-sideband APIs with the proper 
> > logic.  I don't believe it is correct for me to submit a set of 
> > patches authored by you without your signed off tag. (checkpatch 
> > throws an error saying the author did not sign off on the change)
>
> Note that the first patch "xhci: split allocate interrupter into 
> separate alloacte and add parts"
> is already in usb-next on its way to 6.5
>
> Maybe Co-developed-by would work in this case, with a small explanation 
> at the end of the commit message.
> Something like:
>
> Locking, DMA something and feataure x added by Wesley Cheng to
> complete original concept code by Mathias
>
> Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
> Co-developed-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx>
> Signed-off-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx>

Sounds good!  Thanks for helping with a non-technical question :).  Just 
wanted to make sure I wasn't overstepping anywhere.

Thanks
Wesley Cheng