Re: [PATCH v2 1/3] usb: chipidea: add controller reset API
On Mon, Nov 17, 2014 at 10:19:49AM +0800, Peter Chen wrote: On Fri, Nov 14, 2014 at 09:06:47AM -0600, Felipe Balbi wrote: On Fri, Nov 14, 2014 at 03:33:44PM +0800, Peter Chen wrote: On Thu, Nov 13, 2014 at 09:55:34PM -0600, Felipe Balbi wrote: Hi, On Fri, Nov 14, 2014 at 08:03:15AM +0800, Peter Chen wrote: Add controller reset API, it may be used for host/otg driver in future. you need a better commit log here. How would it be used ? when ? And, more importantly, why ? I will add more information Why would we need to reset the IP outside of -probe() ? I don't oppose to the refactoring, but why would we ever need to reset this IP ? Usually, we need to below reset controller at below situations: - Role switch, host-device and device-host - Return from hibernation, which the controller will lost its power, and the content of the register are lost. resetting is probably not the best solution in either case, though. It might be the easiest, but not the best. For suspend/resume, you would be better off saving/restoring contenxt and for role switch, you could just reprogram the few registers which need to change when role swapping. For safe reason, it is better to do reset controller at above two situations. Why we need to do reset at probe, it may also work if we program register directly. safety ? That's marketting parlance for I don't know how this works :-) In any case, it has been there for such a long time anyway so I won't block $subject. Still something to add to your TODO list. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 1/3] usb: chipidea: add controller reset API
On Fri, Nov 14, 2014 at 02:55:41PM +0300, Sergei Shtylyov wrote:
Hello.
On 11/14/2014 3:03 AM, Peter Chen wrote:
Add controller reset API, it may be used for host/otg driver in future.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
Changes for v2:
- Add return value check for controller reset at hw_device_reset
drivers/usb/chipidea/core.c | 30 +++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index bd74f27..dffd89b 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
[...]
@@ -392,13 +412,17 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
*/
int hw_device_reset(struct ci_hdrc *ci, u32 mode)
{
+int ret;
+
/* should flush stop before reset */
hw_write(ci, OP_ENDPTFLUSH, ~0, ~0);
hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
-hw_write(ci, OP_USBCMD, USBCMD_RST, USBCMD_RST);
-while (hw_read(ci, OP_USBCMD, USBCMD_RST))
-udelay(10); /* not RTOS friendly */
+ret = hw_controller_reset(ci);
+if (ret) {
+dev_err(ci-dev, error for reset, ret=%d\n, ret);
Perhaps error resetting?
Thanks, will change to error resetting controller.
+return ret;
+}
WBR, Sergei
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] usb: chipidea: add controller reset API
On Fri, Nov 14, 2014 at 09:06:47AM -0600, Felipe Balbi wrote: On Fri, Nov 14, 2014 at 03:33:44PM +0800, Peter Chen wrote: On Thu, Nov 13, 2014 at 09:55:34PM -0600, Felipe Balbi wrote: Hi, On Fri, Nov 14, 2014 at 08:03:15AM +0800, Peter Chen wrote: Add controller reset API, it may be used for host/otg driver in future. you need a better commit log here. How would it be used ? when ? And, more importantly, why ? I will add more information Why would we need to reset the IP outside of -probe() ? I don't oppose to the refactoring, but why would we ever need to reset this IP ? Usually, we need to below reset controller at below situations: - Role switch, host-device and device-host - Return from hibernation, which the controller will lost its power, and the content of the register are lost. resetting is probably not the best solution in either case, though. It might be the easiest, but not the best. For suspend/resume, you would be better off saving/restoring contenxt and for role switch, you could just reprogram the few registers which need to change when role swapping. For safe reason, it is better to do reset controller at above two situations. Why we need to do reset at probe, it may also work if we program register directly. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] usb: chipidea: add controller reset API
On Thu, Nov 13, 2014 at 09:55:34PM -0600, Felipe Balbi wrote:
Hi,
On Fri, Nov 14, 2014 at 08:03:15AM +0800, Peter Chen wrote:
Add controller reset API, it may be used for host/otg driver in future.
you need a better commit log here. How would it be used ? when ? And,
more importantly, why ?
I will add more information
Why would we need to reset the IP outside of -probe() ?
I don't oppose to the refactoring, but why would we ever need to reset
this IP ?
Usually, we need to below reset controller at below situations:
- Role switch, host-device and device-host
- Return from hibernation, which the controller will lost its power,
and the content of the register are lost.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
Changes for v2:
- Add return value check for controller reset at hw_device_reset
drivers/usb/chipidea/core.c | 30 +++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index bd74f27..dffd89b 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -385,6 +385,26 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
}
/**
+ * hw_controller_reset: do controller reset
+ * @ci: the controller
+ *
minor nit: indentation
Will change
+ * This function returns an error code
+ */
+static int hw_controller_reset(struct ci_hdrc *ci)
+{
+ int count = 0;
+
+ hw_write(ci, OP_USBCMD, USBCMD_RST, USBCMD_RST);
+ while (hw_read(ci, OP_USBCMD, USBCMD_RST)) {
+ udelay(10);
+ if (count++ 1000)
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+/**
* hw_device_reset: resets chip (execute without interruption)
* @ci: the controller
*
@@ -392,13 +412,17 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
*/
int hw_device_reset(struct ci_hdrc *ci, u32 mode)
{
+ int ret;
+
/* should flush stop before reset */
hw_write(ci, OP_ENDPTFLUSH, ~0, ~0);
hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
- hw_write(ci, OP_USBCMD, USBCMD_RST, USBCMD_RST);
- while (hw_read(ci, OP_USBCMD, USBCMD_RST))
- udelay(10); /* not RTOS friendly */
+ ret = hw_controller_reset(ci);
+ if (ret) {
+ dev_err(ci-dev, error for reset, ret=%d\n, ret);
+ return ret;
+ }
if (ci-platdata-notify_event)
ci-platdata-notify_event(ci,
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
balbi
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] usb: chipidea: add controller reset API
Hello.
On 11/14/2014 3:03 AM, Peter Chen wrote:
Add controller reset API, it may be used for host/otg driver in future.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
Changes for v2:
- Add return value check for controller reset at hw_device_reset
drivers/usb/chipidea/core.c | 30 +++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index bd74f27..dffd89b 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
[...]
@@ -392,13 +412,17 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
*/
int hw_device_reset(struct ci_hdrc *ci, u32 mode)
{
+ int ret;
+
/* should flush stop before reset */
hw_write(ci, OP_ENDPTFLUSH, ~0, ~0);
hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
- hw_write(ci, OP_USBCMD, USBCMD_RST, USBCMD_RST);
- while (hw_read(ci, OP_USBCMD, USBCMD_RST))
- udelay(10); /* not RTOS friendly */
+ ret = hw_controller_reset(ci);
+ if (ret) {
+ dev_err(ci-dev, error for reset, ret=%d\n, ret);
Perhaps error resetting?
+ return ret;
+ }
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] usb: chipidea: add controller reset API
On Fri, Nov 14, 2014 at 03:33:44PM +0800, Peter Chen wrote: On Thu, Nov 13, 2014 at 09:55:34PM -0600, Felipe Balbi wrote: Hi, On Fri, Nov 14, 2014 at 08:03:15AM +0800, Peter Chen wrote: Add controller reset API, it may be used for host/otg driver in future. you need a better commit log here. How would it be used ? when ? And, more importantly, why ? I will add more information Why would we need to reset the IP outside of -probe() ? I don't oppose to the refactoring, but why would we ever need to reset this IP ? Usually, we need to below reset controller at below situations: - Role switch, host-device and device-host - Return from hibernation, which the controller will lost its power, and the content of the register are lost. resetting is probably not the best solution in either case, though. It might be the easiest, but not the best. For suspend/resume, you would be better off saving/restoring contenxt and for role switch, you could just reprogram the few registers which need to change when role swapping. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 1/3] usb: chipidea: add controller reset API
Hi,
On Fri, Nov 14, 2014 at 08:03:15AM +0800, Peter Chen wrote:
Add controller reset API, it may be used for host/otg driver in future.
you need a better commit log here. How would it be used ? when ? And,
more importantly, why ?
Why would we need to reset the IP outside of -probe() ?
I don't oppose to the refactoring, but why would we ever need to reset
this IP ?
Signed-off-by: Peter Chen peter.c...@freescale.com
---
Changes for v2:
- Add return value check for controller reset at hw_device_reset
drivers/usb/chipidea/core.c | 30 +++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index bd74f27..dffd89b 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -385,6 +385,26 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
}
/**
+ * hw_controller_reset: do controller reset
+ * @ci: the controller
+ *
minor nit: indentation
+ * This function returns an error code
+ */
+static int hw_controller_reset(struct ci_hdrc *ci)
+{
+ int count = 0;
+
+ hw_write(ci, OP_USBCMD, USBCMD_RST, USBCMD_RST);
+ while (hw_read(ci, OP_USBCMD, USBCMD_RST)) {
+ udelay(10);
+ if (count++ 1000)
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+/**
* hw_device_reset: resets chip (execute without interruption)
* @ci: the controller
*
@@ -392,13 +412,17 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
*/
int hw_device_reset(struct ci_hdrc *ci, u32 mode)
{
+ int ret;
+
/* should flush stop before reset */
hw_write(ci, OP_ENDPTFLUSH, ~0, ~0);
hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
- hw_write(ci, OP_USBCMD, USBCMD_RST, USBCMD_RST);
- while (hw_read(ci, OP_USBCMD, USBCMD_RST))
- udelay(10); /* not RTOS friendly */
+ ret = hw_controller_reset(ci);
+ if (ret) {
+ dev_err(ci-dev, error for reset, ret=%d\n, ret);
+ return ret;
+ }
if (ci-platdata-notify_event)
ci-platdata-notify_event(ci,
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
balbi
signature.asc
Description: Digital signature
