* [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init @ 2015-08-11 7:56 ` Yunzhi Li 0 siblings, 0 replies; 12+ messages in thread From: Yunzhi Li @ 2015-08-11 7:56 UTC (permalink / raw) To: jwerner-F7+t8E8rja9g9hUCZPvPmw, dianders-F7+t8E8rja9g9hUCZPvPmw Cc: huangtao-TNX95d0MmH7DzftRWevZcw, cf-TNX95d0MmH7DzftRWevZcw, hl-TNX95d0MmH7DzftRWevZcw, wulf-TNX95d0MmH7DzftRWevZcw, gregory.herrero-ral2JQCrhuEAvxtiuMwx3w, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Yunzhi Li, John Youn, Greg Kroah-Hartman, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA We initiate dwc2 usb controller in BIOS, when kernel driver start-up we should reset AHB hclk domain to reset all AHB interface registers to default. Without this the FIFO value setting might be incorrect because calculating FIFO size need the power-on value of GRXFSIZ/GNPTXFSIZ/HPTXFSIZ registers. This patch could avoid warnning massage like in rk3288 platform: [ 2.074764] dwc2 ff580000.usb: 256 invalid for host_perio_tx_fifo_size. Check HW configuration. Signed-off-by: Yunzhi Li <lyz-TNX95d0MmH7DzftRWevZcw@public.gmane.org> --- drivers/usb/dwc2/platform.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index 9093530..ec6bf6b 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -42,6 +42,7 @@ #include <linux/of_device.h> #include <linux/mutex.h> #include <linux/platform_device.h> +#include <linux/reset.h> #include <linux/usb/of.h> @@ -165,6 +166,7 @@ static int dwc2_driver_probe(struct platform_device *dev) struct resource *res; struct phy *phy; struct usb_phy *uphy; + struct reset_control *rst; int retval; int irq; @@ -189,6 +191,16 @@ static int dwc2_driver_probe(struct platform_device *dev) hsotg->dev = &dev->dev; + /* AHB hclk domain reset, set all AHB interface registers to default */ + rst = devm_reset_control_get_optional(&dev->dev, "ahb"); + if (IS_ERR(rst)) { + dev_dbg(&dev->dev, "Can't get dwc2 AHB reset\n"); + } else { + reset_control_assert(rst); + udelay(5); + reset_control_deassert(rst); + } + /* * Use reasonable defaults so platforms don't have to provide these. */ -- 2.0.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init @ 2015-08-11 7:56 ` Yunzhi Li 0 siblings, 0 replies; 12+ messages in thread From: Yunzhi Li @ 2015-08-11 7:56 UTC (permalink / raw) To: jwerner, dianders Cc: huangtao, cf, hl, wulf, gregory.herrero, linux-rockchip, Yunzhi Li, John Youn, Greg Kroah-Hartman, linux-usb, linux-kernel We initiate dwc2 usb controller in BIOS, when kernel driver start-up we should reset AHB hclk domain to reset all AHB interface registers to default. Without this the FIFO value setting might be incorrect because calculating FIFO size need the power-on value of GRXFSIZ/GNPTXFSIZ/HPTXFSIZ registers. This patch could avoid warnning massage like in rk3288 platform: [ 2.074764] dwc2 ff580000.usb: 256 invalid for host_perio_tx_fifo_size. Check HW configuration. Signed-off-by: Yunzhi Li <lyz@rock-chips.com> --- drivers/usb/dwc2/platform.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index 9093530..ec6bf6b 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -42,6 +42,7 @@ #include <linux/of_device.h> #include <linux/mutex.h> #include <linux/platform_device.h> +#include <linux/reset.h> #include <linux/usb/of.h> @@ -165,6 +166,7 @@ static int dwc2_driver_probe(struct platform_device *dev) struct resource *res; struct phy *phy; struct usb_phy *uphy; + struct reset_control *rst; int retval; int irq; @@ -189,6 +191,16 @@ static int dwc2_driver_probe(struct platform_device *dev) hsotg->dev = &dev->dev; + /* AHB hclk domain reset, set all AHB interface registers to default */ + rst = devm_reset_control_get_optional(&dev->dev, "ahb"); + if (IS_ERR(rst)) { + dev_dbg(&dev->dev, "Can't get dwc2 AHB reset\n"); + } else { + reset_control_assert(rst); + udelay(5); + reset_control_deassert(rst); + } + /* * Use reasonable defaults so platforms don't have to provide these. */ -- 2.0.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 2/3] Documentation: dt-bindings: add dt binding info for dwc2 reset control 2015-08-11 7:56 ` Yunzhi Li (?) @ 2015-08-11 7:56 ` Yunzhi Li [not found] ` <1439279787-26674-2-git-send-email-lyz-TNX95d0MmH7DzftRWevZcw@public.gmane.org> -1 siblings, 1 reply; 12+ messages in thread From: Yunzhi Li @ 2015-08-11 7:56 UTC (permalink / raw) To: jwerner, dianders Cc: huangtao, cf, hl, wulf, gregory.herrero, linux-rockchip, Yunzhi Li, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Greg Kroah-Hartman, Felipe Balbi, Paul Zimmerman, Kever Yang, devicetree, linux-kernel Signed-off-by: Yunzhi Li <lyz@rock-chips.com> --- Documentation/devicetree/bindings/usb/dwc2.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt index fd132cb..6a84099 100644 --- a/Documentation/devicetree/bindings/usb/dwc2.txt +++ b/Documentation/devicetree/bindings/usb/dwc2.txt @@ -24,6 +24,12 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties - g-rx-fifo-size: size of rx fifo size in gadget mode. - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode. - g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode. +- resets: A list of phandle + reset-specifier pairs for the resets listed in + reset-names +- reset-names: Should contain the following: + "ahb": AHB interface reset + "phy": PHY reset + "con": controller reset Example: -- 2.0.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <1439279787-26674-2-git-send-email-lyz-TNX95d0MmH7DzftRWevZcw@public.gmane.org>]
* Re: [PATCH v1 2/3] Documentation: dt-bindings: add dt binding info for dwc2 reset control 2015-08-11 7:56 ` [PATCH v1 2/3] Documentation: dt-bindings: add dt binding info for dwc2 reset control Yunzhi Li @ 2015-08-11 15:20 ` Doug Anderson 0 siblings, 0 replies; 12+ messages in thread From: Doug Anderson @ 2015-08-11 15:20 UTC (permalink / raw) To: Yunzhi Li Cc: Tao Huang, Herrero, Gregory, Lin Huang, Pawel Moll, Ian Campbell, Greg Kroah-Hartman, Kumar Gala, Kever Yang, Felipe Balbi, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, open list:ARM/Rockchip SoC..., devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring, wulf, Eddie Cai, Julius Werner, Mark Rutland, Paul Zimmerman lyz, On Tue, Aug 11, 2015 at 12:56 AM, Yunzhi Li <lyz-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote: > Signed-off-by: Yunzhi Li <lyz-TNX95d0MmH7DzftRWevZcw@public.gmane.org> > --- > > Documentation/devicetree/bindings/usb/dwc2.txt | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt > index fd132cb..6a84099 100644 > --- a/Documentation/devicetree/bindings/usb/dwc2.txt > +++ b/Documentation/devicetree/bindings/usb/dwc2.txt > @@ -24,6 +24,12 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties > - g-rx-fifo-size: size of rx fifo size in gadget mode. > - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode. > - g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode. > +- resets: A list of phandle + reset-specifier pairs for the resets listed in > + reset-names > +- reset-names: Should contain the following: > + "ahb": AHB interface reset > + "phy": PHY reset > + "con": controller reset Would it be worth it to leave the "phy" out? You don't use it in the driver yet and it's unclear to me whether it belongs here or as part of "rockchip,rk3288-usb-phy"? -Doug ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/3] Documentation: dt-bindings: add dt binding info for dwc2 reset control @ 2015-08-11 15:20 ` Doug Anderson 0 siblings, 0 replies; 12+ messages in thread From: Doug Anderson @ 2015-08-11 15:20 UTC (permalink / raw) To: Yunzhi Li Cc: Julius Werner, Tao Huang, Eddie Cai, Lin Huang, wulf, Herrero, Gregory, open list:ARM/Rockchip SoC..., Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Greg Kroah-Hartman, Felipe Balbi, Paul Zimmerman, Kever Yang, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org lyz, On Tue, Aug 11, 2015 at 12:56 AM, Yunzhi Li <lyz@rock-chips.com> wrote: > Signed-off-by: Yunzhi Li <lyz@rock-chips.com> > --- > > Documentation/devicetree/bindings/usb/dwc2.txt | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt > index fd132cb..6a84099 100644 > --- a/Documentation/devicetree/bindings/usb/dwc2.txt > +++ b/Documentation/devicetree/bindings/usb/dwc2.txt > @@ -24,6 +24,12 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties > - g-rx-fifo-size: size of rx fifo size in gadget mode. > - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode. > - g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode. > +- resets: A list of phandle + reset-specifier pairs for the resets listed in > + reset-names > +- reset-names: Should contain the following: > + "ahb": AHB interface reset > + "phy": PHY reset > + "con": controller reset Would it be worth it to leave the "phy" out? You don't use it in the driver yet and it's unclear to me whether it belongs here or as part of "rockchip,rk3288-usb-phy"? -Doug ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v1 3/3] ARM: dts: rockchip: add dwc2 ahb reset property for rk3288 2015-08-11 7:56 ` Yunzhi Li (?) (?) @ 2015-08-11 7:56 ` Yunzhi Li -1 siblings, 0 replies; 12+ messages in thread From: Yunzhi Li @ 2015-08-11 7:56 UTC (permalink / raw) To: jwerner, dianders Cc: huangtao, cf, hl, wulf, gregory.herrero, linux-rockchip, Yunzhi Li, Heiko Stuebner, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, linux-arm-kernel, devicetree, linux-kernel This patch adds dwc2 reset property for rk3288 dwc2 usb controller to fix FIFO setting bug Signed-off-by: Yunzhi Li <lyz@rock-chips.com> --- arch/arm/boot/dts/rk3288.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi index 22316d0..440aa42 100644 --- a/arch/arm/boot/dts/rk3288.dtsi +++ b/arch/arm/boot/dts/rk3288.dtsi @@ -473,6 +473,10 @@ dr_mode = "host"; phys = <&usbphy2>; phy-names = "usb2-phy"; + resets = <&cru SRST_USBHOST1_AHB>, + <&cru SRST_USBHOST1_PHY>, + <&cru SRST_USBHOST1_CON>; + reset-names = "ahb", "phy", "con"; status = "disabled"; }; @@ -490,6 +494,10 @@ g-use-dma; phys = <&usbphy0>; phy-names = "usb2-phy"; + resets = <&cru SRST_USBOTG_AHB>, + <&cru SRST_USBOTG_PHY>, + <&cru SRST_USBOTG_CON>; + reset-names = "ahb", "phy", "con"; status = "disabled"; }; -- 2.0.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init 2015-08-11 7:56 ` Yunzhi Li ` (2 preceding siblings ...) (?) @ 2015-08-14 0:09 ` John Youn [not found] ` <2B3535C5ECE8B5419E3ECBE300772909017528DC44-Yu2iAY70zvrYN67daEjeMPufCSb+aD3WLzEdoUbNIic@public.gmane.org> -1 siblings, 1 reply; 12+ messages in thread From: John Youn @ 2015-08-14 0:09 UTC (permalink / raw) To: Yunzhi Li, jwerner@chromium.org, dianders@chromium.org Cc: huangtao@rock-chips.com, cf@rock-chips.com, hl@rock-chips.com, wulf@rock-chips.com, gregory.herrero@intel.com, linux-rockchip@lists.infradead.org, John Youn, Greg Kroah-Hartman, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org On 8/11/2015 12:57 AM, Yunzhi Li wrote: > We initiate dwc2 usb controller in BIOS, when kernel driver > start-up we should reset AHB hclk domain to reset all AHB > interface registers to default. Without this the FIFO value > setting might be incorrect because calculating FIFO size need the > power-on value of GRXFSIZ/GNPTXFSIZ/HPTXFSIZ registers. > > This patch could avoid warnning massage like in rk3288 platform: > [ 2.074764] dwc2 ff580000.usb: 256 invalid for > host_perio_tx_fifo_size. Check HW configuration. > > Signed-off-by: Yunzhi Li <lyz@rock-chips.com> > --- > > drivers/usb/dwc2/platform.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c > index 9093530..ec6bf6b 100644 > --- a/drivers/usb/dwc2/platform.c > +++ b/drivers/usb/dwc2/platform.c > @@ -42,6 +42,7 @@ > #include <linux/of_device.h> > #include <linux/mutex.h> > #include <linux/platform_device.h> > +#include <linux/reset.h> > > #include <linux/usb/of.h> > > @@ -165,6 +166,7 @@ static int dwc2_driver_probe(struct platform_device *dev) > struct resource *res; > struct phy *phy; > struct usb_phy *uphy; > + struct reset_control *rst; > int retval; > int irq; > > @@ -189,6 +191,16 @@ static int dwc2_driver_probe(struct platform_device *dev) > > hsotg->dev = &dev->dev; > > + /* AHB hclk domain reset, set all AHB interface registers to default */ > + rst = devm_reset_control_get_optional(&dev->dev, "ahb"); > + if (IS_ERR(rst)) { > + dev_dbg(&dev->dev, "Can't get dwc2 AHB reset\n"); > + } else { > + reset_control_assert(rst); > + udelay(5); > + reset_control_deassert(rst); > + } > + > /* > * Use reasonable defaults so platforms don't have to provide these. > */ > I didn't receive the other two patches in this series so I was confused about where the "ahb_reset" was coming from when I replied to your other patch. I see you changed the name and documented the DT so never mind. Another thing is that there probably shouldn't be a debug message on the IS_ERR condition since that is the common case and of no interest to other platforms. The other two resets you added aren't used by the driver anywhere right? Maybe those should be left out until they are. John ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <2B3535C5ECE8B5419E3ECBE300772909017528DC44-Yu2iAY70zvrYN67daEjeMPufCSb+aD3WLzEdoUbNIic@public.gmane.org>]
* Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init 2015-08-14 0:09 ` [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init John Youn @ 2015-08-14 3:28 ` Yunzhi Li 0 siblings, 0 replies; 12+ messages in thread From: Yunzhi Li @ 2015-08-14 3:28 UTC (permalink / raw) To: John Youn, jwerner-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org, dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org Cc: huangtao-TNX95d0MmH7DzftRWevZcw@public.gmane.org, cf-TNX95d0MmH7DzftRWevZcw@public.gmane.org, hl-TNX95d0MmH7DzftRWevZcw@public.gmane.org, wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org, gregory.herrero-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Greg Kroah-Hartman, linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 在 2015/8/14 8:09, John Youn 写道: > On 8/11/2015 12:57 AM, Yunzhi Li wrote: >> We initiate dwc2 usb controller in BIOS, when kernel driver >> start-up we should reset AHB hclk domain to reset all AHB >> interface registers to default. Without this the FIFO value >> setting might be incorrect because calculating FIFO size need the >> power-on value of GRXFSIZ/GNPTXFSIZ/HPTXFSIZ registers. >> >> This patch could avoid warnning massage like in rk3288 platform: >> [ 2.074764] dwc2 ff580000.usb: 256 invalid for >> host_perio_tx_fifo_size. Check HW configuration. >> >> ...... > I didn't receive the other two patches in this series so I was > confused about where the "ahb_reset" was coming from when I > replied to your other patch. > > I see you changed the name and documented the DT so never mind. > > Another thing is that there probably shouldn't be a debug > message on the IS_ERR condition since that is the common case > and of no interest to other platforms. > > The other two resets you added aren't used by the driver > anywhere right? Maybe those should be left out until they are. > > John > Hi John , Here is the other two patches : https://patchwork.kernel.org/patch/6989541/ https://patchwork.kernel.org/patch/6989531/ ahb_reset is hreset_n signal of dwc2 IP. Our rk3288 SoC implement connect this signal to a special register in clock ang reset unit (CRU) module, set this register will reset dwc2 control and status registers(CSR) to default value. You could find more info in <<DesignWare Cores USB 2.0 Hi Speed On-TheGo (OTG) Databook 3.10a>> 4.4.1 System Clock and Reset Signals. Our problem is that dwc2_get_hwparams() reads fifo size registers and reguards it as the power-on reset value, then dwc2_set_param_host_perio_tx_fifo_size() will check this value and make sure the new fifo size value is no bigger than the power-on reset value. But we init and set these fifo registers in BIOS, so here hw->xxx_fifo_size is not the real power-on reset vaule. So we hope to reset CSR before dwc2_get_hwparams(). I have another ideal: we might use GRSTCTL.CSftRst instead of hreset_n to reset dwc2 CSR. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init @ 2015-08-14 3:28 ` Yunzhi Li 0 siblings, 0 replies; 12+ messages in thread From: Yunzhi Li @ 2015-08-14 3:28 UTC (permalink / raw) To: John Youn, jwerner@chromium.org, dianders@chromium.org Cc: huangtao@rock-chips.com, cf@rock-chips.com, hl@rock-chips.com, wulf@rock-chips.com, gregory.herrero@intel.com, linux-rockchip@lists.infradead.org, Greg Kroah-Hartman, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org 在 2015/8/14 8:09, John Youn 写道: > On 8/11/2015 12:57 AM, Yunzhi Li wrote: >> We initiate dwc2 usb controller in BIOS, when kernel driver >> start-up we should reset AHB hclk domain to reset all AHB >> interface registers to default. Without this the FIFO value >> setting might be incorrect because calculating FIFO size need the >> power-on value of GRXFSIZ/GNPTXFSIZ/HPTXFSIZ registers. >> >> This patch could avoid warnning massage like in rk3288 platform: >> [ 2.074764] dwc2 ff580000.usb: 256 invalid for >> host_perio_tx_fifo_size. Check HW configuration. >> >> ...... > I didn't receive the other two patches in this series so I was > confused about where the "ahb_reset" was coming from when I > replied to your other patch. > > I see you changed the name and documented the DT so never mind. > > Another thing is that there probably shouldn't be a debug > message on the IS_ERR condition since that is the common case > and of no interest to other platforms. > > The other two resets you added aren't used by the driver > anywhere right? Maybe those should be left out until they are. > > John > Hi John , Here is the other two patches : https://patchwork.kernel.org/patch/6989541/ https://patchwork.kernel.org/patch/6989531/ ahb_reset is hreset_n signal of dwc2 IP. Our rk3288 SoC implement connect this signal to a special register in clock ang reset unit (CRU) module, set this register will reset dwc2 control and status registers(CSR) to default value. You could find more info in <<DesignWare Cores USB 2.0 Hi Speed On-TheGo (OTG) Databook 3.10a>> 4.4.1 System Clock and Reset Signals. Our problem is that dwc2_get_hwparams() reads fifo size registers and reguards it as the power-on reset value, then dwc2_set_param_host_perio_tx_fifo_size() will check this value and make sure the new fifo size value is no bigger than the power-on reset value. But we init and set these fifo registers in BIOS, so here hw->xxx_fifo_size is not the real power-on reset vaule. So we hope to reset CSR before dwc2_get_hwparams(). I have another ideal: we might use GRSTCTL.CSftRst instead of hreset_n to reset dwc2 CSR. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init 2015-08-14 3:28 ` Yunzhi Li @ 2015-08-14 19:41 ` John Youn -1 siblings, 0 replies; 12+ messages in thread From: John Youn @ 2015-08-14 19:41 UTC (permalink / raw) To: Yunzhi Li, John Youn, jwerner-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org, dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org Cc: huangtao-TNX95d0MmH7DzftRWevZcw@public.gmane.org, cf-TNX95d0MmH7DzftRWevZcw@public.gmane.org, hl-TNX95d0MmH7DzftRWevZcw@public.gmane.org, wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org, gregory.herrero-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Greg Kroah-Hartman, linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On 8/13/2015 8:29 PM, Yunzhi Li wrote: > > > 在 2015/8/14 8:09, John Youn 写道: >> On 8/11/2015 12:57 AM, Yunzhi Li wrote: >>> We initiate dwc2 usb controller in BIOS, when kernel driver >>> start-up we should reset AHB hclk domain to reset all AHB >>> interface registers to default. Without this the FIFO value >>> setting might be incorrect because calculating FIFO size need the >>> power-on value of GRXFSIZ/GNPTXFSIZ/HPTXFSIZ registers. >>> >>> This patch could avoid warnning massage like in rk3288 platform: >>> [ 2.074764] dwc2 ff580000.usb: 256 invalid for >>> host_perio_tx_fifo_size. Check HW configuration. >>> >>> ...... >> I didn't receive the other two patches in this series so I was >> confused about where the "ahb_reset" was coming from when I >> replied to your other patch. >> >> I see you changed the name and documented the DT so never mind. >> >> Another thing is that there probably shouldn't be a debug >> message on the IS_ERR condition since that is the common case >> and of no interest to other platforms. >> >> The other two resets you added aren't used by the driver >> anywhere right? Maybe those should be left out until they are. >> >> John >> > Hi John , > > Here is the other two patches : > https://patchwork.kernel.org/patch/6989541/ > https://patchwork.kernel.org/patch/6989531/ > > ahb_reset is hreset_n signal of dwc2 IP. Our rk3288 SoC implement > connect this signal to a special > register in clock ang reset unit (CRU) module, set this register will > reset dwc2 control and status registers(CSR) > to default value. You could find more info in <<DesignWare Cores USB 2.0 > Hi Speed On-TheGo (OTG) Databook 3.10a>> > 4.4.1 System Clock and Reset Signals. > > Our problem is that dwc2_get_hwparams() reads fifo size registers and > reguards it as the power-on reset value, > then dwc2_set_param_host_perio_tx_fifo_size() will check this value and > make sure the new fifo size value is no bigger > than the power-on reset value. But we init and set these fifo registers > in BIOS, so here hw->xxx_fifo_size is not the > real power-on reset vaule. So we hope to reset CSR before > dwc2_get_hwparams(). > > I have another ideal: we might use GRSTCTL.CSftRst instead of hreset_n > to reset dwc2 CSR. Yes, please try doing that before calling dwc2_get_hwparams(). Maybe by calling dwc2_core_reset(). If that works for you, I think it would be better. John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init @ 2015-08-14 19:41 ` John Youn 0 siblings, 0 replies; 12+ messages in thread From: John Youn @ 2015-08-14 19:41 UTC (permalink / raw) To: Yunzhi Li, John Youn, jwerner@chromium.org, dianders@chromium.org Cc: huangtao@rock-chips.com, cf@rock-chips.com, hl@rock-chips.com, wulf@rock-chips.com, gregory.herrero@intel.com, linux-rockchip@lists.infradead.org, Greg Kroah-Hartman, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org On 8/13/2015 8:29 PM, Yunzhi Li wrote: > > > 在 2015/8/14 8:09, John Youn 写道: >> On 8/11/2015 12:57 AM, Yunzhi Li wrote: >>> We initiate dwc2 usb controller in BIOS, when kernel driver >>> start-up we should reset AHB hclk domain to reset all AHB >>> interface registers to default. Without this the FIFO value >>> setting might be incorrect because calculating FIFO size need the >>> power-on value of GRXFSIZ/GNPTXFSIZ/HPTXFSIZ registers. >>> >>> This patch could avoid warnning massage like in rk3288 platform: >>> [ 2.074764] dwc2 ff580000.usb: 256 invalid for >>> host_perio_tx_fifo_size. Check HW configuration. >>> >>> ...... >> I didn't receive the other two patches in this series so I was >> confused about where the "ahb_reset" was coming from when I >> replied to your other patch. >> >> I see you changed the name and documented the DT so never mind. >> >> Another thing is that there probably shouldn't be a debug >> message on the IS_ERR condition since that is the common case >> and of no interest to other platforms. >> >> The other two resets you added aren't used by the driver >> anywhere right? Maybe those should be left out until they are. >> >> John >> > Hi John , > > Here is the other two patches : > https://patchwork.kernel.org/patch/6989541/ > https://patchwork.kernel.org/patch/6989531/ > > ahb_reset is hreset_n signal of dwc2 IP. Our rk3288 SoC implement > connect this signal to a special > register in clock ang reset unit (CRU) module, set this register will > reset dwc2 control and status registers(CSR) > to default value. You could find more info in <<DesignWare Cores USB 2.0 > Hi Speed On-TheGo (OTG) Databook 3.10a>> > 4.4.1 System Clock and Reset Signals. > > Our problem is that dwc2_get_hwparams() reads fifo size registers and > reguards it as the power-on reset value, > then dwc2_set_param_host_perio_tx_fifo_size() will check this value and > make sure the new fifo size value is no bigger > than the power-on reset value. But we init and set these fifo registers > in BIOS, so here hw->xxx_fifo_size is not the > real power-on reset vaule. So we hope to reset CSR before > dwc2_get_hwparams(). > > I have another ideal: we might use GRSTCTL.CSftRst instead of hreset_n > to reset dwc2 CSR. Yes, please try doing that before calling dwc2_get_hwparams(). Maybe by calling dwc2_core_reset(). If that works for you, I think it would be better. John ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init 2015-08-14 19:41 ` John Youn (?) @ 2015-08-18 9:44 ` Yunzhi Li -1 siblings, 0 replies; 12+ messages in thread From: Yunzhi Li @ 2015-08-18 9:44 UTC (permalink / raw) To: John Youn, jwerner@chromium.org, dianders@chromium.org Cc: huangtao@rock-chips.com, cf@rock-chips.com, hl@rock-chips.com, wulf@rock-chips.com, gregory.herrero@intel.com, linux-rockchip@lists.infradead.org, Greg Kroah-Hartman, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Hi John, 在 2015/8/15 3:41, John Youn 写道: > On 8/13/2015 8:29 PM, Yunzhi Li wrote: >> >> 在 2015/8/14 8:09, John Youn 写道: >>> On 8/11/2015 12:57 AM, Yunzhi Li wrote: >>>> We initiate dwc2 usb controller in BIOS, when kernel driver >>>> start-up we should reset AHB hclk domain to reset all AHB >>>> interface registers to default. Without this the FIFO value >>>> setting might be incorrect because calculating FIFO size need the >>>> power-on value of GRXFSIZ/GNPTXFSIZ/HPTXFSIZ registers. >>>> >>>> This patch could avoid warnning massage like in rk3288 platform: >>>> [ 2.074764] dwc2 ff580000.usb: 256 invalid for >>>> host_perio_tx_fifo_size. Check HW configuration. >>>> >>>> ...... >>> I didn't receive the other two patches in this series so I was >>> confused about where the "ahb_reset" was coming from when I >>> replied to your other patch. >>> >>> I see you changed the name and documented the DT so never mind. >>> >>> Another thing is that there probably shouldn't be a debug >>> message on the IS_ERR condition since that is the common case >>> and of no interest to other platforms. >>> >>> The other two resets you added aren't used by the driver >>> anywhere right? Maybe those should be left out until they are. >>> >>> John >>> >> Hi John , >> >> Here is the other two patches : >> https://patchwork.kernel.org/patch/6989541/ >> https://patchwork.kernel.org/patch/6989531/ >> >> ahb_reset is hreset_n signal of dwc2 IP. Our rk3288 SoC implement >> connect this signal to a special >> register in clock ang reset unit (CRU) module, set this register will >> reset dwc2 control and status registers(CSR) >> to default value. You could find more info in <<DesignWare Cores USB 2.0 >> Hi Speed On-TheGo (OTG) Databook 3.10a>> >> 4.4.1 System Clock and Reset Signals. >> >> Our problem is that dwc2_get_hwparams() reads fifo size registers and >> reguards it as the power-on reset value, >> then dwc2_set_param_host_perio_tx_fifo_size() will check this value and >> make sure the new fifo size value is no bigger >> than the power-on reset value. But we init and set these fifo registers >> in BIOS, so here hw->xxx_fifo_size is not the >> real power-on reset vaule. So we hope to reset CSR before >> dwc2_get_hwparams(). >> >> I have another ideal: we might use GRSTCTL.CSftRst instead of hreset_n >> to reset dwc2 CSR. > > Yes, please try doing that before calling dwc2_get_hwparams(). > Maybe by calling dwc2_core_reset(). If that works for you, I think > it would be better. > > John > > It works and please help review the new patch. Thanks. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-08-18 9:44 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-11 7:56 [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init Yunzhi Li
2015-08-11 7:56 ` Yunzhi Li
2015-08-11 7:56 ` [PATCH v1 2/3] Documentation: dt-bindings: add dt binding info for dwc2 reset control Yunzhi Li
[not found] ` <1439279787-26674-2-git-send-email-lyz-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2015-08-11 15:20 ` Doug Anderson
2015-08-11 15:20 ` Doug Anderson
2015-08-11 7:56 ` [PATCH v1 3/3] ARM: dts: rockchip: add dwc2 ahb reset property for rk3288 Yunzhi Li
2015-08-14 0:09 ` [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init John Youn
[not found] ` <2B3535C5ECE8B5419E3ECBE300772909017528DC44-Yu2iAY70zvrYN67daEjeMPufCSb+aD3WLzEdoUbNIic@public.gmane.org>
2015-08-14 3:28 ` Yunzhi Li
2015-08-14 3:28 ` Yunzhi Li
2015-08-14 19:41 ` John Youn
2015-08-14 19:41 ` John Youn
2015-08-18 9:44 ` Yunzhi Li
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.