| Message ID | 20230828055212.5600-1-stanley_chang@realtek.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,RESEND,1/2] usb: dwc3: core: configure TX/RX threshold for DWC3_IP | expand |
Hi Thinh, Can you help review this patch? Thanks, Stanley > -----Original Message----- > From: Stanley Chang <stanley_chang@realtek.com> > Sent: Monday, August 28, 2023 1:52 PM > To: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > Cc: Stanley Chang[昌育德] <stanley_chang@realtek.com>; Greg > Kroah-Hartman <gregkh@linuxfoundation.org>; Rob Herring > <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > Felipe Balbi <balbi@kernel.org>; linux-usb@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [PATCH v2 RESEND 1/2] usb: dwc3: core: configure TX/RX threshold for > DWC3_IP > > 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@realtek.com> > --- > v1 to v2 change: > Add the properties for TX/RX threshold setting > --- > drivers/usb/dwc3/core.c | 51 > +++++++++++++++++++++++++++++++++++++++++ > drivers/usb/dwc3/core.h | 13 +++++++++++ > 2 files changed, 64 insertions(+) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index > 9c6bf054f15d..1f74a53816c3 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -1246,6 +1246,39 @@ static int dwc3_core_init(struct dwc3 *dwc) > } > } > > + if (DWC3_IP_IS(DWC3)) { > + u8 rx_thr_num = dwc->rx_thr_num_pkt; > + u8 rx_maxburst = dwc->rx_max_burst; > + u8 tx_thr_num = dwc->tx_thr_num_pkt; > + u8 tx_maxburst = dwc->tx_max_burst; > + > + 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); > + } > + } > + > return 0; > > err_power_off_phy: > @@ -1380,6 +1413,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 +1479,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); > device_property_read_u8(dev, "snps,rx-thr-num-pkt-prd", > &rx_thr_num_pkt_prd); > device_property_read_u8(dev, "snps,rx-max-burst-prd", @@ -1523,6 > +1568,12 @@ static void dwc3_get_properties(struct dwc3 *dwc) > > dwc->hird_threshold = hird_threshold; > > + dwc->rx_thr_num_pkt = rx_thr_num_pkt; > + dwc->rx_max_burst = rx_max_burst; > + > + dwc->tx_thr_num_pkt = tx_thr_num_pkt; > + dwc->tx_max_burst = tx_max_burst; > + > dwc->rx_thr_num_pkt_prd = rx_thr_num_pkt_prd; > dwc->rx_max_burst_prd = rx_max_burst_prd; > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index > a69ac67d89fe..6782ec8bfd64 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -211,6 +211,11 @@ > #define DWC3_GRXTHRCFG_RXPKTCNT(n) (((n) & 0xf) << 24) #define > DWC3_GRXTHRCFG_PKTCNTSEL BIT(29) > > +/* Global TX Threshold Configuration Register */ #define > +DWC3_GTXTHRCFG_MAXTXBURSTSIZE(n) (((n) & 0xff) << 16) #define > +DWC3_GTXTHRCFG_TXPKTCNT(n) (((n) & 0xf) << 24) #define > +DWC3_GTXTHRCFG_PKTCNTSEL BIT(29) > + > /* Global RX Threshold Configuration Register for DWC_usb31 only */ > #define DWC31_GRXTHRCFG_MAXRXBURSTSIZE(n) (((n) & 0x1f) << 16) > #define DWC31_GRXTHRCFG_RXPKTCNT(n) (((n) & 0x1f) << 21) > @@ -1045,6 +1050,10 @@ struct dwc3_scratchpad_array { > * @test_mode_nr: test feature selector > * @lpm_nyet_threshold: LPM NYET response threshold > * @hird_threshold: HIRD threshold > + * @rx_thr_num_pkt: USB receive packet count > + * @rx_max_burst: max USB receive burst size > + * @tx_thr_num_pkt: USB transmit packet count > + * @tx_max_burst: max USB transmit burst size > * @rx_thr_num_pkt_prd: periodic ESS receive packet count > * @rx_max_burst_prd: max periodic ESS receive burst size > * @tx_thr_num_pkt_prd: periodic ESS transmit packet count @@ -1273,6 > +1282,10 @@ struct dwc3 { > u8 test_mode_nr; > u8 lpm_nyet_threshold; > u8 hird_threshold; > + u8 rx_thr_num_pkt; > + u8 rx_max_burst; > + u8 tx_thr_num_pkt; > + u8 tx_max_burst; > u8 rx_thr_num_pkt_prd; > u8 rx_max_burst_prd; > u8 tx_thr_num_pkt_prd; > -- > 2.34.1
Hi Thinh, > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index > > 9c6bf054f15d..1f74a53816c3 100644 > > --- a/drivers/usb/dwc3/core.c > > +++ b/drivers/usb/dwc3/core.c > > @@ -1246,6 +1246,39 @@ static int dwc3_core_init(struct dwc3 *dwc) > > } > > } > > > > + if (DWC3_IP_IS(DWC3)) { > > Would you mind also add the checks for DWC_usb31 and DWC_usb32? Both > the > DWC_usb31 and DWC_usb32 share the same field offsets within > GTX/RXTHRCFG registers. The macros are already defined for those IPs. DWC3 and DWC31, DWC32 seem to have different register definition as follows. /* Global RX Threshold Configuration Register */ #define DWC3_GRXTHRCFG_MAXRXBURSTSIZE(n) (((n) & 0x1f) << 19) #define DWC3_GRXTHRCFG_RXPKTCNT(n) (((n) & 0xf) << 24) #define DWC3_GRXTHRCFG_PKTCNTSEL BIT(29) /* Global TX Threshold Configuration Register */ #define DWC3_GTXTHRCFG_MAXTXBURSTSIZE(n) (((n) & 0xff) << 16) #define DWC3_GTXTHRCFG_TXPKTCNT(n) (((n) & 0xf) << 24) #define DWC3_GTXTHRCFG_PKTCNTSEL BIT(29) /* Global RX Threshold Configuration Register for DWC_usb31 only */ #define DWC31_GRXTHRCFG_MAXRXBURSTSIZE(n) (((n) & 0x1f) << 16) #define DWC31_GRXTHRCFG_RXPKTCNT(n) (((n) & 0x1f) << 21) #define DWC31_GRXTHRCFG_PKTCNTSEL BIT(26) #define DWC31_RXTHRNUMPKTSEL_HS_PRD BIT(15) #define DWC31_RXTHRNUMPKT_HS_PRD(n) (((n) & 0x3) << 13) #define DWC31_RXTHRNUMPKTSEL_PRD BIT(10) #define DWC31_RXTHRNUMPKT_PRD(n) (((n) & 0x1f) << 5) #define DWC31_MAXRXBURSTSIZE_PRD(n) ((n) & 0x1f) /* Global TX Threshold Configuration Register for DWC_usb31 only */ #define DWC31_GTXTHRCFG_MAXTXBURSTSIZE(n) (((n) & 0x1f) << 16) #define DWC31_GTXTHRCFG_TXPKTCNT(n) (((n) & 0x1f) << 21) #define DWC31_GTXTHRCFG_PKTCNTSEL BIT(26) #define DWC31_TXTHRNUMPKTSEL_HS_PRD BIT(15) #define DWC31_TXTHRNUMPKT_HS_PRD(n) (((n) & 0x3) << 13) #define DWC31_TXTHRNUMPKTSEL_PRD BIT(10) #define DWC31_TXTHRNUMPKT_PRD(n) (((n) & 0x1f) << 5) #define DWC31_MAXTXBURSTSIZE_PRD(n) ((n) & 0x1f) > Also, I think this new addition will make dwc3_core_init() lengthy. Can we split > this logic to a separate function, perhaps dwc3_config_threshold()? > Ok, I will move this logic to a new function. Thanks, Stanley
On Tue, Sep 12, 2023, Stanley Chang[昌育德] wrote: > Hi Thinh, > > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index > > > 9c6bf054f15d..1f74a53816c3 100644 > > > --- a/drivers/usb/dwc3/core.c > > > +++ b/drivers/usb/dwc3/core.c > > > @@ -1246,6 +1246,39 @@ static int dwc3_core_init(struct dwc3 *dwc) > > > } > > > } > > > > > > + if (DWC3_IP_IS(DWC3)) { > > > > Would you mind also add the checks for DWC_usb31 and DWC_usb32? Both > > the > > DWC_usb31 and DWC_usb32 share the same field offsets within > > GTX/RXTHRCFG registers. The macros are already defined for those IPs. > > DWC3 and DWC31, DWC32 seem to have different register definition as follows. Yes. That's what I meant. They are already define in the core.h for DWC_usb31. DWC_usb32 also shares the same offsets as DWC_usb31 for this. Can you also include the setting of GTX/RXTHRCFG logic for those 2 IPs? Thanks, Thinh
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 9c6bf054f15d..1f74a53816c3 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1246,6 +1246,39 @@ static int dwc3_core_init(struct dwc3 *dwc) } } + if (DWC3_IP_IS(DWC3)) { + u8 rx_thr_num = dwc->rx_thr_num_pkt; + u8 rx_maxburst = dwc->rx_max_burst; + u8 tx_thr_num = dwc->tx_thr_num_pkt; + u8 tx_maxburst = dwc->tx_max_burst; + + 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); + } + } + return 0; err_power_off_phy: @@ -1380,6 +1413,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 +1479,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); device_property_read_u8(dev, "snps,rx-thr-num-pkt-prd", &rx_thr_num_pkt_prd); device_property_read_u8(dev, "snps,rx-max-burst-prd", @@ -1523,6 +1568,12 @@ static void dwc3_get_properties(struct dwc3 *dwc) dwc->hird_threshold = hird_threshold; + dwc->rx_thr_num_pkt = rx_thr_num_pkt; + dwc->rx_max_burst = rx_max_burst; + + dwc->tx_thr_num_pkt = tx_thr_num_pkt; + dwc->tx_max_burst = tx_max_burst; + dwc->rx_thr_num_pkt_prd = rx_thr_num_pkt_prd; dwc->rx_max_burst_prd = rx_max_burst_prd; diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index a69ac67d89fe..6782ec8bfd64 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -211,6 +211,11 @@ #define DWC3_GRXTHRCFG_RXPKTCNT(n) (((n) & 0xf) << 24) #define DWC3_GRXTHRCFG_PKTCNTSEL BIT(29) +/* Global TX Threshold Configuration Register */ +#define DWC3_GTXTHRCFG_MAXTXBURSTSIZE(n) (((n) & 0xff) << 16) +#define DWC3_GTXTHRCFG_TXPKTCNT(n) (((n) & 0xf) << 24) +#define DWC3_GTXTHRCFG_PKTCNTSEL BIT(29) + /* Global RX Threshold Configuration Register for DWC_usb31 only */ #define DWC31_GRXTHRCFG_MAXRXBURSTSIZE(n) (((n) & 0x1f) << 16) #define DWC31_GRXTHRCFG_RXPKTCNT(n) (((n) & 0x1f) << 21) @@ -1045,6 +1050,10 @@ struct dwc3_scratchpad_array { * @test_mode_nr: test feature selector * @lpm_nyet_threshold: LPM NYET response threshold * @hird_threshold: HIRD threshold + * @rx_thr_num_pkt: USB receive packet count + * @rx_max_burst: max USB receive burst size + * @tx_thr_num_pkt: USB transmit packet count + * @tx_max_burst: max USB transmit burst size * @rx_thr_num_pkt_prd: periodic ESS receive packet count * @rx_max_burst_prd: max periodic ESS receive burst size * @tx_thr_num_pkt_prd: periodic ESS transmit packet count @@ -1273,6 +1282,10 @@ struct dwc3 { u8 test_mode_nr; u8 lpm_nyet_threshold; u8 hird_threshold; + u8 rx_thr_num_pkt; + u8 rx_max_burst; + u8 tx_thr_num_pkt; + u8 tx_max_burst; u8 rx_thr_num_pkt_prd; u8 rx_max_burst_prd; u8 tx_thr_num_pkt_prd;
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@realtek.com> --- v1 to v2 change: Add the properties for TX/RX threshold setting --- drivers/usb/dwc3/core.c | 51 +++++++++++++++++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 13 +++++++++++ 2 files changed, 64 insertions(+)