Re: [PATCH v10 13/14] usb: gadget: udc: adapt to OTG core
On Mon, Jun 13, 2016 at 10:37:59AM +0300, Roger Quadros wrote:
> On 13/06/16 10:20, Peter Chen wrote:
> > On Mon, Jun 13, 2016 at 10:14:31AM +0300, Roger Quadros wrote:
> >> On 12/06/16 14:36, Peter Chen wrote:
> >>> On Fri, Jun 10, 2016 at 04:07:22PM +0300, Roger Quadros wrote:
>
> +/**
> + * usb_otg_add_gadget_udc - adds a new gadget to the udc class driver
> list
> + * @parent: the parent device to this udc. Usually the controller
> + * driver's device.
> >>>
> >>> It seems it should be udc device
> >>
> >> Parent and udc->dev are not the same right?
> >
> > Sure, udc's parent is otg device.
> >
> >>
> >> I guess i'll omit the second statement to avoid confusion. So.
> >>
> >> @parent: the parent device to this udc.
> >
> > Where you call below APIs? It seems to be a udc driver, right?
> > So, when you try to get "otg-controller" from the node, this node
> > should be udc.
>
> @parent is actually the device that represents the USB Device controller
> in the device tree. When you call usb_add_gadget_udc_release() a new
> udc->dev device is created as it's child.
>
> See explanation for the @parent argument in usb_add_gadget_udc_release().
> As we want to keep the parent argument identical to that I will not make
> any changes then.
Oh, yes. The parent should be glue layer udc device or the dual-role controller
device.
Peter
>
> >
> > /**
> > * usb_otg_add_gadget_udc - adds a new gadget to the udc class driver list
> > * @parent: the parent device to this udc. Usually the controller
> > * driver's device.
> > * @gadget: the gadget to be added to the list
> > * @otg_dev: the OTG controller device
> > *
> > * If otg_dev is NULL then device tree node is checked
> > * for OTG controller via the otg-controller property.
> > * Returns zero on success, negative errno otherwise.
> > */
> > int usb_otg_add_gadget_udc(struct device *parent, struct usb_gadget *gadget,
> >struct device *otg_dev)
> > {
> > if (!otg_dev) {
> > gadget->otg_dev = of_usb_get_otg(parent->of_node);
> > if (!gadget->otg_dev)
> > return -ENODEV;
> > } else {
> > gadget->otg_dev = otg_dev;
> > }
> >
> > return usb_add_gadget_udc_release(parent, gadget, NULL);
> > }
> > EXPORT_SYMBOL_GPL(usb_otg_add_gadget_udc);
> >
>
> --
> cheers,
> -roger
--
Best Regards,
Peter Chen
--
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
Re: [PATCH v10 13/14] usb: gadget: udc: adapt to OTG core
On 13/06/16 10:20, Peter Chen wrote:
> On Mon, Jun 13, 2016 at 10:14:31AM +0300, Roger Quadros wrote:
>> On 12/06/16 14:36, Peter Chen wrote:
>>> On Fri, Jun 10, 2016 at 04:07:22PM +0300, Roger Quadros wrote:
+/**
+ * usb_otg_add_gadget_udc - adds a new gadget to the udc class driver list
+ * @parent: the parent device to this udc. Usually the controller
+ * driver's device.
>>>
>>> It seems it should be udc device
>>
>> Parent and udc->dev are not the same right?
>
> Sure, udc's parent is otg device.
>
>>
>> I guess i'll omit the second statement to avoid confusion. So.
>>
>> @parent: the parent device to this udc.
>
> Where you call below APIs? It seems to be a udc driver, right?
> So, when you try to get "otg-controller" from the node, this node
> should be udc.
@parent is actually the device that represents the USB Device controller
in the device tree. When you call usb_add_gadget_udc_release() a new
udc->dev device is created as it's child.
See explanation for the @parent argument in usb_add_gadget_udc_release().
As we want to keep the parent argument identical to that I will not make
any changes then.
>
> /**
> * usb_otg_add_gadget_udc - adds a new gadget to the udc class driver list
> * @parent: the parent device to this udc. Usually the controller
> * driver's device.
> * @gadget: the gadget to be added to the list
> * @otg_dev: the OTG controller device
> *
> * If otg_dev is NULL then device tree node is checked
> * for OTG controller via the otg-controller property.
> * Returns zero on success, negative errno otherwise.
> */
> int usb_otg_add_gadget_udc(struct device *parent, struct usb_gadget *gadget,
> struct device *otg_dev)
> {
> if (!otg_dev) {
> gadget->otg_dev = of_usb_get_otg(parent->of_node);
> if (!gadget->otg_dev)
> return -ENODEV;
> } else {
> gadget->otg_dev = otg_dev;
> }
>
> return usb_add_gadget_udc_release(parent, gadget, NULL);
> }
> EXPORT_SYMBOL_GPL(usb_otg_add_gadget_udc);
>
--
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
Re: [PATCH v10 13/14] usb: gadget: udc: adapt to OTG core
On Mon, Jun 13, 2016 at 10:14:31AM +0300, Roger Quadros wrote:
> On 12/06/16 14:36, Peter Chen wrote:
> > On Fri, Jun 10, 2016 at 04:07:22PM +0300, Roger Quadros wrote:
> >>
> >> +/**
> >> + * usb_otg_add_gadget_udc - adds a new gadget to the udc class driver list
> >> + * @parent: the parent device to this udc. Usually the controller
> >> + * driver's device.
> >
> > It seems it should be udc device
>
> Parent and udc->dev are not the same right?
Sure, udc's parent is otg device.
>
> I guess i'll omit the second statement to avoid confusion. So.
>
> @parent: the parent device to this udc.
Where you call below APIs? It seems to be a udc driver, right?
So, when you try to get "otg-controller" from the node, this node
should be udc.
/**
* usb_otg_add_gadget_udc - adds a new gadget to the udc class driver list
* @parent: the parent device to this udc. Usually the controller
* driver's device.
* @gadget: the gadget to be added to the list
* @otg_dev: the OTG controller device
*
* If otg_dev is NULL then device tree node is checked
* for OTG controller via the otg-controller property.
* Returns zero on success, negative errno otherwise.
*/
int usb_otg_add_gadget_udc(struct device *parent, struct usb_gadget *gadget,
struct device *otg_dev)
{
if (!otg_dev) {
gadget->otg_dev = of_usb_get_otg(parent->of_node);
if (!gadget->otg_dev)
return -ENODEV;
} else {
gadget->otg_dev = otg_dev;
}
return usb_add_gadget_udc_release(parent, gadget, NULL);
}
EXPORT_SYMBOL_GPL(usb_otg_add_gadget_udc);
--
Best Regards,
Peter Chen
--
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
Re: [PATCH v10 13/14] usb: gadget: udc: adapt to OTG core
On 12/06/16 14:36, Peter Chen wrote:
> On Fri, Jun 10, 2016 at 04:07:22PM +0300, Roger Quadros wrote:
>>
>> +/**
>> + * usb_otg_add_gadget_udc - adds a new gadget to the udc class driver list
>> + * @parent: the parent device to this udc. Usually the controller
>> + * driver's device.
>
> It seems it should be udc device
Parent and udc->dev are not the same right?
I guess i'll omit the second statement to avoid confusion. So.
@parent: the parent device to this udc.
>
>> +/* udc_lock must be held */
>> static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver
>> *driver)
>> {
>> int ret;
>> @@ -1282,17 +1449,26 @@ static int udc_bind_to_driver(struct usb_udc *udc,
>> struct usb_gadget_driver *dri
>> ret = driver->bind(udc->gadget, driver);
>> if (ret)
>> goto err1;
>> -ret = usb_gadget_udc_start(udc);
>> -if (ret) {
>> -driver->unbind(udc->gadget);
>> -goto err1;
>> +
>> +/* If OTG/dual-role, the otg core manages UDC start/stop */
>> +if (udc->gadget->otg_dev) {
>> +mutex_unlock(&udc_lock);
>> +usb_otg_gadget_ready(udc->gadget, true);
>> +mutex_lock(&udc_lock);
>> +} else {
>> +ret = usb_gadget_udc_start(udc);
>> +if (ret) {
>> +mutex_unlock(&udc_lock);
>> +driver->unbind(udc->gadget);
>> +goto err1;
>> +}
>> +usb_udc_connect_control(udc);
>> }
>> -usb_udc_connect_control(udc);
>>
>> kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
>> return 0;
>> err1:
>> -if (ret != -EISNAM)
>> +if ((ret != -EISNAM) && (ret != -EPROBE_DEFER))
>
> It seems it will not introduce -EPROBE_DEFER with your changes
> in udc_bind_to_driver
>
Good catch. Left over bit when I moved defer probing out of here.
--
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
Re: [PATCH v10 13/14] usb: gadget: udc: adapt to OTG core
On Fri, Jun 10, 2016 at 04:07:22PM +0300, Roger Quadros wrote:
>
> +/**
> + * usb_otg_add_gadget_udc - adds a new gadget to the udc class driver list
> + * @parent: the parent device to this udc. Usually the controller
> + * driver's device.
It seems it should be udc device
> +/* udc_lock must be held */
> static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver
> *driver)
> {
> int ret;
> @@ -1282,17 +1449,26 @@ static int udc_bind_to_driver(struct usb_udc *udc,
> struct usb_gadget_driver *dri
> ret = driver->bind(udc->gadget, driver);
> if (ret)
> goto err1;
> - ret = usb_gadget_udc_start(udc);
> - if (ret) {
> - driver->unbind(udc->gadget);
> - goto err1;
> +
> + /* If OTG/dual-role, the otg core manages UDC start/stop */
> + if (udc->gadget->otg_dev) {
> + mutex_unlock(&udc_lock);
> + usb_otg_gadget_ready(udc->gadget, true);
> + mutex_lock(&udc_lock);
> + } else {
> + ret = usb_gadget_udc_start(udc);
> + if (ret) {
> + mutex_unlock(&udc_lock);
> + driver->unbind(udc->gadget);
> + goto err1;
> + }
> + usb_udc_connect_control(udc);
> }
> - usb_udc_connect_control(udc);
>
> kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
> return 0;
> err1:
> - if (ret != -EISNAM)
> + if ((ret != -EISNAM) && (ret != -EPROBE_DEFER))
It seems it will not introduce -EPROBE_DEFER with your changes
in udc_bind_to_driver
--
Best Regards,
Peter Chen
--
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
