| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5mjjq3f3tqu4h2f7wbdcfon6csfsuxge5skr6mmgozvq65uods@66yx7yuefzqn>
Date: Wed, 11 Dec 2024 00:08:40 +0100
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: FUKAUMI Naoki <naoki@...xa.com>
Cc: Heiko Stuebner <heiko@...ech.de>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
linux-rockchip@...ts.infradead.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, kernel@...labora.com
Subject: Re: [PATCH v1 1/1] arm64: dts: rockchip: Add USB-C support to ROCK 5B
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@...labora.com>
> > ---
> > .../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
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists