Re: [PATCH v1 1/4] usb: host: export symbols for xhci hooks usage

Krzysztof Kozlowski
2022-03-25T11:36:41Z

On 25/03/2022 02:28, Jung Daehwan wrote:
> On Wed, Mar 23, 2022 at 10:41:23AM +0100, Krzysztof Kozlowski wrote:
>> On 23/03/2022 03:58, Jung Daehwan wrote:
>>> On Mon, Mar 07, 2022 at 10:59:06AM +0100, Krzysztof Kozlowski wrote:
>>>> On 04/03/2022 07:23, Daehwan Jung wrote:
>>>>> Export symbols for xhci hooks usage:
>>>>> xhci_ring_free
>>>>> - Allow xhci hook to free xhci_ring.
>>>>
>>>> Instead of copying-pasting the name of function, please explain why do
>>>> you need these symbols exported.
>>>>
>>>> The "Why" is actually one of most important questions, because "what is
>>>> this patch doing" we can easily see...
>>>>
>>>>>
>>>>> xhci_get_slot_ctx
>>>>> - Allow xhci hook to get slot_ctx from the xhci_container_ctx
>>>>> for getting the slot_ctx information to know which slot is
>>>>> offloading and compare the context in remote subsystem memory
>>>>> if needed.
>>>>>
>>>>
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>>
>>> Hi Krzysztof
>>>
>>> xhci_ring_free has been removed from v3..
>>> The reason why I want to export is for managing vendor specific ring.
>>> I want to alloc and free vendor specific ring on specific address.
>>> It's done with xhci hooks.
>>
>> It's better, but still does not explain why these have to be exported.
>> Please mention where are these hooks going to be. Where are they
>> implemented. I actually expect all of these exports to be used in your
>> patchset.
>>
>> Best regards,
>> Krzysztof
>>
>
> OK. How about adding call stack like below?
>
> xhci_free_endpoint_ring -> xhci_vendor_free_transfer_ring(xhck hooks
> ops) -> xhci_ring_free

What I would like to see is high level explanation, why do you need
these functions exported. Such call trace does not answer this, because
you do not need to export functions just to call the. You need to export
them for modules, so this means that some modules (which - need names)
will be using these functions. Three or four sentences are usually
enough to explain it.

Best regards,
Krzysztof