mbox series

[v2,0/4] arm64: dts: rockchip: add usb3 support to rk356x

Message ID 20220225131602.2283499-1-michael.riesch@wolfvision.net
Headers show
Series arm64: dts: rockchip: add usb3 support to rk356x | expand

Message

Michael Riesch Feb. 25, 2022, 1:15 p.m. UTC
Hi all,

This series introduces the USB 3.0 xHCI controller nodes and enables
them on the RK3568 EVB1. USB 3.0 host and gadget operation have been
tested successfully.

The second version of the series considers Johan's comments which
helped a lot to clean things up and get rid of the dtbs_check issues.

Looking forward to your comments!

Best regards,
Michael

Changes since v1:
- move power domain PD_PIPE to rk356x
- add rockchip,rk3568-dwc3 compatible to documentation
- merge subnodes with corresponding nodes
- remove all quirks (add at a later stage if required)

Michael Riesch (4):
  arm64: dts: rockchip: move power domain PD_PIPE to rk356x
  dt-bindings: usb: add rk3568 compatible to rockchip,dwc3
  arm64: dts: rockchip: add the usb3 nodes to rk356x
  arm64: dts: rockchip: add usb3 support to rk3568-evb1-v10

 .../bindings/usb/rockchip,dwc3.yaml           |  2 +
 .../boot/dts/rockchip/rk3568-evb1-v10.dts     | 46 +++++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3568.dtsi      | 17 ++-----
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      | 44 ++++++++++++++++++
 4 files changed, 95 insertions(+), 14 deletions(-)

Comments

Robin Murphy Feb. 25, 2022, 2:14 p.m. UTC | #1
On 2022-02-25 13:15, Michael Riesch wrote:
> The power domain PD_PIPE was moved to the RK3568 specific dtsi but
> is available on the RK3566 as well. Move it back to the shared dtsi.

Note that a corresponding definition does already exist in rk3568.dtsi. 
That one *could* inherit the base definition and only override the 
"pm_qos" property, but looking back to the original patch series it 
seems like not doing that was a deliberate choice.

Robin.

> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>   arch/arm64/boot/dts/rockchip/rk3568.dtsi | 16 ----------------
>   arch/arm64/boot/dts/rockchip/rk356x.dtsi | 14 ++++++++++++++
>   2 files changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> index 91a0b798b857..ecc0f3015915 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> @@ -100,19 +100,3 @@ opp-1992000000 {
>   		opp-microvolt = <1150000 1150000 1150000>;
>   	};
>   };
> -
> -&power {
> -	power-domain@RK3568_PD_PIPE {
> -		reg = <RK3568_PD_PIPE>;
> -		clocks = <&cru PCLK_PIPE>;
> -		pm_qos = <&qos_pcie2x1>,
> -			 <&qos_pcie3x1>,
> -			 <&qos_pcie3x2>,
> -			 <&qos_sata0>,
> -			 <&qos_sata1>,
> -			 <&qos_sata2>,
> -			 <&qos_usb3_0>,
> -			 <&qos_usb3_1>;
> -		#power-domain-cells = <0>;
> -	};
> -};
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index 8b9fae3d348a..742f5adcdf2b 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -490,6 +490,20 @@ power-domain@RK3568_PD_RKVENC {
>   					 <&qos_rkvenc_wr_m0>;
>   				#power-domain-cells = <0>;
>   			};
> +
> +			power-domain@RK3568_PD_PIPE {
> +				reg = <RK3568_PD_PIPE>;
> +				clocks = <&cru PCLK_PIPE>;
> +				pm_qos = <&qos_pcie2x1>,
> +					 <&qos_pcie3x1>,
> +					 <&qos_pcie3x2>,
> +					 <&qos_sata0>,
> +					 <&qos_sata1>,
> +					 <&qos_sata2>,
> +					 <&qos_usb3_0>,
> +					 <&qos_usb3_1>;
> +				#power-domain-cells = <0>;
> +			};
>   		};
>   	};
>
Michael Riesch Feb. 25, 2022, 3:43 p.m. UTC | #2
Hi Robin,

