| 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: <1j4jj4xik6.fsf@starbuckisacylon.baylibre.com>
Date: Fri, 06 Oct 2023 10:21:37 +0200
From: Jerome Brunet <jbrunet@...libre.com>
To: Neil Armstrong <neil.armstrong@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>
Cc: Kevin Hilman <khilman@...libre.com>, Da Xue <da.xue@...retech.co>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-amlogic@...ts.infradead.org
Subject: Re: [PATCH 2/2] arm64: dts: amlogic: add libretech cottonwood support
On Thu 05 Oct 2023 at 12:04, Neil Armstrong <neil.armstrong@...aro.org> wrote:
> On 05/10/2023 11:42, Jerome Brunet wrote:
>> On Tue 03 Oct 2023 at 09:35, Neil Armstrong <neil.armstrong@...aro.org>
>> wrote:
>>
>>> On 02/10/2023 20:57, Jerome Brunet wrote:
>>>> On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@...aro.org>
>>>> wrote:
>>>>
>>>
>>> <snip>
>>>
>>>>>> +&usb3_pcie_phy {
>>>>>> + #address-cells = <1>;
>>>>>> + #size-cells = <0>;
>>>>>> + phy-supply = <&vcc_5v>;
>>>>>> +
>>>>>> + hub: hub@1 {
>>>>>> + compatible = "usb5e3,626";
>>>>>> + reg = <1>;
>>>>>> + reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>>>>>> + };
>>>>>
>>>>> Not sure the PHY is the right place to put the USB HUB,
>>>>> and it's probable the HUB is connected to both the USB2 and USB3 lines
>>>> It is connected to the USB3.0 only
>>>>
>>>>> so you should have both USB IDs in DT like it'd done for the Odroid-C4:
>>>>>
>>>>> / {
>>>>> ...
>>>>> /* USB hub supports both USB 2.0 and USB 3.0 root hub */
>>>>> usb-hub {
>>>>> dr_mode = "host";
>>>>> #address-cells = <1>;
>>>>> #size-cells = <0>;
>>>>>
>>>>> /* 2.0 hub on port 1 */
>>>>> hub_2_0: hub@1 {
>>>>> compatible = "usb2109,2817";
>>>>> reg = <1>;
>>>>> peer-hub = <&hub_3_0>;
>>>>> reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>>> vdd-supply = <&vcc_5v>;
>>>>> };
>>>>>
>>>>> /* 3.1 hub on port 4 */
>>>>> hub_3_0: hub@2 {
>>>>> compatible = "usb2109,817";
>>>>> reg = <2>;
>>>>> peer-hub = <&hub_2_0>;
>>>>> reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>>> vdd-supply = <&vcc_5v>;
>>>>> };
>>>>> };
>>>>> ...
>>>>> };
>>>>>
>>>>> if it only has a single USB ID, then it should go under the dwc3 node.
>>>> The usb controller is connected to the PHY and what's coming out of the
>>>> PHY
>>>> goes to the hub. It seems logical to hub the hub under it.
>>>> Why bypass the PHY ?
>>>
>>> The USB bindings the USB devices nodes should be under the controller's node,
>>> not the PHY, see:
>>>
>>> Documentation/devicetree/bindings/usb/usb-hcd.yaml
>>> ...
>>> patternProperties:
>>> "^.*@[0-9a-f]{1,2}$":
>>> description: The hard wired USB devices
>>> type: object
>>> $ref: /schemas/usb/usb-device.yaml
>>> ...
>>> and the example.
>>>
>>> Subnodes aren't allowed in the PHY node.
>> Ok, that is what schema says.
>> HW wise there is possible problem though.
>> The phy node has the power supply to the bus.
>> In that case it is a controllable one.
>> If fixed USB devices go under the controller instead of the PHY, isn't
>> it possible that the kernel may attempt to probe them before the bus is
>> powered ? For this particular board, it would make the reset we are
>> trying to apply useless.
>
> The usb core has a special handling for those usb hubs doing the power
> up at the right time during the USB setup, including the PHY powering up.
> So the power sequence should be fine.
>
> This has been done on Odroid-C2 and Odroid-N2 already.
Tried it. Unfortunately something is off with the hub under the dwc3 node
I often get this error (like once in 3 boots):
[ 0.419301] usbcore: registered new interface driver usbfs
[ 0.424434] usbcore: registered new interface driver hub
[ 0.429696] usbcore: registered new device driver usb
[ 0.921460] usbcore: registered new interface driver usb-storage
[ 0.968157] usbcore: registered new interface driver usbhid
[ 0.972114] usbhid: USB HID core driver
[ 1.132529] dwc3-meson-g12a ffe09000.usb: USB2 ports: 2
[ 1.134897] dwc3-meson-g12a ffe09000.usb: USB3 ports: 1
[ 1.144451] dwc2 ff400000.usb: supply vusb_d not found, using dummy regulator
[ 1.147231] dwc2 ff400000.usb: supply vusb_a not found, using dummy regulator
[ 1.154464] dwc2 ff400000.usb: EPs: 7, dedicated fifos, 712 entries in SPRAM
[ 1.219515] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
[ 1.469260] usb 1-1: new high-speed USB device number 2 using xhci-hcd
[ 1.745395] usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
[ 9.794777] usbcore: registered new device driver onboard-usb-hub
[ 10.255484] onboard-usb-hub 1-1: Failed to suspend device, error -32
[ 10.261699] onboard-usb-hub 1-1: can't set config #1, error -71
[ 10.287500] onboard-usb-hub 1-1: Failed to suspend device, error -32
[ 10.287844] onboard-usb-hub 1-1: USB disconnect, device number 2
[ 10.573277] usb 1-1: new high-speed USB device number 3 using xhci-hcd
[ 10.921468] usb 2-1: reset SuperSpeed USB device number 2 using xhci-hcd
[ 11.193453] usb 2-1: reset SuperSpeed USB device number 2 using xhci-hcd
While it works reliably when the onboard-usb-hub is under the phy node.
I added the 5v supply as vdd under the hub for good measure.
>
> Neil
>
>>
>>>
>>> Neil
>>>
>>>>
>>>>>
>>>>>> +};
>>>>>> +
>>>>>> +&usb {
>>>>>> + status = "okay";
>>>>>> +};
>>>
>>> <snip>
>>
Powered by blists - more mailing lists