Re: ucsi and DRD controller interaction

["Shah, Nehal-bakulchandra" <nehal-bakulchandra.shah@amd.com>]

Hi

On 4/22/2020 7:58 PM, Heikki Krogerus wrote:
> On Wed, Apr 22, 2020 at 06:37:10PM +0530, Shah, Nehal-bakulchandra wrote:
>> Hi
>>
>> On 4/22/2020 4:30 PM, Heikki Krogerus wrote:
>>> On Tue, Apr 21, 2020 at 07:23:30PM +0530, Shah, Nehal-bakulchandra wrote:
>>>> Hi
>>>>
>>>> On 4/21/2020 1:13 PM, Heikki Krogerus wrote:
>>>>> Adding linux-usb mailing list.
>>>>>
>>>>> On Mon, Apr 20, 2020 at 07:09:17PM +0530, Shah, Nehal-bakulchandra wrote:
>>>>>> Hi Heikki ,
>>>>>>
>>>>>> I need some pointer from you, so in a system where I have UCSI driver for DRD
>>>>>> Controller, then how call stack will be?
>>>>>>
>>>>>> I am unable to comprehend the flow from UCSI infrastructure to DRD controller
>>>>>> code base.
>>>>> Do you need to tell the negotiated data role to your USB controller
>>>>> driver from the UCSI driver? For that we would need to know which USB
>>>>> controller, or more precisely, which USB port is connected to the
>>>>> USB Type-C connector in question.
>>>>>
>>>>> That would require ACPI tables to be able to describe the connection
>>>>> between the USB Type-C connector and the USB port (and the connection
>>>>> between DisplayPort and the USB Type-C connector etc.). Unfortunately
>>>>> there is currently no documented way of doing that, however, there is
>>>>> a proposal for a document the defines how the USB Type-C connectors
>>>>> should be described in ACPI in general. The proposal does consider
>>>>> this problem as well. For this the solution is to use _DSD device
>>>>> properties "usb2-port" and "usb3-port" that have references to the
>>>>> correct USB port nodes as values.
>>>>>
>>>>> Which USB controller are you using btw?
>>>>>
>>>>> thanks,
>>>> Thanks for the mail.
>>>>
>>>> Here is the configuration
>>>>
>>>> 1. DRD Controller (DWC3 controller)
>>>>
>>>> 2. TI PD Controller
>>>>
>>>> 3. TI PD Controller and Host has I2C as transport layer. So ACPI mechanism wont work here.
>>>>
>>>>
>>>> Hence i was wondering, is there any way from UCSI Driver we inform to DWC3 driver stack about Role change. I can understand one point that,
>>>>
>>>> DWC3 controller can work without UCSI Implementation i.e Only PD firmware. But i want to understand  if there is a role change, PD interrupt will be generated
>>>>
>>>> and UCSI Driver will come to know about this role change. But from this onwards , i am  unable to comprehend how it can be propagated upto DWC3 stack.
>>> If the Type-C drivers need to tell DWC3 driver the data role (USB
>>> role) the connector is operating in, then you use the USB Role Switch
>>> Class for that (drivers/usb/role/). The USB Type-C driver (so ucsi.c
>>> or tps6598x.c in this case) that knows the USB role tells it to the
>>> USB role class by calling usb_role_switch_set_role().
>>>
>>> The USB role switch class then takes care of forwarding the
>>> information to the actual switch, which is DWC3 in this case. The DWC3
>>> driver already registers the USB role switch for you
>>> (drivers/usb/dwc3/drd.c), but the UCSI driver, and also tps6598x.c,
>>> does not use the USB role switch API yet. There has never been need
>>> for that before this.
>>>
>>> Adding USB role switch handling to the UCSI and tps6598x drivers can
>>> easily be fixed, but it's still not enough. You still need to describe
>>> the connection between the USB PD controller and DWC3 somewhere.
>>>
>>> thanks,
>> appreciate your input.  So if understand correctly, something like below  patch has to be done
>>
>> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Flkml.iu.edu%2Fhypermail%2Flinux%2Fkernel%2F2003.1%2F04687.html&data=02%7C01%7Cnehal-bakulchandra.shah%40amd.com%7Cafa9696669a74336186a08d7e6c96354%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637231624939620306&sdata=nwwQbBBlAduAOhIfvfYv9UzR%2B478yN%2Fldb9%2FLK%2F6eiE%3D&reserved=0
>>
>> So this patch is still under review?
> The patch is in mainline:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcommit%2F%3Fid%3D8a0a137997448690a642e2325fe73242788b26fe&data=02%7C01%7Cnehal-bakulchandra.shah%40amd.com%7Cafa9696669a74336186a08d7e6c96354%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637231624939620306&sdata=%2BIf%2Bz0Cm7CG32POOf3tdue%2FSamAqWPGjaZQYwBuRFwE%3D&reserved=0
>
>> right, and this will take for "Registering a role switch in the DRD code". So then from ucsi.c we need to
>>
>> call the role class. Also, the above patch is based on device tree binding (i.e for arm platform)but in my case i have x86 so do i have to expose this via ACPI?
> That patch does not actually have anything devicetree specific, but
> you do need to supply the description of the connection like I said,
> and ideally you can do that in your ACPI tables.
>
> Can you still influence what goes to your ACPI tables, or are we
> talking about products that are already on the market?
>
>
> thanks,

Yes still it is in development phase so i can work out with BIOS team for the same. When you say description of connection ,something like this should be part of _DSD method right? (i.e from below dts
to _DSD of ACPI)

 usb_con: connector {
compatible = "usb-c-connector";
 label = "USB-C";
 data-role = "dual";
 power-role = "dual";
 try-power-role = "sink";
 source-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)>;
 sink-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)
 PDO_VAR(5000, 5000, 1000)>;
 op-sink-microwatt = <10000000>;
 };

regards

Nehal Shah