[PATCH v5 1/2] phy: rockchip: inno-usb2: add soft vbusvalid control
Nicolas Frattaroli
nicolas.frattaroli at collabora.com
Fri Jun 20 07:35:49 PDT 2025
Hi Heiko, a quick follow-up,
On Thursday, 19 June 2025 22:23:35 Central European Summer Time Nicolas Frattaroli wrote:
> 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.
So I tested this on RK3588, and could not reproduce the issue. Then, out of
curiosity, I reverted the patch and tested on the Sige5 again. I could also
not reproduce the issue anymore (?!). Even with the udphy line commented out
that sets those GRF regs as well, I can't get it to have issues enumerating
things on Type-C anymore.
The downstream commit this was based on is here:
https://github.com/rockchip-linux/kernel/commit/7d2237b0adc2e0a0105d63b645528993b44c8c36
So for now, this patch can be considered "abandoned, maybe unnecessary"
until the problem rears its head again for someone. I really don't get
why it works now :(
>
> >
> > 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