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

[Mathias Nyman]

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>

Thanks
-Mathias