On 10/09/15 12:28, Li Jun wrote:
> On Wed, Sep 09, 2015 at 01:01:14PM +0300, Roger Quadros wrote:
> ... ...
>
>>>>>> + return -EINVAL;
>>>>>
>>>>> Return non-zero, then if err, do we need call usb_otg_add_hcd() after
>>>>> usb_otg_register_hcd() fails?
>>>>
>>>> You should not call usb_otg_register_hcd() but usb_add_hcd().
>>>> If that fails then you fail as ususal.
>>>
>>> My point is if we use usb_add_hcd(), but failed in usb_otg_register_hcd(),
>>> then usb_otg_add_hcd() will be called in *all* error case, is this your
>>> expectation?
>>> if (usb_otg_register_hcd(hcd, irqnum, irqflags, &otg_hcd_intf))
>>> return usb_otg_add_hcd(hcd, irqnum, irqflags);
>>>
>>
>> Yes, my intention was that if otg fails then it is a non otg HCD so register normally.
>> Let me correct my previous statement. If you are absolutely sure
>> that the HCD is for otg/dual-role usage then you should call usb_otg_register_hcd().
>>
>
> I think this is not just about a statement, in your usb_otg_register_hcd()
> implementation, there are several places will return error, I think only
> the first two are for a non-otg HCD case, the following error cases seems
> mean this is for otg usage, but it fails in middle of registration, if that
> is the case, is it reasonable to call usb_otg_add_hcd()?
OK. We need to check the return value then and differentiate if it is non-otg
or otg with failure.
If it is non-otg then only we must call usb_otg_add_hcd().
I will fix usb_add_hcd().
>
>>>>>> + * @fsm_running: state machine running/stopped indicator
>>>>>> + */
>>>>>> struct usb_otg {
>>>>>> u8 default_a;
>>>>>>
>>>>>> struct phy *phy;
>>>>>> /* old usb_phy interface */
>>>>>> struct usb_phy *usb_phy;
>>>>>> +
>>>>>
>>>>> add a blank line?
>>>>>
>>>
>>> You missed this.
>>
>> Sorry. Did you suggest to remove that blank line
>> or add a new one before usb_phy?
>>
>
> Remove it.
OK.
>
>>>
>>>>>> struct usb_bus *host;
>>>>>> struct usb_gadget *gadget;
>>>>>>
>>>>>> enum usb_otg_state state;
>>>>>>
>>>>>> + struct device *dev;
>>>>>> + struct usb_otg_caps *caps;
>>>>>> + struct otg_fsm fsm;
>>>>>> + struct otg_fsm_ops fsm_ops;
>>>>>> +
>>>>>> + /* internal use only */
>>>>>> + struct otg_hcd primary_hcd;
>>>>>> + struct otg_hcd shared_hcd;
>>>>>> + struct otg_gadget_ops *gadget_ops;
>>>>>> + struct otg_timer timers[NUM_OTG_FSM_TIMERS];
>>>>>> + struct list_head list;
>>>>>> + struct work_struct work;
>>>>>> --
>>>>>> 2.1.4
>>>
--
cheers,
-roger
--
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
