On 11/15/2016 3:16 AM, Felipe Balbi wrote:
>
> Hi,
>
> John Youn <johny...@synopsys.com> writes:
>> Implement interrupt moderation which allows the interrupt rate to be
>> throttled. To enable this feature the dwc->imod_interval must be set to
>> 1 or greater. This value specifies the minimum inter-interrupt interval,
>> in 250 ns increments. A value of 0 disables interrupt moderation.
>>
>> This applies for DWC_usb3 version 3.00a and higher and for DWC_usb31
>> version 1.20a and higher.
>>
>> Signed-off-by: John Youn <johny...@synopsys.com>
>> ---
>> drivers/usb/dwc3/core.c | 16 ++++++++++++++++
>> drivers/usb/dwc3/core.h | 15 +++++++++++++++
>> drivers/usb/dwc3/gadget.c | 16 ++++++++++++++++
>> 3 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 87d0cfb..889dbab 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -982,12 +982,28 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>> dwc->hird_threshold = hird_threshold
>> | (dwc->is_utmi_l1_suspend << 4);
>>
>> + dwc->imod_interval = 0;
>> +}
>> +
>> +/* check whether the core supports IMOD */
>> +bool dwc3_has_imod(struct dwc3 *dwc)
>> +{
>> + return ((dwc3_is_usb3(dwc) &&
>> + dwc->revision >= DWC3_REVISION_300A) ||
>> + (dwc3_is_usb31(dwc) &&
>> + dwc->revision >= DWC3_USB31_REVISION_120A));
>> }
>>
>> static void dwc3_check_params(struct dwc3 *dwc)
>> {
>> struct device *dev = dwc->dev;
>>
>> + /* Check for proper value of imod_interval */
>> + if (dwc->imod_interval && !dwc3_has_imod(dwc)) {
>> + dev_warn(dwc->dev, "Interrupt moderation not supported\n");
>> + dwc->imod_interval = 0;
>> + }
>> +
>> /* Check the maximum_speed parameter */
>> switch (dwc->maximum_speed) {
>> case USB_SPEED_LOW:
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index bf63756..ef81fa5 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -67,6 +67,7 @@
>> #define DWC3_DEVICE_EVENT_OVERFLOW 11
>>
>> #define DWC3_GEVNTCOUNT_MASK 0xfffc
>> +#define DWC3_GEVNTCOUNT_EHB (1 << 31)
>> #define DWC3_GSNPSID_MASK 0xffff0000
>> #define DWC3_GSNPSREV_MASK 0xffff
>>
>> @@ -149,6 +150,8 @@
>> #define DWC3_DEPCMDPAR0 0x08
>> #define DWC3_DEPCMD 0x0c
>>
>> +#define DWC3_DEV_IMOD(n) (0xca00 + (n * 0x4))
>> +
>> /* OTG Registers */
>> #define DWC3_OCFG 0xcc00
>> #define DWC3_OCTL 0xcc04
>> @@ -465,6 +468,11 @@
>> #define DWC3_DEPCMD_TYPE_BULK 2
>> #define DWC3_DEPCMD_TYPE_INTR 3
>>
>> +#define DWC3_DEV_IMOD_COUNT_SHIFT 16
>> +#define DWC3_DEV_IMOD_COUNT_MASK (0xffff << 16)
>> +#define DWC3_DEV_IMOD_INTERVAL_SHIFT 0
>> +#define DWC3_DEV_IMOD_INTERVAL_MASK (0xffff << 0)
>> +
>> /* Structures */
>>
>> struct dwc3_trb;
>> @@ -846,6 +854,8 @@ struct dwc3_scratchpad_array {
>> * 1 - -3.5dB de-emphasis
>> * 2 - No de-emphasis
>> * 3 - Reserved
>> + * @imod_interval: set the interrupt moderation interval in 250ns
>> + * increments or 0 to disable.
>> */
>> struct dwc3 {
>> struct usb_ctrlrequest *ctrl_req;
>> @@ -933,6 +943,7 @@ struct dwc3 {
>> */
>> #define DWC3_REVISION_IS_DWC31 0x80000000
>> #define DWC3_USB31_REVISION_110A (0x3131302a | DWC3_REVISION_IS_DWC31)
>> +#define DWC3_USB31_REVISION_120A (0x3132302a | DWC3_REVISION_IS_DWC31)
>>
>> enum dwc3_ep0_next ep0_next_event;
>> enum dwc3_ep0_state ep0state;
>> @@ -991,6 +1002,8 @@ struct dwc3 {
>>
>> unsigned tx_de_emphasis_quirk:1;
>> unsigned tx_de_emphasis:2;
>> +
>> + u16 imod_interval;
>> };
>>
>> /*
>> -------------------------------------------------------------------------- */
>> @@ -1162,6 +1175,8 @@ static inline bool dwc3_is_usb31(struct dwc3 *dwc)
>> return !!(dwc->revision & DWC3_REVISION_IS_DWC31);
>> }
>>
>> +bool dwc3_has_imod(struct dwc3 *dwc);
>> +
>> #if IS_ENABLED(CONFIG_USB_DWC3_HOST) ||
>> IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
>> int dwc3_host_init(struct dwc3 *dwc);
>> void dwc3_host_exit(struct dwc3 *dwc);
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index baa2c64..0973167 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -1685,6 +1685,17 @@ static int __dwc3_gadget_start(struct dwc3 *dwc)
>> int ret = 0;
>> u32 reg;
>>
>> + /*
>> + * Use IMOD if enabled via dwc->imod_interval. Otherwise, if
>> + * the core supports IMOD, disable it.
>> + */
>> + if (dwc->imod_interval) {
>> + dwc3_writel(dwc->regs, DWC3_DEV_IMOD(0), dwc->imod_interval);
>> + dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), DWC3_GEVNTCOUNT_EHB);
>
> is this safe? You're doing this after request_threaded_irq(). Sure, IRQs
> are still masked, but couldn't clear events that would get fired as soon
> as we unmask events?
With an IMOD-capable IP, if zero is written to the GEVNTCOUNT.count
field, it has no effect, as long as the whole GEVNTCOUNT is not 0. If
the whole GEVNTCOUNT is zero that means to initialize. So this should
be safe. This is just clearing the EHB in case it is set.
>
>> + } else if (dwc3_has_imod(dwc)) {
>> + dwc3_writel(dwc->regs, DWC3_DEV_IMOD(0), 0);
>> + }
>> +
>> reg = dwc3_readl(dwc->regs, DWC3_DCFG);
>> reg &= ~(DWC3_DCFG_SPEED_MASK);
>>
>> @@ -2847,6 +2858,11 @@ static irqreturn_t dwc3_process_event_buf(struct
>> dwc3_event_buffer *evt)
>> reg &= ~DWC3_GEVNTSIZ_INTMASK;
>> dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0), reg);
>>
>> + if (dwc->imod_interval) {
>> + dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), DWC3_GEVNTCOUNT_EHB);
>
> is this really safe? Aren't you forcefully clearing GEVNTCOUNT back to
> zero here? Why couldn't you do a single write to GEVNTCOUNT?
>
Same as above. This is clearing EHB, telling the core that we're
finished handling the interrupts and it can now reassert the interrupt
line when needed.
> Waiting for you reply before applying. BTW, I've applied patch 1, but
> broke it down into smaller patches. I'll send them to linux-usb shortly.
>
Ok thanks.
Regards,
John
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
