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

From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Wesley Cheng <quic_wcheng@quicinc.com>,
	srinivas.kandagatla@linaro.org, mathias.nyman@intel.com,
	perex@perex.cz, broonie@kernel.org, lgirdwood@gmail.com,
	krzysztof.kozlowski+dt@linaro.org, agross@kernel.org,
	Thinh.Nguyen@synopsys.com, bgoswami@quicinc.com,
	andersson@kernel.org, robh+dt@kernel.org,
	gregkh@linuxfoundation.org, tiwai@suse.com
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	alsa-devel@alsa-project.org, devicetree@vger.kernel.org,
	linux-usb@vger.kernel.org, quic_jackp@quicinc.com,
	quic_plai@quicinc.com
Subject: Re: [PATCH v3 01/28] xhci: Add support to allocate several interrupters
Date: Mon, 26 Jun 2023 16:55:46 +0300	[thread overview]
Message-ID: <da468fe6-709c-b6e6-159d-10f76d296307@linux.intel.com> (raw)
In-Reply-To: <9f30e9f9-280e-b381-fecc-2a032c1117af@quicinc.com>

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@linux.intel.com>
>>>>
>>>> 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@linux.intel.com>
>>>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>>>
>>> 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@linux.intel.com>
Co-developed-by: Wesley Cheng <quic_wcheng@quicinc.com>
Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>

Thanks
-Mathias