Re: [PATCH 3/3] arm64: dts: qcom: shikra: Wire up usb-role-switch for USB Type-C ports

On 26-08-20 20:17:38, Akash Kumar wrote:
> On Shikra CQS/CQM platforms, usb-role-switch is handled by PM4125 on
> the primary Type-C port and Cypress PD controller CYPD6129 on the
> second Type-C port. On Shikra IQS platform, usb-role-switch is
> handled by Cypress PD controller CYPD6129 on both Type-C ports.
> 
> Add the CYPD6129 typec node under i2c3, wire its connector endpoints
> to the corresponding DWC3 controller ports via remote-endpoint
> phandles, and switch the associated USB controllers to OTG mode so
> role switching can take effect.
> 
> Signed-off-by: Akash Kumar <[email protected]>
> ---
>  arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts |  64 +++++++++++-
>  arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts |  64 +++++++++++-
>  arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts | 108 +++++++++++++++++++-
>  3 files changed, 223 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts b/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
> index 3866ab8b0faa..c92ab8e6cc0c 100644
> --- a/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
> +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
> @@ -42,6 +42,43 @@ &remoteproc_mpss {
>  	status = "okay";
>  };
>  
> +&i2c3 {
> +	status = "okay";
> +
> +	typec@40 {
> +		compatible = "cypress,cypd6129";
> +		reg = <0x40>;
> +		interrupts-extended = <&tlmm 136 IRQ_TYPE_LEVEL_LOW>;
> +		pinctrl-0 = <&usb0_intr_state>;
> +		pinctrl-names = "default";
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ccg_typec_con0: connector@0 {
> +			compatible = "usb-c-connector";
> +			reg = <0>;
> +			label = "USB-C";
> +			data-role = "dual";
> +			power-role = "dual";
> +			wakeup-source;

AFAICT, this fails dtbs_check because it isn't allowed by the
usb-connector.yaml.

I see you added it in the example the new schema you added. That
should've failed at dt_bindings_check.

Same comment for the other dtes files.

> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +
> +					ucsi_ccg_port: endpoint {
> +						remote-endpoint = <&usb_1_dwc3_hs>;
> +					};
> +				};
> +			};
> +		};

Where is the second connector?

> +	};
> +};
> +
>  &sdhc_1 {
>  	vmmc-supply = <&pm4125_l20>;
>  	vqmmc-supply = <&pm4125_l14>;
> @@ -58,6 +95,23 @@ &sdhc_1 {
>  	status = "okay";
>  };
>  
> +&tlmm {
> +	usb0_intr_state: usb0-intr-state {
> +		pins = "gpio136";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};
> +};
> +
> +&usb_1 {
> +	/delete-property/ dr_mode;

I don't think this property is actually defined anywhere. I wonder which
tree is this done on.