Re: [RESEND PATCH v5 4/7] usb: chipidea: consolidate ci_role_driver's API for both roles
On Tue, Jan 29, 2013 at 11:37:22AM +0200, Alexander Shishkin wrote: > Peter Chen writes: > > > On Fri, Jan 25, 2013 at 02:12:20PM +0200, Alexander Shishkin wrote: > >> Peter Chen writes: > >> > >> > On Thu, Jan 24, 2013 at 04:35:30PM +0200, Alexander Shishkin wrote: > >> >> Peter Chen writes: > >> >> > >> >> > - Create init/destroy API for probe and remove > >> >> > - start/stop API are only used otg id switch process > >> >> > - Create the gadget at ci_hdrc_probe if the gadget is supported > >> >> > at that port, the main purpose for this is to avoid gadget module > >> >> > load fail at init.rc > >> >> > >> @@ -402,6 +402,12 @@ static ssize_t store_role(struct device *dev, struct > >> device_attribute *attr, > >>if (ret) > >>return ret; > >> > >> + /* > >> + * there won't be an interrupt in case of manual switching, > >> + * so we need to check vbus session manually > >> + */ > >> + ci_handle_vbus_change(ci); > >> + > > It may not be used as there will be a vbus interrupt. > > Not if you write gadget to "role" file. Let me see, we will only use it for standard-A receptacle port soldered at OTG port, there is no ID manual switch, and ID pin is grounded. When the user wants to use it at gadget mode, eg, update image. 1. Plug cable, then write "gadget" to "role" file It will work as there will be ci_handle_vbus_change(ci) at role_start, just like we insmod gadget mode when the cable is connecting to host. 2. Write "gadget" to "role" file first, then plug cable After stopping host, it should close vbus. After gadget role starts, it will enable vbus interrupt. Then, when we plug in A-to-A cable, there will be a vbus interrupt, then active gadget. > >>return -ENOMEM; > >> > >> - rdrv->init = udc_start; > >>rdrv->start = udc_id_switch_for_device; > >>rdrv->stop = udc_id_switch_for_host; > >> - rdrv->destroy = udc_stop; > > Where we call udc_start and udc_stop? And the udc_start should only be > > called > > one time. > > ci_hdrc_gadget_init() and ci_hdrc_gadget_destroy(). Look closer at the > patch. I am sorry, I still not see udc_stop at ci_hdrc_gadget_destroy(). It doesn't matter. I know what you want to do, let me summery - Call udc_start at ci_hdrc_gadget_init, but vbus enable function should only be called if the role is gadget, or vbus interrupt will occur when the host switches gadget (vbus is off), besides, the ci->vbus_active will be set at host mode, and ci13xxx_start will operate register with function hw_device_state(ci, ci->ep0out->qh.dma) if we load module at host mode. Besides, if we set REG_SHARED, it will reset controller. - Create two destroy functions for gadget and host, and call them at ci remove. Anything I forget, or do you think anything else I need to change? If you agree, I will send patch for above change. Do you want this change is on the top of my v5 patch or just a new v6 patch with above change? > > - When the OTG port is at host mode, it should not call any > > register writing operations at gadget's API. > > Furthermore, there shouldn't be any calls to the gadget api. The user may load gadget module when it is at host mode, eg, at their init script. -- 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: [RESEND PATCH v5 4/7] usb: chipidea: consolidate ci_role_driver's API for both roles
Peter Chen writes: > On Fri, Jan 25, 2013 at 02:12:20PM +0200, Alexander Shishkin wrote: >> Peter Chen writes: >> >> > On Thu, Jan 24, 2013 at 04:35:30PM +0200, Alexander Shishkin wrote: >> >> Peter Chen writes: >> >> >> >> > - Create init/destroy API for probe and remove >> >> > - start/stop API are only used otg id switch process >> >> > - Create the gadget at ci_hdrc_probe if the gadget is supported >> >> > at that port, the main purpose for this is to avoid gadget module >> >> > load fail at init.rc >> >> >> @@ -402,6 +402,12 @@ static ssize_t store_role(struct device *dev, struct >> device_attribute *attr, >> if (ret) >> return ret; >> >> +/* >> + * there won't be an interrupt in case of manual switching, >> + * so we need to check vbus session manually >> + */ >> +ci_handle_vbus_change(ci); >> + > It may not be used as there will be a vbus interrupt. Not if you write gadget to "role" file. >> return count; >> } >> >> } >> dbg_remove_files(&ci->gadget.dev); >> device_unregister(&ci->gadget.dev); >> -/* my kobject is dynamic, I swear! */ >> -memset(&ci->gadget, 0, sizeof(ci->gadget)); > Any reason to delete it, another fix? It is currently like this because start and stop functions are called multiple times during the devices lifetime. This patch converts the driver to only call start at driver's probe() and stop at driver's remove(). >> } >> >> /** >> @@ -1842,13 +1839,11 @@ int ci_hdrc_gadget_init(struct ci13xxx *ci) >> if (!rdrv) >> return -ENOMEM; >> >> -rdrv->init = udc_start; >> rdrv->start = udc_id_switch_for_device; >> rdrv->stop = udc_id_switch_for_host; >> -rdrv->destroy = udc_stop; > Where we call udc_start and udc_stop? And the udc_start should only be called > one time. ci_hdrc_gadget_init() and ci_hdrc_gadget_destroy(). Look closer at the patch. >> >> Which is shorter (32 insertions(+), 53 deletions(-)) and makes the main >> probe easier on the eyes. What do you think? > OK, not matter how to change it, it needs to cover below things: > (Covers last email) > - Gadget needs to be only initialized one time. Yes, that's what we agreed on and that's what I'm suggesting in the above patch. > - Host/device function should be OK when the device on it or > the usb cable connects to host during the boots up. Should still work. > - When the OTG port is at host mode, it should not call any > register writing operations at gadget's API. Furthermore, there shouldn't be any calls to the gadget api. Regards, -- Alex -- 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: [RESEND PATCH v5 4/7] usb: chipidea: consolidate ci_role_driver's API for both roles
On Fri, Jan 25, 2013 at 02:12:20PM +0200, Alexander Shishkin wrote: > Peter Chen writes: > > > On Thu, Jan 24, 2013 at 04:35:30PM +0200, Alexander Shishkin wrote: > >> Peter Chen writes: > >> > >> > - Create init/destroy API for probe and remove > >> > - start/stop API are only used otg id switch process > >> > - Create the gadget at ci_hdrc_probe if the gadget is supported > >> > at that port, the main purpose for this is to avoid gadget module > >> > load fail at init.rc > >> > @@ -402,6 +402,12 @@ static ssize_t store_role(struct device *dev, struct > device_attribute *attr, > if (ret) > return ret; > > + /* > + * there won't be an interrupt in case of manual switching, > + * so we need to check vbus session manually > + */ > + ci_handle_vbus_change(ci); > + It may not be used as there will be a vbus interrupt. > return count; > } > > } > dbg_remove_files(&ci->gadget.dev); > device_unregister(&ci->gadget.dev); > - /* my kobject is dynamic, I swear! */ > - memset(&ci->gadget, 0, sizeof(ci->gadget)); Any reason to delete it, another fix? > } > > /** > @@ -1842,13 +1839,11 @@ int ci_hdrc_gadget_init(struct ci13xxx *ci) > if (!rdrv) > return -ENOMEM; > > - rdrv->init = udc_start; > rdrv->start = udc_id_switch_for_device; > rdrv->stop = udc_id_switch_for_host; > - rdrv->destroy = udc_stop; Where we call udc_start and udc_stop? And the udc_start should only be called one time. > > Which is shorter (32 insertions(+), 53 deletions(-)) and makes the main > probe easier on the eyes. What do you think? OK, not matter how to change it, it needs to cover below things: (Covers last email) - Gadget needs to be only initialized one time. - Host/device function should be OK when the device on it or the usb cable connects to host during the boots up. - When the OTG port is at host mode, it should not call any register writing operations at gadget's API. -- 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: [RESEND PATCH v5 4/7] usb: chipidea: consolidate ci_role_driver's API for both roles
Peter Chen writes: > On Thu, Jan 24, 2013 at 04:35:30PM +0200, Alexander Shishkin wrote: >> Peter Chen writes: >> >> > - Create init/destroy API for probe and remove >> > - start/stop API are only used otg id switch process >> > - Create the gadget at ci_hdrc_probe if the gadget is supported >> > at that port, the main purpose for this is to avoid gadget module >> > load fail at init.rc >> >> I don't think it's necessary to mention android-specific init scripts >> here in our patches. > > Ok, will just mention init script. >> >> > >> > +static inline void ci_role_destroy(struct ci13xxx *ci) >> > +{ >> > + enum ci_role role = ci->role; >> > + >> > + if (role == CI_ROLE_END) >> > + return; >> > + >> > + ci->role = CI_ROLE_END; >> > + >> > + ci->roles[role]->destroy(ci); >> > +} >> >> What does this do? It should take role an a parameter, at least. Or it >> can be called ci_roles_destroy() and, well, destroy all the roles. Now >> we're in a situation where we destroy one. > Yes, this function has some problems, I suggest just call two lines at > ci_role_destory, do you think so? > > ci->roles[role]->destroy(ci); > ci->role = CI_ROLE_END; I just don't see why it's needed at all. See below. >> >> The idea is that, with this api, we can (and should) have both roles >> allocated all the time, but only one role start()'ed. >> >> What we can do here is move the udc's .init() code to >> ci_hdrc_gadget_init() and add a complementary ci_hdrc_gadget_destroy(), >> which we call in ci_hdrc_remove() and probe's error path. And we can >> probably do something similar for the host, or just leave it as it is >> now. Seems simpler to me. > Yes, current code has bug that it should call ci_role_destroy at probe's > error path. > For your comments, it still needs to add destory function for udc which will > be called at core.c. I still prefer current way due to below reasons: > > - It can keep ci_hdrc_gadget_init() clean > - .init and .remove are different logical function with .start and .stop. > The .init and .remove are used for create and destroy, .start and .stop > are used at id role switch. True, and we can keep it that way: (again, untested) ---cut--- diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index 342b430..36f495a 100644 --- a/drivers/usb/chipidea/ci.h +++ b/drivers/usb/chipidea/ci.h @@ -67,18 +67,14 @@ enum ci_role { /** * struct ci_role_driver - host/gadget role driver - * init: init this role (used at module probe) * start: start this role (used at id switch) * stop: stop this role (used at id switch) - * destroy: destroy this role (used at module remove) * irq: irq handler for this role * name: role name string (host/gadget) */ struct ci_role_driver { - int (*init)(struct ci13xxx *); int (*start)(struct ci13xxx *); void(*stop)(struct ci13xxx *); - void(*destroy)(struct ci13xxx *); irqreturn_t (*irq)(struct ci13xxx *); const char *name; }; @@ -212,17 +208,6 @@ static inline void ci_role_stop(struct ci13xxx *ci) ci->roles[role]->stop(ci); } -static inline void ci_role_destroy(struct ci13xxx *ci) -{ - enum ci_role role = ci->role; - - if (role == CI_ROLE_END) - return; - - ci->role = CI_ROLE_END; - - ci->roles[role]->destroy(ci); -} /** * REGISTERS */ diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index a61e759..83f35fa 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -402,6 +402,12 @@ static ssize_t store_role(struct device *dev, struct device_attribute *attr, if (ret) return ret; + /* +* there won't be an interrupt in case of manual switching, +* so we need to check vbus session manually +*/ + ci_handle_vbus_change(ci); + return count; } @@ -592,30 +598,6 @@ static int ci_hdrc_probe(struct platform_device *pdev) otgsc = hw_read(ci, OP_OTGSC, ~0); - /* -* If the gadget is supported, call its init unconditionally, -* We need to support load gadget module at init.rc. -*/ - if (ci->roles[CI_ROLE_GADGET]) { - ret = ci->roles[CI_ROLE_GADGET]->init(ci); - if (ret) { - dev_err(dev, "can't init %s role, ret=%d\n", - ci_role(ci)->name, ret); - ret = -ENODEV; - goto rm_wq; - } - } - - if (ci->role == CI_ROLE_HOST) { - ret = ci->roles[CI_ROLE_HOST]->init(ci); - if (ret) { - dev_err(dev, "can't init %s role, ret=%d\n", - ci_role(ci)->n
Re: [RESEND PATCH v5 4/7] usb: chipidea: consolidate ci_role_driver's API for both roles
On Thu, Jan 24, 2013 at 04:35:30PM +0200, Alexander Shishkin wrote: > Peter Chen writes: > > > - Create init/destroy API for probe and remove > > - start/stop API are only used otg id switch process > > - Create the gadget at ci_hdrc_probe if the gadget is supported > > at that port, the main purpose for this is to avoid gadget module > > load fail at init.rc > > I don't think it's necessary to mention android-specific init scripts > here in our patches. Ok, will just mention init script. > > > > > +static inline void ci_role_destroy(struct ci13xxx *ci) > > +{ > > + enum ci_role role = ci->role; > > + > > + if (role == CI_ROLE_END) > > + return; > > + > > + ci->role = CI_ROLE_END; > > + > > + ci->roles[role]->destroy(ci); > > +} > > What does this do? It should take role an a parameter, at least. Or it > can be called ci_roles_destroy() and, well, destroy all the roles. Now > we're in a situation where we destroy one. Yes, this function has some problems, I suggest just call two lines at ci_role_destory, do you think so? ci->roles[role]->destroy(ci); ci->role = CI_ROLE_END; > > The idea is that, with this api, we can (and should) have both roles > allocated all the time, but only one role start()'ed. > > What we can do here is move the udc's .init() code to > ci_hdrc_gadget_init() and add a complementary ci_hdrc_gadget_destroy(), > which we call in ci_hdrc_remove() and probe's error path. And we can > probably do something similar for the host, or just leave it as it is > now. Seems simpler to me. Yes, current code has bug that it should call ci_role_destroy at probe's error path. For your comments, it still needs to add destory function for udc which will be called at core.c. I still prefer current way due to below reasons: - It can keep ci_hdrc_gadget_init() clean - .init and .remove are different logical function with .start and .stop. The .init and .remove are used for create and destroy, .start and .stop are used at id role switch. > > } > > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c > > index caecad9..6024a4f 100644 > > --- a/drivers/usb/chipidea/host.c > > +++ b/drivers/usb/chipidea/host.c > > @@ -92,8 +92,10 @@ int ci_hdrc_host_init(struct ci13xxx *ci) > > if (!rdrv) > > return -ENOMEM; > > > > + rdrv->init = host_start; > > rdrv->start = host_start; > > rdrv->stop = host_stop; > > + rdrv->destroy = host_stop; > > Will this allocate the hcd twice if we start in host role? Looks like that. No, if we start host role, the host will be allocated once at probe. host_start is only called when the id value from 1 to 0. > -- 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: [RESEND PATCH v5 4/7] usb: chipidea: consolidate ci_role_driver's API for both roles
Peter Chen writes: > - Create init/destroy API for probe and remove > - start/stop API are only used otg id switch process > - Create the gadget at ci_hdrc_probe if the gadget is supported > at that port, the main purpose for this is to avoid gadget module > load fail at init.rc I don't think it's necessary to mention android-specific init scripts here in our patches. > Signed-off-by: Peter Chen > --- > drivers/usb/chipidea/ci.h | 19 +++- > drivers/usb/chipidea/core.c | 65 ++ > drivers/usb/chipidea/host.c |2 + > drivers/usb/chipidea/udc.c | 33 - > 4 files changed, 78 insertions(+), 41 deletions(-) > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h > index 325d790..00939e6 100644 > --- a/drivers/usb/chipidea/ci.h > +++ b/drivers/usb/chipidea/ci.h > @@ -67,14 +67,18 @@ enum ci_role { > > /** > * struct ci_role_driver - host/gadget role driver > - * start: start this role > - * stop: stop this role > + * init: init this role (used at module probe) > + * start: start this role (used at id switch) > + * stop: stop this role (used at id switch) > + * destroy: destroy this role (used at module remove) > * irq: irq handler for this role > * name: role name string (host/gadget) > */ > struct ci_role_driver { > + int (*init)(struct ci13xxx *); > int (*start)(struct ci13xxx *); > void(*stop)(struct ci13xxx *); > + void(*destroy)(struct ci13xxx *); > irqreturn_t (*irq)(struct ci13xxx *); > const char *name; > }; > @@ -206,6 +210,17 @@ static inline void ci_role_stop(struct ci13xxx *ci) > ci->roles[role]->stop(ci); > } > > +static inline void ci_role_destroy(struct ci13xxx *ci) > +{ > + enum ci_role role = ci->role; > + > + if (role == CI_ROLE_END) > + return; > + > + ci->role = CI_ROLE_END; > + > + ci->roles[role]->destroy(ci); > +} What does this do? It should take role an a parameter, at least. Or it can be called ci_roles_destroy() and, well, destroy all the roles. Now we're in a situation where we destroy one. The idea is that, with this api, we can (and should) have both roles allocated all the time, but only one role start()'ed. What we can do here is move the udc's .init() code to ci_hdrc_gadget_init() and add a complementary ci_hdrc_gadget_destroy(), which we call in ci_hdrc_remove() and probe's error path. And we can probably do something similar for the host, or just leave it as it is now. Seems simpler to me. > > /** > * REGISTERS > > */ > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c > index f8f8484..a5adf1a 100644 > --- a/drivers/usb/chipidea/core.c > +++ b/drivers/usb/chipidea/core.c > @@ -315,27 +315,16 @@ static void ci_handle_id_switch(struct ci13xxx *ci) > ci_role(ci)->name, ci->roles[role]->name); > > /* 1. Finish the current role */ > - if (ci->role == CI_ROLE_GADGET) { > - usb_gadget_vbus_disconnect(&ci->gadget); > - /* host doesn't care B_SESSION_VALID event */ > - ci_clear_otg_interrupt(ci, OTGSC_BSVIS); > - ci_disable_otg_interrupt(ci, OTGSC_BSVIE); > - ci->role = CI_ROLE_END; > - /* reset controller */ > - hw_device_reset(ci, USBMODE_CM_IDLE); > - } else if (ci->role == CI_ROLE_HOST) { > - ci_role_stop(ci); > - /* reset controller */ > - hw_device_reset(ci, USBMODE_CM_IDLE); > - } > + ci_role_stop(ci); > + hw_device_reset(ci, USBMODE_CM_IDLE); > > /* 2. Turn on/off vbus according to coming role */ > - if (ci_otg_role(ci) == CI_ROLE_GADGET) { > + if (role == CI_ROLE_GADGET) { > otg_set_vbus(&ci->otg, false); > /* wait vbus lower than OTGSC_BSV */ > hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0, > CI_VBUS_STABLE_TIMEOUT); > - } else if (ci_otg_role(ci) == CI_ROLE_HOST) { > + } else if (role == CI_ROLE_HOST) { > otg_set_vbus(&ci->otg, true); > /* wait vbus higher than OTGSC_AVV */ > hw_wait_reg(ci, OP_OTGSC, OTGSC_AVV, OTGSC_AVV, > @@ -343,13 +332,7 @@ static void ci_handle_id_switch(struct ci13xxx *ci) > } > > /* 3. Begin the new role */ > - if (ci_otg_role(ci) == CI_ROLE_GADGET) { > - ci->role = role; > - ci_clear_otg_interrupt(ci, OTGSC_BSVIS); > -
[RESEND PATCH v5 4/7] usb: chipidea: consolidate ci_role_driver's API for both roles
- Create init/destroy API for probe and remove - start/stop API are only used otg id switch process - Create the gadget at ci_hdrc_probe if the gadget is supported at that port, the main purpose for this is to avoid gadget module load fail at init.rc Signed-off-by: Peter Chen --- drivers/usb/chipidea/ci.h | 19 +++- drivers/usb/chipidea/core.c | 65 ++ drivers/usb/chipidea/host.c |2 + drivers/usb/chipidea/udc.c | 33 - 4 files changed, 78 insertions(+), 41 deletions(-) diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index 325d790..00939e6 100644 --- a/drivers/usb/chipidea/ci.h +++ b/drivers/usb/chipidea/ci.h @@ -67,14 +67,18 @@ enum ci_role { /** * struct ci_role_driver - host/gadget role driver - * start: start this role - * stop: stop this role + * init: init this role (used at module probe) + * start: start this role (used at id switch) + * stop: stop this role (used at id switch) + * destroy: destroy this role (used at module remove) * irq: irq handler for this role * name: role name string (host/gadget) */ struct ci_role_driver { + int (*init)(struct ci13xxx *); int (*start)(struct ci13xxx *); void(*stop)(struct ci13xxx *); + void(*destroy)(struct ci13xxx *); irqreturn_t (*irq)(struct ci13xxx *); const char *name; }; @@ -206,6 +210,17 @@ static inline void ci_role_stop(struct ci13xxx *ci) ci->roles[role]->stop(ci); } +static inline void ci_role_destroy(struct ci13xxx *ci) +{ + enum ci_role role = ci->role; + + if (role == CI_ROLE_END) + return; + + ci->role = CI_ROLE_END; + + ci->roles[role]->destroy(ci); +} /** * REGISTERS */ diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index f8f8484..a5adf1a 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -315,27 +315,16 @@ static void ci_handle_id_switch(struct ci13xxx *ci) ci_role(ci)->name, ci->roles[role]->name); /* 1. Finish the current role */ - if (ci->role == CI_ROLE_GADGET) { - usb_gadget_vbus_disconnect(&ci->gadget); - /* host doesn't care B_SESSION_VALID event */ - ci_clear_otg_interrupt(ci, OTGSC_BSVIS); - ci_disable_otg_interrupt(ci, OTGSC_BSVIE); - ci->role = CI_ROLE_END; - /* reset controller */ - hw_device_reset(ci, USBMODE_CM_IDLE); - } else if (ci->role == CI_ROLE_HOST) { - ci_role_stop(ci); - /* reset controller */ - hw_device_reset(ci, USBMODE_CM_IDLE); - } + ci_role_stop(ci); + hw_device_reset(ci, USBMODE_CM_IDLE); /* 2. Turn on/off vbus according to coming role */ - if (ci_otg_role(ci) == CI_ROLE_GADGET) { + if (role == CI_ROLE_GADGET) { otg_set_vbus(&ci->otg, false); /* wait vbus lower than OTGSC_BSV */ hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0, CI_VBUS_STABLE_TIMEOUT); - } else if (ci_otg_role(ci) == CI_ROLE_HOST) { + } else if (role == CI_ROLE_HOST) { otg_set_vbus(&ci->otg, true); /* wait vbus higher than OTGSC_AVV */ hw_wait_reg(ci, OP_OTGSC, OTGSC_AVV, OTGSC_AVV, @@ -343,13 +332,7 @@ static void ci_handle_id_switch(struct ci13xxx *ci) } /* 3. Begin the new role */ - if (ci_otg_role(ci) == CI_ROLE_GADGET) { - ci->role = role; - ci_clear_otg_interrupt(ci, OTGSC_BSVIS); - ci_enable_otg_interrupt(ci, OTGSC_BSVIE); - } else if (ci_otg_role(ci) == CI_ROLE_HOST) { - ci_role_start(ci, role); - } + ci_role_start(ci, role); } } @@ -585,7 +568,7 @@ static int ci_hdrc_probe(struct platform_device *pdev) ret = ci_hdrc_gadget_init(ci); if (ret) - dev_info(dev, "doesn't support gadget\n"); + dev_info(dev, "doesn't support gadget, ret=%d\n", ret); if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) { dev_err(dev, "no supported roles\n"); @@ -607,22 +590,30 @@ static int ci_hdrc_probe(struct platform_device *pdev) : CI_ROLE_GADGET; } - ret = ci_role_start(ci, ci->role); - if (ret) { - d