Re: [PATCH v2 3/3] arm64: dts: rockchip: fix the pcie refclock oscillator on Rock 5 ITX

Dragan Simic
2024-07-18T07:32:42Z

Hello Anand,

On 2024-07-18 09:26, Anand Moon wrote:
> On Mon, 15 Jul 2024 at 16:35, Heiko Stuebner <heiko@sntech.de> wrote:
>>
>> The Rock 5 ITX uses two PCIe controllers to drive both a M.2 slot and
>> its
>> SATA controller with 2 lanes each. The supply for the refclk
>> oscillator is
>> the same that supplies the M.2 slot, but the SATA controller port is
>> supplied by a different rail.
>>
>> This leads to the effect that if the PCIe30x4 controller for the M.2
>> probes first, everything works normally. But if the PCIe30x2
>> controller
>> that is connected to the SATA controller probes first, it will hang on
>> the first DBI read as nothing will have enabled the refclock before.
>
> I just checked the rk3588-rock-5-itx.dts in the linux-next.
> You have not enabled sata0 and sata2, which might be the problem
> for the SATA controller not getting initialized.

Rock 5 ITX doesn't use RK5588's built-in SATA interfaces, so that's
fine.
Please have a look at the board schematic, it uses a separate PCI
Express
SATA controller for its four SATA ports.

>> Fix this by describing the clock generator with its supplies so that
>> both controllers can reference it as needed.
>>
>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>> ---
>> .../boot/dts/rockchip/rk3588-rock-5-itx.dts | 38
>> ++++++++++++++++++-
>> 1 file changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
>> b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
>> index d0b922b8d67e8..37bc53f2796fc 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
>> @@ -72,6 +72,15 @@ hdd-led2 {
>> };
>> };
>>
>> + /* Unnamed voltage oscillator: 100MHz,3.3V,3225 */
>> + pcie30_port0_refclk: pcie30_port1_refclk:
>> pcie-voltage-oscillator {
>> + compatible = "voltage-oscillator";
>> + #clock-cells = <0>;
>> + clock-frequency = <100000000>;
>> + clock-output-names = "pcie30_refclk";
>> + vdd-supply = <&vcc3v3_pi6c_05>;
>> + };
>> +
>> fan0: pwm-fan {
>> compatible = "pwm-fan";
>> #cooling-cells = <2>;
>> @@ -146,13 +155,14 @@ vcc3v3_lan: vcc3v3_lan_phy2:
>> regulator-vcc3v3-lan {
>> vin-supply = <&vcc_3v3_s3>;
>> };
>>
>> - vcc3v3_mkey: regulator-vcc3v3-mkey {
>> + /* The PCIE30x4_PWREN_H controls two regulators */
>> + vcc3v3_mkey: vcc3v3_pi6c_05: regulator-vcc3v3-pi6c-05 {
>> compatible = "regulator-fixed";
>> enable-active-high;
>> gpios = <&gpio1 RK_PA4 GPIO_ACTIVE_HIGH>;
>> pinctrl-names = "default";
>> pinctrl-0 = <&pcie30x4_pwren_h>;
>> - regulator-name = "vcc3v3_mkey";
>> + regulator-name = "vcc3v3_pi6c_05";
>> regulator-min-microvolt = <3300000>;
>> regulator-max-microvolt = <3300000>;
>> startup-delay-us = <5000>;
>> @@ -513,6 +523,18 @@ &pcie30phy {
>>
>> /* ASMedia ASM1164 Sata controller */
>> &pcie3x2 {
>> + /*
>> + * The board has a "pcie_refclk" oscillator that needs
>> enabling,
>> + * so add it to the list of clocks.
>> + */
>> + clocks = <&cru ACLK_PCIE_2L_MSTR>, <&cru ACLK_PCIE_2L_SLV>,
>> + <&cru ACLK_PCIE_2L_DBI>, <&cru PCLK_PCIE_2L>,
>> + <&cru CLK_PCIE_AUX1>, <&cru CLK_PCIE2L_PIPE>,
>> + <&pcie30_port1_refclk>;
>> + clock-names = "aclk_mst", "aclk_slv",
>> + "aclk_dbi", "pclk",
>> + "aux", "pipe",
>> + "ref";
>> pinctrl-names = "default";
>> pinctrl-0 = <&pcie30x2_perstn_m1_l>;
>> reset-gpios = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
>> @@ -522,6 +544,18 @@ &pcie3x2 {
>>
>> /* M.2 M.key */
>> &pcie3x4 {
>> + /*
>> + * The board has a "pcie_refclk" oscillator that needs
>> enabling,
>> + * so add it to the list of clocks.
>> + */
>> + clocks = <&cru ACLK_PCIE_4L_MSTR>, <&cru ACLK_PCIE_4L_SLV>,
>> + <&cru ACLK_PCIE_4L_DBI>, <&cru PCLK_PCIE_4L>,
>> + <&cru CLK_PCIE_AUX0>, <&cru CLK_PCIE4L_PIPE>,
>> + <&pcie30_port0_refclk>;
>> + clock-names = "aclk_mst", "aclk_slv",
>> + "aclk_dbi", "pclk",
>> + "aux", "pipe",
>> + "ref";
>> num-lanes = <2>;
>> pinctrl-names = "default";
>> pinctrl-0 = <&pcie30x4_perstn_m1_l>;
>> --
>> 2.39.2