[PATCH v5 1/2] phy: rockchip: inno-usb2: add soft vbusvalid control
Nicolas Frattaroli
nicolas.frattaroli at collabora.com
Thu Jun 19 13:23:35 PDT 2025
Hi Heiko,
On Thursday, 19 June 2025 21:42:00 Central European Summer Time Heiko Stuebner wrote:
> Hi Nicolas,
>
> Am Donnerstag, 19. Juni 2025, 20:36:36 Mitteleuropäische Sommerzeit schrieb Nicolas Frattaroli:
> > With USB type C connectors, the vbus detect pin of the OTG controller
> > attached to it is pulled high by a USB Type C controller chip such as
> > the fusb302. This means USB enumeration on Type-C ports never works, as
> > the vbus is always seen as high.
> >
> > Rockchip added some GRF register flags to deal with this situation. The
> > RK3576 TRM calls these "soft_vbusvalid_bvalid" (con0 bit index 15) and
> > "soft_vbusvalid_bvalid_sel" (con0 bit index 14).
>
> I would expect a paragraph more about what those bits (and their
> functionality) actually do here :-)
:( fiiine
Quick non-patch explainer though, in case it helps you spot a problem in
the code: looks like svbus_sel to 1 tells the SoC that the OTG
controller's vbus valid and bvalid signal is controlled by the svbus_en
GRF flag instead of whatever the controller does based on what it sees
on the voltage lines.
>
>
> > Downstream introduces a new vendor property which tells the USB 2 PHY
> > that it's connected to a type C port, but we can do better. Since in
> > such an arrangement, we'll have an OF graph connection from the USB
> > controller to the USB connector anyway, we can walk said OF graph and
> > check the connector's compatible to determine this without adding any
> > further vendor properties.
> >
> > Do keep in mind that the usbdp PHY driver seemingly fiddles with these
> > register fields as well, but what it does doesn't appear to be enough
> > for us to get working USB enumeration, presumably because the whole
> > vbus_attach logic needs to be adjusted as well either way.
>
>
> In the rk3588 TRM I find USB2PHY_GRF_CON4
> bit3 - sft_vbus_sel (VBUS software control select)
> bit2 - sft_vbus (When sft_vbus_sel is 1'b1, vbusvalid and bvalid is
> controlled by sft_vbus)
>
> Is that the same stuff as you're adding for rk3576 ?
Yes, these appear to be the same ones. I'd need to check whether Type-C
USB devices enumerate on RK3588 first to see if we have the same problem
there though (it would be weird if it weren't a problem there).
If you pick the DT patch, I can send a new version of the soft vbusvalid
control patch with RK3588 addressed as well, if I manage to confirm it's
the same thing there.
>
> My last dance with rk3588-type-c is already some months back, but I do
> remember running into "some" issue - but don't remember which one ;-)
>
>
> Heiko
>
Kind regards,
Nicolas Frattaroli
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli at collabora.com>
> > ---
> > drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 108 +++++++++++++++++++++++++-
> > 1 file changed, 104 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > index b0f23690ec3002202c0f33a6988f5509622fa10e..71810c07e4150ea81f65a8a932541b144e95a137 100644
> > --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > @@ -17,6 +17,7 @@
> > #include <linux/module.h>
> > #include <linux/mutex.h>
> > #include <linux/of.h>
> > +#include <linux/of_graph.h>
> > #include <linux/of_irq.h>
> > #include <linux/phy/phy.h>
> > #include <linux/platform_device.h>
> > @@ -114,6 +115,8 @@ struct rockchip_chg_det_reg {
> > /**
> > * struct rockchip_usb2phy_port_cfg - usb-phy port configuration.
> > * @phy_sus: phy suspend register.
> > + * @svbus_en: soft vbus bvalid enable register.
> > + * @svbus_sel: soft vbus bvalid selection register.
> > * @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.
> > @@ -140,6 +143,8 @@ struct rockchip_chg_det_reg {
> > */
> > struct rockchip_usb2phy_port_cfg {
> > struct usb2phy_reg phy_sus;
> > + struct usb2phy_reg svbus_en;
> > + struct usb2phy_reg svbus_sel;
> > struct usb2phy_reg bvalid_det_en;
> > struct usb2phy_reg bvalid_det_st;
> > struct usb2phy_reg bvalid_det_clr;
> > @@ -203,6 +208,7 @@ struct rockchip_usb2phy_cfg {
> > * @event_nb: hold event notification callback.
> > * @state: define OTG enumeration states before device reset.
> > * @mode: the dr_mode of the controller.
> > + * @typec_vbus_det: whether to apply Type C logic to OTG vbus detection.
> > */
> > struct rockchip_usb2phy_port {
> > struct phy *phy;
> > @@ -222,6 +228,7 @@ struct rockchip_usb2phy_port {
> > struct notifier_block event_nb;
> > enum usb_otg_state state;
> > enum usb_dr_mode mode;
> > + bool typec_vbus_det;
> > };
> >
> > /**
> > @@ -495,6 +502,13 @@ static int rockchip_usb2phy_init(struct phy *phy)
> > mutex_lock(&rport->mutex);
> >
> > if (rport->port_id == USB2PHY_PORT_OTG) {
> > + if (rport->typec_vbus_det) {
> > + if (rport->port_cfg->svbus_en.enable &&
> > + rport->port_cfg->svbus_sel.enable) {
> > + property_enable(rphy->grf, &rport->port_cfg->svbus_en, true);
> > + property_enable(rphy->grf, &rport->port_cfg->svbus_sel, true);
> > + }
> > + }
> > if (rport->mode != USB_DR_MODE_HOST &&
> > rport->mode != USB_DR_MODE_UNKNOWN) {
> > /* clear bvalid status and enable bvalid detect irq */
> > @@ -535,8 +549,7 @@ static int rockchip_usb2phy_init(struct phy *phy)
> > if (ret)
> > goto out;
> >
> > - schedule_delayed_work(&rport->otg_sm_work,
> > - OTG_SCHEDULE_DELAY * 3);
> > + schedule_delayed_work(&rport->otg_sm_work, 0);
> > } else {
> > /* If OTG works in host only mode, do nothing. */
> > dev_dbg(&rport->phy->dev, "mode %d\n", rport->mode);
> > @@ -666,8 +679,12 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
> > unsigned long delay;
> > bool vbus_attach, sch_work, notify_charger;
> >
> > - vbus_attach = property_enabled(rphy->grf,
> > - &rport->port_cfg->utmi_bvalid);
> > + if (rport->port_cfg->svbus_en.enable && rport->typec_vbus_det) {
> > + vbus_attach = true;
> > + } else {
> > + vbus_attach = property_enabled(rphy->grf,
> > + &rport->port_cfg->utmi_bvalid);
> > + }
> >
> > sch_work = false;
> > notify_charger = false;
> > @@ -1276,6 +1293,83 @@ static int rockchip_otg_event(struct notifier_block *nb,
> > return NOTIFY_DONE;
> > }
> >
> > +static const char *const rockchip_usb2phy_typec_cons[] = {
> > + "usb-c-connector",
> > + NULL,
> > +};
> > +
> > +static struct device_node *rockchip_usb2phy_to_controller(struct rockchip_usb2phy *rphy)
> > +{
> > + struct device_node *np;
> > + struct device_node *parent;
> > +
> > + for_each_node_with_property(np, "phys") {
> > + struct of_phandle_iterator it;
> > + int ret;
> > +
> > + of_for_each_phandle(&it, ret, np, "phys", NULL, 0) {
> > + parent = of_get_parent(it.node);
> > + if (it.node != rphy->dev->of_node && rphy->dev->of_node != parent) {
> > + if (parent)
> > + of_node_put(parent);
> > + continue;
> > + }
> > +
> > + /*
> > + * Either the PHY phandle we're iterating or its parent
> > + * matched, we don't care about which out of the two in
> > + * particular as we just need to know it's the right
> > + * USB controller for this PHY.
> > + */
> > + of_node_put(it.node);
> > + of_node_put(parent);
> > + return np;
> > + }
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static bool rockchip_usb2phy_otg_is_type_c(struct rockchip_usb2phy *rphy)
> > +{
> > + struct device_node *controller = rockchip_usb2phy_to_controller(rphy);
> > + struct device_node *ports;
> > + struct device_node *ep = NULL;
> > + struct device_node *parent;
> > +
> > + if (!controller)
> > + return false;
> > +
> > + ports = of_get_child_by_name(controller, "ports");
> > + if (ports) {
> > + of_node_put(controller);
> > + controller = ports;
> > + }
> > +
> > + for_each_of_graph_port(controller, port) {
> > + ep = of_get_child_by_name(port, "endpoint");
> > + if (!ep)
> > + continue;
> > +
> > + parent = of_graph_get_remote_port_parent(ep);
> > + of_node_put(ep);
> > + if (!parent)
> > + continue;
> > +
> > + if (of_device_compatible_match(parent, rockchip_usb2phy_typec_cons)) {
> > + of_node_put(parent);
> > + of_node_put(controller);
> > + return true;
> > + }
> > +
> > + of_node_put(parent);
> > + }
> > +
> > + of_node_put(controller);
> > +
> > + return false;
> > +}
> > +
> > static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
> > struct rockchip_usb2phy_port *rport,
> > struct device_node *child_np)
> > @@ -1297,6 +1391,8 @@ static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
> >
> > mutex_init(&rport->mutex);
> >
> > + rport->typec_vbus_det = rockchip_usb2phy_otg_is_type_c(rphy);
> > +
> > rport->mode = of_usb_get_dr_mode_by_phy(child_np, -1);
> > if (rport->mode == USB_DR_MODE_HOST ||
> > rport->mode == USB_DR_MODE_UNKNOWN) {
> > @@ -2050,6 +2146,8 @@ static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
> > .port_cfgs = {
> > [USB2PHY_PORT_OTG] = {
> > .phy_sus = { 0x0000, 8, 0, 0, 0x1d1 },
> > + .svbus_en = { 0x0000, 15, 15, 0, 1 },
> > + .svbus_sel = { 0x0000, 14, 14, 0, 1 },
> > .bvalid_det_en = { 0x00c0, 1, 1, 0, 1 },
> > .bvalid_det_st = { 0x00c4, 1, 1, 0, 1 },
> > .bvalid_det_clr = { 0x00c8, 1, 1, 0, 1 },
> > @@ -2087,6 +2185,8 @@ static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
> > .port_cfgs = {
> > [USB2PHY_PORT_OTG] = {
> > .phy_sus = { 0x2000, 8, 0, 0, 0x1d1 },
> > + .svbus_en = { 0x2000, 15, 15, 0, 1 },
> > + .svbus_sel = { 0x2000, 14, 14, 0, 1 },
> > .bvalid_det_en = { 0x20c0, 1, 1, 0, 1 },
> > .bvalid_det_st = { 0x20c4, 1, 1, 0, 1 },
> > .bvalid_det_clr = { 0x20c8, 1, 1, 0, 1 },
> >
> >
>
>
>
>
>
More information about the linux-arm-kernel
mailing list