Skip to contentSkip to conversation

sunxi: H3: Do not clear usb companion clk-gate / reset on remove

PATCH New Hans de Goede 2 participants 3 messages Apr 9, 2016 – Apr 10, 2016

Subsystems (4)
USB Arm/Allwinner sunXi SoC support USB ehci driver USB ohci driver
lore
Share & tools
lore

Hans de Goede

author
#0 lore raw

PATCH 1/1 sunxi: H3: Do not clear usb companion clk-gate / reset on remove

On the H3 we need to enable the clk and de-assert the reset of the companion to be able to talk to the actual usb host controller.

Before this commit we were also disabling the companion clk-gate / asserting its reset on remove, causing the later remove callback of the companion itself to (sometimes) fail with:

ERROR: USB HC reset timed out!

This commit fixes this by not disabling the companion's clk-gate nor asserting its reset on remove.

Signed-off-by Hans de Goede <[email protected]>
---
 drivers/usb/host/ehci-sunxi.c | 10 +++++++---
 drivers/usb/host/ohci-sunxi.c | 10 +++++++---
 2 files changed, 14 insertions(+), 6 deletions(-)
drivers/usb/host/ehci-sunxi.c +7 -3
index d5eb492..f2d83e3 100644
@@ -38,6 +38,7 @@ static int ehci_usb_probe(struct udevice *dev)
 	struct ehci_sunxi_priv *priv = dev_get_priv(dev);
 	struct ehci_hccr *hccr = (struct ehci_hccr *)dev_get_addr(dev);
 	struct ehci_hcor *hcor;
+	int extra_ahb_gate_mask = 0;
 
 	/*
 	 * This should go away once we've moved to the driver model for

@@ -45,15 +46,18 @@ static int ehci_usb_probe(struct udevice *dev)
 	 */
 	priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0;
 #ifdef CONFIG_MACH_SUN8I_H3
-	priv->ahb_gate_mask |= 1 << AHB_GATE_OFFSET_USB_OHCI0;
+	extra_ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_OHCI0;
 #endif
 	priv->phy_index = ((u32)hccr - SUNXI_USB1_BASE) / BASE_DIST;
 	priv->ahb_gate_mask <<= priv->phy_index * AHB_CLK_DIST;
+	extra_ahb_gate_mask <<= priv->phy_index * AHB_CLK_DIST;
 	priv->phy_index++; /* Non otg phys start at 1 */
 
-	setbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);
+	setbits_le32(&ccm->ahb_gate0,
+		     priv->ahb_gate_mask | extra_ahb_gate_mask);
 #ifdef CONFIG_SUNXI_GEN_SUN6I
-	setbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
+	setbits_le32(&ccm->ahb_reset0_cfg,
+		     priv->ahb_gate_mask | extra_ahb_gate_mask);
 #endif
 
 	sunxi_usb_phy_init(priv->phy_index);
drivers/usb/host/ohci-sunxi.c +7 -3
index 6f3f4ce..2a1e8bf 100644
@@ -38,6 +38,7 @@ static int ohci_usb_probe(struct udevice *dev)
 	struct usb_bus_priv *bus_priv = dev_get_uclass_priv(dev);
 	struct ohci_sunxi_priv *priv = dev_get_priv(dev);
 	struct ohci_regs *regs = (struct ohci_regs *)dev_get_addr(dev);
+	int extra_ahb_gate_mask = 0;
 
 	bus_priv->companion = true;
 

@@ -47,18 +48,21 @@ static int ohci_usb_probe(struct udevice *dev)
 	 */
 	priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_OHCI0;
 #ifdef CONFIG_MACH_SUN8I_H3
-	priv->ahb_gate_mask |= 1 << AHB_GATE_OFFSET_USB_EHCI0;
+	extra_ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0;
 #endif
 	priv->usb_gate_mask = CCM_USB_CTRL_OHCI0_CLK;
 	priv->phy_index = ((u32)regs - (SUNXI_USB1_BASE + 0x400)) / BASE_DIST;
 	priv->ahb_gate_mask <<= priv->phy_index * AHB_CLK_DIST;
