Message ID | 20200303171159.246992-1-bryan.odonoghue@linaro.org |
---|---|
Headers | show |
Series | Enable Qualcomm QCS 404 HS/SS USB | expand |
On Tue, 3 Mar 2020 17:11:48 +0000, Bryan O'Donoghue wrote: > A USB connector should be a child node of the USB controller > connector/usb-connector.txt. This patch adds an example of how to do this > to the dwc3 binding descriptions. > > It is necessary to declare a connector as a child-node of a USB controller > for role-switching to work, so this example should be helpful to others > implementing that. > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: linux-usb@vger.kernel.org > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Acked-by: Felipe Balbi <balbi@kernel.org> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > Documentation/devicetree/bindings/usb/dwc3.txt | 8 ++++++++ > 1 file changed, 8 insertions(+) > Please add Acked-by/Reviewed-by tags when posting new versions. However, there's no need to repost patches *only* to add the tags. The upstream maintainer will do that for acks received on the version they apply. If a tag was not added on purpose, please state why and what changed.
On Tue 03 Mar 09:11 PST 2020, Bryan O'Donoghue wrote: > From: Yu Chen <chenyu56@huawei.com> > > The Type-C drivers use USB role switch API to inform the > system about the negotiated data role, so registering a role > switch in the DRD code in order to support platforms with > USB Type-C connectors. > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > CC: ShuFan Lee <shufan_lee@richtek.com> > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Chunfeng Yun <chunfeng.yun@mediatek.com> > Cc: Yu Chen <chenyu56@huawei.com> > Cc: Felipe Balbi <balbi@kernel.org> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > Cc: Jun Li <lijun.kernel@gmail.com> > Cc: Valentin Schneider <valentin.schneider@arm.com> > Cc: Guillaume Gardet <Guillaume.Gardet@arm.com> > Cc: Jack Pham <jackp@codeaurora.org> > Cc: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Cc: linux-usb@vger.kernel.org > Cc: devicetree@vger.kernel.org > Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Signed-off-by: Yu Chen <chenyu56@huawei.com> > Signed-off-by: John Stultz <john.stultz@linaro.org> > --- > drivers/usb/dwc3/core.h | 3 ++ > drivers/usb/dwc3/drd.c | 77 ++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 79 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 77c4a9abe365..a99e57636172 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -25,6 +25,7 @@ > #include <linux/usb/ch9.h> > #include <linux/usb/gadget.h> > #include <linux/usb/otg.h> > +#include <linux/usb/role.h> > #include <linux/ulpi/interface.h> > > #include <linux/phy/phy.h> > @@ -953,6 +954,7 @@ struct dwc3_scratchpad_array { > * @hsphy_mode: UTMI phy mode, one of following: > * - USBPHY_INTERFACE_MODE_UTMI > * - USBPHY_INTERFACE_MODE_UTMIW > + * @role_sw: usb_role_switch handle > * @usb2_phy: pointer to USB2 PHY > * @usb3_phy: pointer to USB3 PHY > * @usb2_generic_phy: pointer to USB2 PHY > @@ -1086,6 +1088,7 @@ struct dwc3 { > struct extcon_dev *edev; > struct notifier_block edev_nb; > enum usb_phy_interface hsphy_mode; > + struct usb_role_switch *role_sw; > > u32 fladj; > u32 irq_gadget; > diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c > index c946d64142ad..331c6e997f0c 100644 > --- a/drivers/usb/dwc3/drd.c > +++ b/drivers/usb/dwc3/drd.c > @@ -476,6 +476,73 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc) > return edev; > } > > +#if IS_ENABLED(CONFIG_USB_ROLE_SWITCH) > +#define ROLE_SWITCH 1 > +static int dwc3_usb_role_switch_set(struct device *dev, enum usb_role role) The prototype for set and get was changed in 'bce3052f0c16 ("usb: roles: Provide the switch drivers handle to the switch in the API")', so this no longer compiles. The new prototype should be: static int dwc3_usb_role_switch_set(struct usb_role_switch *sw, enum usb_role role) > +{ > + struct dwc3 *dwc = dev_get_drvdata(dev); And this would then be: struct dwc3 *dwc = usb_role_switch_get_drvdata(sw); > + u32 mode; > + > + switch (role) { > + case USB_ROLE_HOST: > + mode = DWC3_GCTL_PRTCAP_HOST; > + break; > + case USB_ROLE_DEVICE: > + mode = DWC3_GCTL_PRTCAP_DEVICE; > + break; > + default: > + mode = DWC3_GCTL_PRTCAP_DEVICE; > + break; > + } > + > + dwc3_set_mode(dwc, mode); > + return 0; > +} > + > +static enum usb_role dwc3_usb_role_switch_get(struct device *dev) static enum usb_role dwc3_usb_role_switch_get(struct usb_role_switch sw) > +{ > + struct dwc3 *dwc = dev_get_drvdata(dev); struct dwc3 *dwc = usb_role_switch_get_drvdata(sw); > + unsigned long flags; > + enum usb_role role; > + > + spin_lock_irqsave(&dwc->lock, flags); > + switch (dwc->current_dr_role) { > + case DWC3_GCTL_PRTCAP_HOST: > + role = USB_ROLE_HOST; > + break; > + case DWC3_GCTL_PRTCAP_DEVICE: > + role = USB_ROLE_DEVICE; > + break; > + case DWC3_GCTL_PRTCAP_OTG: > + role = dwc->current_otg_role; > + break; > + default: > + role = USB_ROLE_DEVICE; > + break; > + } > + spin_unlock_irqrestore(&dwc->lock, flags); > + return role; > +} > + > +static int dwc3_setup_role_switch(struct dwc3 *dwc) > +{ > + struct usb_role_switch_desc dwc3_role_switch = {NULL}; > + > + dwc3_role_switch.fwnode = dev_fwnode(dwc->dev); > + dwc3_role_switch.set = dwc3_usb_role_switch_set; > + dwc3_role_switch.get = dwc3_usb_role_switch_get; And you need to pass dwc as .driver_data here: dwc3_role_switch.driver_data = dwc; With this the series compiles and both dwc3 devices probes nicely. I haven't done any further testing though... Regards, Bjorn > + dwc->role_sw = usb_role_switch_register(dwc->dev, &dwc3_role_switch); > + if (IS_ERR(dwc->role_sw)) > + return PTR_ERR(dwc->role_sw); > + > + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE); > + return 0; > +} > +#else > +#define ROLE_SWITCH 0 > +#define dwc3_setup_role_switch(x) 0 > +#endif > + > int dwc3_drd_init(struct dwc3 *dwc) > { > int ret, irq; > @@ -484,7 +551,12 @@ int dwc3_drd_init(struct dwc3 *dwc) > if (IS_ERR(dwc->edev)) > return PTR_ERR(dwc->edev); > > - if (dwc->edev) { > + if (ROLE_SWITCH && > + device_property_read_bool(dwc->dev, "usb-role-switch")) { > + ret = dwc3_setup_role_switch(dwc); > + if (ret < 0) > + return ret; > + } else if (dwc->edev) { > dwc->edev_nb.notifier_call = dwc3_drd_notifier; > ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST, > &dwc->edev_nb); > @@ -531,6 +603,9 @@ void dwc3_drd_exit(struct dwc3 *dwc) > { > unsigned long flags; > > + if (dwc->role_sw) > + usb_role_switch_unregister(dwc->role_sw); > + > if (dwc->edev) > extcon_unregister_notifier(dwc->edev, EXTCON_USB_HOST, > &dwc->edev_nb); > -- > 2.25.1 >
On 08/03/2020 05:23, Bjorn Andersson wrote: > On Tue 03 Mar 09:11 PST 2020, Bryan O'Donoghue wrote: > >> V1: >> This series enables the Primary and Secondary USB controllers on the >> QCS404, associated PHYs, role-switching and DTS descriptions. >> > > Finally took the time to give this a spin on my QCS404 dev board. > > Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org> thanks ! > > As this touches three different subsystems, and doesn't have have > compile time dependencies between the parts, I would suggest that as you > fix up the build error I reported yesterday you send v8 as three > different series - one per maintainer/subsystem. That way we avoid any > questions about whom should merge what parts and in what order. ack, will do. --- bod