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..
> +};
> +
> 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.
--
heikki
--
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
