Hi Marek, There are some platforms' USB functionality that are stuck due to the lack of the reset method introduced in dwc2 v4.20a, such as Canaan K230 SoC and Sophgo CV1800 SoC.
Is there anything else I need to do to get this patch and previous patch series taken? Thanks! On 26/01/2025 00:29, Junhui Liu wrote: > Refactor DWC2 USB gadget driver to replace manual read-modify-write > operations with `clrsetbits_le32`, `setbits_le32`, and `clrbits_le32` > macros, which simplify the code and improve readability. > > Signed-off-by: Junhui Liu <junhui....@pigmoral.tech> > --- > This patch is a supplement of patch series [1]. > > [1] > https://lore.kernel.org/u-boot/20250110-dwc2-dev-v4-0-987f4fd6f...@pigmoral.tech > --- > drivers/usb/gadget/dwc2_udc_otg.c | 16 ++------ > drivers/usb/gadget/dwc2_udc_otg_phy.c | 33 ++++++---------- > drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 63 > +++++++----------------------- > 3 files changed, 31 insertions(+), 81 deletions(-) > > diff --git a/drivers/usb/gadget/dwc2_udc_otg.c > b/drivers/usb/gadget/dwc2_udc_otg.c > index > c3fdc81e9096bb216b63ff0ac672d216bed3f23d..40393141ca95b8947712a8996727391fe8742275 > 100644 > --- a/drivers/usb/gadget/dwc2_udc_otg.c > +++ b/drivers/usb/gadget/dwc2_udc_otg.c > @@ -465,7 +465,6 @@ static void reconfig_usbd(struct dwc2_udc *dev) > { > /* 2. Soft-reset OTG Core and then unreset again. */ > int i; > - unsigned int uTemp; > u32 dflt_gusbcfg; > u32 rx_fifo_sz, tx_fifo_sz, np_tx_fifo_sz; > u32 max_hw_ep; > @@ -497,16 +496,12 @@ static void reconfig_usbd(struct dwc2_udc *dev) > writel(dflt_gusbcfg, ®->global_regs.gusbcfg); > > /* 3. Put the OTG device core in the disconnected state.*/ > - uTemp = readl(®->device_regs.dctl); > - uTemp |= DCTL_SFTDISCON; > - writel(uTemp, ®->device_regs.dctl); > + setbits_le32(®->device_regs.dctl, DCTL_SFTDISCON); > > udelay(20); > > /* 4. Make the OTG device core exit from the disconnected state.*/ > - uTemp = readl(®->device_regs.dctl); > - uTemp = uTemp & ~DCTL_SFTDISCON; > - writel(uTemp, ®->device_regs.dctl); > + clrbits_le32(®->device_regs.dctl, DCTL_SFTDISCON); > > /* 5. Configure OTG Core to initial settings of device mode.*/ > /* [][1: full speed(30Mhz) 0:high speed]*/ > @@ -592,7 +587,6 @@ static void reconfig_usbd(struct dwc2_udc *dev) > > static void set_max_pktsize(struct dwc2_udc *dev, enum usb_device_speed > speed) > { > - unsigned int ep_ctrl; > int i; > > if (speed == USB_SPEED_HIGH) { > @@ -612,12 +606,10 @@ static void set_max_pktsize(struct dwc2_udc *dev, enum > usb_device_speed speed) > dev->ep[i].ep.maxpacket = ep_fifo_size; > > /* EP0 - Control IN (64 bytes)*/ > - ep_ctrl = readl(®->device_regs.in_endp[EP0_CON].diepctl); > - writel(ep_ctrl | (0 << 0), ®->device_regs.in_endp[EP0_CON].diepctl); > + setbits_le32(®->device_regs.in_endp[EP0_CON].diepctl, (0 << 0)); > > /* EP0 - Control OUT (64 bytes)*/ > - ep_ctrl = readl(®->device_regs.out_endp[EP0_CON].doepctl); > - writel(ep_ctrl | (0 << 0), ®->device_regs.out_endp[EP0_CON].doepctl); > + setbits_le32(®->device_regs.out_endp[EP0_CON].doepctl, (0 << 0)); > } > > static int dwc2_ep_enable(struct usb_ep *_ep, > diff --git a/drivers/usb/gadget/dwc2_udc_otg_phy.c > b/drivers/usb/gadget/dwc2_udc_otg_phy.c > index > c7eea7b34421ad9bde86d42334852d2f21a133e8..e0ac5d142b0610461408754a59bfd2aa09848407 > 100644 > --- a/drivers/usb/gadget/dwc2_udc_otg_phy.c > +++ b/drivers/usb/gadget/dwc2_udc_otg_phy.c > @@ -48,29 +48,24 @@ void otg_phy_init(struct dwc2_udc *dev) > printf("USB PHY0 Enable\n"); > > /* Enable PHY */ > - writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, usb_phy_ctrl); > + setbits_le32(usb_phy_ctrl, USB_PHY_CTRL_EN0); > > if (dev->pdata->usb_flags == PHY0_SLEEP) /* C210 Universal */ > - writel((readl(&phy->phypwr) > - &~(PHY_0_SLEEP | OTG_DISABLE_0 | ANALOG_PWRDOWN) > - &~FORCE_SUSPEND_0), &phy->phypwr); > + clrbits_le32(&phy->phypwr, PHY_0_SLEEP | OTG_DISABLE_0 | > + ANALOG_PWRDOWN | FORCE_SUSPEND_0); > else /* C110 GONI */ > - writel((readl(&phy->phypwr) &~(OTG_DISABLE_0 | ANALOG_PWRDOWN) > - &~FORCE_SUSPEND_0), &phy->phypwr); > + clrbits_le32(&phy->phypwr, OTG_DISABLE_0 | ANALOG_PWRDOWN | > FORCE_SUSPEND_0); > > if (s5p_cpu_id == 0x4412) > - writel((readl(&phy->phyclk) & ~(EXYNOS4X12_ID_PULLUP0 | > - EXYNOS4X12_COMMON_ON_N0)) | EXYNOS4X12_CLK_SEL_24MHZ, > - &phy->phyclk); /* PLL 24Mhz */ > + clrsetbits_le32(&phy->phyclk, EXYNOS4X12_ID_PULLUP0 | > EXYNOS4X12_COMMON_ON_N0, > + EXYNOS4X12_CLK_SEL_24MHZ); /* PLL 24Mhz */ > else > - writel((readl(&phy->phyclk) & ~(ID_PULLUP0 | COMMON_ON_N0)) | > - CLK_SEL_24MHZ, &phy->phyclk); /* PLL 24Mhz */ > + clrsetbits_le32(&phy->phyclk, ID_PULLUP0 | COMMON_ON_N0, > + CLK_SEL_24MHZ); /* PLL 24Mhz */ > > - writel((readl(&phy->rstcon) &~(LINK_SW_RST | PHYLNK_SW_RST)) > - | PHY_SW_RST0, &phy->rstcon); > + clrsetbits_le32(&phy->rstcon, LINK_SW_RST | PHYLNK_SW_RST, PHY_SW_RST0); > udelay(10); > - writel(readl(&phy->rstcon) > - &~(PHY_SW_RST0 | LINK_SW_RST | PHYLNK_SW_RST), &phy->rstcon); > + clrbits_le32(&phy->rstcon, PHY_SW_RST0 | LINK_SW_RST | PHYLNK_SW_RST); > udelay(10); > } > > @@ -86,13 +81,11 @@ void otg_phy_off(struct dwc2_udc *dev) > writel(readl(&phy->phypwr) &~PHY_SW_RST0, &phy->rstcon); > udelay(20); > > - writel(readl(&phy->phypwr) | OTG_DISABLE_0 | ANALOG_PWRDOWN > - | FORCE_SUSPEND_0, &phy->phypwr); > + setbits_le32(&phy->phypwr, OTG_DISABLE_0 | ANALOG_PWRDOWN | > FORCE_SUSPEND_0); > > - writel(readl(usb_phy_ctrl) &~USB_PHY_CTRL_EN0, usb_phy_ctrl); > + clrbits_le32(usb_phy_ctrl, USB_PHY_CTRL_EN0); > > - writel((readl(&phy->phyclk) & ~(ID_PULLUP0 | COMMON_ON_N0)), > - &phy->phyclk); > + clrbits_le32(&phy->phyclk, ID_PULLUP0 | COMMON_ON_N0); > > udelay(10000); > > diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > index > 2be93592c423df7a9acea473b0e84e1f948999be..fca052b4556a7d2ae4fe516e39820611d7082e2f > 100644 > --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > @@ -31,15 +31,11 @@ int clear_feature_flag; > > static inline void dwc2_udc_ep0_zlp(struct dwc2_udc *dev) > { > - u32 ep_ctrl; > - > writel(phys_to_bus((unsigned long)usb_ctrl_dma_addr), > ®->device_regs.in_endp[EP0_CON].diepdma); > writel(FIELD_PREP(DXEPTSIZ_PKTCNT_MASK, 1), > ®->device_regs.in_endp[EP0_CON].dieptsiz); > > - ep_ctrl = readl(®->device_regs.in_endp[EP0_CON].diepctl); > - writel(ep_ctrl | DXEPCTL_EPENA | DXEPCTL_CNAK, > - ®->device_regs.in_endp[EP0_CON].diepctl); > + setbits_le32(®->device_regs.in_endp[EP0_CON].diepctl, DXEPCTL_EPENA > | DXEPCTL_CNAK); > > debug_cond(DEBUG_EP0 != 0, "%s:EP0 ZLP DIEPCTL0 = 0x%x\n", > __func__, readl(®->device_regs.in_endp[EP0_CON].diepctl)); > @@ -48,8 +44,6 @@ static inline void dwc2_udc_ep0_zlp(struct dwc2_udc *dev) > > static void dwc2_udc_pre_setup(void) > { > - u32 ep_ctrl; > - > debug_cond(DEBUG_IN_EP, > "%s : Prepare Setup packets.\n", __func__); > > @@ -58,20 +52,16 @@ static void dwc2_udc_pre_setup(void) > writel(phys_to_bus((unsigned long)usb_ctrl_dma_addr), > ®->device_regs.out_endp[EP0_CON].doepdma); > > - ep_ctrl = readl(®->device_regs.out_endp[EP0_CON].doepctl); > - writel(ep_ctrl | DXEPCTL_EPENA, > ®->device_regs.out_endp[EP0_CON].doepctl); > + setbits_le32(®->device_regs.out_endp[EP0_CON].doepctl, > DXEPCTL_EPENA); > > debug_cond(DEBUG_EP0 != 0, "%s:EP0 ZLP DIEPCTL0 = 0x%x\n", > __func__, readl(®->device_regs.in_endp[EP0_CON].diepctl)); > debug_cond(DEBUG_EP0 != 0, "%s:EP0 ZLP DOEPCTL0 = 0x%x\n", > __func__, readl(®->device_regs.out_endp[EP0_CON].doepctl)); > - > } > > static inline void dwc2_ep0_complete_out(void) > { > - u32 ep_ctrl; > - > debug_cond(DEBUG_EP0 != 0, "%s:EP0 ZLP DIEPCTL0 = 0x%x\n", > __func__, readl(®->device_regs.in_endp[EP0_CON].diepctl)); > debug_cond(DEBUG_EP0 != 0, "%s:EP0 ZLP DOEPCTL0 = 0x%x\n", > @@ -85,15 +75,12 @@ static inline void dwc2_ep0_complete_out(void) > writel(phys_to_bus((unsigned long)usb_ctrl_dma_addr), > ®->device_regs.out_endp[EP0_CON].doepdma); > > - ep_ctrl = readl(®->device_regs.out_endp[EP0_CON].doepctl); > - writel(ep_ctrl | DXEPCTL_EPENA | DXEPCTL_CNAK, > - ®->device_regs.out_endp[EP0_CON].doepctl); > + setbits_le32(®->device_regs.out_endp[EP0_CON].doepctl, DXEPCTL_EPENA > | DXEPCTL_CNAK); > > debug_cond(DEBUG_EP0 != 0, "%s:EP0 ZLP DIEPCTL0 = 0x%x\n", > __func__, readl(®->device_regs.in_endp[EP0_CON].diepctl)); > debug_cond(DEBUG_EP0 != 0, "%s:EP0 ZLP DOEPCTL0 = 0x%x\n", > __func__, readl(®->device_regs.out_endp[EP0_CON].doepctl)); > - > } > > static int setdma_rx(struct dwc2_ep *ep, struct dwc2_request *req) > @@ -136,12 +123,11 @@ static int setdma_rx(struct dwc2_ep *ep, struct > dwc2_request *req) > readl(®->device_regs.out_endp[ep_num].doepctl), > buf, pktcnt, length); > return 0; > - > } > > static int setdma_tx(struct dwc2_ep *ep, struct dwc2_request *req) > { > - u32 *buf, ctrl = 0; > + u32 *buf; > u32 length, pktcnt; > u32 ep_num = ep_index(ep); > > @@ -171,16 +157,10 @@ static int setdma_tx(struct dwc2_ep *ep, struct > dwc2_request *req) > FIELD_PREP(DXEPTSIZ_XFERSIZE_MASK, length), > ®->device_regs.in_endp[ep_num].dieptsiz); > > - ctrl = readl(®->device_regs.in_endp[ep_num].diepctl); > - > - /* Write the FIFO number to be used for this endpoint */ > - ctrl &= ~DXEPCTL_TXFNUM_MASK; > - ctrl |= FIELD_PREP(DXEPCTL_TXFNUM_MASK, ep->fifo_num); > - > - /* Clear reserved (Next EP) bits */ > - ctrl &= ~DXEPCTL_NEXTEP_MASK; > - > - writel(DXEPCTL_EPENA | DXEPCTL_CNAK | ctrl, > ®->device_regs.in_endp[ep_num].diepctl); > + clrsetbits_le32(®->device_regs.in_endp[ep_num].diepctl, > + DXEPCTL_TXFNUM_MASK | DXEPCTL_NEXTEP_MASK, > + FIELD_PREP(DXEPCTL_TXFNUM_MASK, ep->fifo_num) | > + DXEPCTL_EPENA | DXEPCTL_CNAK); > > debug_cond(DEBUG_IN_EP, > "%s:EP%d TX DMA start : DIEPDMA0 = 0x%x," > @@ -766,9 +746,7 @@ static int dwc2_fifo_read(struct dwc2_ep *ep, void *cp, > int max) > */ > static void udc_set_address(struct dwc2_udc *dev, unsigned char address) > { > - u32 ctrl = readl(®->device_regs.dcfg); > - > - writel(FIELD_PREP(DCFG_DEVADDR_MASK, address) | ctrl, > ®->device_regs.dcfg); > + setbits_le32(®->device_regs.dcfg, FIELD_PREP(DCFG_DEVADDR_MASK, > address)); > > dwc2_udc_ep0_zlp(dev); > > @@ -892,7 +870,6 @@ static int dwc2_udc_get_status(struct dwc2_udc *dev, > { > u8 ep_num = crq->wIndex & 0x3; > u16 g_status = 0; > - u32 ep_ctrl; > > debug_cond(DEBUG_SETUP != 0, > "%s: *** USB_REQ_GET_STATUS\n", __func__); > @@ -940,9 +917,7 @@ static int dwc2_udc_get_status(struct dwc2_udc *dev, > writel(FIELD_PREP(DXEPTSIZ_PKTCNT_MASK, 1) | > FIELD_PREP(DXEPTSIZ_XFERSIZE_MASK, 2), > ®->device_regs.in_endp[EP0_CON].dieptsiz); > > - ep_ctrl = readl(®->device_regs.in_endp[EP0_CON].diepctl); > - writel(ep_ctrl | DXEPCTL_EPENA | DXEPCTL_CNAK, > - ®->device_regs.in_endp[EP0_CON].diepctl); > + setbits_le32(®->device_regs.in_endp[EP0_CON].diepctl, DXEPCTL_EPENA > | DXEPCTL_CNAK); > dev->ep0state = WAIT_FOR_NULL_COMPLETE; > > return 0; > @@ -951,21 +926,16 @@ static int dwc2_udc_get_status(struct dwc2_udc *dev, > static void dwc2_udc_set_nak(struct dwc2_ep *ep) > { > u8 ep_num; > - u32 ep_ctrl = 0; > > ep_num = ep_index(ep); > debug("%s: ep_num = %d, ep_type = %d\n", __func__, ep_num, ep->ep_type); > > if (ep_is_in(ep)) { > - ep_ctrl = readl(®->device_regs.in_endp[ep_num].diepctl); > - ep_ctrl |= DXEPCTL_SNAK; > - writel(ep_ctrl, ®->device_regs.in_endp[ep_num].diepctl); > + setbits_le32(®->device_regs.in_endp[ep_num].diepctl, > DXEPCTL_SNAK); > debug("%s: set NAK, DIEPCTL%d = 0x%x\n", > __func__, ep_num, > readl(®->device_regs.in_endp[ep_num].diepctl)); > } else { > - ep_ctrl = readl(®->device_regs.out_endp[ep_num].doepctl); > - ep_ctrl |= DXEPCTL_SNAK; > - writel(ep_ctrl, ®->device_regs.out_endp[ep_num].doepctl); > + setbits_le32(®->device_regs.out_endp[ep_num].doepctl, > DXEPCTL_SNAK); > debug("%s: set NAK, DOEPCTL%d = 0x%x\n", > __func__, ep_num, > readl(®->device_regs.out_endp[ep_num].doepctl)); > } > @@ -995,12 +965,8 @@ static void dwc2_udc_ep_set_stall(struct dwc2_ep *ep) > __func__, ep_num, > readl(®->device_regs.in_endp[ep_num].diepctl)); > > } else { > - ep_ctrl = readl(®->device_regs.out_endp[ep_num].doepctl); > - > /* set the stall bit */ > - ep_ctrl |= DXEPCTL_STALL; > - > - writel(ep_ctrl, ®->device_regs.out_endp[ep_num].doepctl); > + setbits_le32(®->device_regs.out_endp[ep_num].doepctl, > DXEPCTL_STALL); > debug("%s: set stall, DOEPCTL%d = 0x%x\n", > __func__, ep_num, > readl(®->device_regs.out_endp[ep_num].doepctl)); > } > @@ -1145,9 +1111,8 @@ static void dwc2_udc_ep_activate(struct dwc2_ep *ep) > } > > /* Unmask EP Interrtupt */ > - writel(readl(®->device_regs.daintmsk) | daintmsk, > ®->device_regs.daintmsk); > + setbits_le32(®->device_regs.daintmsk, daintmsk); > debug("%s: DAINTMSK = 0x%x\n", __func__, > readl(®->device_regs.daintmsk)); > - > } > > static int dwc2_udc_clear_feature(struct usb_ep *_ep) > > --- > base-commit: 05051cfdf0ed6258a945ec181e36d14b7e450dbf > change-id: 20250125-dwc2-clrsetbits-refactor-e485db93d296 > prerequisite-message-id: <20250110-dwc2-dev-v4-0-987f4fd6f...@pigmoral.tech> > prerequisite-patch-id: f05fe7f02791b8f5f6e89e3768584622c49e2ed7 > prerequisite-patch-id: ba89fc49fb08beb94bc5c69c4b82e198a27c334d > prerequisite-patch-id: bc3632796d0010d5711be9597c4222c23adbf9f7 > prerequisite-patch-id: b30e9c649999f42bc5c659bde5650e8aa2a33acd > prerequisite-patch-id: d6623dbacae347c4c7e339a294c60402667a359b > prerequisite-patch-id: 64e64d31939ea7f69818883fbafb8c1906b53ecb > prerequisite-patch-id: 9de6c80cd2996924b8f94a33f3e2a3dd63f1f57b > prerequisite-patch-id: e1752a8f249752de133bda41a387b3d147d60453 > > Best regards, -- Best regards, Junhui Liu