drivers/phy/tegra/xusb.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
From: Aaron Kling <webgeek1234@gmail.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@gmail.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@gmail.com>On 04/04/2025 09:17, Aaron Kling via B4 Relay wrote: > From: Aaron Kling <webgeek1234@gmail.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@gmail.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; It would be simpler to initialise 'role' as USB_ROLE_DEVICE at the start of the function instead of USB_ROLE_NONE. A comment would also be good to why this is the default. Jon -- nvpublic
On Fri, Apr 4, 2025 at 3:18 AM Aaron Kling via B4 Relay
<devnull+webgeek1234.gmail.com@kernel.org> wrote:
>
> From: Aaron Kling <webgeek1234@gmail.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@gmail.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@gmail.com>
>
>
Friendly reminder about this patch.
Sincerely,
Aaron
On Sun, Apr 13, 2025 at 11:45 PM Aaron Kling <webgeek1234@gmail.com> wrote:
>
> On Fri, Apr 4, 2025 at 3:18 AM Aaron Kling via B4 Relay
> <devnull+webgeek1234.gmail.com@kernel.org> wrote:
> >
> > From: Aaron Kling <webgeek1234@gmail.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@gmail.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@gmail.com>
> >
> >
>
> Friendly reminder about this patch.
>
> Sincerely,
> Aaron
Friendly re-reminder about this series.
Sincerely,
Aaron Kling
On Sun, Apr 20, 2025 at 8:44 PM Aaron Kling <webgeek1234@gmail.com> wrote:
>
> On Sun, Apr 13, 2025 at 11:45 PM Aaron Kling <webgeek1234@gmail.com> wrote:
> >
> > On Fri, Apr 4, 2025 at 3:18 AM Aaron Kling via B4 Relay
> > <devnull+webgeek1234.gmail.com@kernel.org> wrote:
> > >
> > > From: Aaron Kling <webgeek1234@gmail.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@gmail.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@gmail.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?
Sincerely,
Aaron Kling
On 05/05/2025 08:44, Aaron Kling wrote:
> On Sun, Apr 20, 2025 at 8:44 PM Aaron Kling <webgeek1234@gmail.com> wrote:
>>
>> On Sun, Apr 13, 2025 at 11:45 PM Aaron Kling <webgeek1234@gmail.com> wrote:
>>>
>>> On Fri, Apr 4, 2025 at 3:18 AM Aaron Kling via B4 Relay
>>> <devnull+webgeek1234.gmail.com@kernel.org> wrote:
>>>>
>>>> From: Aaron Kling <webgeek1234@gmail.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@gmail.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@gmail.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.
Jon
--
nvpublic
On Tue, May 6, 2025 at 4:51 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 05/05/2025 08:44, Aaron Kling wrote:
> > On Sun, Apr 20, 2025 at 8:44 PM Aaron Kling <webgeek1234@gmail.com> wrote:
> >>
> >> On Sun, Apr 13, 2025 at 11:45 PM Aaron Kling <webgeek1234@gmail.com> wrote:
> >>>
> >>> On Fri, Apr 4, 2025 at 3:18 AM Aaron Kling via B4 Relay
> >>> <devnull+webgeek1234.gmail.com@kernel.org> wrote:
> >>>>
> >>>> From: Aaron Kling <webgeek1234@gmail.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@gmail.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@gmail.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
On 06/05/2025 11:03, Aaron Kling wrote:
...
>> 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.
Thanks for the background. I see that the
Documentation/devicetree/bindings/usb/usb-drd.yaml states that ...
role-switch-default-mode:
description:
Indicates if usb-role-switch is enabled, the device default operation
mode of controller while usb role is USB_ROLE_NONE.
$ref: /schemas/types.yaml#/definitions/string
enum: [host, peripheral]
default: peripheral
Rather than reference 'synopsys dwc3' which is not related to the Tegra,
it would be better to update the binding doc for Tegra XUSB padctl device
to list this property and define the default mode.
Jon
--
nvpublic
On Tue, May 6, 2025 at 5:30 AM Jon Hunter <jonathanh@nvidia.com> wrote: > > > On 06/05/2025 11:03, Aaron Kling wrote: > > ... > > >> 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. > > Thanks for the background. I see that the > Documentation/devicetree/bindings/usb/usb-drd.yaml states that ... > > role-switch-default-mode: > description: > Indicates if usb-role-switch is enabled, the device default operation > mode of controller while usb role is USB_ROLE_NONE. > $ref: /schemas/types.yaml#/definitions/string > enum: [host, peripheral] > default: peripheral > > Rather than reference 'synopsys dwc3' which is not related to the Tegra, > it would be better to update the binding doc for Tegra XUSB padctl device > to list this property and define the default mode. Let me make sure I understand you correctly. You're requesting to update the binding as above, listing peripheral as default when unset; fix my commit message; and leave my code change as-is? I'm unclear on if the code is okay as-is or not. Sincerely, Aaron Kling
On 06/05/2025 11:44, Aaron Kling wrote: ... >> Rather than reference 'synopsys dwc3' which is not related to the Tegra, >> it would be better to update the binding doc for Tegra XUSB padctl device >> to list this property and define the default mode. > > Let me make sure I understand you correctly. You're requesting to > update the binding as above, listing peripheral as default when unset; > fix my commit message; and leave my code change as-is? I'm unclear on > if the code is okay as-is or not. If the dt-binding doc states that the default is peripheral mode when the property is not present, then yes your patch should be OK as-is. Jon -- nvpublic
© 2016 - 2026 Red Hat, Inc.