Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1400666 > unrolled thread
| Started by | Roger Quadros <rogerq@ti.com> |
|---|---|
| First post | 2016-05-13 12:10 +0200 |
| Last post | 2016-05-30 16:10 +0200 |
| Articles | 20 on this page of 52 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH v8 00/14] USB OTG/dual-role framework Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-16 09:20 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-16 10:30 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-16 11:40 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-16 12:00 +0200
RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-17 09:40 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-17 10:10 +0200
RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-17 10:50 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-18 14:50 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-18 15:50 +0200
RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-18 16:50 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-19 09:40 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-21 04:40 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-23 05:30 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-23 12:20 +0200
RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-23 12:40 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-23 12:40 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-24 05:00 +0200
RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-18 15:50 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-18 05:30 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-18 14:50 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-20 03:50 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-20 09:30 +0200
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-21 05:00 +0200
[PATCH v8 10/14] usb: otg: add hcd companion support Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
Re: [PATCH v8 10/14] usb: otg: add hcd companion support Rob Herring <robh@kernel.org> - 2016-05-13 20:20 +0200
Re: [PATCH v8 10/14] usb: otg: add hcd companion support Roger Quadros <rogerq@ti.com> - 2016-05-16 10:20 +0200
[PATCH v9 10/14] usb: otg: add hcd companion support Roger Quadros <rogerq@ti.com> - 2016-05-20 11:40 +0200
Re: [PATCH v9 10/14] usb: otg: add hcd companion support Rob Herring <robh@kernel.org> - 2016-05-23 23:10 +0200
[PATCH v8 11/14] usb: otg: use dev_dbg() instead of VDBG() Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 12/14] usb: hcd: Adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 14/14] usb: host: xhci-plat: Add otg device to platform data Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 03/14] usb: hcd.h: Add OTG to HCD interface Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-16 11:10 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-18 15:10 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-20 10:40 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-20 11:20 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Peter Chen <hzpeterchen@gmail.com> - 2016-05-20 12:00 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-23 12:10 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-24 11:50 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Peter Chen <hzpeterchen@gmail.com> - 2016-05-25 04:50 +0200
RE: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Jun Li <jun.li@nxp.com> - 2016-05-25 06:00 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-25 14:30 +0200
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-25 14:30 +0200
RE: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Jun Li <jun.li@nxp.com> - 2016-05-25 16:50 +0200
[PATCH v8 01/14] usb: hcd: Initialize hcd->flags to 0 Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 07/14] usb: otg: get rid of CONFIG_USB_OTG_FSM in favour of CONFIG_USB_OTG Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
[PATCH v8 05/14] usb: otg-fsm: move host controller operations into usb_otg->hcd_ops Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
Re: [PATCH v8 00/14] USB OTG/dual-role framework Peter Chen <hzpeterchen@gmail.com> - 2016-05-30 11:40 +0200
Re: [PATCH v8 00/14] USB OTG/dual-role framework Roger Quadros <rogerq@ti.com> - 2016-05-30 16:10 +0200
Page 1 of 3 [1] 2 3 Next page →
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-13 12:10 +0200 |
| Subject | [PATCH v8 00/14] USB OTG/dual-role framework |
| Message-ID | <ryicF-1fj-3@gated-at.bofh.it> |
Hi,
This series centralizes OTG/Dual-role functionality in the kernel.
As of now I've got Dual-role functionality working pretty reliably on
dra7-evm and am437x-gp-evm.
NOTE: my am437x-gp-evm broke so I couldn't test v8 on it.
But the changes since v7 are trivial and shouldn't impact am437x-gp-evm.
DWC3 controller and platform related patches will be sent separately.
Series is based on v4.6-rc1 and depends on first 2 patches of [1]
[1] - OTG fsm cleanup - https://lkml.org/lkml/2016/3/30/186
Why?:
----
Currently there is no central location where OTG/dual-role functionality is
implemented in the Linux USB stack and every USB controller driver is
doing their own thing for OTG/dual-role. We can benefit from code-reuse
and simplicity by adding the OTG/dual-role core driver.
Newer OTG cores support standard host interface (e.g. xHCI) so
host and gadget functionality are no longer closely knit like older
cores. There needs to be a way to co-ordinate the operation of the
host and gadget controllers in dual-role mode. i.e. to stop and start them
from a central location. This central location should be the
USB OTG/dual-role core.
Host and gadget controllers might be sharing resources and can't
be always running. One has to be stopped for the other to run.
This couldn't be done till now but can be done from the OTG core.
What?:
-----
The OTG/dual-role core consists of a set of APIs that allow
registration of OTG controller device and OTG capable host and gadget
controllers.
- The OTG controller driver can provide the OTG capabilities and the
Finite State Machine work function via 'struct usb_otg_config'
at the time of registration i.e. usb_otg_register();
struct usb_otg *usb_otg_register(struct device *dev,
struct usb_otg_config *config);
int usb_otg_unregister(struct device *dev);
/**
* struct usb_otg_config - otg controller configuration
* @caps: otg capabilities of the controller
* @ops: otg fsm operations
* @otg_work: optional custom otg state machine work function
*/
struct usb_otg_config {
struct usb_otg_caps *otg_caps;
struct otg_fsm_ops *fsm_ops;
void (*otg_work)(struct work_struct *work);
};
The dual-role state machine is built-into the OTG core so nothing
special needs to be provided if only dual-role functionality is desired.
The low level OTG controller driver ops are povided via
'struct otg_fsm_ops *fsm_ops' in the 'struct usb_otg_config'.
After registration, the OTG core waits for host, gadget controller
and the gadget function driver to be registered. Once all resources are
available it instantiates the Finite State Machine (FSM).
The host/gadget controllers are started/stopped according to the FSM.
- Host and gadget controllers that are a part of OTG/dual-role port must
use the OTG core provided APIs to add/remove the host/gadget.
i.e. hosts must use usb_otg_add_hcd() usb_otg_remove_hcd(),,
gadgets must use usb_otg_add_gadget_udc() usb_del_gadget_udc().
This ensures that the host and gadget controllers are not started till
the state machine is ready and the right bus conditions are met.
It also allows the host and gadget controllers to provide the OTG
controller device to link them together. For Device tree boots
the related OTG controller is automatically picked up via the
'otg-controller' property in the Host/Gadget controller nodes.
int usb_otg_add_hcd(struct usb_hcd *hcd,
unsigned int irqnum, unsigned long irqflags,
struct device *otg_dev);
void usb_otg_remove_hcd(struct usb_hcd *hcd);
int usb_otg_add_gadget_udc(struct device *parent,
struct usb_gadget *gadget,
struct device *otg_dev);
usb_del_gadget_udc() must be used for removal.
- During the lifetime of the FSM, the OTG controller driver can provide
inputs event changes using usb_otg_sync_inputs(). The OTG core will
then schedule the FSM work function (or internal dual-role state machine)
to update the FSM state. The FSM then calls the OTG controller
operations (fsm_ops) as necessary.
void usb_otg_sync_inputs(struct usb_otg *otg);
- The following 2 functions are provided as helpers for use by the
OTG controller driver to start/stop the host/gadget controllers.
int usb_otg_start_host(struct usb_otg *otg, int on);
int usb_otg_start_gadget(struct usb_otg *otg, int on);
- The following function is provided for use by the USB host stack
to sync OTG related events to the OTG state machine.
e.g. change in host_bus->b_hnp_enable, gadget->b_hnp_enable
int usb_otg_kick_fsm(struct device *otg_device);
Changelog:
---------
v8:
- split out start/stop gadget and connect/disconnect operations.
- make CONFIG_OTG dpend on CONFIG_USB_GADGET as well apart from CONFIG_USB
- use create_freezable_workqueue() for OTG work as per Peter's suggestion.
- remove usb-otg.h as we're not initializing any OTG timers.
- don't include unnecessary headers in usb-otg.c (i.e. hrtimer.h & ktime.h)
v7:
- added dual-role support for host controllers requiring a companion
controller. e.g. EHCI + OHCI.
- added of_usb_get_otg() to get the OTG controller device
from the USB controller's device node.
- addressed review comments.
v6:
- added otg specific APIs for host/gadget registration. behaviour of
original host/gadget API remains unchanged. Platform devices can now
pass the otg device explicitly while registering host/gadget.
- moved hcd specific operations from struct otg_fsm to struct hcd_ops.
- made struct usb_otg mandatory for all otg related APIs.
- allow otg controller to provide it's own otg_work function so that
it can implement it's own state machine.
- removed otg fsm and timers from usb-otg.c. Only dual-role state machine
is implemented.
- vbus is controlled in the dual-role state machine.
- PM runtime is used around drd_statemachine().
- added otg_dev to xhci platform data to allow platform code to specify
the otg controller tied to the xhci host controller.
v5: Internal version. Not sent to mailing list
v4:
- Added DT support for tying otg-controller to host and gadget
controllers. For DT we no longer have the constraint that
OTG controller needs to be parent of host and gadget. They can be
tied together using the "otg-controller" property.
- Relax the requirement for DT case that otg controller must register
before host/gadget. We maintain a wait list of host/gadget devices
waiting on the otg controller.
- Use a single struct usb_otg for otg data.
- Don't override host/gadget start/stop APIs. Let the controller
drivers do what they want as they know best. Helper API is provided
for controller start/stop that controller driver can use.
- Introduce struct usb_otg_config to pass the otg capabilities,
otg ops and otg timer timeouts during otg controller registration.
- rebased on Greg's usb.git/usb-next
v3:
- all otg related definations now in otg.h
- single kernel config USB_OTG to enable OTG core and FSM.
- resolved symbol dependency issues.
- use dev_vdbg instead of VDBG() in usb-otg-fsm.c
- rebased on v4.2-rc1
v2:
- Use add/remove_hcd() instead of start/stop_hcd() to enable/disable
the host controller
- added dual-role-device (DRD) state machine which is a much simpler
mode of operation when compared to OTG. Here we don't support fancy
OTG features like HNP, SRP, on the fly role-swap. The mode of operation
is determined based on ID pin (cable type) and the role doesn't change
till the cable type changes.
--
cheers,
-roger
Roger Quadros (13):
usb: hcd: Initialize hcd->flags to 0
usb: otg-fsm: Prevent build warning "VDBG" redefined
usb: hcd.h: Add OTG to HCD interface
usb: otg-fsm: use usb_otg wherever possible
usb: otg-fsm: move host controller operations into usb_otg->hcd_ops
usb: gadget.h: Add OTG to gadget interface
usb: otg: get rid of CONFIG_USB_OTG_FSM in favour of CONFIG_USB_OTG
usb: otg: add OTG/dual-role core
usb: of: add an API to get OTG device from USB controller node
usb: otg: use dev_dbg() instead of VDBG()
usb: hcd: Adapt to OTG core
usb: gadget: udc: adapt to OTG core
usb: host: xhci-plat: Add otg device to platform data
Yoshihiro Shimoda (1):
usb: otg: add hcd companion support
Documentation/devicetree/bindings/usb/generic.txt | 6 +
Documentation/usb/chipidea.txt | 2 +-
drivers/usb/chipidea/Makefile | 2 +-
drivers/usb/chipidea/ci.h | 3 +-
drivers/usb/chipidea/core.c | 14 +-
drivers/usb/chipidea/debug.c | 2 +-
drivers/usb/chipidea/otg_fsm.c | 176 ++--
drivers/usb/chipidea/otg_fsm.h | 2 +-
drivers/usb/chipidea/udc.c | 17 +-
drivers/usb/common/Makefile | 3 +-
drivers/usb/common/common.c | 27 +
drivers/usb/common/usb-otg-fsm.c | 203 ++--
drivers/usb/common/usb-otg.c | 1056 +++++++++++++++++++++
drivers/usb/core/Kconfig | 12 +-
drivers/usb/core/hcd.c | 56 ++
drivers/usb/gadget/udc/udc-core.c | 194 +++-
drivers/usb/host/xhci-plat.c | 35 +-
drivers/usb/phy/Kconfig | 2 +-
drivers/usb/phy/phy-fsl-usb.c | 155 +--
drivers/usb/phy/phy-fsl-usb.h | 3 +-
include/linux/usb/gadget.h | 22 +
include/linux/usb/hcd.h | 29 +
include/linux/usb/of.h | 9 +
include/linux/usb/otg-fsm.h | 154 +--
include/linux/usb/otg.h | 264 +++++-
include/linux/usb/xhci_pdriver.h | 3 +
26 files changed, 2013 insertions(+), 438 deletions(-)
create mode 100644 drivers/usb/common/usb-otg.c
--
2.7.4
[toc] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-13 12:10 +0200 |
| Subject | [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <ryicG-1fj-21@gated-at.bofh.it> |
| In reply to | #1400666 |
The OTG state machine needs a mechanism to start and
stop the gadget controller as well as connect/disconnect
from the bus. Add usb_gadget_start(), usb_gadget_stop()
and usb_gadget_connect_control().
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 mutex before calling
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@ti.com>
---
drivers/usb/gadget/udc/udc-core.c | 194 ++++++++++++++++++++++++++++++++++++--
include/linux/usb/gadget.h | 4 +
2 files changed, 189 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index 4151597..21c85ef 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -28,6 +28,11 @@
#include <linux/usb/ch9.h>
#include <linux/usb/gadget.h>
#include <linux/usb.h>
+#include <linux/usb/otg.h>
+#include <linux/usb/of.h>
+
+#include <linux/of.h>
+#include <linux/of_platform.h>
/**
* struct usb_udc - describes one usb device controller
@@ -325,6 +330,119 @@ static inline void usb_gadget_udc_stop(struct usb_udc *udc)
}
/**
+ * usb_gadget_to_udc - get the UDC owning the gadget
+ *
+ * udc_lock must be held.
+ * Returs NULL if UDC is not found.
+ */
+static struct usb_udc *usb_gadget_to_udc(struct usb_gadget *gadget)
+{
+ struct usb_udc *udc;
+
+ list_for_each_entry(udc, &udc_list, list)
+ if (udc->gadget == gadget)
+ return udc;
+
+ return NULL;
+}
+
+/**
+ * usb_gadget_start - start the usb gadget controller
+ * @gadget: the gadget device to start
+ *
+ * This is external API for use by OTG core.
+ *
+ * Start the usb device controller. Does not connect to the bus.
+ */
+static int usb_gadget_start(struct usb_gadget *gadget)
+{
+ int ret;
+ struct usb_udc *udc;
+
+ mutex_lock(&udc_lock);
+ udc = usb_gadget_to_udc(gadget);
+ if (!udc) {
+ dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+ __func__);
+ mutex_unlock(&udc_lock);
+ return -EINVAL;
+ }
+
+ ret = usb_gadget_udc_start(udc);
+ if (ret)
+ dev_err(&udc->dev, "USB Device Controller didn't start: %d\n",
+ ret);
+
+ mutex_unlock(&udc_lock);
+
+ return ret;
+}
+
+/**
+ * usb_gadget_stop - stop the usb gadget controller
+ * @gadget: the gadget device we want to stop
+ *
+ * This is external API for use by OTG core.
+ *
+ * Stop the gadget controller. Does not disconnect from the bus.
+ * Caller must ensure that gadget has disconnected from the bus
+ * before calling usb_gadget_stop().
+ */
+static int usb_gadget_stop(struct usb_gadget *gadget)
+{
+ struct usb_udc *udc;
+
+ mutex_lock(&udc_lock);
+ udc = usb_gadget_to_udc(gadget);
+ if (!udc) {
+ dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+ __func__);
+ mutex_unlock(&udc_lock);
+ return -EINVAL;
+ }
+
+ if (gadget->connected) {
+ dev_err(gadget->dev.parent,
+ "%s: called while still connected\n", __func__);
+ mutex_unlock(&udc_lock);
+ return -EINVAL;
+ }
+
+ usb_gadget_udc_stop(udc);
+ mutex_unlock(&udc_lock);
+
+ return 0;
+}
+
+static int usb_gadget_connect_control(struct usb_gadget *gadget, bool connect)
+{
+ struct usb_udc *udc;
+
+ mutex_lock(&udc_lock);
+ udc = usb_gadget_to_udc(gadget);
+ if (!udc) {
+ dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+ __func__);
+ mutex_unlock(&udc_lock);
+ return -EINVAL;
+ }
+
+ if (connect) {
+ if (!gadget->connected)
+ usb_gadget_connect(udc->gadget);
+ } else {
+ if (gadget->connected) {
+ usb_gadget_disconnect(udc->gadget);
+ udc->driver->disconnect(udc->gadget);
+ }
+ }
+
+ mutex_unlock(&udc_lock);
+
+ return 0;
+}
+
+/**
* usb_udc_release - release the usb_udc struct
* @dev: the dev member within usb_udc
*
@@ -486,6 +604,33 @@ 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) {
+ 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);
+
+/* udc_lock must be held */
static void usb_gadget_remove_driver(struct usb_udc *udc)
{
dev_dbg(&udc->dev, "unregistering UDC driver [%s]\n",
@@ -493,10 +638,18 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
- usb_gadget_disconnect(udc->gadget);
- udc->driver->disconnect(udc->gadget);
+ /* If OTG, the otg core ensures UDC is stopped on unregister */
+ if (udc->gadget->otg_dev) {
+ mutex_unlock(&udc_lock);
+ usb_otg_unregister_gadget(udc->gadget);
+ mutex_lock(&udc_lock);
+ } else {
+ usb_gadget_disconnect(udc->gadget);
+ udc->driver->disconnect(udc->gadget);
+ usb_gadget_udc_stop(udc);
+ }
+
udc->driver->unbind(udc->gadget);
- usb_gadget_udc_stop(udc);
udc->driver = NULL;
udc->dev.driver = NULL;
@@ -530,6 +683,8 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
}
mutex_unlock(&udc_lock);
+ mutex_unlock(&udc_lock);
+
kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
flush_work(&gadget->work);
device_unregister(&udc->dev);
@@ -539,6 +694,13 @@ EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
/* ------------------------------------------------------------------------- */
+struct otg_gadget_ops otg_gadget_intf = {
+ .start = usb_gadget_start,
+ .stop = usb_gadget_stop,
+ .connect_control = usb_gadget_connect_control,
+};
+
+/* udc_lock must be held */
static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *driver)
{
int ret;
@@ -553,12 +715,20 @@ 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, the otg core starts the UDC when needed */
+ if (udc->gadget->otg_dev) {
+ mutex_unlock(&udc_lock);
+ usb_otg_register_gadget(udc->gadget, &otg_gadget_intf);
+ mutex_lock(&udc_lock);
+ } else {
+ ret = usb_gadget_udc_start(udc);
+ if (ret) {
+ 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;
@@ -660,9 +830,15 @@ static ssize_t usb_udc_softconn_store(struct device *dev,
return -EOPNOTSUPP;
}
+ /* In OTG mode we don't support softconnect, but b_bus_req */
+ if (udc->gadget->otg_dev) {
+ dev_err(dev, "soft-connect not supported in OTG mode\n");
+ return -EOPNOTSUPP;
+ }
+
if (sysfs_streq(buf, "connect")) {
usb_gadget_udc_start(udc);
- usb_gadget_connect(udc->gadget);
+ usb_udc_connect_control(udc);
} else if (sysfs_streq(buf, "disconnect")) {
usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 3ecfddd..79d654f 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1162,6 +1162,10 @@ extern int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget);
extern void usb_del_gadget_udc(struct usb_gadget *gadget);
extern char *usb_get_gadget_udc_name(void);
+extern int usb_otg_add_gadget_udc(struct device *parent,
+ struct usb_gadget *gadget,
+ struct device *otg_dev);
+
/*-------------------------------------------------------------------------*/
/* utility to simplify dealing with string descriptors */
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-05-16 09:20 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rzkYN-3UW-1@gated-at.bofh.it> |
| In reply to | #1400667 |
On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote:
> +
> +static int usb_gadget_connect_control(struct usb_gadget *gadget, bool connect)
> +{
> + struct usb_udc *udc;
> +
> + mutex_lock(&udc_lock);
> + udc = usb_gadget_to_udc(gadget);
> + if (!udc) {
> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
> + __func__);
> + mutex_unlock(&udc_lock);
> + return -EINVAL;
> + }
> +
> + if (connect) {
> + if (!gadget->connected)
> + usb_gadget_connect(udc->gadget);
> + } else {
> + if (gadget->connected) {
> + usb_gadget_disconnect(udc->gadget);
> + udc->driver->disconnect(udc->gadget);
> + }
> + }
> +
> + mutex_unlock(&udc_lock);
> +
> + return 0;
> +}
> +
Since this is called for vbus interrupt, why not using
usb_udc_vbus_handler directly, and call udc->driver->disconnect
at usb_gadget_stop.
> return 0;
> @@ -660,9 +830,15 @@ static ssize_t usb_udc_softconn_store(struct device *dev,
> return -EOPNOTSUPP;
> }
>
> + /* In OTG mode we don't support softconnect, but b_bus_req */
> + if (udc->gadget->otg_dev) {
> + dev_err(dev, "soft-connect not supported in OTG mode\n");
> + return -EOPNOTSUPP;
> + }
> +
The soft-connect can be supported at dual-role mode currently, we can
use b_bus_req entry once it is implemented later.
> if (sysfs_streq(buf, "connect")) {
> usb_gadget_udc_start(udc);
> - usb_gadget_connect(udc->gadget);
> + usb_udc_connect_control(udc);
This line seems to be not related with this patch.
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-16 10:30 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rzm4x-4zU-5@gated-at.bofh.it> |
| In reply to | #1401331 |
Hi,
On 16/05/16 10:02, Peter Chen wrote:
> On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote:
>> +
>> +static int usb_gadget_connect_control(struct usb_gadget *gadget, bool connect)
>> +{
>> + struct usb_udc *udc;
>> +
>> + mutex_lock(&udc_lock);
>> + udc = usb_gadget_to_udc(gadget);
>> + if (!udc) {
>> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
>> + __func__);
>> + mutex_unlock(&udc_lock);
>> + return -EINVAL;
>> + }
>> +
>> + if (connect) {
>> + if (!gadget->connected)
>> + usb_gadget_connect(udc->gadget);
>> + } else {
>> + if (gadget->connected) {
>> + usb_gadget_disconnect(udc->gadget);
>> + udc->driver->disconnect(udc->gadget);
>> + }
>> + }
>> +
>> + mutex_unlock(&udc_lock);
>> +
>> + return 0;
>> +}
>> +
>
> Since this is called for vbus interrupt, why not using
> usb_udc_vbus_handler directly, and call udc->driver->disconnect
> at usb_gadget_stop.
We can't assume that this is always called for vbus interrupt so
I decided not to call usb_udc_vbus_handler.
udc->vbus is really pointless for us. We keep vbus states in our
state machine and leave udc->vbus as ture always.
Why do you want to move udc->driver->disconnect() to stop?
If USB controller disconnected from bus then the gadget driver
must be notified about the disconnect immediately. The controller
may or may not be stopped by the core.
>
>> return 0;
>> @@ -660,9 +830,15 @@ static ssize_t usb_udc_softconn_store(struct device *dev,
>> return -EOPNOTSUPP;
>> }
>>
>> + /* In OTG mode we don't support softconnect, but b_bus_req */
>> + if (udc->gadget->otg_dev) {
>> + dev_err(dev, "soft-connect not supported in OTG mode\n");
>> + return -EOPNOTSUPP;
>> + }
>> +
>
> The soft-connect can be supported at dual-role mode currently, we can
> use b_bus_req entry once it is implemented later.
Soft-connect should be done via sysfs handling within the OTG core.
This can be added later. I don't want anything outside the OTG core
to handle soft-connect behaviour as it will be hard to keep things
in sync.
I can update the comment to something like this.
/* In OTG/dual-role mode, soft-connect should be handled by OTG core */
>
>> if (sysfs_streq(buf, "connect")) {
>> usb_gadget_udc_start(udc);
>> - usb_gadget_connect(udc->gadget);
>> + usb_udc_connect_control(udc);
>
> This line seems to be not related with this patch.
>
Right. I'll remove it.
cheers,
-roger
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-05-16 11:40 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rznah-5ch-1@gated-at.bofh.it> |
| In reply to | #1401356 |
On Mon, May 16, 2016 at 11:26:57AM +0300, Roger Quadros wrote:
> Hi,
>
> On 16/05/16 10:02, Peter Chen wrote:
> > On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote:
> >> +
> >> +static int usb_gadget_connect_control(struct usb_gadget *gadget, bool connect)
> >> +{
> >> + struct usb_udc *udc;
> >> +
> >> + mutex_lock(&udc_lock);
> >> + udc = usb_gadget_to_udc(gadget);
> >> + if (!udc) {
> >> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
> >> + __func__);
> >> + mutex_unlock(&udc_lock);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (connect) {
> >> + if (!gadget->connected)
> >> + usb_gadget_connect(udc->gadget);
> >> + } else {
> >> + if (gadget->connected) {
> >> + usb_gadget_disconnect(udc->gadget);
> >> + udc->driver->disconnect(udc->gadget);
> >> + }
> >> + }
> >> +
> >> + mutex_unlock(&udc_lock);
> >> +
> >> + return 0;
> >> +}
> >> +
> >
> > Since this is called for vbus interrupt, why not using
> > usb_udc_vbus_handler directly, and call udc->driver->disconnect
> > at usb_gadget_stop.
>
> We can't assume that this is always called for vbus interrupt so
> I decided not to call usb_udc_vbus_handler.
>
> udc->vbus is really pointless for us. We keep vbus states in our
> state machine and leave udc->vbus as ture always.
>
> Why do you want to move udc->driver->disconnect() to stop?
> If USB controller disconnected from bus then the gadget driver
> must be notified about the disconnect immediately. The controller
> may or may not be stopped by the core.
>
Then, would you give some comments when this API will be used?
I was assumed it is only used for drd state machine.
> >
> >> return 0;
> >> @@ -660,9 +830,15 @@ static ssize_t usb_udc_softconn_store(struct device *dev,
> >> return -EOPNOTSUPP;
> >> }
> >>
> >> + /* In OTG mode we don't support softconnect, but b_bus_req */
> >> + if (udc->gadget->otg_dev) {
> >> + dev_err(dev, "soft-connect not supported in OTG mode\n");
> >> + return -EOPNOTSUPP;
> >> + }
> >> +
> >
> > The soft-connect can be supported at dual-role mode currently, we can
> > use b_bus_req entry once it is implemented later.
>
> Soft-connect should be done via sysfs handling within the OTG core.
> This can be added later. I don't want anything outside the OTG core
> to handle soft-connect behaviour as it will be hard to keep things
> in sync.
>
> I can update the comment to something like this.
>
> /* In OTG/dual-role mode, soft-connect should be handled by OTG core */
Ok, let's Felipe decide it.
>
> >
> >> if (sysfs_streq(buf, "connect")) {
> >> usb_gadget_udc_start(udc);
> >> - usb_gadget_connect(udc->gadget);
> >> + usb_udc_connect_control(udc);
> >
> > This line seems to be not related with this patch.
> >
> Right. I'll remove it.
>
> cheers,
> -roger
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-16 12:00 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rzntF-5j2-35@gated-at.bofh.it> |
| In reply to | #1401393 |
On 16/05/16 12:23, Peter Chen wrote:
> On Mon, May 16, 2016 at 11:26:57AM +0300, Roger Quadros wrote:
>> Hi,
>>
>> On 16/05/16 10:02, Peter Chen wrote:
>>> On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote:
>>>> +
>>>> +static int usb_gadget_connect_control(struct usb_gadget *gadget, bool connect)
>>>> +{
>>>> + struct usb_udc *udc;
>>>> +
>>>> + mutex_lock(&udc_lock);
>>>> + udc = usb_gadget_to_udc(gadget);
>>>> + if (!udc) {
>>>> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
>>>> + __func__);
>>>> + mutex_unlock(&udc_lock);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + if (connect) {
>>>> + if (!gadget->connected)
>>>> + usb_gadget_connect(udc->gadget);
>>>> + } else {
>>>> + if (gadget->connected) {
>>>> + usb_gadget_disconnect(udc->gadget);
>>>> + udc->driver->disconnect(udc->gadget);
>>>> + }
>>>> + }
>>>> +
>>>> + mutex_unlock(&udc_lock);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>
>>> Since this is called for vbus interrupt, why not using
>>> usb_udc_vbus_handler directly, and call udc->driver->disconnect
>>> at usb_gadget_stop.
>>
>> We can't assume that this is always called for vbus interrupt so
>> I decided not to call usb_udc_vbus_handler.
>>
>> udc->vbus is really pointless for us. We keep vbus states in our
>> state machine and leave udc->vbus as ture always.
>>
>> Why do you want to move udc->driver->disconnect() to stop?
>> If USB controller disconnected from bus then the gadget driver
>> must be notified about the disconnect immediately. The controller
>> may or may not be stopped by the core.
>>
>
> Then, would you give some comments when this API will be used?
> I was assumed it is only used for drd state machine.
drd_state machine didn't even need this API in the first place :).
You guys wanted me to separate out start/stop and connect/disconnect for full OTG case.
Won't full OTG state machine want to use this API? If not what would it use?
cheers,
-roger
>
>>>
>>>> return 0;
>>>> @@ -660,9 +830,15 @@ static ssize_t usb_udc_softconn_store(struct device *dev,
>>>> return -EOPNOTSUPP;
>>>> }
>>>>
>>>> + /* In OTG mode we don't support softconnect, but b_bus_req */
>>>> + if (udc->gadget->otg_dev) {
>>>> + dev_err(dev, "soft-connect not supported in OTG mode\n");
>>>> + return -EOPNOTSUPP;
>>>> + }
>>>> +
>>>
>>> The soft-connect can be supported at dual-role mode currently, we can
>>> use b_bus_req entry once it is implemented later.
>>
>> Soft-connect should be done via sysfs handling within the OTG core.
>> This can be added later. I don't want anything outside the OTG core
>> to handle soft-connect behaviour as it will be hard to keep things
>> in sync.
>>
>> I can update the comment to something like this.
>>
>> /* In OTG/dual-role mode, soft-connect should be handled by OTG core */
>
> Ok, let's Felipe decide it.
>
>>
>>>
>>>> if (sysfs_streq(buf, "connect")) {
>>>> usb_gadget_udc_start(udc);
>>>> - usb_gadget_connect(udc->gadget);
>>>> + usb_udc_connect_control(udc);
>>>
>>> This line seems to be not related with this patch.
>>>
>> Right. I'll remove it.
>>
>> cheers,
>> -roger
>
[toc] | [prev] | [next] | [standalone]
| From | Jun Li <jun.li@nxp.com> |
|---|---|
| Date | 2016-05-17 09:40 +0200 |
| Subject | RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rzHLH-1Co-13@gated-at.bofh.it> |
| In reply to | #1401409 |
Hi
> -----Original Message-----
> From: Roger Quadros [mailto:rogerq@ti.com]
> Sent: Monday, May 16, 2016 5:52 PM
> To: Peter Chen <hzpeterchen@gmail.com>
> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org;
> linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
>
> On 16/05/16 12:23, Peter Chen wrote:
> > On Mon, May 16, 2016 at 11:26:57AM +0300, Roger Quadros wrote:
> >> Hi,
> >>
> >> On 16/05/16 10:02, Peter Chen wrote:
> >>> On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote:
> >>>> +
> >>>> +static int usb_gadget_connect_control(struct usb_gadget *gadget,
> >>>> +bool connect) {
> >>>> + struct usb_udc *udc;
> >>>> +
> >>>> + mutex_lock(&udc_lock);
> >>>> + udc = usb_gadget_to_udc(gadget);
> >>>> + if (!udc) {
> >>>> + dev_err(gadget->dev.parent, "%s: gadget not
> registered.\n",
> >>>> + __func__);
> >>>> + mutex_unlock(&udc_lock);
> >>>> + return -EINVAL;
> >>>> + }
> >>>> +
> >>>> + if (connect) {
> >>>> + if (!gadget->connected)
> >>>> + usb_gadget_connect(udc->gadget);
> >>>> + } else {
> >>>> + if (gadget->connected) {
> >>>> + usb_gadget_disconnect(udc->gadget);
> >>>> + udc->driver->disconnect(udc->gadget);
> >>>> + }
> >>>> + }
> >>>> +
> >>>> + mutex_unlock(&udc_lock);
> >>>> +
> >>>> + return 0;
> >>>> +}
> >>>> +
> >>>
> >>> Since this is called for vbus interrupt, why not using
> >>> usb_udc_vbus_handler directly, and call udc->driver->disconnect at
> >>> usb_gadget_stop.
> >>
> >> We can't assume that this is always called for vbus interrupt so I
> >> decided not to call usb_udc_vbus_handler.
> >>
> >> udc->vbus is really pointless for us. We keep vbus states in our
> >> state machine and leave udc->vbus as ture always.
> >>
> >> Why do you want to move udc->driver->disconnect() to stop?
> >> If USB controller disconnected from bus then the gadget driver must
> >> be notified about the disconnect immediately. The controller may or
> >> may not be stopped by the core.
> >>
> >
> > Then, would you give some comments when this API will be used?
> > I was assumed it is only used for drd state machine.
>
> drd_state machine didn't even need this API in the first place :).
> You guys wanted me to separate out start/stop and connect/disconnect for
> full OTG case.
> Won't full OTG state machine want to use this API? If not what would it
> use?
Instead create those new interfaces/symbol here and there just aim to
address build problems in diff configures, Could we only allow meaningful
combination of those 3 drivers configures?
Hcd=y, gadget=y, otg=y or
Hcd=m, gadget=m, otg=m
Li Jun
>
> cheers,
> -roger
>
> >
> >>>
> >>>> return 0;
> >>>> @@ -660,9 +830,15 @@ static ssize_t usb_udc_softconn_store(struct
> device *dev,
> >>>> return -EOPNOTSUPP;
> >>>> }
> >>>>
> >>>> + /* In OTG mode we don't support softconnect, but b_bus_req */
> >>>> + if (udc->gadget->otg_dev) {
> >>>> + dev_err(dev, "soft-connect not supported in OTG mode\n");
> >>>> + return -EOPNOTSUPP;
> >>>> + }
> >>>> +
> >>>
> >>> The soft-connect can be supported at dual-role mode currently, we
> >>> can use b_bus_req entry once it is implemented later.
> >>
> >> Soft-connect should be done via sysfs handling within the OTG core.
> >> This can be added later. I don't want anything outside the OTG core
> >> to handle soft-connect behaviour as it will be hard to keep things in
> >> sync.
> >>
> >> I can update the comment to something like this.
> >>
> >> /* In OTG/dual-role mode, soft-connect should be handled by OTG core
> >> */
> >
> > Ok, let's Felipe decide it.
> >
> >>
> >>>
> >>>> if (sysfs_streq(buf, "connect")) {
> >>>> usb_gadget_udc_start(udc);
> >>>> - usb_gadget_connect(udc->gadget);
> >>>> + usb_udc_connect_control(udc);
> >>>
> >>> This line seems to be not related with this patch.
> >>>
> >> Right. I'll remove it.
> >>
> >> cheers,
> >> -roger
> >
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-17 10:10 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rzIeL-21W-39@gated-at.bofh.it> |
| In reply to | #1402158 |
On 17/05/16 10:38, Jun Li wrote:
> Hi
>
>> -----Original Message-----
>> From: Roger Quadros [mailto:rogerq@ti.com]
>> Sent: Monday, May 16, 2016 5:52 PM
>> To: Peter Chen <hzpeterchen@gmail.com>
>> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
>> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
>> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
>> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
>> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
>> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org;
>> linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org;
>> devicetree@vger.kernel.org
>> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
>>
>> On 16/05/16 12:23, Peter Chen wrote:
>>> On Mon, May 16, 2016 at 11:26:57AM +0300, Roger Quadros wrote:
>>>> Hi,
>>>>
>>>> On 16/05/16 10:02, Peter Chen wrote:
>>>>> On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote:
>>>>>> +
>>>>>> +static int usb_gadget_connect_control(struct usb_gadget *gadget,
>>>>>> +bool connect) {
>>>>>> + struct usb_udc *udc;
>>>>>> +
>>>>>> + mutex_lock(&udc_lock);
>>>>>> + udc = usb_gadget_to_udc(gadget);
>>>>>> + if (!udc) {
>>>>>> + dev_err(gadget->dev.parent, "%s: gadget not
>> registered.\n",
>>>>>> + __func__);
>>>>>> + mutex_unlock(&udc_lock);
>>>>>> + return -EINVAL;
>>>>>> + }
>>>>>> +
>>>>>> + if (connect) {
>>>>>> + if (!gadget->connected)
>>>>>> + usb_gadget_connect(udc->gadget);
>>>>>> + } else {
>>>>>> + if (gadget->connected) {
>>>>>> + usb_gadget_disconnect(udc->gadget);
>>>>>> + udc->driver->disconnect(udc->gadget);
>>>>>> + }
>>>>>> + }
>>>>>> +
>>>>>> + mutex_unlock(&udc_lock);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>
>>>>> Since this is called for vbus interrupt, why not using
>>>>> usb_udc_vbus_handler directly, and call udc->driver->disconnect at
>>>>> usb_gadget_stop.
>>>>
>>>> We can't assume that this is always called for vbus interrupt so I
>>>> decided not to call usb_udc_vbus_handler.
>>>>
>>>> udc->vbus is really pointless for us. We keep vbus states in our
>>>> state machine and leave udc->vbus as ture always.
>>>>
>>>> Why do you want to move udc->driver->disconnect() to stop?
>>>> If USB controller disconnected from bus then the gadget driver must
>>>> be notified about the disconnect immediately. The controller may or
>>>> may not be stopped by the core.
>>>>
>>>
>>> Then, would you give some comments when this API will be used?
>>> I was assumed it is only used for drd state machine.
>>
>> drd_state machine didn't even need this API in the first place :).
>> You guys wanted me to separate out start/stop and connect/disconnect for
>> full OTG case.
>> Won't full OTG state machine want to use this API? If not what would it
>> use?
>
> Instead create those new interfaces/symbol here and there just aim to
> address build problems in diff configures, Could we only allow meaningful
> combination of those 3 drivers configures?
>
> Hcd=y, gadget=y, otg=y or
> Hcd=m, gadget=m, otg=m
This is still a limitation.
It is perfectly fine to have
hcd=m, gadget=y
or
hcd=y, gadget=m
cheers,
-roger
>
> Li Jun
>
>>
>> cheers,
>> -roger
>>
>>>
>>>>>
>>>>>> return 0;
>>>>>> @@ -660,9 +830,15 @@ static ssize_t usb_udc_softconn_store(struct
>> device *dev,
>>>>>> return -EOPNOTSUPP;
>>>>>> }
>>>>>>
>>>>>> + /* In OTG mode we don't support softconnect, but b_bus_req */
>>>>>> + if (udc->gadget->otg_dev) {
>>>>>> + dev_err(dev, "soft-connect not supported in OTG mode\n");
>>>>>> + return -EOPNOTSUPP;
>>>>>> + }
>>>>>> +
>>>>>
>>>>> The soft-connect can be supported at dual-role mode currently, we
>>>>> can use b_bus_req entry once it is implemented later.
>>>>
>>>> Soft-connect should be done via sysfs handling within the OTG core.
>>>> This can be added later. I don't want anything outside the OTG core
>>>> to handle soft-connect behaviour as it will be hard to keep things in
>>>> sync.
>>>>
>>>> I can update the comment to something like this.
>>>>
>>>> /* In OTG/dual-role mode, soft-connect should be handled by OTG core
>>>> */
>>>
>>> Ok, let's Felipe decide it.
>>>
>>>>
>>>>>
>>>>>> if (sysfs_streq(buf, "connect")) {
>>>>>> usb_gadget_udc_start(udc);
>>>>>> - usb_gadget_connect(udc->gadget);
>>>>>> + usb_udc_connect_control(udc);
>>>>>
>>>>> This line seems to be not related with this patch.
>>>>>
>>>> Right. I'll remove it.
>>>>
>>>> cheers,
>>>> -roger
>>>
[toc] | [prev] | [next] | [standalone]
| From | Jun Li <jun.li@nxp.com> |
|---|---|
| Date | 2016-05-17 10:50 +0200 |
| Subject | RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rzIRr-2gu-3@gated-at.bofh.it> |
| In reply to | #1402185 |
Hi Roger,
> -----Original Message-----
> From: Roger Quadros [mailto:rogerq@ti.com]
> Sent: Tuesday, May 17, 2016 4:09 PM
> To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org;
> linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
>
> On 17/05/16 10:38, Jun Li wrote:
> > Hi
> >
> >> -----Original Message-----
> >> From: Roger Quadros [mailto:rogerq@ti.com]
> >> Sent: Monday, May 16, 2016 5:52 PM
> >> To: Peter Chen <hzpeterchen@gmail.com>
> >> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
> >> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
> >> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
> >> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
> >> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
> >> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com;
> >> linux-usb@vger.kernel.org; linux-omap@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> >> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
> >>
> >> On 16/05/16 12:23, Peter Chen wrote:
> >>> On Mon, May 16, 2016 at 11:26:57AM +0300, Roger Quadros wrote:
> >>>> Hi,
> >>>>
> >>>> On 16/05/16 10:02, Peter Chen wrote:
> >>>>> On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote:
> >>>>>> +
> >>>>>> +static int usb_gadget_connect_control(struct usb_gadget *gadget,
> >>>>>> +bool connect) {
> >>>>>> + struct usb_udc *udc;
> >>>>>> +
> >>>>>> + mutex_lock(&udc_lock);
> >>>>>> + udc = usb_gadget_to_udc(gadget);
> >>>>>> + if (!udc) {
> >>>>>> + dev_err(gadget->dev.parent, "%s: gadget not
> >> registered.\n",
> >>>>>> + __func__);
> >>>>>> + mutex_unlock(&udc_lock);
> >>>>>> + return -EINVAL;
> >>>>>> + }
> >>>>>> +
> >>>>>> + if (connect) {
> >>>>>> + if (!gadget->connected)
> >>>>>> + usb_gadget_connect(udc->gadget);
> >>>>>> + } else {
> >>>>>> + if (gadget->connected) {
> >>>>>> + usb_gadget_disconnect(udc->gadget);
> >>>>>> + udc->driver->disconnect(udc->gadget);
> >>>>>> + }
> >>>>>> + }
> >>>>>> +
> >>>>>> + mutex_unlock(&udc_lock);
> >>>>>> +
> >>>>>> + return 0;
> >>>>>> +}
> >>>>>> +
> >>>>>
> >>>>> Since this is called for vbus interrupt, why not using
> >>>>> usb_udc_vbus_handler directly, and call udc->driver->disconnect at
> >>>>> usb_gadget_stop.
> >>>>
> >>>> We can't assume that this is always called for vbus interrupt so I
> >>>> decided not to call usb_udc_vbus_handler.
> >>>>
> >>>> udc->vbus is really pointless for us. We keep vbus states in our
> >>>> state machine and leave udc->vbus as ture always.
> >>>>
> >>>> Why do you want to move udc->driver->disconnect() to stop?
> >>>> If USB controller disconnected from bus then the gadget driver must
> >>>> be notified about the disconnect immediately. The controller may or
> >>>> may not be stopped by the core.
> >>>>
> >>>
> >>> Then, would you give some comments when this API will be used?
> >>> I was assumed it is only used for drd state machine.
> >>
> >> drd_state machine didn't even need this API in the first place :).
> >> You guys wanted me to separate out start/stop and connect/disconnect
> >> for full OTG case.
> >> Won't full OTG state machine want to use this API? If not what would
> >> it use?
> >
> > Instead create those new interfaces/symbol here and there just aim to
> > address build problems in diff configures, Could we only allow
> > meaningful combination of those 3 drivers configures?
> >
> > Hcd=y, gadget=y, otg=y or
> > Hcd=m, gadget=m, otg=m
>
> This is still a limitation.
>
> It is perfectly fine to have
> hcd=m, gadget=y
> or
> hcd=y, gadget=m
I agree it makes sense to have above configs in non-otg case, that is,
the 'y' driver can work without 'm' driver loaded.
But,
in otg enabled(y/m) case, the otherwise config of my list can't make
any sense from my point view. That is: some driver is built-in, but
it can't work at all if another 'm' driver is not loaded,
in another words, the otg driver has to be 'm' if its dependent driver
is 'm', correct?
Li Jun
>
> cheers,
> -roger
>
> >
> > Li Jun
> >
> >>
> >> cheers,
> >> -roger
> >>
> >>>
> >>>>>
> >>>>>> return 0;
> >>>>>> @@ -660,9 +830,15 @@ static ssize_t usb_udc_softconn_store(struct
> >> device *dev,
> >>>>>> return -EOPNOTSUPP;
> >>>>>> }
> >>>>>>
> >>>>>> + /* In OTG mode we don't support softconnect, but b_bus_req */
> >>>>>> + if (udc->gadget->otg_dev) {
> >>>>>> + dev_err(dev, "soft-connect not supported in OTG mode\n");
> >>>>>> + return -EOPNOTSUPP;
> >>>>>> + }
> >>>>>> +
> >>>>>
> >>>>> The soft-connect can be supported at dual-role mode currently, we
> >>>>> can use b_bus_req entry once it is implemented later.
> >>>>
> >>>> Soft-connect should be done via sysfs handling within the OTG core.
> >>>> This can be added later. I don't want anything outside the OTG core
> >>>> to handle soft-connect behaviour as it will be hard to keep things
> >>>> in sync.
> >>>>
> >>>> I can update the comment to something like this.
> >>>>
> >>>> /* In OTG/dual-role mode, soft-connect should be handled by OTG
> >>>> core */
> >>>
> >>> Ok, let's Felipe decide it.
> >>>
> >>>>
> >>>>>
> >>>>>> if (sysfs_streq(buf, "connect")) {
> >>>>>> usb_gadget_udc_start(udc);
> >>>>>> - usb_gadget_connect(udc->gadget);
> >>>>>> + usb_udc_connect_control(udc);
> >>>>>
> >>>>> This line seems to be not related with this patch.
> >>>>>
> >>>> Right. I'll remove it.
> >>>>
> >>>> cheers,
> >>>> -roger
> >>>
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-18 14:50 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rA95f-2dr-17@gated-at.bofh.it> |
| In reply to | #1402200 |
On 17/05/16 11:28, Jun Li wrote:
> Hi Roger,
>
>> -----Original Message-----
>> From: Roger Quadros [mailto:rogerq@ti.com]
>> Sent: Tuesday, May 17, 2016 4:09 PM
>> To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
>> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
>> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
>> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
>> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
>> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
>> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org;
>> linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org;
>> devicetree@vger.kernel.org
>> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
>>
>> On 17/05/16 10:38, Jun Li wrote:
>>> Hi
>>>
>>>> -----Original Message-----
>>>> From: Roger Quadros [mailto:rogerq@ti.com]
>>>> Sent: Monday, May 16, 2016 5:52 PM
>>>> To: Peter Chen <hzpeterchen@gmail.com>
>>>> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
>>>> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
>>>> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
>>>> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
>>>> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
>>>> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com;
>>>> linux-usb@vger.kernel.org; linux-omap@vger.kernel.org;
>>>> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
>>>> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
>>>>
>>>> On 16/05/16 12:23, Peter Chen wrote:
>>>>> On Mon, May 16, 2016 at 11:26:57AM +0300, Roger Quadros wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 16/05/16 10:02, Peter Chen wrote:
>>>>>>> On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote:
>>>>>>>> +
>>>>>>>> +static int usb_gadget_connect_control(struct usb_gadget *gadget,
>>>>>>>> +bool connect) {
>>>>>>>> + struct usb_udc *udc;
>>>>>>>> +
>>>>>>>> + mutex_lock(&udc_lock);
>>>>>>>> + udc = usb_gadget_to_udc(gadget);
>>>>>>>> + if (!udc) {
>>>>>>>> + dev_err(gadget->dev.parent, "%s: gadget not
>>>> registered.\n",
>>>>>>>> + __func__);
>>>>>>>> + mutex_unlock(&udc_lock);
>>>>>>>> + return -EINVAL;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + if (connect) {
>>>>>>>> + if (!gadget->connected)
>>>>>>>> + usb_gadget_connect(udc->gadget);
>>>>>>>> + } else {
>>>>>>>> + if (gadget->connected) {
>>>>>>>> + usb_gadget_disconnect(udc->gadget);
>>>>>>>> + udc->driver->disconnect(udc->gadget);
>>>>>>>> + }
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + mutex_unlock(&udc_lock);
>>>>>>>> +
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>
>>>>>>> Since this is called for vbus interrupt, why not using
>>>>>>> usb_udc_vbus_handler directly, and call udc->driver->disconnect at
>>>>>>> usb_gadget_stop.
>>>>>>
>>>>>> We can't assume that this is always called for vbus interrupt so I
>>>>>> decided not to call usb_udc_vbus_handler.
>>>>>>
>>>>>> udc->vbus is really pointless for us. We keep vbus states in our
>>>>>> state machine and leave udc->vbus as ture always.
>>>>>>
>>>>>> Why do you want to move udc->driver->disconnect() to stop?
>>>>>> If USB controller disconnected from bus then the gadget driver must
>>>>>> be notified about the disconnect immediately. The controller may or
>>>>>> may not be stopped by the core.
>>>>>>
>>>>>
>>>>> Then, would you give some comments when this API will be used?
>>>>> I was assumed it is only used for drd state machine.
>>>>
>>>> drd_state machine didn't even need this API in the first place :).
>>>> You guys wanted me to separate out start/stop and connect/disconnect
>>>> for full OTG case.
>>>> Won't full OTG state machine want to use this API? If not what would
>>>> it use?
>>>
>>> Instead create those new interfaces/symbol here and there just aim to
>>> address build problems in diff configures, Could we only allow
>>> meaningful combination of those 3 drivers configures?
>>>
>>> Hcd=y, gadget=y, otg=y or
>>> Hcd=m, gadget=m, otg=m
>>
>> This is still a limitation.
>>
>> It is perfectly fine to have
>> hcd=m, gadget=y
>> or
>> hcd=y, gadget=m
>
> I agree it makes sense to have above configs in non-otg case, that is,
> the 'y' driver can work without 'm' driver loaded.
>
> But,
> in otg enabled(y/m) case, the otherwise config of my list can't make
> any sense from my point view. That is: some driver is built-in, but
> it can't work at all if another 'm' driver is not loaded,
>
> in another words, the otg driver has to be 'm' if its dependent driver
> is 'm', correct?
If both host and gadget are 'm' then otg can be 'm', but if either host or
gadget is built in then we have no choice but to make otg as built-in.
I didn't want to have complex Kconfig so decided to have otg as built-in only.
What do you want me to change in existing code? and why?
cheers,
-roger
>
> Li Jun
>
>>
>> cheers,
>> -roger
>>
>>>
>>> Li Jun
>>>
>>>>
>>>> cheers,
>>>> -roger
>>>>
>>>>>
>>>>>>>
>>>>>>>> return 0;
>>>>>>>> @@ -660,9 +830,15 @@ static ssize_t usb_udc_softconn_store(struct
>>>> device *dev,
>>>>>>>> return -EOPNOTSUPP;
>>>>>>>> }
>>>>>>>>
>>>>>>>> + /* In OTG mode we don't support softconnect, but b_bus_req */
>>>>>>>> + if (udc->gadget->otg_dev) {
>>>>>>>> + dev_err(dev, "soft-connect not supported in OTG mode\n");
>>>>>>>> + return -EOPNOTSUPP;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>
>>>>>>> The soft-connect can be supported at dual-role mode currently, we
>>>>>>> can use b_bus_req entry once it is implemented later.
>>>>>>
>>>>>> Soft-connect should be done via sysfs handling within the OTG core.
>>>>>> This can be added later. I don't want anything outside the OTG core
>>>>>> to handle soft-connect behaviour as it will be hard to keep things
>>>>>> in sync.
>>>>>>
>>>>>> I can update the comment to something like this.
>>>>>>
>>>>>> /* In OTG/dual-role mode, soft-connect should be handled by OTG
>>>>>> core */
>>>>>
>>>>> Ok, let's Felipe decide it.
>>>>>
>>>>>>
>>>>>>>
>>>>>>>> if (sysfs_streq(buf, "connect")) {
>>>>>>>> usb_gadget_udc_start(udc);
>>>>>>>> - usb_gadget_connect(udc->gadget);
>>>>>>>> + usb_udc_connect_control(udc);
>>>>>>>
>>>>>>> This line seems to be not related with this patch.
>>>>>>>
>>>>>> Right. I'll remove it.
>>>>>>
>>>>>> cheers,
>>>>>> -roger
>>>>>
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-18 15:50 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rAa1k-2O5-33@gated-at.bofh.it> |
| In reply to | #1402953 |
On 18/05/16 16:12, Jun Li wrote:
> Hi
>
>> -----Original Message-----
>> From: Roger Quadros [mailto:rogerq@ti.com]
>> Sent: Wednesday, May 18, 2016 8:43 PM
>> To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
>> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
>> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
>> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
>> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
>> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
>> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org;
>> linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org;
>> devicetree@vger.kernel.org
>> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
>>
>> On 17/05/16 11:28, Jun Li wrote:
>>> Hi Roger,
>>>
>>>> -----Original Message-----
>>>> From: Roger Quadros [mailto:rogerq@ti.com]
>>>> Sent: Tuesday, May 17, 2016 4:09 PM
>>>> To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
>>>> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
>>>> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
>>>> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
>>>> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
>>>> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
>>>> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com;
>>>> linux-usb@vger.kernel.org; linux-omap@vger.kernel.org;
>>>> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
>>>> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
>>>>
>>>> On 17/05/16 10:38, Jun Li wrote:
>>>>> Hi
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Roger Quadros [mailto:rogerq@ti.com]
>>>>>> Sent: Monday, May 16, 2016 5:52 PM
>>>>>> To: Peter Chen <hzpeterchen@gmail.com>
>>>>>> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
>>>>>> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
>>>>>> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
>>>>>> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
>>>>>> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
>>>>>> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com;
>>>>>> linux-usb@vger.kernel.org; linux-omap@vger.kernel.org;
>>>>>> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
>>>>>> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
>>>>>>
>>>>>> On 16/05/16 12:23, Peter Chen wrote:
>>>>>>> On Mon, May 16, 2016 at 11:26:57AM +0300, Roger Quadros wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 16/05/16 10:02, Peter Chen wrote:
>>>>>>>>> On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote:
>>>>>>>>>> +
>>>>>>>>>> +static int usb_gadget_connect_control(struct usb_gadget
>>>>>>>>>> +*gadget, bool connect) {
>>>>>>>>>> + struct usb_udc *udc;
>>>>>>>>>> +
>>>>>>>>>> + mutex_lock(&udc_lock);
>>>>>>>>>> + udc = usb_gadget_to_udc(gadget);
>>>>>>>>>> + if (!udc) {
>>>>>>>>>> + dev_err(gadget->dev.parent, "%s: gadget not
>>>>>> registered.\n",
>>>>>>>>>> + __func__);
>>>>>>>>>> + mutex_unlock(&udc_lock);
>>>>>>>>>> + return -EINVAL;
>>>>>>>>>> + }
>>>>>>>>>> +
>>>>>>>>>> + if (connect) {
>>>>>>>>>> + if (!gadget->connected)
>>>>>>>>>> + usb_gadget_connect(udc->gadget);
>>>>>>>>>> + } else {
>>>>>>>>>> + if (gadget->connected) {
>>>>>>>>>> + usb_gadget_disconnect(udc->gadget);
>>>>>>>>>> + udc->driver->disconnect(udc->gadget);
>>>>>>>>>> + }
>>>>>>>>>> + }
>>>>>>>>>> +
>>>>>>>>>> + mutex_unlock(&udc_lock);
>>>>>>>>>> +
>>>>>>>>>> + return 0;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>
>>>>>>>>> Since this is called for vbus interrupt, why not using
>>>>>>>>> usb_udc_vbus_handler directly, and call udc->driver->disconnect
>>>>>>>>> at usb_gadget_stop.
>>>>>>>>
>>>>>>>> We can't assume that this is always called for vbus interrupt so
>>>>>>>> I decided not to call usb_udc_vbus_handler.
>>>>>>>>
>>>>>>>> udc->vbus is really pointless for us. We keep vbus states in our
>>>>>>>> state machine and leave udc->vbus as ture always.
>>>>>>>>
>>>>>>>> Why do you want to move udc->driver->disconnect() to stop?
>>>>>>>> If USB controller disconnected from bus then the gadget driver
>>>>>>>> must be notified about the disconnect immediately. The controller
>>>>>>>> may or may not be stopped by the core.
>>>>>>>>
>>>>>>>
>>>>>>> Then, would you give some comments when this API will be used?
>>>>>>> I was assumed it is only used for drd state machine.
>>>>>>
>>>>>> drd_state machine didn't even need this API in the first place :).
>>>>>> You guys wanted me to separate out start/stop and
>>>>>> connect/disconnect for full OTG case.
>>>>>> Won't full OTG state machine want to use this API? If not what
>>>>>> would it use?
>>>>>
>>>>> Instead create those new interfaces/symbol here and there just aim
>>>>> to address build problems in diff configures, Could we only allow
>>>>> meaningful combination of those 3 drivers configures?
>>>>>
>>>>> Hcd=y, gadget=y, otg=y or
>>>>> Hcd=m, gadget=m, otg=m
>>>>
>>>> This is still a limitation.
>>>>
>>>> It is perfectly fine to have
>>>> hcd=m, gadget=y
>>>> or
>>>> hcd=y, gadget=m
>>>
>>> I agree it makes sense to have above configs in non-otg case, that is,
>>> the 'y' driver can work without 'm' driver loaded.
>>>
>>> But,
>>> in otg enabled(y/m) case, the otherwise config of my list can't make
>>> any sense from my point view. That is: some driver is built-in, but it
>>> can't work at all if another 'm' driver is not loaded,
>>>
>>> in another words, the otg driver has to be 'm' if its dependent driver
>>> is 'm', correct?
>>
>> If both host and gadget are 'm' then otg can be 'm', but if either host or
>> gadget is built in then we have no choice but to make otg as built-in.
>>
>> I didn't want to have complex Kconfig so decided to have otg as built-in
>> only.
>> What do you want me to change in existing code? and why?
>
> Remove those stuff which only for pass diff driver config
> Like every controller driver need a duplicated
>
> static struct otg_hcd_ops ci_hcd_ops = {
> ...
> }
This is an exception only. Every controller driver doesn't need to implement
hcd_ops. It is implemented in the hcd core.
>
> And here is another example, for gadget connect, otg driver can
> directly call to usb_udc_vbus_handler() in drd state machine,
> but you create another interface:
>
> .connect_control = usb_gadget_connect_control,
>
> If the symbol is defined in one driver which is 'm', another driver
> reference it should be 'm' as well, then there is no this kind of problem
> as my understanding.
That is fine as long as all are 'm'. but how do you solve the case
when Gadget is built in and host is 'm'? OTG has to be built-in and
you will need an hcd to gadget interface.
Do you have any ideas to solve that case?
cheers,
-roger
>>>>>>
>>>>>>>
>>>>>>>>>
>>>>>>>>>> return 0;
>>>>>>>>>> @@ -660,9 +830,15 @@ static ssize_t
>>>>>>>>>> usb_udc_softconn_store(struct
>>>>>> device *dev,
>>>>>>>>>> return -EOPNOTSUPP;
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> + /* In OTG mode we don't support softconnect, but b_bus_req */
>>>>>>>>>> + if (udc->gadget->otg_dev) {
>>>>>>>>>> + dev_err(dev, "soft-connect not supported in OTG mode\n");
>>>>>>>>>> + return -EOPNOTSUPP;
>>>>>>>>>> + }
>>>>>>>>>> +
>>>>>>>>>
>>>>>>>>> The soft-connect can be supported at dual-role mode currently,
>>>>>>>>> we can use b_bus_req entry once it is implemented later.
>>>>>>>>
>>>>>>>> Soft-connect should be done via sysfs handling within the OTG core.
>>>>>>>> This can be added later. I don't want anything outside the OTG
>>>>>>>> core to handle soft-connect behaviour as it will be hard to keep
>>>>>>>> things in sync.
>>>>>>>>
>>>>>>>> I can update the comment to something like this.
>>>>>>>>
>>>>>>>> /* In OTG/dual-role mode, soft-connect should be handled by OTG
>>>>>>>> core */
>>>>>>>
>>>>>>> Ok, let's Felipe decide it.
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> if (sysfs_streq(buf, "connect")) {
>>>>>>>>>> usb_gadget_udc_start(udc);
>>>>>>>>>> - usb_gadget_connect(udc->gadget);
>>>>>>>>>> + usb_udc_connect_control(udc);
>>>>>>>>>
>>>>>>>>> This line seems to be not related with this patch.
>>>>>>>>>
>>>>>>>> Right. I'll remove it.
>>>>>>>>
>>>>>>>> cheers,
>>>>>>>> -roger
>>>>>>>
[toc] | [prev] | [next] | [standalone]
| From | Jun Li <jun.li@nxp.com> |
|---|---|
| Date | 2016-05-18 16:50 +0200 |
| Subject | RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rAaXn-3oK-11@gated-at.bofh.it> |
| In reply to | #1402993 |
> >>
> >> I didn't want to have complex Kconfig so decided to have otg as
> >> built-in only.
> >> What do you want me to change in existing code? and why?
> >
> > Remove those stuff which only for pass diff driver config Like every
> > controller driver need a duplicated
> >
> > static struct otg_hcd_ops ci_hcd_ops = {
> > ...
> > }
>
> This is an exception only. Every controller driver doesn't need to
> implement hcd_ops. It is implemented in the hcd core.
>
> >
> > And here is another example, for gadget connect, otg driver can
> > directly call to usb_udc_vbus_handler() in drd state machine, but you
> > create another interface:
> >
> > .connect_control = usb_gadget_connect_control,
> >
> > If the symbol is defined in one driver which is 'm', another driver
> > reference it should be 'm' as well, then there is no this kind of
> > problem as my understanding.
>
> That is fine as long as all are 'm'. but how do you solve the case when
> Gadget is built in and host is 'm'? OTG has to be built-in and you will
> need an hcd to gadget interface.
Hcd to gadget interface? Or you want to say otg to host interface?
I think hcd and gadget are independent each other, now
Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)
If you directly call to usb_udc_vbus_handler() in drd state machine
Then:
Hcd --> otg; and gadget <--> otg, (gadget and otg will refer to symbol of each other)
Li Jun
>
> Do you have any ideas to solve that case?
>
> cheers,
> -roger
>
> >>>>>>
> >>>>>>>
> >>>>>>>>>
> >>>>>>>>>> return 0;
> >>>>>>>>>> @@ -660,9 +830,15 @@ static ssize_t
> >>>>>>>>>> usb_udc_softconn_store(struct
> >>>>>> device *dev,
> >>>>>>>>>> return -EOPNOTSUPP;
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>> + /* In OTG mode we don't support softconnect, but
> b_bus_req */
> >>>>>>>>>> + if (udc->gadget->otg_dev) {
> >>>>>>>>>> + dev_err(dev, "soft-connect not supported in OTG
> mode\n");
> >>>>>>>>>> + return -EOPNOTSUPP;
> >>>>>>>>>> + }
> >>>>>>>>>> +
> >>>>>>>>>
> >>>>>>>>> The soft-connect can be supported at dual-role mode currently,
> >>>>>>>>> we can use b_bus_req entry once it is implemented later.
> >>>>>>>>
> >>>>>>>> Soft-connect should be done via sysfs handling within the OTG
> core.
> >>>>>>>> This can be added later. I don't want anything outside the OTG
> >>>>>>>> core to handle soft-connect behaviour as it will be hard to
> >>>>>>>> keep things in sync.
> >>>>>>>>
> >>>>>>>> I can update the comment to something like this.
> >>>>>>>>
> >>>>>>>> /* In OTG/dual-role mode, soft-connect should be handled by OTG
> >>>>>>>> core */
> >>>>>>>
> >>>>>>> Ok, let's Felipe decide it.
> >>>>>>>
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> if (sysfs_streq(buf, "connect")) {
> >>>>>>>>>> usb_gadget_udc_start(udc);
> >>>>>>>>>> - usb_gadget_connect(udc->gadget);
> >>>>>>>>>> + usb_udc_connect_control(udc);
> >>>>>>>>>
> >>>>>>>>> This line seems to be not related with this patch.
> >>>>>>>>>
> >>>>>>>> Right. I'll remove it.
> >>>>>>>>
> >>>>>>>> cheers,
> >>>>>>>> -roger
> >>>>>>>
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-19 09:40 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rAqIN-5bD-1@gated-at.bofh.it> |
| In reply to | #1403043 |
On 18/05/16 17:46, Jun Li wrote:
>
>
>>>>
>>>> I didn't want to have complex Kconfig so decided to have otg as
>>>> built-in only.
>>>> What do you want me to change in existing code? and why?
>>>
>>> Remove those stuff which only for pass diff driver config Like every
>>> controller driver need a duplicated
>>>
>>> static struct otg_hcd_ops ci_hcd_ops = {
>>> ...
>>> }
>>
>> This is an exception only. Every controller driver doesn't need to
>> implement hcd_ops. It is implemented in the hcd core.
>>
>>>
>>> And here is another example, for gadget connect, otg driver can
>>> directly call to usb_udc_vbus_handler() in drd state machine, but you
>>> create another interface:
>>>
>>> .connect_control = usb_gadget_connect_control,
>>>
>>> If the symbol is defined in one driver which is 'm', another driver
>>> reference it should be 'm' as well, then there is no this kind of
>>> problem as my understanding.
>>
>> That is fine as long as all are 'm'. but how do you solve the case when
>> Gadget is built in and host is 'm'? OTG has to be built-in and you will
>> need an hcd to gadget interface.
>
> Hcd to gadget interface? Or you want to say otg to host interface?
Sorry, I meant to say host to otg interface.
>
> I think hcd and gadget are independent each other, now
>
> Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)
It is actually a circular dependency for both.
hcd <--> otg and gadget <--> otg
hcd -> otg for usb_otg_register/unregister_hcd
otg -> hcd for usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg, usb_hub_find_child
gadget -> otg for usb_otg_register/unregister_gadget
otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler
Now consider what will happen if I get rid of the otg_hcd and otg_gadget interfaces.
'y' means built-in, 'm' means module.
1) hcd 'y', gadget 'y'
otg has to be 'y' for proper build.
2) hcd 'm', gadget 'm'
otg has to be 'm' for proper build.
3) hcd 'y', gadget 'm'
Build will fail always.
If otg is 'y', otg build will fail due to dependency on gadget.
If otg is 'm', hcd build will fail due to dependency on otg.
4) hcd 'm', gadget 'y'
Build will fail always.
If otg is 'y', otg build will fail due to dependency on hcd.
If otg is 'm', gadget build will fails due to dependency on otg.
So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops
to remove otg->hcd and otg->gadget dependency.
Now we can address 3) and 4) like so
3) hcd 'y', gadget 'm'
otg has to be 'y' for proper build.
4) hcd 'm', gadget 'y'
otg has to be 'y' for proper build.
>
> If you directly call to usb_udc_vbus_handler() in drd state machine
>
> Then:
>
> Hcd --> otg; and gadget <--> otg, (gadget and otg will refer to symbol of each other)
It's not so easy. We are again creating a circular dependency and all
build configurations don't work like I pointed above.
The only optimization I could do is make CONFIG_OTG tristate and
allow it to be built as 'm' if both hcd and gadget are 'm'.
i.e. case (2).
cheers,
-roger
>>
>> Do you have any ideas to solve that case?
>>
>> cheers,
>> -roger
>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> return 0;
>>>>>>>>>>>> @@ -660,9 +830,15 @@ static ssize_t
>>>>>>>>>>>> usb_udc_softconn_store(struct
>>>>>>>> device *dev,
>>>>>>>>>>>> return -EOPNOTSUPP;
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> + /* In OTG mode we don't support softconnect, but
>> b_bus_req */
>>>>>>>>>>>> + if (udc->gadget->otg_dev) {
>>>>>>>>>>>> + dev_err(dev, "soft-connect not supported in OTG
>> mode\n");
>>>>>>>>>>>> + return -EOPNOTSUPP;
>>>>>>>>>>>> + }
>>>>>>>>>>>> +
>>>>>>>>>>>
>>>>>>>>>>> The soft-connect can be supported at dual-role mode currently,
>>>>>>>>>>> we can use b_bus_req entry once it is implemented later.
>>>>>>>>>>
>>>>>>>>>> Soft-connect should be done via sysfs handling within the OTG
>> core.
>>>>>>>>>> This can be added later. I don't want anything outside the OTG
>>>>>>>>>> core to handle soft-connect behaviour as it will be hard to
>>>>>>>>>> keep things in sync.
>>>>>>>>>>
>>>>>>>>>> I can update the comment to something like this.
>>>>>>>>>>
>>>>>>>>>> /* In OTG/dual-role mode, soft-connect should be handled by OTG
>>>>>>>>>> core */
>>>>>>>>>
>>>>>>>>> Ok, let's Felipe decide it.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> if (sysfs_streq(buf, "connect")) {
>>>>>>>>>>>> usb_gadget_udc_start(udc);
>>>>>>>>>>>> - usb_gadget_connect(udc->gadget);
>>>>>>>>>>>> + usb_udc_connect_control(udc);
>>>>>>>>>>>
>>>>>>>>>>> This line seems to be not related with this patch.
>>>>>>>>>>>
>>>>>>>>>> Right. I'll remove it.
>>>>>>>>>>
>>>>>>>>>> cheers,
>>>>>>>>>> -roger
>>>>>>>>>
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-05-21 04:40 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rB4Zz-69T-9@gated-at.bofh.it> |
| In reply to | #1403476 |
On Thu, May 19, 2016 at 10:32:44AM +0300, Roger Quadros wrote:
> On 18/05/16 17:46, Jun Li wrote:
> >
> >
> >>>>
> >>>> I didn't want to have complex Kconfig so decided to have otg as
> >>>> built-in only.
> >>>> What do you want me to change in existing code? and why?
> >>>
> >>> Remove those stuff which only for pass diff driver config Like every
> >>> controller driver need a duplicated
> >>>
> >>> static struct otg_hcd_ops ci_hcd_ops = {
> >>> ...
> >>> }
> >>
> >> This is an exception only. Every controller driver doesn't need to
> >> implement hcd_ops. It is implemented in the hcd core.
> >>
> >>>
> >>> And here is another example, for gadget connect, otg driver can
> >>> directly call to usb_udc_vbus_handler() in drd state machine, but you
> >>> create another interface:
> >>>
> >>> .connect_control = usb_gadget_connect_control,
> >>>
> >>> If the symbol is defined in one driver which is 'm', another driver
> >>> reference it should be 'm' as well, then there is no this kind of
> >>> problem as my understanding.
> >>
> >> That is fine as long as all are 'm'. but how do you solve the case when
> >> Gadget is built in and host is 'm'? OTG has to be built-in and you will
> >> need an hcd to gadget interface.
> >
> > Hcd to gadget interface? Or you want to say otg to host interface?
>
> Sorry, I meant to say host to otg interface.
>
> >
> > I think hcd and gadget are independent each other, now
> >
> > Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)
>
> It is actually a circular dependency for both.
> hcd <--> otg and gadget <--> otg
>
> hcd -> otg for usb_otg_register/unregister_hcd
> otg -> hcd for usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg, usb_hub_find_child
>
> gadget -> otg for usb_otg_register/unregister_gadget
> otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler
>
> Now consider what will happen if I get rid of the otg_hcd and otg_gadget interfaces.
> 'y' means built-in, 'm' means module.
>
> 1) hcd 'y', gadget 'y'
> otg has to be 'y' for proper build.
>
> 2) hcd 'm', gadget 'm'
> otg has to be 'm' for proper build.
>
> 3) hcd 'y', gadget 'm'
> Build will fail always.
> If otg is 'y', otg build will fail due to dependency on gadget.
> If otg is 'm', hcd build will fail due to dependency on otg.
>
> 4) hcd 'm', gadget 'y'
> Build will fail always.
> If otg is 'y', otg build will fail due to dependency on hcd.
> If otg is 'm', gadget build will fails due to dependency on otg.
>
> So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops
> to remove otg->hcd and otg->gadget dependency.
>
> Now we can address 3) and 4) like so
>
> 3) hcd 'y', gadget 'm'
> otg has to be 'y' for proper build.
>
> 4) hcd 'm', gadget 'y'
> otg has to be 'y' for proper build.
>
How about this:
Moving usb_otg_register/unregister_hcd to host driver to remove
dependency hcd->otg. And moving usb_otg_get_data to common.c.
Delete the wait queue at usb-otg.c, and if calling usb_otg_get_data
returns NULL, the host/device driver's probe return -EPROBE_DEFER.
When the otg driver is probed successfully, the host/device will be
re-probed again, and usb_otg_register_hcd will be called again.
And let OTG depends on HCD && GADGET, and delete otg_hcd_ops and
otg_gadget_ops. Below build dependency issues can be fixed.
What do you think?
> 1) hcd 'y', gadget 'y'
> otg has to be 'y' for proper build.
>
> 2) hcd 'm', gadget 'm'
> otg has to be 'm' for proper build.
>
> 3) hcd 'y', gadget 'm'
> Build will fail always.
> If otg is 'y', otg build will fail due to dependency on gadget.
> If otg is 'm', hcd build will fail due to dependency on otg.
>
> 4) hcd 'm', gadget 'y'
> Build will fail always.
> If otg is 'y', otg build will fail due to dependency on hcd.
> If otg is 'm', gadget build will fails due to dependency on otg.
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-05-23 05:30 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rBOJ3-Fj-1@gated-at.bofh.it> |
| In reply to | #1404792 |
On Sat, May 21, 2016 at 10:29:40AM +0800, Peter Chen wrote:
> On Thu, May 19, 2016 at 10:32:44AM +0300, Roger Quadros wrote:
> > On 18/05/16 17:46, Jun Li wrote:
> > >
> > >
> > >>>>
> > >>>> I didn't want to have complex Kconfig so decided to have otg as
> > >>>> built-in only.
> > >>>> What do you want me to change in existing code? and why?
> > >>>
> > >>> Remove those stuff which only for pass diff driver config Like every
> > >>> controller driver need a duplicated
> > >>>
> > >>> static struct otg_hcd_ops ci_hcd_ops = {
> > >>> ...
> > >>> }
> > >>
> > >> This is an exception only. Every controller driver doesn't need to
> > >> implement hcd_ops. It is implemented in the hcd core.
> > >>
> > >>>
> > >>> And here is another example, for gadget connect, otg driver can
> > >>> directly call to usb_udc_vbus_handler() in drd state machine, but you
> > >>> create another interface:
> > >>>
> > >>> .connect_control = usb_gadget_connect_control,
> > >>>
> > >>> If the symbol is defined in one driver which is 'm', another driver
> > >>> reference it should be 'm' as well, then there is no this kind of
> > >>> problem as my understanding.
> > >>
> > >> That is fine as long as all are 'm'. but how do you solve the case when
> > >> Gadget is built in and host is 'm'? OTG has to be built-in and you will
> > >> need an hcd to gadget interface.
> > >
> > > Hcd to gadget interface? Or you want to say otg to host interface?
> >
> > Sorry, I meant to say host to otg interface.
> >
> > >
> > > I think hcd and gadget are independent each other, now
> > >
> > > Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)
> >
> > It is actually a circular dependency for both.
> > hcd <--> otg and gadget <--> otg
> >
> > hcd -> otg for usb_otg_register/unregister_hcd
> > otg -> hcd for usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg, usb_hub_find_child
> >
> > gadget -> otg for usb_otg_register/unregister_gadget
> > otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler
> >
> > Now consider what will happen if I get rid of the otg_hcd and otg_gadget interfaces.
> > 'y' means built-in, 'm' means module.
> >
> > 1) hcd 'y', gadget 'y'
> > otg has to be 'y' for proper build.
> >
> > 2) hcd 'm', gadget 'm'
> > otg has to be 'm' for proper build.
> >
> > 3) hcd 'y', gadget 'm'
> > Build will fail always.
> > If otg is 'y', otg build will fail due to dependency on gadget.
> > If otg is 'm', hcd build will fail due to dependency on otg.
> >
> > 4) hcd 'm', gadget 'y'
> > Build will fail always.
> > If otg is 'y', otg build will fail due to dependency on hcd.
> > If otg is 'm', gadget build will fails due to dependency on otg.
> >
> > So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops
> > to remove otg->hcd and otg->gadget dependency.
> >
> > Now we can address 3) and 4) like so
> >
> > 3) hcd 'y', gadget 'm'
> > otg has to be 'y' for proper build.
> >
> > 4) hcd 'm', gadget 'y'
> > otg has to be 'y' for proper build.
> >
>
> How about this:
> Moving usb_otg_register/unregister_hcd to host driver to remove
> dependency hcd->otg. And moving usb_otg_get_data to common.c.
>
> Delete the wait queue at usb-otg.c, and if calling usb_otg_get_data
> returns NULL, the host/device driver's probe return -EPROBE_DEFER.
> When the otg driver is probed successfully, the host/device will be
> re-probed again, and usb_otg_register_hcd will be called again.
>
> And let OTG depends on HCD && GADGET, and delete otg_hcd_ops and
> otg_gadget_ops. Below build dependency issues can be fixed.
> What do you think?
>
> > 1) hcd 'y', gadget 'y'
> > otg has to be 'y' for proper build.
> >
> > 2) hcd 'm', gadget 'm'
> > otg has to be 'm' for proper build.
> >
> > 3) hcd 'y', gadget 'm'
> > Build will fail always.
> > If otg is 'y', otg build will fail due to dependency on gadget.
> > If otg is 'm', hcd build will fail due to dependency on otg.
> >
> > 4) hcd 'm', gadget 'y'
> > Build will fail always.
> > If otg is 'y', otg build will fail due to dependency on hcd.
> > If otg is 'm', gadget build will fails due to dependency on otg.
> --
>
After thinking more, my suggestion can't work. How about moving
CONFIG_USB_OTG out of CONFIG_USB, in that case, CONFIG_USB_OTG
can only be built in.
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-23 12:20 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rBV7Q-4y5-23@gated-at.bofh.it> |
| In reply to | #1405078 |
On 23/05/16 06:21, Peter Chen wrote:
> On Sat, May 21, 2016 at 10:29:40AM +0800, Peter Chen wrote:
>> On Thu, May 19, 2016 at 10:32:44AM +0300, Roger Quadros wrote:
>>> On 18/05/16 17:46, Jun Li wrote:
>>>>
>>>>
>>>>>>>
>>>>>>> I didn't want to have complex Kconfig so decided to have otg as
>>>>>>> built-in only.
>>>>>>> What do you want me to change in existing code? and why?
>>>>>>
>>>>>> Remove those stuff which only for pass diff driver config Like every
>>>>>> controller driver need a duplicated
>>>>>>
>>>>>> static struct otg_hcd_ops ci_hcd_ops = {
>>>>>> ...
>>>>>> }
>>>>>
>>>>> This is an exception only. Every controller driver doesn't need to
>>>>> implement hcd_ops. It is implemented in the hcd core.
>>>>>
>>>>>>
>>>>>> And here is another example, for gadget connect, otg driver can
>>>>>> directly call to usb_udc_vbus_handler() in drd state machine, but you
>>>>>> create another interface:
>>>>>>
>>>>>> .connect_control = usb_gadget_connect_control,
>>>>>>
>>>>>> If the symbol is defined in one driver which is 'm', another driver
>>>>>> reference it should be 'm' as well, then there is no this kind of
>>>>>> problem as my understanding.
>>>>>
>>>>> That is fine as long as all are 'm'. but how do you solve the case when
>>>>> Gadget is built in and host is 'm'? OTG has to be built-in and you will
>>>>> need an hcd to gadget interface.
>>>>
>>>> Hcd to gadget interface? Or you want to say otg to host interface?
>>>
>>> Sorry, I meant to say host to otg interface.
>>>
>>>>
>>>> I think hcd and gadget are independent each other, now
>>>>
>>>> Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)
>>>
>>> It is actually a circular dependency for both.
>>> hcd <--> otg and gadget <--> otg
>>>
>>> hcd -> otg for usb_otg_register/unregister_hcd
>>> otg -> hcd for usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg, usb_hub_find_child
>>>
>>> gadget -> otg for usb_otg_register/unregister_gadget
>>> otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler
>>>
>>> Now consider what will happen if I get rid of the otg_hcd and otg_gadget interfaces.
>>> 'y' means built-in, 'm' means module.
>>>
>>> 1) hcd 'y', gadget 'y'
>>> otg has to be 'y' for proper build.
>>>
>>> 2) hcd 'm', gadget 'm'
>>> otg has to be 'm' for proper build.
>>>
>>> 3) hcd 'y', gadget 'm'
>>> Build will fail always.
>>> If otg is 'y', otg build will fail due to dependency on gadget.
>>> If otg is 'm', hcd build will fail due to dependency on otg.
>>>
>>> 4) hcd 'm', gadget 'y'
>>> Build will fail always.
>>> If otg is 'y', otg build will fail due to dependency on hcd.
>>> If otg is 'm', gadget build will fails due to dependency on otg.
>>>
>>> So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops
>>> to remove otg->hcd and otg->gadget dependency.
>>>
>>> Now we can address 3) and 4) like so
>>>
>>> 3) hcd 'y', gadget 'm'
>>> otg has to be 'y' for proper build.
>>>
>>> 4) hcd 'm', gadget 'y'
>>> otg has to be 'y' for proper build.
>>>
>>
>> How about this:
>> Moving usb_otg_register/unregister_hcd to host driver to remove
>> dependency hcd->otg. And moving usb_otg_get_data to common.c.
>>
>> Delete the wait queue at usb-otg.c, and if calling usb_otg_get_data
>> returns NULL, the host/device driver's probe return -EPROBE_DEFER.
>> When the otg driver is probed successfully, the host/device will be
>> re-probed again, and usb_otg_register_hcd will be called again.
>>
>> And let OTG depends on HCD && GADGET, and delete otg_hcd_ops and
>> otg_gadget_ops. Below build dependency issues can be fixed.
>> What do you think?
>>
>>> 1) hcd 'y', gadget 'y'
>>> otg has to be 'y' for proper build.
>>>
>>> 2) hcd 'm', gadget 'm'
>>> otg has to be 'm' for proper build.
>>>
>>> 3) hcd 'y', gadget 'm'
>>> Build will fail always.
>>> If otg is 'y', otg build will fail due to dependency on gadget.
>>> If otg is 'm', hcd build will fail due to dependency on otg.
>>>
>>> 4) hcd 'm', gadget 'y'
>>> Build will fail always.
>>> If otg is 'y', otg build will fail due to dependency on hcd.
>>> If otg is 'm', gadget build will fails due to dependency on otg.
>> --
>>
>
> After thinking more, my suggestion can't work. How about moving
> CONFIG_USB_OTG out of CONFIG_USB, in that case, CONFIG_USB_OTG
> can only be built in.
>
I tried this but it still doesn't resolve 3 and 4. I.e.
it can't be automatically set to 'y' when either of hcd/gadget is 'y'
and the other is 'm'.
I think some kconfig trickery can be done to get what we want.
cheers,
-roger
[toc] | [prev] | [next] | [standalone]
| From | Jun Li <jun.li@nxp.com> |
|---|---|
| Date | 2016-05-23 12:40 +0200 |
| Subject | RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rBVrb-4Ex-3@gated-at.bofh.it> |
| In reply to | #1405213 |
Hi
> -----Original Message-----
> From: Roger Quadros [mailto:rogerq@ti.com]
> Sent: Monday, May 23, 2016 6:12 PM
> To: Peter Chen <hzpeterchen@gmail.com>
> Cc: Jun Li <jun.li@nxp.com>; peter.chen@freescale.com; balbi@kernel.org;
> tony@atomide.com; gregkh@linuxfoundation.org; dan.j.williams@intel.com;
> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org;
> linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
>
> On 23/05/16 06:21, Peter Chen wrote:
> > On Sat, May 21, 2016 at 10:29:40AM +0800, Peter Chen wrote:
> >> On Thu, May 19, 2016 at 10:32:44AM +0300, Roger Quadros wrote:
> >>> On 18/05/16 17:46, Jun Li wrote:
> >>>>
> >>>>
> >>>>>>>
> >>>>>>> I didn't want to have complex Kconfig so decided to have otg as
> >>>>>>> built-in only.
> >>>>>>> What do you want me to change in existing code? and why?
> >>>>>>
> >>>>>> Remove those stuff which only for pass diff driver config Like
> >>>>>> every controller driver need a duplicated
> >>>>>>
> >>>>>> static struct otg_hcd_ops ci_hcd_ops = {
> >>>>>> ...
> >>>>>> }
> >>>>>
> >>>>> This is an exception only. Every controller driver doesn't need to
> >>>>> implement hcd_ops. It is implemented in the hcd core.
> >>>>>
> >>>>>>
> >>>>>> And here is another example, for gadget connect, otg driver can
> >>>>>> directly call to usb_udc_vbus_handler() in drd state machine, but
> >>>>>> you create another interface:
> >>>>>>
> >>>>>> .connect_control = usb_gadget_connect_control,
> >>>>>>
> >>>>>> If the symbol is defined in one driver which is 'm', another
> >>>>>> driver reference it should be 'm' as well, then there is no this
> >>>>>> kind of problem as my understanding.
> >>>>>
> >>>>> That is fine as long as all are 'm'. but how do you solve the case
> >>>>> when Gadget is built in and host is 'm'? OTG has to be built-in
> >>>>> and you will need an hcd to gadget interface.
> >>>>
> >>>> Hcd to gadget interface? Or you want to say otg to host interface?
> >>>
> >>> Sorry, I meant to say host to otg interface.
> >>>
> >>>>
> >>>> I think hcd and gadget are independent each other, now
> >>>>
> >>>> Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)
> >>>
> >>> It is actually a circular dependency for both.
> >>> hcd <--> otg and gadget <--> otg
> >>>
> >>> hcd -> otg for usb_otg_register/unregister_hcd otg -> hcd for
> >>> usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg,
> >>> usb_hub_find_child
> >>>
> >>> gadget -> otg for usb_otg_register/unregister_gadget
> >>> otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler
> >>>
> >>> Now consider what will happen if I get rid of the otg_hcd and
> otg_gadget interfaces.
> >>> 'y' means built-in, 'm' means module.
> >>>
> >>> 1) hcd 'y', gadget 'y'
> >>> otg has to be 'y' for proper build.
> >>>
> >>> 2) hcd 'm', gadget 'm'
> >>> otg has to be 'm' for proper build.
> >>>
> >>> 3) hcd 'y', gadget 'm'
> >>> Build will fail always.
> >>> If otg is 'y', otg build will fail due to dependency on gadget.
> >>> If otg is 'm', hcd build will fail due to dependency on otg.
> >>>
> >>> 4) hcd 'm', gadget 'y'
> >>> Build will fail always.
> >>> If otg is 'y', otg build will fail due to dependency on hcd.
> >>> If otg is 'm', gadget build will fails due to dependency on otg.
> >>>
> >>> So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops
> >>> to remove otg->hcd and otg->gadget dependency.
> >>>
> >>> Now we can address 3) and 4) like so
> >>>
> >>> 3) hcd 'y', gadget 'm'
> >>> otg has to be 'y' for proper build.
> >>>
> >>> 4) hcd 'm', gadget 'y'
> >>> otg has to be 'y' for proper build.
> >>>
> >>
> >> How about this:
> >> Moving usb_otg_register/unregister_hcd to host driver to remove
> >> dependency hcd->otg. And moving usb_otg_get_data to common.c.
> >>
> >> Delete the wait queue at usb-otg.c, and if calling usb_otg_get_data
> >> returns NULL, the host/device driver's probe return -EPROBE_DEFER.
> >> When the otg driver is probed successfully, the host/device will be
> >> re-probed again, and usb_otg_register_hcd will be called again.
> >>
> >> And let OTG depends on HCD && GADGET, and delete otg_hcd_ops and
> >> otg_gadget_ops. Below build dependency issues can be fixed.
> >> What do you think?
> >>
> >>> 1) hcd 'y', gadget 'y'
> >>> otg has to be 'y' for proper build.
> >>>
> >>> 2) hcd 'm', gadget 'm'
> >>> otg has to be 'm' for proper build.
> >>>
> >>> 3) hcd 'y', gadget 'm'
> >>> Build will fail always.
> >>> If otg is 'y', otg build will fail due to dependency on gadget.
> >>> If otg is 'm', hcd build will fail due to dependency on otg.
> >>>
> >>> 4) hcd 'm', gadget 'y'
> >>> Build will fail always.
> >>> If otg is 'y', otg build will fail due to dependency on hcd.
> >>> If otg is 'm', gadget build will fails due to dependency on otg.
> >> --
> >>
> >
> > After thinking more, my suggestion can't work. How about moving
> > CONFIG_USB_OTG out of CONFIG_USB, in that case, CONFIG_USB_OTG can
> > only be built in.
> >
> I tried this but it still doesn't resolve 3 and 4. I.e.
> it can't be automatically set to 'y' when either of hcd/gadget is 'y'
> and the other is 'm'.
USB_OTG only can be selected by *user*, can't be automatically set to
'y' like you said, no matter what's hcd/gadget config, USB_OTG can be
disabled in any cases.
I think Peter's intention is to make USB_OTG to be 'bool', either disabled;
or built-in(if enabled), something like your original idea?
Li Jun
>
> I think some kconfig trickery can be done to get what we want.
>
> cheers,
> -roger
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2016-05-23 12:40 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rBVrc-4Ex-33@gated-at.bofh.it> |
| In reply to | #1405218 |
On 23/05/16 13:34, Jun Li wrote:
> Hi
>
>> -----Original Message-----
>> From: Roger Quadros [mailto:rogerq@ti.com]
>> Sent: Monday, May 23, 2016 6:12 PM
>> To: Peter Chen <hzpeterchen@gmail.com>
>> Cc: Jun Li <jun.li@nxp.com>; peter.chen@freescale.com; balbi@kernel.org;
>> tony@atomide.com; gregkh@linuxfoundation.org; dan.j.williams@intel.com;
>> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
>> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
>> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
>> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org;
>> linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org;
>> devicetree@vger.kernel.org
>> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
>>
>> On 23/05/16 06:21, Peter Chen wrote:
>>> On Sat, May 21, 2016 at 10:29:40AM +0800, Peter Chen wrote:
>>>> On Thu, May 19, 2016 at 10:32:44AM +0300, Roger Quadros wrote:
>>>>> On 18/05/16 17:46, Jun Li wrote:
>>>>>>
>>>>>>
>>>>>>>>>
>>>>>>>>> I didn't want to have complex Kconfig so decided to have otg as
>>>>>>>>> built-in only.
>>>>>>>>> What do you want me to change in existing code? and why?
>>>>>>>>
>>>>>>>> Remove those stuff which only for pass diff driver config Like
>>>>>>>> every controller driver need a duplicated
>>>>>>>>
>>>>>>>> static struct otg_hcd_ops ci_hcd_ops = {
>>>>>>>> ...
>>>>>>>> }
>>>>>>>
>>>>>>> This is an exception only. Every controller driver doesn't need to
>>>>>>> implement hcd_ops. It is implemented in the hcd core.
>>>>>>>
>>>>>>>>
>>>>>>>> And here is another example, for gadget connect, otg driver can
>>>>>>>> directly call to usb_udc_vbus_handler() in drd state machine, but
>>>>>>>> you create another interface:
>>>>>>>>
>>>>>>>> .connect_control = usb_gadget_connect_control,
>>>>>>>>
>>>>>>>> If the symbol is defined in one driver which is 'm', another
>>>>>>>> driver reference it should be 'm' as well, then there is no this
>>>>>>>> kind of problem as my understanding.
>>>>>>>
>>>>>>> That is fine as long as all are 'm'. but how do you solve the case
>>>>>>> when Gadget is built in and host is 'm'? OTG has to be built-in
>>>>>>> and you will need an hcd to gadget interface.
>>>>>>
>>>>>> Hcd to gadget interface? Or you want to say otg to host interface?
>>>>>
>>>>> Sorry, I meant to say host to otg interface.
>>>>>
>>>>>>
>>>>>> I think hcd and gadget are independent each other, now
>>>>>>
>>>>>> Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)
>>>>>
>>>>> It is actually a circular dependency for both.
>>>>> hcd <--> otg and gadget <--> otg
>>>>>
>>>>> hcd -> otg for usb_otg_register/unregister_hcd otg -> hcd for
>>>>> usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg,
>>>>> usb_hub_find_child
>>>>>
>>>>> gadget -> otg for usb_otg_register/unregister_gadget
>>>>> otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler
>>>>>
>>>>> Now consider what will happen if I get rid of the otg_hcd and
>> otg_gadget interfaces.
>>>>> 'y' means built-in, 'm' means module.
>>>>>
>>>>> 1) hcd 'y', gadget 'y'
>>>>> otg has to be 'y' for proper build.
>>>>>
>>>>> 2) hcd 'm', gadget 'm'
>>>>> otg has to be 'm' for proper build.
>>>>>
>>>>> 3) hcd 'y', gadget 'm'
>>>>> Build will fail always.
>>>>> If otg is 'y', otg build will fail due to dependency on gadget.
>>>>> If otg is 'm', hcd build will fail due to dependency on otg.
>>>>>
>>>>> 4) hcd 'm', gadget 'y'
>>>>> Build will fail always.
>>>>> If otg is 'y', otg build will fail due to dependency on hcd.
>>>>> If otg is 'm', gadget build will fails due to dependency on otg.
>>>>>
>>>>> So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops
>>>>> to remove otg->hcd and otg->gadget dependency.
>>>>>
>>>>> Now we can address 3) and 4) like so
>>>>>
>>>>> 3) hcd 'y', gadget 'm'
>>>>> otg has to be 'y' for proper build.
>>>>>
>>>>> 4) hcd 'm', gadget 'y'
>>>>> otg has to be 'y' for proper build.
>>>>>
>>>>
>>>> How about this:
>>>> Moving usb_otg_register/unregister_hcd to host driver to remove
>>>> dependency hcd->otg. And moving usb_otg_get_data to common.c.
>>>>
>>>> Delete the wait queue at usb-otg.c, and if calling usb_otg_get_data
>>>> returns NULL, the host/device driver's probe return -EPROBE_DEFER.
>>>> When the otg driver is probed successfully, the host/device will be
>>>> re-probed again, and usb_otg_register_hcd will be called again.
>>>>
>>>> And let OTG depends on HCD && GADGET, and delete otg_hcd_ops and
>>>> otg_gadget_ops. Below build dependency issues can be fixed.
>>>> What do you think?
>>>>
>>>>> 1) hcd 'y', gadget 'y'
>>>>> otg has to be 'y' for proper build.
>>>>>
>>>>> 2) hcd 'm', gadget 'm'
>>>>> otg has to be 'm' for proper build.
>>>>>
>>>>> 3) hcd 'y', gadget 'm'
>>>>> Build will fail always.
>>>>> If otg is 'y', otg build will fail due to dependency on gadget.
>>>>> If otg is 'm', hcd build will fail due to dependency on otg.
>>>>>
>>>>> 4) hcd 'm', gadget 'y'
>>>>> Build will fail always.
>>>>> If otg is 'y', otg build will fail due to dependency on hcd.
>>>>> If otg is 'm', gadget build will fails due to dependency on otg.
>>>> --
>>>>
>>>
>>> After thinking more, my suggestion can't work. How about moving
>>> CONFIG_USB_OTG out of CONFIG_USB, in that case, CONFIG_USB_OTG can
>>> only be built in.
>>>
>> I tried this but it still doesn't resolve 3 and 4. I.e.
>> it can't be automatically set to 'y' when either of hcd/gadget is 'y'
>> and the other is 'm'.
>
> USB_OTG only can be selected by *user*, can't be automatically set to
> 'y' like you said, no matter what's hcd/gadget config, USB_OTG can be
> disabled in any cases.
Yes. I am on the same page with that.
>
> I think Peter's intention is to make USB_OTG to be 'bool', either disabled;
> or built-in(if enabled), something like your original idea?
I'm fine with my original idea if you guys can confirm :).
cheers,
-roger
>
> Li Jun
>
>>
>> I think some kconfig trickery can be done to get what we want.
>>
>> cheers,
>> -roger
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-05-24 05:00 +0200 |
| Subject | Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rCaJz-5IK-1@gated-at.bofh.it> |
| In reply to | #1405224 |
On Mon, May 23, 2016 at 01:36:51PM +0300, Roger Quadros wrote:
> On 23/05/16 13:34, Jun Li wrote:
> > Hi
> >
> >> -----Original Message-----
> >> From: Roger Quadros [mailto:rogerq@ti.com]
> >> Sent: Monday, May 23, 2016 6:12 PM
> >> To: Peter Chen <hzpeterchen@gmail.com>
> >> Cc: Jun Li <jun.li@nxp.com>; peter.chen@freescale.com; balbi@kernel.org;
> >> tony@atomide.com; gregkh@linuxfoundation.org; dan.j.williams@intel.com;
> >> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
> >> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
> >> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
> >> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org;
> >> linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> devicetree@vger.kernel.org
> >> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
> >>
> >> On 23/05/16 06:21, Peter Chen wrote:
> >>> On Sat, May 21, 2016 at 10:29:40AM +0800, Peter Chen wrote:
> >>>> On Thu, May 19, 2016 at 10:32:44AM +0300, Roger Quadros wrote:
> >>>>> On 18/05/16 17:46, Jun Li wrote:
> >>>>>>
> >>>>>>
> >>>>>>>>>
> >>>>>>>>> I didn't want to have complex Kconfig so decided to have otg as
> >>>>>>>>> built-in only.
> >>>>>>>>> What do you want me to change in existing code? and why?
> >>>>>>>>
> >>>>>>>> Remove those stuff which only for pass diff driver config Like
> >>>>>>>> every controller driver need a duplicated
> >>>>>>>>
> >>>>>>>> static struct otg_hcd_ops ci_hcd_ops = {
> >>>>>>>> ...
> >>>>>>>> }
> >>>>>>>
> >>>>>>> This is an exception only. Every controller driver doesn't need to
> >>>>>>> implement hcd_ops. It is implemented in the hcd core.
> >>>>>>>
> >>>>>>>>
> >>>>>>>> And here is another example, for gadget connect, otg driver can
> >>>>>>>> directly call to usb_udc_vbus_handler() in drd state machine, but
> >>>>>>>> you create another interface:
> >>>>>>>>
> >>>>>>>> .connect_control = usb_gadget_connect_control,
> >>>>>>>>
> >>>>>>>> If the symbol is defined in one driver which is 'm', another
> >>>>>>>> driver reference it should be 'm' as well, then there is no this
> >>>>>>>> kind of problem as my understanding.
> >>>>>>>
> >>>>>>> That is fine as long as all are 'm'. but how do you solve the case
> >>>>>>> when Gadget is built in and host is 'm'? OTG has to be built-in
> >>>>>>> and you will need an hcd to gadget interface.
> >>>>>>
> >>>>>> Hcd to gadget interface? Or you want to say otg to host interface?
> >>>>>
> >>>>> Sorry, I meant to say host to otg interface.
> >>>>>
> >>>>>>
> >>>>>> I think hcd and gadget are independent each other, now
> >>>>>>
> >>>>>> Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)
> >>>>>
> >>>>> It is actually a circular dependency for both.
> >>>>> hcd <--> otg and gadget <--> otg
> >>>>>
> >>>>> hcd -> otg for usb_otg_register/unregister_hcd otg -> hcd for
> >>>>> usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg,
> >>>>> usb_hub_find_child
> >>>>>
> >>>>> gadget -> otg for usb_otg_register/unregister_gadget
> >>>>> otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler
> >>>>>
> >>>>> Now consider what will happen if I get rid of the otg_hcd and
> >> otg_gadget interfaces.
> >>>>> 'y' means built-in, 'm' means module.
> >>>>>
> >>>>> 1) hcd 'y', gadget 'y'
> >>>>> otg has to be 'y' for proper build.
> >>>>>
> >>>>> 2) hcd 'm', gadget 'm'
> >>>>> otg has to be 'm' for proper build.
> >>>>>
> >>>>> 3) hcd 'y', gadget 'm'
> >>>>> Build will fail always.
> >>>>> If otg is 'y', otg build will fail due to dependency on gadget.
> >>>>> If otg is 'm', hcd build will fail due to dependency on otg.
> >>>>>
> >>>>> 4) hcd 'm', gadget 'y'
> >>>>> Build will fail always.
> >>>>> If otg is 'y', otg build will fail due to dependency on hcd.
> >>>>> If otg is 'm', gadget build will fails due to dependency on otg.
> >>>>>
> >>>>> So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops
> >>>>> to remove otg->hcd and otg->gadget dependency.
> >>>>>
> >>>>> Now we can address 3) and 4) like so
> >>>>>
> >>>>> 3) hcd 'y', gadget 'm'
> >>>>> otg has to be 'y' for proper build.
> >>>>>
> >>>>> 4) hcd 'm', gadget 'y'
> >>>>> otg has to be 'y' for proper build.
> >>>>>
> >>>>
> >>>> How about this:
> >>>> Moving usb_otg_register/unregister_hcd to host driver to remove
> >>>> dependency hcd->otg. And moving usb_otg_get_data to common.c.
> >>>>
> >>>> Delete the wait queue at usb-otg.c, and if calling usb_otg_get_data
> >>>> returns NULL, the host/device driver's probe return -EPROBE_DEFER.
> >>>> When the otg driver is probed successfully, the host/device will be
> >>>> re-probed again, and usb_otg_register_hcd will be called again.
> >>>>
> >>>> And let OTG depends on HCD && GADGET, and delete otg_hcd_ops and
> >>>> otg_gadget_ops. Below build dependency issues can be fixed.
> >>>> What do you think?
> >>>>
> >>>>> 1) hcd 'y', gadget 'y'
> >>>>> otg has to be 'y' for proper build.
> >>>>>
> >>>>> 2) hcd 'm', gadget 'm'
> >>>>> otg has to be 'm' for proper build.
> >>>>>
> >>>>> 3) hcd 'y', gadget 'm'
> >>>>> Build will fail always.
> >>>>> If otg is 'y', otg build will fail due to dependency on gadget.
> >>>>> If otg is 'm', hcd build will fail due to dependency on otg.
> >>>>>
> >>>>> 4) hcd 'm', gadget 'y'
> >>>>> Build will fail always.
> >>>>> If otg is 'y', otg build will fail due to dependency on hcd.
> >>>>> If otg is 'm', gadget build will fails due to dependency on otg.
> >>>> --
> >>>>
> >>>
> >>> After thinking more, my suggestion can't work. How about moving
> >>> CONFIG_USB_OTG out of CONFIG_USB, in that case, CONFIG_USB_OTG can
> >>> only be built in.
> >>>
> >> I tried this but it still doesn't resolve 3 and 4. I.e.
> >> it can't be automatically set to 'y' when either of hcd/gadget is 'y'
> >> and the other is 'm'.
> >
> > USB_OTG only can be selected by *user*, can't be automatically set to
> > 'y' like you said, no matter what's hcd/gadget config, USB_OTG can be
> > disabled in any cases.
>
> Yes. I am on the same page with that.
> >
> > I think Peter's intention is to make USB_OTG to be 'bool', either disabled;
> > or built-in(if enabled), something like your original idea?
>
> I'm fine with my original idea if you guys can confirm :).
>
Please do it.
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Jun Li <jun.li@nxp.com> |
|---|---|
| Date | 2016-05-18 15:50 +0200 |
| Subject | RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core |
| Message-ID | <rAa1k-2O5-35@gated-at.bofh.it> |
| In reply to | #1402953 |
Hi
> -----Original Message-----
> From: Roger Quadros [mailto:rogerq@ti.com]
> Sent: Wednesday, May 18, 2016 8:43 PM
> To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org;
> linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
>
> On 17/05/16 11:28, Jun Li wrote:
> > Hi Roger,
> >
> >> -----Original Message-----
> >> From: Roger Quadros [mailto:rogerq@ti.com]
> >> Sent: Tuesday, May 17, 2016 4:09 PM
> >> To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
> >> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
> >> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
> >> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
> >> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
> >> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
> >> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com;
> >> linux-usb@vger.kernel.org; linux-omap@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> >> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
> >>
> >> On 17/05/16 10:38, Jun Li wrote:
> >>> Hi
> >>>
> >>>> -----Original Message-----
> >>>> From: Roger Quadros [mailto:rogerq@ti.com]
> >>>> Sent: Monday, May 16, 2016 5:52 PM
> >>>> To: Peter Chen <hzpeterchen@gmail.com>
> >>>> Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com;
> >>>> gregkh@linuxfoundation.org; dan.j.williams@intel.com;
> >>>> mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com;
> >>>> sergei.shtylyov@cogentembedded.com; jun.li@freescale.com;
> >>>> grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com;
> >>>> robh@kernel.org; nsekhar@ti.com; b-liu@ti.com;
> >>>> linux-usb@vger.kernel.org; linux-omap@vger.kernel.org;
> >>>> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> >>>> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
> >>>>
> >>>> On 16/05/16 12:23, Peter Chen wrote:
> >>>>> On Mon, May 16, 2016 at 11:26:57AM +0300, Roger Quadros wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> On 16/05/16 10:02, Peter Chen wrote:
> >>>>>>> On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote:
> >>>>>>>> +
> >>>>>>>> +static int usb_gadget_connect_control(struct usb_gadget
> >>>>>>>> +*gadget, bool connect) {
> >>>>>>>> + struct usb_udc *udc;
> >>>>>>>> +
> >>>>>>>> + mutex_lock(&udc_lock);
> >>>>>>>> + udc = usb_gadget_to_udc(gadget);
> >>>>>>>> + if (!udc) {
> >>>>>>>> + dev_err(gadget->dev.parent, "%s: gadget not
> >>>> registered.\n",
> >>>>>>>> + __func__);
> >>>>>>>> + mutex_unlock(&udc_lock);
> >>>>>>>> + return -EINVAL;
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + if (connect) {
> >>>>>>>> + if (!gadget->connected)
> >>>>>>>> + usb_gadget_connect(udc->gadget);
> >>>>>>>> + } else {
> >>>>>>>> + if (gadget->connected) {
> >>>>>>>> + usb_gadget_disconnect(udc->gadget);
> >>>>>>>> + udc->driver->disconnect(udc->gadget);
> >>>>>>>> + }
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + mutex_unlock(&udc_lock);
> >>>>>>>> +
> >>>>>>>> + return 0;
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>
> >>>>>>> Since this is called for vbus interrupt, why not using
> >>>>>>> usb_udc_vbus_handler directly, and call udc->driver->disconnect
> >>>>>>> at usb_gadget_stop.
> >>>>>>
> >>>>>> We can't assume that this is always called for vbus interrupt so
> >>>>>> I decided not to call usb_udc_vbus_handler.
> >>>>>>
> >>>>>> udc->vbus is really pointless for us. We keep vbus states in our
> >>>>>> state machine and leave udc->vbus as ture always.
> >>>>>>
> >>>>>> Why do you want to move udc->driver->disconnect() to stop?
> >>>>>> If USB controller disconnected from bus then the gadget driver
> >>>>>> must be notified about the disconnect immediately. The controller
> >>>>>> may or may not be stopped by the core.
> >>>>>>
> >>>>>
> >>>>> Then, would you give some comments when this API will be used?
> >>>>> I was assumed it is only used for drd state machine.
> >>>>
> >>>> drd_state machine didn't even need this API in the first place :).
> >>>> You guys wanted me to separate out start/stop and
> >>>> connect/disconnect for full OTG case.
> >>>> Won't full OTG state machine want to use this API? If not what
> >>>> would it use?
> >>>
> >>> Instead create those new interfaces/symbol here and there just aim
> >>> to address build problems in diff configures, Could we only allow
> >>> meaningful combination of those 3 drivers configures?
> >>>
> >>> Hcd=y, gadget=y, otg=y or
> >>> Hcd=m, gadget=m, otg=m
> >>
> >> This is still a limitation.
> >>
> >> It is perfectly fine to have
> >> hcd=m, gadget=y
> >> or
> >> hcd=y, gadget=m
> >
> > I agree it makes sense to have above configs in non-otg case, that is,
> > the 'y' driver can work without 'm' driver loaded.
> >
> > But,
> > in otg enabled(y/m) case, the otherwise config of my list can't make
> > any sense from my point view. That is: some driver is built-in, but it
> > can't work at all if another 'm' driver is not loaded,
> >
> > in another words, the otg driver has to be 'm' if its dependent driver
> > is 'm', correct?
>
> If both host and gadget are 'm' then otg can be 'm', but if either host or
> gadget is built in then we have no choice but to make otg as built-in.
>
> I didn't want to have complex Kconfig so decided to have otg as built-in
> only.
> What do you want me to change in existing code? and why?
Remove those stuff which only for pass diff driver config
Like every controller driver need a duplicated
static struct otg_hcd_ops ci_hcd_ops = {
...
}
And here is another example, for gadget connect, otg driver can
directly call to usb_udc_vbus_handler() in drd state machine,
but you create another interface:
.connect_control = usb_gadget_connect_control,
If the symbol is defined in one driver which is 'm', another driver
reference it should be 'm' as well, then there is no this kind of problem
as my understanding.
Li Jun
>
> cheers,
> -roger
>
> >
> > Li Jun
> >
> >>
> >> cheers,
> >> -roger
> >>
> >>>
> >>> Li Jun
> >>>
> >>>>
> >>>> cheers,
> >>>> -roger
> >>>>
> >>>>>
> >>>>>>>
> >>>>>>>> return 0;
> >>>>>>>> @@ -660,9 +830,15 @@ static ssize_t
> >>>>>>>> usb_udc_softconn_store(struct
> >>>> device *dev,
> >>>>>>>> return -EOPNOTSUPP;
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> + /* In OTG mode we don't support softconnect, but b_bus_req */
> >>>>>>>> + if (udc->gadget->otg_dev) {
> >>>>>>>> + dev_err(dev, "soft-connect not supported in OTG mode\n");
> >>>>>>>> + return -EOPNOTSUPP;
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>
> >>>>>>> The soft-connect can be supported at dual-role mode currently,
> >>>>>>> we can use b_bus_req entry once it is implemented later.
> >>>>>>
> >>>>>> Soft-connect should be done via sysfs handling within the OTG core.
> >>>>>> This can be added later. I don't want anything outside the OTG
> >>>>>> core to handle soft-connect behaviour as it will be hard to keep
> >>>>>> things in sync.
> >>>>>>
> >>>>>> I can update the comment to something like this.
> >>>>>>
> >>>>>> /* In OTG/dual-role mode, soft-connect should be handled by OTG
> >>>>>> core */
> >>>>>
> >>>>> Ok, let's Felipe decide it.
> >>>>>
> >>>>>>
> >>>>>>>
> >>>>>>>> if (sysfs_streq(buf, "connect")) {
> >>>>>>>> usb_gadget_udc_start(udc);
> >>>>>>>> - usb_gadget_connect(udc->gadget);
> >>>>>>>> + usb_udc_connect_control(udc);
> >>>>>>>
> >>>>>>> This line seems to be not related with this patch.
> >>>>>>>
> >>>>>> Right. I'll remove it.
> >>>>>>
> >>>>>> cheers,
> >>>>>> -roger
> >>>>>
[toc] | [prev] | [next] | [standalone]
Page 1 of 3 [1] 2 3 Next page →
Back to top | Article view | linux.kernel
csiph-web