On 2/25/22 15:14, Robin Murphy wrote:
> On 2022-02-25 13:15, Michael Riesch wrote:
>> The power domain PD_PIPE was moved to the RK3568 specific dtsi but
>> is available on the RK3566 as well. Move it back to the shared dtsi.
> 
> Note that a corresponding definition does already exist in rk3568.dtsi.
> That one *could* inherit the base definition and only override the
> "pm_qos" property, but looking back to the original patch series it
> seems like not doing that was a deliberate choice.

OK, my bad. I overlooked that both rk3566 and rk3568 have their
definition in their dtsi files, so there is no need for this patch.
Sorry for the confusion!

Best regards,
Michael

> 
> Robin.
> 
>> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
>> ---
>>   arch/arm64/boot/dts/rockchip/rk3568.dtsi | 16 ----------------
>>   arch/arm64/boot/dts/rockchip/rk356x.dtsi | 14 ++++++++++++++
>>   2 files changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> index 91a0b798b857..ecc0f3015915 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> @@ -100,19 +100,3 @@ opp-1992000000 {
>>           opp-microvolt = <1150000 1150000 1150000>;
>>       };
>>   };
>> -
>> -&power {
>> -    power-domain@RK3568_PD_PIPE {
>> -        reg = <RK3568_PD_PIPE>;
>> -        clocks = <&cru PCLK_PIPE>;
>> -        pm_qos = <&qos_pcie2x1>,
>> -             <&qos_pcie3x1>,
>> -             <&qos_pcie3x2>,
>> -             <&qos_sata0>,
>> -             <&qos_sata1>,
>> -             <&qos_sata2>,
>> -             <&qos_usb3_0>,
>> -             <&qos_usb3_1>;
>> -        #power-domain-cells = <0>;
>> -    };
>> -};
>> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
>> index 8b9fae3d348a..742f5adcdf2b 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
>> @@ -490,6 +490,20 @@ power-domain@RK3568_PD_RKVENC {
>>                        <&qos_rkvenc_wr_m0>;
>>                   #power-domain-cells = <0>;
>>               };
>> +
>> +            power-domain@RK3568_PD_PIPE {
>> +                reg = <RK3568_PD_PIPE>;
>> +                clocks = <&cru PCLK_PIPE>;
>> +                pm_qos = <&qos_pcie2x1>,
>> +                     <&qos_pcie3x1>,
>> +                     <&qos_pcie3x2>,
>> +                     <&qos_sata0>,
>> +                     <&qos_sata1>,
>> +                     <&qos_sata2>,
>> +                     <&qos_usb3_0>,
>> +                     <&qos_usb3_1>;
>> +                #power-domain-cells = <0>;
>> +            };
>>           };
>>       };
>>
Johan Jonker Feb. 25, 2022, 4:42 p.m. UTC | #3
Hi Michael,

Some more comments. Have a look if it's useful.

On 2/25/22 14:15, Michael Riesch wrote:
> The power domain PD_PIPE was moved to the RK3568 specific dtsi but
> is available on the RK3566 as well. Move it back to the shared dtsi.
> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  arch/arm64/boot/dts/rockchip/rk3568.dtsi | 16 ----------------
>  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 14 ++++++++++++++
>  2 files changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> index 91a0b798b857..ecc0f3015915 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> @@ -100,19 +100,3 @@ opp-1992000000 {
>  		opp-microvolt = <1150000 1150000 1150000>;
>  	};
>  };

