linux-kernel - Re: [PATCH] phy: tegra: xusb: Default otg mode to peripheral

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALHNRZ8uXZQObwQBC-sLudUdtprM24qU5yYdb4D3FEP2AQVkmQ@mail.gmail.com>
Date: Tue, 6 May 2025 05:03:47 -0500
From: Aaron Kling <webgeek1234@...il.com>
To: Jon Hunter <jonathanh@...dia.com>
Cc: JC Kuo <jckuo@...dia.com>, Vinod Koul <vkoul@...nel.org>, 
	Kishon Vijay Abraham I <kishon@...nel.org>, Thierry Reding <thierry.reding@...il.com>, 
	linux-phy@...ts.infradead.org, linux-tegra@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] phy: tegra: xusb: Default otg mode to peripheral

On Tue, May 6, 2025 at 4:51 AM Jon Hunter <jonathanh@...dia.com> wrote:
>
>
> On 05/05/2025 08:44, Aaron Kling wrote:
> > On Sun, Apr 20, 2025 at 8:44 PM Aaron Kling <webgeek1234@...il.com> wrote:
> >>
> >> On Sun, Apr 13, 2025 at 11:45 PM Aaron Kling <webgeek1234@...il.com> wrote:
> >>>
> >>> On Fri, Apr 4, 2025 at 3:18 AM Aaron Kling via B4 Relay
> >>> <devnull+webgeek1234.gmail.com@...nel.org> wrote:
> >>>>
> >>>> From: Aaron Kling <webgeek1234@...il.com>
> >>>>
> >>>> Currently, if usb-role-switch is set and role-switch-default-mode is
> >>>> not, a xusb port will be inoperable until that port is hotplugged,
> >>>> because the driver defaults to role none. Instead of requiring all
> >>>> devices to set the default mode, assume that the port is primarily
> >>>> intended for use in device mode. This assumption already has precedence
> >>>> in the synopsys dwc3 driver.
> >>>>
> >>>> Signed-off-by: Aaron Kling <webgeek1234@...il.com>
> >>>> ---
> >>>>   drivers/phy/tegra/xusb.c | 8 +++-----
> >>>>   1 file changed, 3 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> >>>> index 79d4814d758d5e1f0e8200d61e131606adbb0e2d..c56e83216d0f566a09b67377172fb04c8406f4cf 100644
> >>>> --- a/drivers/phy/tegra/xusb.c
> >>>> +++ b/drivers/phy/tegra/xusb.c
> >>>> @@ -731,13 +731,11 @@ static void tegra_xusb_parse_usb_role_default_mode(struct tegra_xusb_port *port)
> >>>>
> >>>>          if (mode == USB_DR_MODE_HOST)
> >>>>                  role = USB_ROLE_HOST;
> >>>> -       else if (mode == USB_DR_MODE_PERIPHERAL)
> >>>> +       else
> >>>>                  role = USB_ROLE_DEVICE;
> >>>>
> >>>> -       if (role != USB_ROLE_NONE) {
> >>>> -               usb_role_switch_set_role(port->usb_role_sw, role);
> >>>> -               dev_dbg(&port->dev, "usb role default mode is %s", modes[mode]);
> >>>> -       }
> >>>> +       usb_role_switch_set_role(port->usb_role_sw, role);
> >>>> +       dev_dbg(&port->dev, "usb role default mode is %s", modes[mode]);
> >>>>   }
> >>>>
> >>>>   static int tegra_xusb_usb2_port_parse_dt(struct tegra_xusb_usb2_port *usb2)
> >>>>
> >>>> ---
> >>>> base-commit: 91e5bfe317d8f8471fbaa3e70cf66cae1314a516
> >>>> change-id: 20250404-xusb-peripheral-c45b1637f33b
> >>>>
> >>>> Best regards,
> >>>> --
> >>>> Aaron Kling <webgeek1234@...il.com>
> >>>>
> >>>>
> >>>
> >>> Friendly reminder about this patch.
> >>>
> >>> Sincerely,
> >>> Aaron
> >>
> >> Friendly re-reminder about this series.
> >>
> >> Sincerely,
> >> Aaron Kling
> >
> > It has been over a month since this patch was submitted. And neither
> > this nor any other patch I've submitted since have been reviewed or
> > responded to by any Tegra subsystem maintainer. Is there anyone else
> > that can review these and pick them up? Or is there any other path
> > forward for series that get ignored by the subsystem maintainers?
>
>
> Sorry for the delay. I have had a look at this patch and I am not sure
> about this. The function you are changing is called
> 'tegra_xusb_parse_usb_role_default_mode' and it is doing precisely what
> it was intended to do. In other words, parse device-tree and set the
> mode accordingly. So forcing the mode in this function does not feel
> correct.
>
> Also from the description it is not 100% clear to me the exact scenario
> where this is really a problem.

My specific use case is booting AOSP/Android on Tegra devices using
mainline support. Android debug bridge is configured to use xudc on
the otg ports. As mainline is currently set up, the default usb role
is 'none'. So if I boot a unit with a usb cable already plugged into
the debug port, I cannot access adb.

I originally fixed this by setting role-switch-default-mode in the
device tree for every device I'm targeting. Then I looked at just
defaulting to peripheral mode in code. And as mentioned in the commit
message, other usb drivers already default to peripheral mode instead
of none. I'm open to other solutions, but requiring every device tree
to set a default role doesn't seem like a good solution either.

Sincerely,
Aaron Kling