+	extra_ahb_gate_mask <<= priv->phy_index * AHB_CLK_DIST;
 	priv->usb_gate_mask <<= priv->phy_index;
 	priv->phy_index++; /* Non otg phys start at 1 */
 
-	setbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);
+	setbits_le32(&ccm->ahb_gate0,
+		     priv->ahb_gate_mask | extra_ahb_gate_mask);
 	setbits_le32(&ccm->usb_clk_cfg, priv->usb_gate_mask);
 #ifdef CONFIG_SUNXI_GEN_SUN6I
-	setbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
+	setbits_le32(&ccm->ahb_reset0_cfg,
+		     priv->ahb_gate_mask | extra_ahb_gate_mask);
 #endif
 
 	sunxi_usb_phy_init(priv->phy_index);
Signature · 1 line
2.7.3
#1 lore raw

PATCH 1/1 sunxi: H3: Do not clear usb companion clk-gate / reset on remove

Quoted 11 lines · Hans de Goede

On the H3 we need to enable the clk and de-assert the reset of the companion to be able to talk to the actual usb host controller.

Before this commit we were also disabling the companion clk-gate / asserting its reset on remove, causing the later remove callback of the companion itself to (sometimes) fail with:

ERROR: USB HC reset timed out!

This commit fixes this by not disabling the companion's clk-gate nor asserting its reset on remove.

This sounds like a bug (or perhaps lack of expressiveness) in the DM clk bindings to me, but I can see why we want to do it this way shirt term:

Quoted 1 line
Signed-off-by Hans de Goede <[email protected]>
Acked-by Ian Campbell <[email protected]>
Quoted 82 lines

---
?drivers/usb/host/ehci-sunxi.c | 10 +++++++--- ?drivers/usb/host/ohci-sunxi.c | 10 +++++++--- ?2 files changed, 14 insertions(+), 6 deletions(-)

drivers/usb/host/ehci- renamed from drivers/usb/host/ehci-sunxi.c +0 -0

sunxi.c
index d5eb492..f2d83e3 100644

drivers/usb/host/ehci-sunxi.c +0 -0
@@ -38,6 +38,7 @@ static int ehci_usb_probe(struct udevice *dev)
 ?	struct ehci_sunxi_priv *priv = dev_get_priv(dev);

? struct ehci_hccr *hccr = (struct ehci_hccr
*)dev_get_addr(dev);
? struct ehci_hcor *hcor;

drivers/usb/host/ehci-sunxi.c +1 -0
+	int extra_ahb_gate_mask = 0;

?
? /*
? ?* This should go away once we've moved to the driver model
for

drivers/usb/host/ehci-sunxi.c +0 -0
@@ -45,15 +46,18 @@ static int ehci_usb_probe(struct udevice *dev)

? ?*/
? priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0;
?#ifdef CONFIG_MACH_SUN8I_H3

drivers/usb/host/ehci-sunxi.c +1 -1
-	priv->ahb_gate_mask |= 1 << AHB_GATE_OFFSET_USB_OHCI0;
+	extra_ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_OHCI0;

?#endif
? priv->phy_index = ((u32)hccr - SUNXI_USB1_BASE) / BASE_DIST;
? priv->ahb_gate_mask <<= priv->phy_index * AHB_CLK_DIST;

drivers/usb/host/ehci-sunxi.c +1 -0
+	extra_ahb_gate_mask <<= priv->phy_index * AHB_CLK_DIST;

? priv->phy_index++; /* Non otg phys start at 1 */
?

drivers/usb/host/ehci-sunxi.c +2 -1
-	setbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);
+	setbits_le32(&ccm->ahb_gate0,
+		?????priv->ahb_gate_mask | extra_ahb_gate_mask);

?#ifdef CONFIG_SUNXI_GEN_SUN6I

drivers/usb/host/ehci-sunxi.c +2 -1
-	setbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
+	setbits_le32(&ccm->ahb_reset0_cfg,
+		?????priv->ahb_gate_mask | extra_ahb_gate_mask);

