Hi Heiko, Kishon, I'll try to pick up this patch. Some comments below, just for self-reference.
On Wed, 2019-06-26 at 12:32 -0300, Ezequiel Garcia wrote: > On Wed, 2019-05-15 at 18:20 -0400, Gaël PORTAY wrote: > > From: Enric Balletbo i Serra <enric.balle...@collabora.com> > > > > The OTG disconnection event is generated after the presence/absence of > > an ID connection, but some platforms don't have the ID pin connected, so > > the event is not generated. In such case, for detecting the > > disconnection event, we can get the cable state from an extcon driver. > > We need, though, to force to set the B-Device Session Valid bit on the > > PHY to have the device respond to the setup address. Otherwise, the > > following error is shown: > > > > usb 2-2: Device not responding to setup address. > > usb 2-2: device not accepting address 14, error -71 > > usb usb2-port2: unable to enumerate USB device > > > > The patch tells the PHY to force the B-Device Session Valid bit when the > > OTG role is device and clear that bit if the OTG role is host, when an > > extcon is available. > > > > Signed-off-by: Enric Balletbo i Serra <enric.balle...@collabora.com> > > Signed-off-by: Gaël PORTAY <gael.por...@collabora.com> > > --- > > > > Hi all, > > > > The main purpose of this patch is have the Type-C port on the Samsung > > Chromebook Plus work as a device or in OTG mode. > > > > That patch was originally a part of that patchset[1]; all other patches > > was merged recently in master. > > > > The patch was tested on a Samsung Chromebook Plus by configuring one > > port to work as device, configure a cdc ethernet gadget and communicate > > via ethernet gadget my workstation with the chromebook through a usb-a > > to type-c cable. > > > > Best regards, > > Gaël > > > > [1]: https://lkml.org/lkml/2018/8/15/141 > > We still need the above devicetree changes. > > Changes since v1: > > - [PATCH 3/4] Remove introduction of dt property "rockchip,force-bvalid" > > and replace cable state using extcon instead (if set). > > > > drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 51 +++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > > > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > > b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > > index ba07121c3eff..5e9d50b5ae16 100644 > > --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > > +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > > @@ -125,6 +125,7 @@ struct rockchip_chg_det_reg { > > * @bvalid_det_en: vbus valid rise detection enable register. > > * @bvalid_det_st: vbus valid rise detection status register. > > * @bvalid_det_clr: vbus valid rise detection clear register. > > + * @bvalid_session: force B-device session valid register. > > * @ls_det_en: linestate detection enable register. > > * @ls_det_st: linestate detection state register. > > * @ls_det_clr: linestate detection clear register. > > @@ -138,6 +139,7 @@ struct rockchip_usb2phy_port_cfg { > > struct usb2phy_reg bvalid_det_en; > > struct usb2phy_reg bvalid_det_st; > > struct usb2phy_reg bvalid_det_clr; > > + struct usb2phy_reg bvalid_session; > > struct usb2phy_reg ls_det_en; > > struct usb2phy_reg ls_det_st; > > struct usb2phy_reg ls_det_clr; > > @@ -169,6 +171,7 @@ struct rockchip_usb2phy_cfg { > > * @port_id: flag for otg port or host port. > > * @suspended: phy suspended flag. > > * @vbus_attached: otg device vbus status. > > + * @force_bvalid: force the control of the B-device session valid bit. > > * @bvalid_irq: IRQ number assigned for vbus valid rise detection. > > * @ls_irq: IRQ number assigned for linestate detection. > > * @otg_mux_irq: IRQ number which multiplex otg-id/otg-bvalid/linestate > > @@ -187,6 +190,7 @@ struct rockchip_usb2phy_port { > > unsigned int port_id; > > bool suspended; > > bool vbus_attached; > > + bool force_bvalid; > > int bvalid_irq; > > int ls_irq; > > int otg_mux_irq; > > @@ -553,6 +557,13 @@ static void rockchip_usb2phy_otg_sm_work(struct > > work_struct *work) > > switch (rport->state) { > > case OTG_STATE_UNDEFINED: > > rport->state = OTG_STATE_B_IDLE; > > + if (rport->force_bvalid) { > > + property_enable(rphy->grf, > > + &rport->port_cfg->bvalid_session, > > + true); > > + dev_dbg(&rport->phy->dev, > > + "set the B-Device Session Valid\n"); > > + } > > if (!vbus_attach) > > rockchip_usb2phy_power_off(rport->phy); > > /* fall through */ > > @@ -560,6 +571,14 @@ static void rockchip_usb2phy_otg_sm_work(struct > > work_struct *work) > > if (extcon_get_state(rphy->edev, EXTCON_USB_HOST) > 0) { > > dev_dbg(&rport->phy->dev, "usb otg host connect\n"); > > rport->state = OTG_STATE_A_HOST; > > + /* When leaving device mode force end the session */ > > + if (rport->force_bvalid) { > > + property_enable(rphy->grf, > > + &rport->port_cfg->bvalid_session, > > + false); > > + dev_dbg(&rport->phy->dev, > > + "clear the B-Device Session Valid\n"); > > + } > > rockchip_usb2phy_power_on(rport->phy); > > return; > > } else if (vbus_attach) { > > @@ -634,6 +653,14 @@ static void rockchip_usb2phy_otg_sm_work(struct > > work_struct *work) > > if (extcon_get_state(rphy->edev, EXTCON_USB_HOST) == 0) { > > dev_dbg(&rport->phy->dev, "usb otg host disconnect\n"); > > rport->state = OTG_STATE_B_IDLE; > > + /* When leaving host mode force start the session */ > > + if (rport->force_bvalid) { > > + property_enable(rphy->grf, > > + &rport->port_cfg->bvalid_session, > > + true); > > + dev_dbg(&rport->phy->dev, > > + "set the B-Device Session Valid\n"); > > + } > > rockchip_usb2phy_power_off(rport->phy); > > } > > break; > > @@ -1016,6 +1043,28 @@ static int rockchip_usb2phy_otg_port_init(struct > > rockchip_usb2phy *rphy, > > INIT_DELAYED_WORK(&rport->chg_work, rockchip_chg_detect_work); > > INIT_DELAYED_WORK(&rport->otg_sm_work, rockchip_usb2phy_otg_sm_work); > > > > + /* > > + * Some platforms doesn't have the ID pin connected to the phy, hence > > + * the OTD ID event is not generated, but in some cases we can get the > > + * cable state from an extcon driver. In such case we can force to set > > + * the B-Device Session Valid bit on the PHY to have the device working > > + * as a OTG. > > + */ > > + if (rphy->edev) { I might be missing something, but this check seems bogus. edev can't be NULL as the driver creates an extcon if there is none assigned in the devicetree. > > + /* > > + * Check if bvalid_session register is set in the structure > > + * rockchip_usb2phy_cfg for this SoC. > > + */ > > + if (rport->port_cfg->bvalid_session.offset == 0x0) { > > + rport->force_bvalid = false; > > + dev_err(rphy->dev, > > + "cannot force B-device session, the register is > > not set for that SoC\n"); > > + } else { > > + rport->force_bvalid = true; > > + dev_info(rphy->dev, "force B-device session enabled\n"); > > + } I think we should be doing something more like: if (HAS_REG(rport->port_cfg->bvalid_session)) { rport->force_bvalid = true; dev_info(rphy->dev, "force B-device session enabled\n"); } And not error verbosely on platforms that don't care about this. Thanks, Ezequiel
