>> +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..
Hi Heikki
Thanks for your valuable comments.
The charger detection part is really hard to separate out, because it happens when OTG state moves
from b_idle to b_peripheral, detect DCP / SDP / CDP needs actions on D+/D-, so if we move this part out of USB transceiver driver.
We have more work on synchronization for USB bus actions.
For power supply, I think it is another topic. We needs these events notification for SDP / CDP cases even we move to use power supply,
USB stack negotiation on charging current is needed, but they only can be get from USB udc driver and gadget driver.
>
>> +};
>> +
>> 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.
I will investigate more here, actually I did not expect that this solution
will be useful to all implementation but I must ensure it is harmless to
exist ones in the first step.
I have a question here, as you mentioned,
if any possible to add otg_get_transceiver() to those drivers which did
not claim the otg but need to use 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.
Actually I did not want to break anything here. A lot of things(usbdev/mon/usbfs/...)
use the blocking one, i prefer keep them not touched. so I have to use something different.
>
>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.
As I mentioned above, I did not expect this solution will be fitted into all implementation.
But I will spend more time to check the omap implementation. Thanks again for your comment.
Hao
--
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