?#endif
?
? sunxi_usb_phy_init(priv->phy_index);

drivers/usb/host/ohci- renamed from drivers/usb/host/ohci-sunxi.c +0 -0

sunxi.c
index 6f3f4ce..2a1e8bf 100644

drivers/usb/host/ohci-sunxi.c +0 -0
@@ -38,6 +38,7 @@ static int ohci_usb_probe(struct udevice *dev)
 ?	struct usb_bus_priv *bus_priv = dev_get_uclass_priv(dev);
 ?	struct ohci_sunxi_priv *priv = dev_get_priv(dev);

? struct ohci_regs *regs = (struct ohci_regs
*)dev_get_addr(dev);

drivers/usb/host/ohci-sunxi.c +1 -0
+	int extra_ahb_gate_mask = 0;

?
? bus_priv->companion = true;
?

drivers/usb/host/ohci-sunxi.c +0 -0
@@ -47,18 +48,21 @@ static int ohci_usb_probe(struct udevice *dev)

? ?*/
? priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_OHCI0;
?#ifdef CONFIG_MACH_SUN8I_H3

drivers/usb/host/ohci-sunxi.c +1 -1
-	priv->ahb_gate_mask |= 1 << AHB_GATE_OFFSET_USB_EHCI0;
+	extra_ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0;

?#endif
? priv->usb_gate_mask = CCM_USB_CTRL_OHCI0_CLK;
? priv->phy_index = ((u32)regs - (SUNXI_USB1_BASE + 0x400)) /
BASE_DIST;
? priv->ahb_gate_mask <<= priv->phy_index * AHB_CLK_DIST;

drivers/usb/host/ohci-sunxi.c +1 -0
+	extra_ahb_gate_mask <<= priv->phy_index * AHB_CLK_DIST;

? priv->usb_gate_mask <<= priv->phy_index;
? priv->phy_index++; /* Non otg phys start at 1 */
?

drivers/usb/host/ohci-sunxi.c +2 -1
-	setbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);
+	setbits_le32(&ccm->ahb_gate0,
+		?????priv->ahb_gate_mask | extra_ahb_gate_mask);

? setbits_le32(&ccm->usb_clk_cfg, priv->usb_gate_mask);
?#ifdef CONFIG_SUNXI_GEN_SUN6I

drivers/usb/host/ohci-sunxi.c +2 -1
-	setbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
+	setbits_le32(&ccm->ahb_reset0_cfg,
+		?????priv->ahb_gate_mask | extra_ahb_gate_mask);

?#endif
?
? sunxi_usb_phy_init(priv->phy_index);

author
#2 lore raw

PATCH 1/1 sunxi: H3: Do not clear usb companion clk-gate / reset on remove

Hi,

Quoted 8 lines · Ian Campbell
Quoted 7 lines · Hans de Goede

On the H3 we need to enable the clk and de-assert the reset of the companion to be able to talk to the actual usb host controller.

Before this commit we were also disabling the companion clk-gate / asserting its reset on remove, causing the later remove callback of the companion itself to (sometimes) fail with:

Quoted 8 lines
Quoted 4 lines

ERROR: USB HC reset timed out!

This commit fixes this by not disabling the companion's clk-gate nor asserting its reset on remove.

This sounds like a bug (or perhaps lack of expressiveness) in the DM clk bindings to me, but I can see why we want to do it this way shirt term:

Actually the problem is that in u-boot the sunxi glue is not using a proper clk / reset driver. The in kernel clk/reset driver keep a usage count and only disable the clk on the last disable / (re)assert the reset on the last assert.

Quoted 3 lines
Quoted 1 line
Signed-off-by Hans de Goede <[email protected]>
Acked-by Ian Campbell <[email protected]>

Thanks & Regards,

Hans

Quoted 83 lines
Quoted 83 lines
---
  drivers/usb/host/ehci-sunxi.c | 10 +++++++---
  drivers/usb/host/ohci-sunxi.c | 10 +++++++---
  2 files changed, 14 insertions(+), 6 deletions(-)
