> -----Original Message----- > From: Peter Chen > Sent: 2019年8月8日 17:31 > To: Jun Li <jun.li@xxxxxxx> > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; Jun Li <jun.li@xxxxxxx>; dl-linux-imx > <linux-imx@xxxxxxx>; linux-usb@xxxxxxxxxxxxxxx > Subject: RE: [PATCH v2 2/2] usb: chipidea: add role switch class support > > > > USB role is fully controlled by usb role switch consumer(e.g. typec), > > usb port can be at host mode(USB_ROLE_HOST), device mode connected to > > host(USB_ROLE_DEVICE), or not connecting any parter(USB_ROLE_NONE). > > > > %s/parter/partner ? Yes, I will update. > > Are there any ways you could get external cable status from usb-switch or type-c like > external connector? If there are, you could update otgsc value for otgsc_read, or change > cable status, and avoid changing common handling, like ci_hdrc_probe and ci_otg_work. > And it could benefit for other use cases, like booting with cable connected and switch role > through /sys. The new role switch class has nothing to do with extcon, it's using graph bindings to link the connection, so there is no extcon_dev, change for ci_hdrc_probe() is required as property usb-role-switch has to be specified, there may be some code reuse if still touch the external cable even no extcon dev, but that will make existing external cable complicated and not clean. On the other hand, a new GPIO based role switch driver is on review: https://patchwork.kernel.org/patch/11056379/ Seems this is the direction to move out from extcon. Li Jun > > Peter > > > Signed-off-by: Li Jun <jun.li@xxxxxxx> > > --- > > > > Change for v2: > > - Support USB_ROLE_NONE, which for usb port not connecting any > > device or host, and will be in low power mode. > > > > drivers/usb/chipidea/ci.h | 3 ++ > > drivers/usb/chipidea/core.c | 120 > > +++++++++++++++++++++++++++++++++------- > > ---- > > drivers/usb/chipidea/otg.c | 20 ++++++++ > > 3 files changed, 114 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h > > index 82b86cd..f0aec1d 100644 > > --- a/drivers/usb/chipidea/ci.h > > +++ b/drivers/usb/chipidea/ci.h > > @@ -205,6 +205,7 @@ struct ci_hdrc { > > int irq; > > struct ci_role_driver *roles[USB_ROLE_DEVICE + 1]; > > enum usb_role role; > > + enum usb_role target_role; > > bool is_otg; > > struct usb_otg otg; > > struct otg_fsm fsm; > > @@ -212,6 +213,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 +246,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..965ce17 100644 > > --- a/drivers/usb/chipidea/core.c > > +++ b/drivers/usb/chipidea/core.c > > @@ -587,6 +587,42 @@ 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) > > + return 0; > > + > > + spin_lock_irqsave(&ci->lock, flags); > > + ci->role_switch_event = true; > > + ci->target_role = role; > > + 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 +725,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 > > +947,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 +1127,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 > > +1142,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 +1192,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..03675b6 100644 > > --- a/drivers/usb/chipidea/otg.c > > +++ b/drivers/usb/chipidea/otg.c > > @@ -214,6 +214,26 @@ static void ci_otg_work(struct work_struct *work) > > ci_handle_vbus_change(ci); > > } > > > > + if (ci->role_switch_event) { > > + ci->role_switch_event = false; > > + > > + /* Stop current role */ > > + if (ci->role == USB_ROLE_DEVICE) { > > + usb_gadget_vbus_disconnect(&ci->gadget); > > + ci->role = USB_ROLE_NONE; > > + } else if (ci->role == USB_ROLE_HOST) { > > + ci_role_stop(ci); > > + } > > + > > + /* Start target role */ > > + if (ci->target_role == USB_ROLE_DEVICE) { > > + usb_gadget_vbus_connect(&ci->gadget); > > + ci->role = USB_ROLE_DEVICE; > > + } else if (ci->target_role == USB_ROLE_HOST) { > > + ci_role_start(ci, USB_ROLE_HOST); > > + } > > + } > > + > > pm_runtime_put_sync(ci->dev); > > > > enable_irq(ci->irq); > > -- > > 2.7.4
