Re: [PATCH 2/3] usb: chipidea: parameter 'mode' isn't needed for hw_device_reset
On Fri, Nov 14, 2014 at 09:05:16AM -0600, Felipe Balbi wrote:
Hi,
On Fri, Nov 14, 2014 at 03:40:55PM +0800, Peter Chen wrote:
@@ -1660,7 +1660,7 @@ static int ci_udc_start(struct usb_gadget *gadget,
pm_runtime_get_sync(ci-gadget.dev);
if (ci-vbus_active) {
spin_lock_irqsave(ci-lock, flags);
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci);
wow, you guys reset this IP all the time... that's scary, could be
hiding bugs easily.
In current chipidea design, it indeed resets controller often, it
resets controller when the usb cable connects to pc and the gadget
driver is there, something like which we do for RUN/STOP bit.
RUN/STOP on dwc3 is not an IP reset though. Neither it is on XHCI/EHCI.
Of cos, I mean the situation which the chipidea does reset controller
is not necessary sometimes, it can control RUN/STOP bit instead of it.
--
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 2/3] usb: chipidea: parameter 'mode' isn't needed for hw_device_reset
On Mon, Nov 17, 2014 at 10:11:38AM +0800, Peter Chen wrote:
On Fri, Nov 14, 2014 at 09:05:16AM -0600, Felipe Balbi wrote:
Hi,
On Fri, Nov 14, 2014 at 03:40:55PM +0800, Peter Chen wrote:
@@ -1660,7 +1660,7 @@ static int ci_udc_start(struct usb_gadget
*gadget,
pm_runtime_get_sync(ci-gadget.dev);
if (ci-vbus_active) {
spin_lock_irqsave(ci-lock, flags);
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci);
wow, you guys reset this IP all the time... that's scary, could be
hiding bugs easily.
In current chipidea design, it indeed resets controller often, it
resets controller when the usb cable connects to pc and the gadget
driver is there, something like which we do for RUN/STOP bit.
RUN/STOP on dwc3 is not an IP reset though. Neither it is on XHCI/EHCI.
Of cos, I mean the situation which the chipidea does reset controller
is not necessary sometimes, it can control RUN/STOP bit instead of it.
got it now, that would be a very welcome optimization.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 2/3] usb: chipidea: parameter 'mode' isn't needed for hw_device_reset
On Thu, Nov 13, 2014 at 09:56:58PM -0600, Felipe Balbi wrote:
On Fri, Nov 14, 2014 at 08:03:16AM +0800, Peter Chen wrote:
The hw_device_reset is dedicated to be used at device mode initializaiton,
so delete the parameter 'mode'. For host driver, the ehci driver will
all things.
this last sentence doesn't parse very well.
Thanks, will add.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/chipidea/ci.h | 2 +-
drivers/usb/chipidea/core.c| 8
drivers/usb/chipidea/otg_fsm.c | 2 +-
drivers/usb/chipidea/udc.c | 4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 5bbfcc7..65913d4 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -352,7 +352,7 @@ u32 hw_read_intr_enable(struct ci_hdrc *ci);
u32 hw_read_intr_status(struct ci_hdrc *ci);
-int hw_device_reset(struct ci_hdrc *ci, u32 mode);
+int hw_device_reset(struct ci_hdrc *ci);
int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index dffd89b..053ab4f 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -410,7 +410,7 @@ static int hw_controller_reset(struct ci_hdrc *ci)
*
* This function returns an error code
*/
-int hw_device_reset(struct ci_hdrc *ci, u32 mode)
+int hw_device_reset(struct ci_hdrc *ci)
{
int ret;
@@ -440,12 +440,12 @@ int hw_device_reset(struct ci_hdrc *ci, u32 mode)
/* USBMODE should be configured step by step */
hw_write(ci, OP_USBMODE, USBMODE_CM, USBMODE_CM_IDLE);
- hw_write(ci, OP_USBMODE, USBMODE_CM, mode);
+ hw_write(ci, OP_USBMODE, USBMODE_CM, USBMODE_CM_DC);
/* HW = 2.3 */
hw_write(ci, OP_USBMODE, USBMODE_SLOM, USBMODE_SLOM);
- if (hw_read(ci, OP_USBMODE, USBMODE_CM) != mode) {
- pr_err(cannot enter in %s mode, ci_role(ci)-name);
+ if (hw_read(ci, OP_USBMODE, USBMODE_CM) != USBMODE_CM_DC) {
+ pr_err(cannot enter in %s device mode, ci_role(ci)-name);
pr_err(lpm = %i, ci-hw_bank.lpm);
return -ENODEV;
}
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 21058ae..e8c936d 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -543,7 +543,7 @@ static int ci_otg_start_host(struct otg_fsm *fsm, int
on)
ci_role_start(ci, CI_ROLE_HOST);
} else {
ci_role_stop(ci);
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci);
ci_role_start(ci, CI_ROLE_GADGET);
}
mutex_lock(fsm-lock);
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index bdaa7ba..4fe18ce 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1471,7 +1471,7 @@ static int ci_udc_vbus_session(struct usb_gadget
*_gadget, int is_active)
if (gadget_ready) {
if (is_active) {
pm_runtime_get_sync(_gadget-dev);
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci);
hw_device_state(ci, ci-ep0out-qh.dma);
usb_gadget_set_state(_gadget, USB_STATE_POWERED);
} else {
@@ -1660,7 +1660,7 @@ static int ci_udc_start(struct usb_gadget *gadget,
pm_runtime_get_sync(ci-gadget.dev);
if (ci-vbus_active) {
spin_lock_irqsave(ci-lock, flags);
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci);
wow, you guys reset this IP all the time... that's scary, could be
hiding bugs easily.
In current chipidea design, it indeed resets controller often, it
resets controller when the usb cable connects to pc and the gadget
driver is there, something like which we do for RUN/STOP bit.
--
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 2/3] usb: chipidea: parameter 'mode' isn't needed for hw_device_reset
Hi,
On Fri, Nov 14, 2014 at 03:40:55PM +0800, Peter Chen wrote:
@@ -1660,7 +1660,7 @@ static int ci_udc_start(struct usb_gadget *gadget,
pm_runtime_get_sync(ci-gadget.dev);
if (ci-vbus_active) {
spin_lock_irqsave(ci-lock, flags);
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci);
wow, you guys reset this IP all the time... that's scary, could be
hiding bugs easily.
In current chipidea design, it indeed resets controller often, it
resets controller when the usb cable connects to pc and the gadget
driver is there, something like which we do for RUN/STOP bit.
RUN/STOP on dwc3 is not an IP reset though. Neither it is on XHCI/EHCI.
--
balbi
signature.asc
Description: Digital signature
[PATCH 2/3] usb: chipidea: parameter 'mode' isn't needed for hw_device_reset
The hw_device_reset is dedicated to be used at device mode initializaiton,
so delete the parameter 'mode'. For host driver, the ehci driver will
all things.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/chipidea/ci.h | 2 +-
drivers/usb/chipidea/core.c| 8
drivers/usb/chipidea/otg_fsm.c | 2 +-
drivers/usb/chipidea/udc.c | 4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 5bbfcc7..65913d4 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -352,7 +352,7 @@ u32 hw_read_intr_enable(struct ci_hdrc *ci);
u32 hw_read_intr_status(struct ci_hdrc *ci);
-int hw_device_reset(struct ci_hdrc *ci, u32 mode);
+int hw_device_reset(struct ci_hdrc *ci);
int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index dffd89b..053ab4f 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -410,7 +410,7 @@ static int hw_controller_reset(struct ci_hdrc *ci)
*
* This function returns an error code
*/
-int hw_device_reset(struct ci_hdrc *ci, u32 mode)
+int hw_device_reset(struct ci_hdrc *ci)
{
int ret;
@@ -440,12 +440,12 @@ int hw_device_reset(struct ci_hdrc *ci, u32 mode)
/* USBMODE should be configured step by step */
hw_write(ci, OP_USBMODE, USBMODE_CM, USBMODE_CM_IDLE);
- hw_write(ci, OP_USBMODE, USBMODE_CM, mode);
+ hw_write(ci, OP_USBMODE, USBMODE_CM, USBMODE_CM_DC);
/* HW = 2.3 */
hw_write(ci, OP_USBMODE, USBMODE_SLOM, USBMODE_SLOM);
- if (hw_read(ci, OP_USBMODE, USBMODE_CM) != mode) {
- pr_err(cannot enter in %s mode, ci_role(ci)-name);
+ if (hw_read(ci, OP_USBMODE, USBMODE_CM) != USBMODE_CM_DC) {
+ pr_err(cannot enter in %s device mode, ci_role(ci)-name);
pr_err(lpm = %i, ci-hw_bank.lpm);
return -ENODEV;
}
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 21058ae..e8c936d 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -543,7 +543,7 @@ static int ci_otg_start_host(struct otg_fsm *fsm, int on)
ci_role_start(ci, CI_ROLE_HOST);
} else {
ci_role_stop(ci);
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci);
ci_role_start(ci, CI_ROLE_GADGET);
}
mutex_lock(fsm-lock);
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index bdaa7ba..4fe18ce 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1471,7 +1471,7 @@ static int ci_udc_vbus_session(struct usb_gadget
*_gadget, int is_active)
if (gadget_ready) {
if (is_active) {
pm_runtime_get_sync(_gadget-dev);
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci);
hw_device_state(ci, ci-ep0out-qh.dma);
usb_gadget_set_state(_gadget, USB_STATE_POWERED);
} else {
@@ -1660,7 +1660,7 @@ static int ci_udc_start(struct usb_gadget *gadget,
pm_runtime_get_sync(ci-gadget.dev);
if (ci-vbus_active) {
spin_lock_irqsave(ci-lock, flags);
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci);
} else {
pm_runtime_put_sync(ci-gadget.dev);
return retval;
--
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
Re: [PATCH 2/3] usb: chipidea: parameter 'mode' isn't needed for hw_device_reset
On Fri, Nov 14, 2014 at 08:03:16AM +0800, Peter Chen wrote:
The hw_device_reset is dedicated to be used at device mode initializaiton,
so delete the parameter 'mode'. For host driver, the ehci driver will
all things.
this last sentence doesn't parse very well.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/chipidea/ci.h | 2 +-
drivers/usb/chipidea/core.c| 8
drivers/usb/chipidea/otg_fsm.c | 2 +-
drivers/usb/chipidea/udc.c | 4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 5bbfcc7..65913d4 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -352,7 +352,7 @@ u32 hw_read_intr_enable(struct ci_hdrc *ci);
u32 hw_read_intr_status(struct ci_hdrc *ci);
-int hw_device_reset(struct ci_hdrc *ci, u32 mode);
+int hw_device_reset(struct ci_hdrc *ci);
int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index dffd89b..053ab4f 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -410,7 +410,7 @@ static int hw_controller_reset(struct ci_hdrc *ci)
*
* This function returns an error code
*/
-int hw_device_reset(struct ci_hdrc *ci, u32 mode)
+int hw_device_reset(struct ci_hdrc *ci)
{
int ret;
@@ -440,12 +440,12 @@ int hw_device_reset(struct ci_hdrc *ci, u32 mode)
/* USBMODE should be configured step by step */
hw_write(ci, OP_USBMODE, USBMODE_CM, USBMODE_CM_IDLE);
- hw_write(ci, OP_USBMODE, USBMODE_CM, mode);
+ hw_write(ci, OP_USBMODE, USBMODE_CM, USBMODE_CM_DC);
/* HW = 2.3 */
hw_write(ci, OP_USBMODE, USBMODE_SLOM, USBMODE_SLOM);
- if (hw_read(ci, OP_USBMODE, USBMODE_CM) != mode) {
- pr_err(cannot enter in %s mode, ci_role(ci)-name);
+ if (hw_read(ci, OP_USBMODE, USBMODE_CM) != USBMODE_CM_DC) {
+ pr_err(cannot enter in %s device mode, ci_role(ci)-name);
pr_err(lpm = %i, ci-hw_bank.lpm);
return -ENODEV;
}
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 21058ae..e8c936d 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -543,7 +543,7 @@ static int ci_otg_start_host(struct otg_fsm *fsm, int on)
ci_role_start(ci, CI_ROLE_HOST);
} else {
ci_role_stop(ci);
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci);
ci_role_start(ci, CI_ROLE_GADGET);
}
mutex_lock(fsm-lock);
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index bdaa7ba..4fe18ce 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1471,7 +1471,7 @@ static int ci_udc_vbus_session(struct usb_gadget
*_gadget, int is_active)
if (gadget_ready) {
if (is_active) {
pm_runtime_get_sync(_gadget-dev);
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci);
hw_device_state(ci, ci-ep0out-qh.dma);
usb_gadget_set_state(_gadget, USB_STATE_POWERED);
} else {
@@ -1660,7 +1660,7 @@ static int ci_udc_start(struct usb_gadget *gadget,
pm_runtime_get_sync(ci-gadget.dev);
if (ci-vbus_active) {
spin_lock_irqsave(ci-lock, flags);
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci);
wow, you guys reset this IP all the time... that's scary, could be
hiding bugs easily.
--
balbi
signature.asc
Description: Digital signature
