| Message ID | 20190703071953.38082-2-jun.li@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/5] usb: chipidea: replace ci_role with usb_role | expand |
> USB role is fully controlled by usb role switch consumer(e.g. typec), > usb port either at host mode, or at device connected mode, will not > stay at USB_ROLE_NONE mode. > Then, if the Type-C cable is disconnected from PC host, the controller driver can't be notified?If that, how controller enters low power mode at this situation? Peter > Signed-off-by: Li Jun <jun.li@nxp.com> > --- > drivers/usb/chipidea/ci.h | 2 + > drivers/usb/chipidea/core.c | 125 ++++++++++++++++++++++++++++++++++---------- > drivers/usb/chipidea/otg.c | 13 +++++ > 3 files changed, 111 insertions(+), 29 deletions(-) > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h > index 82b86cd..5e2f0bc 100644 > --- a/drivers/usb/chipidea/ci.h > +++ b/drivers/usb/chipidea/ci.h > @@ -212,6 +212,7 @@ struct ci_hdrc { > ktime_t hr_timeouts[NUM_OTG_FSM_TIMERS]; > unsigned enabled_otg_timer_bits; > enum otg_fsm_timer next_otg_timer; > + struct usb_role_switch *role_switch; > struct work_struct work; > struct workqueue_struct *wq; > > @@ -244,6 +245,7 @@ struct ci_hdrc { > struct dentry *debugfs; > bool id_event; > bool b_sess_valid_event; > + bool role_switch_event; > bool imx28_write_fix; > bool supports_runtime_pm; > bool in_lpm; > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c > index bc24c5b..1bcf6f6 100644 > --- a/drivers/usb/chipidea/core.c > +++ b/drivers/usb/chipidea/core.c > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data) > return ret; > } > > +static int ci_usb_role_switch_set(struct device *dev, enum usb_role role) > +{ > + struct ci_hdrc *ci = dev_get_drvdata(dev); > + unsigned long flags; > + > + if (ci->role == role || role == USB_ROLE_NONE) > + return 0; > + > + spin_lock_irqsave(&ci->lock, flags); > + ci->role_switch_event = true; > + if (ci->role == USB_ROLE_NONE) { > + if (role == USB_ROLE_DEVICE) > + ci->role = USB_ROLE_HOST; > + else > + ci->role = USB_ROLE_DEVICE; > + } > + spin_unlock_irqrestore(&ci->lock, flags); > + > + ci_otg_queue_work(ci); > + > + return 0; > +} > + > +static enum usb_role ci_usb_role_switch_get(struct device *dev) > +{ > + struct ci_hdrc *ci = dev_get_drvdata(dev); > + unsigned long flags; > + enum usb_role role; > + > + spin_lock_irqsave(&ci->lock, flags); > + role = ci->role; > + spin_unlock_irqrestore(&ci->lock, flags); > + > + return role; > +} > + > +static struct usb_role_switch_desc ci_role_switch = { > + .set = ci_usb_role_switch_set, > + .get = ci_usb_role_switch_get, > +}; > + > static int ci_cable_notifier(struct notifier_block *nb, unsigned long event, > void *ptr) > { > @@ -689,6 +730,9 @@ static int ci_get_platdata(struct device *dev, > if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL)) > platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA; > > + if (device_property_read_bool(dev, "usb-role-switch")) > + ci_role_switch.fwnode = dev->fwnode; > + > ext_id = ERR_PTR(-ENODEV); > ext_vbus = ERR_PTR(-ENODEV); > if (of_property_read_bool(dev->of_node, "extcon")) { > @@ -908,6 +952,43 @@ static const struct attribute_group ci_attr_group = { > .attrs = ci_attrs, > }; > > +static int ci_start_initial_role(struct ci_hdrc *ci) > +{ > + int ret = 0; > + > + if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { > + if (ci->is_otg) { > + ci->role = ci_otg_role(ci); > + /* Enable ID change irq */ > + hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > + } else { > + /* > + * If the controller is not OTG capable, but support > + * role switch, the defalt role is gadget, and the > + * user can switch it through debugfs. > + */ > + ci->role = USB_ROLE_DEVICE; > + } > + } else { > + ci->role = ci->roles[USB_ROLE_HOST] > + ? USB_ROLE_HOST > + : USB_ROLE_DEVICE; > + } > + > + if (!ci_otg_is_fsm_mode(ci)) { > + /* only update vbus status for peripheral */ > + if (ci->role == USB_ROLE_DEVICE) > + ci_handle_vbus_change(ci); > + > + ret = ci_role_start(ci, ci->role); > + if (ret) > + dev_err(ci->dev, "can't start %s role\n", > + ci_role(ci)->name); > + } > + > + return ret; > +} > + > static int ci_hdrc_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct platform_device *pdev) > } > } > > - if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { > - if (ci->is_otg) { > - ci->role = ci_otg_role(ci); > - /* Enable ID change irq */ > - hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > - } else { > - /* > - * If the controller is not OTG capable, but support > - * role switch, the defalt role is gadget, and the > - * user can switch it through debugfs. > - */ > - ci->role = USB_ROLE_DEVICE; > - } > - } else { > - ci->role = ci->roles[USB_ROLE_HOST] > - ? USB_ROLE_HOST > - : USB_ROLE_DEVICE; > - } > - > - if (!ci_otg_is_fsm_mode(ci)) { > - /* only update vbus status for peripheral */ > - if (ci->role == USB_ROLE_DEVICE) > - ci_handle_vbus_change(ci); > - > - ret = ci_role_start(ci, ci->role); > - if (ret) { > - dev_err(dev, "can't start %s role\n", > - ci_role(ci)->name); > + if (!ci_role_switch.fwnode) { > + ret = ci_start_initial_role(ci); > + if (ret) > goto stop; > - } > } > > ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED, > @@ -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct platform_device *pdev) > if (ret) > goto stop; > > + if (ci_role_switch.fwnode) { > + ci->role_switch = usb_role_switch_register(dev, > + &ci_role_switch); > + if (IS_ERR(ci->role_switch)) { > + ret = PTR_ERR(ci->role_switch); > + goto stop; > + } > + } > + > if (ci->supports_runtime_pm) { > pm_runtime_set_active(&pdev->dev); > pm_runtime_enable(&pdev->dev); > @@ -1133,6 +1197,9 @@ static int ci_hdrc_remove(struct platform_device *pdev) > { > struct ci_hdrc *ci = platform_get_drvdata(pdev); > > + if (ci->role_switch) > + usb_role_switch_unregister(ci->role_switch); > + > if (ci->supports_runtime_pm) { > pm_runtime_get_sync(&pdev->dev); > pm_runtime_disable(&pdev->dev); > diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c > index 5bde0b5..0a22855 100644 > --- a/drivers/usb/chipidea/otg.c > +++ b/drivers/usb/chipidea/otg.c > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work) > ci_handle_vbus_change(ci); > } > > + if (ci->role_switch_event) { > + ci->role_switch_event = false; > + > + if (ci->role == USB_ROLE_DEVICE) { > + usb_gadget_vbus_disconnect(&ci->gadget); > + ci_role_start(ci, USB_ROLE_HOST); > + } else if (ci->role == USB_ROLE_HOST) { > + ci_role_stop(ci); > + usb_gadget_vbus_connect(&ci->gadget); > + ci->role = USB_ROLE_DEVICE; > + } > + } > + > pm_runtime_put_sync(ci->dev); > > enable_irq(ci->irq); > -- > 2.7.4 >
> -----Original Message----- > From: Peter Chen <hzpeterchen@gmail.com> > Sent: 2019年8月2日 17:41 > To: Jun Li <jun.li@nxp.com> > Cc: Peter Chen <peter.chen@nxp.com>; Greg Kroah-Hartman > <gregkh@linuxfoundation.org>; dl-linux-imx <linux-imx@nxp.com>; USB list > <linux-usb@vger.kernel.org> > Subject: Re: [PATCH 2/5] usb: chipidea: add role switch class support > > > USB role is fully controlled by usb role switch consumer(e.g. typec), > > usb port either at host mode, or at device connected mode, will not > > stay at USB_ROLE_NONE mode. > > > > Then, if the Type-C cable is disconnected from PC host, the controller driver can't be > notified?If that, how controller enters low power mode at this situation? The controller driver can be notified, but there are only role(host or device) Information, so in you mentioned case, controller driver will get the same input before and after disconnect from host, can't know detachment and enter low power mode accordingly, this is more like something internal handling under device role. Li Jun > > Peter > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > --- > > drivers/usb/chipidea/ci.h | 2 + > > drivers/usb/chipidea/core.c | 125 > > ++++++++++++++++++++++++++++++++++---------- > > drivers/usb/chipidea/otg.c | 13 +++++ > > 3 files changed, 111 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h > > index 82b86cd..5e2f0bc 100644 > > --- a/drivers/usb/chipidea/ci.h > > +++ b/drivers/usb/chipidea/ci.h > > @@ -212,6 +212,7 @@ struct ci_hdrc { > > ktime_t hr_timeouts[NUM_OTG_FSM_TIMERS]; > > unsigned enabled_otg_timer_bits; > > enum otg_fsm_timer next_otg_timer; > > + struct usb_role_switch *role_switch; > > struct work_struct work; > > struct workqueue_struct *wq; > > > > @@ -244,6 +245,7 @@ struct ci_hdrc { > > struct dentry *debugfs; > > bool id_event; > > bool b_sess_valid_event; > > + bool role_switch_event; > > bool imx28_write_fix; > > bool supports_runtime_pm; > > bool in_lpm; > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c > > index bc24c5b..1bcf6f6 100644 > > --- a/drivers/usb/chipidea/core.c > > +++ b/drivers/usb/chipidea/core.c > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data) > > return ret; > > } > > > > +static int ci_usb_role_switch_set(struct device *dev, enum usb_role > > +role) { > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > + unsigned long flags; > > + > > + if (ci->role == role || role == USB_ROLE_NONE) > > + return 0; > > + > > + spin_lock_irqsave(&ci->lock, flags); > > + ci->role_switch_event = true; > > + if (ci->role == USB_ROLE_NONE) { > > + if (role == USB_ROLE_DEVICE) > > + ci->role = USB_ROLE_HOST; > > + else > > + ci->role = USB_ROLE_DEVICE; > > + } > > + spin_unlock_irqrestore(&ci->lock, flags); > > + > > + ci_otg_queue_work(ci); > > + > > + return 0; > > +} > > + > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) { > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > + unsigned long flags; > > + enum usb_role role; > > + > > + spin_lock_irqsave(&ci->lock, flags); > > + role = ci->role; > > + spin_unlock_irqrestore(&ci->lock, flags); > > + > > + return role; > > +} > > + > > +static struct usb_role_switch_desc ci_role_switch = { > > + .set = ci_usb_role_switch_set, > > + .get = ci_usb_role_switch_get, }; > > + > > static int ci_cable_notifier(struct notifier_block *nb, unsigned long event, > > void *ptr) { @@ -689,6 +730,9 @@ static > > int ci_get_platdata(struct device *dev, > > if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL)) > > platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA; > > > > + if (device_property_read_bool(dev, "usb-role-switch")) > > + ci_role_switch.fwnode = dev->fwnode; > > + > > ext_id = ERR_PTR(-ENODEV); > > ext_vbus = ERR_PTR(-ENODEV); > > if (of_property_read_bool(dev->of_node, "extcon")) { @@ -908,6 > > +952,43 @@ static const struct attribute_group ci_attr_group = { > > .attrs = ci_attrs, > > }; > > > > +static int ci_start_initial_role(struct ci_hdrc *ci) { > > + int ret = 0; > > + > > + if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { > > + if (ci->is_otg) { > > + ci->role = ci_otg_role(ci); > > + /* Enable ID change irq */ > > + hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > + } else { > > + /* > > + * If the controller is not OTG capable, but support > > + * role switch, the defalt role is gadget, and the > > + * user can switch it through debugfs. > > + */ > > + ci->role = USB_ROLE_DEVICE; > > + } > > + } else { > > + ci->role = ci->roles[USB_ROLE_HOST] > > + ? USB_ROLE_HOST > > + : USB_ROLE_DEVICE; > > + } > > + > > + if (!ci_otg_is_fsm_mode(ci)) { > > + /* only update vbus status for peripheral */ > > + if (ci->role == USB_ROLE_DEVICE) > > + ci_handle_vbus_change(ci); > > + > > + ret = ci_role_start(ci, ci->role); > > + if (ret) > > + dev_err(ci->dev, "can't start %s role\n", > > + ci_role(ci)->name); > > + } > > + > > + return ret; > > +} > > + > > static int ci_hdrc_probe(struct platform_device *pdev) { > > struct device *dev = &pdev->dev; > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct platform_device *pdev) > > } > > } > > > > - if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { > > - if (ci->is_otg) { > > - ci->role = ci_otg_role(ci); > > - /* Enable ID change irq */ > > - hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > - } else { > > - /* > > - * If the controller is not OTG capable, but support > > - * role switch, the defalt role is gadget, and the > > - * user can switch it through debugfs. > > - */ > > - ci->role = USB_ROLE_DEVICE; > > - } > > - } else { > > - ci->role = ci->roles[USB_ROLE_HOST] > > - ? USB_ROLE_HOST > > - : USB_ROLE_DEVICE; > > - } > > - > > - if (!ci_otg_is_fsm_mode(ci)) { > > - /* only update vbus status for peripheral */ > > - if (ci->role == USB_ROLE_DEVICE) > > - ci_handle_vbus_change(ci); > > - > > - ret = ci_role_start(ci, ci->role); > > - if (ret) { > > - dev_err(dev, "can't start %s role\n", > > - ci_role(ci)->name); > > + if (!ci_role_switch.fwnode) { > > + ret = ci_start_initial_role(ci); > > + if (ret) > > goto stop; > > - } > > } > > > > ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED, @@ > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct platform_device *pdev) > > if (ret) > > goto stop; > > > > + if (ci_role_switch.fwnode) { > > + ci->role_switch = usb_role_switch_register(dev, > > + &ci_role_switch); > > + if (IS_ERR(ci->role_switch)) { > > + ret = PTR_ERR(ci->role_switch); > > + goto stop; > > + } > > + } > > + > > if (ci->supports_runtime_pm) { > > pm_runtime_set_active(&pdev->dev); > > pm_runtime_enable(&pdev->dev); @@ -1133,6 +1197,9 @@ > > static int ci_hdrc_remove(struct platform_device *pdev) { > > struct ci_hdrc *ci = platform_get_drvdata(pdev); > > > > + if (ci->role_switch) > > + usb_role_switch_unregister(ci->role_switch); > > + > > if (ci->supports_runtime_pm) { > > pm_runtime_get_sync(&pdev->dev); > > pm_runtime_disable(&pdev->dev); diff --git > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index > > 5bde0b5..0a22855 100644 > > --- a/drivers/usb/chipidea/otg.c > > +++ b/drivers/usb/chipidea/otg.c > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work) > > ci_handle_vbus_change(ci); > > } > > > > + if (ci->role_switch_event) { > > + ci->role_switch_event = false; > > + > > + if (ci->role == USB_ROLE_DEVICE) { > > + usb_gadget_vbus_disconnect(&ci->gadget); > > + ci_role_start(ci, USB_ROLE_HOST); > > + } else if (ci->role == USB_ROLE_HOST) { > > + ci_role_stop(ci); > > + usb_gadget_vbus_connect(&ci->gadget); > > + ci->role = USB_ROLE_DEVICE; > > + } > > + } > > + > > pm_runtime_put_sync(ci->dev); > > > > enable_irq(ci->irq); > > -- > > 2.7.4 > >
> > > > > USB role is fully controlled by usb role switch consumer(e.g. typec), > > > usb port either at host mode, or at device connected mode, will not > > > stay at USB_ROLE_NONE mode. > > > > > > > Then, if the Type-C cable is disconnected from PC host, the controller driver can't be > > notified?If that, how controller enters low power mode at this situation? > > The controller driver can be notified, but there are only role(host or device) > Information, so in you mentioned case, controller driver will get the same > input before and after disconnect from host, can't know detachment and > enter low power mode accordingly, this is more like something internal > handling under device role. > If the Type-C can't pass "gadget disconnect" event, you may handle it at UDC driver itself, eg at ci_usb_role_switch_set. Otherwise, the gadget class driver can't be notified disconnection, and the controller itself can't enter LPM as well. Peter > Li Jun > > > > > Peter > > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > > --- > > > drivers/usb/chipidea/ci.h | 2 + > > > drivers/usb/chipidea/core.c | 125 > > > ++++++++++++++++++++++++++++++++++---------- > > > drivers/usb/chipidea/otg.c | 13 +++++ > > > 3 files changed, 111 insertions(+), 29 deletions(-) > > > > > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h > > > index 82b86cd..5e2f0bc 100644 > > > --- a/drivers/usb/chipidea/ci.h > > > +++ b/drivers/usb/chipidea/ci.h > > > @@ -212,6 +212,7 @@ struct ci_hdrc { > > > ktime_t hr_timeouts[NUM_OTG_FSM_TIMERS]; > > > unsigned enabled_otg_timer_bits; > > > enum otg_fsm_timer next_otg_timer; > > > + struct usb_role_switch *role_switch; > > > struct work_struct work; > > > struct workqueue_struct *wq; > > > > > > @@ -244,6 +245,7 @@ struct ci_hdrc { > > > struct dentry *debugfs; > > > bool id_event; > > > bool b_sess_valid_event; > > > + bool role_switch_event; > > > bool imx28_write_fix; > > > bool supports_runtime_pm; > > > bool in_lpm; > > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c > > > index bc24c5b..1bcf6f6 100644 > > > --- a/drivers/usb/chipidea/core.c > > > +++ b/drivers/usb/chipidea/core.c > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data) > > > return ret; > > > } > > > > > > +static int ci_usb_role_switch_set(struct device *dev, enum usb_role > > > +role) { > > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > > + unsigned long flags; > > > + > > > + if (ci->role == role || role == USB_ROLE_NONE) > > > + return 0; > > > + > > > + spin_lock_irqsave(&ci->lock, flags); > > > + ci->role_switch_event = true; > > > + if (ci->role == USB_ROLE_NONE) { > > > + if (role == USB_ROLE_DEVICE) > > > + ci->role = USB_ROLE_HOST; > > > + else > > > + ci->role = USB_ROLE_DEVICE; > > > + } > > > + spin_unlock_irqrestore(&ci->lock, flags); > > > + > > > + ci_otg_queue_work(ci); > > > + > > > + return 0; > > > +} > > > + > > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) { > > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > > + unsigned long flags; > > > + enum usb_role role; > > > + > > > + spin_lock_irqsave(&ci->lock, flags); > > > + role = ci->role; > > > + spin_unlock_irqrestore(&ci->lock, flags); > > > + > > > + return role; > > > +} > > > + > > > +static struct usb_role_switch_desc ci_role_switch = { > > > + .set = ci_usb_role_switch_set, > > > + .get = ci_usb_role_switch_get, }; > > > + > > > static int ci_cable_notifier(struct notifier_block *nb, unsigned long event, > > > void *ptr) { @@ -689,6 +730,9 @@ static > > > int ci_get_platdata(struct device *dev, > > > if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL)) > > > platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA; > > > > > > + if (device_property_read_bool(dev, "usb-role-switch")) > > > + ci_role_switch.fwnode = dev->fwnode; > > > + > > > ext_id = ERR_PTR(-ENODEV); > > > ext_vbus = ERR_PTR(-ENODEV); > > > if (of_property_read_bool(dev->of_node, "extcon")) { @@ -908,6 > > > +952,43 @@ static const struct attribute_group ci_attr_group = { > > > .attrs = ci_attrs, > > > }; > > > > > > +static int ci_start_initial_role(struct ci_hdrc *ci) { > > > + int ret = 0; > > > + > > > + if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { > > > + if (ci->is_otg) { > > > + ci->role = ci_otg_role(ci); > > > + /* Enable ID change irq */ > > > + hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > > + } else { > > > + /* > > > + * If the controller is not OTG capable, but support > > > + * role switch, the defalt role is gadget, and the > > > + * user can switch it through debugfs. > > > + */ > > > + ci->role = USB_ROLE_DEVICE; > > > + } > > > + } else { > > > + ci->role = ci->roles[USB_ROLE_HOST] > > > + ? USB_ROLE_HOST > > > + : USB_ROLE_DEVICE; > > > + } > > > + > > > + if (!ci_otg_is_fsm_mode(ci)) { > > > + /* only update vbus status for peripheral */ > > > + if (ci->role == USB_ROLE_DEVICE) > > > + ci_handle_vbus_change(ci); > > > + > > > + ret = ci_role_start(ci, ci->role); > > > + if (ret) > > > + dev_err(ci->dev, "can't start %s role\n", > > > + ci_role(ci)->name); > > > + } > > > + > > > + return ret; > > > +} > > > + > > > static int ci_hdrc_probe(struct platform_device *pdev) { > > > struct device *dev = &pdev->dev; > > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct platform_device *pdev) > > > } > > > } > > > > > > - if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { > > > - if (ci->is_otg) { > > > - ci->role = ci_otg_role(ci); > > > - /* Enable ID change irq */ > > > - hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > > - } else { > > > - /* > > > - * If the controller is not OTG capable, but support > > > - * role switch, the defalt role is gadget, and the > > > - * user can switch it through debugfs. > > > - */ > > > - ci->role = USB_ROLE_DEVICE; > > > - } > > > - } else { > > > - ci->role = ci->roles[USB_ROLE_HOST] > > > - ? USB_ROLE_HOST > > > - : USB_ROLE_DEVICE; > > > - } > > > - > > > - if (!ci_otg_is_fsm_mode(ci)) { > > > - /* only update vbus status for peripheral */ > > > - if (ci->role == USB_ROLE_DEVICE) > > > - ci_handle_vbus_change(ci); > > > - > > > - ret = ci_role_start(ci, ci->role); > > > - if (ret) { > > > - dev_err(dev, "can't start %s role\n", > > > - ci_role(ci)->name); > > > + if (!ci_role_switch.fwnode) { > > > + ret = ci_start_initial_role(ci); > > > + if (ret) > > > goto stop; > > > - } > > > } > > > > > > ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED, @@ > > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct platform_device *pdev) > > > if (ret) > > > goto stop; > > > > > > + if (ci_role_switch.fwnode) { > > > + ci->role_switch = usb_role_switch_register(dev, > > > + &ci_role_switch); > > > + if (IS_ERR(ci->role_switch)) { > > > + ret = PTR_ERR(ci->role_switch); > > > + goto stop; > > > + } > > > + } > > > + > > > if (ci->supports_runtime_pm) { > > > pm_runtime_set_active(&pdev->dev); > > > pm_runtime_enable(&pdev->dev); @@ -1133,6 +1197,9 @@ > > > static int ci_hdrc_remove(struct platform_device *pdev) { > > > struct ci_hdrc *ci = platform_get_drvdata(pdev); > > > > > > + if (ci->role_switch) > > > + usb_role_switch_unregister(ci->role_switch); > > > + > > > if (ci->supports_runtime_pm) { > > > pm_runtime_get_sync(&pdev->dev); > > > pm_runtime_disable(&pdev->dev); diff --git > > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index > > > 5bde0b5..0a22855 100644 > > > --- a/drivers/usb/chipidea/otg.c > > > +++ b/drivers/usb/chipidea/otg.c > > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work) > > > ci_handle_vbus_change(ci); > > > } > > > > > > + if (ci->role_switch_event) { > > > + ci->role_switch_event = false; > > > + > > > + if (ci->role == USB_ROLE_DEVICE) { > > > + usb_gadget_vbus_disconnect(&ci->gadget); > > > + ci_role_start(ci, USB_ROLE_HOST); > > > + } else if (ci->role == USB_ROLE_HOST) { > > > + ci_role_stop(ci); > > > + usb_gadget_vbus_connect(&ci->gadget); > > > + ci->role = USB_ROLE_DEVICE; > > > + } > > > + } > > > + > > > pm_runtime_put_sync(ci->dev); > > > > > > enable_irq(ci->irq); > > > -- > > > 2.7.4 > > >
> -----Original Message----- > From: Peter Chen <hzpeterchen@gmail.com> > Sent: 2019年8月5日 11:15 > To: Jun Li <jun.li@nxp.com> > Cc: Peter Chen <peter.chen@nxp.com>; Greg Kroah-Hartman > <gregkh@linuxfoundation.org>; dl-linux-imx <linux-imx@nxp.com>; USB list > <linux-usb@vger.kernel.org> > Subject: Re: [PATCH 2/5] usb: chipidea: add role switch class support > > > > > > > > USB role is fully controlled by usb role switch consumer(e.g. > > > > typec), usb port either at host mode, or at device connected mode, > > > > will not stay at USB_ROLE_NONE mode. > > > > > > > > > > Then, if the Type-C cable is disconnected from PC host, the > > > controller driver can't be notified?If that, how controller enters low power mode at this > situation? > > > > The controller driver can be notified, but there are only role(host or > > device) Information, so in you mentioned case, controller driver will > > get the same input before and after disconnect from host, can't know > > detachment and enter low power mode accordingly, this is more like > > something internal handling under device role. > > > > If the Type-C can't pass "gadget disconnect" event, you may handle it at UDC driver itself, > eg at ci_usb_role_switch_set. Otherwise, the gadget class driver can't be notified > disconnection, and the controller itself can't enter LPM as well. OK, I will enable VBUS irq to handle this in v2. Li Jun > > Peter > > > Li Jun > > > > > > > > Peter > > > > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > > > --- > > > > drivers/usb/chipidea/ci.h | 2 + > > > > drivers/usb/chipidea/core.c | 125 > > > > ++++++++++++++++++++++++++++++++++---------- > > > > drivers/usb/chipidea/otg.c | 13 +++++ > > > > 3 files changed, 111 insertions(+), 29 deletions(-) > > > > > > > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h > > > > index 82b86cd..5e2f0bc 100644 > > > > --- a/drivers/usb/chipidea/ci.h > > > > +++ b/drivers/usb/chipidea/ci.h > > > > @@ -212,6 +212,7 @@ struct ci_hdrc { > > > > ktime_t > hr_timeouts[NUM_OTG_FSM_TIMERS]; > > > > unsigned enabled_otg_timer_bits; > > > > enum otg_fsm_timer next_otg_timer; > > > > + struct usb_role_switch *role_switch; > > > > struct work_struct work; > > > > struct workqueue_struct *wq; > > > > > > > > @@ -244,6 +245,7 @@ struct ci_hdrc { > > > > struct dentry *debugfs; > > > > bool id_event; > > > > bool b_sess_valid_event; > > > > + bool role_switch_event; > > > > bool imx28_write_fix; > > > > bool supports_runtime_pm; > > > > bool in_lpm; > > > > diff --git a/drivers/usb/chipidea/core.c > > > > b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6 100644 > > > > --- a/drivers/usb/chipidea/core.c > > > > +++ b/drivers/usb/chipidea/core.c > > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data) > > > > return ret; > > > > } > > > > > > > > +static int ci_usb_role_switch_set(struct device *dev, enum > > > > +usb_role > > > > +role) { > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > > > + unsigned long flags; > > > > + > > > > + if (ci->role == role || role == USB_ROLE_NONE) > > > > + return 0; > > > > + > > > > + spin_lock_irqsave(&ci->lock, flags); > > > > + ci->role_switch_event = true; > > > > + if (ci->role == USB_ROLE_NONE) { > > > > + if (role == USB_ROLE_DEVICE) > > > > + ci->role = USB_ROLE_HOST; > > > > + else > > > > + ci->role = USB_ROLE_DEVICE; > > > > + } > > > > + spin_unlock_irqrestore(&ci->lock, flags); > > > > + > > > > + ci_otg_queue_work(ci); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) { > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > > > + unsigned long flags; > > > > + enum usb_role role; > > > > + > > > > + spin_lock_irqsave(&ci->lock, flags); > > > > + role = ci->role; > > > > + spin_unlock_irqrestore(&ci->lock, flags); > > > > + > > > > + return role; > > > > +} > > > > + > > > > +static struct usb_role_switch_desc ci_role_switch = { > > > > + .set = ci_usb_role_switch_set, > > > > + .get = ci_usb_role_switch_get, }; > > > > + > > > > static int ci_cable_notifier(struct notifier_block *nb, unsigned long event, > > > > void *ptr) { @@ -689,6 +730,9 @@ > > > > static int ci_get_platdata(struct device *dev, > > > > if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL)) > > > > platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA; > > > > > > > > + if (device_property_read_bool(dev, "usb-role-switch")) > > > > + ci_role_switch.fwnode = dev->fwnode; > > > > + > > > > ext_id = ERR_PTR(-ENODEV); > > > > ext_vbus = ERR_PTR(-ENODEV); > > > > if (of_property_read_bool(dev->of_node, "extcon")) { @@ > > > > -908,6 > > > > +952,43 @@ static const struct attribute_group ci_attr_group = { > > > > .attrs = ci_attrs, > > > > }; > > > > > > > > +static int ci_start_initial_role(struct ci_hdrc *ci) { > > > > + int ret = 0; > > > > + > > > > + if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { > > > > + if (ci->is_otg) { > > > > + ci->role = ci_otg_role(ci); > > > > + /* Enable ID change irq */ > > > > + hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > > > + } else { > > > > + /* > > > > + * If the controller is not OTG capable, but support > > > > + * role switch, the defalt role is gadget, and the > > > > + * user can switch it through debugfs. > > > > + */ > > > > + ci->role = USB_ROLE_DEVICE; > > > > + } > > > > + } else { > > > > + ci->role = ci->roles[USB_ROLE_HOST] > > > > + ? USB_ROLE_HOST > > > > + : USB_ROLE_DEVICE; > > > > + } > > > > + > > > > + if (!ci_otg_is_fsm_mode(ci)) { > > > > + /* only update vbus status for peripheral */ > > > > + if (ci->role == USB_ROLE_DEVICE) > > > > + ci_handle_vbus_change(ci); > > > > + > > > > + ret = ci_role_start(ci, ci->role); > > > > + if (ret) > > > > + dev_err(ci->dev, "can't start %s role\n", > > > > + ci_role(ci)->name); > > > > + } > > > > + > > > > + return ret; > > > > +} > > > > + > > > > static int ci_hdrc_probe(struct platform_device *pdev) { > > > > struct device *dev = &pdev->dev; > > > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct platform_device *pdev) > > > > } > > > > } > > > > > > > > - if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { > > > > - if (ci->is_otg) { > > > > - ci->role = ci_otg_role(ci); > > > > - /* Enable ID change irq */ > > > > - hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > > > - } else { > > > > - /* > > > > - * If the controller is not OTG capable, but support > > > > - * role switch, the defalt role is gadget, and the > > > > - * user can switch it through debugfs. > > > > - */ > > > > - ci->role = USB_ROLE_DEVICE; > > > > - } > > > > - } else { > > > > - ci->role = ci->roles[USB_ROLE_HOST] > > > > - ? USB_ROLE_HOST > > > > - : USB_ROLE_DEVICE; > > > > - } > > > > - > > > > - if (!ci_otg_is_fsm_mode(ci)) { > > > > - /* only update vbus status for peripheral */ > > > > - if (ci->role == USB_ROLE_DEVICE) > > > > - ci_handle_vbus_change(ci); > > > > - > > > > - ret = ci_role_start(ci, ci->role); > > > > - if (ret) { > > > > - dev_err(dev, "can't start %s role\n", > > > > - ci_role(ci)->name); > > > > + if (!ci_role_switch.fwnode) { > > > > + ret = ci_start_initial_role(ci); > > > > + if (ret) > > > > goto stop; > > > > - } > > > > } > > > > > > > > ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED, > > > > @@ > > > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct platform_device *pdev) > > > > if (ret) > > > > goto stop; > > > > > > > > + if (ci_role_switch.fwnode) { > > > > + ci->role_switch = usb_role_switch_register(dev, > > > > + &ci_role_switch); > > > > + if (IS_ERR(ci->role_switch)) { > > > > + ret = PTR_ERR(ci->role_switch); > > > > + goto stop; > > > > + } > > > > + } > > > > + > > > > if (ci->supports_runtime_pm) { > > > > pm_runtime_set_active(&pdev->dev); > > > > pm_runtime_enable(&pdev->dev); @@ -1133,6 +1197,9 > > > > @@ static int ci_hdrc_remove(struct platform_device *pdev) { > > > > struct ci_hdrc *ci = platform_get_drvdata(pdev); > > > > > > > > + if (ci->role_switch) > > > > + usb_role_switch_unregister(ci->role_switch); > > > > + > > > > if (ci->supports_runtime_pm) { > > > > pm_runtime_get_sync(&pdev->dev); > > > > pm_runtime_disable(&pdev->dev); diff --git > > > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index > > > > 5bde0b5..0a22855 100644 > > > > --- a/drivers/usb/chipidea/otg.c > > > > +++ b/drivers/usb/chipidea/otg.c > > > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work) > > > > ci_handle_vbus_change(ci); > > > > } > > > > > > > > + if (ci->role_switch_event) { > > > > + ci->role_switch_event = false; > > > > + > > > > + if (ci->role == USB_ROLE_DEVICE) { > > > > + usb_gadget_vbus_disconnect(&ci->gadget); > > > > + ci_role_start(ci, USB_ROLE_HOST); > > > > + } else if (ci->role == USB_ROLE_HOST) { > > > > + ci_role_stop(ci); > > > > + usb_gadget_vbus_connect(&ci->gadget); > > > > + ci->role = USB_ROLE_DEVICE; > > > > + } > > > > + } > > > > + > > > > pm_runtime_put_sync(ci->dev); > > > > > > > > enable_irq(ci->irq); > > > > -- > > > > 2.7.4 > > > >
> > > > > USB role is fully controlled by usb role switch consumer(e.g. > > > > > typec), usb port either at host mode, or at device connected > > > > > mode, will not stay at USB_ROLE_NONE mode. > > > > > > > > > > > > > Then, if the Type-C cable is disconnected from PC host, the > > > > controller driver can't be notified?If that, how controller enters > > > > low power mode at this > > situation? > > > > > > The controller driver can be notified, but there are only role(host > > > or > > > device) Information, so in you mentioned case, controller driver > > > will get the same input before and after disconnect from host, can't > > > know detachment and enter low power mode accordingly, this is more > > > like something internal handling under device role. > > > > > > > If the Type-C can't pass "gadget disconnect" event, you may handle it > > at UDC driver itself, eg at ci_usb_role_switch_set. Otherwise, the > > gadget class driver can't be notified disconnection, and the controller itself can't > enter LPM as well. > > OK, I will enable VBUS irq to handle this in v2. > You may use connect bit at portsc since VBUS may not connect to SoC. Peter > Li Jun > > > > Peter > > > > > Li Jun > > > > > > > > > > > Peter > > > > > > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > > > > --- > > > > > drivers/usb/chipidea/ci.h | 2 + > > > > > drivers/usb/chipidea/core.c | 125 > > > > > ++++++++++++++++++++++++++++++++++---------- > > > > > drivers/usb/chipidea/otg.c | 13 +++++ > > > > > 3 files changed, 111 insertions(+), 29 deletions(-) > > > > > > > > > > diff --git a/drivers/usb/chipidea/ci.h > > > > > b/drivers/usb/chipidea/ci.h index 82b86cd..5e2f0bc 100644 > > > > > --- a/drivers/usb/chipidea/ci.h > > > > > +++ b/drivers/usb/chipidea/ci.h > > > > > @@ -212,6 +212,7 @@ struct ci_hdrc { > > > > > ktime_t > > hr_timeouts[NUM_OTG_FSM_TIMERS]; > > > > > unsigned enabled_otg_timer_bits; > > > > > enum otg_fsm_timer next_otg_timer; > > > > > + struct usb_role_switch *role_switch; > > > > > struct work_struct work; > > > > > struct workqueue_struct *wq; > > > > > > > > > > @@ -244,6 +245,7 @@ struct ci_hdrc { > > > > > struct dentry *debugfs; > > > > > bool id_event; > > > > > bool b_sess_valid_event; > > > > > + bool role_switch_event; > > > > > bool imx28_write_fix; > > > > > bool supports_runtime_pm; > > > > > bool in_lpm; > > > > > diff --git a/drivers/usb/chipidea/core.c > > > > > b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6 100644 > > > > > --- a/drivers/usb/chipidea/core.c > > > > > +++ b/drivers/usb/chipidea/core.c > > > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data) > > > > > return ret; > > > > > } > > > > > > > > > > +static int ci_usb_role_switch_set(struct device *dev, enum > > > > > +usb_role > > > > > +role) { > > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > > > > + unsigned long flags; > > > > > + > > > > > + if (ci->role == role || role == USB_ROLE_NONE) > > > > > + return 0; > > > > > + > > > > > + spin_lock_irqsave(&ci->lock, flags); > > > > > + ci->role_switch_event = true; > > > > > + if (ci->role == USB_ROLE_NONE) { > > > > > + if (role == USB_ROLE_DEVICE) > > > > > + ci->role = USB_ROLE_HOST; > > > > > + else > > > > > + ci->role = USB_ROLE_DEVICE; > > > > > + } > > > > > + spin_unlock_irqrestore(&ci->lock, flags); > > > > > + > > > > > + ci_otg_queue_work(ci); > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) { > > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > > > > + unsigned long flags; > > > > > + enum usb_role role; > > > > > + > > > > > + spin_lock_irqsave(&ci->lock, flags); > > > > > + role = ci->role; > > > > > + spin_unlock_irqrestore(&ci->lock, flags); > > > > > + > > > > > + return role; > > > > > +} > > > > > + > > > > > +static struct usb_role_switch_desc ci_role_switch = { > > > > > + .set = ci_usb_role_switch_set, > > > > > + .get = ci_usb_role_switch_get, }; > > > > > + > > > > > static int ci_cable_notifier(struct notifier_block *nb, unsigned long event, > > > > > void *ptr) { @@ -689,6 +730,9 @@ > > > > > static int ci_get_platdata(struct device *dev, > > > > > if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL)) > > > > > platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA; > > > > > > > > > > + if (device_property_read_bool(dev, "usb-role-switch")) > > > > > + ci_role_switch.fwnode = dev->fwnode; > > > > > + > > > > > ext_id = ERR_PTR(-ENODEV); > > > > > ext_vbus = ERR_PTR(-ENODEV); > > > > > if (of_property_read_bool(dev->of_node, "extcon")) { @@ > > > > > -908,6 > > > > > +952,43 @@ static const struct attribute_group ci_attr_group = { > > > > > .attrs = ci_attrs, > > > > > }; > > > > > > > > > > +static int ci_start_initial_role(struct ci_hdrc *ci) { > > > > > + int ret = 0; > > > > > + > > > > > + if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { > > > > > + if (ci->is_otg) { > > > > > + ci->role = ci_otg_role(ci); > > > > > + /* Enable ID change irq */ > > > > > + hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > > > > + } else { > > > > > + /* > > > > > + * If the controller is not OTG capable, but support > > > > > + * role switch, the defalt role is gadget, and the > > > > > + * user can switch it through debugfs. > > > > > + */ > > > > > + ci->role = USB_ROLE_DEVICE; > > > > > + } > > > > > + } else { > > > > > + ci->role = ci->roles[USB_ROLE_HOST] > > > > > + ? USB_ROLE_HOST > > > > > + : USB_ROLE_DEVICE; > > > > > + } > > > > > + > > > > > + if (!ci_otg_is_fsm_mode(ci)) { > > > > > + /* only update vbus status for peripheral */ > > > > > + if (ci->role == USB_ROLE_DEVICE) > > > > > + ci_handle_vbus_change(ci); > > > > > + > > > > > + ret = ci_role_start(ci, ci->role); > > > > > + if (ret) > > > > > + dev_err(ci->dev, "can't start %s role\n", > > > > > + ci_role(ci)->name); > > > > > + } > > > > > + > > > > > + return ret; > > > > > +} > > > > > + > > > > > static int ci_hdrc_probe(struct platform_device *pdev) { > > > > > struct device *dev = &pdev->dev; > > > > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct > platform_device *pdev) > > > > > } > > > > > } > > > > > > > > > > - if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { > > > > > - if (ci->is_otg) { > > > > > - ci->role = ci_otg_role(ci); > > > > > - /* Enable ID change irq */ > > > > > - hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > > > > - } else { > > > > > - /* > > > > > - * If the controller is not OTG capable, but support > > > > > - * role switch, the defalt role is gadget, and the > > > > > - * user can switch it through debugfs. > > > > > - */ > > > > > - ci->role = USB_ROLE_DEVICE; > > > > > - } > > > > > - } else { > > > > > - ci->role = ci->roles[USB_ROLE_HOST] > > > > > - ? USB_ROLE_HOST > > > > > - : USB_ROLE_DEVICE; > > > > > - } > > > > > - > > > > > - if (!ci_otg_is_fsm_mode(ci)) { > > > > > - /* only update vbus status for peripheral */ > > > > > - if (ci->role == USB_ROLE_DEVICE) > > > > > - ci_handle_vbus_change(ci); > > > > > - > > > > > - ret = ci_role_start(ci, ci->role); > > > > > - if (ret) { > > > > > - dev_err(dev, "can't start %s role\n", > > > > > - ci_role(ci)->name); > > > > > + if (!ci_role_switch.fwnode) { > > > > > + ret = ci_start_initial_role(ci); > > > > > + if (ret) > > > > > goto stop; > > > > > - } > > > > > } > > > > > > > > > > ret = devm_request_irq(dev, ci->irq, ci_irq, > > > > > IRQF_SHARED, @@ > > > > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct platform_device > *pdev) > > > > > if (ret) > > > > > goto stop; > > > > > > > > > > + if (ci_role_switch.fwnode) { > > > > > + ci->role_switch = usb_role_switch_register(dev, > > > > > + &ci_role_switch); > > > > > + if (IS_ERR(ci->role_switch)) { > > > > > + ret = PTR_ERR(ci->role_switch); > > > > > + goto stop; > > > > > + } > > > > > + } > > > > > + > > > > > if (ci->supports_runtime_pm) { > > > > > pm_runtime_set_active(&pdev->dev); > > > > > pm_runtime_enable(&pdev->dev); @@ -1133,6 > > > > > +1197,9 @@ static int ci_hdrc_remove(struct platform_device *pdev) { > > > > > struct ci_hdrc *ci = platform_get_drvdata(pdev); > > > > > > > > > > + if (ci->role_switch) > > > > > + usb_role_switch_unregister(ci->role_switch); > > > > > + > > > > > if (ci->supports_runtime_pm) { > > > > > pm_runtime_get_sync(&pdev->dev); > > > > > pm_runtime_disable(&pdev->dev); diff --git > > > > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index > > > > > 5bde0b5..0a22855 100644 > > > > > --- a/drivers/usb/chipidea/otg.c > > > > > +++ b/drivers/usb/chipidea/otg.c > > > > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work) > > > > > ci_handle_vbus_change(ci); > > > > > } > > > > > > > > > > + if (ci->role_switch_event) { > > > > > + ci->role_switch_event = false; > > > > > + > > > > > + if (ci->role == USB_ROLE_DEVICE) { > > > > > + usb_gadget_vbus_disconnect(&ci->gadget); > > > > > + ci_role_start(ci, USB_ROLE_HOST); > > > > > + } else if (ci->role == USB_ROLE_HOST) { > > > > > + ci_role_stop(ci); > > > > > + usb_gadget_vbus_connect(&ci->gadget); > > > > > + ci->role = USB_ROLE_DEVICE; > > > > > + } > > > > > + } > > > > > + > > > > > pm_runtime_put_sync(ci->dev); > > > > > > > > > > enable_irq(ci->irq); > > > > > -- > > > > > 2.7.4 > > > > >
Hi > -----Original Message----- > From: Peter Chen > Sent: 2019年8月5日 12:57 > To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; dl-linux-imx > <linux-imx@nxp.com>; USB list <linux-usb@vger.kernel.org> > Subject: RE: [PATCH 2/5] usb: chipidea: add role switch class support > > > > > > > > USB role is fully controlled by usb role switch consumer(e.g. > > > > > > typec), usb port either at host mode, or at device connected > > > > > > mode, will not stay at USB_ROLE_NONE mode. > > > > > > > > > > > > > > > > Then, if the Type-C cable is disconnected from PC host, the > > > > > controller driver can't be notified?If that, how controller > > > > > enters low power mode at this > > > situation? > > > > > > > > The controller driver can be notified, but there are only > > > > role(host or > > > > device) Information, so in you mentioned case, controller driver > > > > will get the same input before and after disconnect from host, > > > > can't know detachment and enter low power mode accordingly, this > > > > is more like something internal handling under device role. > > > > > > > > > > If the Type-C can't pass "gadget disconnect" event, you may handle > > > it at UDC driver itself, eg at ci_usb_role_switch_set. Otherwise, > > > the gadget class driver can't be notified disconnection, and the > > > controller itself can't > > enter LPM as well. > > > > OK, I will enable VBUS irq to handle this in v2. > > > > You may use connect bit at portsc since VBUS may not connect to SoC. By this way, disconnect can be decided but connect is a problem in current driver, as controller was stopped in low power mode so can't detect connection from host, unless we also update this behavior too. Li Jun > > Peter > > > Li Jun > > > > > > Peter > > > > > > > Li Jun > > > > > > > > > > > > > > Peter > > > > > > > > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > > > > > --- > > > > > > drivers/usb/chipidea/ci.h | 2 + > > > > > > drivers/usb/chipidea/core.c | 125 > > > > > > ++++++++++++++++++++++++++++++++++---------- > > > > > > drivers/usb/chipidea/otg.c | 13 +++++ > > > > > > 3 files changed, 111 insertions(+), 29 deletions(-) > > > > > > > > > > > > diff --git a/drivers/usb/chipidea/ci.h > > > > > > b/drivers/usb/chipidea/ci.h index 82b86cd..5e2f0bc 100644 > > > > > > --- a/drivers/usb/chipidea/ci.h > > > > > > +++ b/drivers/usb/chipidea/ci.h > > > > > > @@ -212,6 +212,7 @@ struct ci_hdrc { > > > > > > ktime_t > > > hr_timeouts[NUM_OTG_FSM_TIMERS]; > > > > > > unsigned enabled_otg_timer_bits; > > > > > > enum otg_fsm_timer next_otg_timer; > > > > > > + struct usb_role_switch *role_switch; > > > > > > struct work_struct work; > > > > > > struct workqueue_struct *wq; > > > > > > > > > > > > @@ -244,6 +245,7 @@ struct ci_hdrc { > > > > > > struct dentry *debugfs; > > > > > > bool id_event; > > > > > > bool b_sess_valid_event; > > > > > > + bool role_switch_event; > > > > > > bool imx28_write_fix; > > > > > > bool supports_runtime_pm; > > > > > > bool in_lpm; > > > > > > diff --git a/drivers/usb/chipidea/core.c > > > > > > b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6 100644 > > > > > > --- a/drivers/usb/chipidea/core.c > > > > > > +++ b/drivers/usb/chipidea/core.c > > > > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data) > > > > > > return ret; > > > > > > } > > > > > > > > > > > > +static int ci_usb_role_switch_set(struct device *dev, enum > > > > > > +usb_role > > > > > > +role) { > > > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > > > > > + unsigned long flags; > > > > > > + > > > > > > + if (ci->role == role || role == USB_ROLE_NONE) > > > > > > + return 0; > > > > > > + > > > > > > + spin_lock_irqsave(&ci->lock, flags); > > > > > > + ci->role_switch_event = true; > > > > > > + if (ci->role == USB_ROLE_NONE) { > > > > > > + if (role == USB_ROLE_DEVICE) > > > > > > + ci->role = USB_ROLE_HOST; > > > > > > + else > > > > > > + ci->role = USB_ROLE_DEVICE; > > > > > > + } > > > > > > + spin_unlock_irqrestore(&ci->lock, flags); > > > > > > + > > > > > > + ci_otg_queue_work(ci); > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) { > > > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > > > > > + unsigned long flags; > > > > > > + enum usb_role role; > > > > > > + > > > > > > + spin_lock_irqsave(&ci->lock, flags); > > > > > > + role = ci->role; > > > > > > + spin_unlock_irqrestore(&ci->lock, flags); > > > > > > + > > > > > > + return role; > > > > > > +} > > > > > > + > > > > > > +static struct usb_role_switch_desc ci_role_switch = { > > > > > > + .set = ci_usb_role_switch_set, > > > > > > + .get = ci_usb_role_switch_get, }; > > > > > > + > > > > > > static int ci_cable_notifier(struct notifier_block *nb, unsigned long event, > > > > > > void *ptr) { @@ -689,6 +730,9 @@ > > > > > > static int ci_get_platdata(struct device *dev, > > > > > > if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL)) > > > > > > platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA; > > > > > > > > > > > > + if (device_property_read_bool(dev, "usb-role-switch")) > > > > > > + ci_role_switch.fwnode = dev->fwnode; > > > > > > + > > > > > > ext_id = ERR_PTR(-ENODEV); > > > > > > ext_vbus = ERR_PTR(-ENODEV); > > > > > > if (of_property_read_bool(dev->of_node, "extcon")) { > > > > > > @@ > > > > > > -908,6 > > > > > > +952,43 @@ static const struct attribute_group ci_attr_group = > > > > > > +{ > > > > > > .attrs = ci_attrs, > > > > > > }; > > > > > > > > > > > > +static int ci_start_initial_role(struct ci_hdrc *ci) { > > > > > > + int ret = 0; > > > > > > + > > > > > > + if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) > { > > > > > > + if (ci->is_otg) { > > > > > > + ci->role = ci_otg_role(ci); > > > > > > + /* Enable ID change irq */ > > > > > > + hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > > > > > + } else { > > > > > > + /* > > > > > > + * If the controller is not OTG capable, but support > > > > > > + * role switch, the defalt role is gadget, and the > > > > > > + * user can switch it through debugfs. > > > > > > + */ > > > > > > + ci->role = USB_ROLE_DEVICE; > > > > > > + } > > > > > > + } else { > > > > > > + ci->role = ci->roles[USB_ROLE_HOST] > > > > > > + ? USB_ROLE_HOST > > > > > > + : USB_ROLE_DEVICE; > > > > > > + } > > > > > > + > > > > > > + if (!ci_otg_is_fsm_mode(ci)) { > > > > > > + /* only update vbus status for peripheral */ > > > > > > + if (ci->role == USB_ROLE_DEVICE) > > > > > > + ci_handle_vbus_change(ci); > > > > > > + > > > > > > + ret = ci_role_start(ci, ci->role); > > > > > > + if (ret) > > > > > > + dev_err(ci->dev, "can't start %s role\n", > > > > > > + ci_role(ci)->name); > > > > > > + } > > > > > > + > > > > > > + return ret; > > > > > > +} > > > > > > + > > > > > > static int ci_hdrc_probe(struct platform_device *pdev) { > > > > > > struct device *dev = &pdev->dev; > > > > > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct > > platform_device *pdev) > > > > > > } > > > > > > } > > > > > > > > > > > > - if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { > > > > > > - if (ci->is_otg) { > > > > > > - ci->role = ci_otg_role(ci); > > > > > > - /* Enable ID change irq */ > > > > > > - hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > > > > > - } else { > > > > > > - /* > > > > > > - * If the controller is not OTG capable, but support > > > > > > - * role switch, the defalt role is gadget, and the > > > > > > - * user can switch it through debugfs. > > > > > > - */ > > > > > > - ci->role = USB_ROLE_DEVICE; > > > > > > - } > > > > > > - } else { > > > > > > - ci->role = ci->roles[USB_ROLE_HOST] > > > > > > - ? USB_ROLE_HOST > > > > > > - : USB_ROLE_DEVICE; > > > > > > - } > > > > > > - > > > > > > - if (!ci_otg_is_fsm_mode(ci)) { > > > > > > - /* only update vbus status for peripheral */ > > > > > > - if (ci->role == USB_ROLE_DEVICE) > > > > > > - ci_handle_vbus_change(ci); > > > > > > - > > > > > > - ret = ci_role_start(ci, ci->role); > > > > > > - if (ret) { > > > > > > - dev_err(dev, "can't start %s role\n", > > > > > > - ci_role(ci)->name); > > > > > > + if (!ci_role_switch.fwnode) { > > > > > > + ret = ci_start_initial_role(ci); > > > > > > + if (ret) > > > > > > goto stop; > > > > > > - } > > > > > > } > > > > > > > > > > > > ret = devm_request_irq(dev, ci->irq, ci_irq, > > > > > > IRQF_SHARED, @@ > > > > > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct > > > > > > platform_device > > *pdev) > > > > > > if (ret) > > > > > > goto stop; > > > > > > > > > > > > + if (ci_role_switch.fwnode) { > > > > > > + ci->role_switch = usb_role_switch_register(dev, > > > > > > + &ci_role_switch); > > > > > > + if (IS_ERR(ci->role_switch)) { > > > > > > + ret = PTR_ERR(ci->role_switch); > > > > > > + goto stop; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > if (ci->supports_runtime_pm) { > > > > > > pm_runtime_set_active(&pdev->dev); > > > > > > pm_runtime_enable(&pdev->dev); @@ -1133,6 > > > > > > +1197,9 @@ static int ci_hdrc_remove(struct platform_device > > > > > > +*pdev) { > > > > > > struct ci_hdrc *ci = platform_get_drvdata(pdev); > > > > > > > > > > > > + if (ci->role_switch) > > > > > > + usb_role_switch_unregister(ci->role_switch); > > > > > > + > > > > > > if (ci->supports_runtime_pm) { > > > > > > pm_runtime_get_sync(&pdev->dev); > > > > > > pm_runtime_disable(&pdev->dev); diff --git > > > > > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c > > > > > > index > > > > > > 5bde0b5..0a22855 100644 > > > > > > --- a/drivers/usb/chipidea/otg.c > > > > > > +++ b/drivers/usb/chipidea/otg.c > > > > > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work) > > > > > > ci_handle_vbus_change(ci); > > > > > > } > > > > > > > > > > > > + if (ci->role_switch_event) { > > > > > > + ci->role_switch_event = false; > > > > > > + > > > > > > + if (ci->role == USB_ROLE_DEVICE) { > > > > > > + usb_gadget_vbus_disconnect(&ci->gadget); > > > > > > + ci_role_start(ci, USB_ROLE_HOST); > > > > > > + } else if (ci->role == USB_ROLE_HOST) { > > > > > > + ci_role_stop(ci); > > > > > > + usb_gadget_vbus_connect(&ci->gadget); > > > > > > + ci->role = USB_ROLE_DEVICE; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > pm_runtime_put_sync(ci->dev); > > > > > > > > > > > > enable_irq(ci->irq); > > > > > > -- > > > > > > 2.7.4 > > > > > >
> > > > You may use connect bit at portsc since VBUS may not connect to SoC. > > By this way, disconnect can be decided but connect is a problem in current driver, > as controller was stopped in low power mode so can't detect connection from host, > unless we also update this behavior too. > For connection, if current role is USB_ROLE_NONE, you may start device mode. Peter > Li Jun > > > > Peter > > > > > Li Jun > > > > > > > > Peter > > > > > > > > > Li Jun > > > > > > > > > > > > > > > > > Peter > > > > > > > > > > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > > > > > > --- > > > > > > > drivers/usb/chipidea/ci.h | 2 + > > > > > > > drivers/usb/chipidea/core.c | 125 > > > > > > > ++++++++++++++++++++++++++++++++++---------- > > > > > > > drivers/usb/chipidea/otg.c | 13 +++++ > > > > > > > 3 files changed, 111 insertions(+), 29 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/usb/chipidea/ci.h > > > > > > > b/drivers/usb/chipidea/ci.h index 82b86cd..5e2f0bc 100644 > > > > > > > --- a/drivers/usb/chipidea/ci.h > > > > > > > +++ b/drivers/usb/chipidea/ci.h > > > > > > > @@ -212,6 +212,7 @@ struct ci_hdrc { > > > > > > > ktime_t > > > > hr_timeouts[NUM_OTG_FSM_TIMERS]; > > > > > > > unsigned enabled_otg_timer_bits; > > > > > > > enum otg_fsm_timer next_otg_timer; > > > > > > > + struct usb_role_switch *role_switch; > > > > > > > struct work_struct work; > > > > > > > struct workqueue_struct *wq; > > > > > > > > > > > > > > @@ -244,6 +245,7 @@ struct ci_hdrc { > > > > > > > struct dentry *debugfs; > > > > > > > bool id_event; > > > > > > > bool b_sess_valid_event; > > > > > > > + bool role_switch_event; > > > > > > > bool imx28_write_fix; > > > > > > > bool supports_runtime_pm; > > > > > > > bool in_lpm; > > > > > > > diff --git a/drivers/usb/chipidea/core.c > > > > > > > b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6 100644 > > > > > > > --- a/drivers/usb/chipidea/core.c > > > > > > > +++ b/drivers/usb/chipidea/core.c > > > > > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data) > > > > > > > return ret; > > > > > > > } > > > > > > > > > > > > > > +static int ci_usb_role_switch_set(struct device *dev, enum > > > > > > > +usb_role > > > > > > > +role) { > > > > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > > > > > > + unsigned long flags; > > > > > > > + > > > > > > > + if (ci->role == role || role == USB_ROLE_NONE) > > > > > > > + return 0; > > > > > > > + > > > > > > > + spin_lock_irqsave(&ci->lock, flags); > > > > > > > + ci->role_switch_event = true; > > > > > > > + if (ci->role == USB_ROLE_NONE) { > > > > > > > + if (role == USB_ROLE_DEVICE) > > > > > > > + ci->role = USB_ROLE_HOST; > > > > > > > + else > > > > > > > + ci->role = USB_ROLE_DEVICE; > > > > > > > + } > > > > > > > + spin_unlock_irqrestore(&ci->lock, flags); > > > > > > > + > > > > > > > + ci_otg_queue_work(ci); > > > > > > > + > > > > > > > + return 0; > > > > > > > +} > > > > > > > + > > > > > > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) { > > > > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > > > > > > + unsigned long flags; > > > > > > > + enum usb_role role; > > > > > > > + > > > > > > > + spin_lock_irqsave(&ci->lock, flags); > > > > > > > + role = ci->role; > > > > > > > + spin_unlock_irqrestore(&ci->lock, flags); > > > > > > > + > > > > > > > + return role; > > > > > > > +} > > > > > > > + > > > > > > > +static struct usb_role_switch_desc ci_role_switch = { > > > > > > > + .set = ci_usb_role_switch_set, > > > > > > > + .get = ci_usb_role_switch_get, }; > > > > > > > + > > > > > > > static int ci_cable_notifier(struct notifier_block *nb, unsigned long event, > > > > > > > void *ptr) { @@ -689,6 +730,9 > > > > > > > @@ static int ci_get_platdata(struct device *dev, > > > > > > > if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL)) > > > > > > > platdata->flags |= > > > > > > > CI_HDRC_SET_NON_ZERO_TTHA; > > > > > > > > > > > > > > + if (device_property_read_bool(dev, "usb-role-switch")) > > > > > > > + ci_role_switch.fwnode = dev->fwnode; > > > > > > > + > > > > > > > ext_id = ERR_PTR(-ENODEV); > > > > > > > ext_vbus = ERR_PTR(-ENODEV); > > > > > > > if (of_property_read_bool(dev->of_node, "extcon")) { > > > > > > > @@ > > > > > > > -908,6 > > > > > > > +952,43 @@ static const struct attribute_group ci_attr_group > > > > > > > += { > > > > > > > .attrs = ci_attrs, > > > > > > > }; > > > > > > > > > > > > > > +static int ci_start_initial_role(struct ci_hdrc *ci) { > > > > > > > + int ret = 0; > > > > > > > + > > > > > > > + if (ci->roles[USB_ROLE_HOST] && > > > > > > > + ci->roles[USB_ROLE_DEVICE]) > > { > > > > > > > + if (ci->is_otg) { > > > > > > > + ci->role = ci_otg_role(ci); > > > > > > > + /* Enable ID change irq */ > > > > > > > + hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > > > > > > + } else { > > > > > > > + /* > > > > > > > + * If the controller is not OTG capable, but support > > > > > > > + * role switch, the defalt role is gadget, and the > > > > > > > + * user can switch it through debugfs. > > > > > > > + */ > > > > > > > + ci->role = USB_ROLE_DEVICE; > > > > > > > + } > > > > > > > + } else { > > > > > > > + ci->role = ci->roles[USB_ROLE_HOST] > > > > > > > + ? USB_ROLE_HOST > > > > > > > + : USB_ROLE_DEVICE; > > > > > > > + } > > > > > > > + > > > > > > > + if (!ci_otg_is_fsm_mode(ci)) { > > > > > > > + /* only update vbus status for peripheral */ > > > > > > > + if (ci->role == USB_ROLE_DEVICE) > > > > > > > + ci_handle_vbus_change(ci); > > > > > > > + > > > > > > > + ret = ci_role_start(ci, ci->role); > > > > > > > + if (ret) > > > > > > > + dev_err(ci->dev, "can't start %s role\n", > > > > > > > + ci_role(ci)->name); > > > > > > > + } > > > > > > > + > > > > > > > + return ret; > > > > > > > +} > > > > > > > + > > > > > > > static int ci_hdrc_probe(struct platform_device *pdev) { > > > > > > > struct device *dev = &pdev->dev; > > > > > > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct > > > platform_device *pdev) > > > > > > > } > > > > > > > } > > > > > > > > > > > > > > - if (ci->roles[USB_ROLE_HOST] && ci- > >roles[USB_ROLE_DEVICE]) { > > > > > > > - if (ci->is_otg) { > > > > > > > - ci->role = ci_otg_role(ci); > > > > > > > - /* Enable ID change irq */ > > > > > > > - hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > > > > > > - } else { > > > > > > > - /* > > > > > > > - * If the controller is not OTG capable, but support > > > > > > > - * role switch, the defalt role is gadget, and the > > > > > > > - * user can switch it through debugfs. > > > > > > > - */ > > > > > > > - ci->role = USB_ROLE_DEVICE; > > > > > > > - } > > > > > > > - } else { > > > > > > > - ci->role = ci->roles[USB_ROLE_HOST] > > > > > > > - ? USB_ROLE_HOST > > > > > > > - : USB_ROLE_DEVICE; > > > > > > > - } > > > > > > > - > > > > > > > - if (!ci_otg_is_fsm_mode(ci)) { > > > > > > > - /* only update vbus status for peripheral */ > > > > > > > - if (ci->role == USB_ROLE_DEVICE) > > > > > > > - ci_handle_vbus_change(ci); > > > > > > > - > > > > > > > - ret = ci_role_start(ci, ci->role); > > > > > > > - if (ret) { > > > > > > > - dev_err(dev, "can't start %s role\n", > > > > > > > - ci_role(ci)->name); > > > > > > > + if (!ci_role_switch.fwnode) { > > > > > > > + ret = ci_start_initial_role(ci); > > > > > > > + if (ret) > > > > > > > goto stop; > > > > > > > - } > > > > > > > } > > > > > > > > > > > > > > ret = devm_request_irq(dev, ci->irq, ci_irq, > > > > > > > IRQF_SHARED, @@ > > > > > > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct > > > > > > > platform_device > > > *pdev) > > > > > > > if (ret) > > > > > > > goto stop; > > > > > > > > > > > > > > + if (ci_role_switch.fwnode) { > > > > > > > + ci->role_switch = usb_role_switch_register(dev, > > > > > > > + &ci_role_switch); > > > > > > > + if (IS_ERR(ci->role_switch)) { > > > > > > > + ret = PTR_ERR(ci->role_switch); > > > > > > > + goto stop; > > > > > > > + } > > > > > > > + } > > > > > > > + > > > > > > > if (ci->supports_runtime_pm) { > > > > > > > pm_runtime_set_active(&pdev->dev); > > > > > > > pm_runtime_enable(&pdev->dev); @@ -1133,6 > > > > > > > +1197,9 @@ static int ci_hdrc_remove(struct platform_device > > > > > > > +*pdev) { > > > > > > > struct ci_hdrc *ci = platform_get_drvdata(pdev); > > > > > > > > > > > > > > + if (ci->role_switch) > > > > > > > + usb_role_switch_unregister(ci->role_switch); > > > > > > > + > > > > > > > if (ci->supports_runtime_pm) { > > > > > > > pm_runtime_get_sync(&pdev->dev); > > > > > > > pm_runtime_disable(&pdev->dev); diff --git > > > > > > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c > > > > > > > index > > > > > > > 5bde0b5..0a22855 100644 > > > > > > > --- a/drivers/usb/chipidea/otg.c > > > > > > > +++ b/drivers/usb/chipidea/otg.c > > > > > > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct > *work) > > > > > > > ci_handle_vbus_change(ci); > > > > > > > } > > > > > > > > > > > > > > + if (ci->role_switch_event) { > > > > > > > + ci->role_switch_event = false; > > > > > > > + > > > > > > > + if (ci->role == USB_ROLE_DEVICE) { > > > > > > > + usb_gadget_vbus_disconnect(&ci->gadget); > > > > > > > + ci_role_start(ci, USB_ROLE_HOST); > > > > > > > + } else if (ci->role == USB_ROLE_HOST) { > > > > > > > + ci_role_stop(ci); > > > > > > > + usb_gadget_vbus_connect(&ci->gadget); > > > > > > > + ci->role = USB_ROLE_DEVICE; > > > > > > > + } > > > > > > > + } > > > > > > > + > > > > > > > pm_runtime_put_sync(ci->dev); > > > > > > > > > > > > > > enable_irq(ci->irq); > > > > > > > -- > > > > > > > 2.7.4 > > > > > > >
> -----Original Message----- > From: Peter Chen > Sent: 2019年8月6日 15:52 > To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; dl-linux-imx > <linux-imx@nxp.com>; USB list <linux-usb@vger.kernel.org> > Subject: RE: [PATCH 2/5] usb: chipidea: add role switch class support > > > > > > > > You may use connect bit at portsc since VBUS may not connect to SoC. > > > > By this way, disconnect can be decided but connect is a problem in > > current driver, as controller was stopped in low power mode so can't > > detect connection from host, unless we also update this behavior too. > > > > For connection, if current role is USB_ROLE_NONE, you may start device mode. This is assuming set role only can be called one time between disconnect and connect to host, this may not always true, but this can be resolved, I think we need handle the case device mode is started but connection does not happen, so the gadget need be stopped and enter low power mode again, if this approach is OK to you, I will go in this direction for my v2. Li Jun > > Peter > > > Li Jun > > > > > > Peter > > > > > > > Li Jun > > > > > > > > > > Peter > > > > > > > > > > > Li Jun > > > > > > > > > > > > > > > > > > > > Peter > > > > > > > > > > > > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > > > > > > > --- > > > > > > > > drivers/usb/chipidea/ci.h | 2 + > > > > > > > > drivers/usb/chipidea/core.c | 125 > > > > > > > > ++++++++++++++++++++++++++++++++++---------- > > > > > > > > drivers/usb/chipidea/otg.c | 13 +++++ > > > > > > > > 3 files changed, 111 insertions(+), 29 deletions(-) > > > > > > > > > > > > > > > > diff --git a/drivers/usb/chipidea/ci.h > > > > > > > > b/drivers/usb/chipidea/ci.h index 82b86cd..5e2f0bc 100644 > > > > > > > > --- a/drivers/usb/chipidea/ci.h > > > > > > > > +++ b/drivers/usb/chipidea/ci.h > > > > > > > > @@ -212,6 +212,7 @@ struct ci_hdrc { > > > > > > > > ktime_t > > > > > hr_timeouts[NUM_OTG_FSM_TIMERS]; > > > > > > > > unsigned enabled_otg_timer_bits; > > > > > > > > enum otg_fsm_timer next_otg_timer; > > > > > > > > + struct usb_role_switch *role_switch; > > > > > > > > struct work_struct work; > > > > > > > > struct workqueue_struct *wq; > > > > > > > > > > > > > > > > @@ -244,6 +245,7 @@ struct ci_hdrc { > > > > > > > > struct dentry *debugfs; > > > > > > > > bool id_event; > > > > > > > > bool b_sess_valid_event; > > > > > > > > + bool role_switch_event; > > > > > > > > bool imx28_write_fix; > > > > > > > > bool supports_runtime_pm; > > > > > > > > bool in_lpm; > > > > > > > > diff --git a/drivers/usb/chipidea/core.c > > > > > > > > b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6 > > > > > > > > 100644 > > > > > > > > --- a/drivers/usb/chipidea/core.c > > > > > > > > +++ b/drivers/usb/chipidea/core.c > > > > > > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data) > > > > > > > > return ret; > > > > > > > > } > > > > > > > > > > > > > > > > +static int ci_usb_role_switch_set(struct device *dev, > > > > > > > > +enum usb_role > > > > > > > > +role) { > > > > > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > > > > > > > + unsigned long flags; > > > > > > > > + > > > > > > > > + if (ci->role == role || role == USB_ROLE_NONE) > > > > > > > > + return 0; > > > > > > > > + > > > > > > > > + spin_lock_irqsave(&ci->lock, flags); > > > > > > > > + ci->role_switch_event = true; > > > > > > > > + if (ci->role == USB_ROLE_NONE) { > > > > > > > > + if (role == USB_ROLE_DEVICE) > > > > > > > > + ci->role = USB_ROLE_HOST; > > > > > > > > + else > > > > > > > > + ci->role = USB_ROLE_DEVICE; > > > > > > > > + } > > > > > > > > + spin_unlock_irqrestore(&ci->lock, flags); > > > > > > > > + > > > > > > > > + ci_otg_queue_work(ci); > > > > > > > > + > > > > > > > > + return 0; > > > > > > > > +} > > > > > > > > + > > > > > > > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) { > > > > > > > > + struct ci_hdrc *ci = dev_get_drvdata(dev); > > > > > > > > + unsigned long flags; > > > > > > > > + enum usb_role role; > > > > > > > > + > > > > > > > > + spin_lock_irqsave(&ci->lock, flags); > > > > > > > > + role = ci->role; > > > > > > > > + spin_unlock_irqrestore(&ci->lock, flags); > > > > > > > > + > > > > > > > > + return role; > > > > > > > > +} > > > > > > > > + > > > > > > > > +static struct usb_role_switch_desc ci_role_switch = { > > > > > > > > + .set = ci_usb_role_switch_set, > > > > > > > > + .get = ci_usb_role_switch_get, }; > > > > > > > > + > > > > > > > > static int ci_cable_notifier(struct notifier_block *nb, unsigned long event, > > > > > > > > void *ptr) { @@ -689,6 > > > > > > > > +730,9 @@ static int ci_get_platdata(struct device *dev, > > > > > > > > if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL)) > > > > > > > > platdata->flags |= > > > > > > > > CI_HDRC_SET_NON_ZERO_TTHA; > > > > > > > > > > > > > > > > + if (device_property_read_bool(dev, "usb-role-switch")) > > > > > > > > + ci_role_switch.fwnode = dev->fwnode; > > > > > > > > + > > > > > > > > ext_id = ERR_PTR(-ENODEV); > > > > > > > > ext_vbus = ERR_PTR(-ENODEV); > > > > > > > > if (of_property_read_bool(dev->of_node, "extcon")) > > > > > > > > { @@ > > > > > > > > -908,6 > > > > > > > > +952,43 @@ static const struct attribute_group > > > > > > > > +ci_attr_group = { > > > > > > > > .attrs = ci_attrs, }; > > > > > > > > > > > > > > > > +static int ci_start_initial_role(struct ci_hdrc *ci) { > > > > > > > > + int ret = 0; > > > > > > > > + > > > > > > > > + if (ci->roles[USB_ROLE_HOST] && > > > > > > > > + ci->roles[USB_ROLE_DEVICE]) > > > { > > > > > > > > + if (ci->is_otg) { > > > > > > > > + ci->role = ci_otg_role(ci); > > > > > > > > + /* Enable ID change irq */ > > > > > > > > + hw_write_otgsc(ci, OTGSC_IDIE, > OTGSC_IDIE); > > > > > > > > + } else { > > > > > > > > + /* > > > > > > > > + * If the controller is not OTG capable, but support > > > > > > > > + * role switch, the defalt role is gadget, and the > > > > > > > > + * user can switch it through debugfs. > > > > > > > > + */ > > > > > > > > + ci->role = USB_ROLE_DEVICE; > > > > > > > > + } > > > > > > > > + } else { > > > > > > > > + ci->role = ci->roles[USB_ROLE_HOST] > > > > > > > > + ? USB_ROLE_HOST > > > > > > > > + : USB_ROLE_DEVICE; > > > > > > > > + } > > > > > > > > + > > > > > > > > + if (!ci_otg_is_fsm_mode(ci)) { > > > > > > > > + /* only update vbus status for peripheral */ > > > > > > > > + if (ci->role == USB_ROLE_DEVICE) > > > > > > > > + ci_handle_vbus_change(ci); > > > > > > > > + > > > > > > > > + ret = ci_role_start(ci, ci->role); > > > > > > > > + if (ret) > > > > > > > > + dev_err(ci->dev, "can't start %s role\n", > > > > > > > > + ci_role(ci)->name); > > > > > > > > + } > > > > > > > > + > > > > > > > > + return ret; > > > > > > > > +} > > > > > > > > + > > > > > > > > static int ci_hdrc_probe(struct platform_device *pdev) { > > > > > > > > struct device *dev = &pdev->dev; > > > > > > > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct > > > > platform_device *pdev) > > > > > > > > } > > > > > > > > } > > > > > > > > > > > > > > > > - if (ci->roles[USB_ROLE_HOST] && ci- > > >roles[USB_ROLE_DEVICE]) { > > > > > > > > - if (ci->is_otg) { > > > > > > > > - ci->role = ci_otg_role(ci); > > > > > > > > - /* Enable ID change irq */ > > > > > > > > - hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > > > > > > > > - } else { > > > > > > > > - /* > > > > > > > > - * If the controller is not OTG capable, but support > > > > > > > > - * role switch, the defalt role is gadget, and the > > > > > > > > - * user can switch it through debugfs. > > > > > > > > - */ > > > > > > > > - ci->role = USB_ROLE_DEVICE; > > > > > > > > - } > > > > > > > > - } else { > > > > > > > > - ci->role = ci->roles[USB_ROLE_HOST] > > > > > > > > - ? USB_ROLE_HOST > > > > > > > > - : USB_ROLE_DEVICE; > > > > > > > > - } > > > > > > > > - > > > > > > > > - if (!ci_otg_is_fsm_mode(ci)) { > > > > > > > > - /* only update vbus status for peripheral */ > > > > > > > > - if (ci->role == USB_ROLE_DEVICE) > > > > > > > > - ci_handle_vbus_change(ci); > > > > > > > > - > > > > > > > > - ret = ci_role_start(ci, ci->role); > > > > > > > > - if (ret) { > > > > > > > > - dev_err(dev, "can't start %s role\n", > > > > > > > > - ci_role(ci)->name); > > > > > > > > + if (!ci_role_switch.fwnode) { > > > > > > > > + ret = ci_start_initial_role(ci); > > > > > > > > + if (ret) > > > > > > > > goto stop; > > > > > > > > - } > > > > > > > > } > > > > > > > > > > > > > > > > ret = devm_request_irq(dev, ci->irq, ci_irq, > > > > > > > > IRQF_SHARED, @@ > > > > > > > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct > > > > > > > > platform_device > > > > *pdev) > > > > > > > > if (ret) > > > > > > > > goto stop; > > > > > > > > > > > > > > > > + if (ci_role_switch.fwnode) { > > > > > > > > + ci->role_switch = usb_role_switch_register(dev, > > > > > > > > + &ci_role_switch); > > > > > > > > + if (IS_ERR(ci->role_switch)) { > > > > > > > > + ret = PTR_ERR(ci->role_switch); > > > > > > > > + goto stop; > > > > > > > > + } > > > > > > > > + } > > > > > > > > + > > > > > > > > if (ci->supports_runtime_pm) { > > > > > > > > pm_runtime_set_active(&pdev->dev); > > > > > > > > pm_runtime_enable(&pdev->dev); @@ -1133,6 > > > > > > > > +1197,9 @@ static int ci_hdrc_remove(struct > > > > > > > > +platform_device > > > > > > > > +*pdev) { > > > > > > > > struct ci_hdrc *ci = platform_get_drvdata(pdev); > > > > > > > > > > > > > > > > + if (ci->role_switch) > > > > > > > > + > > > > > > > > + usb_role_switch_unregister(ci->role_switch); > > > > > > > > + > > > > > > > > if (ci->supports_runtime_pm) { > > > > > > > > pm_runtime_get_sync(&pdev->dev); > > > > > > > > pm_runtime_disable(&pdev->dev); diff --git > > > > > > > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c > > > > > > > > index > > > > > > > > 5bde0b5..0a22855 100644 > > > > > > > > --- a/drivers/usb/chipidea/otg.c > > > > > > > > +++ b/drivers/usb/chipidea/otg.c > > > > > > > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct > > > > > > > > work_struct > > *work) > > > > > > > > ci_handle_vbus_change(ci); > > > > > > > > } > > > > > > > > > > > > > > > > + if (ci->role_switch_event) { > > > > > > > > + ci->role_switch_event = false; > > > > > > > > + > > > > > > > > + if (ci->role == USB_ROLE_DEVICE) { > > > > > > > > + usb_gadget_vbus_disconnect(&ci->gadget); > > > > > > > > + ci_role_start(ci, USB_ROLE_HOST); > > > > > > > > + } else if (ci->role == USB_ROLE_HOST) { > > > > > > > > + ci_role_stop(ci); > > > > > > > > + usb_gadget_vbus_connect(&ci->gadget); > > > > > > > > + ci->role = USB_ROLE_DEVICE; > > > > > > > > + } > > > > > > > > + } > > > > > > > > + > > > > > > > > pm_runtime_put_sync(ci->dev); > > > > > > > > > > > > > > > > enable_irq(ci->irq); > > > > > > > > -- > > > > > > > > 2.7.4 > > > > > > > >
> > > > > > > > > > > > You may use connect bit at portsc since VBUS may not connect to SoC. > > > > > > By this way, disconnect can be decided but connect is a problem in > > > current driver, as controller was stopped in low power mode so can't > > > detect connection from host, unless we also update this behavior too. > > > > > > > For connection, if current role is USB_ROLE_NONE, you may start device mode. > > This is assuming set role only can be called one time between disconnect and > connect to host, this may not always true, but this can be resolved, I think we need > handle the case device mode is started but connection does not happen, so the > gadget need be stopped and enter low power mode again, if this approach is OK to > you, I will go in this direction for my v2. > After thinking more, I think Type-C tcpm code should set usb_role correctly, it knows physical connection status well, and there are already USB_ROLE_NONE references at tcpm now. Depending on USB device driver workaround to know USB_ROLE_NONE is not a good solution. Look at another USB role driver, intel-xhci-usb-role-switch.c, it could also get USB_ROLE_NONE state. Peter
> -----Original Message----- > From: Peter Chen > Sent: 2019年8月7日 10:41 > To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; dl-linux-imx > <linux-imx@nxp.com>; USB list <linux-usb@vger.kernel.org> > Subject: RE: [PATCH 2/5] usb: chipidea: add role switch class support > > > > > > > > > > > > > > > > > > > You may use connect bit at portsc since VBUS may not connect to SoC. > > > > > > > > By this way, disconnect can be decided but connect is a problem in > > > > current driver, as controller was stopped in low power mode so > > > > can't detect connection from host, unless we also update this behavior too. > > > > > > > > > > For connection, if current role is USB_ROLE_NONE, you may start device mode. > > > > This is assuming set role only can be called one time between > > disconnect and connect to host, this may not always true, but this can > > be resolved, I think we need handle the case device mode is started > > but connection does not happen, so the gadget need be stopped and > > enter low power mode again, if this approach is OK to you, I will go in this direction for my > v2. > > > > After thinking more, I think Type-C tcpm code should set usb_role correctly, it knows > physical connection status well, and there are already USB_ROLE_NONE references at > tcpm now. Depending on USB device driver workaround to know USB_ROLE_NONE is > not a good solution. Look at another USB role driver, intel-xhci-usb-role-switch.c, it could > also get USB_ROLE_NONE state. Sorry, I re-checked the latest tcpm code found there is already USB_ROLE_NONE setting for not disconnect, so we are fine to handle it in usb controller driver, will post v2 for this soon. Li Jun > > Peter
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index 82b86cd..5e2f0bc 100644 --- a/drivers/usb/chipidea/ci.h +++ b/drivers/usb/chipidea/ci.h @@ -212,6 +212,7 @@ struct ci_hdrc { ktime_t hr_timeouts[NUM_OTG_FSM_TIMERS]; unsigned enabled_otg_timer_bits; enum otg_fsm_timer next_otg_timer; + struct usb_role_switch *role_switch; struct work_struct work; struct workqueue_struct *wq; @@ -244,6 +245,7 @@ struct ci_hdrc { struct dentry *debugfs; bool id_event; bool b_sess_valid_event; + bool role_switch_event; bool imx28_write_fix; bool supports_runtime_pm; bool in_lpm; diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data) return ret; } +static int ci_usb_role_switch_set(struct device *dev, enum usb_role role) +{ + struct ci_hdrc *ci = dev_get_drvdata(dev); + unsigned long flags; + + if (ci->role == role || role == USB_ROLE_NONE) + return 0; + + spin_lock_irqsave(&ci->lock, flags); + ci->role_switch_event = true; + if (ci->role == USB_ROLE_NONE) { + if (role == USB_ROLE_DEVICE) + ci->role = USB_ROLE_HOST; + else + ci->role = USB_ROLE_DEVICE; + } + spin_unlock_irqrestore(&ci->lock, flags); + + ci_otg_queue_work(ci); + + return 0; +} + +static enum usb_role ci_usb_role_switch_get(struct device *dev) +{ + struct ci_hdrc *ci = dev_get_drvdata(dev); + unsigned long flags; + enum usb_role role; + + spin_lock_irqsave(&ci->lock, flags); + role = ci->role; + spin_unlock_irqrestore(&ci->lock, flags); + + return role; +} + +static struct usb_role_switch_desc ci_role_switch = { + .set = ci_usb_role_switch_set, + .get = ci_usb_role_switch_get, +}; + static int ci_cable_notifier(struct notifier_block *nb, unsigned long event, void *ptr) { @@ -689,6 +730,9 @@ static int ci_get_platdata(struct device *dev, if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL)) platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA; + if (device_property_read_bool(dev, "usb-role-switch")) + ci_role_switch.fwnode = dev->fwnode; + ext_id = ERR_PTR(-ENODEV); ext_vbus = ERR_PTR(-ENODEV); if (of_property_read_bool(dev->of_node, "extcon")) { @@ -908,6 +952,43 @@ static const struct attribute_group ci_attr_group = { .attrs = ci_attrs, }; +static int ci_start_initial_role(struct ci_hdrc *ci) +{ + int ret = 0; + + if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { + if (ci->is_otg) { + ci->role = ci_otg_role(ci); + /* Enable ID change irq */ + hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); + } else { + /* + * If the controller is not OTG capable, but support + * role switch, the defalt role is gadget, and the + * user can switch it through debugfs. + */ + ci->role = USB_ROLE_DEVICE; + } + } else { + ci->role = ci->roles[USB_ROLE_HOST] + ? USB_ROLE_HOST + : USB_ROLE_DEVICE; + } + + if (!ci_otg_is_fsm_mode(ci)) { + /* only update vbus status for peripheral */ + if (ci->role == USB_ROLE_DEVICE) + ci_handle_vbus_change(ci); + + ret = ci_role_start(ci, ci->role); + if (ret) + dev_err(ci->dev, "can't start %s role\n", + ci_role(ci)->name); + } + + return ret; +} + static int ci_hdrc_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct platform_device *pdev) } } - if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) { - if (ci->is_otg) { - ci->role = ci_otg_role(ci); - /* Enable ID change irq */ - hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); - } else { - /* - * If the controller is not OTG capable, but support - * role switch, the defalt role is gadget, and the - * user can switch it through debugfs. - */ - ci->role = USB_ROLE_DEVICE; - } - } else { - ci->role = ci->roles[USB_ROLE_HOST] - ? USB_ROLE_HOST - : USB_ROLE_DEVICE; - } - - if (!ci_otg_is_fsm_mode(ci)) { - /* only update vbus status for peripheral */ - if (ci->role == USB_ROLE_DEVICE) - ci_handle_vbus_change(ci); - - ret = ci_role_start(ci, ci->role); - if (ret) { - dev_err(dev, "can't start %s role\n", - ci_role(ci)->name); + if (!ci_role_switch.fwnode) { + ret = ci_start_initial_role(ci); + if (ret) goto stop; - } } ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED, @@ -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct platform_device *pdev) if (ret) goto stop; + if (ci_role_switch.fwnode) { + ci->role_switch = usb_role_switch_register(dev, + &ci_role_switch); + if (IS_ERR(ci->role_switch)) { + ret = PTR_ERR(ci->role_switch); + goto stop; + } + } + if (ci->supports_runtime_pm) { pm_runtime_set_active(&pdev->dev); pm_runtime_enable(&pdev->dev); @@ -1133,6 +1197,9 @@ static int ci_hdrc_remove(struct platform_device *pdev) { struct ci_hdrc *ci = platform_get_drvdata(pdev); + if (ci->role_switch) + usb_role_switch_unregister(ci->role_switch); + if (ci->supports_runtime_pm) { pm_runtime_get_sync(&pdev->dev); pm_runtime_disable(&pdev->dev); diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index 5bde0b5..0a22855 100644 --- a/drivers/usb/chipidea/otg.c +++ b/drivers/usb/chipidea/otg.c @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work) ci_handle_vbus_change(ci); } + if (ci->role_switch_event) { + ci->role_switch_event = false; + + if (ci->role == USB_ROLE_DEVICE) { + usb_gadget_vbus_disconnect(&ci->gadget); + ci_role_start(ci, USB_ROLE_HOST); + } else if (ci->role == USB_ROLE_HOST) { + ci_role_stop(ci); + usb_gadget_vbus_connect(&ci->gadget); + ci->role = USB_ROLE_DEVICE; + } + } + pm_runtime_put_sync(ci->dev); enable_irq(ci->irq);