diff mbox series

[V3,5/7] arm64: dts: qcom: sc7280: Add EUD dt node and dwc3 connector

Message ID baf41480be71bd4472fe09c2993e43a780554d94.1641288286.git.quic_schowdhu@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Add Embedded USB Debugger (EUD) driver | expand

Commit Message

Souradeep Chowdhury Jan. 4, 2022, 11:58 a.m. UTC
Add the Embedded USB Debugger(EUD) device tree node. The
node contains EUD base register region and EUD mode
manager register regions along with the interrupt entry.
Also add the connector to EUD which is mapped as the child
of dwc3. The connector is attached to EUD via port. Also add
the role-switch property to dwc3 node.

Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Comments

Dmitry Baryshkov Jan. 4, 2022, 7:24 p.m. UTC | #1
On Tue, 4 Jan 2022 at 15:00, Souradeep Chowdhury
<quic_schowdhu@quicinc.com> wrote:
>
> Add the Embedded USB Debugger(EUD) device tree node. The
> node contains EUD base register region and EUD mode
> manager register regions along with the interrupt entry.
> Also add the connector to EUD which is mapped as the child
> of dwc3. The connector is attached to EUD via port. Also add
> the role-switch property to dwc3 node.
>
> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 53a21d0..2d14e5c 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1315,6 +1315,18 @@
>                                 phys = <&usb_2_hsphy>;
>                                 phy-names = "usb2-phy";
>                                 maximum-speed = "high-speed";
> +                               usb-role-switch;
> +                               usb_con: eud_usb_connector {

Is this node meant to be named connector instead?

> +                                       compatible = "qcom,usb-connector-eud",
> +                                                    "usb-c-connector";
> +                                       ports {
> +                                               port@0 {
> +                                                       usb2_role_switch: endpoint {
> +                                                               remote-endpoint = <&eud_ep>;
> +                                                       };
> +                                               };
> +                                       };
> +                               };
>                         };
>                 };
>
> @@ -1339,6 +1351,19 @@
>                         interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
>                 };
>
> +               eud:  eud@88e0000 {
> +                       compatible = "qcom,sc7280-eud","qcom,eud";
> +                       reg = <0 0x88e0000 0 0x2000>,
> +                             <0 0x88e2000 0 0x1000>;
> +                       interrupt-parent = <&pdc>;
> +                       interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
> +                       port {
> +                               eud_ep: endpoint {
> +                                       remote-endpoint = <&usb2_role_switch>;
> +                               };
> +                       };
> +               };
> +
>                 nsp_noc: interconnect@a0c0000 {
>                         reg = <0 0x0a0c0000 0 0x10000>;
>                         compatible = "qcom,sc7280-nsp-noc";
> --
> 2.7.4
>
Bjorn Andersson Jan. 7, 2022, 12:53 a.m. UTC | #2
On Tue 04 Jan 03:58 PST 2022, Souradeep Chowdhury wrote:

> Add the Embedded USB Debugger(EUD) device tree node. The
> node contains EUD base register region and EUD mode
> manager register regions along with the interrupt entry.
> Also add the connector to EUD which is mapped as the child
> of dwc3. The connector is attached to EUD via port. Also add
> the role-switch property to dwc3 node.
> 
> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 53a21d0..2d14e5c 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1315,6 +1315,18 @@
>  				phys = <&usb_2_hsphy>;
>  				phy-names = "usb2-phy";
>  				maximum-speed = "high-speed";
> +				usb-role-switch;
> +				usb_con: eud_usb_connector {

I think this is backwards.

Iiuc you have dwc3 <-> EUD <-> connector, where the connector typically
would be represented in relation to a Type-C controller.

So I think this graph should be looking something like:


dwc3 {
    ports {
        port {
            dwc3_eud: endpoint {
                remote-endpoint = <&eud_ep>;
            };
        };
    };
};

eud {
    ports {
        port {
            eud_dwc3: endpoint {
                remote-endpoint = <&dwc3_eud>;
            };
        };

        port {
            eud_con: endpoint {
                remote-endpoint = <&con_eud>;
            };
        };
    };
};

connector {
    compatible = "usb-c-connector";

    ports {
        port {
            con_eud: endpoint {
                remote-endpoint = <&eud_con>;
            };
        };
    };
};

With EUD "forwarding" the role switching requests from the connector, or
overriding them if the EUD is enabled.

Alternatively the dwc3 would be in a graph with both the eud and the
connector, but I don't know how to ensure that the EUD decision takes
precedence over the type-c controllers role request...


In the case of us having a type-c controller, such as the pmic_glink,
the connector node would then be a child of the controller.

Regards,
Bjorn

> +					compatible = "qcom,usb-connector-eud",
> +						     "usb-c-connector";
> +					ports {
> +						port@0 {
> +							usb2_role_switch: endpoint {
> +								remote-endpoint = <&eud_ep>;
> +							};
> +						};
> +					};
> +				};
>  			};
>  		};
>  
> @@ -1339,6 +1351,19 @@
>  			interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
>  		};
>  
> +		eud:  eud@88e0000 {
> +			compatible = "qcom,sc7280-eud","qcom,eud";
> +			reg = <0 0x88e0000 0 0x2000>,
> +			      <0 0x88e2000 0 0x1000>;
> +			interrupt-parent = <&pdc>;
> +			interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
> +			port {
> +				eud_ep: endpoint {
> +					remote-endpoint = <&usb2_role_switch>;
> +				};
> +			};
> +		};
> +
>  		nsp_noc: interconnect@a0c0000 {
>  			reg = <0 0x0a0c0000 0 0x10000>;
>  			compatible = "qcom,sc7280-nsp-noc";
> -- 
> 2.7.4
>
Souradeep Chowdhury Jan. 17, 2022, 5:07 a.m. UTC | #3
On 1/7/2022 6:23 AM, Bjorn Andersson wrote:
> On Tue 04 Jan 03:58 PST 2022, Souradeep Chowdhury wrote:
>
>> Add the Embedded USB Debugger(EUD) device tree node. The
>> node contains EUD base register region and EUD mode
>> manager register regions along with the interrupt entry.
>> Also add the connector to EUD which is mapped as the child
>> of dwc3. The connector is attached to EUD via port. Also add
>> the role-switch property to dwc3 node.
>>
>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/sc7280.dtsi | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 53a21d0..2d14e5c 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -1315,6 +1315,18 @@
>>   				phys = <&usb_2_hsphy>;
>>   				phy-names = "usb2-phy";
>>   				maximum-speed = "high-speed";
>> +				usb-role-switch;
>> +				usb_con: eud_usb_connector {
> I think this is backwards.
>
> Iiuc you have dwc3 <-> EUD <-> connector, where the connector typically
> would be represented in relation to a Type-C controller.
>
> So I think this graph should be looking something like:
>
>
> dwc3 {
>      ports {
>          port {
>              dwc3_eud: endpoint {
>                  remote-endpoint = <&eud_ep>;
>              };
>          };
>      };
> };
>
> eud {
>      ports {
>          port {
>              eud_dwc3: endpoint {
>                  remote-endpoint = <&dwc3_eud>;
>              };
>          };
>
>          port {
>              eud_con: endpoint {
>                  remote-endpoint = <&con_eud>;
>              };
>          };
>      };
> };
>
> connector {
>      compatible = "usb-c-connector";
>
>      ports {
>          port {
>              con_eud: endpoint {
>                  remote-endpoint = <&eud_con>;
>              };
>          };
>      };
> };
>
> With EUD "forwarding" the role switching requests from the connector, or
> overriding them if the EUD is enabled.
>
> Alternatively the dwc3 would be in a graph with both the eud and the
> connector, but I don't know how to ensure that the EUD decision takes
> precedence over the type-c controllers role request...
>
>
> In the case of us having a type-c controller, such as the pmic_glink,
> the connector node would then be a child of the controller.
Ack. Will align the device tree with this structure.
>
> Regards,
> Bjorn
>
>> +					compatible = "qcom,usb-connector-eud",
>> +						     "usb-c-connector";
>> +					ports {
>> +						port@0 {
>> +							usb2_role_switch: endpoint {
>> +								remote-endpoint = <&eud_ep>;
>> +							};
>> +						};
>> +					};
>> +				};
>>   			};
>>   		};
>>   
>> @@ -1339,6 +1351,19 @@
>>   			interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
>>   		};
>>   
>> +		eud:  eud@88e0000 {
>> +			compatible = "qcom,sc7280-eud","qcom,eud";
>> +			reg = <0 0x88e0000 0 0x2000>,
>> +			      <0 0x88e2000 0 0x1000>;
>> +			interrupt-parent = <&pdc>;
>> +			interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
>> +			port {
>> +				eud_ep: endpoint {
>> +					remote-endpoint = <&usb2_role_switch>;
>> +				};
>> +			};
>> +		};
>> +
>>   		nsp_noc: interconnect@a0c0000 {
>>   			reg = <0 0x0a0c0000 0 0x10000>;
>>   			compatible = "qcom,sc7280-nsp-noc";
>> -- 
>> 2.7.4
>>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 53a21d0..2d14e5c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1315,6 +1315,18 @@ 
 				phys = <&usb_2_hsphy>;
 				phy-names = "usb2-phy";
 				maximum-speed = "high-speed";
+				usb-role-switch;
+				usb_con: eud_usb_connector {
+					compatible = "qcom,usb-connector-eud",
+						     "usb-c-connector";
+					ports {
+						port@0 {
+							usb2_role_switch: endpoint {
+								remote-endpoint = <&eud_ep>;
+							};
+						};
+					};
+				};
 			};
 		};
 
@@ -1339,6 +1351,19 @@ 
 			interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
+		eud:  eud@88e0000 {
+			compatible = "qcom,sc7280-eud","qcom,eud";
+			reg = <0 0x88e0000 0 0x2000>,
+			      <0 0x88e2000 0 0x1000>;
+			interrupt-parent = <&pdc>;
+			interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
+			port {
+				eud_ep: endpoint {
+					remote-endpoint = <&usb2_role_switch>;
+				};
+			};
+		};
+
 		nsp_noc: interconnect@a0c0000 {
 			reg = <0 0x0a0c0000 0 0x10000>;
 			compatible = "qcom,sc7280-nsp-noc";