| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 18 Apr 2016 14:59:54 +0800
From: Peter Chen <hzpeterchen@...il.com>
To: Roger Quadros <rogerq@...com>
Cc: stern@...land.harvard.edu, balbi@...nel.org,
gregkh@...uxfoundation.org, peter.chen@...escale.com,
dan.j.williams@...el.com, jun.li@...escale.com,
mathias.nyman@...ux.intel.com, tony@...mide.com,
Joao.Pinto@...opsys.com, abrestic@...omium.org,
r.baldyga@...sung.com, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org
Subject: Re: [PATCH v6 09/12] usb: gadget: udc: adapt to OTG core
On Tue, Apr 05, 2016 at 05:05:14PM +0300, Roger Quadros wrote:
> The OTG state machine needs a mechanism to start and
> stop the gadget controller. Add usb_gadget_start()
> and usb_gadget_stop().
>
> Introduce usb_otg_add_gadget_udc() to allow controller drivers
> to register a gadget controller that is part of an OTG instance.
>
> Register with OTG core when gadget function driver
> is available and unregister when function driver is unbound.
>
> We need to unlock the usb_lock mutexbefore calling
...mutex before...
> usb_otg_register_gadget() in udc_bind_to_driver() and
> usb_gadget_remove_driver() else it will cause a circular
> locking dependency.
>
> Ignore softconnect sysfs control when we're in OTG
> mode as OTG FSM takes care of gadget softconnect using
> the b_bus_req mechanism.
>
> Signed-off-by: Roger Quadros <rogerq@...com>
> ---
> drivers/usb/gadget/udc/udc-core.c | 166 +++++++++++++++++++++++++++++++++++---
> include/linux/usb/gadget.h | 4 +
> 2 files changed, 161 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index 4151597..9b9702f 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -28,6 +28,10 @@
> #include <linux/usb/ch9.h>
> #include <linux/usb/gadget.h>
> #include <linux/usb.h>
> +#include <linux/usb/otg.h>
> +
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
>
> /**
> * struct usb_udc - describes one usb device controller
> @@ -304,6 +308,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_udc_reset);
> */
> static inline int usb_gadget_udc_start(struct usb_udc *udc)
> {
> + dev_dbg(&udc->dev, "%s\n", __func__);
> return udc->gadget->ops->udc_start(udc->gadget, udc->driver);
> }
You may delete the debug message next time.
>
> @@ -321,10 +326,81 @@ static inline int usb_gadget_udc_start(struct usb_udc *udc)
> */
> static inline void usb_gadget_udc_stop(struct usb_udc *udc)
> {
> + dev_dbg(&udc->dev, "%s\n", __func__);
> udc->gadget->ops->udc_stop(udc->gadget);
> }
The same.
>
> /**
> + * usb_gadget_start - start the usb gadget controller and connect to bus
> + * @gadget: the gadget device to start
> + *
> + * This is external API for use by OTG core.
> + *
> + * Start the usb device controller and connect to bus (enable pull).
> + */
> +static int usb_gadget_start(struct usb_gadget *gadget)
> +{
> + int ret;
> + struct usb_udc *udc = NULL;
> +
> + dev_dbg(&gadget->dev, "%s\n", __func__);
The same
> + mutex_lock(&udc_lock);
> + list_for_each_entry(udc, &udc_list, list)
> + if (udc->gadget == gadget)
> + goto found;
> +
> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
> + __func__);
> + mutex_unlock(&udc_lock);
> + return -EINVAL;
> +
> +found:
> + ret = usb_gadget_udc_start(udc);
> + if (ret)
> + dev_err(&udc->dev, "USB Device Controller didn't start: %d\n",
> + ret);
> + else
> + usb_udc_connect_control(udc);
> +
> + mutex_unlock(&udc_lock);
> +
> + return ret;
> +}
> +
> +/**
> + * usb_gadget_stop - disconnect from bus and stop the usb gadget
> + * @gadget: The gadget device we want to stop
> + *
> + * This is external API for use by OTG core.
> + *
> + * Disconnect from the bus (disable pull) and stop the
> + * gadget controller.
> + */
> +static int usb_gadget_stop(struct usb_gadget *gadget)
> +{
> + struct usb_udc *udc = NULL;
> +
> + dev_dbg(&gadget->dev, "%s\n", __func__);
The same
> + mutex_lock(&udc_lock);
> + list_for_each_entry(udc, &udc_list, list)
> + if (udc->gadget == gadget)
> + goto found;
> +
> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
> + __func__);
> + mutex_unlock(&udc_lock);
> + return -EINVAL;
The above finding gadget code is the same with usb_gadget_start, can we
use one common API to instead of it?
> +
> +found:
> + usb_gadget_disconnect(udc->gadget);
> + udc->driver->disconnect(udc->gadget);
> + usb_gadget_udc_stop(udc);
> + mutex_unlock(&udc_lock);
> +
> + return 0;
> +}
> +
> +/**
> * usb_udc_release - release the usb_udc struct
> * @dev: the dev member within usb_udc
> *
> @@ -486,6 +562,48 @@ int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget)
> }
> EXPORT_SYMBOL_GPL(usb_add_gadget_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) {
> + struct device_node *np;
> + struct platform_device *pdev;
> +
> + np = of_parse_phandle(parent->of_node, "otg-controller", 0);
> + if (!np) {
> + dev_err(parent,
> + "otg_dev is NULL or no otg-controller property in DT\n");
> + return -EINVAL;
> + }
> +
> + pdev = of_find_device_by_node(np);
> + of_node_put(np);
> + if (!pdev) {
> + dev_err(parent, "couldn't get otg-controller device\n");
> + return -ENODEV;
> + }
> +
> + gadget->otg_dev = &pdev->dev;
> + } else {
> + gadget->otg_dev = otg_dev;
> + }
The above the code is duplicated with hcd's. Can we do something common
at usb-otg.c, eg define an API get_usb_otg_dev(struct usb_gadget *gadget, struct
usb_hcd *hcd), in this API we can try to get otg_dev for both gadget and
hcd, and we can call it at controller driver during register otg.
--
Best Regards,
Peter Chen
Powered by blists - more mailing lists