All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] sunxi: H3: Do not clear usb companion clk-gate / reset on remove
@ 2016-04-09 13:23 Hans de Goede
  2016-04-10  8:18 ` Ian Campbell
  0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2016-04-09 13:23 UTC (permalink / raw)
  To: u-boot

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 <hdegoede@redhat.com>
---
 drivers/usb/host/ehci-sunxi.c | 10 +++++++---
 drivers/usb/host/ohci-sunxi.c | 10 +++++++---
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
index d5eb492..f2d83e3 100644
--- a/drivers/usb/host/ehci-sunxi.c
+++ b/drivers/usb/host/ehci-sunxi.c
@@ -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);
diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c
index 6f3f4ce..2a1e8bf 100644
--- a/drivers/usb/host/ohci-sunxi.c
+++ b/drivers/usb/host/ohci-sunxi.c
@@ -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);
-- 
2.7.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* [U-Boot] [PATCH] sunxi: H3: Do not clear usb companion clk-gate / reset on remove
  2016-04-09 13:23 [U-Boot] [PATCH] sunxi: H3: Do not clear usb companion clk-gate / reset on remove Hans de Goede
@ 2016-04-10  8:18 ` Ian Campbell
  2016-04-10  8:45   ` Hans de Goede
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Campbell @ 2016-04-10  8:18 UTC (permalink / raw)
  To: u-boot

On Sat, 2016-04-09 at 15:23 +0200, Hans de Goede wrote:
> 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:

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

> ---
> ?drivers/usb/host/ehci-sunxi.c | 10 +++++++---
> ?drivers/usb/host/ohci-sunxi.c | 10 +++++++---
> ?2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-
> sunxi.c
> index d5eb492..f2d83e3 100644
> --- a/drivers/usb/host/ehci-sunxi.c
> +++ b/drivers/usb/host/ehci-sunxi.c
> @@ -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);
> diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-
> sunxi.c
> index 6f3f4ce..2a1e8bf 100644
> --- a/drivers/usb/host/ohci-sunxi.c
> +++ b/drivers/usb/host/ohci-sunxi.c
> @@ -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);

^ permalink raw reply	[flat|nested] 3+ messages in thread
* [U-Boot] [PATCH] sunxi: H3: Do not clear usb companion clk-gate / reset on remove
  2016-04-10  8:18 ` Ian Campbell
@ 2016-04-10  8:45   ` Hans de Goede
  0 siblings, 0 replies; 3+ messages in thread
From: Hans de Goede @ 2016-04-10  8:45 UTC (permalink / raw)
  To: u-boot

Hi,

On 10-04-16 10:18, Ian Campbell wrote:
> On Sat, 2016-04-09 at 15:23 +0200, Hans de Goede wrote:
>> 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:

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.

>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> Acked-by: Ian Campbell <ijc@hellion.org.uk>

Thanks & Regards,

Hans


>
>> ---
>>   drivers/usb/host/ehci-sunxi.c | 10 +++++++---
>>   drivers/usb/host/ohci-sunxi.c | 10 +++++++---
>>   2 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-
>> sunxi.c
>> index d5eb492..f2d83e3 100644
>> --- a/drivers/usb/host/ehci-sunxi.c
>> +++ b/drivers/usb/host/ehci-sunxi.c
>> @@ -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);
>> diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-
>> sunxi.c
>> index 6f3f4ce..2a1e8bf 100644
>> --- a/drivers/usb/host/ohci-sunxi.c
>> +++ b/drivers/usb/host/ohci-sunxi.c
>> @@ -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);

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-04-10  8:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-09 13:23 [U-Boot] [PATCH] sunxi: H3: Do not clear usb companion clk-gate / reset on remove Hans de Goede
2016-04-10  8:18 ` Ian Campbell
2016-04-10  8:45   ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.