> -
> -&power {
> -	power-domain@RK3568_PD_PIPE {
> -		reg = <RK3568_PD_PIPE>;
> -		clocks = <&cru PCLK_PIPE>;

Should contain a complete list of rk3568 clocks for which RK3568_PD_PIPE
must be enabled.

Could someone check which we need here?
Same for rk3566 but then reduced.

<&cru PCLK_PIPE>,

<&cru PCLK_XPCS>,

<&cru CLK_USB3OTG0_REF>,
<&cru CLK_USB3OTG0_SUSPEND>,
<&cru ACLK_USB3OTG0>,

<&cru CLK_USB3OTG1_REF>,
<&cru CLK_USB3OTG1_SUSPEND>,
<&cru ACLK_USB3OTG1>,

<&cru ACLK_SATA0>,
<&cru CLK_SATA0_PMALIVE>,
<&cru CLK_SATA0_RXOOB>

<&cru ACLK_SATA1>,
<&cru CLK_SATA1_PMALIVE>,
<&cru CLK_SATA1_RXOOB>,

<&cru ACLK_SATA2>,
<&cru CLK_SATA2_PMALIVE>,
<&cru CLK_SATA2_RXOOB>

<&cru ACLK_PCIE20_MST>,
<&cru ACLK_PCIE20_SLV>,
<&cru ACLK_PCIE20_DBI>,
<&cru PCLK_PCIE20>,
<&cru CLK_PCIE20_AUX_NDFT>

<&cru ACLK_PCIE30X1_MST>,
<&cru ACLK_PCIE30X1_SLV>,
<&cru ACLK_PCIE30X1_DBI>,
<&cru PCLK_PCIE30X1>,
<&cru CLK_PCIE30X1_AUX_NDFT>

<&cru ACLK_PCIE30X2_MST>,
<&cru ACLK_PCIE30X2_SLV>,
<&cru ACLK_PCIE30X2_DBI>,
<&cru PCLK_PCIE30X2>,
<&cru CLK_PCIE30X2_AUX_NDFT>;

> -		pm_qos = <&qos_pcie2x1>,
> -			 <&qos_pcie3x1>,
> -			 <&qos_pcie3x2>,
> -			 <&qos_sata0>,
> -			 <&qos_sata1>,
> -			 <&qos_sata2>,
> -			 <&qos_usb3_0>,
> -			 <&qos_usb3_1>;
> -		#power-domain-cells = <0>;
> -	};

Maybe keep it here for rk3568.

> -};
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index 8b9fae3d348a..742f5adcdf2b 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -490,6 +490,20 @@ power-domain@RK3568_PD_RKVENC {
>  					 <&qos_rkvenc_wr_m0>;
>  				#power-domain-cells = <0>;
>  			};

> +
> +			power-domain@RK3568_PD_PIPE {
> +				reg = <RK3568_PD_PIPE>;

> +				clocks = <&cru PCLK_PIPE>;
> +				pm_qos = <&qos_pcie2x1>,
> +					 <&qos_pcie3x1>,
> +					 <&qos_pcie3x2>,
> +					 <&qos_sata0>,
> +					 <&qos_sata1>,
> +					 <&qos_sata2>,
> +					 <&qos_usb3_0>,
> +					 <&qos_usb3_1>;
> +				#power-domain-cells = <0>;
> +			};

rk3566 doesn't have a combphy0
Already in rk3566.dtsi

&power {
	power-domain@RK3568_PD_PIPE {
		reg = <RK3568_PD_PIPE>;

		clocks = <&cru PCLK_PIPE>;


Should contain a complete list of rk3566 clocks for which RK3568_PD_PIPE
must be enabled.

		pm_qos = <&qos_pcie2x1>,
			 <&qos_sata1>,
			 <&qos_sata2>,

			 <&qos_usb3_0>,

Does rk3566 have a qos_usb3_0 ??
See support list below.

			 <&qos_usb3_1>;
		#power-domain-cells = <0>;
	};
};

===

Rockchip RK3568 Datasheet V1.0-20201210.pdf page 16

Multi-PHY0 support one of the following interfaces
USB3.0 OTG
SATA0

Multi-PHY1 support one of the following interfaces
USB3.0 Host
SATA1
QSGMII/SGMII

Multi-PHY2 support one of the following interfaces
PCIe2.1
SATA2
QSGMII/SGMII

===

Rockchip RK3566 Datasheet V1.0-20201210.pdf page 16

Multi-PHY1 support one of the following interfaces
USB3.0 Host
SATA1

Multi-PHY2 support one of the following interfaces
PCIe2.1
SATA2

===

https://eji4evk5kxx.exactdn.com/wp-content/uploads/2020/12/RK3568-multiplexed-sata-usb-3.0-pcie.jpg?lossy=1&ssl=1


On rk3568:
&usb_host0_xhci {
	phys = <&usb2phy0_otg>, <&combphy0 PHY_TYPE_USB3>;
	phy-names = "usb2-phy", "usb3-phy";
};

Does this exists on rk3566?

&usb_host0_xhci {
	phys = <&usb2phy0_otg>;
	phy-names = "usb2-phy";
};

If not then why is usb_host0_xhci in a common rk356x.dtsi ??
Else fix rk3566.dtsi

===

Johan



>  		};
>  	};
>