Re: [PATCH v3 1/2] usb: dwc3: core: configure TX/RX threshold for DWC3_IP
From: Thinh Nguyen
Date: Tue Sep 12 2023 - 21:07:03 EST
On Tue, Sep 12, 2023, Wesley Cheng wrote:
> Hi,
>
> On 9/11/2023 9:19 PM, Stanley Chang wrote:
> > In Synopsys's dwc3 data book:
> > To avoid underrun and overrun during the burst, in a high-latency bus
> > system (like USB), threshold and burst size control is provided through
> > GTXTHRCFG and GRXTHRCFG registers.
> >
> > In Realtek DHC SoC, DWC3 USB 3.0 uses AHB system bus. When dwc3 is
> > connected with USB 2.5G Ethernet, there will be overrun problem.
> > Therefore, setting TX/RX thresholds can avoid this issue.
> >
> > Signed-off-by: Stanley Chang <stanley_chang@xxxxxxxxxxx>
> > ---
> > v2 to v3 change:
> > Move the threshold setting to new function dwc3_config_threshold.
> > Add the threshold setting for usb31 and usb32
> > v1 to v2 change:
> > Add the properties for TX/RX threshold setting
> > ---
> > drivers/usb/dwc3/core.c | 160 +++++++++++++++++++++++++++++++---------
> > drivers/usb/dwc3/core.h | 13 ++++
> > 2 files changed, 137 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 9c6bf054f15d..44ee8526dc28 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -1057,6 +1057,111 @@ static void dwc3_set_power_down_clk_scale(struct dwc3 *dwc)
> > }
> > }
> > +static void dwc3_config_threshold(struct dwc3 *dwc)
> > +{
> > + u32 reg;
> > + u8 rx_thr_num;
> > + u8 rx_maxburst;
> > + u8 tx_thr_num;
> > + u8 tx_maxburst;
> > +
> > + /*
> > + * Must config both number of packets and max burst settings to enable
> > + * RX and/or TX threshold.
> > + */
> > + if (!DWC3_IP_IS(DWC3) && dwc->dr_mode == USB_DR_MODE_HOST) {
> > + rx_thr_num = dwc->rx_thr_num_pkt_prd;
> > + rx_maxburst = dwc->rx_max_burst_prd;
> > + tx_thr_num = dwc->tx_thr_num_pkt_prd;
> > + tx_maxburst = dwc->tx_max_burst_prd;
> > +
> > + if (rx_thr_num && rx_maxburst) {
> > + reg = dwc3_readl(dwc->regs, DWC3_GRXTHRCFG);
> > + reg |= DWC31_RXTHRNUMPKTSEL_PRD;
> > +
> > + reg &= ~DWC31_RXTHRNUMPKT_PRD(~0);
> > + reg |= DWC31_RXTHRNUMPKT_PRD(rx_thr_num);
> > +
> > + reg &= ~DWC31_MAXRXBURSTSIZE_PRD(~0);
> > + reg |= DWC31_MAXRXBURSTSIZE_PRD(rx_maxburst);
> > +
> > + dwc3_writel(dwc->regs, DWC3_GRXTHRCFG, reg);
> > + }
> > +
> > + if (tx_thr_num && tx_maxburst) {
> > + reg = dwc3_readl(dwc->regs, DWC3_GTXTHRCFG);
> > + reg |= DWC31_TXTHRNUMPKTSEL_PRD;
> > +
> > + reg &= ~DWC31_TXTHRNUMPKT_PRD(~0);
> > + reg |= DWC31_TXTHRNUMPKT_PRD(tx_thr_num);
> > +
> > + reg &= ~DWC31_MAXTXBURSTSIZE_PRD(~0);
> > + reg |= DWC31_MAXTXBURSTSIZE_PRD(tx_maxburst);
> > +
> > + dwc3_writel(dwc->regs, DWC3_GTXTHRCFG, reg);
> > + }
> > + }
> > +
> > + rx_thr_num = dwc->rx_thr_num_pkt;
> > + rx_maxburst = dwc->rx_max_burst;
> > + tx_thr_num = dwc->tx_thr_num_pkt;
> > + tx_maxburst = dwc->tx_max_burst;
> > +
> > + if (DWC3_IP_IS(DWC3)) {
> > + if (rx_thr_num && rx_maxburst) {
> > + reg = dwc3_readl(dwc->regs, DWC3_GRXTHRCFG);
> > + reg |= DWC3_GRXTHRCFG_PKTCNTSEL;
> > +
> > + reg &= ~DWC3_GRXTHRCFG_RXPKTCNT(~0);
> > + reg |= DWC3_GRXTHRCFG_RXPKTCNT(rx_thr_num);
> > +
> > + reg &= ~DWC3_GRXTHRCFG_MAXRXBURSTSIZE(~0);
> > + reg |= DWC3_GRXTHRCFG_MAXRXBURSTSIZE(rx_maxburst);
> > +
> > + dwc3_writel(dwc->regs, DWC3_GRXTHRCFG, reg);
> > + }
> > +
> > + if (tx_thr_num && tx_maxburst) {
> > + reg = dwc3_readl(dwc->regs, DWC3_GTXTHRCFG);
> > + reg |= DWC3_GTXTHRCFG_PKTCNTSEL;
> > +
> > + reg &= ~DWC3_GTXTHRCFG_TXPKTCNT(~0);
> > + reg |= DWC3_GTXTHRCFG_TXPKTCNT(tx_thr_num);
> > +
> > + reg &= ~DWC3_GTXTHRCFG_MAXTXBURSTSIZE(~0);
> > + reg |= DWC3_GTXTHRCFG_MAXTXBURSTSIZE(tx_maxburst);
> > +
> > + dwc3_writel(dwc->regs, DWC3_GTXTHRCFG, reg);
> > + }
> > + } else {
> > + if (rx_thr_num && rx_maxburst) {
> > + reg = dwc3_readl(dwc->regs, DWC3_GRXTHRCFG);
> > + reg |= DWC31_GRXTHRCFG_PKTCNTSEL;
> > +
> > + reg &= ~DWC31_GRXTHRCFG_RXPKTCNT(~0);
> > + reg |= DWC31_GRXTHRCFG_RXPKTCNT(rx_thr_num);
> > +
> > + reg &= ~DWC31_GRXTHRCFG_MAXRXBURSTSIZE(~0);
> > + reg |= DWC31_GRXTHRCFG_MAXRXBURSTSIZE(rx_maxburst);
> > +
> > + dwc3_writel(dwc->regs, DWC3_GRXTHRCFG, reg);
> > + }
> > +
> > + if (tx_thr_num && tx_maxburst) {
> > + reg = dwc3_readl(dwc->regs, DWC3_GTXTHRCFG);
> > + reg |= DWC31_GTXTHRCFG_PKTCNTSEL;
> > +
> > + reg &= ~DWC31_GTXTHRCFG_TXPKTCNT(~0);
> > + reg |= DWC31_GTXTHRCFG_TXPKTCNT(tx_thr_num);
> > +
> > + reg &= ~DWC31_GTXTHRCFG_MAXTXBURSTSIZE(~0);
> > + reg |= DWC31_GTXTHRCFG_MAXTXBURSTSIZE(tx_maxburst);
> > +
> > + dwc3_writel(dwc->regs, DWC3_GTXTHRCFG, reg);
> > + }
> > + }
> > +}
> > +
> > /**
> > * dwc3_core_init - Low-level initialization of DWC3 Core
> > * @dwc: Pointer to our controller context structure
> > @@ -1209,42 +1314,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
> > dwc3_writel(dwc->regs, DWC3_GUCTL1, reg);
> > }
> > - /*
> > - * Must config both number of packets and max burst settings to enable
> > - * RX and/or TX threshold.
> > - */
> > - if (!DWC3_IP_IS(DWC3) && dwc->dr_mode == USB_DR_MODE_HOST) {
> > - u8 rx_thr_num = dwc->rx_thr_num_pkt_prd;
> > - u8 rx_maxburst = dwc->rx_max_burst_prd;
> > - u8 tx_thr_num = dwc->tx_thr_num_pkt_prd;
> > - u8 tx_maxburst = dwc->tx_max_burst_prd;
> > -
> > - if (rx_thr_num && rx_maxburst) {
> > - reg = dwc3_readl(dwc->regs, DWC3_GRXTHRCFG);
> > - reg |= DWC31_RXTHRNUMPKTSEL_PRD;
> > -
> > - reg &= ~DWC31_RXTHRNUMPKT_PRD(~0);
> > - reg |= DWC31_RXTHRNUMPKT_PRD(rx_thr_num);
> > -
> > - reg &= ~DWC31_MAXRXBURSTSIZE_PRD(~0);
> > - reg |= DWC31_MAXRXBURSTSIZE_PRD(rx_maxburst);
> > -
> > - dwc3_writel(dwc->regs, DWC3_GRXTHRCFG, reg);
> > - }
> > -
> > - if (tx_thr_num && tx_maxburst) {
> > - reg = dwc3_readl(dwc->regs, DWC3_GTXTHRCFG);
> > - reg |= DWC31_TXTHRNUMPKTSEL_PRD;
> > -
> > - reg &= ~DWC31_TXTHRNUMPKT_PRD(~0);
> > - reg |= DWC31_TXTHRNUMPKT_PRD(tx_thr_num);
> > -
> > - reg &= ~DWC31_MAXTXBURSTSIZE_PRD(~0);
> > - reg |= DWC31_MAXTXBURSTSIZE_PRD(tx_maxburst);
> > -
> > - dwc3_writel(dwc->regs, DWC3_GTXTHRCFG, reg);
> > - }
> > - }
> > + dwc3_config_threshold(dwc);
> > return 0;
> > @@ -1380,6 +1450,10 @@ static void dwc3_get_properties(struct dwc3 *dwc)
> > u8 lpm_nyet_threshold;
> > u8 tx_de_emphasis;
> > u8 hird_threshold;
> > + u8 rx_thr_num_pkt = 0;
> > + u8 rx_max_burst = 0;
> > + u8 tx_thr_num_pkt = 0;
> > + u8 tx_max_burst = 0;
> > u8 rx_thr_num_pkt_prd = 0;
> > u8 rx_max_burst_prd = 0;
> > u8 tx_thr_num_pkt_prd = 0;
> > @@ -1442,6 +1516,14 @@ static void dwc3_get_properties(struct dwc3 *dwc)
> > "snps,usb2-lpm-disable");
> > dwc->usb2_gadget_lpm_disable = device_property_read_bool(dev,
> > "snps,usb2-gadget-lpm-disable");
> > + device_property_read_u8(dev, "snps,rx-thr-num-pkt",
> > + &rx_thr_num_pkt);
> > + device_property_read_u8(dev, "snps,rx-max-burst",
> > + &rx_max_burst);
> > + device_property_read_u8(dev, "snps,tx-thr-num-pkt",
> > + &tx_thr_num_pkt);
> > + device_property_read_u8(dev, "snps,tx-max-burst",
> > + &tx_max_burst);
>
> Aren't there value limitations for some of these (if not all) settings? For
> example, in the DWC3 programming guide, the max burst fields say (for
> DWC31):
> "Note: This field can only be set to 2, 4, 8 or 16."
>
> And for the packet threshold count:
> "Valid values are from 1 to 16."
>
Stanley documented this in the dt binding. I think what he had written
there is fine.
Thanks,
Thinh