On Mon, Aug 22, 2016 at 05:32:57PM +0200, Hans de Goede wrote:
> HI,
> 
> On 22-08-16 17:24, Bin Liu wrote:
> >On Mon, Aug 22, 2016 at 05:08:56PM +0200, Hans de Goede wrote:
> >>Hi,
> >>
> >>On 22-08-16 16:11, Bin Liu wrote:
> >>>Hi,
> >>>
> >>>On Sun, Aug 21, 2016 at 12:10:26PM +0200, Hans de Goede wrote:
> >>>>Hi,
> >>>>
> >>>>On 19-08-16 23:30, Bin Liu wrote:
> >>>>>Hi,
> >>>>>
> >>>>>On Mon, Aug 15, 2016 at 09:21:32PM +0200, Hans de Goede wrote:
> >>>>>>This allows run-time dr_mode switching support via the "mode" musb
> >>>>>>sysfs attribute.
> >>>>>>
> >>>>>>Signed-off-by: Hans de Goede <hdego...@redhat.com>
> >>>>>>---
> >>>>>>drivers/usb/musb/sunxi.c | 52 
> >>>>>>++++++++++++++++++++++++++++++++++++++++++++----
> >>>>>>1 file changed, 48 insertions(+), 4 deletions(-)
> >>>>>>
> >>>>>>diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> >>>>>>index c6ee166..1fe7451 100644
> >>>>>>--- a/drivers/usb/musb/sunxi.c
> >>>>>>+++ b/drivers/usb/musb/sunxi.c
> >>>>>>@@ -74,6 +74,7 @@
> >>>>>>#define SUNXI_MUSB_FL_HAS_SRAM                  5
> >>>>>>#define SUNXI_MUSB_FL_HAS_RESET                 6
> >>>>>>#define SUNXI_MUSB_FL_NO_CONFIGDATA             7
> >>>>>>+#define SUNXI_MUSB_FL_PHY_MODE_PEND            8
> >>>>>>
> >>>>>>/* Our read/write methods need access and do not get passed in a musb 
> >>>>>>ref :| */
> >>>>>>static struct musb *sunxi_musb;
> >>>>>>@@ -87,6 +88,7 @@ struct sunxi_glue {
> >>>>>>        struct phy              *phy;
> >>>>>>        struct platform_device  *usb_phy;
> >>>>>>        struct usb_phy          *xceiv;
> >>>>>>+       enum phy_mode           phy_mode;
> >>>>>>        unsigned long           flags;
> >>>>>>        struct work_struct      work;
> >>>>>>        struct extcon_dev       *extcon;
> >>>>>>@@ -140,6 +142,9 @@ static void sunxi_musb_work(struct work_struct 
> >>>>>>*work)
> >>>>>>                        clear_bit(SUNXI_MUSB_FL_PHY_ON, &glue->flags);
> >>>>>>                }
> >>>>>>        }
> >>>>>>+
> >>>>>>+       if (test_and_clear_bit(SUNXI_MUSB_FL_PHY_MODE_PEND, 
> >>>>>>&glue->flags))
> >>>>>>+               phy_set_mode(glue->phy, glue->phy_mode);
> >>>>>>}
> >>>>>>
> >>>>>>static void sunxi_musb_set_vbus(struct musb *musb, int is_on)
> >>>>>>@@ -341,6 +346,41 @@ static void 
> >>>>>>sunxi_musb_dma_controller_destroy(struct dma_controller *c)
> >>>>>>{
> >>>>>>}
> >>>>>>
> >>>>>>+static int sunxi_musb_set_mode(struct musb *musb, u8 mode)
> >>>>>>+{
> >>>>>>+       struct sunxi_glue *glue = 
> >>>>>>dev_get_drvdata(musb->controller->parent);
> >>>>>>+       enum phy_mode new_mode;
> >>>>>>+
> >>>>>>+       switch (mode) {
> >>>>>>+       case MUSB_HOST:         new_mode = PHY_MODE_USB_HOST; break;
> >>>>>>+       case MUSB_PERIPHERAL:   new_mode = PHY_MODE_USB_DEVICE; break;
> >>>>>>+       case MUSB_OTG:          new_mode = PHY_MODE_USB_OTG; break;
> >>>>>
> >>>>>Please fix the code style as commented in patch 4/7.
> >>>>
> >>>>Ok I will send a new version with this fixed.
> >>>>
> >>>>>
> >>>>>>+       default:
> >>>>>>+               dev_err(musb->controller->parent,
> >>>>>>+                       "Error requested mode not supported by this 
> >>>>>>kernel\n");
> >>>>>>+               return -EINVAL;
> >>>>>>+       }
> >>>>>>+
> >>>>>>+       if (glue->phy_mode == new_mode)
> >>>>>>+               return 0;
> >>>>>>+
> >>>>>>+       if (musb->port_mode != MUSB_PORT_MODE_DUAL_ROLE) {
> >>>>>>+               dev_err(musb->controller->parent,
> >>>>>>+                       "Error changing modes is only supported in dual 
> >>>>>>role mode\n");
> >>>>>>+               return -EINVAL;
> >>>>>>+       }
> >>>>>>+
> >>>>>>+       /*
> >>>>>>+        * phy_set_mode may sleep, and we're called with a spinlock 
> >>>>>>held,
> >>>>>>+        * so let sunxi_musb_work deal with it.
> >>>>>>+        */
> >>>>>>+       glue->phy_mode = new_mode;
> >>>>>>+       set_bit(SUNXI_MUSB_FL_PHY_MODE_PEND, &glue->flags);
> >>>>>>+       schedule_work(&glue->work);
> >>>>>
> >>>>>When switching from host to peripheral mode, if an usb device is still
> >>>>>plugged and enumerated, how do you handle the device disconnect?
> >>>>
> >>>>The phy code will report vbus low for long enough for the musb to end
> >>>>the current session. It already does this for boards which do not
> >>>>have working vbus detection.
> >>>
> >>>But you didn't disconnect DP/DM, right? then musb detects vbus is gone
> >>>without receiving disconnect event, this is vbus error case, not a normal
> >>>teardown.
> >>
> >>Correct, there is no way to disconnect DP/DM and reporting Vbus low for
> >>a while does the trick.
> >
> >Without physically disconnecting DP/DM, we still have a way to properly
> >teardown the enumerated devices. Please check musb_softconnect_write()
> >in musb_debugfs.c.
> 
> That is manipulating the session bit in the devctl reg, that does not
> work to switch from device to host role or from host to device role,
> at least not on allwinner's musb implementation. I've already tried that
> before writing the code to report VBus low.

I would think you have to call musb_root_disconnect() first to notify
the core to teardown the enumerated devices.

> 
> Regards,
> 
> Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to