Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init
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 ff58.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 <> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init
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 ff58.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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init
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 ff58.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 < 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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init
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 ff58.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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init
在 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 ff58.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> 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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init
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 ff58.usb: 256 invalid for
> host_perio_tx_fifo_size. Check HW configuration.
>
> Signed-off-by: Yunzhi Li
> ---
>
> 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
> #include
> #include
> +#include
>
> #include
>
> @@ -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;
>
> + /* AHB hclk domain reset, set all AHB interface registers to default */
> + rst = devm_reset_control_get_optional(>dev, "ahb");
> + if (IS_ERR(rst)) {
> + dev_dbg(>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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init
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 ff58.usb: 256 invalid for
host_perio_tx_fifo_size. Check HW configuration.
Signed-off-by: Yunzhi Li l...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init
在 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 ff58.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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init
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 ff58.usb: 256 invalid for
host_perio_tx_fifo_size. Check HW configuration.
Signed-off-by: Yunzhi Li
---
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
#include
#include
+#include
#include
@@ -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;
+ /* AHB hclk domain reset, set all AHB interface registers to default */
+ rst = devm_reset_control_get_optional(>dev, "ahb");
+ if (IS_ERR(rst)) {
+ dev_dbg(>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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init
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 ff58.usb: 256 invalid for
host_perio_tx_fifo_size. Check HW configuration.
Signed-off-by: Yunzhi Li l...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
