Re: [PATCH] usb: dwc2: update reset method for host and device mode

On 3/28/24 2:14 PM, Kongyang Liu wrote:

[...]

@@ -464,12 +464,26 @@ static void reconfig_usbd(struct dwc2_udc *dev) { /* 2. Soft-reset OTG Core and then unreset again. */ int i;

• unsigned int uTemp = writel(CORE_SOFT_RESET, &reg->grstctl);

• unsigned int uTemp; uint32_t dflt_gusbcfg; uint32_t rx_fifo_sz, tx_fifo_sz, np_tx_fifo_sz; u32 max_hw_ep; int pdata_hw_ep;

Drop this newline

• u32 snpsid, greset;
• snpsid = readl(&reg->gsnpsid);
• writel(CORE_SOFT_RESET, &reg->grstctl);
• if ((snpsid & SNPSID_VER_MASK) < (SNPSID_VER_420a & SNPSID_VER_MASK)) {

Can you use FIELD_GET()/FIELD_PREP() for this ?

• wait_for_bit_le32(&reg->grstctl, CORE_SOFT_RESET, false, 1000, false);
  

• } else {

• wait_for_bit_le32(&reg->grstctl, CORE_SOFT_RESET_DONE, true, 1000, false);
  

• greset = readl(&reg->grstctl);
  

• greset &= ~CORE_SOFT_RESET;
  

• greset |= CORE_SOFT_RESET_DONE;
  

• writel(greset, &reg->grstctl);
  

clrsetbits_le32()

[...]

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 637eb2dd06..1baeff96ee 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -159,6 +159,7 @@ static void dwc_otg_core_reset(struct udevice *dev, struct dwc2_core_regs *regs) { int ret;

• u32 snpsid, greset;

  /* Wait for AHB master IDLE state. */ ret = wait_for_bit_le32(&regs->grstctl, DWC2_GRSTCTL_AHBIDLE,

@@ -167,9 +168,20 @@ static void dwc_otg_core_reset(struct udevice *dev, dev_info(dev, "%s: Timeout!\n", __func__);

/* Core Soft Reset */

• snpsid = readl(&regs->gsnpsid); writel(DWC2_GRSTCTL_CSFTRST, &regs->grstctl);

• ret = wait_for_bit_le32(&regs->grstctl, DWC2_GRSTCTL_CSFTRST,

• 		false, 1000, false);
  

• if ((snpsid & DWC2_SNPSID_VER_MASK) < (DWC2_SNPSID_DEVID_VER_420a & DWC2_SNPSID_VER_MASK)) {

• ret = wait_for_bit_le32(&regs->grstctl, DWC2_GRSTCTL_CSFTRST,
  

• 			false, 1000, false);
  

• } else {

• ret = wait_for_bit_le32(&regs->grstctl, DWC2_GRSTCTL_GSFTRST_DONE,
  

• 			true, 1000, false);
  

• greset = readl(&regs->grstctl);
  

• greset &= ~DWC2_GRSTCTL_CSFTRST;
  

• greset |= DWC2_GRSTCTL_GSFTRST_DONE;
  

• writel(greset, &regs->grstctl);
  

Same comments as above.

Maybe this should be pulled into dedicated function to avoid duplication?

• }
• if (ret) dev_info(dev, "%s: Timeout!\n", __func__);

@@ -1180,7 +1192,8 @@ static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv) snpsid >> 12 & 0xf, snpsid & 0xfff);

if ((snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_2xx &&

•    (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx) {
  

• (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx &&
  

•    (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_4xx) {
  

Try FIELD_GET/FIELD_PREP