On Mon, Dec 13, 2010 at 10:05:38AM +0200, Heikki Krogerus wrote:
On Fri, Dec 10, 2010 at 05:40:44PM +0100, ext Hao Wu wrote:This patch adds a unified otg_notify function to otg_transceiver data structure. It is used for USB host/peripheral to notify otg related event to otg_transceiver. Signed-off-by: Hao Wu <hao.wu@xxxxxxxxx> --- include/linux/usb/otg.h | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 42 insertions(+), 0 deletions(-) diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h index 0a5b371..97292ab 100644 --- a/include/linux/usb/otg.h +++ b/include/linux/usb/otg.h @@ -35,6 +35,29 @@ enum usb_otg_state { OTG_STATE_A_VBUS_ERR, }; +enum usb_otg_events { + /* according to OTG Spec */ + USB_OTG_CONNECT, /* a_conn/b_conn */ + USB_OTG_DISCON, + USB_OTG_HOST_SUSP, /* bus request */ + USB_OTG_HOST_RESU, + USB_OTG_DEV_SUSP, /* bus suspend/resume event */ + USB_OTG_DEV_RESU, + + /* others */ + USB_OTG_HOST_ACTIVE, /* host driver added/active */ + USB_OTG_HOST_STOP, /* host driver removed/stopped */ + USB_OTG_DEV_ACTIVE, /* udc driver added/active */ + USB_OTG_DEV_STOP, /* udc driver removed/stopped */ + + /* Battery charging Spec 1.1, requires different charging + * current in FullSpeed mode/HighSpeed mode with CDP, in + * such case, transceiver needs these events to update the + * current limitation */ + USB_OTG_DEV_FS, /* enter FS */ + USB_OTG_DEV_HS, /* enter HS */I have already advised you guys to make the charger detection part of power_supply. If you separate it, there is no need for this kind of details in otg..
that's a never ending fight: should USB know about power supply ? should power supply know about USB ? IMO, we need a drivers/usb/charging kinda thing grouping all of those.
+}; + enum usb_xceiv_events { USB_EVENT_NONE, /* no events or cable disconnected */ USB_EVENT_VBUS, /* vbus valid event */ @@ -110,6 +133,9 @@ struct otg_transceiver { /* start or continue HNP role switch */ int (*start_hnp)(struct otg_transceiver *otg); + /* OTG events notifications to transceiver */ + int (*otg_notify)(struct otg_transceiver *otg, + enum usb_otg_events); }; @@ -230,6 +256,22 @@ otg_start_srp(struct otg_transceiver *otg) return otg->start_srp(otg); } +static inline int +otg_notify_event(enum usb_otg_events event) +{ + struct otg_transceiver *otg; + int retval = 0; + + otg = otg_get_transceiver(); + + if (otg && otg->otg_notify) + retval = otg->otg_notify(otg, event); + + otg_put_transceiver(otg);Do not do it like this. If no driver has claimed (with otg_get_transceiver()) the otg when this is called, the reference counter goes to 0 and you risk unwanted destruction of the transceiver. Please do not expect that the controller will be the only driver that needs struct otg_transceiver. My advice is to use the existing notifiers. If they are not quite what you want, first try to make them work for you. If it's a problem that they are blocking and not atomic, then make them atomic. Don't try to first bring something totally new. To me it seems like you are expecting that the charger detection will be done always by the transceiver. Do not think this way. Omap twl5031-bcc for example is not part of the transceiver. You are clearly not thinking about other drivers in general with this kind of solutions.
Agree with you here. We know the mess it was to come up with a cool approach with three (or 4) separate chips involved in charger detection: . musb . twl4030-usb . twl5031-bcc . bq24150 that's really a mess but as a starting point: DO NOT power up link until you're sure you have finished charger detection. Also, put transceiver to suspended NON-DRIVING mode to avoid it trying to update anything on the link via RxCMDs. -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
