Re: [PATCH 3/8] dt-bindings: phy: tegra-xusb-padctl: Add nvidia,usb3-port-fake entry

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



There won't be a case that all of the super-speed ports are in used but none of the ports can be paired with the OTG capable USB2 port. T210/T210B01(aka T214) has 4 super-speed ports and 4 USB2 ports. A USB 3.0 connection (either host role or device role) must have one super-speed port pair with one USB2 port. If a platform has an USB 2.0 OTG connector, there must be at least one super-speed port unused.

To make XUSB device mode working with the USB 2.0 OTG connector, driver just have to: 1. pair the designated unused super-speed port with the USB2 port that has OTG capability.
2. remove CLAMP_EN_EARLY/CLAMP_EN/VCORE_DOWN signals.

Please note the corresponding UPHY lane of the designated unused super-speed port can still be used by another IP (PCIE or SATA).

Agree that it'd be better to keep the complexity within driver, rather than confusing device tree authors. Thanks Thierry.

Thanks,
JC


On 2/14/19 11:01 PM, Nagarjuna Kristam wrote:

On 14-02-2019 14:29, Thierry Reding wrote:
On Thu, Feb 14, 2019 at 10:58:13AM +0530, Nagarjuna Kristam wrote:

On 13-02-2019 18:46, Thierry Reding wrote:
On Wed, Feb 13, 2019 at 01:56:24PM +0100, Thierry Reding wrote:
On Wed, Feb 13, 2019 at 04:08:15PM +0530, Nagarjuna Kristam wrote:

On 04-02-2019 17:18, Thierry Reding wrote:
On Thu, Jan 03, 2019 at 03:34:54PM +0530, Nagarjuna Kristam wrote:
Add binding details regarding nvidia,usb3-port-fake

Signed-off-by: Nagarjuna Kristam <nkristam@xxxxxxxxxx>
---
  Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
index 3742c15..21a5541 100644
--- a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
+++ b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
@@ -158,6 +158,9 @@ Optional properties:
    is internal. In the absence of this property the port is considered to be
    external.
  - vbus-supply: phandle to a regulator supplying the VBUS voltage.
+- nvidia,usb3-port-fake: A integer property whose presense informs that a
+  fake USB3 port needs to be mapped for corresponding USB2 port. This entry
+  is applicable only for Tegra210 based platforms which has USB2 only ports.
You don't provide a rationale fo why this fake USB3 port needs to be
specified. Doesn't the OTG port work without the fake USB3 port? Why
does it need to be specified and which one should be used as fake?

Yes, on Tegra210 based platforms, USB2 only device/OTG ports don't work

You mention elsewhere that an unused USB3 port is used on Jetson TX1,
but what if there are no unused ports on a platform? Can we use any
USB3 port for this purpose?

If we dont have any unused port, we cannot get device mode working on USB2 only port.
We cannot use any USB3 port, we need to use an un-used USB3 port, so that we can
fake that corresponding USB2 only port as USB2 and USB3 port.
Hm... seems to me like we should be able to do this without an extra
device tree property. Surely we can determine at runtime which of the
ports are actually unused and pick one? If it doesn't matter which one,
we can just pick the first one that's unused, or an unused one at
random.
It's slightly more complicated than I thought because there's an
optimization in tegra_xusb_add_usb3_port() that will cause only used
ports to get registered. But I think we can do something like this
(untested) to get hold of an unused one, since we only need the index
of the port:

	static int tegra_xusb_find_unused_usb3_port(struct tegra_xusb_padctl *padctl)
	{
		struct device_node *np;
		unsigned int i;

		for (i = 0; i < padctl->soc->ports.usb3.count; i++) {
			np = tegra_xusb_find_port_node(padctl, "usb3", i);
			if (!np || !of_device_is_available(np))
				return index;
		}

		return -ENODEV;
	}

Thierry

It not just about un-used USB3 port, we need to find which usb2 port
is registered as otg/device and then check if it doesnot have any
companion usb3 port. On this condition we need to assign unused usb3
port as fake port.
Yeah, we should only be running this code for the OTG port in the first
place. Once we know which one the OTG port is it's easy to check whether
it has a USB3 companion port.

Above is possible, however port_fake need is must for T210 and only
for device mode operation and not for other SOC or USB mode.
So shall i place SOC data(tegra210_xusb_padctl_soc) to read if port
fake is needed ?
Yeah, something like "needs_usb3_for_otg" should do the trick.

So to confirm whether I understand this correctly: if we want to do USB2
and USB3 device mode, then we'd usually have a USB2/USB3 port combo, in
which case we wouldn't need a fake USB3 port. But if we have a USB2 only
OTG port, then we need to program registers for the USB3 port in order
to make USB2 only OTG work, but only on Tegra210. With Tegra186 and
later we don't need this work around?

If yes, and no un-used port available, will mark case as warning(not
error) and make probe(tegra_xusb_padctl_probe) success irrespective of
this. Since port can be otg but can be designed to use only for Host
mode.
Yes, sounds good. Or perhaps we can use the mode property to determine
whether or not this should be a fatal error? For example if we've got an
OTG port but it is designed to be used only in host mode, then we could
require the mode to be set to "host". If it is set to "peripheral" or
"otg" and we don't find an un-used USB3 port to use as fake, it could
still be a fatal error so that people can fix their device tree. Right?

I am not sure if above complication is better than an dt entry.
I understand that this may seem like a lot of complication, but there
are two reasons why I think it's worth it. On one hand we don't want to
add redundant information to device tree. Everything that can already be
derived from the compatible string doesn't need to be explicitly added
in device tree via extra properties. In this case we know that Tegra210
has this bug and we can identify the need for the workaround by looking
at the compatible string. Similarly we know exactly what to do in order
to work around the problem (i.e. find an unused port and use it as the
fake companion port).

On the other hand I think that this moves the complexity where it
belongs: in the driver. If we document this behaviour in the device
tree bindings, we need to be very explicit about what this means and
very careful about how to word it so as not to confuse readers. It's
also not entirely trivial for device tree authors to come up with the
fake port index. They have to carefully check the design to see which
one is unused. The driver can do a much better job at that, and then
we don't have to burden the device tree authors with that task.

Thanks for details, considering the need for tegra 210 only, will put
logic to update port_fake as tegra210 specific code handling during port
enable API

- Nagarjuna
Thierry




[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux