Hi,
John Youn <John.Youn@xxxxxxxxxxxx> writes:
>>> John Youn <John.Youn@xxxxxxxxxxxx> writes:
>>>> On 8/31/2016 2:38 AM, Felipe Balbi wrote:
>>>>> If we don't know what are the actual U1/U2 exit
>>>>> latencies from the UDC, we're better off using
>>>>> maximum latencies as default. This should avoid
>>>>> any problems with too frequent U1/U2 entry.
>>>>>
>>>>> Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
>>>>> ---
>>>>> include/linux/usb/gadget.h | 12 ++++++++++--
>>>>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>>>>> index 3667d667cab1..20cb590c658e 100644
>>>>> --- a/include/linux/usb/gadget.h
>>>>> +++ b/include/linux/usb/gadget.h
>>>>> @@ -276,11 +276,19 @@ static inline void usb_ep_fifo_flush(struct usb_ep *ep)
>>>>>
>>>>> /*-------------------------------------------------------------------------*/
>>>>>
>>>>> +/**
>>>>> + * struct usb_dcd_config_params - Default U1/U2 exit latencies
>>>>> + * @bU1DevExitLat: U1 Exit Latency in us
>>>>> + * @bU2DevExitLat: U2 Exit Latency in us
>>>>> + *
>>>>> + * Note that we will be setting U1/U2 exit latencies to their maximum
>>>>> + * by default if no value is passed by the UDC Driver.
>>>>> + */
>>>>> struct usb_dcd_config_params {
>>>>> __u8 bU1DevExitLat; /* U1 Device exit Latency */
>>>>> -#define USB_DEFAULT_U1_DEV_EXIT_LAT 0x01 /* Less then 1 microsec */
>>>>> +#define USB_DEFAULT_U1_DEV_EXIT_LAT 10 /* us */
>>>>> __le16 wU2DevExitLat; /* U2 Device exit Latency */
>>>>> -#define USB_DEFAULT_U2_DEV_EXIT_LAT 0x1F4 /* Less then 500 microsec */
>>>>> +#define USB_DEFAULT_U2_DEV_EXIT_LAT 2047 /* us */
>>>>> };
>>>>>
>>>>>
>>>>>
>>>>
>>>> Hi Felipe,
>>>>
>>>> Speaking of this, how would you feel about adding module parameters in
>>>> the dwc3-pci to set these? And we also have several more settings in
>>>> our internal tree that we need for IP validation and debugging.
>>>>
>>>> As you know the IP is highly configurable, and we do much of the
>>>> testing against these configurations via software settings. And our
>>>> validation platform is not a typical platform. Basically we have a
>>>> single platform, but the instantiated IP and PHY could be configured
>>>> in any way so it could need different values passed in for
>>>> testing. Like the U1/U2 exit latencies, LPM NYET threshold, HIRD,
>>>> USB2/3 SUSPHY, LPM sleep mode, GFLADJ, etc.
>>>>
>>>> And some things that are automatically detected we need to restrict or
>>>> disable to get full coverage. Such as disabling U1/U2 and LPM, maximum
>>>> speed, dr_mode, NUMP, RX thresholding, RX thresholding packet count.
>>>>
>>>> I know module parameters are typically frowned upon so do you
>>>> recommend another approach?
>>>
>>> I completely understand the situation. Module parameters are, well,
>>> rather unsophisticated. FPGA validation is, however, a 'peculiar' use
>>> case.
>>>
>>> I want to avoid module parameters as much as possible, but apart from
>>> module parameters, the only thing I can think of is a specific
>>> sub-directory on debugfs which only gets compile if EXPERT &&
>>> DWC3_VALIDATION_MODE or whatever.
>>>
>>> Would debugfs work for you? The only problem is for things which get
>>> discovered during probe()
>>
>> Yes that's the problem, otherwise I'd be fine with debugfs. Almost
>> everything we need is initialized on probe. I thought of maybe
>> refactoring the dwc3 code so that this doesn't have to happen on probe
>> and we can trigger a "module reset" or something. But that is not
>> exactly clean either.
>>
>> Regards,
>> John
>>
>
> Does it seem reasonable to add module params to the PCI driver for
> this use case? At least until/if there is some better solution. We can
> guard it with a config option if necessary.
module parameters are user-space ABI. Once added, we can't easily
remove. I've been considering if kprobes could be used to change stuff
like this.
module parameters also feel like a big, big hammer to hit a tiny nail
head. I don't want to add any module parameters for stuff like this. And
since you've been pushing for them for a while, it only shows that
you're only concerned about your use case ;-)
Remember that module parameters are 'global', every instance of
dwc3-pci.ko/dwc3.ko will behave with the same set of module parameters.
Sorry, but I have to say 'no' again. module parameters are not an
option; and I'd strongly suggest you don't do it for dwc2 either because
it'll make maintaining the driver a lot more difficult.
--
balbi
Attachment:
signature.asc
Description: PGP signature
