On 29/04/2020 15:03, Heikki Krogerus wrote:
On Wed, Apr 22, 2020 at 03:50:08PM +0100, Bryan O'Donoghue wrote:
What's your feeling on the following.
In DT if we find a child connector then we can determine the state we are
supposed to be in ?
That way it _shouldn't_ change the logic you already depend on the ACPI
systems.
typec1_con: connector {
compatible = "usb-c-connector";
label = "USB-C";
power-role = "dual";
data-role = "dual";
try-power-role = "sink";
source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
PDO_VAR(5000, 20000, 3000)>;
op-sink-microwatt = <15000000>;
self-powered;
};
we need to control :
http://www.ti.com/lit/an/slva843a/slva843a.pdf
- InitiateSwapToDPF
- InitiateSwapToUFP
- process-swap-to-dfp;
- process-swap-to-ufp;
- InitiateVconnSwap;
- ProcessVconnSwap;
The documentation makes clear some of these settings are mutually exclusive.
You can capture the logic of that with the connector
- power-role = "dual";
- data-role = "dual";
from the connector declaration. Absent the connector declaration the ACPI
launched code should still work as-is.
Alternatively it could be something specific to the chip - as opposed to the
connector.
My thought is you either have these two at the type-c controller level or
inside a child connector node.
Either way tps6598x would consume them.
You should always have a child node for every connector. Please Note
that we usually have one for each connector in ACPI as well.
For now on every platform the application code (the PD controler FW)
has been platform specific, which means we have not needed to
configure things like the System Configuration register, because we've
known that the application code has already done that for us.
In your case I'm assuming you do not have a platform specific PD
controler FW, so you are going to need to use device properties, which
is the correct thing to do.
On our platform I found the settings "just worked" asking around it
turns out our FW has been configured with the TI configuration tool, so
at this moment in time I don't think the stuff I'm working on has a
specific need to configure these options.
So we can always check those properties in tps6598x.c. If we have
them, we configure the System Configuration, and what have you,
according to those. If we don't have them, then we know the PD
controller FW is platform specific.
Something else ? It's important we get the changes upstream, so I'd
appreciate any thoughts you have on the right way to go about this.
So what exactly is the problem here?
Which USB controller are you using? Is it dual-role capable, or do you
have separate xHCI controller and separate USB device controller plus
a mux between them?
Err, checks notes.
Its a ChipIdea HDRC. That part just works.
As I suggested above,
tps6598x: tps6598x@38 {
compatible = "ti,tps6598x";
reg = <0x38>;
interrupt-parent = <&gpio>;
interrupts = <107 IRQ_TYPE_LEVEL_LOW>;
interrupt-names = "irq";
pinctrl-names = "default";
pinctrl-0 = <&typec_pins>;
port {
typec1_dr_sw: endpoint {
remote-endpoint = <&usb1_drd_sw>;
};
};
That looks a bit odd to me. I think you want to place that under the
connector node too, however, OF graph is a bit problematic with
Type-C.
I discovered that as I came to implement, also looking at the other
typec controllers like the fusb302 and hd3ss3220 it became obvious a
connector {} should be included.
tps6598x: tps6598x@38 {
compatible = "ti,tps6598x";
reg = <0x38>;
interrupt-parent = <&msmgpio>;
interrupts = <107 IRQ_TYPE_LEVEL_LOW>;
interrupt-names = "irq";
reset-gpios = <&msmgpio 116 GPIO_ACTIVE_LOW>;
pinctrl-names = "default";
pinctrl-0 = <&typec_pins>;
typec_con: connector {
compatible = "usb-c-connector";
label = "USB-C";
port {
typec_ep: endpoint {
remote-endpoint = <&otg_ep>;
};
};
};
};
&otg {
status = "okay";
usb-role-switch;
ulpi {
usb_hs_phy: phy {
v1p8-supply = <&pm8916_l7>;
v3p3-supply = <&pm8916_l13>;
};
};
port {
otg_ep: endpoint {
remote-endpoint = <&typec_ep>;
};
};
};
with no more change than adding OF binding and the role-switch stuff we
discussed.
Not quite ready to send out yet.
The problem is that there is no way to identify the OF graph
ports/endpoints. It means that there does not seem to be any way to
know which port/endpoint/remote-endpoint is for the DisplayPort, the
mux, USB port, TBT 3 port, retimer #1, retimer #2, etc.
There is a proposal to define device property that simply contains
reference to the correct node for every type of connection. For USB
role switch the property is named "usb-role-switch":
connector {
...
usb-role-switch = <the remote port parent of &usb1_drd_sw>;
...
};
I'll take a look.
Thanks for the feedback.
---
bod