drivers/usb/host/ehci- renamed from drivers/usb/host/ehci-sunxi.c +0 -0

sunxi.c
index d5eb492..f2d83e3 100644

drivers/usb/host/ehci-sunxi.c +1 -0
@@ -38,6 +38,7 @@ static int ehci_usb_probe(struct udevice *dev)
  	struct ehci_sunxi_priv *priv = dev_get_priv(dev);
  	struct ehci_hccr *hccr = (struct ehci_hccr
 *)dev_get_addr(dev);
  	struct ehci_hcor *hcor;
+	int extra_ahb_gate_mask = 0;
 
  	/*

* This should go away once we've moved to the driver model
for

drivers/usb/host/ehci-sunxi.c +6 -3
@@ -45,15 +46,18 @@ static int ehci_usb_probe(struct udevice *dev)
  	 */
  	priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0;
  #ifdef CONFIG_MACH_SUN8I_H3
-	priv->ahb_gate_mask |= 1 << AHB_GATE_OFFSET_USB_OHCI0;
+	extra_ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_OHCI0;
  #endif
  	priv->phy_index = ((u32)hccr - SUNXI_USB1_BASE) / BASE_DIST;
  	priv->ahb_gate_mask <<= priv->phy_index * AHB_CLK_DIST;
+	extra_ahb_gate_mask <<= priv->phy_index * AHB_CLK_DIST;
  	priv->phy_index++; /* Non otg phys start at 1 */
 
-	setbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);
+	setbits_le32(&ccm->ahb_gate0,
+		     priv->ahb_gate_mask | extra_ahb_gate_mask);
  #ifdef CONFIG_SUNXI_GEN_SUN6I
-	setbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
+	setbits_le32(&ccm->ahb_reset0_cfg,
+		     priv->ahb_gate_mask | extra_ahb_gate_mask);
  #endif
 
  	sunxi_usb_phy_init(priv->phy_index);
drivers/usb/host/ohci- renamed from drivers/usb/host/ohci-sunxi.c +0 -0

sunxi.c
index 6f3f4ce..2a1e8bf 100644

drivers/usb/host/ohci-sunxi.c +7 -3
@@ -38,6 +38,7 @@ static int ohci_usb_probe(struct udevice *dev)
  	struct usb_bus_priv *bus_priv = dev_get_uclass_priv(dev);
  	struct ohci_sunxi_priv *priv = dev_get_priv(dev);
  	struct ohci_regs *regs = (struct ohci_regs
 *)dev_get_addr(dev);
+	int extra_ahb_gate_mask = 0;
 
  	bus_priv->companion = true;

@@ -47,18 +48,21 @@ static int ohci_usb_probe(struct udevice *dev)
  	 */
  	priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_OHCI0;
  #ifdef CONFIG_MACH_SUN8I_H3
-	priv->ahb_gate_mask |= 1 << AHB_GATE_OFFSET_USB_EHCI0;
+	extra_ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0;
  #endif
  	priv->usb_gate_mask = CCM_USB_CTRL_OHCI0_CLK;
  	priv->phy_index = ((u32)regs - (SUNXI_USB1_BASE + 0x400)) /
 BASE_DIST;
  	priv->ahb_gate_mask <<= priv->phy_index * AHB_CLK_DIST;
+	extra_ahb_gate_mask <<= priv->phy_index * AHB_CLK_DIST;
  	priv->usb_gate_mask <<= priv->phy_index;
  	priv->phy_index++; /* Non otg phys start at 1 */
 
-	setbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);
+	setbits_le32(&ccm->ahb_gate0,
+		     priv->ahb_gate_mask | extra_ahb_gate_mask);
  	setbits_le32(&ccm->usb_clk_cfg, priv->usb_gate_mask);
  #ifdef CONFIG_SUNXI_GEN_SUN6I
-	setbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
+	setbits_le32(&ccm->ahb_reset0_cfg,
+		     priv->ahb_gate_mask | extra_ahb_gate_mask);
  #endif
 

  	sunxi_usb_phy_init(priv->phy_index);