On 06/07/2017 05:33 AM, rock-chips(daniel.meng) wrote:

On 2017/6/6 23:02, Marek Vasut wrote:

...

On 06/06/2017 12:33 PM, Meng Dongyang wrote:

...

In current code, after running the command of "usb start", the controller will keep in otg mode and can't switch to host mode if not support SNP/SRP capability. So add the property of "hnp-srp-disable" in the DTS to config the contrller work in force mode of host.

Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com

Changes in v3:

• revert change of macro definition in dwc2 driver
• support host mode without HNP/SRP capability through DTS

Changes in v2:

• Splited from patch [07/08] of v1

  drivers/usb/host/dwc2.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 0e5df15..73a0290 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -43,6 +43,7 @@ struct dwc2_priv { struct dwc2_core_regs *regs; int root_hub_devnum; bool ext_vbus;

• bool hnp_srp_disable; bool oc_disable; }; @@ -394,6 +395,9 @@ static void dwc_otg_core_init(struct dwc2_priv

*priv) usbcfg |= DWC2_GUSBCFG_ULPI_CLK_SUS_M; } #endif

• if (priv->hnp_srp_disable)

•    usbcfg |= DWC2_GUSBCFG_FORCEHOSTMODE;
  

• writel(usbcfg, &regs->gusbcfg);
    /* Program the GAHBCFG Register. */
  

@@ -423,7 +427,9 @@ static void dwc_otg_core_init(struct dwc2_priv *priv) writel(ahbcfg, &regs->gahbcfg); /* Program the GUSBCFG register for HNP/SRP. */

• setbits_le32(&regs->gusbcfg, DWC2_GUSBCFG_HNPCAP |

DWC2_GUSBCFG_SRPCAP);

• if (!priv->hnp_srp_disable)

•    setbits_le32(&regs->gusbcfg,
  

•             DWC2_GUSBCFG_HNPCAP | DWC2_GUSBCFG_SRPCAP);
  

I guess you can prepare the mask and then do setbits...() once ? Looks good otherwise.

I have attempted to move this config to the last writel of usbcfg and checked on our platform. Do you think it is better?

There is no last writel() call in this function, so no. There're only setbits() call and this can be reduced to a single setbits() call at the end. Note that setbits...() is a RMW call.

Prepare a mask maybe looks a little strange since it only use with writel() and the other setbits..() in this function use macro definition directly.

Not sure I understand this concern. You need to set conditional bits in the mask.

ie. u32 mask = FOO;

if (bar) mask |= BAZ;

mask |= QUUX; setbits...(reg, mask);

...

...

#ifdef CONFIG_DWC2_IC_USB_CAP setbits_le32(&regs->gusbcfg, DWC2_GUSBCFG_IC_USB_CAP); @@ -1244,6 +1250,11 @@ static int dwc2_usb_ofdata_to_platdata(struct udevice *dev) if (prop) priv->oc_disable = true;

• prop = fdt_getprop(gd->fdt_blob, dev_of_offset(dev),

•           "hnp-srp-disable", NULL);
  

• if (prop)

•    priv->hnp_srp_disable = true;
  

• return 0;
  

  }