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
>
