Hi,
On Thu, Sep 02, 2021 at 06:15:55PM +0530, Shah, Nehal-bakulchandra wrote:
> Hi Heikki,
> On 8/26/2021 7:36 PM, Heikki Krogerus wrote:
> > Hi Alexander,
> >
> > On Wed, Aug 25, 2021 at 01:50:48PM +0000, Deucher, Alexander wrote:
> > > I'm not a USB expert, but I think the idea was to pop up a message asking the
> > > user what role they wanted when they plugged in USB cable? Then based on
> > > their input, the role could be changed.
> >
> > What exactly is the ACPI/EC interrupt in this case?
> >
> > Note, that simply selecting one role will only work if the partner
> > device happens to be in the opposite role at the same time (actually,
> > even that may not be enough). So for example by selecting host role
> > will only work if the partner happens to be in device role. If the
> > parter is also in host role, nothing happens, or both ends just fail
> > to enumerate each other.
> >
> > So you always have to negotiate the role with the partner one way or
> > the other. Now we need to understand how that negotiation is handled
> > (or is expected to be handled) on this platform.
> >
> > Which type of connector are we talking about here? Is it USB Type-C,
> > or is it something else?
> >
> > thanks,
> >
> Sorry for the delayed response due to few designed changes. Now we have more
> clarity for the customer platform with respect to usage of DWC3 controller
> driver. So it is type C controller which will be using ACPI based UCSI
> driver. As UCSI driver has already role switch support we may not need this
> patch. However we need your input to understand this,
>
> con->usb_role_sw = fwnode_usb_role_switch_get(cap->fwnode);
>
> For this to work, how should be ACPI entry to be defined. Do you have sample
> code, we had discussion on similar point in past but still need some clarity
> if we have sample ACPI ASL Code. I remember something on this line from
> previous discussion with following sample code.
>
> /*
> * I2C1 is the I2C host, and PDC1 is the USB PD Controller (I2C slave
> device).
> */
> Scope (\_SB.PCI0.I2C1.PDC1)
> {
> /* Each connector should have its own ACPI device entry (node). */
> Device (CON0)
> {
> Name (_ADR, 0)
>
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package() {
> Package () {"usb-role-switch", \_SB.PCI0.DWC3},
> }
> })
> }
> }
In your case, the dwc3 is also the USB host controller, no?
The ACPI guys tell me that in ACPI we should rely on the _PLD
(Physical Location of Device) when determining to which USB Type-C
connector any given USB port (or any other port - like DP) is
connected to. Basically, the connector ACPI device node and the USB
port ACPI device node share the same _PLD, and that's how we know they
are connected. I'm already using that to create a symlink "connector"
for every USB port here: drivers/usb/typec/port-mapper.c
I'm not actually sure how did the ACPI guys think this will work with
USB device controllers, but if your controller is also the USB host
controller, then you will have a separate device node for every
port the host is controlling, and each of those will have the _PLD.
Can you send acpidump so I can take a look what you actually have
under your dwc3 ACPI device node?
% acpidump -o my_acpi.dump
We most likely do need to update the fwnode_usb_role_switch_get() api
so that it also considers the _PLD, but your ACPI tables maybe
already OK (big maybe).
> So here is the another question , if we can not achieve this in BIOS , Can
> we register the software node with quirk in DWC3 controller something like
> this
>
> static const struct software_node amd_dwc3_node[] = {
> { "amd-dwc3-usb-sw", NULL, amd_dwc3_props },
> {},
> };
>
> if (dwc->use_sw_node_quirk) {
> ret = software_node_register_nodes(amd_dwc3_node);
> if (ret)
> return ret;
> dwc3_role_switch.fwnode = software_node_fwnode(&amd_dwc3_node[0]);
> } else {
> dwc3_role_switch.fwnode = dev_fwnode(dwc->dev);
> }
>
>
> And in UCSI driver again with quirk,
>
> swnode = software_node_find_by_name(NULL, "amd-dwc3-usb-sw");
>
> fwnode = software_node_fwnode(swnode);
>
> con->usb_role_sw = usb_role_switch_find_by_fwnode(fwnode);
>
>
> Please provide your input that will help us .
Let's first check if we can we use _PLD for this.
thanks,
--
heikki
