Hi, Roger
On 02/24/2015 06:13 PM, Roger Quadros wrote:
On Sat, Feb 21, 2015 at 11:03:05PM +0800, zhangfei wrote:
+static void hi6220_start_peripheral(struct hi6220_priv *priv, bool on)
+{
+ struct usb_otg *otg = priv->phy.otg;
+
+ if (!otg->gadget)
+ return;
+
+ if (on)
+ usb_gadget_connect(otg->gadget);
+ else
+ usb_gadget_disconnect(otg->gadget);
why is the PHY fiddling with pullups ?
We use this to enable/disable otg gadget mode.
I got that, but the pullups don't belong to the PHY, they belong to the
gadget.
The gpio_id & gpio_vbus are used to distinguish otg gadget mode or
host mode.
When micro usb or otg device attached to otg, gpio_vbus falling down.
And gpio_id = 1 is micro usb, gpio_id = 0 is otg device.
all of that I understood clearly :-)
So when micro usb attached, we enable gadget mode; while micro usb
detached, we disable gadget mode, and dwc2 will automatically set to
host mode.
that's all fine, I'm concerned about letting the PHY fiddle with
something it doesn't own. If I am to change pullups rules in udc-core,
this is likely to break down miserably and I don't want to have to go
through that.
Thanks for the clarifying.
no problem.
How about using usb_gadget_vbus_connect/disconnect, which are used in many
files under drivers/usb/phy.
There is no vbus_session in dwc2/gadget.c, I thought it would be same as
pullup.
However, usb_gadget_vbus_connect still need para gadget, where should we put
this file, drivers/usb/phy or drivers/phy
drivers/phy, if the framework misses anything you need, it's a great
opportunity to give back to the community by extending the framework.
Sorry, I am a little confused.
I need some concrete suggestion for the next step of this patch, which is required for the community board, hikey board.
Do you mean in the future we need use hsotg->phy instead of hsotg->uphy.
struct phy *phy;
struct usb_phy *uphy;
usb_phy has many members that struct phy does not have, including otg.
struct usb_otg *otg;
Is that mean we need port such member from usb_phy to phy.
In my opinion otg structure should belong to the USB core part that takes care
of the OTG/DRD state machine. We still don't have a clear solution here and
I'm currently investigating this.
My current work is to get Dual role functionality working with DWC3 controller and TI
platforms.
Currently phy drivers take care of OTG operation themselves but there is an opportunity
to share code and centralize USB role switching.
The USB core should be the owner of the Host controller, Gadget controller and the OTG phy
and should take care of the that.
Good idea.
If you have any patch, I will be very happy to verify.
How about adding these things in drivers/phy/phy-core.c, it is also
sharable, though not in usb core.
Just tried adding one member struct usb_otg otg to struct phy, since not
find any good member can hold usb_otg.
In drivers/phy/phy-core.c, adding extcon_register_interest,
phy_vbus_notifier, phy_set_peripheral, it works for me, dwc2 on hikey board.
Besides, are you ok with using usb_gadget_vbus_connect/disconnect.
I don't think PHY is the right place for this even though older drivers seem to be doing so.
But at the same time there is nowhere else to add this at the moment.
The right place should be the USB core that is aware of host/gadget, phy and the state of the bus.
Understand.
Scratching one's own itch kinda thing...
+static void hi6220_detect_work(struct work_struct *work)
+{
+ struct hi6220_priv *priv =
+ container_of(work, struct hi6220_priv, work.work);
+ int gpio_id, gpio_vbus;
+ enum usb_otg_state state;
+
+ if (!gpio_is_valid(priv->gpio_id) || !gpio_is_valid(priv->gpio_vbus))
+ return;
+
+ gpio_id = gpio_get_value_cansleep(priv->gpio_id);
+ gpio_vbus = gpio_get_value_cansleep(priv->gpio_vbus);
looks like this should be using extcon
Not used extcon before.
However, we need gpio_vbus interrupt.
Checked phy-tahvo.c and phy-omap-otg.c, not find extcon related with
interrupt.
Will investigate tomorrow.
drivers/extcon/extcon-gpio.c
I think there is no need to use extcon, gpio is clear enough.
extcon-gpio.c even do not support dt.
well, add DT. The whole idea of free software is that we improve on
things we already have. EXTCON is *the* API to handle such things.
I wrote the extcon-gpio-usb.c driver for exactly your use case. It is
queued for v4.1
https://lkml.org/lkml/2015/2/2/187
That's great, thanks.
It takes care of debouncing for you. Although currently it supports only ID gpio,
it should be very easy to extend to VBUS sense GPIO.
I think I am still not understanding extcon-gpio, not sure why need use this API here.
several reasons. Let me list a few.
1) Code reuse. Every PHY driver doesn't need to implement GPIO/interrupt handling and debouncing.
It just registers what cable events it wants to hear and gets a notification.
2) The events (ID/VBUS) are not only interesting for the PHY driver but also the controller
driver and the OTG state machine (whenever it exists at a common place) ;).
3) standardization because of common API.
Thanks for the explanation.
Zhangfei
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
