Re: [PATCH v1 1/1] arm64: dts: rockchip: Add USB-C support to ROCK 5B

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

 



Hello Naoki,

On Wed, Dec 11, 2024 at 07:10:55AM +0900, FUKAUMI Naoki wrote:
> Hi Sebastian,
> 
> Thank you very much for your work!
> 
> $ cat /sys/class/power_supply/tcpm-source-psy-4-0022/{current_max,current_now,online,type,usb_type,voltage_max,voltage_min,voltage_now}
> 1500000
> 1500000
> 1
> USB
> C [PD] PD_PPS
> 20000000
> 20000000
> 20000000
> 
> $ cat /sys/class/power_supply/tcpm-source-psy-4-0022/{current_max,current_now,online,type,usb_type,voltage_max,voltage_min,voltage_now}
> 5000000
> 5000000
> 1
> USB
> C PD [PD_PPS]
> 20000000
> 20000000
> 20000000
> 
> $ ls /sys/class/udc/
> fc000000.usb
> 
> I can configure it as CDC-NCM and host detects it.
> But I could not use it as a HOST port. How to use it?

You can switch between host and peripheral for Type-C ports like
this depending on the remote sides capabilities:

 * echo host > /sys/class/typec/<port>/data_role
 * echo device > /sys/class/typec/<port>/data_role

I tested this with a USB-C hub connected to the port, which works
in host mode.

> some minor nitpick is below:
> 
> On 12/11/24 01:36, Sebastian Reichel wrote:
> > Add hardware description for the USB-C port in the Radxa Rock 5 Model B.
> > This describes the OHCI, EHCI and XHCI USB parts, but not yet the
> > DisplayPort AltMode (bindings are not yet upstream).
> > 
> > The fusb302 node is marked with status "fail", since the board is usually
> > powered through the USB-C port. Handling of errors can result in hard
> > resets, which removed the bus power for some time resulting in a board
> > reset.
> > 
> > The main problem is that devices are supposed to interact with the
> > power-supply within 5 seconds after the plug event according to the
> > USB PD specification. This is more or less impossible to achieve when
> > the kernel is the first software communicating with the power-supply.
> > 
> > Recent U-Boot (v2025.01) will start doing USB-PD communication, which
> > solves this issue. Upstream U-Boot doing USB-PD communication will also
> > set the fusb302 node status to "okay". That way booting a kernel with
> > the updated DT on an old U-Boot avoids a reset loop.
> > 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
> > ---
> >   .../boot/dts/rockchip/rk3588-rock-5b.dts      | 121 ++++++++++++++++++
> >   1 file changed, 121 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> > index d597112f1d5b..cb5990df6ccb 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> > @@ -5,6 +5,7 @@
> >   #include <dt-bindings/gpio/gpio.h>
> >   #include <dt-bindings/leds/common.h>
> >   #include <dt-bindings/soc/rockchip,vop2.h>
> > +#include <dt-bindings/usb/pd.h>
> >   #include "rk3588.dtsi"
> >   / {
> > @@ -84,6 +85,15 @@ rfkill-bt {
> >   		shutdown-gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
> >   	};
> > +	vcc12v_dcin: regulator-vcc12v-dcin {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "vcc12v_dcin";
> 
> typec_vin by schematic.

Ack. Will update in v2.

> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <12000000>;
> > +		regulator-max-microvolt = <12000000>;
> > +	};
> > +
> >   	vcc3v3_pcie2x1l0: regulator-vcc3v3-pcie2x1l0 {
> >   		compatible = "regulator-fixed";
> >   		enable-active-high;
> > @@ -142,6 +152,7 @@ vcc5v0_sys: regulator-vcc5v0-sys {
> >   		regulator-boot-on;
> >   		regulator-min-microvolt = <5000000>;
> >   		regulator-max-microvolt = <5000000>;
> > +		vin-supply = <&vcc12v_dcin>;
> 
> typec_vin.
> 
> >   	};
> >   	vcc_1v1_nldo_s3: regulator-vcc-1v1-nldo-s3 {
> > @@ -264,6 +275,67 @@ regulator-state-mem {
> >   	};
> >   };
> > +&i2c4 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&i2c4m1_xfer>;
> > +	status = "okay";
> > +
> > +	usbc0: usb-typec@22 {
> 
> Is "usbc0" label necessary?

no, but does it hurt?

> > +		compatible = "fcs,fusb302";
> > +		reg = <0x22>;
> > +		interrupt-parent = <&gpio3>;
> > +		interrupts = <RK_PB4 IRQ_TYPE_LEVEL_LOW>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&usbc0_int>;
> 
> cc_int_l by schematic.

Ack. I intentionally switched away from this naming, since cc prefix
is imho a way worse prefix than usbc0. The _l suffix just means
active low, which is already encoded in DT.

But I don't have a strong opinion and can fix this in v2.

> > +		vbus-supply = <&vcc12v_dcin>;
> 
> typec_vin
> 
> > +		/*
> > +		 * When the board is starting to send power-delivery messages
> > +		 * too late (5 seconds according to the specification), the
> > +		 * power-supply reacts with a hard-reset. That removes the
> > +		 * power from VBUS for some time, which resets te whole board.
> 
> ... resets "the" whole board.

Ack.

> 
> > +		 */
> > +		status = "fail";
> > +
> > +		usb_con: connector {
> 
> Is "usb_con" label necessary?

No. It should either be changed to "usbc0_con" or removed. In
general I tend to add some labels when they might be needed by
something in the future. They are more or less free anyways.

-- Sebastian

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux