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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <041b79cb-ca73-46b6-9477-0e735cc5f951@linaro.org>
Date: Mon, 15 Sep 2025 09:51:57 +0200
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Xilin Wu <sophon@...xa.com>, Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Cc: linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 Viken Dadhaniya <viken.dadhaniya@....qualcomm.com>,
 Ram Kumar Dwivedi <quic_rdwivedi@...cinc.com>
Subject: Re: [PATCH DNM v2 5/5] arm64: dts: qcom: qcs6490-radxa-dragon-q6a:
 Enable USB 3.0 and HDMI ports

On 15/09/2025 09:04, Xilin Wu wrote:
> On 9/15/2025 2:51 PM, Neil Armstrong wrote:
>> On 14/09/2025 17:57, Xilin Wu wrote:
>>> This board doesn't feature a regular Type-C port. The usb_1_qmpphy's
>>> RX1/TX1 pair is statically connected to the USB-A port, while its RX0/TX0
>>> pair is connected to the RA620 DP-to-HDMI bridge.
>>>
>>> Add and enable the nodes for the features to work.
>>>
>>> Signed-off-by: Xilin Wu <sophon@...xa.com>
>>>
>>> ---
>>>
>>> This change depends on the following patch series:
>>> https://lore.kernel.org/all/20250908-topic-x1e80100-hdmi-v3-4- c53b0f2bc2fb@...aro.org/
>>> ---
>>>   .../boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts     | 152 +++++++++++ ++++++++++
>>>   1 file changed, 152 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts b/ arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts
>>> index 3bf85d68c97891db1f1f0b84fb5649803948e06f..12bc9a0fcfbfeaabf6ede351f96c61193a8261c0 100644
>>> --- a/arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts
>>> +++ b/arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts
>>> @@ -78,6 +78,71 @@ chosen {
>>>           stdout-path = "serial0:115200n8";
>>>       };
>>> +    usb3_con: connector {
>>> +        compatible = "usb-a-connector";
>>> +
>>> +        ports {
>>> +            #address-cells = <1>;
>>> +            #size-cells = <0>;
>>> +
>>> +            port@0 {
>>> +                reg = <0>;
>>> +
>>> +                usb3_con_hs_in: endpoint {
>>> +                    remote-endpoint = <&usb_1_dwc3_hs>;
>>> +                };
>>> +            };
>>> +
>>> +            port@1 {
>>> +                reg = <1>;
>>> +
>>> +                usb3_con_ss_in: endpoint {
>>> +                    remote-endpoint = <&usb_dp_qmpphy_out_usb>;
>>> +                };
>>> +            };
>>> +        };
>>> +    };
>>> +
>>> +    hdmi-bridge {
>>> +        compatible = "radxa,ra620";
>>> +
>>> +        pinctrl-0 = <&dp_hot_plug_det>;
>>> +        pinctrl-names = "default";
>>> +
>>> +        ports {
>>> +            #address-cells = <1>;
>>> +            #size-cells = <0>;
>>> +
>>> +            port@0 {
>>> +                reg = <0>;
>>> +
>>> +                hdmi_bridge_in: endpoint {
>>> +                    remote-endpoint = <&usb_dp_qmpphy_out_dp>;
>>> +                };
>>> +            };
>>> +
>>> +            port@1 {
>>> +                reg = <1>;
>>> +
>>> +                hdmi_bridge_out: endpoint {
>>> +                    remote-endpoint = <&hdmi_connector_in>;
>>> +                };
>>> +            };
>>> +        };
>>> +    };
>>> +
>>> +    hdmi-connector {
>>> +        compatible = "hdmi-connector";
>>> +        label = "hdmi";
>>> +        type = "a";
>>> +
>>> +        port {
>>> +            hdmi_connector_in: endpoint {
>>> +                remote-endpoint = <&hdmi_bridge_out>;
>>> +            };
>>> +        };
>>> +    };
>>> +
>>>       leds {
>>>           compatible = "gpio-leds";
>>> @@ -504,6 +569,21 @@ &lpass_va_macro {
>>>       status = "okay";
>>>   };
>>> +&mdss {
>>> +    status = "okay";
>>> +};
>>> +
>>> +&mdss_dp {
>>> +    sound-name-prefix = "Display Port0";
>>> +
>>> +    status = "okay";
>>> +};
>>> +
>>> +&mdss_dp_out {
>>> +    data-lanes = <0 1>;
>>> +    remote-endpoint = <&usb_dp_qmpphy_dp_in>;
>>> +};
>>> +
>>>   &pcie0 {
>>>       perst-gpios = <&tlmm 87 GPIO_ACTIVE_LOW>;
>>>       wake-gpios = <&tlmm 89 GPIO_ACTIVE_HIGH>;
>>> @@ -753,6 +833,22 @@ platform {
>>>               sound-dai = <&q6apm>;
>>>           };
>>>       };
>>> +
>>> +    dp0-dai-link {
>>> +        link-name = "DP0 Playback";
>>> +
>>> +        codec {
>>> +            sound-dai = <&mdss_dp>;
>>> +        };
>>> +
>>> +        cpu {
>>> +            sound-dai = <&q6apmbedai DISPLAY_PORT_RX_0>;
>>> +        };
>>> +
>>> +        platform {
>>> +            sound-dai = <&q6apm>;
>>> +        };
>>> +    };
>>>   };
>>>   /* Pin 11, 29, 31, 32 in GPIO header */
>>> @@ -967,6 +1063,58 @@ &ufs_mem_phy {
>>>       status = "okay";
>>>   };
>>> +&usb_1 {
>>> +    dr_mode = "host";
>>> +
>>> +    status = "okay";
>>> +};
>>> +
>>> +&usb_1_dwc3_hs {
>>> +    remote-endpoint = <&usb3_con_hs_in>;
>>> +};
>>> +
>>> +&usb_1_hsphy {
>>> +    vdda-pll-supply = <&vreg_l10c_0p88>;
>>> +    vdda33-supply = <&vreg_l2b_3p072>;
>>> +    vdda18-supply = <&vreg_l1c_1p8>;
>>> +
>>> +    status = "okay";
>>> +};
>>> +
>>> +&usb_1_qmpphy {
>>> +    vdda-phy-supply = <&vreg_l6b_1p2>;
>>> +    vdda-pll-supply = <&vreg_l1b_0p912>;
>>> +
>>> +    /delete-property/ orientation-switch;
>>> +
>>> +    status = "okay";
>>> +
>>> +    ports {
>>> +        port@0 {
>>> +            #address-cells = <1>;
>>> +            #size-cells = <0>;
>>> +
>>> +            /delete-node/ endpoint;
>>> +
>>> +            /* RX1/TX1 is statically connected to USB-A port */
>>> +            usb_dp_qmpphy_out_usb: endpoint@0 {
>>> +                reg = <0>;
>>> +
>>> +                data-lanes = <2 3>;
>>> +                remote-endpoint = <&usb3_con_ss_in>;
>>> +            };
>>> +
>>> +            /* RX0/TX0 is statically connected to RA620 bridge */
>>> +            usb_dp_qmpphy_out_dp: endpoint@1 {
>>> +                reg = <1>;
>>> +
>>> +                data-lanes = <3 2>;
>>> +                remote-endpoint = <&hdmi_bridge_in>;
>>> +            };
>>
>> In this WiP patchset endpoint@0 is suposed to be DisplayPort, and endpoint@1 for USB3 lanes.
>>
>> And you must not have colliding data-lanes, so it should be something like:
>>
>>              /* DP0/DP1 is statically connected to a RA620 bridge*/
>>              usb_dp_qmpphy_out_dp: endpoint@0 {
>>                  reg = <0>;
>>
>>                  data-lanes = <0 1>;
>>                  remote-endpoint = <&hdmi_bridge_in>;
>>              };
>>
>>              /* RX0/TX0 is statically connected to an USB-A Connector */
>>              usb_dp_qmpphy_out_usb: endpoint@1 {
>>                  reg = <1>;
>>
>>                  data-lanes = <2 3>;
>>                  remote-endpoint = <&usb3_con_ss_in>;
>>              };
>>
>> But I just found out while reviewed my patchset is wrong... it should be:
>>
>> +          endpoint@0:
>> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
>> +            description: Display Port Output lanes of the PHY when used with static mapping
>> +            unevaluatedProperties: false
>> +
>> +            properties:
>> +              data-lanes:
>> +                $ref: /schemas/types.yaml#/definitions/uint32-array
>> +                minItems: 2
>> +                maxItems: 4
>> +                oneOf:
>> +                  - items: # DisplayPort 2 lanes, normal orientation
>> +                      - const: 3
>> +                      - const: 2
>> +                  - items: # DisplayPort 2 lanes, flipped orientation
>> +                      - const: 0
>> +                      - const: 1
>> +                  - items: # DisplayPort 4 lanes, normal orientation
>> +                      - const: 0
>> +                      - const: 1
>> +                      - const: 2
>> +                      - const: 3
>> +                  - items: # DisplayPort 4 lanes, flipped orientation
>> +                      - const: 3
>> +                      - const: 2
>> +                      - const: 1
>> +                      - const: 0
>>
>> and in driver:
>> +static const u32 dp_2_data_lanes_mapping[][2] = {
>> +    [TYPEC_ORIENTATION_NORMAL] = { 3, 2 },
>> +    [TYPEC_ORIENTATION_REVERSE] = { 0, 1 },
>>
>> Neil
> 
> The driver change in the WIP patchset assumes endpoint@0 to be USB, I forgot to mention that. Either the driver or the binding needs to be fixed indeed.

Damn you're right, another thing to fix.


> 
> And I think there's another mistake in the driver:
> 
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -4221,7 +4221,7 @@ static int qmp_combo_probe(struct platform_device *pdev)
>                                  if (!memcmp(data_lanes, usb3_data_lane_mapping[i], sizeof(u32) * 2))
>                                          break;
> 
> -                       if (i >= TYPEC_ORIENTATION_REVERSE)
> +                       if (i > TYPEC_ORIENTATION_REVERSE)
>                                  /* Property value is invalid, ignore property */
>                                  goto usb3_mapping_done;
> 
> @@ -4265,7 +4265,7 @@ static int qmp_combo_probe(struct platform_device *pdev)
>                                          break;
>                          }
> 
> -                       if (i >= TYPEC_ORIENTATION_REVERSE)
> +                       if (i > TYPEC_ORIENTATION_REVERSE)
>                                  /* Property value is invalid, ignore property */
>                                  goto dp_mapping_done;
> 
> 
> After fixing this, the driver works properly with my DT at least.

Ack, thanks for the feedback, I'll post a new version ASAP with all fixed.

Neil

> 
>>
>>   +        };
>>> +    };
>>> +};
>>> +
>>>   &usb_2 {
>>>       dr_mode = "host";
>>> @@ -986,6 +1134,10 @@ &venus {
>>>   };
>>>   /* PINCTRL - additions to nodes defined in sc7280.dtsi */
>>> +&dp_hot_plug_det {
>>> +    bias-disable;
>>> +};
>>> +
>>>   &pcie0_clkreq_n {
>>>       bias-pull-up;
>>>       drive-strength = <2>;
>>>
>>
>>
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