On 17/07/15 12:02, Li Jun wrote:
> On Wed, Jul 08, 2015 at 01:19:36PM +0300, Roger Quadros wrote:
>
> [...]
>
>> struct otg_fsm *usb_otg_register(struct device *parent_dev,
>> - struct otg_fsm_ops *fsm_ops)
>> + struct otg_fsm_ops *fsm_ops,
>> + bool drd_only)
>> {
>> struct otg_data *otgd;
>> int ret = 0;
>> @@ -328,7 +482,15 @@ struct otg_fsm *usb_otg_register(struct device
>> *parent_dev,
>> goto err_wq;
>> }
>>
>> - usb_otg_init_timers(otgd);
>> + otgd->drd_only = drd_only;
>> + /* For DRD mode we don't need OTG timers */
>> + if (!drd_only) {
>> + usb_otg_init_timers(otgd);
>> +
>> + /* FIXME: we ignore caller's timer ops */
>> + otgd->fsm_ops.add_timer = usb_otg_add_timer;
>> + otgd->fsm_ops.del_timer = usb_otg_del_timer;
>> + }
>>
>> /* save original start host/gadget ops */
>> otgd->start_host = fsm_ops->start_host;
>> @@ -338,9 +500,6 @@ struct otg_fsm *usb_otg_register(struct device
>> *parent_dev,
>
> Your above override will be override back to be fsm_ops's by below copy:
>
> /* create copy of original ops */
> otgd->fsm_ops = *fsm_ops;
>
> So add/del_timer must be override after the copy.
Good catch :).
>
>> /* override ops */
>> otgd->fsm_ops.start_host = usb_otg_start_host;
>> otgd->fsm_ops.start_gadget = usb_otg_start_gadget;
>> - /* FIXME: we ignore caller's timer ops */
>> - otgd->fsm_ops.add_timer = usb_otg_add_timer;
>> - otgd->fsm_ops.del_timer = usb_otg_del_timer;
>> /* set otg ops */
>> otgd->fsm.ops = &otgd->fsm_ops;
>> otgd->fsm.otg = &otgd->otg;
>> @@ -443,8 +602,10 @@ static void usb_otg_stop_fsm(struct otg_fsm *fsm)
>> otgd->fsm_running = false;
>>
>> /* Stop state machine / timers */
>> - for (i = 0; i < ARRAY_SIZE(otgd->timers); i++)
>> - hrtimer_cancel(&otgd->timers[i].timer);
>> + if (!otgd->drd_only) {
>> + for (i = 0; i < ARRAY_SIZE(otgd->timers); i++)
>> + hrtimer_cancel(&otgd->timers[i].timer);
>> + }
>>
>> flush_workqueue(otgd->wq);
>> fsm->otg->state = OTG_STATE_UNDEFINED;
>> diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
>> index 22d8baa..ae9c30a 100644
>> --- a/include/linux/usb/otg-fsm.h
>> +++ b/include/linux/usb/otg-fsm.h
>> @@ -48,6 +48,11 @@ enum otg_fsm_timer {
>> /**
>> * struct otg_fsm - OTG state machine according to the OTG spec
>> *
>> + * DRD mode hardware Inputs
>> + *
>> + * @id: TRUE for B-device, FALSE for A-device.
>> + * @vbus: VBUS voltage in regulation.
>> + *
>> * OTG hardware Inputs
>> *
>> * Common inputs for A and B device
>> @@ -122,7 +127,8 @@ enum otg_fsm_timer {
>> */
>> struct otg_fsm {
>> /* Input */
>> - int id;
>> + int id; /* DRD + OTG */
>> + int vbus; /* DRD only */
>
> Existing b_sess_vld can be also used for drd only case, no need create
> a new flag.
b_sess_vld is a bit confusing to people not familiar with OTG.
My suggestion is to use dedicated 'vbus' flag for DRD case
for simplicity.
>
>> int adp_change;
>> int power_up;
>> int a_srp_det;
cheers,
-roger
--
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
